Skip to content

feat(linter): implement eslint/camelcase rule#16908

Closed
tt-a1i wants to merge 10 commits intooxc-project:mainfrom
tt-a1i:feat/eslint-camelcase
Closed

feat(linter): implement eslint/camelcase rule#16908
tt-a1i wants to merge 10 commits intooxc-project:mainfrom
tt-a1i:feat/eslint-camelcase

Conversation

@tt-a1i
Copy link
Contributor

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

Summary

Implements the ESLint camelcase rule that enforces camelCase naming conventions for identifiers.

Features:

  • Checks variable declarations, function names, parameters, object properties, class members, imports, exports, and destructuring patterns
  • Supports properties: "always" | "never" option to control property name checking
  • Supports ignoreDestructuring: true option to skip destructuring identifiers
  • Supports ignoreImports: true option to skip import identifiers
  • Supports allow option with literal strings or regex patterns (patterns starting with ^ or ending with $ are treated as regex)
  • Allows leading/trailing underscores (_private, trailing_)
  • Allows ALL_CAPS constant style (CONSTANT_VALUE)

Implementation notes:

  • Pre-compiles regex patterns in from_configuration for better performance
  • Handles both destructuring declarations (const { foo_bar } = obj) and destructuring assignments (({ foo_bar } = obj))

Closes #5535

Implements the `camelcase` rule from ESLint that enforces camelCase
naming conventions for identifiers.

Supports options:
- `properties`: "always" (default) or "never"
- `ignoreDestructuring`: skip destructuring patterns
- `ignoreImports`: skip import specifiers
- `allow`: array of allowed names or regex patterns

Checks:
- Variable declarations
- Function declarations and parameters
- Object property definitions
- Class properties and methods
- Assignment expressions
- Import/export declarations
- Labels
@tt-a1i tt-a1i requested a review from camc314 as a code owner December 15, 2025 15:27
@github-actions github-actions bot added A-linter Area - Linter C-enhancement Category - New feature or request labels Dec 15, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 15, 2025

CodSpeed Performance Report

Merging #16908 will not alter performance

Comparing tt-a1i:feat/eslint-camelcase (fb2d605) with main (8c9cafe)1

Summary

✅ 4 untouched
⏩ 41 skipped2

Footnotes

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

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

@camc314 camc314 self-assigned this Dec 15, 2025
tt-a1i and others added 5 commits December 16, 2025 09:41
- ignoreDestructuring: only skip when local name equals property name
  (e.g., { category_id } skipped, but { category_id: other } checked)
- ignoreImports: only skip when local equals imported name
  (e.g., import { snake } skipped, but import { snake as other } checked)
- allow: treat each entry as both literal and regex (ESLint behavior)
- Add ExportSpecifier check for named exports
- Add BreakStatement/ContinueStatement label reference checks
…elcase

- Add BindingRestElement check for rest destructuring (e.g., `...other_props`)
- Add ArrayPattern check for array destructuring (e.g., `[foo_bar]`)
- Both respect ignoreDestructuring option
- Handles default values in array destructuring (e.g., `[foo_bar = 1]`)
…ucturing

ESLint's ignoreDestructuring only skips object destructuring where
local name equals property name. Array destructuring has no property
names (only indices), so ESLint always checks array elements.

- Remove ignoreDestructuring check from check_binding_pattern
- Move array destructuring test cases to fail section
- Update comments to reflect actual ESLint behavior
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.

I re-ported the test cases and they're failing.

It looks like you missed a lot of test cases when porting this rule originally.

@tt-a1i tt-a1i marked this pull request as draft December 17, 2025 02:27
- Replace AST-handler approach with `run_once` + semantic symbol/reference iteration
- Fix ignoreGlobals to only skip explicitly configured globals + env globals
  (matching ESLint behavior, not skipping all unresolved references)
- Add equalsToOriginalName support for AssignmentTargetPropertyProperty
  (handles `({ foo: foo } = obj)` pattern)
- Fix AssignmentPattern to only skip identifiers on the `right` side
- Split tests requiring globals config into separate Tester run
- Add fail tests for undefined variables with ignoreGlobals: true
@tt-a1i tt-a1i marked this pull request as ready for review December 17, 2025 07:07
@tt-a1i
Copy link
Contributor Author

tt-a1i commented Dec 17, 2025

I re-ported the test cases and they're failing.

It looks like you missed a lot of test cases when porting this rule originally.

Thanks for re-porting these — you’re right, my initial port missed a bunch of ESLint coverage
and I was drifting by patching via extra AST handlers.

I’ve reworked the rule to follow ESLint’s approach more closely using semantic analysis in
run_once (declared symbols + resolved references + unresolved/through refs), and updated the
snapshots based on a full re-port. While doing that I also fixed a few semantic gaps:

  • ignoreGlobals: now only skips explicitly configured globals (and env globals), undefined/
    implicit globals still report
  • ignoreDestructuring: added ({ foo: foo } = obj) equals-to-original handling
  • AssignmentPattern: only skips the right side identifier (ESLint backward-compat behavior)
  • ExportSpecifier: checks the exported name (not local)

The rewrite is in 07ef1b6 (plus 1c38fec to resolve conflicts). Could you take another look /

@tt-a1i tt-a1i requested a review from camc314 December 23, 2025 02:08
@tt-a1i
Copy link
Contributor Author

tt-a1i commented Dec 31, 2025

hey @camc314, just checking if you had a chance to look at the rewrite? no rush, happy holidays!

@bradzacher
Copy link
Collaborator

Based on the PR description and code comments I presume this was - at least in large part - generated using AI tooling.

Please ensure that the PR description is updated to note the AI usage in accordance with our policy, preferably including models/tools used and a confirmation that you have personally reviewed the code generated and ensured it works/makes sense: https://oxc.rs/docs/contribute/introduction.html#ai-usage-policy.


That aside, we've discussed internally and we'd instead be looking to implement @typescript-eslint/naming-convention instead of camelcase given that naming-convention is allows you to do camelcase and much, much more.

@bradzacher bradzacher closed this Jan 1, 2026
@tt-a1i
Copy link
Contributor Author

tt-a1i commented Jan 1, 2026

Based on the PR description and code comments I presume this was - at least in large part - generated using AI tooling.

Please ensure that the PR description is updated to note the AI usage in accordance with our policy, preferably including models/tools used and a confirmation that you have personally reviewed the code generated and ensured it works/makes sense: https://oxc.rs/docs/contribute/introduction.html#ai-usage-policy.

That aside, we've discussed internally and we'd instead be looking to implement @typescript-eslint/naming-convention instead of camelcase given that naming-convention is allows you to do camelcase and much, much more.

makes sense, naming-convention is more comprehensive anyway.
and yeah my bad on the AI disclosure — been adding it to my other PRs

connorshea added a commit that referenced this pull request Jan 1, 2026
…of another rule.

Preference is to implement this via `@typescript-eslint/naming-convention` instead.

See this comment for further context: #16908 (comment)
graphite-app bot pushed a commit that referenced this pull request Jan 2, 2026
…of another rule. (#17571)

Preference is to implement this via [`@typescript-eslint/naming-convention`](https://typescript-eslint.io/rules/naming-convention/) instead.

See this comment for further context: #16908 (comment)
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