Skip to content

Conversation

@howenyap
Copy link
Contributor

Related to this issue where missing commas in declare_oxc_lint! would cause documentation to be generated wrongly without erroring.

In the image below, there is no warning about the missing comma after dangerou_suggestion, this causes config to not be generated in the documentation.

image

With the fix, users will be notified about the missing comma.

image

Copilot AI review requested due to automatic review settings October 22, 2025 06:14
@howenyap howenyap requested a review from camc314 as a code owner October 22, 2025 06:14
@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 22, 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.

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 fixes a bug in the declare_oxc_lint! macro where missing commas between parameters would silently fail, causing incorrect documentation generation. The fix adds validation to error when unexpected tokens remain after parsing all expected parameters.

  • Added error checking for remaining tokens after parsing lint rule metadata
  • Fixed existing lint rule declarations that had missing or duplicate commas
  • Improved error message to guide users toward the likely issue (missing comma)

Reviewed Changes

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

File Description
crates/oxc_macros/src/declare_oxc_lint.rs Added validation to detect and error on unexpected tokens with helpful error message
crates/oxc_linter/src/rules/react/forward_ref_uses_ref.rs Removed duplicate "suggestion" parameter
crates/oxc_linter/src/rules/oxc/bad_bitwise_operator.rs Added missing comma after "restriction" parameter
crates/oxc_linter/src/rules/eslint/max_depth.rs Added missing comma after "pedantic" parameter

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

@github-actions github-actions bot added A-linter Area - Linter C-bug Category - Bug labels Oct 22, 2025
@howenyap
Copy link
Contributor Author

cc @connorshea

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.

Thank you!

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 22, 2025

CodSpeed Performance Report

Merging #14881 will not alter performance

Comparing howenyap:linter/missing-commas-in-lint-rule-declarations (bf0f75d) with main (a1cda35)

Summary

✅ 4 untouched
⏩ 33 skipped1

Footnotes

  1. 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.

@camc314 camc314 merged commit 801e1d1 into oxc-project:main Oct 22, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants