Skip to content

Conversation

@camchenry
Copy link
Member

@camchenry camchenry commented Sep 17, 2025

Adds linter codegen to specify run information about what functions a given rule implements. Looks for the impl block for the given rule and looks at all of the rules and checks what functions are implemented. multiple impl blocks cannot be specified for the same struct, so we are guaranteed to be able to know this just from the one block.

On its own, this information doesn't do much for us, but it will allow us to improve the linter runtime performance by optimizing how rules are run based on which functions are implemented and which are no-ops.

@github-actions github-actions bot added A-linter Area - Linter C-performance Category - Solution not expected to change functional behavior, only performance labels Sep 17, 2025
Copy link
Member Author

camchenry commented Sep 17, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

@camchenry camchenry changed the title perf(linter): filter out rules with no applicable run implementations perf(linter): filter out rules that only run on node types if there are no relevant nodes Sep 17, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 17, 2025

CodSpeed Performance Report

Merging #13839 will not alter performance

Comparing 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations (75a529c) with main (2c228ae)1

Summary

✅ 4 untouched
⏩ 33 skipped2

Footnotes

  1. No successful run was found on main (75a529c) during the generation of this report, so 2c228ae was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 33 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.

@camchenry camchenry force-pushed the 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations branch from a764552 to 57f318f Compare September 17, 2025 03:23
@camc314 camc314 changed the base branch from 09-12-perf_linter_add_node_type_codegen_support_for_match_node.kind_ to graphite-base/13839 September 18, 2025 09:34
@camchenry camchenry force-pushed the 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations branch from 57f318f to 882aef8 Compare September 26, 2025 02:46
@camchenry camchenry changed the base branch from graphite-base/13839 to main September 26, 2025 02:46
@camchenry camchenry force-pushed the 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations branch 2 times, most recently from 097cfc4 to 48ae87e Compare September 26, 2025 03:15
@camchenry camchenry removed the C-performance Category - Solution not expected to change functional behavior, only performance label Sep 26, 2025
@camchenry camchenry changed the title perf(linter): filter out rules that only run on node types if there are no relevant nodes feat(linter_codegen): include info about what functions rules implement Sep 26, 2025
@github-actions github-actions bot added the C-enhancement Category - New feature or request label Sep 26, 2025
@camchenry camchenry changed the title feat(linter_codegen): include info about what functions rules implement feat(linter): add run info about what functions rules implement Sep 26, 2025
@camchenry camchenry force-pushed the 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations branch from 48ae87e to 9e44e01 Compare October 9, 2025 01:43
@camchenry camchenry force-pushed the 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations branch from 9e44e01 to 5255b38 Compare October 9, 2025 01:54
@camchenry camchenry marked this pull request as ready for review October 9, 2025 01:56
@camchenry camchenry requested a review from camc314 as a code owner October 9, 2025 01:56
Copilot AI review requested due to automatic review settings October 9, 2025 01:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds linter codegen functionality to detect and specify which functions a rule implements (e.g., run, run_once, run_on_symbol, run_on_jest_node). This information will enable future runtime performance optimizations by allowing the linter to skip calling unimplemented functions.

Key changes:

  • Added detection logic to analyze rule implementation blocks and identify which run functions are implemented
  • Created RuleRunFunctionsImplemented enum to represent the different function implementation patterns
  • Extended code generation to include run function information in the generated rule runner implementations

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

File Description
tasks/linter_codegen/src/main.rs Added detect_rule_run_implementations function and logic to generate run function information
crates/oxc_macros/src/declare_all_lint_rules.rs Added run_info method to rule enum for accessing run function information
crates/oxc_linter/src/rule.rs Defined RuleRunFunctionsImplemented enum and added RUN_FUNCTIONS constant to RuleRunner trait
crates/oxc_linter/src/lib.rs Exported the new RuleRunFunctionsImplemented type

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@camchenry camchenry force-pushed the 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations branch from b396106 to 36c2471 Compare October 9, 2025 02:24
@camchenry camchenry force-pushed the 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations branch from 36c2471 to b19a73d Compare October 11, 2025 23:55
@camc314 camc314 force-pushed the 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations branch from b19a73d to fe5e8c4 Compare October 12, 2025 10:16
Copy link
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.

nice work!

Adds linter codegen to specify run information about what functions a given rule implements. Looks for the `impl` block for the given rule and looks at all of the rules and checks what functions are implemented. multiple `impl` blocks cannot be specified for the same struct, so we are guaranteed to be able to know this just from the one block.

On its own, this information doesn't do much for us, but it will allow us to improve the linter runtime performance by optimizing how rules are run based on which functions are implemented and which are no-ops.
@graphite-app graphite-app bot force-pushed the 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations branch from fe5e8c4 to 75a529c Compare October 12, 2025 10:22
@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 12, 2025

Merge activity

@graphite-app graphite-app bot merged commit 75a529c into main Oct 12, 2025
20 checks passed
@graphite-app graphite-app bot deleted the 09-16-perf_linter_filter_out_rules_with_no_applicable_run_implementations branch October 12, 2025 10:27
camc314 added a commit that referenced this pull request Oct 13, 2025
## [1.23.0] - 2025-10-13

### 🚀 Features

- 452d128 linter: Node/no_process_env (#14536) (Kenneth Skovhus)
- a9e1261 linter: Implement react-refresh/only-export-components
(#14122) (Rintaro Itokawa)
- a8d542b linter/no-duplicate-imports: Support
`allowSeparateTypeImports` option (#14527) (camc314)
- 75a529c linter: Add run info about what functions rules implement
(#13839) (camchenry)
- 4fbdef2 language_server: Support `fmt.configPath` configuration
(#14502) (Sysix)
- 4d3ce2e language_server: Autodetect root `.oxfmtrc.json` (#14466)
(Sysix)

### 🐛 Bug Fixes

- fb4a20d language_server: Add whitespace for `//
oxlint-disable-next-line` fix (#14356) (Sysix)
- 6fce7f4 oxlint/changelog: Remove duplicate changelog entries (#14528)
(camc314)
- 74e52f3 linter/plugins: Resolve JS plugins with ESM condition names
(#14541) (magic-akari)
- 9965676 linter/exhaustive-deps: Ignore empty strings passed to
`additionalHooks` option (#14464) (Redas)
- d0a8e3e linter: `vue/no_required_prop_with_default` called
`Option::unwrap()` on a `None` value (#14491) (Sysix)
- 1192ea2 linter: Correct `nextjs/no-typos` path handling (#14480)
(shulaoda)
- 2796b16 tsgolint: Pipe tsgolints stderr (#14477) (camc314)

### 🚜 Refactor

- 6440cde linter: Remove lifetime of `Message` (#14481) (Sysix)
- f599bef language_server: Move `Message` to `DiagnosticReport`
transformation to one place (#14447) (Sysix)
- 4f301de napi/parser, linter/plugins: Improve formatting of generated
code (#14554) (overlookmotel)
- 68c0252 napi/parser, linter/plugins: Shorten generated raw transfer
deserializer code (#14553) (overlookmotel)
- 20e884e linter: Store `LintService` in `LintRunner` (#14471) (Sysix)
- 9e9c5ba linter: Simplify built-in lint plugin checks (#14518)
(camchenry)
- 1986e0f linter/no-ex-assign: Use let-else chain (#14526) (camc314)
- ce9bcf0 linter/no-func-assign: Use let-else chain (#14525) (camc314)
- a9cea7c language_server: Use `FxHashSet` for
`ServerLinter::extended_paths` (#14517) (Sysix)
- ea5838e linter/no-import-assign: Use let-else chain (#14524) (camc314)
- f977700 language_server: Pass `LintOptions` to
`Worker::refresh_server_linter` (#14510) (Sysix)
- 54b001f linter/no-new-require: Improve diagnostic message clarity
(#14511) (shulaoda)
- 650ea68 linter: Improve nextjs/no-typos rule (#14476) (shulaoda)

### 📚 Documentation

- 198f2e9 linter: Fix code example for `branches-sharing-code` (#14514)
(camc314)
- d776a17 linter: Improve `nextjs/no-typos` rule documentation (#14470)
(shulaoda)

### ⚡ Performance

- b44a30e language_server: Transform `Message` to `DiagnosticReport`
with one call (#14448) (Sysix)
- 31766fd linter/plugins: Provide `loc` via prototype (#14552)
(overlookmotel)
- aec0c08 linter: Allow analyzing node types in match blocks with guards
(#14459) (camchenry)
- 9044187 linter: Skip running node-specific rule if file contains no
relevant nodes (#14457) (camchenry)
- 422f54e linter: Only run rule run functions if implemented (#14454)
(camchenry)
- 2c228ae liner: Use top-level match for `no_obj_calls` (#14523)
(camchenry)
- c49d891 linter: Use match for `no_negated_condition` (#14522)
(camchenry)
- e222fc2 linter: Use match for `no_multi_assign` (#14521) (camchenry)
- 4440516 linter: Refactor rules to take advantage of node type skipping
(#14519) (camchenry)
- 78261d6 linter: Refactor `no-invalid-fetch-options` to be more easily
analyzed (#14458) (camchenry)

### 🧪 Testing

- 8d8881d linter/plugins: Expand tests for module resolution of plugins
(#14559) (overlookmotel)
- 33b6cde language_server: Add basic tests for
`WorkspaceWorker::did_change_configuration` (#14531) (Sysix)
- bfe1ecd language_server: Add tests for
`WorkspaceWorker::init_watchers` (#14516) (Sysix)

Co-authored-by: camc314 <18101008+camc314@users.noreply.github.com>
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.

3 participants