Skip to content

Add 21 lints for correctness, style, naming and CLVM cost - #234

Merged
Rigidity merged 13 commits into
mainfrom
rigidity/more-lints
Nov 12, 2026
Merged

Rigidity merged 13 commits into
mainfrom
rigidity/more-lints

Conversation

@Rigidity

@Rigidity Rigidity commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

Adds 21 lints. Each one has a fixture in tests/warnings/ that includes cases it shouldn't report.

Lint Default Reports
self_comparison warn x == x, x < x, x - x, x ^ x, x & x and similar
identical_branches warn if expressions whose branches are the same
duplicate_condition warn Repeated conditions in an else if chain, and repeated operands of &&, ||, &, | on booleans
bool_comparison warn x == true, x != false and so on
needless_bool warn if c { true } else { false }
negated_condition allow if !c { a } else { b } and if a != b { .. } else { .. }
collapsible_if warn else { if .. }, which can be else if
manual_assert warn if c { raise; }, which can be assert !c; (assert narrows the same way)
needless_let_return warn let x = expr; x at the end of a block
redundant_field_names warn Point { x: x }, in initializers and destructuring
unnecessary_parens warn Parentheses around atomic expressions, or around whole expressions such as conditions, let values and arguments
naming_convention warn Names that aren't snake_case (functions, modules, bindings), UpperCamelCase (structs, aliases, generics) or SCREAMING_SNAKE_CASE (constants)
shadowed_binding allow Bindings and parameters that shadow another one
debug_statement allow debug statements, meant to be turned on for release builds
unconditional_recursion warn Functions that call themselves on every path
redundant_inline warn inline const and inline let that are used once
large_inline warn Large inline fns whose body is duplicated at several call sites
repeated_expensive_call warn The same hash, coinid, BLS/secp call or function call evaluated again with the same arguments
bytes_int_confusion warn 0xff as Int (which is -1) and 0x0001 as Int (not equal to 1)
unused_struct_field warn Fields that are never read (in the unused group)
unused_export warn exports of a mod that are only used inside of it (in the unused group)

Design notes

  • Shared HIR equality (rue-lint/src/equality.rs). Code has no side effects other than raising, so two expressions written the same way are equal. Bindings are compared by value, so the same let in both branches counts as equal. Casts are ignored. self_comparison, identical_branches, duplicate_condition and repeated_expensive_call use it.
  • redundant_inline only covers constants and bindings. I checked the compiler output: a non-inline const or let used once is already inlined, and the output is identical. A function used once is not inlined automatically, so inline fn used once isn't reported. A binding whose only use is in a lambda isn't reported either, since the lambda captures it.
  • repeated_expensive_call only reports when the first call always runs before the second. The compiler doesn't reuse repeated results, so these calls cost twice. Calls in different branches of an if, or on the right side of &&/||, aren't reported, and neither are calls inside an already-reported call.
  • unused_struct_field is conservative. A struct's value is also its layout, so its fields are only reported if every value of it is read by field. Values that are passed to a function taking that struct, bound with let, or used in if branches or block bodies don't count as escaping. Values that are hashed, returned, put into other values, cast or compared do, and so does any type guard against the struct. Structs that never have a value in the code (such as ones only used in an extern fn signature) are skipped.
  • naming_convention also allows SCREAMING_SNAKE_CASE and Title_Snake_Case for parameters and fields. Chialisp's convention for curried arguments (and the structs that hold them) is uppercase. Truths are conventionally Title_Snake_Case, such as a Truth parameter and fields like Ephemeral_State. Let bindings must still be snake_case.
  • large_inline thresholds: a body of at least 40 HIR nodes, with at least 200 nodes duplicated in total, counting each use after the first. These are rough guesses; a configurable threshold could come later.
  • Infrastructure. HirLintPass gets a finish hook for passes that report after the walk. Passes are now built from the LintInput. The compiler records two new LintSites: struct field reads (destructured fields are only in the HIR through bindings the walk doesn't visit) and mod items (modules aren't declarations).
  • Suggestions that are definitely safe are MachineApplicable. They drop to MaybeIncorrect when the code being replaced contains a comment.

Effect on existing code

  • examples/puzzles/cat.rue: fixed the one new warning (tail_info: tail_info).
  • Snapshot diagnostics changed in about 55 test YAML files. All of them are expected hits on the new lints. The largest groups are repeated_expensive_call, unused_struct_field and needless_let_return in test code.
  • The existing lint unit tests and the LSP protocol test use let a = ..; a, which is now needless_let_return. I updated their expectations, or the source in the LSP test.

Testing

  • cargo test --workspace --all-features, cargo clippy --workspace --all-features --all-targets, cargo fmt --check, and rue fmt --check all pass. The snapshot runner is stable.
  • New unit tests apply every machine-applicable suggestion. They check that a combined style fixture and a removal fixture (debug, inline, export) both end up with no diagnostics.

Adds self_comparison, identical_branches, duplicate_condition,
bool_comparison, needless_bool, negated_condition, collapsible_if,
manual_assert, needless_let_return, redundant_field_names,
unnecessary_parens, naming_convention, shadowed_binding, debug_statement,
unconditional_recursion, redundant_inline, large_inline,
repeated_expensive_call, bytes_int_confusion, unused_struct_field and
unused_export.

HIR passes can now report after the walk with a finish hook, and are
constructed from the lint input. The compiler records struct field reads
and mod items as lint sites, since neither can be recovered from the HIR.
@Rigidity Rigidity mentioned this pull request Nov 12, 2026
11 tasks
Truths are conventionally named in Title_Snake_Case, such as a Truth
parameter and fields like Ephemeral_State, so naming_convention accepts
it for parameters and struct fields alongside snake_case and
SCREAMING_SNAKE_CASE.
- bool_comparison and needless_bool keep a negation in parentheses when
  it's an operand, rather than letting the operator around it split it.
- unnecessary_parens keeps the parentheses around a struct initializer
  anywhere in an if condition, where it can't be written without them.
- unused_export treats nested modules as outside of the module, counts
  named imports and re-exports as uses, and leaves alone types used by
  another export of the module.
- Add Hir::children and Statement::hirs, and use them for the lint walk,
  large_inline's body size and unconditional_recursion's search.
- repeated_expensive_call groups calls by kind, callee and arity before
  comparing them, unused_struct_field identifies structs by index and
  shares the structs mentioned by each type, and passes borrow the
  entrypoints instead of cloning them.
- Share encloses between the passes that check locations.
…unused_export

- Negations replacing an operand of a postfix operator (such as `as` or
  `is`) are parenthesized, since those bind more tightly than `!`.
  Replacement text now depends on whether the expression is a whole
  expression, an operand, or the operand of a postfix operator.
- unnecessary_parens parses the code again without the parentheses, and
  only reports them if the syntax tree is otherwise the same, rather than
  copying the parser's rules for where struct initializers are allowed.
- unused_export uses references through a module path, which the
  compiler now records as a lint site, rather than where references are.
  Paths to a module's own items need them to be exported, and bare names
  from nested modules don't. Only the signatures of other exports (not
  their bodies) keep a type exported.
- large_inline counts the result of a const expression, not its source.
…n one pass

- Replacements in an if condition keep the parentheses around anything
  that could contain a struct initializer, which can't be written there
  without them.
- unnecessary_parens only parses the statement or item that the
  parentheses are in again, and skips it where the lint is allowed, so
  it's no longer quadratic in the size of the file.
- unused_export computes the exports that need to be exported once per
  module: the ones used outside of it, and the types in their signatures
  (including the exports of modules it exports), until nothing changes.
  A type's own fields don't keep it exported, and the signatures of
  exports that are reported don't either.
- Qualified references are recorded in a set.
…ested exports

- Replacements in an if condition get parentheses whenever they could
  contain a struct initializer, including when the parentheses belonged
  to the node being replaced, or a flipped comparison.
- Negations look through parentheses, so a parenthesized `!=` or `!` is
  flipped or removed rather than negated again.
- unused_export only counts a nested module's export as needing this
  module's types if it's used outside of this module, using the
  locations of qualified references and imports. Items the module
  re-exports count as part of what it exports. The names each signature
  reaches are found in one walk of its types.
- A condition replacement only gets parentheses for a `{` outside of
  strings and comments, so a fix doesn't add parentheses that
  unnecessary_parens then reports.
- Share the key that identifies a declaration by its name's location
  between unused_struct_field and unused_export.
- Only record qualified references in the documents being linted.
- Walk signature types without collecting their children first, and
  drop a self-reference check that the used set already handles.
Rue strings don't have escapes and block comments nest, which the
hand-written scanners got wrong, so has_code_brace and is_parenthesized
now use the lexer's tokens instead.
A line comment without a newline after it (at the end of a file, or at
the end of text that a lint tokenizes) made the lexer loop forever. Line
comments now end at the end of the source.

Removing parentheses around a line comment would let it continue onto
the code after it, so suggestions keep them.
- redundant_inline removes the parsed `inline` keyword, rather than
  searching the text before the name, which could find one in a comment.
  The export and inline keywords share how they're removed.
- Parentheses are only kept when the code inside of them ends in a line
  comment, and negations are found by looking through every pair of
  parentheses, so negated_condition and `!` removal see through them.
- bool_comparison fixes that rebuild a comparison with comments in it
  aren't applied automatically, and comments are found with the lexer.
- Parentheses and comments are checked with one pass of the lexer.
A suggestion whose edits replace code with fewer comments than it had
isn't applied automatically, which now covers every lint (including
unnecessary_cast, unnecessary_plus and unnecessary_return, which could
remove comments before). The checks in each lint are removed, so fixes
that keep every comment (such as a comparison whose parentheses are kept
around a line comment) are applied again.

The locations of the export and inline keywords are found by the same
helper, and the inline ones are only found if there's something to
report.
An automatically applied replacement that starts or ends with a word
next to another word gets a space, so removing the `+` in `return+a`
gives `return a` rather than `returna`. The comment check also only
looks at suggestions that would be applied automatically.
The order that a directory's entries are listed in depends on the file
system, so a project's files were compiled (and their diagnostics
reported) in a different order on Linux than on macOS. The slot machine
project now has diagnostics in several files, so its snapshot differed
in CI. Entries are sorted by path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant