Skip to content

Conversation

@arichard4
Copy link

No description provided.

@arichard4 arichard4 requested a review from alerque June 20, 2025 01:27
@arichard4
Copy link
Author

Note that we can't use "code_lines"- it's a boolean, and it can't be made numeric because it has potential false positives here. (It tracks "line" as well as "state.lexer.line", potentially the same or next line.)

@alerque alerque force-pushed the one_statement_per_line branch 5 times, most recently from 09da9b9 to 8500117 Compare June 25, 2025 08:28
@arichard4
Copy link
Author

@alerque Oh, I see you tested one other case- this current technically passes this case, but only by flatly ignoring anything not at the top level of the AST, so it misses almost everything. Good catch, I can fix that up by checking over each block of statements instead.

@arichard4
Copy link
Author

@alerque Updated.

For the sort of case you were looking at, the following will not trigger the new warning:

function () print("a") end
if true then; bar = 42; end

However, if you stick multiple statements on the same line inside those blocks, they will emit warnings now. There is one case in luacheck's existing code which fails, which I have modified:

   parser:flag("-v --version", "Show version info and exit.")
      :action(function() print(version.string) os.exit(exit_codes.ok) end)

The new lint flags that the print and exit statements are on the same line; I've put them on different lines.

@arichard4
Copy link
Author

The latter case is, I think, wrong regardless; but we might want to add an explicit escape hatch to ignore multiple semicolon separated statements per line. I've changed it to this:

   parser:flag("-v --version", "Show version info and exit.")
      :action(function()
         print(version.string)
         os.exit(exit_codes.ok)
      end)

I'm not sure if we'd want this lint to explicitly permit this:

   parser:flag("-v --version", "Show version info and exit.")
      :action(function() print(version.string); os.exit(exit_codes.ok); end)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant