fix(extensions): should only report on fixable paths #395
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 1ad9595 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 |
JounQin
changed the title
WalkthroughThis change refines the logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant ESLint
participant ExtensionsRule
participant PathResolver
ESLint->>ExtensionsRule: Analyze import statement
ExtensionsRule->>PathResolver: Resolve original import path
ExtensionsRule->>PathResolver: Construct and resolve fixed path
PathResolver-->>ExtensionsRule: Return resolved paths
ExtensionsRule->>ExtensionsRule: Compare resolved paths
alt Paths align and fix is valid
ExtensionsRule->>ESLint: Report and suggest fix
else Paths differ or fix is invalid
ExtensionsRule->>ESLint: Skip reporting
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(No out-of-scope changes found.) Possibly related PRs
Suggested labels
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.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js test/rules/extensions.spec.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js ✨ 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 (
|
|
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. |
Signed-off-by: JounQin <admin@1stg.me>
There was a problem hiding this comment.
Pull Request Overview
This PR refines the extensions rule so that it only reports fixes when the computed import path actually resolves to the same file, and it adds a new valid test case for external imports.
- Introduce resolution guards in both “missing extension” and “unexpected extension” branches to skip fixes when the resolved path differs.
- Simplify construction of the fix path by extracting
pathnameand reusing it instringifyPath. - Add a
tValidspec for an external import (import eslint from 'eslint';).
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/rules/extensions.spec.ts | Added a valid test case for external module imports. |
| src/rules/extensions.ts | Inserted guards preventing reports when computed fixes aren’t applicable and refactored fix path construction. |
Comments suppressed due to low confidence (1)
test/rules/extensions.spec.ts:175
- [nitpick] Consider adding tests for cases where import paths match the extension rule pattern but resolve to different files, to verify the new guard correctly prevents inappropriate fixes.
code: "import eslint from 'eslint';",
commit: |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 5d7ddfc in 1 minute and 29 seconds. Click for details.
- Reviewed
97lines of code in2files - 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:358
- Draft comment:
New variables 'pathname' and 'fixedImportPath' are computed with a conditional check. Please add a brief comment explaining that this computation ensures the fix is only applied when the resolved file matches the expected result, avoiding false positives on external or non‑fixable paths. - 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 comment is requesting purely informative documentation. While the code is somewhat complex, adding documentation is not a required code change - the code works correctly as is. The comment is not pointing out a bug or suggesting a concrete improvement to the code structure. The code is complex and documentation could help future maintainers understand the logic better. The variables are used in a non-obvious way to validate path resolution. While documentation could be helpful, our rules explicitly state not to make purely informative comments. The comment is not suggesting a concrete code change. Delete this comment since it only requests additional documentation without suggesting any concrete code changes.
2. src/rules/extensions.ts:372
- Draft comment:
The added condition 'if (importPath === fixedImportPath || resolvedPath !== resolve(pathname, context)) { return }' effectively prevents reporting errors when the suggested fix wouldn’t resolve correctly. A comment here describing its purpose would aid future maintainers. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is suggesting adding a comment in the code to describe the purpose of a condition. This is a request for documentation, which is not allowed according to the rules. The comment does not provide a specific code suggestion or ask for a test to be written, so it should be removed.
3. src/rules/extensions.ts:420
- Draft comment:
In the branch handling forbidden extensions, extracting 'pathname' from a possible '/index' suffix and verifying with 'if (resolvedPath !== resolve(pathname, context))' is a good safeguard. Consider adding inline documentation to clarify that this check avoids modifying external or unfixable imports. - 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 comment is suggesting purely informative documentation changes, not actual code changes. While the logic being described is complex, adding documentation is not a critical code change. The rules explicitly state not to make purely informative comments. The code is complex and documentation could help future maintainers understand the purpose of these checks. Maybe this would prevent future bugs. While documentation could be helpful, the rules are clear that we should only keep comments that require actual code changes. Documentation suggestions should be handled separately from code review. Delete this comment since it only suggests adding documentation and doesn't require any actual code changes.
4. test/rules/extensions.spec.ts:170
- Draft comment:
A valid test case for external dependencies ('import eslint from "eslint"') has been added. This confirms the fix for issue #394. The test clearly covers that external imports are not reported. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_Y1jmWzwlMYVpNEUK
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 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #395 +/- ##
==========================================
- Coverage 96.09% 96.06% -0.04%
==========================================
Files 96 96
Lines 4944 4952 +8
Branches 1854 1859 +5
==========================================
+ Hits 4751 4757 +6
- Misses 192 194 +2
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/rules/extensions.ts (1)
369-370: Simplify by removing redundant checkThe
extension &&check is redundant here sinceextensionRequiredalready guarantees thatextensionis truthy.- const fixedImportPath = - extension && stringifyPath({ pathname, query, hash }) + const fixedImportPath = stringifyPath({ pathname, query, hash })
🧹 Nitpick comments (2)
src/rules/extensions.ts (2)
427-429: Consider test coverage for this validation branch as wellSimilar to the earlier comment, line 428's early return is not covered by tests. Adding test coverage for edge cases where extension removal validation fails would improve confidence in the fix.
372-377: Consider adding test coverage for validation branchesThe early return on line 376 (when validation fails) is not covered by tests according to static analysis. Consider adding test cases for scenarios where the resolved path doesn't match the expected resolution.
#!/bin/bash # Description: Check for existing tests that might cover path resolution mismatches # Search for test cases that might trigger the validation failure branches rg -A 5 -B 5 "resolve.*context|resolvedPath" test/rules/extensions.spec.ts # Look for mock resolution setups in tests ast-grep --pattern 'resolve($_, $_)'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/dull-wombats-boil.md(1 hunks)src/rules/extensions.ts(5 hunks)test/rules/extensions.spec.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/rules/extensions.ts
[warning] 376-376: src/rules/extensions.ts#L376
Added line #L376 was not covered by tests
[warning] 428-428: src/rules/extensions.ts#L428
Added line #L428 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (198)
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 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 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 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 20 and ESLint 8 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 22 and ESLint 8.56 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 24 and ESLint 8 on macos-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 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 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 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 20 and ESLint 8 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 22 and ESLint 8.56 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 24 and ESLint 8 on macos-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 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 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 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 20 and ESLint 8 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 22 and ESLint 8.56 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 24 and ESLint 8 on macos-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 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 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 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 20 and ESLint 8 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 22 and ESLint 8.56 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 24 and ESLint 8 on macos-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 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 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 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 20 and ESLint 8 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 22 and ESLint 8.56 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 24 and ESLint 8 on macos-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 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 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 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 20 and ESLint 8 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 22 and ESLint 8.56 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 24 and ESLint 8 on macos-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 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 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 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 20 and ESLint 8 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 22 and ESLint 8.56 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 24 and ESLint 8 on macos-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 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 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 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 20 and ESLint 8 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 22 and ESLint 8.56 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 24 and ESLint 8 on macos-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 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 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 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 20 and ESLint 8 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 22 and ESLint 8.56 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 24 and ESLint 8 on macos-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 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 22 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 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 20 and ESLint 8 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 22 and ESLint 8.56 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 24 and ESLint 8 on macos-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
🔇 Additional comments (4)
.changeset/dull-wombats-boil.md (1)
1-6: LGTM!The changeset properly documents the patch update with a clear description of the fix.
test/rules/extensions.spec.ts (1)
174-177: Good test coverage for external packages!This test case ensures that external packages like 'eslint' are not incorrectly flagged when the fix option is enabled, which aligns with the PR's goal of preventing false positives for external dependencies.
src/rules/extensions.ts (2)
358-377: Excellent fix for path validation!The addition of path resolution validation before reporting prevents false positives by ensuring the suggested fix would actually resolve to a valid file. This is a crucial improvement for external dependencies and edge cases.
420-430: Consistent validation approach for removing extensionsGood job applying the same validation pattern for the extension removal case. This ensures symmetry in the rule's behavior.
|
Working as expected, |
This was referenced Jun 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
close #394
Important
Ensure only fixable paths are reported in
extensions.tsby adding checks and updating tests accordingly.extensions.ts, added checks to ensure only fixable paths are reported by comparingimportPathwithfixedImportPathandresolvedPathwithresolve(pathname, context).extensions.spec.tsforimport eslint from 'eslint';withfix: trueandpattern: { js: 'always' }.This description was created by
for 5d7ddfc. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Bug Fixes
Tests
Chores