A strictness mod is born

.luacheckrc

@@ -0,0 +1 @@
+globals = {"strictest"; "string", "math"; "minetest", "vector", "ItemStack"} -- allow overwriting standard library funcs

License.txt

@@ -0,0 +1,7 @@
+Copyright 2022 Lars Müller
+
+Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

Readme.md

@@ -0,0 +1,23 @@
+# Strictest
+
+## Runtime Strictness for Minetest Mods
+
+*Strictest* consists of two components:
+
+* Lua strictness: Will disallow string indexing and string - number coercion.
+* Minetest strictness: Disallows usage of deprecated APIs & using entity-only or player-only methods on the wrong type of object.
+
+Particularly useful when writing new mods that don't target older Minetest versions.
+
+## Configuration
+
+`strictest.action` can be set to either `error` or `log`:
+
+* `error`: Immediately throw an error on strictness violations.
+* `log`: Merely log the error (including a stacktrace).
+
+Potentially partially redundant with the `deprecated_lua_api_handling` setting.
+
+## License
+
+Written by Lars Müller and licensed under the MIT license.

init.lua

@@ -0,0 +1,22 @@
+-- TODO arity checks (does this evolve into a type checker?)
+
+local action_setting = minetest.settings:get("strictest.action") or "log"
+
+local action
+if action_setting == "error" then
+	function action(message)
+		error(message, 2)
+	end
+else
+	assert(action_setting == "log", "invalid value for setting `strictness.action`: expected `error` or `log`")
+	function action(message)
+		minetest.log("error", debug.traceback(message, 2))
+	end
+end
+
+local function load_strictness(name)
+	return assert(loadfile(minetest.get_modpath(minetest.get_current_modname()) .. ("/%s.lua"):format(name)))(action)
+end
+
+load_strictness"lua"
+load_strictness"minetest"

lua.lua

@@ -0,0 +1,66 @@
+-- Lua strictness
+
+local action = ...
+
+local string = string
+
+-- Don't allow indexing strings to fail, returning `nil`.
+-- This may lead to mistakingly treating a string like an empty table.
+-- Does still allow indexing the global `string` table as it doesn't use `setmetatable(string, {...})`.
+
+local str_mt = getmetatable""
+assert(str_mt.__index == string)
+function str_mt.__index(_, key)
+	local func = string[key]
+	if func == nil then
+		action"attempt to index a string value"
+	end
+	return func
+end
+
+-- Completely disable string-to-number coercion
+
+local arithmetic_ops = {"add", "sub", "mul", "div", "mod", "pow", "unm"}
+for _, op in pairs(arithmetic_ops) do
+	str_mt["__" .. op] = function()
+		action"attempt to perform arithmetic on a string value"
+	end
+end
+
+-- Override string methods to reject anything that isn't a string
+for name, func in pairs(string) do
+	if not (name == "char" or name == "dump") then
+		string[name] = function(str, ...)
+			if type(str) ~= "string" then
+				action"string expected as first argument"
+			end
+			return func(str, ...)
+		end
+	end
+end
+
+local function assert_nums(...)
+	for i = 1, select("#", ...) do
+		if type(select(i, ...)) ~= "number" then
+			action"only numbers expected as arguments"
+		end
+	end
+end
+
+local string_char = string.char
+function string.char(...)
+	assert_nums(...)
+	return string_char(...)
+end
+
+-- Number-to-string coercion (f.E. `"x" .. 1`) is commonplace and considered fine
+
+-- Override math methods to reject anything that isn't a number
+for name, func in pairs(math) do
+	if type(func) == "function" then -- don't override math.pi & math.huge
+		math[name] = function(...)
+			assert_nums(...)
+			return func(...)
+		end
+	end
+end

minetest.lua

@@ -0,0 +1,215 @@
+-- Minetest strictness
+
+local action = ...
+
+-- Helpers
+
+local function deprecated(method_table, prefix, deprecations)
+	for method_name, recommended in pairs(deprecations) do
+		local original_method = method_table[method_name]
+		method_table[method_name] = function(...)
+			action(("deprecated, use `%s%s` instead"):format(prefix, recommended))
+			return original_method(...)
+		end
+	end
+end
+
+local function only_def_expected(method_table, method_name, def_name)
+	local method = method_table[method_name]
+	method_table[method_name] = function(...)
+		if select("#", ...) ~= 1 then
+			action(("only %s expected"):format(def_name))
+		end
+		return method(...)
+	end
+end
+
+-- Enforce deprecation of indexing `minetest` with `env` as key
+
+assert(not getmetatable(minetest))
+setmetatable(minetest, {__index = function(_, key)
+	if key == "env" then
+		action"`minetest.env.*` is deprecated, use just `minetest.*` instead"
+	end
+	return nil
+end})
+
+-- Throw when calling player-only methods on entities or calling entity-only methods on players
+
+local ObjRef
+local player_only = {
+	"get_player_name",
+	"get_player_velocity",
+	"add_player_velocity",
+	"get_look_dir",
+	"get_look_vertical",
+	"get_look_horizontal",
+	"set_look_vertical",
+	"set_look_horizontal",
+	"get_look_pitch",
+	"get_look_yaw",
+	"set_look_pitch",
+	"set_look_yaw",
+	"get_breath",
+	"set_breath",
+	"set_fov",
+	"get_fov",
+	"set_attribute",
+	"get_attribute",
+	"get_meta",
+	"set_inventory_formspec",
+	"get_inventory_formspec",
+	"set_formspec_prepend",
+	"get_formspec_prepend",
+	"get_player_control",
+	"get_player_control_bits",
+	"set_physics_override",
+	"get_physics_override",
+	"hud_add",
+	"hud_remove",
+	"hud_change",
+	"hud_get",
+	"hud_set_flags",
+	"hud_get_flags",
+	"hud_set_hotbar_itemcount",
+	"hud_set_hotbar_image",
+	"hud_set_hotbar_selected_image",
+	"set_minimap_modes",
+	"set_sky",
+	"set_sky",
+	"set_sky",
+	"get_sky",
+	"set_sky",
+	"get_sky_color",
+	"get_sky",
+	"set_sun",
+	"get_sun",
+	"set_moon",
+	"get_moon",
+	"set_stars",
+	"get_stars",
+	"set_clouds",
+	"get_clouds",
+	"override_day_night_ratio",
+	"get_day_night_ratio",
+	"set_local_animation",
+	"get_local_animation",
+	"set_eye_offset",
+	"get_eye_offset",
+	"send_mapblock",
+	"set_lighting",
+	"get_lighting",
+	"respawn",
+}
+local entity_only = {
+	"remove",
+	"set_velocity",
+	"set_acceleration",
+	"get_acceleration",
+	"set_rotation",
+	"get_rotation",
+	"set_yaw",
+	"get_yaw",
+	"set_texture_mod",
+	"get_texture_mod",
+	"set_sprite",
+	"get_entity_name",
+	"get_luaentity",
+}
+minetest.register_on_joinplayer(function(player)
+	-- TODO implement `textures = {itemname}` deprecation for `wielditem` drawtype
+	if ObjRef then return end
+	ObjRef = getmetatable(player)
+	-- (get|add)_player_velocity are deliberately not included here as their deprecation is still somewhat recent
+	deprecated(ObjRef, "player:", {
+		get_look_pitch = "get_look_vertical()",
+		set_look_pitch = "set_look_vertical(radians)",
+		get_look_yaw = "get_look_horizontal()",
+		set_look_yaw = "set_look_horizontal(radians)",
+		get_attribute = "get_meta()",
+		set_attribute = "get_meta()",
+		get_sky_color = "get_sky(as_table)"
+	})
+	only_def_expected(ObjRef, "set_sky", "sky params")
+	local ObjRef_get_sky = ObjRef.get_sky
+	function ObjRef:get_sky(as_table)
+		if not as_table then
+			action"deprecated call `player:get_sky(false or nil)`, use `player:get_sky(true)` instead"
+		end
+		return ObjRef_get_sky(self, deprecated)
+	end
+	for _, method in pairs(player_only) do
+		local original_method = ObjRef[method]
+		ObjRef[method] = function(self, ...)
+			if self:is_player() then
+				return original_method(self, ...)
+			end
+			action"player-only method called on entity"
+		end
+	end
+	function ObjRef.get_entity_name()
+		action"`object:get_entity_name()` is deprecated, use `object:get_luaentity().name` instead"
+	end
+	for _, method in pairs(entity_only) do
+		local original_method = ObjRef[method]
+		ObjRef[method] = function(self, ...)
+			if self:is_player() then
+				action"entity-only method called on player"
+			end
+			return original_method(self, ...)
+		end
+	end
+end)
+
+local vector_new = vector.new
+function vector.new(...)
+	local n_args = select("#", ...)
+	if n_args == 1 then
+		if type(...) ~= "number" then
+			action"number expected"
+		end
+	elseif n_args == 3 then
+		for i = 1, 3 do
+			if type(select(i, ...)) ~= "number" then
+				action"3 numbers expected"
+			end
+		end
+	else
+		action"1 or 3 args expected"
+	end
+	return vector_new(...)
+end
+
+-- Schur product/quotient deprecation is not implemented for good reason
+
+only_def_expected(_G, "PerlinNoise", "noiseparams")
+
+only_def_expected(minetest, "get_perlin", "noiseparams")
+
+only_def_expected(minetest, "add_particle", "particle def")
+
+only_def_expected(minetest, "add_particlespawner", "particle spawner def")
+
+
+deprecated(minetest, "minetest.", {
+	register_on_auth_fail = "register_on_authplayer(name, ip, is_success)",
+	get_mapgen_params = "get_mapgen_setting(name)",
+	set_mapgen_params = "set_mapgen_setting(name, value, override)",
+	item_place_object = "add_item",
+	get_node_group = "get_item_group(name, group)"
+})
+
+local ItemStackMT = getmetatable(ItemStack())
+
+deprecated(ItemStackMT, "stack:", {
+	get_metadata = "get_meta()",
+	set_metadata = "get_meta()",
+})
+
+--[[
+	TODO: implement the following deprecations:
+	- Tile def `image` field (replaced by `name`)
+	- HTTPRequest `post_data` field (replaced by `data`)
+	- Item filtering by string matching (groups should be used instead)
+	- The mapgen alias "mapgen_lava_source" (replaced by mapgen liquid params)
+]]

mod.conf

@@ -0,0 +1,4 @@
+# HACK use a double underscore to load before "all" other mods (reverse alphabetical order)
+name = __strictest
+title = Modding Strictness
+description = Vaguely inspired by JS's `"strict"` mode

settingtypes.txt

@@ -0,0 +1,2 @@
+# Which action to take when strictness is violated
+strictest.action (Strictness Violation Action) enum error error,log