Skip to content

Commit

Permalink
Circular references shouldn't mark a variable as being accessed
Browse files Browse the repository at this point in the history
  • Loading branch information
arichard4 committed Oct 31, 2021
1 parent 581ea14 commit daccec9
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 19 deletions.
35 changes: 19 additions & 16 deletions spec/cli_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,9 @@ Checking spec/samples/bad_code.lua 5 warnings
spec/samples/bad_code.lua:8:10: variable 'opt' was previously defined as an argument on line 7
spec/samples/bad_code.lua:9:11: accessing undefined variable 'hepler'
Checking spec/samples/unused_code.lua 9 warnings
Checking spec/samples/unused_code.lua 10 warnings
spec/samples/unused_code.lua:1:7: variable 'foo' is mutated but never accessed
spec/samples/unused_code.lua:3:18: unused argument 'baz'
spec/samples/unused_code.lua:4:8: unused loop variable 'i'
spec/samples/unused_code.lua:5:13: unused variable 'q'
Expand All @@ -203,7 +204,7 @@ Checking spec/samples/unused_code.lua 9 warnings
spec/samples/unused_code.lua:14:1: value assigned to variable 'x' is overwritten on line 15 before use
spec/samples/unused_code.lua:21:7: variable 'z' is never accessed
Total: 14 warnings / 0 errors in 3 files
Total: 15 warnings / 0 errors in 3 files
]], get_output "-q spec/samples/bad_code.lua spec/samples/good_code.lua spec/samples/unused_code.lua --no-config")
assert.equal([[
Total: 0 warnings / 0 errors in 1 file
Expand All @@ -213,14 +214,14 @@ Total: 0 warnings / 0 errors in 1 file
it("suppresses warnings output with -qq", function()
assert.equal([[
Checking spec/samples/bad_code.lua 5 warnings
Checking spec/samples/unused_code.lua 9 warnings
Checking spec/samples/unused_code.lua 10 warnings
Total: 14 warnings / 0 errors in 3 files
Total: 15 warnings / 0 errors in 3 files
]], get_output "-qq spec/samples/bad_code.lua spec/samples/good_code.lua spec/samples/unused_code.lua --no-config")
end)

it("suppresses file info output with -qqq", function()
assert.equal([[Total: 14 warnings / 0 errors in 3 files
assert.equal([[Total: 15 warnings / 0 errors in 3 files
]], get_output "-qqq spec/samples/bad_code.lua spec/samples/good_code.lua spec/samples/unused_code.lua --no-config")
end)

Expand Down Expand Up @@ -318,8 +319,9 @@ Total: 1 warning / 0 errors in 1 file

it("recognizes different types of variables", function()
assert.equal([[
Checking spec/samples/unused_code.lua 9 warnings
Checking spec/samples/unused_code.lua 10 warnings
spec/samples/unused_code.lua:1:7: variable 'foo' is mutated but never accessed
spec/samples/unused_code.lua:3:18: unused argument 'baz'
spec/samples/unused_code.lua:4:8: unused loop variable 'i'
spec/samples/unused_code.lua:5:13: unused variable 'q'
Expand All @@ -330,20 +332,21 @@ Checking spec/samples/unused_code.lua 9 warnings
spec/samples/unused_code.lua:14:1: value assigned to variable 'x' is overwritten on line 15 before use
spec/samples/unused_code.lua:21:7: variable 'z' is never accessed
Total: 9 warnings / 0 errors in 1 file
Total: 10 warnings / 0 errors in 1 file
]], get_output "spec/samples/unused_code.lua --no-config")
end)

it("allows to ignore unused arguments", function()
assert.equal([[
Checking spec/samples/unused_code.lua 4 warnings
Checking spec/samples/unused_code.lua 5 warnings
spec/samples/unused_code.lua:1:7: variable 'foo' is mutated but never accessed
spec/samples/unused_code.lua:5:13: unused variable 'q'
spec/samples/unused_code.lua:13:7: value assigned to variable 'x' is overwritten on line 14 before use
spec/samples/unused_code.lua:14:1: value assigned to variable 'x' is overwritten on line 15 before use
spec/samples/unused_code.lua:21:7: variable 'z' is never accessed
Total: 4 warnings / 0 errors in 1 file
Total: 5 warnings / 0 errors in 1 file
]], get_output "spec/samples/unused_code.lua --no-unused-args --no-config")
end)

Expand Down Expand Up @@ -1190,7 +1193,7 @@ Total: 5 warnings / 0 errors in 2 files

it("allows using cli-specific options in top level config", function()
assert.equal([[Files: 2
Warnings: 14
Warnings: 15
Errors: 0
Quiet: 0
Color: false
Expand All @@ -1213,12 +1216,12 @@ Checking spec/samples/read_globals.lua 5 warnings
Checking spec/samples/read_globals_inline_options.lua 3 warnings
Checking spec/samples/redefined.lua 7 warnings
Checking spec/samples/reversed_fornum.lua 1 warning
Checking spec/samples/unused_code.lua 9 warnings
Checking spec/samples/unused_code.lua 10 warnings
Checking spec/samples/unused_secondaries.lua 4 warnings
Checking spec/samples/utf8.lua 4 warnings
Checking spec/samples/utf8_error.lua 1 error
Total: 72 warnings / 5 errors in 19 files
Total: 73 warnings / 5 errors in 19 files
]]):gsub("(spec/samples)/", "%1"..package.config:sub(1, 1)),
get_output "spec/samples --config=spec/configs/exclude_files_config.luacheckrc -qq --exclude-files spec/samples/global_fields.lua")
end)
Expand All @@ -1238,12 +1241,12 @@ Checking read_globals.lua 5 warnings
Checking read_globals_inline_options.lua 3 warnings
Checking redefined.lua 7 warnings
Checking reversed_fornum.lua 1 warning
Checking unused_code.lua 9 warnings
Checking unused_code.lua 10 warnings
Checking unused_secondaries.lua 4 warnings
Checking utf8.lua 4 warnings
Checking utf8_error.lua 1 error
Total: 72 warnings / 5 errors in 19 files
Total: 73 warnings / 5 errors in 19 files
]], get_output(". --config=spec/configs/exclude_files_config.luacheckrc -qq --exclude-files global_fields.lua", "spec/samples/"))
end)

Expand All @@ -1260,12 +1263,12 @@ Checking line_length.lua 8 warnings
Checking python_code.lua 1 error
Checking redefined.lua 7 warnings
Checking reversed_fornum.lua 1 warning
Checking unused_code.lua 9 warnings
Checking unused_code.lua 10 warnings
Checking unused_secondaries.lua 4 warnings
Checking utf8.lua 4 warnings
Checking utf8_error.lua 1 error
Total: 64 warnings / 5 errors in 17 files
Total: 65 warnings / 5 errors in 17 files
]], get_output(". --config=spec/configs/exclude_files_config.luacheckrc -qq --exclude-files global_fields.lua --exclude-files " .. quote("./read*"), "spec/samples/"))
end)

