feat(linter/approx-constant): implement fixer#16847
Conversation
github-actions
bot
added
A-linter
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
camc314
force-pushed
the
c/12-14-feat_linter_approx-constant_implement_fixer
branch
2 times, most recently
from
9cbb26e to
5c58c95
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements an auto-fixer for the oxc/approx-constant linter rule and adds comprehensive oxlint plugin demo files. The approx-constant rule now provides suggestions to replace approximate constant values with their Math object equivalents (e.g., 3.14159 → Math.PI). The PR also includes a detailed investigation document about an unrelated oxlint hang issue in the next.js ecosystem CI.
Key Changes
- Implements suggestion fixer for the
approx-constantrule that replaces numeric literals with Math constants - Adds 5 progressive demo examples showing how to write custom oxlint plugins (from basic rules to complex configurations)
- Includes HANG_INVESTIGATION.md documenting a performance issue with the
constructor-superrule
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
crates/oxc_linter/src/rules/oxc/approx_constant.rs |
Added fixer implementation using diagnostic_with_suggestion and changed rule status from pending to suggestion. Includes comprehensive test cases for all supported Math constants. |
oxlint-plugin-demo/README.md |
Comprehensive documentation for writing custom oxlint plugins with 5 progressive examples covering basic rules, fixers, selectors, configuration options, and message IDs. |
oxlint-plugin-demo/demo-1/01-basic-plugin.ts |
Basic plugin example demonstrating comment detection (TODO/FIXME/HACK). |
oxlint-plugin-demo/demo-2/02-fixable-rule.ts |
Demonstrates auto-fixable rules with no-var and no-debugger examples. |
oxlint-plugin-demo/demo-3/03-selectors.ts |
Shows CSS-like selector usage for matching AST patterns. |
oxlint-plugin-demo/demo-4/04-rule-with-options.ts |
Demonstrates configurable rules with user-provided options. |
oxlint-plugin-demo/demo-5/05-message-ids.ts |
Shows message IDs and placeholders for organized error messages. |
oxlint-plugin-demo/demo-[1-5]/{test.js,oxlintrc.json,package.json} |
Supporting test files and configuration for each demo. |
oxlint-plugin-demo/test-file.js |
Comprehensive test file exercising all demo plugin rules. |
HANG_INVESTIGATION.md |
Detailed analysis of exponential path explosion in constructor-super rule causing hangs on large Next.js files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merge activity
|
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The core ApproxConstant fixer is a solid step forward, but it should avoid emitting multiple competing fixes for one literal and should be more conservative when Math may be shadowed. The new oxlint-plugin-demo content has several correctness/UX issues: inconsistent #oxlint vs oxlint imports, a top-level config that points at non-existent plugin paths, and a test/config that references rules that aren’t implemented. A committed *.profraw profiling artifact should not live in the repo and needs removal/ignore rules.
Additional notes (2)
-
Maintainability |
default_14849635966089712973_0_16428.profraw:1-1
This adds a raw profiler output file to the repo. These artifacts are typically machine-specific, large, and noisy in diffs, and they tend to bloat the repository history. Unless there’s a clear policy/need for committing*.profraw(e.g., reproducible perf regression evidence that is consumed by CI), it should be excluded and uploaded to CI artifacts instead. -
Maintainability |
crates/oxc_linter/src/rules/oxc/approx_constant.rs:51-51
number_lit_stris generated vianumber_literal.value.to_string(), which can normalize/round values and produce formats (e.g., scientific notation) that don’t reflect the original source. That can cause the matcher to trigger (or not trigger) inconsistently compared to what the user typed, and it can also generate a fix that replaces a literal that wasn’t actually “approximate” textually.
Given this PR adds a fixer, correctness here matters more: the rule should ideally compare against the source text of the literal (including user formatting) and only then apply a fix.
Summary of changes
Summary of changes
ApproxConstant rule now offers an auto-fix
- Updated
crates/oxc_linter/src/rules/oxc/approx_constant.rsto make the rule fixable via suggestions:- Switched from
pendingtosuggestionindeclare_oxc_lint!. - Added
ctx.diagnostic_with_suggestion(...)to propose replacing numeric approximations withMath.<CONST>. - Introduced helper
ApproxConstant::fix_with_math_constant(...)usingRuleFixer::replace. - Extended tests to validate fixes via
Tester::expect_fix(...).
- Switched from
Added documentation / demo content
- Added
oxlint-plugin-demo/with five progressive plugin examples (basic rule, fixable rule, selectors, options, message IDs) plus configs and test files. - Added
HANG_INVESTIGATION.mddocumenting an ecosystem CI hang inconstructor-super.
Added profiling artifact
- Added a binary profiling file:
default_14849635966089712973_0_16428.profraw.
CodSpeed Performance ReportMerging #16847 will not alter performanceComparing Summary
Footnotes
|
camc314
force-pushed
the
c/12-14-feat_linter_approx-constant_implement_fixer
branch
from
5c58c95 to
fe8f421
Compare
graphite-app
bot
force-pushed
the
c/12-14-feat_linter_approx-constant_implement_fixer
branch
from
fe8f421 to
99c0496
Compare
graphite-app
bot
deleted the
c/12-14-feat_linter_approx-constant_implement_fixer
branch
No description provided.