Skip to content

feat(erasing-op): add erasing-op rule#1521

Open
bartlomieju wants to merge 1 commit into
mainfrom
oxlint-port-oxc-correctness
Open

feat(erasing-op): add erasing-op rule#1521
bartlomieju wants to merge 1 commit into
mainfrom
oxlint-port-oxc-correctness

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

Ports oxlint's oxc/erasing-op rule (itself based on Clippy's erasing_op)
to a native Rust rule. It flags binary operations whose result is always
zero, which is almost always a mistake rather than intent:

  • x * 0 and 0 * x
  • x & 0 and 0 & x
  • 0 / x

It deliberately does not flag 0 / 0 (evaluates to NaN) or x / 0
(Infinity / NaN), since neither is zero. Parenthesized zeros such as
x * (0) are unwrapped and still flagged.

The rule reuses the existing Handler/bin_expr visitor and reports the
whole binary expression. Tests are ported from the oxc rule's own pass/fail
fixtures, plus a few extra valid cases that lock the operator scope (+,
-, | are not erasing) and the 0 / 0 / x / 0 carve-outs.

This is the first of the oxc-correctness oxlint ports tracked in the lint
roadmap. One open decision for a maintainer: it is tagged recommended
(oxc classifies it as correctness), which makes it on-by-default for every
deno lint user. Happy to drop the recommended tag if you would rather
land it opt-in first.

Reference: https://github.com/oxc-project/oxc/blob/main/crates/oxc_linter/src/rules/oxc/erasing_op.rs

Ports oxlint's oxc/erasing-op (itself based on Clippy's erasing_op) to a
native Rust rule. Flags binary operations that always evaluate to zero:
`x * 0`, `0 * x`, `x & 0`, `0 & x`, and `0 / x` (but not `0 / 0`, which is
NaN, nor `x / 0`). Parenthesized zeros are unwrapped.

Tagged `recommended` (oxc category: correctness), so it would be on by
default -- flagging for maintainer sign-off.
@bartlomieju

Copy link
Copy Markdown
Member Author

Verdict: LGTM on the logic — needs the RECOMMENDED sign-off you flagged + a rebase. Not auto-merging (new rule).

Faithful port of oxc/Clippy erasing_op.

Correctness — verified

  • Mul/BitAnd: is_zero(left) || is_zero(right) correctly catches x*0, 0*x, x&0, 0&x.
  • Div: is_zero(left) && !is_zero(right) flags 0/x but correctly exempts the literal 0/0 (NaN) and x/0 (Infinity/NaN). The valid tests lock these carve-outs and the non-erasing operators (+, -, |).
  • is_zero unwraps ParenExpr recursively, so x * (0) is flagged; and it matches only Lit::Num == 0.0, so +/-/| and non-zero literals are untouched.

Minor / faithful-to-reference edges (non-blocking)

  • Only a numeric literal 0 counts: -0 (a UnaryExpr, not a Num) and BigInt 0n (Lit::BigInt) are not flagged. That matches the reference and is reasonable (x * 0n would TypeError on mixed operands anyway) — just noting.
  • The message "will always evaluate to zero" is slightly imprecise for 0 / x when x is 0 at runtime (NaN); the rule intentionally flags the pattern as suspicious regardless, matching oxc. Could soften the wording, but fine as-is.

Perf: trivial — one is_zero check per bin_expr.

Housekeeping: RECOMMENDED/on-by-default is the maintainer call you raised (oxc classifies it correctness, so on-by-default is defensible). mergeable: UNKNOWN → rebase; a docs/rules/*.md isn't in the diff (new rules conventionally ship one).

Logic is solid and well-tested. Blockers are just the tagging decision + rebase (+ docs).

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