Expand Down
1 change: 0 additions & 1 deletion spec/samples/bad_code.lua
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,3 @@ function embrace(opt)
local opt = opt or "default"
return hepler(opt.."?")
end

15 changes: 15 additions & 0 deletions spec/samples/unused_code.lua
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,18 @@ y = 6
local z = 5;
(function() z = 4 end)()
z = 6

foo[foo] = 1
foo[1] = foo
foo[foo] = foo
function foo.meta()
return function() print(foo) end
end

local t = {}
function t.func() print(t) return {val = 1} end
t[t] = t.func().val + 1

local s = {}
function s:func() print(self) return {val = 1} end
s[s] = s:func().val + 1
48 changes: 46 additions & 2 deletions src/luacheck/stages/resolve_locals.lua
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,46 @@ local function in_scope(var, index)
return (var.scope_start <= index) and (index <= var.scope_end)
end

local function contains_call(node)
if node.tag == "Call" or node.tag == "Invoke" then
return true
end

if node.tag ~= "Function" then
for _, sub_node in ipairs(node) do
if type(sub_node) == 'table' and contains_call(sub_node) then
return true
end
end
end

return false
end

local function is_circular_reference(item, var)
-- No support for matching multiple assignment to the specific assignment
if not item.lhs or #item.lhs ~= 1 or not item.rhs or #item.rhs ~= 1 then
return false
end

-- Case t[t.function()] = t.func()
-- Functions can have side-effects, so this isn't purely circular
local right_assignment = item.rhs[1]
if contains_call(right_assignment) then
return false
end

local left_assignment = item.lhs[1]
if contains_call(left_assignment) then
return false
end
local node = left_assignment[1]
if node.var == var then
return true
end
return false
end

-- Called when main assignment propagation reaches a line item.
local function main_assignment_propagation_callback(line, index, item, var, value)
-- Check entrance condition.
Expand All @@ -91,7 +131,9 @@ local function main_assignment_propagation_callback(line, index, item, var, valu

-- Accesses (and mutations) of the variable can resolve to reaching assignment.
if item.accesses and item.accesses[var] then
add_resolution(line, item, var, value)
if not is_circular_reference(item, var) then
add_resolution(line, item, var, value)
end
end

if item.mutations and item.mutations[var] then
Expand All @@ -102,7 +144,9 @@ local function main_assignment_propagation_callback(line, index, item, var, valu
-- can resolve to reaching assignment.
if item.lines then
for _, created_line in ipairs(item.lines) do
add_resolutions(created_line, created_line.accessed_upvalues[var], var, value)
if not is_circular_reference(item, var) then
add_resolutions(created_line, created_line.accessed_upvalues[var], var, value)
end
add_resolutions(created_line, created_line.mutated_upvalues[var], var, value, true)
end
end
Expand Down

0 comments on commit daccec9

Please sign in to comment.