feat(extensions): support pathGroupOverrides and fix options
#327
Conversation
🦋 Changeset detectedLatest commit: 1474a62 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
WalkthroughThis update enhances the Changes
Sequence Diagram(s)sequenceDiagram
participant ESLint
participant ExtensionsRule
participant Utils(parsePath)
participant UserConfig
ESLint->>ExtensionsRule: Analyze import statement
ExtensionsRule->>UserConfig: Retrieve fix and pathGroupOverrides options
ExtensionsRule->>Utils(parsePath): Parse import path
ExtensionsRule->>ExtensionsRule: Compute override action
alt Action is 'enforce'
ExtensionsRule->>ExtensionsRule: Check for extension issues
alt Issue found and fix enabled
ExtensionsRule->>ESLint: Apply autofix
else Issue found and fix disabled
ExtensionsRule->>ESLint: Suggest fix
end
else Action is 'ignore'
ExtensionsRule->>ESLint: Skip enforcement
end
Assessment against linked issues
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/utils/parse-path.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)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (21)
✨ 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 (
|
WalkthroughThis pull request introduces support for Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
|
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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #327 +/- ##
==========================================
+ Coverage 96.02% 96.04% +0.01%
==========================================
Files 92 93 +1
Lines 4758 4802 +44
Branches 1789 1818 +29
==========================================
+ Hits 4569 4612 +43
- Misses 188 189 +1
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
commit: |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![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 dc718b1ce19e997b21d36ad52dd7fc2b8b9c846c in 1 minute and 43 seconds. Click for details.
- Reviewed
725lines of code in9files - Skipped
0files when reviewing. - Skipped posting
1draft 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:164
- Draft comment:
Non-intuitive behavior in buildProperties: if the defaultConfig is 'ignorePackages', it is converted to 'always' and ignorePackages is set to true. This conversion might be confusing—please ensure that the rationale is clearly documented or consider a more explicit handling of this scenario. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_vWfHGOQNP2wckNMD
You can customize 
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Co-authored-by: "Xunnamius (Romulus)" <[email protected]>
JounQin
force-pushed
the
feat/extensions-fix-and-overrides
branch
from
dc718b1 to
ffe866c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (6)
test/utils/parse-path.spec.ts (1)
1-20: Well-structured tests for new path parsing utilitiesThe test suite effectively validates both the parsing and stringifying functions with a good variety of path formats, including paths with query parameters and hash fragments. This ensures the utilities will correctly handle complex import paths when fixing extension issues.
A few additional test cases that might be worth considering:
- Paths with file extensions (e.g., 'foo.js', 'foo.js?query')
- Paths with multiple dots (e.g., 'foo.bar.js')
- Paths with special characters or URL-encoded components
const cases = [ 'foo', 'foo?query', 'foo#hash', 'foo?query#hash', 'foo#hash?query', + 'foo.js', + 'foo.js?query', + 'foo.bar.js', + 'module/path.with.dots.js', + 'path/with%20space.js', ]src/utils/parse-path.ts (2)
7-22: Add explicit return type for stronger type-safety
parsePathcurrently relies on type inference. ReturningParsedPathexplicitly prevents accidental shape changes and makes the intent clearer.-export const parsePath = (path: string) => { +export const parsePath = (path: string): ParsedPath => {
24-25: Also typestringifyPath’s return valueFor symmetry with
parsePathand to avoid widening toany, declare thestringreturn type.-export const stringifyPath = ({ pathname, query, hash }: ParsedPath) => - pathname + query + hash +export const stringifyPath = ( + { pathname, query, hash }: ParsedPath, +): string => pathname + query + hashsrc/rules/extensions.ts (3)
155-162: Merging overrides arrays instead of overwriting
result.pathGroupOverridesis replaced by every matching option object,
so only the last array survives. Concatenating preserves all user
configuration.- if (Array.isArray(obj.pathGroupOverrides)) { - result.pathGroupOverrides = obj.pathGroupOverrides - } + if (Array.isArray(obj.pathGroupOverrides)) { + result.pathGroupOverrides.push(...obj.pathGroupOverrides) + }
180-188: Default minimatch options are dropped when user options existPassing
patternOptions || { nocomment: true }omits the safe default
when the user supplies their own options. Consider shallow-merging
instead ({ nocomment: true, ...patternOptions }).
355-360: Autofix forces double-quotes
JSON.stringifyalways produces double-quoted strings and may clash
with the project’s preferred quote style. Using
context.sourceCode.getText(source)or preserving the original quotes
would avoid noisy diffs.
🛑 Comments failed to post (2)
src/rules/extensions.ts (2)
76-80:
⚠️ Potential issue
patternOptionsis mis-typed
minimatchexpects a singleMinimatchOptionsobject, not a map.
UsingRecord<string, MinimatchOptions>both confuses intent and breaks
autocomplete.-export interface PathGroupOverride { - pattern: string - patternOptions?: Record<string, MinimatchOptions> - action: 'enforce' | 'ignore' -} +export interface PathGroupOverride { + pattern: string + /** options forwarded to minimatch() */ + patternOptions?: MinimatchOptions + action: 'enforce' | 'ignore' +}📝 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.export interface PathGroupOverride { pattern: string /** options forwarded to minimatch() */ patternOptions?: MinimatchOptions action: 'enforce' | 'ignore' }🤖 Prompt for AI Agents
In src/rules/extensions.ts around lines 76 to 80, the patternOptions property in the PathGroupOverride interface is incorrectly typed as a Record<string, MinimatchOptions> when it should be a single MinimatchOptions object. Change the type of patternOptions from Record<string, MinimatchOptions> to MinimatchOptions to match the expected usage by minimatch and improve clarity and autocomplete support.
131-136: 🛠️ Refactor suggestion
Old-structure detector ignores new properties
An options object that contains only
fixorpathGroupOverrides
is wrongly classified as a “pattern map”, silently discarding those
fields.- if ( - (!('pattern' in obj) || obj.pattern == null) && - obj.ignorePackages == null && - obj.checkTypeImports == null - ) { + if ( + (!('pattern' in obj) || obj.pattern == null) && + obj.ignorePackages == null && + obj.checkTypeImports == null && + obj.fix == null && + obj.pathGroupOverrides == null + ) {📝 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.// If this is not the new structure, transfer all props to result.pattern if ( (!('pattern' in obj) || obj.pattern == null) && obj.ignorePackages == null && obj.checkTypeImports == null && obj.fix == null && obj.pathGroupOverrides == null ) {🤖 Prompt for AI Agents
In src/rules/extensions.ts around lines 131 to 136, the condition detecting the old structure incorrectly classifies objects containing only the new properties 'fix' or 'pathGroupOverrides' as a pattern map, causing those fields to be discarded. Update the condition to explicitly check for the presence of these new properties and ensure that objects containing only 'fix' or 'pathGroupOverrides' are not treated as old structure, preserving those fields correctly.
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed ffe866c in 2 minutes and 2 seconds. Click for details.
- Reviewed
726lines of code in9files - Skipped
0files when reviewing. - Skipped posting
5draft 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:164
- Draft comment:
The block that converts a default config of 'ignorePackages' into 'always' and sets ignorePackages to true may not be immediately obvious. Consider adding a comment (or ensure documentation) to clarify this behavior. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. src/utils/parse-path.ts:7
- Draft comment:
The parsePath function correctly parses the pathname, query, and hash parts even for nonstandard orders (e.g. 'foo#hash?query'). Consider documenting that it preserves the original segment order if it doesn’t follow standard URL format. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The code is straightforward URL parsing that splits on standard delimiters. The comment seems to be making assumptions about edge cases that aren't clearly demonstrated in the code. The behavior for non-standard formats isn't explicitly handled - it just uses basic string operations. The code might actually have special handling for non-standard formats that I'm missing. There could be important edge cases that make this documentation valuable. The code is simple string parsing using standard delimiters. Any "preservation of order" is just a natural result of the basic string operations, not a special feature worth documenting. The comment should be deleted as it suggests documenting behavior that isn't a special feature of the code, just an implementation detail of basic string parsing.
3. src/rules/extensions.ts:367
- Draft comment:
Good use of the suggestion mechanism: when the 'fix' option isn’t enabled, providing a suggestion (with messageId 'addMissing') ensures editors can guide the developer to apply the appropriate fix. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining the use of a suggestion mechanism without providing any actionable feedback or code suggestions. It doesn't align with the rules for good comments, which should provide specific suggestions or ask for clarifications on potential issues.
4. test/rules/extensions.spec.ts:20
- Draft comment:
The test suite for the 'extensions' rule is very comprehensive, covering various scenarios including type imports, query strings, and both auto-fix and suggestion cases. Great job ensuring wide coverage. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. test/rules/extensions.spec.ts:975
- Draft comment:
Typo/grammar suggestion: The comment reads "// pathGroupOverrides: an enforce pattern matches bad bespoke specifiers". Consider rephrasing it (e.g., "// pathGroupOverrides: a pattern that enforces correct matching for bespoke specifiers") 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% This is a purely stylistic suggestion about the wording of a test description comment. It doesn't affect functionality or code quality. The original comment is clear enough to understand the test case's purpose. The suggested rewording doesn't add significant clarity or value. The suggested wording might be slightly more grammatically correct. Test descriptions are important for maintainability. While test descriptions are important, the original wording is sufficiently clear and the change is too minor to warrant a comment. This falls under "do not make purely informative comments" and "do not make comments that are obvious or unimportant". Delete this comment as it's a minor stylistic suggestion that doesn't meaningfully improve code quality or clarity.
Workflow ID: wflow_f4nxuk1IZaBCMEkx
You can customize
| ) | ||
| const fixedImportPath = stringifyPath({ | ||
| pathname: `${ | ||
| /([\\/]|[\\/]?\.?\.)$/.test(pathname) |
There was a problem hiding this comment.
Consider adding an inline comment explaining the regex /([\/]|[\/]?\.?\\.)$/ used here to decide whether to append /index.<extension> (for directory-like paths) versus just .<extension>. This would improve code clarity.
There was a problem hiding this comment.
@stephenjason89 You also need to change your fix implementation for import-js/eslint-plugin-import#3177.
| @@ -286,6 +395,14 @@ export default createRule<Options, MessageId>({ | |||
| extension, | |||
| importPath: importPathWithQueryString, | |||
| }, | |||
| ...(props.fix && { | |||
There was a problem hiding this comment.
Let's also add suggestions here in another PR.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/rules/extensions.ts (1)
180-189: Properly implemented pattern matching with minimatch.The
computeOverrideActionfunction correctly uses the minimatch library to determine if an import path matches a pattern and returns the corresponding action.The
minimatchfunction is used here without specifying thenocommentoption explicitly. While the default istrue, it would be safer to explicitly set it to avoid unexpected behavior if the library defaults change in the future.
📜 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 ignored due to path filters (1)
test/utils/__snapshots__/parse-path.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
.github/workflows/ci.yml(1 hunks)README.md(1 hunks)docs/rules/extensions.md(1 hunks)src/rules/extensions.ts(12 hunks)src/utils/index.ts(1 hunks)src/utils/parse-path.ts(1 hunks)test/rules/extensions.spec.ts(6 hunks)test/utils/parse-path.spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/rules/extensions.md
- test/utils/parse-path.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/ci.yml
- src/utils/index.ts
- README.md
- src/utils/parse-path.ts
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/rules/extensions.ts
[warning] 370-370: src/rules/extensions.ts#L370
Added line #L370 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (200)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on macos-latest
🔇 Additional comments (7)
test/rules/extensions.spec.ts (4)
164-172: Good addition of test cases for the newfixoption.These test cases correctly verify that the rule accepts imports both with and without extensions when the
fixoption is enabled.
196-206: Well-structured test enhancement for suggestions.The addition of suggestion information to the test cases is well-implemented, providing clear expectations for the error message, data, and output format when suggestions are enabled.
875-952: Comprehensive test coverage for pathGroupOverrides.These tests thoroughly validate the
pathGroupOverridesfeature with different scenarios:
- When no patterns match imports
- When an enforce pattern matches imports
- When an ignore pattern matches imports
The test cases cover all key behaviors of the new feature.
974-1026: Good coverage of error cases and fix behavior.These invalid test cases properly verify both the error reporting for pattern matching and the automatic fix behavior for removing extensions.
src/rules/extensions.ts (3)
42-60: Well-structured schema definition for new options.The schema definitions for
pathGroupOverridesandfixoptions are properly structured and typed. The pathGroupOverrides array with pattern properties and actions will allow for flexible rule configuration.
339-352: Well-implemented path handling for auto-fixes.The code correctly parses import paths and handles special cases like directory imports by appending
/index.${extension}. The previously identified issue with trailing slashes has been fixed.
398-405: Good implementation of automatic fixing for unexpected extensions.The code correctly implements automatic removal of unexpected extensions when the
fixoption is enabled.
JounQin
changed the title
pathGroupOverrides and fix options
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 1474a62 in 42 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft 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/utils/parse-path.ts:7
- Draft comment:
Explicit return type enhances clarity for parsePath's output. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. src/utils/parse-path.ts:10
- Draft comment:
Logic for extracting query and hash is clear; consider adding inline comments to document behavior when both '?' and '#' exist. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_BhoH2nsGDmBT5hed
You can customize
|
| hash: string | ||
| } | ||
|
|
||
| export const parsePath = (path: string): ParsedPath => { |
There was a problem hiding this comment.
@stephenjason89 You'd better to use this pattern to support ?/# better in
original importPath for import-js/eslint-plugin-import#3177
|
Hey, I appreciate that the changes were used and improved with tests — thank you for that. That said, I would’ve really preferred if my original PR was merged and any follow-up changes were pushed to my branch or discussed first. It would’ve felt more collaborative and helped retain proper credit for the initial effort. I hope future contributions can follow that approach. Thanks again. |
|
@stephenjason89 See #326 (comment) I got no permission to push changes to your branch, and I've already added you add commit co-author: 90c1cd0 |
|
@JounQin Thanks for the clarification, and for adding me as a co-author — I really appreciate that! Next time I’ll make sure to allow edits on the branch so things can move more smoothly. Looking forward to contributing more 🙌 |
|
close #318
close #326
Important
Enhances
extensionsrule withpathGroupOverridesandfixoptions, updates documentation, and expands tests.pathGroupOverridesandfixoptions toextensionsrule inextensions.ts.parsePathandstringifyPathfunctions inparse-path.tsfor handling path strings.extensions.mdandREADME.mdto document new rule options and fixability.extensions.spec.tsfor new options and TypeScript scenarios.parse-path.spec.tsto test new utility functions.This description was created by
for 1474a62. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Documentation
Tests