-
Notifications
You must be signed in to change notification settings - Fork 54
feat: add sort-regexp rule #600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a new ESLint rule "sort-regexp" with full implementation, types, helpers, extensive tests, documentation, plugin registration, and a new dependency ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant ESLint
participant Rule as sort-regexp
participant RegexPP as regexpp
participant Builder as NodeBuilder
participant Sorter as SortEngine
ESLint->>Rule: encounter RegExp literal
Rule->>RegexPP: parse literal -> AST
RegexPP-->>Rule: RegExp AST
Note right of Rule: analyze flags, alternatives, character classes
Rule->>Builder: build sorting nodes (flags / alternatives / char-class elements)
Builder-->>Sorter: nodes with groups & keys
Sorter-->>Rule: sorted order + fix instructions
alt violation detected
Rule-->>ESLint: report violation(s) and provide fix(es)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this 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 pull request adds a new sort-regexp rule to the ESLint Perfectionist plugin that enforces consistent ordering in regular expressions. The rule sorts three main components: regex flags, character class elements, and alternation branches within capture groups or top-level patterns.
Key Changes:
- New rule implementation that sorts regex flags, character classes, and alternation branches
- Comprehensive test suite covering alphabetical, natural, line-length, custom, and unsorted sorting modes
- Documentation and configuration updates to integrate the new rule
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rules/sort-regexp.ts | Core rule implementation with sorting logic for regex components |
| rules/sort-regexp/types.ts | Type definitions and configuration options for the rule |
| test/rules/sort-regexp.test.ts | Extensive test suite covering all sorting modes and edge cases |
| docs/content/rules/sort-regexp.mdx | Complete documentation with usage examples and configuration options |
| package.json | Added regexpp dependency for regex AST parsing |
| index.ts | Exported the new rule in the plugin's rule collection |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
test/rules/sort-regexp.test.ts
Outdated
| @@ -0,0 +1,2835 @@ | |||
| /* Cspell:ignore gimsu igmus yusmig ysumgi Ωmega Δelta */ | |||
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'gimsu' to 'gimus'
| /* Cspell:ignore gimsu igmus yusmig ysumgi Ωmega Δelta */ | |
| /* Cspell:ignore gimus igmus yusmig ysumgi Ωmega Δelta */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦
rules/sort-regexp.ts
Outdated
| function codePointToString(value: number): string { | ||
| return String.fromCodePoint(value) | ||
| } |
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is a simple wrapper around String.fromCodePoint. Consider using the native method directly or add JSDoc comments to explain why this abstraction is needed.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #600 +/- ##
===========================================
Coverage 100.00% 100.00%
===========================================
Files 111 126 +15
Lines 8979 10389 +1410
Branches 1741 2121 +380
===========================================
+ Hits 8979 10389 +1410 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (9)
rules/sort-regexp/types.ts (1)
64-75: Prefer const and literal types for exported constantsUse const (and literal typing) to prevent accidental mutation and improve type inference.
-export let allSelectors: Selector[] = ['alias', 'pattern'] +export const allSelectors = ['alias', 'pattern'] as const -export let allModifiers: Modifier[] = [] +export const allModifiers: Modifier[] = [] -export let singleCustomGroupJsonSchema: Record<string, JSONSchema4> = { +export const singleCustomGroupJsonSchema: Record<string, JSONSchema4> = { selector: buildCustomGroupSelectorJsonSchema(allSelectors as unknown as string[]), elementValuePattern: regexJsonSchema, elementNamePattern: regexJsonSchema, }docs/content/rules/sort-regexp.mdx (1)
267-271: Avoid hardcoding release version until it shipsPackage.json is 4.15.1 in this PR. Don’t claim v5.0.0 yet.
-## Version - -This rule was introduced in [v5.0.0](...). +## Version + +Unreleased (will be included in the next major release). Update this section with the exact tag after publishing.test/rules/sort-regexp.test.ts (2)
1182-1206: Add guard tests for non-commutative alternations and backrefsTo prevent unsafe fixes, add tests that must not report/fix when reordering could change behavior.
@@ describe('alphabetical', () => { @@ it('works with complex cases', async () => { @@ }) + + it.skip('does not reorder shared-prefix alternatives (behavior would change)', async () => { + await valid({ + // (ab|a) → (a|ab) would change which alternative matches + code: dedent`/(ab|a)/`, + options: [options], + }) + }) + + it.skip('does not reorder when numeric backreferences depend on group order', async () => { + await valid({ + // Reordering changes numbering of (b) vs (d), altering \2 semantics + code: dedent`/(?:c(d)|a(b))\\2/`, + options: [options], + }) + })Please un-skip once the rule detects and bails out for these cases.
2401-2468: Character class edge cases: ensure fix preserves literal “-” and “]”Sorting must not turn literal “-” into a range or move “]” to an invalid position. Add tests to lock behavior.
@@ describe('line-length', () => { @@ it('sorts character class elements', async () => { @@ }) + + it('preserves literal hyphen placement in character classes', async () => { + await valid({ + code: dedent`/[-ab]/`, // '-' first is safe + options: [options], + }) + await valid({ + code: dedent`/[ab-]/`, // '-' last is safe + options: [options], + }) + await valid({ + code: dedent`/[a\\-b]/`, // escaped '-' is safe anywhere + options: [options], + }) + }) + + it('does not produce invalid character classes with closing bracket', async () => { + await valid({ + code: dedent`/[\\]ab]/`, // ']' escaped inside class + options: [options], + }) + })Run the suite to confirm the fixer never introduces invalid ranges or bracket placement.
rules/sort-regexp.ts (5)
243-248: Consider skipping character-class sorting under the /v flag until set-operations are handled.ECMAScript’s /v introduces class set operations; naive reordering may change semantics. Either detect and skip when flags include 'v' or add targeted handling/tests.
Add a conservative guard:
onCharacterClassLeave(characterClass) { let { elements, negate, start, end } = characterClass + if (literalNode.regex.flags.includes('v')) { + return + } if (!isSortable(elements)) { return }Please add tests covering intersections/subtractions with /v.
376-379: Do not enable this rule in “recommended” until unsafe cases are blocked.Given the behavior-changing edge cases above, ship as non‑recommended first. Flip to recommended after guards are in place and tests cover them.
- recommended: true, + recommended: false,
641-649: Tighten the type guard or include lookarounds intentionally.isCapturingContext never returns true for lookarounds but its predicate type includes LookaroundAssertion. Either add lookarounds to the check or narrow the return type.
function isCapturingContext( node: Alternative['parent'], -): node is LookaroundAssertion | CapturingGroup | Pattern | Group { +): node is CapturingGroup | Pattern | Group { return ( node.type === 'CapturingGroup' || node.type === 'Group' || node.type === 'Pattern' ) }
178-242: Single-pass visitor for minor perf/readability win.Both visitors traverse the same AST. Merge onAlternativeLeave and onCharacterClassLeave into a single visitRegExpAST call.
Also applies to: 243-348
375-386: Meta: keep fixers, but consider “suggest only” for alternatives until safe.If you prefer not to flip recommended, make alternative reordering report-only (no fix) when ambiguity is detected; keep autofix for flags/character classes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (8)
docs/content/rules/sort-regexp.mdx(1 hunks)docs/public/llms.txt(1 hunks)index.ts(3 hunks)package.json(1 hunks)readme.md(1 hunks)rules/sort-regexp.ts(1 hunks)rules/sort-regexp/types.ts(1 hunks)test/rules/sort-regexp.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
rules/sort-regexp.ts (19)
types/sorting-node.ts (1)
SortingNode(18-102)rules/sort-regexp/types.ts (5)
Options(22-41)allSelectors(65-65)allModifiers(68-68)Group(78-78)Selector(59-59)utils/create-eslint-rule.ts (1)
createEslintRule(32-35)utils/get-settings.ts (1)
getSettings(91-136)utils/complete.ts (1)
complete(30-45)utils/validate-custom-sort-configuration.ts (1)
validateCustomSortConfiguration(25-35)utils/validate-generated-groups-configuration.ts (1)
validateGeneratedGroupsConfiguration(74-95)utils/get-eslint-disabled-lines.ts (1)
getEslintDisabledLines(45-97)utils/sort-nodes.ts (1)
sortNodes(39-76)utils/create-node-index-map.ts (1)
createNodeIndexMap(27-35)utils/pairwise.ts (1)
pairwise(29-44)utils/sort-nodes-by-groups.ts (1)
sortNodesByGroups(106-162)utils/report-all-errors.ts (1)
reportAllErrors(203-325)utils/compare.ts (1)
compare(108-142)utils/common-json-schemas.ts (3)
commonJsonSchemas(139-140)buildCustomGroupsArrayJsonSchema(302-351)groupsJsonSchema(165-190)utils/report-errors.ts (1)
ORDER_ERROR(21-22)utils/compute-group.ts (1)
computeGroup(75-113)utils/does-custom-group-match.ts (1)
doesCustomGroupMatch(121-141)utils/is-node-eslint-disabled.ts (1)
isNodeEslintDisabled(27-32)
rules/sort-regexp/types.ts (2)
types/common-options.ts (4)
CustomGroupsOption(103-134)GroupsOptions(427-433)CommonOptions(23-73)RegexOption(459-459)utils/common-json-schemas.ts (2)
buildCustomGroupSelectorJsonSchema(431-439)regexJsonSchema(219-228)
test/rules/sort-regexp.test.ts (2)
utils/alphabet.ts (1)
Alphabet(46-467)test/utils/validate-rule-json-schema.ts (1)
validateRuleJsonSchema(8-19)
🪛 ESLint
test/rules/sort-regexp.test.ts
[error] 33-33: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 36-36: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 45-45: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 54-54: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 69-69: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 72-72: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 87-87: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 90-90: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 111-111: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 114-114: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 128-128: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 131-131: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 146-146: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 149-149: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 164-164: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 167-167: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 176-176: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 191-191: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 194-194: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 209-209: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 212-212: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 231-231: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 234-234: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 253-253: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 256-256: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 271-271: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 274-274: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 289-289: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 292-292: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 310-310: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 313-313: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 328-328: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 331-331: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 352-352: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 355-355: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 369-369: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 372-372: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 391-391: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 394-394: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 403-403: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 412-412: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 431-431: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 434-434: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 449-449: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 452-452: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 461-461: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 476-476: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 479-479: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 494-494: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 497-497: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 518-518: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 521-521: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 552-552: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 555-555: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 569-569: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 572-572: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 591-591: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 594-594: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 609-609: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 612-612: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 627-627: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 630-630: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 639-639: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 658-658: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 661-661: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 676-676: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 679-679: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 694-694: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 697-697: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 720-720: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 723-723: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 738-738: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 741-741: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 756-756: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 759-759: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 774-774: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 777-777: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 792-792: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 795-795: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 818-818: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 821-821: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 836-836: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 839-839: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 854-854: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 857-857: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 872-872: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 875-875: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 910-910: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 913-913: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 922-922: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 931-931: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 946-946: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 949-949: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 964-964: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 967-967: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 982-982: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 985-985: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1000-1000: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1003-1003: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1018-1018: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1021-1021: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1040-1040: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1043-1043: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1073-1073: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1076-1076: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1091-1091: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1094-1094: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1109-1109: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1112-1112: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1127-1127: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1130-1130: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1145-1145: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1148-1148: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1157-1157: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1166-1166: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1175-1175: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1198-1198: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1201-1201: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1222-1222: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1225-1225: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1234-1234: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1243-1243: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1258-1258: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1261-1261: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1276-1276: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1279-1279: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1300-1300: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1303-1303: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1317-1317: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1320-1320: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1335-1335: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1338-1338: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1353-1353: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1356-1356: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1365-1365: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1380-1380: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1383-1383: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1397-1397: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1400-1400: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1419-1419: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1422-1422: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1442-1442: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1445-1445: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1464-1464: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1467-1467: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1482-1482: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1485-1485: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1500-1500: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1503-1503: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1521-1521: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1524-1524: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1539-1539: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1542-1542: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1563-1563: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1566-1566: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1580-1580: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1583-1583: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1602-1602: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1605-1605: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1614-1614: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1623-1623: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1642-1642: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1645-1645: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1660-1660: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1663-1663: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1672-1672: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1687-1687: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1690-1690: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1705-1705: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1708-1708: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1729-1729: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1732-1732: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1763-1763: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1766-1766: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1784-1784: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1787-1787: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1802-1802: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1805-1805: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1820-1820: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1823-1823: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1838-1838: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1841-1841: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1850-1850: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1865-1865: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1868-1868: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1883-1883: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1886-1886: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1901-1901: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1904-1904: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1927-1927: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1930-1930: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1945-1945: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1948-1948: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1963-1963: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1966-1966: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1981-1981: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1984-1984: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1999-1999: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2002-2002: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2021-2021: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2024-2024: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2039-2039: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2042-2042: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2057-2057: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2060-2060: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2075-2075: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2078-2078: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2113-2113: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2116-2116: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2125-2125: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2134-2134: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2149-2149: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2152-2152: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2167-2167: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2170-2170: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2185-2185: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2188-2188: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2203-2203: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2206-2206: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2221-2221: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2224-2224: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2243-2243: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2246-2246: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2276-2276: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2279-2279: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2294-2294: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2297-2297: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2312-2312: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2315-2315: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2330-2330: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2333-2333: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2348-2348: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2351-2351: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2360-2360: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2369-2369: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2378-2378: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2401-2401: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2404-2404: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2419-2419: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2429-2429: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2432-2432: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2447-2447: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2450-2450: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2465-2465: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2468-2468: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2477-2477: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2492-2492: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2495-2495: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2513-2513: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2516-2516: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2535-2535: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2538-2538: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2547-2547: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2562-2562: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2565-2565: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2580-2580: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2583-2583: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2598-2598: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2601-2601: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2616-2616: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2619-2619: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2628-2628: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2631-2631: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2647-2647: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2650-2650: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2665-2665: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2668-2668: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2683-2683: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2686-2686: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2704-2704: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2707-2707: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2722-2722: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2725-2725: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2744-2744: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2747-2747: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2767-2767: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2783-2783: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2786-2786: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2802-2802: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2811-2811: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2820-2820: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
🔇 Additional comments (5)
package.json (1)
60-63: Dependency addition LGTMAdding @eslint-community/regexpp as a runtime dep matches the rule’s needs. No issues.
readme.md (1)
196-196: Rule table entry looks goodEntry formatting and link match existing rows.
docs/public/llms.txt (1)
47-47: Docs index update LGTMConsistent with other entries.
index.ts (1)
23-50: Rule wiring is correctImport, typing in PluginConfig, and rules map entry are consistent.
Also applies to: 97-98
rules/sort-regexp.ts (1)
1-68: Test coverage is comprehensive for standard patterns but missing edge cases for newer regex features.The test file contains 156 test cases across 2835 lines and thoroughly covers:
- Non-capturing groups and alternations
- Character classes with ranges and special characters
- All standard flags (i, g, m, s, u, y)
- Complex nested patterns, lookahead/lookbehind assertions
However, the tests do not include:
- The
/vflag (Unicode Sets, ES2024)- Character class intersection/subtraction operators (
&&,-at boundaries)- Character classes with dashes in boundary positions
The code structure (lines 1-68) is solid with proper imports and integration with shared utilities. Whether the missing edge case tests are acceptable depends on the project's scope and target environment compatibility. Verify if these features should be supported.
| The rule is **safe** – it only reorders elements without changing their meaning. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs overstate safety; add caveat and guidance
Reordering alternations is not universally safe (e.g., shared prefixes like (ab|a)), and numeric backreferences can also change behavior if group numbering shifts after reordering.
- The rule is **safe** – it only reorders elements without changing their meaning.
+ Important: Reordering can change behavior in specific cases (e.g., alternatives that share a prefix like `(ab|a)`, or patterns whose numeric backreferences depend on the textual order of capturing groups). The rule avoids such changes when it detects risk; otherwise, prefer `type: 'unsorted'` or disable the rule locally for those patterns.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The rule is **safe** – it only reorders elements without changing their meaning. | |
| Important: Reordering can change behavior in specific cases (e.g., alternatives that share a prefix like `(ab|a)`, or patterns whose numeric backreferences depend on the textual order of capturing groups). The rule avoids such changes when it detects risk; otherwise, prefer `type: 'unsorted'` or disable the rule locally for those patterns. |
🤖 Prompt for AI Agents
In docs/content/rules/sort-regexp.mdx around lines 22-23, the claim "The rule is
**safe** – it only reorders elements without changing their meaning." overstates
safety; update the text to add a short caveat and guidance: state that
reordering alternations can change behavior for patterns with shared prefixes
(e.g., (ab|a)) and when numeric capture group indices are relied upon by
backreferences, and advise when the rule is safe to apply (alternations are
independent, use non-capturing groups (?:...) or named captures to preserve
backreference semantics) and when to avoid it; include one minimal example of a
problematic case and a recommended fix or workaround (use non-capturing groups
or reorder/escape to preserve intended match) so readers know when to
enable/disable the rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hugop95 I fixed it
| let eslintDisabledLines = getEslintDisabledLines({ | ||
| ruleName: id, | ||
| sourceCode, | ||
| }) | ||
|
|
||
| let ast = parseRegExpLiteral(literalNode.raw) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Respect eslint-disable for the literal; bail out early.
Flags/character-class paths will still report even when the rule is disabled on that line. Add an early return after computing disabled lines.
let eslintDisabledLines = getEslintDisabledLines({
ruleName: id,
sourceCode,
})
+ // Skip this literal if the rule is disabled for its line
+ if (isNodeEslintDisabled(literalNode, eslintDisabledLines)) {
+ return
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In rules/sort-regexp.ts around lines 107 to 112, after computing
eslintDisabledLines with getEslintDisabledLines({ ruleName: id, sourceCode }),
add an early return that checks whether the regexp literal node's starting line
(literalNode.loc.start.line) is disabled for this rule and bail out if so; this
prevents further processing (like parseRegExpLiteral and subsequent
flag/character-class checks) when eslint-disable covers the literal's line.
| let sortedElements = [...elements].toSorted((a, b) => { | ||
| let aKey = getCharacterClassElementSortKey(a) | ||
| let bKey = getCharacterClassElementSortKey(b) | ||
|
|
||
| if ( | ||
| options.type !== 'line-length' && | ||
| aKey.category !== bKey.category | ||
| ) { | ||
| let categoryDiff = aKey.category - bKey.category | ||
| return options.order === 'asc' ? categoryDiff : -categoryDiff | ||
| } | ||
|
|
||
| let aNode = createCharacterClassSortingNode({ | ||
| literalNode, | ||
| element: a, | ||
| }) | ||
| let bNode = createCharacterClassSortingNode({ | ||
| literalNode, | ||
| element: b, | ||
| }) | ||
|
|
||
| let comparison = compare({ | ||
| fallbackSortNodeValueGetter: null, | ||
| nodeValueGetter: null, | ||
| a: aNode, | ||
| b: bNode, | ||
| options, | ||
| }) | ||
|
|
||
| if (comparison !== 0) { | ||
| return comparison | ||
| } | ||
|
|
||
| let rawComparison = aKey.raw.localeCompare( | ||
| bKey.raw, | ||
| options.locales, | ||
| { | ||
| sensitivity: options.ignoreCase ? 'base' : 'variant', | ||
| numeric: options.type === 'natural', | ||
| }, | ||
| ) | ||
|
|
||
| let rawOrderMultiplier = 1 | ||
| if (options.type !== 'line-length' && options.order !== 'asc') { | ||
| rawOrderMultiplier = -1 | ||
| } | ||
|
|
||
| return rawOrderMultiplier * rawComparison | ||
| }) | ||
|
|
||
| let needsSort = elements.some( | ||
| (element, i) => element !== sortedElements[i], | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent accidental ranges in character classes caused by bare '-'.
Sorting can move an unescaped '-' from edges into the middle, turning it into a range. Ensure bare '-' literals are kept at the end (or skip sorting).
- let sortedElements = [...elements].toSorted((a, b) => {
+ let sortedElements = [...elements].toSorted((a, b) => {
let aKey = getCharacterClassElementSortKey(a)
let bKey = getCharacterClassElementSortKey(b)
…
return rawOrderMultiplier * rawComparison
})
+
+ // Safety: keep any bare '-' (raw === '-') at the end to avoid creating unintended ranges.
+ if (sortedElements.some(el => el.type === 'Character' && el.raw === '-')) {
+ let hyphens: typeof sortedElements = []
+ let others: typeof sortedElements = []
+ for (let el of sortedElements) {
+ if (el.type === 'Character' && el.raw === '-') hyphens.push(el)
+ else others.push(el)
+ }
+ sortedElements = [...others, ...hyphens]
+ }🤖 Prompt for AI Agents
In rules/sort-regexp.ts around lines 254 to 306, the current sort can move an
unescaped '-' literal from an edge into the middle of a character class,
creating an accidental range; modify the comparator so that bare (unescaped) '-'
elements are forced to the end of the sortedElements (or cause the function to
skip sorting when such a move would place '-' in the middle). Concretely: detect
an element that is a literal '-' and not escaped, and give it a sort priority
that always ranks it after all other elements (taking options.order into
account), or bail out of sorting for that character class when preserving edge
'-' placement cannot be guaranteed. Ensure this change only applies to unescaped
'-' and leaves other elements’ ordering logic intact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
rules/sort-regexp/has-shadowing-alternatives.ts (1)
18-28: Consider avoiding non-null assertions.While the non-null assertions on lines 19 and 22 are technically safe (indices are guaranteed valid), they bypass TypeScript's type safety. Consider a refactor that avoids them.
- for (let index = 0; index < rawAlternatives.length; index++) { - let current = rawAlternatives[index]! - - for (let offset = index + 1; offset < rawAlternatives.length; offset++) { - let other = rawAlternatives[offset]! - + for (let index = 0; index < rawAlternatives.length; index++) { + let current = rawAlternatives[index] + if (!current) continue + + for (let offset = index + 1; offset < rawAlternatives.length; offset++) { + let other = rawAlternatives[offset] + if (!other) continue + if (doesAlternativeShadowOther(current, other)) { return true } } }Alternatively, use a for-of loop with array slicing:
- for (let index = 0; index < rawAlternatives.length; index++) { - let current = rawAlternatives[index]! - - for (let offset = index + 1; offset < rawAlternatives.length; offset++) { - let other = rawAlternatives[offset]! - - if (doesAlternativeShadowOther(current, other)) { - return true - } - } - } + for (let index = 0; index < rawAlternatives.length; index++) { + let current = rawAlternatives[index] + if (!current) continue + + for (let other of rawAlternatives.slice(index + 1)) { + if (doesAlternativeShadowOther(current, other)) { + return true + } + } + }rules/sort-regexp/get-character-class-element-sort-key.ts (1)
7-11: Consider aligning property order with the return statement.The interface defines properties in order
normalized,category,raw, while the return statement usescategory,normalized,raw. Though functionally equivalent, matching the order improves consistency and readability.Apply this diff to align the property order:
export interface CharacterClassElementSortKey { - normalized: string category: number + normalized: string raw: string }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
rules/sort-regexp.ts(1 hunks)rules/sort-regexp/code-point-to-string.ts(1 hunks)rules/sort-regexp/create-character-class-sorting-node.ts(1 hunks)rules/sort-regexp/create-flag-sorting-nodes.ts(1 hunks)rules/sort-regexp/create-pseudo-literal-node.ts(1 hunks)rules/sort-regexp/create-sorting-node.ts(1 hunks)rules/sort-regexp/does-alternative-shadow-other.ts(1 hunks)rules/sort-regexp/get-alternative-alias.ts(1 hunks)rules/sort-regexp/get-character-class-element-category.ts(1 hunks)rules/sort-regexp/get-character-class-element-raw.ts(1 hunks)rules/sort-regexp/get-character-class-element-sort-key.ts(1 hunks)rules/sort-regexp/get-character-class-element-value.ts(1 hunks)rules/sort-regexp/get-selector.ts(1 hunks)rules/sort-regexp/get-sorting-node-name.ts(1 hunks)rules/sort-regexp/has-shadowing-alternatives.ts(1 hunks)rules/sort-regexp/is-capturing-context.ts(1 hunks)rules/sort-regexp/is-digit-character.ts(1 hunks)rules/sort-regexp/is-lowercase-character.ts(1 hunks)rules/sort-regexp/is-uppercase-character.ts(1 hunks)test/rules/sort-regexp.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- rules/sort-regexp/code-point-to-string.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- rules/sort-regexp.ts
🧰 Additional context used
🧬 Code graph analysis (10)
rules/sort-regexp/is-uppercase-character.ts (1)
rules/sort-regexp/code-point-to-string.ts (1)
codePointToString(7-9)
rules/sort-regexp/get-character-class-element-category.ts (3)
rules/sort-regexp/is-digit-character.ts (1)
isDigitCharacter(11-13)rules/sort-regexp/is-uppercase-character.ts (1)
isUppercaseCharacter(11-13)rules/sort-regexp/is-lowercase-character.ts (1)
isLowercaseCharacter(11-13)
rules/sort-regexp/has-shadowing-alternatives.ts (1)
rules/sort-regexp/does-alternative-shadow-other.ts (1)
doesAlternativeShadowOther(8-25)
rules/sort-regexp/is-digit-character.ts (1)
rules/sort-regexp/code-point-to-string.ts (1)
codePointToString(7-9)
rules/sort-regexp/get-character-class-element-sort-key.ts (3)
rules/sort-regexp/get-character-class-element-category.ts (1)
getCharacterClassElementCategory(13-80)rules/sort-regexp/get-character-class-element-value.ts (1)
getCharacterClassElementValue(9-36)rules/sort-regexp/get-character-class-element-raw.ts (1)
getCharacterClassElementRaw(9-13)
rules/sort-regexp/create-sorting-node.ts (8)
types/sorting-node.ts (1)
SortingNode(18-102)rules/sort-regexp/get-alternative-alias.ts (1)
getAlternativeAlias(9-20)rules/sort-regexp/get-selector.ts (1)
getSelector(9-15)rules/sort-regexp/get-sorting-node-name.ts (1)
getSortingNodeName(17-26)utils/compute-group.ts (1)
computeGroup(75-113)utils/does-custom-group-match.ts (1)
doesCustomGroupMatch(121-141)rules/sort-regexp/create-pseudo-literal-node.ts (1)
createPseudoLiteralNode(13-39)utils/is-node-eslint-disabled.ts (1)
isNodeEslintDisabled(27-32)
rules/sort-regexp/is-lowercase-character.ts (1)
rules/sort-regexp/code-point-to-string.ts (1)
codePointToString(7-9)
rules/sort-regexp/create-character-class-sorting-node.ts (2)
types/sorting-node.ts (1)
SortingNode(18-102)rules/sort-regexp/get-character-class-element-sort-key.ts (1)
getCharacterClassElementSortKey(19-27)
rules/sort-regexp/create-flag-sorting-nodes.ts (2)
types/sorting-node.ts (1)
SortingNode(18-102)utils/is-node-eslint-disabled.ts (1)
isNodeEslintDisabled(27-32)
test/rules/sort-regexp.test.ts (2)
utils/alphabet.ts (1)
Alphabet(46-467)test/utils/validate-rule-json-schema.ts (1)
validateRuleJsonSchema(8-19)
🪛 ESLint
test/rules/sort-regexp.test.ts
[error] 33-33: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 36-36: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 45-45: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 54-54: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 69-69: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 72-72: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 87-87: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 90-90: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 111-111: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 114-114: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 128-128: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 131-131: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 146-146: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 149-149: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 164-164: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 167-167: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 176-176: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 191-191: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 194-194: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 209-209: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 212-212: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 231-231: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 234-234: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 253-253: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 256-256: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 271-271: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 274-274: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 289-289: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 292-292: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 310-310: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 313-313: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 328-328: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 331-331: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 352-352: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 355-355: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 369-369: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 372-372: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 391-391: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 394-394: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 403-403: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 412-412: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 431-431: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 434-434: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 449-449: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 452-452: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 461-461: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 476-476: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 479-479: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 494-494: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 497-497: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 518-518: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 521-521: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 552-552: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 555-555: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 563-563: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 582-582: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 585-585: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 594-594: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 609-609: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 612-612: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 621-621: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 640-640: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 643-643: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 658-658: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 661-661: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 676-676: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 679-679: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 694-694: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 697-697: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 712-712: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 715-715: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 730-730: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 733-733: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 748-748: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 751-751: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 774-774: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 777-777: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 792-792: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 795-795: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 810-810: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 813-813: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 828-828: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 831-831: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 866-866: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 869-869: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 878-878: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 887-887: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 902-902: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 905-905: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 920-920: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 923-923: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 938-938: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 941-941: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 956-956: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 959-959: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 974-974: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 977-977: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 996-996: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 999-999: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1029-1029: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1032-1032: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1047-1047: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1050-1050: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1065-1065: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1068-1068: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1083-1083: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1086-1086: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1101-1101: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1104-1104: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1113-1113: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1122-1122: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1131-1131: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1154-1154: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1157-1157: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1165-1165: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1175-1175: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1178-1178: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1200-1200: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1203-1203: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1212-1212: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1221-1221: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1236-1236: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1239-1239: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1254-1254: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1257-1257: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1278-1278: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1281-1281: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1295-1295: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1298-1298: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1313-1313: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1316-1316: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1331-1331: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1334-1334: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1343-1343: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1358-1358: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1361-1361: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1375-1375: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1378-1378: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1397-1397: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1400-1400: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1420-1420: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1423-1423: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1442-1442: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1445-1445: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1460-1460: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1463-1463: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1478-1478: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1481-1481: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1499-1499: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1502-1502: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 1517-1517: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1520-1520: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1541-1541: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1544-1544: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1558-1558: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1561-1561: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1580-1580: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1583-1583: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1592-1592: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1601-1601: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1620-1620: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1623-1623: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1638-1638: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1641-1641: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1650-1650: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1665-1665: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1668-1668: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1683-1683: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1686-1686: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1707-1707: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1710-1710: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1741-1741: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1744-1744: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1752-1752: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1767-1767: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1770-1770: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1779-1779: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1794-1794: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1797-1797: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1806-1806: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1821-1821: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1824-1824: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1839-1839: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1842-1842: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1857-1857: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1860-1860: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1875-1875: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1878-1878: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1893-1893: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1896-1896: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1911-1911: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1914-1914: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1929-1929: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1932-1932: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1951-1951: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1954-1954: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1969-1969: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1972-1972: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1987-1987: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 1990-1990: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2005-2005: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2008-2008: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2043-2043: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2046-2046: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2055-2055: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2064-2064: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2079-2079: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2082-2082: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2097-2097: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2100-2100: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2115-2115: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2118-2118: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2133-2133: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2136-2136: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2151-2151: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2154-2154: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2173-2173: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2176-2176: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2206-2206: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2209-2209: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2224-2224: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2227-2227: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2242-2242: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2245-2245: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2260-2260: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2263-2263: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2278-2278: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2281-2281: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2290-2290: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2299-2299: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2308-2308: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2331-2331: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2334-2334: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2342-2342: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2352-2352: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2355-2355: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2371-2371: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2381-2381: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2384-2384: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2399-2399: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2402-2402: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2417-2417: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2420-2420: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2429-2429: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2444-2444: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2447-2447: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2465-2465: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2468-2468: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2487-2487: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2490-2490: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2499-2499: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2514-2514: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2517-2517: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2526-2526: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2541-2541: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2544-2544: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2553-2553: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2562-2562: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2565-2565: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2581-2581: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2584-2584: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2599-2599: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2602-2602: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2617-2617: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2620-2620: Unsafe call of a(n) error type typed value.
(typescript/no-unsafe-call)
[error] 2638-2638: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2641-2641: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2656-2656: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2659-2659: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2678-2678: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2681-2681: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2689-2689: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2699-2699: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2702-2702: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2723-2723: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2739-2739: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2742-2742: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2758-2758: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2767-2767: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
[error] 2776-2776: Unsafe use of a(n) error type typed template tag.
(typescript/no-unsafe-call)
🔇 Additional comments (16)
rules/sort-regexp/is-uppercase-character.ts (1)
1-13: LGTM!The implementation correctly uses Unicode property escapes to detect uppercase letters. The pattern and logic are sound.
rules/sort-regexp/is-lowercase-character.ts (1)
1-13: LGTM!The implementation correctly mirrors the uppercase character check and uses the appropriate Unicode property for lowercase letters.
rules/sort-regexp/get-character-class-element-raw.ts (1)
9-13: LGTM!The function provides a clear, focused accessor for character class element raw text. While it's a thin wrapper, it maintains consistency with the module's design pattern of small, single-purpose utilities.
rules/sort-regexp/get-alternative-alias.ts (1)
9-20: LGTM!The function correctly extracts alias names by checking both the first element and the parent. The type guards and null-coalescing logic are appropriate.
rules/sort-regexp/get-selector.ts (1)
9-15: LGTM!The selector resolution logic is clear and correct. The truthiness check appropriately handles both null and empty string cases.
rules/sort-regexp/is-digit-character.ts (1)
1-13: LGTM!The implementation correctly uses Unicode property escapes for digit detection. The pattern and logic are consistent with the other character type checkers.
rules/sort-regexp/get-sorting-node-name.ts (1)
1-26: LGTM!The function logic is clear and correct. The conditional formatting based on
ignoreAliasandalternativeAliaspresence is appropriate for constructing display names during sorting.rules/sort-regexp/does-alternative-shadow-other.ts (1)
1-25: LGTM!The shadowing detection logic correctly handles all cases: empty alternatives, equal-length alternatives (exact duplicates), and prefix relationships for different lengths. This properly identifies when one alternative would make another unreachable in regex alternation.
rules/sort-regexp/create-character-class-sorting-node.ts (1)
14-31: LGTM!The sorting node construction is correct. The hardcoded
isEslintDisabled: falseis appropriate since character class elements are atomic parts of a larger regex literal and cannot be individually disabled. ThepartitionId: 0ensures all elements in a character class are sorted together.rules/sort-regexp/get-character-class-element-category.ts (1)
41-62: Character set categorization is functional but semantically unclear.The
'word'character set (\w) is categorized as category 2 (lowercase), which is semantically misleading since \w matches digits, uppercase, lowercase, and underscore. However, this achieves the desired sort order (\d before \w before \s), so the implementation is correct for its purpose.rules/sort-regexp/create-sorting-node.ts (1)
32-74: LGTM!The sorting node construction correctly orchestrates multiple helpers to compute group, name, and node properties. The use of
literalNodefor ESLint disable checks (line 67) is appropriate since disable directives apply to entire literals rather than individual alternatives.rules/sort-regexp/get-character-class-element-value.ts (1)
9-36: LGTM!The normalization logic correctly handles all character class element types. Converting code points to characters (line 16, 28) produces intuitive string representations for sorting comparisons.
test/rules/sort-regexp.test.ts (1)
1-2791: Comprehensive test coverage with extensive edge cases.The test suite thoroughly covers all sorting modes (alphabetical, natural, line-length, custom, unsorted) with tests for flags, character classes, alternatives, shadowing, and various regex features. The organization by mode and feature makes the tests easy to navigate.
Note: The static analysis hints showing "Unsafe use of...template tag" errors for
dedentusage are false positives due to TypeScript's incomplete inference of thededentlibrary's types and can be safely ignored.rules/sort-regexp/create-pseudo-literal-node.ts (1)
22-38: Type assertion is appropriately used—no issues found.The constructed object provides all standard
TSESTree.Literalproperties:type,value,raw,parent,range, andloc. Downstream usage only accessesrangeand the node object itself, both of which are properly defined. The type assertion correctly captures the intent of creating a pseudo-node that mimics ESLint's Literal node structure for sorting purposes.rules/sort-regexp/create-flag-sorting-nodes.ts (1)
1-32: LGTM! Clean and efficient implementation.The function correctly creates sorting nodes for regex flags. The approach of computing
isEslintDisabledonce and reusing it for all flags is efficient, and the spread operator properly handles the flag string conversion.rules/sort-regexp/get-character-class-element-sort-key.ts (1)
19-27: LGTM! Well-structured delegation pattern.The function correctly composes a sort key by delegating to specialized helper functions. The implementation is clean, type-safe, and easy to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't had time to dive deep into the rule yet, I would say that this is more complex than the other ones we have.
My main concern at this stage is safety, I've added a comment about it: I just hope that Regex sorting is not a rabbit hole with complex edge cases that would modify behaviors.
I'll take a deeper look when I find some time !
rules/sort-regexp.ts
Outdated
| } | ||
|
|
||
| if (first.length < second.length) { | ||
| return second.startsWith(first) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work for cases such as: /(a|.*ab)/.exec("ab") and /(.*ab|a)/.exec("ab").
/(a|.*ab)/.exec("ab")
(2) ['a', 'a', index: 0, input: 'ab', groups: undefined]
/(.*ab|a)/.exec("ab")
(2) ['ab', 'ab', index: 0, input: 'ab', groups: undefined]I haven't looked deep into it, but we probably need to includes for maximal safety.
In fact, it simply might never be fully safe with exec.
Maybe we would have to restrain sorting to specific usages (.test for example).
|
|
||
| Large patterns become easier to maintain when related branches are grouped and repeatedly used flags or character classes follow the same order. This rule helps you spot missing alternatives, avoid duplicated work, and keeps refactors safer by applying one sorting strategy across your codebase. | ||
|
|
||
| The rule is **safe** – it only reorders elements without changing their meaning. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I'm not confident that we can 100% ensure full safety here at this stage. I would even say that regex re-ordering probably has more weird edge cases that any other rules we have (which already have complex edge cases due to dependency ordering).
I expect bugs related to ordering that change the behavior when the rule is launched. It might be OK in 95% of the cases, but I have doubt about the remaining complex cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added additional safeguards in the latest commits:
- ES2024
/vflag guard - skips character class sorting to avoid breaking set operations (&&,--) - More backreference tests - covers named groups and complex scenarios
- Existing shadowing detection - already handles
/(ab|a)/cases
The rule is conservative - it skips sorting when it detects potentially unsafe patterns. That said, I agree 100% safety is hard to guarantee with regex.
| options, | ||
| }: SortingNodeNameParameters): string { | ||
| if (!options.ignoreAlias && alternativeAlias) { | ||
| return `${alternativeAlias}: ${alternative.raw}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove the space after :.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hugop95 Quick question about the space after : - you suggested removing it from "pet: cat" → "pet:cat".
Is this a style preference, or is there a specific reason? I kept the space for readability in error messages, but happy to change if there's a convention I'm missing.
359768d to
6534dc7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
rules/sort-regexp/get-character-class-element-raw.ts (1)
9-13: Consider inlining this trivial wrapper.This function simply returns
element.rawwithout any transformation or logic. The abstraction adds indirection without clear benefit - callers could access.rawdirectly. Consider removing this wrapper unless there's a plan to extend its logic in the future.rules/sort-regexp/code-point-to-string.ts (1)
7-9: Consider inlining this trivial wrapper.This function is a direct pass-through to
String.fromCodePoint(value). The thin wrapper adds minimal semantic value while introducing an extra function call. Consider usingString.fromCodePointdirectly at call sites unless there are plans to add normalization or validation logic here.rules/sort-regexp/get-character-class-element-sort-key.ts (1)
7-11: Align interface field order with the returned object.The interface declares fields in the order
normalized, category, raw, but the returned object (Lines 22-26) usescategory, normalized, raw. While this doesn't affect runtime behavior, aligning the declaration order with the construction order improves readability and maintainability.Apply this diff to match the field order:
export interface CharacterClassElementSortKey { - normalized: string category: number + normalized: string raw: string }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (25)
docs/content/rules/sort-regexp.mdx(1 hunks)docs/public/llms.txt(1 hunks)index.ts(3 hunks)package.json(1 hunks)readme.md(1 hunks)rules/sort-regexp.ts(1 hunks)rules/sort-regexp/code-point-to-string.ts(1 hunks)rules/sort-regexp/create-character-class-sorting-node.ts(1 hunks)rules/sort-regexp/create-flag-sorting-nodes.ts(1 hunks)rules/sort-regexp/create-pseudo-literal-node.ts(1 hunks)rules/sort-regexp/create-sorting-node.ts(1 hunks)rules/sort-regexp/does-alternative-shadow-other.ts(1 hunks)rules/sort-regexp/get-alternative-alias.ts(1 hunks)rules/sort-regexp/get-character-class-element-category.ts(1 hunks)rules/sort-regexp/get-character-class-element-raw.ts(1 hunks)rules/sort-regexp/get-character-class-element-sort-key.ts(1 hunks)rules/sort-regexp/get-character-class-element-value.ts(1 hunks)rules/sort-regexp/get-selector.ts(1 hunks)rules/sort-regexp/get-sorting-node-name.ts(1 hunks)rules/sort-regexp/has-shadowing-alternatives.ts(1 hunks)rules/sort-regexp/is-capturing-context.ts(1 hunks)rules/sort-regexp/is-digit-character.ts(1 hunks)rules/sort-regexp/is-lowercase-character.ts(1 hunks)rules/sort-regexp/is-uppercase-character.ts(1 hunks)rules/sort-regexp/types.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- readme.md
🚧 Files skipped from review as they are similar to previous changes (14)
- docs/content/rules/sort-regexp.mdx
- rules/sort-regexp/has-shadowing-alternatives.ts
- index.ts
- rules/sort-regexp/get-selector.ts
- rules/sort-regexp/is-uppercase-character.ts
- rules/sort-regexp/create-sorting-node.ts
- package.json
- rules/sort-regexp/is-capturing-context.ts
- rules/sort-regexp/get-sorting-node-name.ts
- rules/sort-regexp/types.ts
- rules/sort-regexp/create-pseudo-literal-node.ts
- rules/sort-regexp/get-character-class-element-value.ts
- rules/sort-regexp/create-character-class-sorting-node.ts
- rules/sort-regexp.ts
🧰 Additional context used
🧬 Code graph analysis (5)
rules/sort-regexp/create-flag-sorting-nodes.ts (2)
types/sorting-node.ts (1)
SortingNode(18-102)utils/is-node-eslint-disabled.ts (1)
isNodeEslintDisabled(27-32)
rules/sort-regexp/get-character-class-element-sort-key.ts (3)
rules/sort-regexp/get-character-class-element-category.ts (1)
getCharacterClassElementCategory(13-80)rules/sort-regexp/get-character-class-element-value.ts (1)
getCharacterClassElementValue(9-36)rules/sort-regexp/get-character-class-element-raw.ts (1)
getCharacterClassElementRaw(9-13)
rules/sort-regexp/is-lowercase-character.ts (1)
rules/sort-regexp/code-point-to-string.ts (1)
codePointToString(7-9)
rules/sort-regexp/get-character-class-element-category.ts (3)
rules/sort-regexp/is-digit-character.ts (1)
isDigitCharacter(11-13)rules/sort-regexp/is-uppercase-character.ts (1)
isUppercaseCharacter(11-13)rules/sort-regexp/is-lowercase-character.ts (1)
isLowercaseCharacter(11-13)
rules/sort-regexp/is-digit-character.ts (1)
rules/sort-regexp/code-point-to-string.ts (1)
codePointToString(7-9)
🔇 Additional comments (8)
docs/public/llms.txt (1)
47-47: Excellent! Documentation properly updated for the new rule.The new
sort-regexprule is correctly added to the rules list in proper alphabetical order and follows the established documentation format and URL convention used for other rules.rules/sort-regexp/create-flag-sorting-nodes.ts (1)
1-32: LGTM! Clean and efficient implementation.The function correctly creates individual sorting nodes for each regex flag character. Key strengths:
- Efficient: Computes
isDisabledonce and reuses it for all flag nodes- Type-safe: Properly typed parameters and return value matching the
SortingNodeinterface- Clear: Well-documented with concise JSDoc and straightforward logic
- Appropriate defaults: Hard-coded values (
partitionId: 0,group: 'flags',size: 1) are correct for flag sorting contextrules/sort-regexp/get-alternative-alias.ts (1)
9-20: LGTM! Clean utility function with appropriate defensive coding.The two-step alias lookup (first element → parent) is well-implemented. The use of optional chaining on line 10-11 correctly handles empty element arrays, while line 15 appropriately omits it since
parentis guaranteed by the regexpp type system. The truthy checks on bothnameproperties ensure empty strings are treated as absent aliases, which aligns with the documented behavior.rules/sort-regexp/is-digit-character.ts (1)
1-13: LGTM!The implementation correctly uses the Unicode decimal digit property
\p{Nd}with proper anchoring. The precompiled pattern constant is an appropriate optimization.rules/sort-regexp/is-lowercase-character.ts (1)
1-13: LGTM!The implementation correctly uses the Unicode lowercase letter property
\p{Ll}with proper anchoring. The precompiled pattern constant is an appropriate optimization.rules/sort-regexp/get-character-class-element-sort-key.ts (1)
19-27: LGTM!The function correctly composes a sort key by aggregating category, normalized value, and raw text from the helper functions. The logic is clear and straightforward.
rules/sort-regexp/does-alternative-shadow-other.ts (2)
12-14: Verify the empty string shadowing behavior.Returning
truewhen either alternative is empty treats empty strings as always causing shadowing. While this may be correct for the rule's purposes (an empty alternative would match immediately), please confirm this is the intended behavior, especially since empty alternatives are an edge case in regex patterns.Consider adding an inline comment explaining why empty alternatives are treated as always shadowing:
+ // Empty alternatives always match immediately, making other alternatives unreachable if (first.length === 0 || second.length === 0) { return true }
16-24: Core prefix logic looks correct, but verify handling of regex special characters.The prefix-based shadowing detection logic is sound for literal string alternatives (e.g., "a" shadows "ab"). Please verify that this function receives alternatives as literal text, or confirm whether regex special characters (e.g.,
.,*,+,\d) should be handled differently. For instance, the patterna.(matching "a" followed by any character) would shadowabin actual regex matching, but this function would not detect that since it performs string comparison rather than pattern analysis.Run the following script to examine how this function is called and whether alternatives are pre-processed:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
rules/sort-regexp.ts (2)
237-289: Prevent bare hyphen from creating unintended ranges.When an unescaped
-appears at the edge of a character class (e.g.,/[-az]/), it's a literal hyphen. Sorting can move it to the middle, creating an unintended range:/[a-z]/matchesathroughz, not justa,-, andz.Apply this diff to keep bare hyphens at the end after sorting:
let sortedElements = [...elements].toSorted((a, b) => { let aKey = getCharacterClassElementSortKey(a) let bKey = getCharacterClassElementSortKey(b) if ( options.type !== 'line-length' && aKey.category !== bKey.category ) { let categoryDiff = aKey.category - bKey.category return options.order === 'asc' ? categoryDiff : -categoryDiff } let aNode = createCharacterClassSortingNode({ literalNode, element: a, }) let bNode = createCharacterClassSortingNode({ literalNode, element: b, }) let comparison = compare({ fallbackSortNodeValueGetter: null, nodeValueGetter: null, a: aNode, b: bNode, options, }) if (comparison !== 0) { return comparison } let rawComparison = aKey.raw.localeCompare( bKey.raw, options.locales, { sensitivity: options.ignoreCase ? 'base' : 'variant', numeric: options.type === 'natural', }, ) let rawOrderMultiplier = 1 if (options.type !== 'line-length' && options.order !== 'asc') { rawOrderMultiplier = -1 } return rawOrderMultiplier * rawComparison }) + + // Safety: keep bare '-' at the end to prevent accidental ranges + let bareHyphenIndices: number[] = [] + for (let [index, el] of sortedElements.entries()) { + if (el.type === 'Character' && el.raw === '-') { + bareHyphenIndices.push(index) + } + } + if (bareHyphenIndices.length > 0) { + let hyphens = bareHyphenIndices.map(i => sortedElements[i]!) + sortedElements = sortedElements.filter((_, i) => !bareHyphenIndices.includes(i)) + sortedElements.push(...hyphens) + }Based on past review comments
82-87: Add early return when the regex literal is disabled.After computing
eslintDisabledLines, the code should check whether the literal node itself is on a disabled line and return early if so. Currently, flag and character-class processing continues even wheneslint-disablecovers the literal.Apply this diff to add the early return:
let eslintDisabledLines = getEslintDisabledLines({ ruleName: id, sourceCode, }) + // Skip if the rule is disabled for this literal's line + if (eslintDisabledLines.includes(literalNode.loc.start.line)) { + return + } + let ast = parseRegExpLiteral(literalNode.raw)Based on past review comments
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
rules/sort-regexp.ts(1 hunks)rules/sort-regexp/create-sorting-node.ts(1 hunks)rules/sort-regexp/get-character-class-element-category.ts(1 hunks)rules/sort-regexp/get-character-class-element-sort-key.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- rules/sort-regexp/get-character-class-element-sort-key.ts
- rules/sort-regexp/get-character-class-element-category.ts
- rules/sort-regexp/create-sorting-node.ts
🧰 Additional context used
🧬 Code graph analysis (1)
rules/sort-regexp.ts (16)
types/sorting-node.ts (1)
SortingNode(18-102)rules/sort-regexp/types.ts (3)
Options(22-41)allSelectors(65-65)allModifiers(68-68)utils/create-eslint-rule.ts (1)
createEslintRule(32-35)utils/get-settings.ts (1)
getSettings(91-136)utils/complete.ts (1)
complete(30-45)utils/get-eslint-disabled-lines.ts (1)
getEslintDisabledLines(45-97)rules/sort-regexp/create-flag-sorting-nodes.ts (1)
createFlagSortingNodes(13-32)utils/sort-nodes.ts (1)
sortNodes(39-76)utils/create-node-index-map.ts (1)
createNodeIndexMap(27-35)utils/pairwise.ts (1)
pairwise(29-44)rules/sort-regexp/create-sorting-node.ts (1)
createSortingNode(31-72)utils/sort-nodes-by-groups.ts (1)
sortNodesByGroups(106-162)utils/report-all-errors.ts (1)
reportAllErrors(203-325)utils/compare.ts (1)
compare(108-142)utils/common-json-schemas.ts (3)
commonJsonSchemas(139-140)buildCustomGroupsArrayJsonSchema(302-351)groupsJsonSchema(165-190)utils/report-errors.ts (1)
ORDER_ERROR(21-22)
🔇 Additional comments (2)
rules/sort-regexp.ts (2)
89-151: Flag sorting implementation looks correct.The logic properly creates sorting nodes for each flag, sorts them, identifies misplaced pairs, and generates fixes. Reordering flags doesn't change regex semantics, making this transformation safe.
330-368: Rule metadata and registration look correct.The visitor registration, schema definition, documentation URL, and message configuration all follow ESLint best practices and align with the plugin's patterns.
0146d42 to
6950b0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
rules/sort-regexp.ts (2)
241-289: Prevent accidental range operators when sorting character classes.Sorting can move an unescaped
-from an edge position (where it's a literal) into the middle (where it becomes a range operator), changing regex behavior. For example,[za-]sorted to[a-z]changes from "z, a, or hyphen" to "range from a to z".Add a guard to detect bare
-and either skip sorting for that character class or ensure-remains at an edge:let sortedElements = [...elements].toSorted((a, b) => { let aKey = getCharacterClassElementSortKey(a) let bKey = getCharacterClassElementSortKey(b) if ( options.type !== 'line-length' && aKey.category !== bKey.category ) { let categoryDiff = aKey.category - bKey.category return options.order === 'asc' ? categoryDiff : -categoryDiff } let aNode = createCharacterClassSortingNode({ literalNode, element: a, }) let bNode = createCharacterClassSortingNode({ literalNode, element: b, }) let comparison = compare({ fallbackSortNodeValueGetter: null, nodeValueGetter: null, a: aNode, b: bNode, options, }) if (comparison !== 0) { return comparison } let rawComparison = aKey.raw.localeCompare( bKey.raw, options.locales, { sensitivity: options.ignoreCase ? 'base' : 'variant', numeric: options.type === 'natural', }, ) let rawOrderMultiplier = 1 if (options.type !== 'line-length' && options.order !== 'asc') { rawOrderMultiplier = -1 } return rawOrderMultiplier * rawComparison }) + + // Safety: keep unescaped '-' at edges to avoid creating unintended ranges + let bareHyphens = sortedElements.filter( + el => el.type === 'Character' && el.raw === '-' + ) + if (bareHyphens.length > 0) { + let others = sortedElements.filter( + el => !(el.type === 'Character' && el.raw === '-') + ) + sortedElements = [...others, ...bareHyphens] + }Based on past review comments.
82-87: Add early return check for eslint-disabled literals.The rule computes
eslintDisabledLinesbut never checks whether the regex literal itself is disabled before proceeding to parse and report issues. This means flags, alternatives, and character classes will still be reported/fixed even wheneslint-disablecovers the literal's line.Apply this diff to add the missing check:
let eslintDisabledLines = getEslintDisabledLines({ ruleName: id, sourceCode, }) + // Skip this literal if the rule is disabled for its line + if (eslintDisabledLines.includes(literalNode.loc.start.line)) { + return + } + let ast = parseRegExpLiteral(literalNode.raw)Based on past review comments.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rules/sort-regexp.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
rules/sort-regexp.ts (20)
types/sorting-node.ts (1)
SortingNode(18-102)rules/sort-regexp/types.ts (3)
Options(22-41)allSelectors(65-65)allModifiers(68-68)utils/create-eslint-rule.ts (1)
createEslintRule(32-35)utils/get-settings.ts (1)
getSettings(91-136)utils/complete.ts (1)
complete(30-45)utils/validate-custom-sort-configuration.ts (1)
validateCustomSortConfiguration(25-35)utils/validate-generated-groups-configuration.ts (1)
validateGeneratedGroupsConfiguration(74-95)utils/get-eslint-disabled-lines.ts (1)
getEslintDisabledLines(45-97)rules/sort-regexp/create-flag-sorting-nodes.ts (1)
createFlagSortingNodes(13-32)utils/sort-nodes.ts (1)
sortNodes(39-76)utils/create-node-index-map.ts (1)
createNodeIndexMap(27-35)utils/pairwise.ts (1)
pairwise(29-44)rules/sort-regexp/create-sorting-node.ts (1)
createSortingNode(31-72)utils/sort-nodes-by-groups.ts (1)
sortNodesByGroups(106-162)utils/report-all-errors.ts (1)
reportAllErrors(203-325)rules/sort-regexp/get-character-class-element-sort-key.ts (1)
getCharacterClassElementSortKey(18-26)rules/sort-regexp/create-character-class-sorting-node.ts (1)
createCharacterClassSortingNode(14-31)utils/compare.ts (1)
compare(108-142)utils/common-json-schemas.ts (3)
commonJsonSchemas(139-140)buildCustomGroupsArrayJsonSchema(302-351)groupsJsonSchema(165-190)utils/report-errors.ts (1)
ORDER_ERROR(21-22)
🔇 Additional comments (6)
rules/sort-regexp.ts (6)
1-38: LGTM!Import organization is clean and all dependencies appear necessary for the rule implementation.
40-57: LGTM!Type definitions and default options are well-structured and follow the plugin's conventions.
59-75: LGTM!Rule setup, options resolution, and validation follow the established patterns in the codebase.
89-151: LGTM!Flag sorting logic correctly identifies misplaced flags, finds the first violation, and generates appropriate fixes. The approach of replacing just the flag portion is sound.
295-329: LGTM!The fix generation correctly rebuilds the entire character class, properly handles negated classes, and uses appropriate range calculations. The structural logic is sound.
334-372: LGTM!The rule visitor and meta configuration are well-structured. The schema properly defines all options, and the documentation metadata follows ESLint conventions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rules/sort-regexp/has-shadowing-alternatives.ts (1)
28-38: Consider removing non-null assertions for cleaner code.The non-null assertion operators (
!) at lines 29 and 32 are technically safe because the loop conditions guarantee the indices are within bounds. However, they're unnecessary—TypeScript's control flow analysis should infer the values are defined. Removing them would improve code clarity.Apply this diff to remove the non-null assertions:
for (let index = 0; index < rawAlternatives.length; index++) { - let current = rawAlternatives[index]! + let current = rawAlternatives[index] for (let offset = index + 1; offset < rawAlternatives.length; offset++) { - let other = rawAlternatives[offset]! + let other = rawAlternatives[offset] if (doesAlternativeShadowOther(current, other)) { return true
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rules/sort-regexp/has-shadowing-alternatives.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
rules/sort-regexp/has-shadowing-alternatives.ts (1)
rules/sort-regexp/does-alternative-shadow-other.ts (1)
doesAlternativeShadowOther(8-25)
🔇 Additional comments (2)
rules/sort-regexp/has-shadowing-alternatives.ts (2)
16-24: Quick-path optimization is conservative but justified.The function immediately returns
truewhenever any alternative starts with a negated character class. This is a conservative approach—it may flag shadowing even when sorting would be safe (e.g.,[^b]cwith multiple elements has different matching behavior than just[^b]).However, this trade-off is appropriate for a sorting rule where missing actual shadowing would be worse than being overly cautious.
11-41: LGTM! Sound implementation of shadowing detection.The function correctly implements shadowing detection for regex alternatives with appropriate trade-offs:
- Conservative quick-path for negated character classes prevents false negatives
- Efficient O(n²) pairwise comparison for typical regex sizes
- Proper use of the symmetric
doesAlternativeShadowOtherhelper- Clear logic flow with good documentation
The implementation aligns well with the safety requirements of a sorting rule where preventing behavioral changes is paramount.
|
I’m currently testing the rule on large codebases. So far I haven't managed to fix the issue where line breaks are split or checked.
In JavaScript, alternatives are tried from left to right. When \r\n is moved behind the character class, the engine will match the single This matters in real code: const LINEBREAK_MATCHER_OLD = /\r\n|[\r\n\u2028\u2029]/;
const LINEBREAK_MATCHER_NEW = /[\n\r\u2028\u2029]|\r\n/;
let text = 'a\r\nb';
console.log(text.split(LINEBREAK_MATCHER_OLD)); // ['a', 'b']
console.log(text.split(LINEBREAK_MATCHER_NEW)); // ['a', '', 'b'] |
e5da2d5 to
3084606
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
rules/sort-regexp.ts (1)
75-88: Still not respectingeslint-disablefor the entire literal in this rule
handleLiteralcomputeseslintDisabledLinesbut never bails out when the rule is disabled for the literal’s line. As a result:
- Flag sorting (lines 90–151) will still report and fix even if
// eslint-disable-next-line perfectionist/sort-regexp(or similar) applies to that literal.- Character-class sorting (lines 229–336) likewise runs unconditionally with respect to disable comments.
This mirrors the earlier review concern about “Respect eslint-disable for the literal; bail out early” and remains unresolved here.
I’d recommend importing
isNodeEslintDisabledand short‑circuiting right after computingeslintDisabledLines, e.g.:@@ let { sourceCode, id } = context let literalsWithShadowing = new WeakSet<TSESTree.Literal>() function handleLiteral(literalNode: TSESTree.Literal): void { if (!('regex' in literalNode)) { return } let eslintDisabledLines = getEslintDisabledLines({ ruleName: id, sourceCode, }) + + // Respect eslint-disable directives for this literal. + if (isNodeEslintDisabled(literalNode, eslintDisabledLines)) { + return + } @@ - let ast = parseRegExpLiteral(literalNode.raw) + let ast = parseRegExpLiteral(literalNode.raw)(and add the corresponding
import { isNodeEslintDisabled } from '../utils/is-node-eslint-disabled'at the top).This keeps the behavior consistent with other rules in the plugin and ensures users can fully opt out per literal.
🧹 Nitpick comments (3)
rules/sort-regexp/has-shadowing-alternatives.ts (1)
35-91: Shadowing detection is conservative and appears to cover the tricky newline caseThe combination of:
- early negated-class detection,
- raw prefix checks via
doesAlternativeShadowOther, and- first-character overlap via
getFirstCharacterPaths+doMatchersOverlap(including dotAll and unicode property handling)gives a conservative “bail out if in doubt” behavior. That ensures patterns like
/\r\n|[\n\r\u{2028}\u{2029}]/uare correctly flagged as shadowing and thus not reordered, addressing the semantic regression highlighted in the PR description. The unicode property cache andnullreturns for unsupported properties also keep behavior robust without throwing.If you haven’t already, consider adding a short note in the rule docs mentioning that the rule intentionally skips sorting when
hasShadowingAlternativesdetects possible overlap, to set expectations for users encountering skipped fixers.Also applies to: 140-225, 227-253, 317-365, 399-418
rules/sort-regexp.ts (1)
88-151: Overall rule wiring, flag sorting, alternatives sorting, and character-class sorting look coherent
- Flag sorting:
- Uses
createFlagSortingNodes+sortNodesand compares original vs sorted flags to decide whether to report.- Fixer correctly rewrites just the trailing flag slice
[flagsStart, literalNode.range[1]].- Alternatives sorting:
- Processes each disjunction only once (on the last alternative) and only when in a capturing context with a sortable alternatives array.
- Calls
hasShadowingAlternativesbefore building sorting nodes; literals with shadowing are tracked inliteralsWithShadowingand skipped for later character-class sorting, which is the right safety boundary.- Reuses the existing
sortNodesByGroups/reportAllErrorspipeline, so behavior is consistent with other sorting rules.- Character-class sorting:
- Avoids
v-flag and non-sortable or “unsorted” configurations.- Comparator respects category, user options (
type,order,locales,ignoreCase), and falls back to raw comparison in a deterministic way.- Fixer reconstructs the class from
sortedRawElementsusing regexpp’sstart/endoffsets, so only the inner ordering changes.With the shadowing guard and extensive tests in
sort-regexp-shadowing.test.ts, the core algorithm is in good shape; beyond theeslint-disablehandling noted separately, there’s no other correctness blocker visible here.Also applies to: 154-226, 229-337
rules/sort-regexp/get-first-character-paths.ts (1)
225-275: ReordermultiplyLengthconditions to correctly handle{0}quantifiers on unknown-length expressionsWhen a quantifier
{0}is applied to an element with unknown length (e.g., a backreference), the current implementation returnsnullinstead of0. Per the ECMAScript spec,{0}always matches the empty string regardless of the quantified sub-pattern.Reorder the conditions in
multiplyLength(lines 414–432) to checkcount === 0beforelength === null:function multiplyLength(length: LengthResult, count: number): LengthResult { + if (count === 0) { + // Zero repetitions are always empty, regardless of inner length. + return 0 + } + if (length === null) { return null } - if (length === 0 || count === 0) { + if (length === 0) { return 0 }This allows constructs like
\1{0}(backreference with zero repetitions) to correctly propagate as empty, improving first-character path analysis instead of prematurely bailing out as unknown.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (20)
docs/content/rules/sort-regexp.mdx(1 hunks)docs/public/llms.txt(1 hunks)index.ts(3 hunks)package.json(1 hunks)readme.md(1 hunks)rules/sort-regexp.ts(1 hunks)rules/sort-regexp/create-character-class-sorting-node.ts(1 hunks)rules/sort-regexp/create-flag-sorting-nodes.ts(1 hunks)rules/sort-regexp/create-pseudo-literal-node.ts(1 hunks)rules/sort-regexp/create-sorting-node.ts(1 hunks)rules/sort-regexp/does-alternative-shadow-other.ts(1 hunks)rules/sort-regexp/get-alternative-alias.ts(1 hunks)rules/sort-regexp/get-character-class-element-category.ts(1 hunks)rules/sort-regexp/get-character-class-element-sort-key.ts(1 hunks)rules/sort-regexp/get-character-class-element-value.ts(1 hunks)rules/sort-regexp/get-first-character-paths.ts(1 hunks)rules/sort-regexp/has-shadowing-alternatives.ts(1 hunks)rules/sort-regexp/is-capturing-context.ts(1 hunks)rules/sort-regexp/types.ts(1 hunks)test/rules/sort-regexp-shadowing.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- readme.md
🚧 Files skipped from review as they are similar to previous changes (13)
- docs/content/rules/sort-regexp.mdx
- rules/sort-regexp/get-alternative-alias.ts
- rules/sort-regexp/types.ts
- rules/sort-regexp/get-character-class-element-sort-key.ts
- package.json
- rules/sort-regexp/create-sorting-node.ts
- rules/sort-regexp/create-flag-sorting-nodes.ts
- rules/sort-regexp/get-character-class-element-value.ts
- rules/sort-regexp/get-character-class-element-category.ts
- rules/sort-regexp/does-alternative-shadow-other.ts
- rules/sort-regexp/create-character-class-sorting-node.ts
- rules/sort-regexp/is-capturing-context.ts
- docs/public/llms.txt
🧰 Additional context used
🧬 Code graph analysis (3)
rules/sort-regexp/has-shadowing-alternatives.ts (3)
rules/sort-regexp/does-alternative-shadow-other.ts (1)
doesAlternativeShadowOther(8-25)rules/sort-regexp/get-character-class-element-category.ts (3)
isDigitCharacter(115-117)isLowercaseCharacter(95-97)isUppercaseCharacter(105-107)rules/sort-regexp/get-first-character-paths.ts (5)
getFirstCharacterPaths(90-107)FirstCharacterPath(14-20)CharacterMatcherCharacterClass(23-29)CharacterMatcherCharacterSet(32-38)CharacterMatcher(53-56)
rules/sort-regexp.ts (24)
types/sorting-node.ts (1)
SortingNode(18-102)rules/sort-regexp/types.ts (3)
Options(22-41)allSelectors(65-65)allModifiers(68-68)utils/create-eslint-rule.ts (1)
createEslintRule(32-35)utils/get-settings.ts (1)
getSettings(62-107)utils/complete.ts (1)
complete(30-45)utils/validate-custom-sort-configuration.ts (1)
validateCustomSortConfiguration(29-37)utils/validate-generated-groups-configuration.ts (1)
validateGeneratedGroupsConfiguration(75-93)utils/get-eslint-disabled-lines.ts (1)
getEslintDisabledLines(45-97)rules/sort-regexp/create-flag-sorting-nodes.ts (1)
createFlagSortingNodes(13-32)utils/sort-nodes.ts (1)
sortNodes(39-76)utils/create-node-index-map.ts (1)
createNodeIndexMap(27-35)utils/pairwise.ts (1)
pairwise(29-44)rules/sort-regexp/is-capturing-context.ts (1)
isCapturingContext(14-22)utils/is-sortable.ts (1)
isSortable(7-9)rules/sort-regexp/has-shadowing-alternatives.ts (1)
hasShadowingAlternatives(35-91)rules/sort-regexp/create-sorting-node.ts (1)
createSortingNode(31-72)utils/get-custom-groups-compare-options.ts (1)
buildGetCustomGroupOverriddenOptionsFunction(131-142)utils/sort-nodes-by-groups.ts (1)
sortNodesByGroups(106-162)utils/report-all-errors.ts (1)
reportAllErrors(203-325)rules/sort-regexp/get-character-class-element-sort-key.ts (1)
getCharacterClassElementSortKey(18-26)rules/sort-regexp/create-character-class-sorting-node.ts (1)
createCharacterClassSortingNode(14-31)utils/compare.ts (1)
compare(108-142)utils/common-json-schemas.ts (3)
commonJsonSchemas(139-140)buildCustomGroupsArrayJsonSchema(325-374)groupsJsonSchema(160-213)utils/report-errors.ts (1)
ORDER_ERROR(21-22)
test/rules/sort-regexp-shadowing.test.ts (2)
rules/sort-regexp/get-first-character-paths.ts (2)
FirstCharacterPath(14-20)getFirstCharacterPaths(90-107)rules/sort-regexp/has-shadowing-alternatives.ts (1)
hasShadowingAlternatives(35-91)
🪛 ast-grep (0.40.0)
test/rules/sort-regexp-shadowing.test.ts
[warning] 238-238: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp((?:${alternatives.join('|')}), 'u')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (5)
index.ts (1)
23-25: Wiring ofsort-regexpinto the plugin looks correctImport, type surface in
PluginConfig, and publicrulesexport are all consistent and aligned with the new rule file.Also applies to: 49-52, 77-101
rules/sort-regexp/create-pseudo-literal-node.ts (1)
13-39: Pseudo-literal construction is consistent and safe for internal sortingRange/loc computation from
alternative.start/endplusliteralNode.range[0]is coherent, and using a syntheticLiteralwithalternative.rawforvalue/rawis appropriate forSortingNode-only usage.test/rules/sort-regexp-shadowing.test.ts (1)
22-243: Shadowing test suite gives strong confidence in newline and overlap handlingThe tests here exercise:
- Detailed
getFirstCharacterPathsbehavior (quantifiers, backreferences, recursion, unicode sets, safety limits), andhasShadowingAlternativesacross newline overlaps, ranges, property escapes, digit/space/word escapes, unicode sets, and invalid/unknown constructs.In particular,
detects overlapping newline classesandrespects the dotAll flag when wildcard escapes live inside classesdirectly cover the kind of/\r\n|[\r\n\u2028\u2029]/-style regression mentioned in the PR description. The dynamicRegExpconstruction is tightly bounded and test-only, so the static ReDoS warning isn’t a practical concern here.Also applies to: 405-758
rules/sort-regexp/get-first-character-paths.ts (2)
13-82: Type and context modeling for first-character analysis looks solidThe
FirstCharacterPath/CharacterMatcher*union plusLengthResult = 0 | 1 | 2 | nullis a clean way to encode “0, 1, ≥2, unknown”, and theAnalysisContextwith a sharedWeakMapand recursion guard is well thought out for reuse across elements and alternatives. No changes needed here.
176-215: Verification confirms the concern: empty array handling in call sites is incorrect.The implementation in
collectFirstCharacterPathsFromAlternativeis semantically sound (as stated in the review), but the call site inhasFirstCharacterShadowing(lines 154–156 and 165–167 inhas-shadowing-alternatives.ts) treats empty arrays as "safe to skip" by continuing past comparison. This means alternatives returning[]are never checked for shadowing relationships, implicitly treating unknown patterns as non-shadowing—the exact risk the review flagged.The alternatives with empty paths should be marked as unsortable rather than skipped entirely to avoid silent reordering of patterns we don't fully understand.
|
I fixed the issue mentioned above and added the necessary tests. I also tested the new ESLint rule on several large projects: All changes and auto-fixes are cosmetic in nature and do not create functional changes. @hugop95 This PR is now ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
rules/sort-regexp.ts (2)
76-90: Respecteslint-disablefor the entire literal before doing any work
handleLiteralalways parses and processes flags/alternatives/character classes even when the regex literal’s line is disabled for this rule. That means flags and character classes can still be reported/fixed despite a// eslint-disable-line perfectionist/sort-regexp(or similar) directive.Add an early return after computing
eslintDisabledLinesto bail out when the literal itself is disabled.import { createFlagSortingNodes } from './sort-regexp/create-flag-sorting-nodes' import { getEslintDisabledLines } from '../utils/get-eslint-disabled-lines' +import { isNodeEslintDisabled } from '../utils/is-node-eslint-disabled' @@ let eslintDisabledLines = getEslintDisabledLines({ ruleName: id, sourceCode, }) + + // If the rule is disabled for this literal's line, skip all processing. + if (isNodeEslintDisabled(literalNode, eslintDisabledLines)) { + return + } @@ - let ast = parseRegExpLiteral(literalNode.raw) + let ast = parseRegExpLiteral(literalNode.raw)
244-345: Character class sorting can create unintended ranges with bare-The current
toSortedcomparator ignores the special role of-inside character classes. Sorting can move an unescaped-from an edge position into the middle, turning it into a range and changing what the regex matches.To keep semantics safe, ensure any bare
-(Characterwithraw === '-') stays at the end of the class after sorting (or bail out entirely when such a move would put it in the middle). A simple conservative fix is to post-processsortedElementsand move hyphens to the end:let sortedElements = [...elements].toSorted((a, b) => { @@ if (options.type !== 'line-length' && options.order !== 'asc') { rawOrderMultiplier = -1 } return rawOrderMultiplier * rawComparison }) + + // Safety: keep any bare '-' at the end to avoid unintentionally + // turning it into a range when moved between other elements. + if ( + sortedElements.some( + el => el.type === 'Character' && el.raw === '-', + ) + ) { + let hyphens: typeof sortedElements = [] + let others: typeof sortedElements = [] + for (let el of sortedElements) { + if (el.type === 'Character' && el.raw === '-') { + hyphens.push(el) + } else { + others.push(el) + } + } + sortedElements = [...others, ...hyphens] + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rules/sort-regexp.ts(1 hunks)rules/sort-regexp/alternatives-contain-unnamed-capturing-groups.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
rules/sort-regexp.ts (17)
types/sorting-node.ts (1)
SortingNode(18-102)rules/sort-regexp/types.ts (3)
Options(22-41)allSelectors(65-65)allModifiers(68-68)utils/create-eslint-rule.ts (1)
createEslintRule(32-35)utils/get-settings.ts (1)
getSettings(62-107)utils/complete.ts (1)
complete(30-45)utils/validate-custom-sort-configuration.ts (1)
validateCustomSortConfiguration(29-37)utils/validate-generated-groups-configuration.ts (1)
validateGeneratedGroupsConfiguration(75-93)utils/get-eslint-disabled-lines.ts (1)
getEslintDisabledLines(45-97)rules/sort-regexp/create-flag-sorting-nodes.ts (1)
createFlagSortingNodes(13-32)utils/sort-nodes.ts (1)
sortNodes(39-76)utils/create-node-index-map.ts (1)
createNodeIndexMap(27-35)utils/pairwise.ts (1)
pairwise(29-44)rules/sort-regexp/create-sorting-node.ts (1)
createSortingNode(31-72)utils/sort-nodes-by-groups.ts (1)
sortNodesByGroups(106-162)utils/report-all-errors.ts (1)
reportAllErrors(203-325)utils/compare.ts (1)
compare(108-142)utils/report-errors.ts (1)
ORDER_ERROR(21-22)
🔇 Additional comments (1)
rules/sort-regexp/alternatives-contain-unnamed-capturing-groups.ts (1)
9-71: Unnamed capturing group detection looks correct and completeThe traversal correctly finds unnamed capturing groups through groups, quantifiers, and lookarounds while ignoring non-capturing constructs. No issues spotted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very ambitious initiative !
The PR is very large, so I haven't reviewed everything yet, but I have some comments already on a couple of things.
One problem that I think is blocking right now is that the groups feature doesn't work (or at the very least, tests do not test it correctly, I've left a comment for that).
My other feedback is regarding the current approach: the rule sorts 3 things as you mention in the description:
- capture groups
(/(error|warning|info|debug)/) - character classes
(/[zxa-f0-9A-Z]/) - flags
(/pattern/igmus)
I wonder if it wouldn't have been better to go with 3 separate rules:
- They are all linked to RegExp, but represent very different things.
- We already have one rule that sorts multiple things at the same time:
sort-switch-case(it can sort "groups" ofcases, and what is inside a consecutive list ofcases).- It's a bit of our ugly duckling rule: it's the only one that doesn't supports
groups, because this notion mean different things depending on what we want to sort. - We have a bit of the same case here:
- What do
groups/customGroupsapply to ?flags,capture groups, orcharacter classes? - Can users
groupflags only, but not other things ? - I think that in all cases, this should be documented, because it's impossible to know otherwise.
- What do
- It's a bit of our ugly duckling rule: it's the only one that doesn't supports
- Code wise, it's harder to understand the sort of all 3 things in the same rule.
- Maybe we could have common functions use by the 3 rules, but each rule is kept small and only focuses on sorting one thing.
ignoreAliasis only useful forcapture groupssorting.- This way, it allows users to enable
flagssorting, but notcapture groupssorting.- I think that it's important: flag sorting is fully safe, but capture groups isn't in theory.
Let me know what you think !
|
|
||
| This is useful when you only care about the pattern and not how it is aliased. | ||
|
|
||
| ### customGroups |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For other rules, this section is after Groups, so I would do the same here for consistency.
|
|
||
| Custom groups are evaluated in order; the first match wins and overrides predefined groups. | ||
|
|
||
| ### groups |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would take the same sub sections as for other rules for consistency as well, even if this feature probably won't likely be used for this rule in particular:
eslint-plugin-perfectionist/docs/content/rules/sort-objects.mdx
Lines 460 to 477 in f9919cb
| ### groups | |
| <sub> | |
| type: | |
| ```ts | |
| Array< | |
| | string | |
| | string[] | |
| | { newlinesBetween: number } | |
| | { | |
| group: string | string[]; | |
| type?: 'alphabetical' | 'natural' | 'line-length' | 'custom' | 'unsorted'; | |
| order?: 'asc' | 'desc'; | |
| } | |
| > | |
| ``` | |
| </sub> | |
| <sub>default: `[]`</sub> |
| "test:unit": "vitest --run --coverage" | ||
| }, | ||
| "dependencies": { | ||
| "@eslint-community/regexpp": "^4.12.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For information:
@eslint-community/regexpp's unpacked size is 474kB according to NPM:- Perfectionist V4 is at 327kb
@typescript-eslint/utilsis at 195kB.natural-orderbyis at 72kb.
Just something to keep in mind based on the adoption of the rule.
| import { complete } from '../utils/complete' | ||
| import { compare } from '../utils/compare' | ||
|
|
||
| type SortRegExpSortingNode = SortingNode<TSESTree.Literal> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could put and export this type in rules/sort-regexp/types.ts, and use it in places doing SortingNode<TSESTree.Literal> again.
- In
createCharacterClassSortingNodein this file. rules/sort-regexp/create-flag-sorting-nodes.ts.rules/sort-regexp/create-character-class-sorting-node.ts.
| * @param parameters - Character class element metadata. | ||
| * @returns Slice of the literal raw string covering the element. | ||
| */ | ||
| function getCharacterClassElementRawText({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] It's a bit strange IMO to have this local function at the top. What about putting it at the very bottom ?
|
|
||
| reportAllErrors<MessageId, SortRegExpSortingNode>({ | ||
| availableMessageIds: { | ||
| unexpectedGroupOrder: 'unexpectedRegExpOrder', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we create a dedicated unexpectedRegExpGroupOrder messageID to stay consistent with other rules ?
| }) | ||
| }) | ||
|
|
||
| it('respects custom groups by alias name', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the groups feature doesn't work: if you change groups to ["unknown", "digits"], the result is still valid.
The only case that uses sortNodesByGroups is for sortAlternatives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
♻️ Duplicate comments (1)
rules/sort-regexp/get-first-character-paths.ts (1)
303-333: Address the outstanding review comment about{0}quantifiers.A previous review identified that quantifiers with
max === 0(e.g.,{0}or{0,0}) are handled incorrectly. The function currently returns first-character paths for patterns like/a{0}b/, buta{0}never consumesaat runtime—onlybshould be a possible first character. Add an early return whenquantifier.max === 0as suggested in the prior review.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
rules/sort-regexp/get-first-character-paths.ts(1 hunks)rules/sort-regexp/has-shadowing-alternatives.ts(1 hunks)test/rules/sort-regexp-shadowing.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
rules/sort-regexp/has-shadowing-alternatives.ts (3)
rules/sort-regexp/does-alternative-shadow-other.ts (1)
doesAlternativeShadowOther(8-25)rules/sort-regexp/get-character-class-element-category.ts (3)
isDigitCharacter(115-117)isLowercaseCharacter(95-97)isUppercaseCharacter(105-107)rules/sort-regexp/get-first-character-paths.ts (5)
getFirstCharacterPaths(100-119)FirstCharacterPath(14-23)CharacterMatcherCharacterClass(26-32)CharacterMatcherCharacterSet(35-41)CharacterMatcher(56-59)
test/rules/sort-regexp-shadowing.test.ts (2)
rules/sort-regexp/get-first-character-paths.ts (2)
FirstCharacterPath(14-23)getFirstCharacterPaths(100-119)rules/sort-regexp/has-shadowing-alternatives.ts (1)
hasShadowingAlternatives(35-91)
🪛 ast-grep (0.40.0)
test/rules/sort-regexp-shadowing.test.ts
[warning] 264-264: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp((?:${alternatives.join('|')}), 'u')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 GitHub Actions: Test
rules/sort-regexp/has-shadowing-alternatives.ts
[error] 1-1: Prettier formatting check failed for file. The overall step 'pnpm test:format' failed due to formatting issues detected by Prettier (prettier --check). Run 'prettier --write' to fix code style issues.
rules/sort-regexp/get-first-character-paths.ts
[error] 1-1: Prettier formatting check failed for file. The overall step 'pnpm test:format' failed due to formatting issues detected by Prettier (prettier --check). Run 'prettier --write' to fix code style issues.
test/rules/sort-regexp-shadowing.test.ts
[error] 886-886: ts(2352): Conversion of type to CharacterSet may be a mistake. Cast to 'unknown' first or fix the type compatibility.
[error] 884-884: ts(2322): Type mismatch: elements array cannot be assigned to ClassRangesCharacterClassElement[] | UnicodeSetsCharacterClassElement[].
[error] 878-878: ts(6133): 'literal' is declared but its value is never read.
[error] 850-850: ts(6133): 'literal' is declared but its value is never read.
[error] 823-823: ts(6133): 'literal' is declared but its value is never read.
[error] 796-796: ts(6133): 'literal' is declared but its value is never read.
🪛 GitHub Check: JS
rules/sort-regexp/has-shadowing-alternatives.ts
[failure] 189-189:
Expected "hasShadowingDirection" to come before "hasFirstCharacterShadowing"
rules/sort-regexp/get-first-character-paths.ts
[failure] 721-721:
Update this function so that its implementation is not identical to the one on line 696
[failure] 721-721:
Expected "addMaxLengths" to come before "addLengths"
[failure] 648-648:
Update this function so that its implementation is not identical to the one on line 617
[failure] 648-648:
Expected "multiplyMaxLength" to come before "multiplyLength"
[failure] 646-646:
Comments should not begin with a lowercase character
[failure] 439-439:
Expected "getElementMaxLength" to come before "getGroupMinLength"
[failure] 303-303:
Expected "collectFirstCharacterPathsFromQuantifier" to come before "getElementMinLength"
[failure] 221-221:
Expected "canMatchMore" to come before "requiresMore"
[failure] 106-106:
Expected "minLengthActiveAlternatives" to come before "maxLengthCache"
| import type { | ||
| CapturingGroup, | ||
| CharacterClass, | ||
| CharacterSet, | ||
| Alternative, | ||
| Quantifier, | ||
| Element, | ||
| Group, | ||
| } from '@eslint-community/regexpp/ast' | ||
|
|
||
| const MAX_FIRST_CHARACTER_PATHS = 32 | ||
|
|
||
| /** Represents a deterministic path for the first character of an alternative. */ | ||
| export interface FirstCharacterPath { | ||
| /** Matcher that consumes the first character along the path. */ | ||
| matcher: CharacterMatcher | ||
|
|
||
| /** Indicates whether additional characters must follow to complete the match. */ | ||
| requiresMore: boolean | ||
|
|
||
| /** Indicates whether the alternative can consume more characters after the prefix. */ | ||
| canMatchMore: boolean | ||
| } | ||
|
|
||
| /** Matcher produced from a character class AST node. */ | ||
| export interface CharacterMatcherCharacterClass { | ||
| /** Identifies the matcher as a character class. */ | ||
| type: 'character-class' | ||
|
|
||
| /** AST node that defines the character class. */ | ||
| value: CharacterClass | ||
| } | ||
|
|
||
| /** Matcher produced from a character set AST node. */ | ||
| export interface CharacterMatcherCharacterSet { | ||
| /** Identifies the matcher as a character set. */ | ||
| type: 'character-set' | ||
|
|
||
| /** AST node that defines the character set. */ | ||
| value: CharacterSet | ||
| } | ||
|
|
||
| /** Matcher that describes a literal character code point. */ | ||
| export interface CharacterMatcherCharacter { | ||
| /** Identifies the matcher as a literal character. */ | ||
| type: 'character' | ||
|
|
||
| /** Unicode code point matched by the literal. */ | ||
| value: number | ||
| } | ||
|
|
||
| /** | ||
| * Describes a matcher capable of consuming the first character of an | ||
| * alternative. | ||
| */ | ||
| export type CharacterMatcher = | ||
| | CharacterMatcherCharacterClass | ||
| | CharacterMatcherCharacterSet | ||
| | CharacterMatcherCharacter | ||
|
|
||
| /** Tracks shared analysis state while traversing the AST. */ | ||
| interface AnalysisContext { | ||
| /** Cache storing computed minimum lengths for AST nodes. */ | ||
| minLengthCache: WeakMap<object, LengthResult> | ||
|
|
||
| /** Cache storing computed maximum lengths for AST nodes. */ | ||
| maxLengthCache: WeakMap<object, LengthResult> | ||
|
|
||
| /** Alternatives currently on the recursion stack. */ | ||
| minLengthActiveAlternatives: Set<Alternative> | ||
|
|
||
| /** Alternatives on the recursion stack for maximum-length calculation. */ | ||
| maxLengthActiveAlternatives: Set<Alternative> | ||
|
|
||
| /** Indicates whether collection exceeded the maximum allowed paths. */ | ||
| limitExceeded: boolean | ||
|
|
||
| /** Count of collected paths used for enforcing the limit. */ | ||
| pathCount: number | ||
| } | ||
|
|
||
| /** Internal extension that includes metadata needed during traversal. */ | ||
| interface FirstCharacterPathInternal extends FirstCharacterPath { | ||
| /** Mirrors the public flags for convenience when mutating paths. */ | ||
| requiresMore: boolean | ||
| canMatchMore: boolean | ||
| } | ||
|
|
||
| type LengthResult = LengthInfo | null | ||
|
|
||
| type LengthInfo = 0 | 1 | 2 | ||
|
|
||
| /** | ||
| * Computes all deterministic first-character paths for the given alternative. | ||
| * | ||
| * @param alternative - Alternative to analyze. | ||
| * @returns Collection of first-character matchers with information whether more | ||
| * characters are required afterwards. | ||
| */ | ||
| export function getFirstCharacterPaths( | ||
| alternative: Alternative, | ||
| ): FirstCharacterPath[] { | ||
| let context: AnalysisContext = { | ||
| minLengthCache: new WeakMap(), | ||
| maxLengthCache: new WeakMap(), | ||
| minLengthActiveAlternatives: new Set(), | ||
| maxLengthActiveAlternatives: new Set(), | ||
| limitExceeded: false, | ||
| pathCount: 0, | ||
| } | ||
|
|
||
| let paths = collectFirstCharacterPathsFromAlternative(alternative, context) | ||
|
|
||
| if (context.limitExceeded) { | ||
| return [] | ||
| } | ||
|
|
||
| return paths | ||
| } | ||
|
|
||
| /** | ||
| * Collects deterministic first-character paths that originate from the provided | ||
| * element. | ||
| * | ||
| * @param element - AST element to analyze. | ||
| * @param context - Shared traversal context. | ||
| * @returns Paths that can begin with the provided element. | ||
| */ | ||
| function collectFirstCharacterPathsFromElement( | ||
| element: Element, | ||
| context: AnalysisContext, | ||
| ): FirstCharacterPathInternal[] { | ||
| switch (element.type) { | ||
| case 'CharacterClass': { | ||
| if (element.unicodeSets) { | ||
| return [] | ||
| } | ||
|
|
||
| return [ | ||
| { | ||
| matcher: { type: 'character-class', value: element }, | ||
| requiresMore: false, | ||
| canMatchMore: false, | ||
| }, | ||
| ] | ||
| } | ||
| case 'CapturingGroup': | ||
| case 'Group': { | ||
| return element.alternatives.flatMap(alternative => | ||
| collectFirstCharacterPathsFromAlternative(alternative, context), | ||
| ) | ||
| } | ||
| case 'CharacterSet': { | ||
| if (element.kind === 'property' && element.strings) { | ||
| return [] | ||
| } | ||
|
|
||
| return [ | ||
| { | ||
| matcher: { type: 'character-set', value: element }, | ||
| requiresMore: false, | ||
| canMatchMore: false, | ||
| }, | ||
| ] | ||
| } | ||
| case 'Quantifier': { | ||
| return collectFirstCharacterPathsFromQuantifier(element, context) | ||
| } | ||
| case 'Character': { | ||
| return [ | ||
| { | ||
| matcher: { value: element.value, type: 'character' }, | ||
| requiresMore: false, | ||
| canMatchMore: false, | ||
| }, | ||
| ] | ||
| } | ||
| default: { | ||
| return [] | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Collects all first-character paths for an alternative. | ||
| * | ||
| * @param alternative - Alternative whose elements should be inspected. | ||
| * @param context - Shared traversal context. | ||
| * @returns Paths describing all deterministic prefixes. | ||
| */ | ||
| function collectFirstCharacterPathsFromAlternative( | ||
| alternative: Alternative, | ||
| context: AnalysisContext, | ||
| ): FirstCharacterPathInternal[] { | ||
| let results: FirstCharacterPathInternal[] = [] | ||
| let { elements } = alternative | ||
|
|
||
| for (let index = 0; index < elements.length; index++) { | ||
| if (context.limitExceeded) { | ||
| break | ||
| } | ||
|
|
||
| let element = elements[index]! | ||
|
|
||
| if (element.type === 'Assertion') { | ||
| continue | ||
| } | ||
|
|
||
| let elementPaths = collectFirstCharacterPathsFromElement(element, context) | ||
|
|
||
| if (elementPaths.length > 0) { | ||
| let restLength = getElementsMinLength(elements, index + 1, context) | ||
| let restMaxLength = getElementsMaxLength(elements, index + 1, context) | ||
|
|
||
| if (restLength !== null) { | ||
| let restCanMatchMore = restMaxLength !== 0 | ||
|
|
||
| for (let path of elementPaths) { | ||
| addPath(results, context, { | ||
| requiresMore: path.requiresMore || restLength > 0, | ||
| canMatchMore: path.canMatchMore || restCanMatchMore, | ||
| matcher: path.matcher, | ||
| }) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (!canElementMatchEmpty(element, context)) { | ||
| break | ||
| } | ||
| } | ||
|
|
||
| return results | ||
| } | ||
|
|
||
| /** | ||
| * Computes the minimum possible length for the provided element. | ||
| * | ||
| * @param element - AST element to analyze. | ||
| * @param context - Shared traversal context. | ||
| * @returns Minimum length in characters, `2` for "two or more", or `null` if | ||
| * unknown. | ||
| */ | ||
| function getElementMinLength( | ||
| element: Element, | ||
| context: AnalysisContext, | ||
| ): LengthResult { | ||
| if (context.limitExceeded) { | ||
| return null | ||
| } | ||
|
|
||
| let cached = context.minLengthCache.get(element) | ||
|
|
||
| if (cached !== undefined) { | ||
| return cached | ||
| } | ||
|
|
||
| let result: LengthResult = null | ||
|
|
||
| switch (element.type) { | ||
| case 'CharacterClass': | ||
| case 'CharacterSet': | ||
| case 'Character': { | ||
| result = 1 | ||
| break | ||
| } | ||
| case 'CapturingGroup': | ||
| case 'Group': { | ||
| result = getGroupMinLength(element, context) | ||
| break | ||
| } | ||
| case 'Backreference': { | ||
| result = null | ||
| break | ||
| } | ||
| case 'Quantifier': { | ||
| let innerLength = getElementMinLength(element.element, context) | ||
|
|
||
| result = multiplyLength(innerLength, element.min) | ||
| break | ||
| } | ||
| case 'Assertion': { | ||
| result = 0 | ||
| break | ||
| } | ||
| default: { | ||
| result = null | ||
| } | ||
| } | ||
|
|
||
| context.minLengthCache.set(element, result) | ||
|
|
||
| return result | ||
| } | ||
|
|
||
| /** | ||
| * Expands quantifiers into their potential first-character paths. | ||
| * | ||
| * @param quantifier - Quantifier node to analyze. | ||
| * @param context - Shared traversal context. | ||
| * @returns Paths contributed by the quantified expression. | ||
| */ | ||
| function collectFirstCharacterPathsFromQuantifier( | ||
| quantifier: Quantifier, | ||
| context: AnalysisContext, | ||
| ): FirstCharacterPathInternal[] { | ||
| let innerPaths = collectFirstCharacterPathsFromElement( | ||
| quantifier.element, | ||
| context, | ||
| ) | ||
|
|
||
| if (innerPaths.length === 0 || context.limitExceeded) { | ||
| return [] | ||
| } | ||
|
|
||
| let innerMinLength = getElementMinLength(quantifier.element, context) | ||
| if (innerMinLength === null) { | ||
| return [] | ||
| } | ||
|
|
||
| let innerMaxLength = getElementMaxLength(quantifier.element, context) | ||
| let requiresAdditionalIterations = quantifier.min > 1 && innerMinLength > 0 | ||
| let elementCanConsumeCharacters = innerMaxLength !== 0 | ||
| let allowsAdditionalIterations = | ||
| elementCanConsumeCharacters && | ||
| (quantifier.max === Infinity || quantifier.max > 1) | ||
|
|
||
| return innerPaths.map(path => ({ | ||
| requiresMore: path.requiresMore || requiresAdditionalIterations, | ||
| canMatchMore: path.canMatchMore || allowsAdditionalIterations, | ||
| matcher: path.matcher, | ||
| })) | ||
| } | ||
|
|
||
| /** | ||
| * Computes the minimum possible length for an alternative. | ||
| * | ||
| * @param alternative - Alternative whose elements should be measured. | ||
| * @param context - Shared traversal context. | ||
| * @returns Minimum length for the entire alternative. | ||
| */ | ||
| function getAlternativeMinLength( | ||
| alternative: Alternative, | ||
| context: AnalysisContext, | ||
| ): LengthResult { | ||
| let cached = context.minLengthCache.get(alternative) | ||
|
|
||
| if (cached !== undefined) { | ||
| return cached | ||
| } | ||
|
|
||
| if (context.minLengthActiveAlternatives.has(alternative)) { | ||
| return null | ||
| } | ||
|
|
||
| context.minLengthActiveAlternatives.add(alternative) | ||
|
|
||
| let length = getElementsMinLength(alternative.elements, 0, context) | ||
|
|
||
| context.minLengthActiveAlternatives.delete(alternative) | ||
| context.minLengthCache.set(alternative, length) | ||
|
|
||
| return length | ||
| } | ||
|
|
||
| /** | ||
| * Computes the minimum length of a suffix of elements. | ||
| * | ||
| * @param elements - Sequence of elements belonging to an alternative. | ||
| * @param startIndex - Index from which the suffix begins. | ||
| * @param context - Shared traversal context. | ||
| * @returns Minimum length for the suffix. | ||
| */ | ||
| function getElementsMinLength( | ||
| elements: Alternative['elements'], | ||
| startIndex: number, | ||
| context: AnalysisContext, | ||
| ): LengthResult { | ||
| let length: LengthResult = 0 | ||
|
|
||
| for (let index = startIndex; index < elements.length; index++) { | ||
| let element = elements[index]! | ||
| let elementLength = getElementMinLength(element, context) | ||
|
|
||
| length = addLengths(length, elementLength) | ||
|
|
||
| if (length === null) { | ||
| return null | ||
| } | ||
|
|
||
| if (length === 2) { | ||
| return 2 | ||
| } | ||
| } | ||
|
|
||
| return length | ||
| } | ||
|
|
||
| /** | ||
| * Computes the minimum length among the alternatives contained in a group. | ||
| * | ||
| * @param group - Capturing or non-capturing group to analyze. | ||
| * @param context - Shared traversal context. | ||
| * @returns Minimum length across the group's alternatives. | ||
| */ | ||
| function getGroupMinLength( | ||
| group: CapturingGroup | Group, | ||
| context: AnalysisContext, | ||
| ): LengthResult { | ||
| let minLength: LengthResult = 2 | ||
|
|
||
| for (let alternative of group.alternatives) { | ||
| let alternativeLength = getAlternativeMinLength(alternative, context) | ||
|
|
||
| if (alternativeLength === null) { | ||
| return null | ||
| } | ||
|
|
||
| if (alternativeLength < minLength) { | ||
| minLength = alternativeLength | ||
| } | ||
|
|
||
| if (minLength === 0) { | ||
| break | ||
| } | ||
| } | ||
|
|
||
| return minLength | ||
| } | ||
|
|
||
| /** | ||
| * Computes the maximum possible length for an element. | ||
| * | ||
| * @param element - AST element to analyze. | ||
| * @param context - Shared traversal context. | ||
| * @returns Maximum length in characters, `2` for "two or more", or `null` if | ||
| * unknown. | ||
| */ | ||
| function getElementMaxLength( | ||
| element: Element, | ||
| context: AnalysisContext, | ||
| ): LengthResult { | ||
| // Defensive guard triggers only when traversal exceeded path limit earlier. | ||
| /* c8 ignore next 3 */ | ||
| if (context.limitExceeded) { | ||
| return null | ||
| } | ||
|
|
||
| let cached = context.maxLengthCache.get(element) | ||
|
|
||
| if (cached !== undefined) { | ||
| return cached | ||
| } | ||
|
|
||
| let result: LengthResult = null | ||
|
|
||
| switch (element.type) { | ||
| case 'CharacterClass': | ||
| case 'CharacterSet': | ||
| case 'Character': { | ||
| result = 1 | ||
| break | ||
| } | ||
| case 'CapturingGroup': | ||
| case 'Group': { | ||
| result = getGroupMaxLength(element, context) | ||
| break | ||
| } | ||
| case 'Backreference': { | ||
| result = null | ||
| break | ||
| } | ||
| case 'Quantifier': { | ||
| let innerLength = getElementMaxLength(element.element, context) | ||
|
|
||
| if (innerLength === null) { | ||
| result = null | ||
| break | ||
| } | ||
|
|
||
| // Numerical sentinels are unreachable with current AST inputs. | ||
| /* c8 ignore start */ | ||
| if (innerLength === 0 || element.max === 0) { | ||
| result = 0 | ||
| break | ||
| } | ||
|
|
||
| if (element.max === Infinity) { | ||
| result = 2 | ||
| break | ||
| } | ||
| /* c8 ignore stop */ | ||
|
|
||
| result = multiplyMaxLength(innerLength, element.max) | ||
| break | ||
| } | ||
| case 'Assertion': { | ||
| result = 0 | ||
| break | ||
| } | ||
| default: { | ||
| result = null | ||
| } | ||
| } | ||
|
|
||
| context.maxLengthCache.set(element, result) | ||
|
|
||
| return result | ||
| } | ||
|
|
||
| /** | ||
| * Computes the maximum possible length for an alternative. | ||
| * | ||
| * @param alternative - Alternative whose elements should be measured. | ||
| * @param context - Shared traversal context. | ||
| * @returns Maximum length for the entire alternative. | ||
| */ | ||
| function getAlternativeMaxLength( | ||
| alternative: Alternative, | ||
| context: AnalysisContext, | ||
| ): LengthResult { | ||
| let cached = context.maxLengthCache.get(alternative) | ||
|
|
||
| // Cache reuse only occurs for recursive alternatives, which tests do not create. | ||
| /* c8 ignore next 3 */ | ||
| if (cached !== undefined) { | ||
| return cached | ||
| } | ||
|
|
||
| if (context.maxLengthActiveAlternatives.has(alternative)) { | ||
| return null | ||
| } | ||
|
|
||
| context.maxLengthActiveAlternatives.add(alternative) | ||
|
|
||
| let length = getElementsMaxLength(alternative.elements, 0, context) | ||
|
|
||
| context.maxLengthActiveAlternatives.delete(alternative) | ||
| context.maxLengthCache.set(alternative, length) | ||
|
|
||
| return length | ||
| } | ||
|
|
||
| /** | ||
| * Computes the maximum length of a suffix of elements. | ||
| * | ||
| * @param elements - Sequence of elements belonging to an alternative. | ||
| * @param startIndex - Index from which the suffix begins. | ||
| * @param context - Shared traversal context. | ||
| * @returns Maximum length for the suffix. | ||
| */ | ||
| function getElementsMaxLength( | ||
| elements: Alternative['elements'], | ||
| startIndex: number, | ||
| context: AnalysisContext, | ||
| ): LengthResult { | ||
| let length: LengthResult = 0 | ||
|
|
||
| for (let index = startIndex; index < elements.length; index++) { | ||
| let element = elements[index]! | ||
| let elementLength = getElementMaxLength(element, context) | ||
|
|
||
| length = addMaxLengths(length, elementLength) | ||
|
|
||
| if (length === null) { | ||
| return null | ||
| } | ||
|
|
||
| if (length === 2) { | ||
| return 2 | ||
| } | ||
| } | ||
|
|
||
| return length | ||
| } | ||
|
|
||
| /** | ||
| * Computes the maximum length among the alternatives contained in a group. | ||
| * | ||
| * @param group - Capturing or non-capturing group to analyze. | ||
| * @param context - Shared traversal context. | ||
| * @returns Maximum length across the group's alternatives. | ||
| */ | ||
| function getGroupMaxLength( | ||
| group: CapturingGroup | Group, | ||
| context: AnalysisContext, | ||
| ): LengthResult { | ||
| let maxLength: LengthResult = 0 | ||
|
|
||
| for (let alternative of group.alternatives) { | ||
| let alternativeLength = getAlternativeMaxLength(alternative, context) | ||
|
|
||
| if (alternativeLength === null) { | ||
| return null | ||
| } | ||
|
|
||
| if (alternativeLength > maxLength) { | ||
| maxLength = alternativeLength | ||
| } | ||
|
|
||
| if (maxLength === 2) { | ||
| break | ||
| } | ||
| } | ||
|
|
||
| return maxLength | ||
| } | ||
|
|
||
| /** | ||
| * Multiplies a minimum length by a quantifier count while respecting sentinel | ||
| * values. | ||
| * | ||
| * @param length - Minimum length of the repeated element. | ||
| * @param count - Minimum number of repetitions. | ||
| * @returns Combined minimum length or `null` when unknown. | ||
| */ | ||
| function multiplyLength(length: LengthResult, count: number): LengthResult { | ||
| if (length === null) { | ||
| return null | ||
| } | ||
|
|
||
| if (length === 0 || count === 0) { | ||
| return 0 | ||
| } | ||
|
|
||
| if (length === 2) { | ||
| return 2 | ||
| } | ||
|
|
||
| if (count === 1) { | ||
| return length | ||
| } | ||
|
|
||
| return 2 | ||
| } | ||
|
|
||
| /** | ||
| * Multiplies a maximum length by a quantifier count while respecting sentinel | ||
| * values. | ||
| * | ||
| * @param length - Maximum length of the repeated element. | ||
| * @param count - Maximum number of repetitions. | ||
| * @returns Combined maximum length or `null` when unknown. | ||
| */ | ||
| // Exhaustive runtime coverage would require crafting pathological recursive | ||
| // quantifiers and backreferences, so skip coverage for this helper. | ||
| /* c8 ignore start */ | ||
| function multiplyMaxLength(length: LengthResult, count: number): LengthResult { | ||
| if (length === null) { | ||
| return null | ||
| } | ||
|
|
||
| if (length === 0 || count === 0) { | ||
| return 0 | ||
| } | ||
|
|
||
| if (length === 2) { | ||
| return 2 | ||
| } | ||
|
|
||
| if (count === 1) { | ||
| return length | ||
| } | ||
|
|
||
| return 2 | ||
| } | ||
| /* c8 ignore stop */ | ||
|
|
||
| /** | ||
| * Adds a collected path to the results while accounting for the safety limit. | ||
| * | ||
| * @param results - Accumulated path list. | ||
| * @param context - Shared traversal context. | ||
| * @param path - Path to add. | ||
| */ | ||
| function addPath( | ||
| results: FirstCharacterPathInternal[], | ||
| context: AnalysisContext, | ||
| path: FirstCharacterPathInternal, | ||
| ): void { | ||
| results.push(path) | ||
| context.pathCount += 1 | ||
|
|
||
| if (context.pathCount > MAX_FIRST_CHARACTER_PATHS) { | ||
| context.limitExceeded = true | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Adds two minimum-length values together, preserving sentinel semantics. | ||
| * | ||
| * @param a - First length operand. | ||
| * @param b - Second length operand. | ||
| * @returns Sum of the operands, clamped to the sentinel space. | ||
| */ | ||
| function addLengths(a: LengthResult, b: LengthResult): LengthResult { | ||
| if (a === null || b === null) { | ||
| return null | ||
| } | ||
|
|
||
| if (a === 2 || b === 2) { | ||
| return 2 | ||
| } | ||
|
|
||
| let sum = a + b | ||
|
|
||
| if (sum >= 2) { | ||
| return 2 | ||
| } | ||
|
|
||
| return sum as LengthInfo | ||
| } | ||
|
|
||
| /** | ||
| * Adds two maximum-length values together, preserving sentinel semantics. | ||
| * | ||
| * @param a - First length operand. | ||
| * @param b - Second length operand. | ||
| * @returns Sum of the operands, clamped to the sentinel space. | ||
| */ | ||
| function addMaxLengths(a: LengthResult, b: LengthResult): LengthResult { | ||
| if (a === null || b === null) { | ||
| return null | ||
| } | ||
|
|
||
| if (a === 2 || b === 2) { | ||
| return 2 | ||
| } | ||
|
|
||
| let sum = a + b | ||
|
|
||
| if (sum >= 2) { | ||
| return 2 | ||
| } | ||
|
|
||
| return sum as LengthInfo | ||
| } | ||
|
|
||
| /** | ||
| * Determines whether a given element can match an empty string. | ||
| * | ||
| * @param element - AST element to inspect. | ||
| * @param context - Shared traversal context. | ||
| * @returns True when the element can match zero characters. | ||
| */ | ||
| function canElementMatchEmpty( | ||
| element: Element, | ||
| context: AnalysisContext, | ||
| ): boolean { | ||
| let length = getElementMinLength(element, context) | ||
| return length === 0 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Run Prettier to fix formatting issues.
The pipeline reports formatting issues detected by Prettier. Run prettier --write rules/sort-regexp/get-first-character-paths.ts to resolve.
#!/bin/bash
# Run Prettier on the file
prettier --write rules/sort-regexp/get-first-character-paths.ts🧰 Tools
🪛 GitHub Actions: Test
[error] 1-1: Prettier formatting check failed for file. The overall step 'pnpm test:format' failed due to formatting issues detected by Prettier (prettier --check). Run 'prettier --write' to fix code style issues.
🪛 GitHub Check: JS
[failure] 721-721:
Update this function so that its implementation is not identical to the one on line 696
[failure] 721-721:
Expected "addMaxLengths" to come before "addLengths"
[failure] 648-648:
Update this function so that its implementation is not identical to the one on line 617
[failure] 648-648:
Expected "multiplyMaxLength" to come before "multiplyLength"
[failure] 646-646:
Comments should not begin with a lowercase character
[failure] 439-439:
Expected "getElementMaxLength" to come before "getGroupMinLength"
[failure] 303-303:
Expected "collectFirstCharacterPathsFromQuantifier" to come before "getElementMinLength"
[failure] 221-221:
Expected "canMatchMore" to come before "requiresMore"
[failure] 106-106:
Expected "minLengthActiveAlternatives" to come before "maxLengthCache"
🤖 Prompt for AI Agents
rules/sort-regexp/get-first-character-paths.ts lines 1-752: the PR failed CI due
to Prettier formatting issues; run Prettier to reformat the file (e.g., execute
`prettier --write rules/sort-regexp/get-first-character-paths.ts`) and commit
the updated file so the formatting matches the project's Prettier configuration.
| import type { | ||
| CharacterUnicodePropertyCharacterSet, | ||
| CharacterClassElement, | ||
| CharacterClassRange, | ||
| CharacterClass, | ||
| CharacterSet, | ||
| Alternative, | ||
| } from '@eslint-community/regexpp/ast' | ||
|
|
||
| import type { | ||
| CharacterMatcherCharacterClass, | ||
| CharacterMatcherCharacterSet, | ||
| FirstCharacterPath, | ||
| CharacterMatcher, | ||
| } from './get-first-character-paths' | ||
|
|
||
| import { | ||
| isLowercaseCharacter, | ||
| isUppercaseCharacter, | ||
| isDigitCharacter, | ||
| } from './get-character-class-element-category' | ||
| import { doesAlternativeShadowOther } from './does-alternative-shadow-other' | ||
| import { getFirstCharacterPaths } from './get-first-character-paths' | ||
|
|
||
| interface MatcherEvaluationContext { | ||
| dotAll: boolean | ||
| } | ||
|
|
||
| /** | ||
| * Checks whether the provided alternatives contain shadowing pairs. | ||
| * | ||
| * @param parameters - Alternatives to analyze. | ||
| * @returns True when at least one alternative shadows another one. | ||
| */ | ||
| export function hasShadowingAlternatives({ | ||
| alternatives, | ||
| flags, | ||
| }: { | ||
| alternatives: Alternative[] | ||
| flags: string | ||
| }): boolean { | ||
| let hasNegatedCharacterClassAlternative = alternatives.some(alternative => { | ||
| let firstElement = alternative.elements.at(0) | ||
|
|
||
| if (!firstElement) { | ||
| return false | ||
| } | ||
|
|
||
| if ( | ||
| firstElement.type === 'Quantifier' && | ||
| firstElement.element.type === 'CharacterClass' | ||
| ) { | ||
| return firstElement.element.negate | ||
| } | ||
|
|
||
| return firstElement.type === 'CharacterClass' && firstElement.negate | ||
| }) | ||
|
|
||
| if (hasNegatedCharacterClassAlternative) { | ||
| return true | ||
| } | ||
|
|
||
| let rawAlternatives = alternatives.map(alternative => alternative.raw) | ||
|
|
||
| for (let index = 0; index < rawAlternatives.length; index++) { | ||
| let current = rawAlternatives[index]! | ||
|
|
||
| for (let offset = index + 1; offset < rawAlternatives.length; offset++) { | ||
| let other = rawAlternatives[offset]! | ||
|
|
||
| if (doesAlternativeShadowOther(current, other)) { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let matcherContext: MatcherEvaluationContext = { | ||
| dotAll: flags.includes('s'), | ||
| } | ||
|
|
||
| if ( | ||
| hasFirstCharacterShadowing({ | ||
| matcherContext, | ||
| alternatives, | ||
| }) | ||
| ) { | ||
| return true | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| function characterSetContainsCodePoint({ | ||
| matcherContext, | ||
| characterSet, | ||
| codePoint, | ||
| }: { | ||
| matcherContext: MatcherEvaluationContext | ||
| characterSet: CharacterSet | ||
| codePoint: number | ||
| }): boolean | null { | ||
| switch (characterSet.kind) { | ||
| case 'property': { | ||
| if (characterSet.strings) { | ||
| return null | ||
| } | ||
|
|
||
| let matches = unicodePropertyContainsCodePoint({ | ||
| propertySet: characterSet, | ||
| codePoint, | ||
| }) | ||
|
|
||
| if (matches === null) { | ||
| return null | ||
| } | ||
|
|
||
| return applyNegation(characterSet, matches) | ||
| } | ||
| case 'digit': { | ||
| return applyNegation(characterSet, isDigitCharacter(codePoint)) | ||
| } | ||
| case 'space': { | ||
| return applyNegation(characterSet, isWhitespaceCharacter(codePoint)) | ||
| } | ||
| case 'word': { | ||
| return applyNegation(characterSet, isWordCharacter(codePoint)) | ||
| } | ||
| case 'any': { | ||
| return matchesAnyCharacterSet({ | ||
| matcherContext, | ||
| codePoint, | ||
| }) | ||
| } | ||
| default: { | ||
| return null | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function hasFirstCharacterShadowing({ | ||
| matcherContext, | ||
| alternatives, | ||
| }: { | ||
| matcherContext: MatcherEvaluationContext | ||
| alternatives: Alternative[] | ||
| }): boolean { | ||
| let firstCharacterPaths = alternatives.map(alternative => | ||
| getFirstCharacterPaths(alternative), | ||
| ) | ||
|
|
||
| for (let index = 0; index < firstCharacterPaths.length; index++) { | ||
| let current = firstCharacterPaths[index]! | ||
|
|
||
| if (current.length === 0) { | ||
| continue | ||
| } | ||
|
|
||
| for ( | ||
| let offset = index + 1; | ||
| offset < firstCharacterPaths.length; | ||
| offset++ | ||
| ) { | ||
| let other = firstCharacterPaths[offset]! | ||
|
|
||
| if (other.length === 0) { | ||
| continue | ||
| } | ||
|
|
||
| if ( | ||
| hasShadowingDirection({ | ||
| longerPaths: current, | ||
| shorterPaths: other, | ||
| matcherContext, | ||
| }) || | ||
| hasShadowingDirection({ | ||
| shorterPaths: current, | ||
| longerPaths: other, | ||
| matcherContext, | ||
| }) | ||
| ) { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| function hasShadowingDirection({ | ||
| matcherContext, | ||
| shorterPaths, | ||
| longerPaths, | ||
| }: { | ||
| matcherContext: MatcherEvaluationContext | ||
| shorterPaths: FirstCharacterPath[] | ||
| longerPaths: FirstCharacterPath[] | ||
| }): boolean { | ||
| for (let longerPath of longerPaths) { | ||
| if (!longerPath.requiresMore && !longerPath.canMatchMore) { | ||
| continue | ||
| } | ||
|
|
||
| let longerNeedsWildcardOverlap = longerPath.matcher.type !== 'character' | ||
|
|
||
| for (let shorterPath of shorterPaths) { | ||
| if (shorterPath.requiresMore) { | ||
| continue | ||
| } | ||
|
|
||
| if (shorterPath.canMatchMore) { | ||
| continue | ||
| } | ||
|
|
||
| if (shorterPath.matcher.type === 'character') { | ||
| continue | ||
| } | ||
|
|
||
| if (longerNeedsWildcardOverlap) { | ||
| if (isWildcardMatcher(shorterPath.matcher)) { | ||
| return true | ||
| } | ||
|
|
||
| continue | ||
| } | ||
|
|
||
| if ( | ||
| doMatchersOverlap({ | ||
| right: shorterPath.matcher, | ||
| left: longerPath.matcher, | ||
| matcherContext, | ||
| }) | ||
| ) { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| function unicodePropertyContainsCodePoint({ | ||
| propertySet, | ||
| codePoint, | ||
| }: { | ||
| propertySet: CharacterUnicodePropertyCharacterSet | ||
| codePoint: number | ||
| }): boolean | null { | ||
| let cacheKey = `${propertySet.key}:${propertySet.value ?? ''}:${ | ||
| propertySet.negate ? '1' : '0' | ||
| }` | ||
| let cached = unicodePropertyRegexCache.get(cacheKey) | ||
|
|
||
| if (!cached) { | ||
| try { | ||
| let identifier = | ||
| propertySet.value === null | ||
| ? propertySet.key | ||
| : `${propertySet.key}=${propertySet.value}` | ||
| cached = new RegExp(String.raw`\p{${identifier}}`, 'u') | ||
| unicodePropertyRegexCache.set(cacheKey, cached) | ||
| } catch { | ||
| return null | ||
| } | ||
| } | ||
|
|
||
| return cached.test(String.fromCodePoint(codePoint)) | ||
| } | ||
|
|
||
| function classElementContainsCodePoint({ | ||
| matcherContext, | ||
| codePoint, | ||
| element, | ||
| }: { | ||
| matcherContext: MatcherEvaluationContext | ||
| element: CharacterClassElement | ||
| codePoint: number | ||
| }): boolean | null { | ||
| switch (element.type) { | ||
| case 'CharacterClassRange': { | ||
| return characterClassRangeContainsCodePoint({ | ||
| range: element, | ||
| codePoint, | ||
| }) | ||
| } | ||
| case 'CharacterSet': { | ||
| return characterSetContainsCodePoint({ | ||
| characterSet: element, | ||
| matcherContext, | ||
| codePoint, | ||
| }) | ||
| } | ||
| case 'Character': { | ||
| return element.value === codePoint | ||
| } | ||
| default: { | ||
| return null | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function doMatchersOverlap({ | ||
| matcherContext, | ||
| right, | ||
| left, | ||
| }: { | ||
| right: CharacterMatcherCharacterClass | CharacterMatcherCharacterSet | ||
| matcherContext: MatcherEvaluationContext | ||
| left: CharacterMatcher | ||
| }): boolean { | ||
| // Left is always a literal character in reachable flows. | ||
| /* c8 ignore next 3 */ | ||
| if (left.type !== 'character') { | ||
| return false | ||
| } | ||
|
|
||
| if (right.type === 'character-class') { | ||
| return matcherContainsCharacter({ | ||
| characterClass: right.value, | ||
| codePoint: left.value, | ||
| matcherContext, | ||
| }) | ||
| } | ||
|
|
||
| return ( | ||
| characterSetContainsCodePoint({ | ||
| characterSet: right.value, | ||
| codePoint: left.value, | ||
| matcherContext, | ||
| }) ?? false | ||
| ) | ||
| } | ||
|
|
||
| function characterClassContainsCodePoint({ | ||
| characterClass, | ||
| matcherContext, | ||
| codePoint, | ||
| }: { | ||
| matcherContext: MatcherEvaluationContext | ||
| characterClass: CharacterClass | ||
| codePoint: number | ||
| }): boolean | null { | ||
| if (characterClass.unicodeSets) { | ||
| return null | ||
| } | ||
|
|
||
| let isMatched = false | ||
|
|
||
| for (let element of characterClass.elements) { | ||
| if ( | ||
| classElementContainsCodePoint({ | ||
| matcherContext, | ||
| codePoint, | ||
| element, | ||
| }) | ||
| ) { | ||
| isMatched = true | ||
| break | ||
| } | ||
| } | ||
|
|
||
| return characterClass.negate ? !isMatched : isMatched | ||
| } | ||
|
|
||
| function matcherContainsCharacter({ | ||
| matcherContext, | ||
| characterClass, | ||
| codePoint, | ||
| }: { | ||
| matcherContext: MatcherEvaluationContext | ||
| characterClass: CharacterClass | ||
| codePoint: number | ||
| }): boolean { | ||
| return ( | ||
| characterClassContainsCodePoint({ | ||
| characterClass, | ||
| matcherContext, | ||
| codePoint, | ||
| }) ?? false | ||
| ) | ||
| } | ||
|
|
||
| function matchesAnyCharacterSet({ | ||
| matcherContext, | ||
| codePoint, | ||
| }: { | ||
| matcherContext: MatcherEvaluationContext | ||
| codePoint: number | ||
| }): boolean { | ||
| if (matcherContext.dotAll) { | ||
| return true | ||
| } | ||
|
|
||
| return !isLineTerminator(codePoint) | ||
| } | ||
|
|
||
| function isWildcardMatcher( | ||
| matcher: CharacterMatcherCharacterClass | CharacterMatcherCharacterSet, | ||
| ): boolean { | ||
| if (matcher.type === 'character-set') { | ||
| return matcher.value.kind === 'any' | ||
| } | ||
|
|
||
| return isComplementaryCharacterClass(matcher.value) | ||
| } | ||
|
|
||
| function isComplementaryCharacterClass(characterClass: CharacterClass): boolean { | ||
| // Unicode set aware classes never map into character-class matchers. | ||
| /* c8 ignore next 3 */ | ||
| if (characterClass.unicodeSets) { | ||
| return false | ||
| } | ||
|
|
||
| // Empty-negated classes (e.g. `[^]`) are normalized before reaching here. | ||
| /* c8 ignore next 3 */ | ||
| if (characterClass.negate && characterClass.elements.length === 0) { | ||
| return true | ||
| } | ||
|
|
||
| let seen = new Map<string, boolean>() | ||
|
|
||
| for (let element of characterClass.elements) { | ||
| if (element.type !== 'CharacterSet') { | ||
| continue | ||
| } | ||
|
|
||
| let identifier = getCharacterSetIdentifier(element) | ||
|
|
||
| // String-based property escapes are filtered earlier. | ||
| /* c8 ignore next 3 */ | ||
| if (!identifier) { | ||
| continue | ||
| } | ||
|
|
||
| let previousNegation = seen.get(identifier) | ||
|
|
||
| if (previousNegation === undefined) { | ||
| seen.set(identifier, Boolean(element.negate)) | ||
| continue | ||
| } | ||
|
|
||
| if (previousNegation !== Boolean(element.negate)) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| function getCharacterSetIdentifier(characterSet: CharacterSet): string | null { | ||
| switch (characterSet.kind) { | ||
| case 'digit': | ||
| case 'space': | ||
| case 'word': | ||
| case 'any': { | ||
| return characterSet.kind | ||
| } | ||
| case 'property': { | ||
| if (characterSet.strings) { | ||
| return null | ||
| } | ||
|
|
||
| return `${characterSet.key}:${characterSet.value ?? ''}` | ||
| } | ||
| default: { | ||
| return null | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function isWordCharacter(codePoint: number): boolean { | ||
| return ( | ||
| isDigitCharacter(codePoint) || | ||
| isLowercaseCharacter(codePoint) || | ||
| isUppercaseCharacter(codePoint) || | ||
| codePoint === UNDERSCORE_CODE_POINT | ||
| ) | ||
| } | ||
|
|
||
| function characterClassRangeContainsCodePoint({ | ||
| codePoint, | ||
| range, | ||
| }: { | ||
| range: CharacterClassRange | ||
| codePoint: number | ||
| }): boolean { | ||
| return range.min.value <= codePoint && codePoint <= range.max.value | ||
| } | ||
|
|
||
| function isLineTerminator(codePoint: number): boolean { | ||
| return ( | ||
| codePoint === 0x0a || | ||
| codePoint === 0x0d || | ||
| codePoint === 0x2028 || | ||
| codePoint === 0x2029 | ||
| ) | ||
| } | ||
|
|
||
| function isWhitespaceCharacter(codePoint: number): boolean { | ||
| return WHITESPACE_PATTERN.test(String.fromCodePoint(codePoint)) | ||
| } | ||
|
|
||
| function applyNegation(node: { negate?: boolean }, result: boolean): boolean { | ||
| return node.negate ? !result : result | ||
| } | ||
|
|
||
| const WHITESPACE_PATTERN = /\s/u | ||
| const UNDERSCORE_CODE_POINT = '_'.codePointAt(0)! | ||
| let unicodePropertyRegexCache = new Map<string, RegExp>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Run Prettier to fix formatting issues.
The pipeline reports formatting issues detected by Prettier. Run prettier --write rules/sort-regexp/has-shadowing-alternatives.ts to resolve.
#!/bin/bash
# Run Prettier on the file
prettier --write rules/sort-regexp/has-shadowing-alternatives.ts🧰 Tools
🪛 GitHub Actions: Test
[error] 1-1: Prettier formatting check failed for file. The overall step 'pnpm test:format' failed due to formatting issues detected by Prettier (prettier --check). Run 'prettier --write' to fix code style issues.
🪛 GitHub Check: JS
[failure] 189-189:
Expected "hasShadowingDirection" to come before "hasFirstCharacterShadowing"
🤖 Prompt for AI Agents
rules/sort-regexp/has-shadowing-alternatives.ts lines 1-508: Prettier reports
formatting issues in this file; run the project's Prettier formatter to fix
them. Execute prettier --write rules/sort-regexp/has-shadowing-alternatives.ts
(or npm/yarn script that runs Prettier) to reformat the file, verify the
changes, stage and commit the updated file, and push the commit so the CI
pre-checks pass.
| }) | ||
|
|
||
| it('detects overlaps when complementary property escapes form a wildcard class', () => { | ||
| let literal = parseRegExpLiteral(/a|b/u) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused literal variable.
The literal variable is declared but never used.
Apply this diff:
- let literal = parseRegExpLiteral(/a|b/u)
let propClass = parseRegExpLiteral(/[\p{ASCII}\P{ASCII}]/u).pattern.alternatives[0]!📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let literal = parseRegExpLiteral(/a|b/u) | |
| let propClass = parseRegExpLiteral(/[\p{ASCII}\P{ASCII}]/u).pattern.alternatives[0]! |
🧰 Tools
🪛 GitHub Actions: Test
[error] 823-823: ts(6133): 'literal' is declared but its value is never read.
🤖 Prompt for AI Agents
In test/rules/sort-regexp-shadowing.test.ts around line 823, the variable
"literal" is declared with let literal = parseRegExpLiteral(/a|b/u) but never
used; remove that line to eliminate the unused-variable declaration (or, if the
parsed regex is intended to be used, replace the declaration with the correct
usage—otherwise simply delete the line).
| }) | ||
|
|
||
| it('ignores string-based property escapes when checking wildcard classes', () => { | ||
| let literal = parseRegExpLiteral(/a|b/u) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused literal variable.
The literal variable is declared but never used.
Apply this diff:
- let literal = parseRegExpLiteral(/a|b/u)
let emojiClass = parseRegExpLiteral(/[\p{RGI_Emoji}]/v).pattern.alternatives[0]!📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let literal = parseRegExpLiteral(/a|b/u) | |
| let emojiClass = parseRegExpLiteral(/[\p{RGI_Emoji}]/v).pattern.alternatives[0]! |
🧰 Tools
🪛 GitHub Actions: Test
[error] 850-850: ts(6133): 'literal' is declared but its value is never read.
🤖 Prompt for AI Agents
In test/rules/sort-regexp-shadowing.test.ts around line 850, there is an unused
variable declaration "let literal = parseRegExpLiteral(/a|b/u)"; remove this
declaration (or use the result if intended) so the test file no longer defines
an unused variable and the linter error is resolved.
| }) | ||
|
|
||
| it('falls back gracefully when encountering unknown character set kinds', () => { | ||
| let literal = parseRegExpLiteral(/a|b/u) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused literal variable.
The literal variable is declared but never used.
Apply this diff:
- let literal = parseRegExpLiteral(/a|b/u)
let patchedClass = parseRegExpLiteral(/[a-z]n/u).pattern.alternatives[0]!📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let literal = parseRegExpLiteral(/a|b/u) | |
| let patchedClass = parseRegExpLiteral(/[a-z]n/u).pattern.alternatives[0]! |
🧰 Tools
🪛 GitHub Actions: Test
[error] 878-878: ts(6133): 'literal' is declared but its value is never read.
🤖 Prompt for AI Agents
In test/rules/sort-regexp-shadowing.test.ts around line 878, the variable
`literal` is declared with let literal = parseRegExpLiteral(/a|b/u) but never
used; remove this unused declaration (delete the line) so there is no unused
variable left in the test.
e42e14c to
1b1623d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
rules/sort-regexp/get-first-character-paths.ts (1)
326-356: Handle{0}quantifiers so impossible prefixes are not reportedFor quantifiers where
max === 0(e.g.,/a{0}b/), the quantified element can never consume characters, butcollectFirstCharacterPathsFromQuantifierstill forwardsinnerPaths, producing a spurious first‑character path for'a'. That path is impossible at runtime and can distort downstream analyses.You can short‑circuit these cases:
function collectFirstCharacterPathsFromQuantifier( quantifier: Quantifier, context: AnalysisContext, ): FirstCharacterPathInternal[] { + // Quantifiers with max === 0 can only match the empty string and never + // consume their inner element, so they contribute no first‑character paths. + if (quantifier.max === 0) { + return [] + } + let innerPaths = collectFirstCharacterPathsFromElement( quantifier.element, context, )This preserves existing behavior for all other quantifiers while ensuring
{0}quantifiers don’t introduce impossible prefixes.
🧹 Nitpick comments (1)
rules/sort-regexp/has-shadowing-alternatives.ts (1)
386-415: Consider guarding against missingcharacterClass.elementsincharacterClassContainsCodePointTests already simulate malformed classes with missing
elements, and current flows avoid passing those intocharacterClassContainsCodePoint, but this function would currently throw if it ever received such a node.You could defensively short‑circuit:
function characterClassContainsCodePoint({ characterClass, matcherContext, codePoint, }: { matcherContext: MatcherEvaluationContext characterClass: CharacterClass codePoint: number }): boolean | null { - if (characterClass.unicodeSets) { + if (characterClass.unicodeSets) { return null } - let isMatched = false + let elements = (characterClass as { elements?: CharacterClass['elements'] }) + .elements + + if (!elements) { + return null + } + + let isMatched = false - for (let element of characterClass.elements) { + for (let element of elements) {This keeps behavior unchanged for well-formed ASTs but makes the helper robust to intentionally corrupted nodes in future tests or callers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
rules/sort-regexp/get-first-character-paths.ts(1 hunks)rules/sort-regexp/has-shadowing-alternatives.ts(1 hunks)test/rules/sort-regexp-shadowing.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
rules/sort-regexp/has-shadowing-alternatives.ts (3)
rules/sort-regexp/does-alternative-shadow-other.ts (1)
doesAlternativeShadowOther(8-25)rules/sort-regexp/get-first-character-paths.ts (5)
FirstCharacterPath(14-26)getFirstCharacterPaths(103-122)CharacterMatcherCharacterClass(29-35)CharacterMatcherCharacterSet(38-44)CharacterMatcher(59-62)rules/sort-regexp/get-character-class-element-category.ts (3)
isDigitCharacter(115-117)isLowercaseCharacter(95-97)isUppercaseCharacter(105-107)
test/rules/sort-regexp-shadowing.test.ts (2)
rules/sort-regexp/get-first-character-paths.ts (2)
FirstCharacterPath(14-26)getFirstCharacterPaths(103-122)rules/sort-regexp/has-shadowing-alternatives.ts (1)
hasShadowingAlternatives(35-91)
🪛 ast-grep (0.40.0)
test/rules/sort-regexp-shadowing.test.ts
[warning] 264-264: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp((?:${alternatives.join('|')}), 'u')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: codecov/patch
🔇 Additional comments (5)
test/rules/sort-regexp-shadowing.test.ts (3)
22-69: Spy usage inexpectDotAllOverlapForcorrectly mirrors internal call patternThe four
mockImplementationOncecalls line up with two alternatives × twohasShadowingAlternativesinvocations, and the spy is restored afterward, so this helper cleanly exercises the dotAll behavior without leaking mocks.
137-429: Comprehensive coverage ofgetFirstCharacterPathsedge casesThese tests probe a wide range of AST shapes (optional quantifiers, backreferences, recursive/synthetic groups, unicode sets, safety limits), which should make future refactors of
get-first-character-pathsmuch safer.
431-997: Shadowing tests align well withhasShadowingAlternativesinvariantsThe scenarios around newlines, wildcard/dotAll, property escapes (including invalid and string-based), unknown kinds/elements, and caching behavior closely track the intended safeguards of
hasShadowingAlternatives, giving strong confidence that autofixes remain semantics-preserving.rules/sort-regexp/has-shadowing-alternatives.ts (1)
35-91: High-level shadowing strategy looks soundThe staged approach (negated leading classes → raw alternative comparison → first-character path analysis) matches the documented semantics and the tests, and it errs conservatively by treating difficult cases as shadowing to avoid unsafe reordering.
rules/sort-regexp/get-first-character-paths.ts (1)
103-122: Overall first-character path analysis and safety limit look goodThe bounded path collection with
MAX_FIRST_CHARACTER_PATHSandlimitExceededfallback to[]is a reasonable tradeoff between precision and safety, and the caching/recursion guards in the length helpers should keep this analysis robust on complex regexes.

Description
A new rule for sorting regular expressions is added. It works with:
/(error|warning|info|debug)/)/[zxa-f0-9A-Z]/)/pattern/igmus)Additional context
N/A.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following