Skip to content

Commit

Permalink
feat: add check for modified constant variables (#116)
Browse files Browse the repository at this point in the history
  • Loading branch information
arichard4 committed Dec 12, 2024
1 parent c697e32 commit 030ce6a
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 5 deletions.
1 change: 1 addition & 0 deletions docsrc/warnings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Code Description
431 Shadowing an upvalue.
432 Shadowing an upvalue argument.
433 Shadowing an upvalue loop variable.
441 Constant local variable is modified.
511 Unreachable code.
512 Loop can be executed at most once.
521 Unused label.
Expand Down
75 changes: 75 additions & 0 deletions spec/linearize_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -404,3 +404,78 @@ g, a = f()]]))
end)
end)
end)

describe("constant modification detection", function()
it("detects modified constants", function()
assert.same({
{code = "441", line = 4, column = 1, end_column = 17, defined_line = 1, name = 'b'}
}, helper.get_stage_warnings("linearize", [[
local a, b <const>, c = 1, 1, 1
print(a, b, c)
a, b, c = 2, 2, 2
print(a, b, c)
]]))
end)

it("detects a constant overwritten by a function", function()
assert.same({
{code = "441", line = 4, column = 1, end_column = 16, defined_line = 1, name = 'a'}
}, helper.get_stage_warnings("linearize", [[
local a <const> = 1
print(a)
function a() end
a()
]]))
end)

it("doesn't trigger on a constant redefinition", function()
assert.same({
{code = "411", line = 4, column = 7, end_column = 7, name = 'a',
prev_column = 7, prev_end_column = 7, prev_line = 1}
}, helper.get_stage_warnings("linearize", [[
local a <const> = 1
print(a)
local a = 1
print(a)
]]))
end)

it("doesn't trigger on a loop overriding a constant", function()
assert.same({
{code = "421", line = 4, column = 5, end_column = 5, name = 'a',
prev_column = 7, prev_end_column = 7, prev_line = 1}
}, helper.get_stage_warnings("linearize", [[
local a <const> = 1
print(a)
for a = 1,10 do
print(a)
end
]]))
end)

it("handles variable scoping correctly", function()
assert.same({
{code = "421", line = 5, column = 10, end_column = 10, name = 'a',
prev_column = 7, prev_end_column = 7, prev_line = 1},
{code = "421", line = 5, column = 13, end_column = 13, name = 'b',
prev_column = 18, prev_end_column = 18, prev_line = 1},
{code = "441", line = 9, column = 1, end_column = 11, name = 'a',
defined_line = 1}
}, helper.get_stage_warnings("linearize", [[
local a <const>, b = 1, 1
print(a, b)
do
local a, b <const> = 2, 2
print(a, b)
end
a, b = 3, 3
print(a, b)
]]))
end)
end)
14 changes: 11 additions & 3 deletions spec/parser_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -492,14 +492,14 @@ describe("parser", function()
)
end)

it("accepts (and ignores for now) Lua 5.4 attributes", function()
it("accepts Lua 5.4 attributes, ignoring close and parsing const", function()
assert.same({tag = "Local", {
{tag = "Id", "a"}
}
}, get_node("local a <close>"))
assert.same({tag = "Local", {
{tag = "Id", "a"},
{tag = "Id", "b"}
{tag = "Id", const = true, "b"}
}
}, get_node("local a <close>, b <const>"))
assert.same({
Expand All @@ -512,7 +512,7 @@ describe("parser", function()
assert.same({
tag = "Local", {
{tag = "Id", "a"},
{tag = "Id", "b"}
{tag = "Id", const = true, "b"}
}, {
{tag = "Id", "c"},
{tag = "Id", "d"}
Expand All @@ -522,6 +522,14 @@ describe("parser", function()
{line = 1, offset = 16, end_offset = 16, msg = "expected '>' near '='"},
get_error("local a <close = ")
)
assert.same(
{line = 1, offset = 10, end_offset = 13, msg = "Unknown attribute 'cons'"},
get_error("local a <cons = ")
)
assert.same(
{line = 1, offset = 18, end_offset = 18, msg = "expected '(' near '<'"},
get_error("local function y <const> () end")
)
end)

it("parses local declaration with assignment correctly", function()
Expand Down
10 changes: 8 additions & 2 deletions src/luacheck/parser.lua
Original file line number Diff line number Diff line change
Expand Up @@ -821,10 +821,16 @@ statements["local"] = function(state)
lhs[#lhs + 1] = parse_id(state)

-- Check if a Lua 5.4 attribute is present
-- TODO: Warn on different syntax between lua versions?
if state.token == "<" then
-- For now, just consume and ignore it.
skip_token(state)
check_name(state)
local attribute = check_name(state)
if attribute == "const" then
lhs[#lhs].const = true
-- Accept but ignore close
elseif attribute ~= "close" then
parser.syntax_error("Unknown attribute '" .. attribute .. "'", state)
end
skip_token(state)
check_and_skip_token(state, ">")
end
Expand Down
15 changes: 15 additions & 0 deletions src/luacheck/stages/linearize.lua
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ stage.warnings = {
["431"] = redefined_warning("shadowing upvalue {name!} on line {prev_line}"),
["432"] = redefined_warning("shadowing upvalue argument {name!} on line {prev_line}"),
["433"] = redefined_warning("shadowing upvalue loop variable {name!} on line {prev_line}"),
["441"] = {
message_format = "variable {name!} was defined as const on line {defined_line}",
fields = {"name", "defined_line"}
},
["521"] = {message_format = "unused label {label!}", fields = {"label"}}
}

Expand All @@ -42,6 +46,13 @@ 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, {
defined_line = var.node.line,
name = var.name
})
end

local function warn_unused_label(chstate, label)
chstate:warn_range("521", label.range, {
label = label.name
Expand Down Expand Up @@ -518,6 +529,10 @@ function LinState:emit_stmt_Set(node)

if var then
self:register_upvalue_action(item, var, "set_upvalues")

if var.node.const then
warn_modified_const_label(self.chstate, node, var)
end
end
else
assert(expr.tag == "Index")
Expand Down

0 comments on commit 030ce6a

Please sign in to comment.