Skip to content

feat(parser): lua 5.5 global const #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yeshan333 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat(parser): lua 5.5 global const #1

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3cb3721
feat(parser): add support for Lua 5.4 attributes in global declarations
yeshan333 Mar 30, 2026
b0379aa
build: add Lua 5.5 to test matrix
yeshan333 Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions spec/parser_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,48 @@ describe("parser", function()
end)
end)

describe("when parsing global declarations", function()
it("parses global declaration correctly", function()
assert.same({tag = "Global", {
{tag = "Id", "a"}
}
}, get_node("global a"))
assert.same({tag = "Global", {
{tag = "Id", "a"},
{tag = "Id", "b"}
}
}, get_node("global a, b"))
end)

it("accepts Lua 5.4 attributes on global declarations", function()
assert.same({tag = "Global", {
{tag = "Id", "a"}
}
}, get_node("global a <close>"))
assert.same({tag = "Global", {
{tag = "Id", "a"},
{tag = "Id", "b"}
}
}, get_node("global a <close>, b <const>"))
assert.same({
tag = "Global", {
{tag = "Id", "a"}
}, {
{tag = "Id", "b"}
}
}, get_node("global a <close> = b"))
Comment on lines +606 to +612
Copy link
Copy Markdown

@llamapreview llamapreview Bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 | Confidence: High

The test suite validates that the parser produces a Global node but does not verify the presence or structure of attribute data. This creates a false sense of correctness. The tests pass because attributes are ignored, but they do not test the semantic requirement that attributes be captured. The test suite should be updated to expect an attribute field in the AST node (once the parser is fixed) to prevent future regressions.

Code Suggestion:

-- Example of a test expecting attributes. Assumes the parser is updated to store them.
assert.same({
   tag = "Global", {
      {tag = "Id", "a"}
   }, {
      {tag = "Id", "b"}
   }, {  -- Third child: attributes list
      "close"  -- Attribute for 'a', nil for variables without attributes
   }
}, get_node("global a <close> = b"))

assert.same({
tag = "Global", {
{tag = "Id", "a"},
{tag = "Id", "b"}
}, {
{tag = "Id", "c"},
{tag = "Id", "d"}
}
}, get_node("global a <close>, b <const> = c, d"))
end)
end)

describe("when parsing assignments", function()
it("parses single target assignment correctly", function()
assert.same({
Expand Down
29 changes: 29 additions & 0 deletions src/luacheck/parser.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,35 @@ statements["local"] = function(state)
return new_inner_node(start_range, rhs and rhs[#rhs] or lhs[#lhs], "Local", {lhs, rhs})
end

statements["global"] = function(state)
Copy link
Copy Markdown

@llamapreview llamapreview Bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 | Confidence: High

Speculative: The implementation creates a "Global" AST node but ignores attributes entirely (<close>, <const>). This is an architectural mismatch. The parser's output is presumably consumed by a later linting/analysis phase that may need to know about const or <close> attributes to enforce correct semantics (e.g., preventing reassignment of const globals). The downstream components cannot perform accurate analysis if attribute information is discarded. While this allows parsing to succeed for syntax compatibility, it creates a hidden defect: the tool will produce incorrect or incomplete warnings for annotated Lua 5.4+ code. This is a data loss issue where the AST omits semantically critical information present in the source.

Code Suggestion:

-- Store attributes as a third element in the node structure, similar to how 'Local' might handle them.
local attributes = {}
repeat
   lhs[#lhs + 1] = parse_id(state)
   local attr = nil
   if state.token == "<" then
      skip_token(state)
      attr = check_name(state)  -- e.g., "const" or "close"
      skip_token(state)
      check_and_skip_token(state, ">")
   end
   attributes[#attributes + 1] = attr
until not test_and_skip_token(state, ",")

-- ... rest of parsing ...

return new_inner_node(start_range, rhs and rhs[#rhs] or lhs[#lhs], "Global", {lhs, rhs, attributes})

local start_range = copy_range(state)
-- Skip "global".
skip_token(state)
Comment on lines +840 to +843
Copy link
Copy Markdown

@llamapreview llamapreview Bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 | Confidence: High

The new statements["global"] function closely mirrors the existing statements["local"] function (parse_local). This duplication violates DRY (Don't Repeat Yourself) and increases maintenance burden. Any future bug fix or enhancement to local/global parsing logic must be applied in two places. This structural similarity suggests an opportunity to refactor common parsing logic into a helper function.

Code Suggestion:

local function parse_declaration(state, is_local)
   local start_range = copy_range(state)
   -- Skip "local" or "global"
   skip_token(state)

   local lhs = {}
   local rhs
   local attributes = {} -- If storing attributes

   repeat
      lhs[#lhs + 1] = parse_id(state)
      -- ... attribute parsing logic ...
   until not test_and_skip_token(state, ",")

   if test_and_skip_token(state, "=") then
      rhs = parse_expression_list(state)
   end

   local tag = is_local and "Local" or "Global"
   return new_inner_node(start_range, rhs and rhs[#rhs] or lhs[#lhs], tag, {lhs, rhs, attributes})
end

statements["local"] = function(state) return parse_declaration(state, true) end
statements["global"] = function(state) return parse_declaration(state, false) end


-- Global definition, potentially with assignment.
local lhs = {}
local rhs

repeat
lhs[#lhs + 1] = parse_id(state)

-- Check if a Lua 5.4/5.5 attribute is present
if state.token == "<" then
-- Consume and ignore attribute for now
skip_token(state)
check_name(state)
skip_token(state)
check_and_skip_token(state, ">")
end
Comment on lines +852 to +859
Copy link
Copy Markdown

@llamapreview llamapreview Bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 | Confidence: High

  • The attribute parsing logic does not match Lua's grammar for multiple attributes (e.g., <const, close>), causing rejection of valid Lua 5.4 syntax.
  • The temporary implementation comment signals technical debt, indicating an incomplete feature that may require future breaking changes.

Code Suggestion:

if state.token == "<" then
   skip_token(state)
   repeat
      -- Allow optional whitespace; skip_token handles it if lexer produces separate tokens.
      check_name(state) -- Validates attribute name
      skip_token(state) -- Consume the name
   until not test_and_skip_token(state, ",")
   check_and_skip_token(state, ">")
end

until not test_and_skip_token(state, ",")

if test_and_skip_token(state, "=") then
rhs = parse_expression_list(state)
end

return new_inner_node(start_range, rhs and rhs[#rhs] or lhs[#lhs], "Global", {lhs, rhs})
end

statements["::"] = function(state)
local start_range = copy_range(state)
-- Skip "::".
Expand Down
Loading