Skip to content

feat(linter): implement eslint/no-implicit-coercion rule#16735

Merged
camc314 merged 7 commits intooxc-project:mainfrom
tt-a1i:feat/linter-no-implicit-coercion
Dec 11, 2025
Merged

feat(linter): implement eslint/no-implicit-coercion rule#16735
camc314 merged 7 commits intooxc-project:mainfrom
tt-a1i:feat/linter-no-implicit-coercion

Conversation

@tt-a1i
Copy link
Copy Markdown
Contributor

@tt-a1i tt-a1i commented Dec 11, 2025

Summary

Implements the ESLint no-implicit-coercion rule which disallows shorthand type conversions.

Detected patterns

Pattern Type Suggested Fix
!!foo boolean Boolean(foo)
+foo number Number(foo)
1 * foo / foo * 1 number Number(foo)
foo - 0 number Number(foo)
- -foo number Number(foo)
"" + foo / foo + "" string String(foo)
~foo.indexOf() boolean -
`${foo}` string String(foo)

Configuration options

{
  "boolean": true,
  "number": true,
  "string": true,
  "disallowTemplateShorthand": false,
  "allow": ["!!", "+"]
}

References

Test plan

  • Added comprehensive test cases covering all coercion patterns
  • Tests pass locally
  • Clippy passes with no warnings

@tt-a1i tt-a1i requested a review from camc314 as a code owner December 11, 2025 07:01
@github-actions github-actions bot added A-linter Area - Linter C-enhancement Category - New feature or request labels Dec 11, 2025
@camc314 camc314 self-assigned this Dec 11, 2025
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Dec 11, 2025

CodSpeed Performance Report

Merging #16735 will not alter performance

Comparing tt-a1i:feat/linter-no-implicit-coercion (18ab8db) with main (725a5c0)

Summary

✅ 4 untouched
⏩ 41 skipped1

Footnotes

  1. 41 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Copy Markdown
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

This looks good - thanks for working on this!

@tt-a1i
Copy link
Copy Markdown
Contributor Author

tt-a1i commented Dec 11, 2025

Thanks for the review! I've addressed all your suggestions:

  1. Switched to DefaultRuleConfig for cleaner config parsing - good call, it's much simpler! (Note: kept the Box::new() wrapper since our struct is NoImplicitCoercion(Box<...>), so can't directly return into_inner())
  2. Ran cargo lintgen to regenerate rule_runner_impls.rs
  3. Replaced the is_number_one helper with Expression::is_number_value(1.0) from oxc_ast

Really appreciate you pointing these out, learned some useful patterns here.

Copy link
Copy Markdown
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

Thank you! I pushed two more commits, one

The first adding support for lastIndexOf. Annoying I couldn't see any eslint test cases for this scenario. So i added some

https://github.com/eslint/eslint/blob/46eea6d1cbed41d020cb76841ebba30710b0afd0/lib/rules/no-implicit-coercion.js#L14

And the second is refactoring the allow config property into bitflags (see the commit message for more detail/why)

tt-a1i and others added 7 commits December 11, 2025 11:18
This rule disallows shorthand type conversions using operators like
`!!`, `+`, `""+ `, `*1`, etc.

Detected patterns:
- `!!foo` → boolean coercion (suggest: Boolean(foo))
- `+foo` → number coercion (suggest: Number(foo))
- `1 * foo` / `foo * 1` → number coercion
- `foo - 0` → number coercion
- `- -foo` → number coercion via double negation
- `"" + foo` / `foo + ""` → string coercion (suggest: String(foo))
- `~foo.indexOf()` → boolean coercion
- \`\${foo}\` → string coercion (when disallowTemplateShorthand is true)

Configuration options:
- boolean: warn on boolean coercion (default: true)
- number: warn on number coercion (default: true)
- string: warn on string coercion (default: true)
- disallowTemplateShorthand: warn on template literal coercion (default: false)
- allow: list of operators to allow (e.g., ["!!", "+"])
- Rename is_number_call to is_already_numeric for clarity
- Extract repeated diagnostic logic into report_coercion helper
- Add CoercionKind enum for type-safe coercion classification
- Improve comments explaining template literal whitespace check
- Use DefaultRuleConfig for simpler configuration parsing
- Replace is_number_one helper with existing Expression::is_number_value
eslint doesnt have test cases for these, but it does flag `lastIndexOf`, this commit adds some test cases, and adds support for `lastIndexof`
Replace `Vec<CompactStr>` with `AllowedOperators` bitflags for checking allowed operators. Since the set of valid operators is fixed and small (6 values), bitflags provide O(1) lookup via bit operations instead of O(n) iteration, and avoid heap allocation for the Vec.

The custom `Deserialize` impl uses a `Visitor` to parse directly from the JSON array without intermediate allocation, and now reports an error for unrecognized operator strings instead of silently ignoring them.
@camc314 camc314 force-pushed the feat/linter-no-implicit-coercion branch from a92a268 to 18ab8db Compare December 11, 2025 11:18
@camc314 camc314 merged commit 3ffe342 into oxc-project:main Dec 11, 2025
21 checks passed
@tt-a1i
Copy link
Copy Markdown
Contributor Author

tt-a1i commented Dec 11, 2025

Thanks for the enhancements! The lastIndexOf addition and bitflags refactor look great 👍

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

Labels

A-linter Area - Linter C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants