From 79cb1193d8bba701186f466c840a8ad62410d65d Mon Sep 17 00:00:00 2001 From: arichard Date: Mon, 9 May 2022 10:43:03 -0700 Subject: [PATCH] Adds support for continued analysis through calls --- spec/table_field_limitations_spec.lua | 43 ----- spec/table_fields_functions_spec.lua | 199 +++++++++++++++++++++ src/luacheck/stages/check_table_fields.lua | 92 +++++++++- 3 files changed, 283 insertions(+), 51 deletions(-) create mode 100644 spec/table_fields_functions_spec.lua diff --git a/spec/table_field_limitations_spec.lua b/spec/table_field_limitations_spec.lua index c148e596..f6592bc9 100644 --- a/spec/table_field_limitations_spec.lua +++ b/spec/table_field_limitations_spec.lua @@ -112,24 +112,6 @@ end ]]) end) - it("stops checking referenced upvalues if function call is known to not have table as an upvalue", function() - assert_warnings({}, [[ -local x = {} -x[1] = 1 -local function printx() x = 1 end -local function ret2() return 2 end -ret2() -x[1] = 1 - -local y = {} -y[1] = 1 -function y.printx() y = 1 end -function y.ret2() return 2 end -y.ret2() -y[1] = 1 - ]]) - end) - it("halts checking at the end of control flow blocks with jumps", function() assert_warnings({}, [[ local x = {1} @@ -156,29 +138,4 @@ end a[1] = a[1] ]]) end) - - it("stops checking if a function is called", function() - assert_warnings({ - {line = 8, column = 3, name = 'y', end_column = 3, field = 'x', code = '315', set_is_nil = '' }, - {line = 8, column = 9, name = 'y', end_column = 9, field = 'a', code = '325', }, - {line = 14, column = 9, name = 't', end_column = 9, field = 'a', code = '325', }, - }, [[ -local x = {} -x.y = 1 -print("Unrelated text") -x.y = 2 -x[1] = x.z - -local y = {} -y.x = y.a -y.x = 1 -function y:func() return 1 end -y:func() - -local t = {} -t.x = t.a -local var = 'func' -t.x = y[var]() + 1 - ]]) - end) end) \ No newline at end of file diff --git a/spec/table_fields_functions_spec.lua b/spec/table_fields_functions_spec.lua new file mode 100644 index 00000000..84cc6577 --- /dev/null +++ b/spec/table_fields_functions_spec.lua @@ -0,0 +1,199 @@ +local helper = require "spec.helper" + +local function assert_warnings(warnings, src) + assert.same(warnings, helper.get_stage_warnings("check_table_fields", src)) +end + +describe("table field checks", function() + it("method invocation ends tracking of the base table", function() + assert_warnings({ + {code = "315", line = 27, column = 3, end_column = 3, name = 'y', field = 'y', set_is_nil = ''}, + {code = "325", line = 27, column = 9, end_column = 9, name = 'y', field = 'x'}, + }, [[ +local y = {} + +local x = {} +x.y = 1 +function x:func() print(self) end +x.y = x:func() +x.y = x.z + +local a = {} +a.y = 1 +function a:func() print(self) end +a[a:func()] = 1 +a.y = a.z + +local b = {} +b.y = 1 +function b:func() print(self) return 1 end +b[1] = a[b:func()] +b.y = b.z + +local c = {} +c.y = 1 +function c:func() print(self) return 1 end +c:func() +c.y = c.z + +y.y = y.x + ]]) + end) + + it("functions calls using a table as a whole end tracking of that table", function() + assert_warnings({ + {code = "315", line = 31, column = 3, end_column = 3, name = 'y', field = 'y', set_is_nil = ''}, + {code = "325", line = 31, column = 9, end_column = 9, name = 'y', field = 'x'}, + }, [[ +local y = {} + +local x = {} +x.y = 1 +x.y = print(x) +x.y = x.z + +local a = {} +a.y = 1 +function a:func() print(self) end +a[print(a)] = 1 +a.y = a.z + +local b = {} +b.y = 1 +function b:func() print(self) return 1 end +b[1] = a[print(b)] +b.y = b.z + +local c = {} +c.y = 1 +function c:func() print(self) return 1 end +print(c) +c.y = c.z + +local d = {} +d.y = 1 +print(print(d)) +d.y = d.z + +y.y = y.x + ]]) + end) + + it("functions calls using a table field access only that field", function() + assert_warnings({ + {code = "315", line = 1, column = 14, end_column = 14, name = 'x', field = 2, set_is_nil = ''}, + }, [[ +local x = {1,1} +print(x[1]) +x[1], x[2] = 1, 2 + +local y = {} +function y.func(var) print(var) return 1 end +y[1] = y.func(x[1]) +x[1] = 1 +y[2] = y:func(x[1]) +x[1] = 1 + +return x, y + ]]) + end) + + it("functions calls stop checking for tables accessed as upvalues", function() + assert_warnings({ + {code = "315", line = 27, column = 3, end_column = 3, name = 'y', field = 'y', set_is_nil = ''}, + }, [[ +local y = {} + +local x = {} +x.y = 1 +function x.func() print(x) end +x.y = x.func() +x.y = x.z + +local a = {} +a.y = 1 +function a.func() print(a) end +a[a.func()] = 1 +a.y = a.z + +local b = {} +b.y = 1 +function glob_func() print(b) return 1 end +b[1] = a[glob_func()] +b.y = b.z + +local c = {} +c.y = 1 +local function func() print(c) return 1 end +func() +c.y = c.z + +y.y = 1 + +return x, a, b, c + ]]) + end) + + it("handles multiple layers of nested function calls correctly", function() + assert_warnings({ + {code = "315", line = 2, column = 3, end_column = 3, name = 'x', field = 'y', set_is_nil = ''}, + }, [[ +local x = {...} +x.y = x[2] + ]]) + end) + + it("assumes that parameters and upvalues have all keys accessed or written to on a function call", function() + assert_warnings({ + }, [[ +local function other_func() end +local x = {} +function func(t) + t = {1} + x = {1} + other_func() + t[2] = t[2] + x[2] = x[2] +end + ]]) + end) + + it("stop tracking for tables passed externally", function() + assert_warnings({}, [[ +local t +local function func1(var) + t = var +end +local x = {} +func1(x) +x[1] = 1 +print(t[1]) + +local z = {} +local a +function z:func() a = self end +z:func() +z[1] = 1 +print(a[1]) + +local y +local function func2() return y end +function func3() + y = {} + local t = func2() + y[1] = 1 + print(t[1]) +end + ]]) + end) + + it("continues analysis through a function call", function() + assert_warnings({ + {line = 3, column = 12, name = 'x', end_column = 15, field = 'oops', code = '325', }, + }, [[ + local x = {} + print("Meh") + print(x.oops) + ]]) + end) +end) \ No newline at end of file diff --git a/src/luacheck/stages/check_table_fields.lua b/src/luacheck/stages/check_table_fields.lua index 9e36e6d6..4ce2cac1 100644 --- a/src/luacheck/stages/check_table_fields.lua +++ b/src/luacheck/stages/check_table_fields.lua @@ -1,4 +1,5 @@ local utils = require "luacheck.utils" +local builtin_standards = require 'luacheck.builtin_standards' local stage = {} @@ -20,9 +21,12 @@ function ClosureState:__init(chstate) -- Map from table name => table info. See new_local_table for the format. self.current_tables = {} - -- externally reference variable means that its fields could potentially all - -- be referenced externally + -- external access means that any field could be set or accessed, or arbitrary aliases set self.external_references_accessed = {} + -- after we leave the current scope for any reason, these tables can have arbitrarily different + -- fields set, but cannot have additional accesses. (This combines set, i.e. overwriting the entirely variable, + -- and mutation, i.e. writing to some fields of the table.) + self.external_references_set = {} self.max_item = 0 @@ -202,6 +206,23 @@ function ClosureState:stop_tracking_tables() end end +-- Leaving the current scope, but we may return to it later +-- We need to account for externally referenceable variables, which +-- may be modified or accessed +function ClosureState:exit_current_scope_for_func_call(node) + for table_name in pairs(self.external_references_accessed) do + if self.current_tables[table_name] then + self:wipe_table_data(table_name) + end + end + for table_name in pairs(self.external_references_set) do + local table_info = self.current_tables[table_name] + if table_info then + table_info.potentially_all_set = node + end + end +end + function ClosureState:on_scope_end_for_var(table_name) local table_info = self.current_tables[table_name] local has_external_references = false @@ -223,21 +244,41 @@ function ClosureState:on_scope_end() end end +-- Builtin functions generally cannot access tables in hidden ways +function ClosureState:is_builtin_function(node) + local call_node = node[1] + local called_name = call_node[1] + if builtin_standards.max[called_name] then + if call_node.tag == "Index" then + local key = call_node[2][1] + if key.tag == "String" and builtin_standards.max[called_name][key] then + -- Debug does weird stuff; invalidate everything + if called_name == "debug" and key == "traceback" then + self:stop_tracking_tables() + return true + else + return true + end + end + else + return true + end + end + return false +end + -- A function call leaves the current scope, and does potentially arbitrary modifications -- To any externally referencable tables: either upvalues to other functions -- Or parameters function ClosureState:check_for_function_calls(node) if node.tag ~= "Function" then - if function_call_tags[node.tag] then - self:stop_tracking_tables(node) - return true + if function_call_tags[node.tag] and not self:is_builtin_function(node) then + self:exit_current_scope_for_func_call(node) end for _, sub_node in ipairs(node) do if type(sub_node) == "table" then - if self:check_for_function_calls(sub_node) then - return true - end + self:check_for_function_calls(sub_node) end end end @@ -260,6 +301,34 @@ function ClosureState:record_field_accesses(node) end end +-- More complicated than record_table_accesses below +-- Because invocation can cause accesses to a table at an arbitrary point in logic: +-- = t[x][y:func()] causes a reference to y (passed to func) +function ClosureState:record_table_invocations(node) + if node.tag == "Invoke" then + local self_node = node[1] + if self_node.var and self.current_tables[self_node.var.name] then + self.current_tables[self_node.var.name].potentially_all_set = node + self.current_tables[self_node.var.name].potentially_all_accessed = node + self.external_references_accessed[self_node.var.name] = true + end + end + + if function_call_tags[node.tag] then + for _, sub_node in ipairs(node) do + if type(sub_node) == 'table' then + self:record_table_accesses(sub_node) + end + end + elseif node.tag ~= "Function" then + for _, sub_node in ipairs(node) do + if type(sub_node) == 'table' then + self:record_table_invocations(sub_node) + end + end + end +end + -- Records accesses to the table as a whole, i.e. for table x, either t[x] = val or x = t -- For the former, we stop tracking the table; for the latter, we mark x and t down as aliases if x is a local -- For existing table t, in "local x = t", x is passed in as the aliased node @@ -297,6 +366,7 @@ function ClosureState:record_table_accesses(node, aliased_node) end end + self:record_table_invocations(node) return alias_info end @@ -470,6 +540,12 @@ local function detect_unused_table_fields(closure, check_state) for var in pairs(func_scope.accessed_upvalues) do closure_state.external_references_accessed[var.name] = true end + for var in pairs(func_scope.set_upvalues) do + closure_state.external_references_set[var.name] = true + end + for var in pairs(func_scope.mutated_upvalues) do + closure_state.external_references_set[var.name] = true + end end end item_callbacks[item.tag](closure_state, item)