feat(extensions): support pathGroupOverrides and fix options
#326
Conversation
|
WalkthroughThis PR adds support for two new options in the extensions rule: Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| return true | ||
| function computeOverrideAction(overrides: PathGroupOverride[], path: string) { | ||
| for (const { pattern, patternOptions, action } of overrides) { | ||
| if (minimatch(path, pattern, patternOptions || { nocomment: true })) { |
There was a problem hiding this comment.
The use of minimatch here should be carefully validated to ensure that the patternOptions do not introduce any security vulnerabilities, such as ReDoS (Regular Expression Denial of Service). Consider validating or sanitizing patternOptions before use.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
""" WalkthroughThis update to the extensions rule introduces a new configuration option, Changes
Sequence Diagram(s)sequenceDiagram
participant UserConfig
participant Rule
participant computeOverrideAction
participant Fixer
UserConfig->>Rule: Provide import path and options (with pathGroupOverrides, fix)
Rule->>computeOverrideAction: Check for override action on import path
computeOverrideAction-->>Rule: Return 'enforce', 'ignore', or undefined
alt action is 'ignore'
Rule-->>Rule: Skip extension enforcement
else action is 'enforce' or undefined
Rule-->>Rule: Proceed with extension checks
alt fix option is true and issue found
Rule->>Fixer: Apply autofix (add/remove extension)
end
end
Poem
""" Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/rules/extensions.tsOops! Something went wrong! :( ESLint: 9.27.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (12)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to c71d953 in 2 minutes and 43 seconds. Click for details.
- Reviewed
193lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/rules/extensions.ts:271
- Draft comment:
Consider using an explicit check (e.g. 'overrideAction === undefined') instead of relying on the falsy check (!overrideAction) for built-in and external module conditions for clarity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The current !overrideAction check is technically correct since overrideAction can only be 'enforce', 'ignore' or undefined. Both 'enforce' and 'ignore' are truthy strings, so !overrideAction will only be true when it's undefined. While an explicit check might be marginally clearer, the current code is not incorrect or confusing enough to warrant a comment. The suggestion could make the code slightly more explicit and self-documenting. Some developers prefer explicit undefined checks over falsy checks. While explicit checks can be good practice, this case is straightforward enough that the current code is clear and correct. The comment is suggesting a style preference rather than fixing a real issue. This comment should be removed as it suggests a minor style change that doesn't meaningfully improve the code's clarity or correctness.
2. src/rules/extensions.ts:321
- Draft comment:
Consider refactoring the duplicate fix logic into a helper function to reduce redundancy and ease future maintenance. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. src/rules/extensions.ts:171
- Draft comment:
Verify that the minimatch option '{ nocomment: true }' is intended; consider checking the minimatch documentation to ensure correctness. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. src/rules/extensions.ts:36
- Draft comment:
Review the strict schema for pathGroupOverrides (with additionalProperties: false); ensure that this restriction is intentional for future extensibility. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_GPqJm5q3tJgq7bZj
You can customize 
| const extensionRequired = isUseOfExtensionRequired( | ||
| extension, | ||
| isPackage, | ||
| !overrideAction && isPackage, |
There was a problem hiding this comment.
Pass an explicit value for the isPackage parameter instead of using !overrideAction && isPackage when calling isUseOfExtensionRequired for improved readability.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/rules/extensions.ts (1)
26-53: 🛠️ Refactor suggestion
fixoption is implemented but not declared in the JSON-schemaThe runtime code already consumes
props.fix, yet the option is not listed in the schema insideproperties.
Without the schema entry, tooling (e.g. editors &eslint --print-config) will not offer autocompletion/validation and some CI pipelines that treat unknown properties as errors will fail.checkTypeImports: { type: 'boolean' as const, }, + fix: { + type: 'boolean' as const, + },
♻️ Duplicate comments (1)
src/rules/extensions.ts (1)
169-175: Security reminder: minimatch patterns can be expensiveThe earlier review already highlighted the need to validate/limit
patternOptions(and potentially the pattern length) to mitigate ReDoS vectors inminimatch.
Re-evaluate whether additional safeguards (e.g. timeouts, max pattern length) are required here.
🧹 Nitpick comments (2)
src/rules/extensions.ts (2)
144-151:pathGroupOverridesfrom multiple config objects silently overwrite each other
buildPropertiesassigns the last-seen overrides array and discards earlier ones, which is surprising if a user splits configuration across several option objects.Consider concatenating instead:
- result.pathGroupOverrides = obj.pathGroupOverrides + result.pathGroupOverrides = [ + ...result.pathGroupOverrides, + ...obj.pathGroupOverrides, + ]
321-330: Autofix changes quote style
JSON.stringifyalways produces double-quoted strings, which may alter the original quote style (',", or`) and trigger other style-rules.
You can preserve the existing delimiter viasource.raw[0]orcontext.getSourceCode().getText(source).A minimal fix:
- JSON.stringify(`${importPathWithQueryString}.${extension}`), + `${source.raw[0]}${importPathWithQueryString}.${extension}${source.raw[0]}`,Apply the same technique in the “unexpected” branch below.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/rules/extensions.ts(12 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
commit: |
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #326 +/- ##
==========================================
- Coverage 96.02% 95.81% -0.22%
==========================================
Files 92 92
Lines 4758 4778 +20
Branches 1789 1803 +14
==========================================
+ Hits 4569 4578 +9
- Misses 188 198 +10
- Partials 1 2 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
@stephenjason89 CI is only broken on Node 20 and please add test cases for it.
| ignorePackages?: boolean | ||
| checkTypeImports?: boolean | ||
| pattern: ModifierByFileExtension | ||
| fix?: boolean |
Got no idea about this. |
JounQin
changed the title
pathGroupOverrides and fix options
Hey, just catching up — I wasn’t able to reply earlier since it was the weekend here in the Philippines. I saw the 403 error you encountered. Just to clarify, it looks like you were already working locally on the branch since your own commits came after mine. So I was a bit surprised the original PR wasn’t kept open and merged — it would’ve helped preserve the original history and attribution. Totally appreciate the improvements you made though. Just sharing this for context and in the spirit of smoother collaboration moving forward. Thanks again! |
I believe that's because your fork is based on https://github.com/import-js/eslint-plugin-import instead of this repo, so I got no permission to push changes to your fork. |
|
Ahh that makes sense now — thanks for explaining! I really hate how the fork network works sometimes 😅 |
|
I guess I was the last known victim of the cursed fork network 😅 |
Actually co-author a common pattern to collaborate for git, example: angular/angular#41349. But I've just added you into the co-authors in the release note: https://github.com/un-ts/eslint-plugin-import-x/releases/tag/v4.12.0, hope it will make you feel better. |
3 participants
This PR introduces support for two additional options in the extensions rule:
• fix – enables auto-fixing of extension usage based on the configured pattern.
• pathGroupOverrides – allows overriding extension behavior for specific path groups.
close #318
Important
Adds
fixandpathGroupOverridesoptions toextensionsrule for auto-fixing and path-specific behavior overrides.fixoption toextensionsrule inextensions.tsfor auto-fixing extension usage.pathGroupOverridesoption to override extension behavior for specific paths.moduleVisitorto respectpathGroupOverridesand applyfixlogic.propertiesschema to includefixandpathGroupOverrides.NormalizedOptionsand related interfaces to support new options.computeOverrideAction()to determine action based onpathGroupOverrides.buildProperties()to handle new options.This description was created by
for c71d953. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements