feat(parser): lua 5.5 global const

[yeshan333]

No description provided.

[llamapreview]

AI Code Review by LlamaPReview

🎯 TL;DR & Recommendation

Recommendation: Approve with suggestions.

This PR adds support for Lua 5.4 attributes in global declarations, but introduces architectural mismatches and incomplete parsing that could lead to hidden defects in linting accuracy and increased maintenance overhead.

📄 Documentation Diagram

This diagram documents the new global declaration parsing flow with Lua 5.4 attribute support in the Luacheck parser.

sequenceDiagram
    participant P as Parser
    participant T as TokenState
    participant A as ASTBuilder

    P->>T: Skip "global" token
    loop Parse identifiers
        P->>T: Parse identifier
        alt Attribute present
            P->>T: Skip "<" token
            loop Parse attributes
                P->>T: Parse attribute name
                P->>T: Skip optional comma
            end
            P->>T: Skip ">" token
        end
        P->>T: Skip optional comma
    end
    alt Assignment present
        P->>T: Skip "=" token
        P->>T: Parse expression list
    end
    P->>A: Create Global node with identifiers and attributes
    note over P: PR #35;1 adds support for Lua 5.4 attributes in global declarations.

Loading

🌟 Strengths

• Adds parsing support for Lua 5.4 global attributes, extending tool compatibility with newer Lua versions.
• Well-integrated test cases for basic parsing functionality.

Priority	File	Category	Impact Summary	Anchors
P2	src/luacheck/parser.lua	Architecture	Attributes ignored, causing data loss in AST for Lua 5.4+ code.
P2	src/luacheck/parser.lua	Bug	Incorrect attribute parsing rejects valid Lua 5.4 syntax with commas.
P2	spec/parser_spec.lua	Testing	Tests miss attribute validation, allowing hidden defects to pass.
P2	src/luacheck/parser.lua	Maintainability	Code duplication with local parsing increases maintenance overhead.
P2	src/luacheck/parser.lua	Maintainability	Temporary implementation comment signals technical debt for attributes.

🔍 Notable Themes

• Incomplete Attribute Handling: Multiple findings highlight missing or incorrect attribute parsing and storage, which could compromise linting accuracy for Lua 5.4+ code.
• Code Duplication and Maintenance: The new global parsing logic duplicates existing local parsing, increasing the risk of bugs and maintenance burden.

---

💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.

[llamapreview]

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})

[llamapreview]

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

[llamapreview]

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"))

[llamapreview]

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