Skip to content

Commit

Permalink
feat: warn on mutated loop control variables (#109)
Browse files Browse the repository at this point in the history
  • Loading branch information
arichard4 committed Dec 12, 2024
1 parent 030ce6a commit a508497
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 11 deletions.
1 change: 1 addition & 0 deletions docsrc/cli.rst
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ Option Meaning
``--enable | -e <patt> [<patt>] ...`` Do not filter out warnings matching patterns.
``--only | -o <patt> [<patt>] ...`` Filter out warnings not matching patterns.
``--operators <patt> [<patt>] ...`` Allow compound operators matching patterns. (Multiple assignment not supported, as this is specifically for the Playdate SDK.)
``--const-loop-control-vars`` Flag loop control variables as <const>.
``--config <config>`` Path to custom configuration file (default: ``.luacheckrc``).
``--no-config`` Do not look up custom configuration file.
``--default-config <config>`` Default path to custom configuration file, to be used if ``--[no-]config`` is not used and ``.luacheckrc`` is not found.
Expand Down
1 change: 1 addition & 0 deletions docsrc/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Option Type Default v
``new_read_globals`` Array of strings or field definition map (Do not overwrite)
``not_globals`` Array of strings ``{}``
``operators`` Array of strings ``{}``
``const_loop_control_vars`` Boolean ``false``
``compat`` Boolean ``false``
``allow_defined`` Boolean ``false``
``allow_defined_top`` Boolean ``false``
Expand Down
1 change: 1 addition & 0 deletions docsrc/warnings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Code Description
432 Shadowing an upvalue argument.
433 Shadowing an upvalue loop variable.
441 Constant local variable is modified.
442 Loop control variable is modified.
511 Unreachable code.
512 Loop can be executed at most once.
521 Unused label.
Expand Down
19 changes: 19 additions & 0 deletions spec/linearize_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,25 @@ end
a, b = 3, 3
print(a, b)
]]))
end)

it("handles loop control variables correctly", function()
assert.same({
{code = "442", line = 2, column = 3, end_column = 11, name = 'i',
defined_line = 1},
{code = "442", line = 7, column = 3, end_column = 13, name = 'k',
defined_line = 6},
}, helper.get_stage_warnings("linearize", [[
for i = 1, 10 do
i = i - 1
print(i)
end
for k,_ in pairs({ foo = "bar" }) do
k = "k:"..k
print(k)
end
]]))
end)
end)
12 changes: 6 additions & 6 deletions spec/parser_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ describe("parser", function()
describe("when parsing for", function()
it("parses fornum correctly", function()
assert.same({tag = "Fornum",
{tag = "Id", "i"},
{tag = "Id", const_loop_var = true, "i"},
{tag = "Number", "1"},
{tag = "Op", "len", {tag = "Id", "t"}},
{}
Expand Down Expand Up @@ -283,7 +283,7 @@ describe("parser", function()

it("parses fornum with step correctly", function()
assert.same({tag = "Fornum",
{tag = "Id", "i"},
{tag = "Id", const_loop_var = true, "i"},
{tag = "Number", "1"},
{tag = "Op", "len", {tag = "Id", "t"}},
{tag = "Number", "2"},
Expand All @@ -297,14 +297,14 @@ describe("parser", function()

it("parses forin correctly", function()
assert.same({tag = "Forin", {
{tag = "Id", "i"}
{tag = "Id", const_loop_var = true, "i"}
}, {
{tag = "Id", "t"}
},
{}
}, get_node("for i in t do end"))
assert.same({tag = "Forin", {
{tag = "Id", "i"},
{tag = "Id", const_loop_var = true, "i"},
{tag = "Id", "j"}
}, {
{tag = "Id", "t"},
Expand Down Expand Up @@ -987,7 +987,7 @@ describe("parser", function()
},
{tag = "Do",
{tag = "Fornum",
{tag = "Id", "i"},
{tag = "Id", const_loop_var = true, "i"},
{tag = "Number", "1"},
{tag = "Number", "2"},
{
Expand All @@ -998,7 +998,7 @@ describe("parser", function()
},
{tag = "Forin",
{
{tag = "Id", "k"},
{tag = "Id", const_loop_var = true, "k"},
{tag = "Id", "v"}
},
{
Expand Down
7 changes: 4 additions & 3 deletions src/luacheck/parser.lua
Original file line number Diff line number Diff line change
Expand Up @@ -719,14 +719,15 @@ statements["for"] = function(state)

local ast_node = {}
local tag
local first_var = parse_id(state)
local control_var = parse_id(state)
control_var.const_loop_var = true

if state.token == "=" then
-- Numeric "for" loop.
tag = "Fornum"
-- Skip "=".
skip_token(state)
ast_node[1] = first_var
ast_node[1] = control_var
ast_node[2] = parse_expression(state)
check_and_skip_token(state, ",")
ast_node[3] = parse_expression(state)
Expand All @@ -741,7 +742,7 @@ statements["for"] = function(state)
-- Generic "for" loop.
tag = "Forin"

local iter_vars = {first_var}
local iter_vars = {control_var}
while test_and_skip_token(state, ",") do
iter_vars[#iter_vars + 1] = parse_id(state)
end
Expand Down
12 changes: 11 additions & 1 deletion src/luacheck/stages/linearize.lua
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ stage.warnings = {
message_format = "variable {name!} was defined as const on line {defined_line}",
fields = {"name", "defined_line"}
},
["442"] = {
message_format =
"variable {name!}, defined on line {defined_line}, is a loop control variable and shouldn't be modified",
fields = {"name", "defined_line"}
},
["521"] = {message_format = "unused label {label!}", fields = {"label"}}
}

Expand All @@ -47,7 +52,8 @@ local function warn_redefined(chstate, var, prev_var, is_same_scope)
end

local function warn_modified_const_label(chstate, node, var)
chstate:warn_range("441", node, {
local code = "44" .. (var.node.const and "1" or "2")
chstate:warn_range(code, node, {
defined_line = var.node.line,
name = var.name
})
Expand Down Expand Up @@ -533,6 +539,10 @@ function LinState:emit_stmt_Set(node)
if var.node.const then
warn_modified_const_label(self.chstate, node, var)
end

if var.node.const_loop_var then
warn_modified_const_label(self.chstate, node, var)
end
end
else
assert(expr.tag == "Index")
Expand Down
3 changes: 2 additions & 1 deletion src/luacheck/standards.lua
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ local function add_fields(def, fields, overwrite, ignore_array_part, default_rea
return
end

for field_name, field_def in pairs(fields) do
for k, v in pairs(fields) do
local field_name, field_def = k, v
if type(field_name) == "string" or not ignore_array_part then
if type(field_name) ~= "string" then
field_name = field_def
Expand Down

0 comments on commit a508497

Please sign in to comment.