-
-
Notifications
You must be signed in to change notification settings - Fork 53
feat: add suggestions support for extensions
unexpected
case
#329
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
Conversation
🦋 Changeset detectedLatest commit: 813a4a0 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 |
WalkthroughA patch update for the Changes
Sequence Diagram(s)sequenceDiagram
participant Linter as ESLint
participant Rule as extensions Rule
participant Fixer as Fix Logic
Linter->>Rule: Analyze import statement
Rule->>Rule: Parse import path (with query/hash)
alt Unexpected extension found
Rule->>Fixer: Prepare fix suggestions
Fixer->>Rule: Suggest remove extension\nand/or /index
Rule->>Linter: Report error with suggestions
else No issue
Rule->>Linter: No report
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 test/rules/extensions.spec.tsOops! Something went wrong! :( ESLint: 9.27.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js test/utils/parse-path.spec.tsOops! Something went wrong! :( ESLint: 9.27.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js 📜 Recent review detailsConfiguration used: CodeRabbit UI 📥 CommitsReviewing files that changed from the base of the PR and between 868532792258e96844e6ece42f1570d6f2645306 and 813a4a0. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (180)
✨ 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 suggestions in the Changes
|
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. |
commit: |
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 PR adds suggestion support for the unexpected
case in the extensions
ESLint rule and refines parse-path test snapshots.
- Introduces a new
removeUnexpected
messageId and suggestion/fix logic in the rule implementation. - Adds
suggestions
arrays inextensions.spec.ts
to cover removal of unexpected extensions across many scenarios. - Updates parse-path tests to name snapshots by input and adjusts snapshot keys.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
test/utils/parse-path.spec.ts | Passes input as the snapshot name in toMatchSnapshot . |
test/utils/snapshots/parse-path.spec.ts.snap | Renames snapshot keys to include the input path. |
test/rules/extensions.spec.ts | Adds suggestions blocks for removeUnexpected in multiple test cases. |
src/rules/extensions.ts | Defines removeUnexpected , adds fixSource helper, and wires up suggestion/fix logic. |
Comments suppressed due to low confidence (1)
src/rules/extensions.ts:251
- [nitpick] The message template uses 'into' which may be confusing; consider rewording to something like 'Remove unexpected "{{extension}}" extension from "{{importPath}}"; use "{{fixedImportPath}}"' for clarity.
removeUnexpected:
'Remove unexpected "{{extension}}" file extension from "{{importPath}}" into "{{fixedImportPath}}"',
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.
Important
Looks good to me! 👍
Reviewed everything up to 46585a6c52b72bab67a1b3fb3f2c9325f5606bdc in 2 minutes and 13 seconds. Click for details.
- Reviewed
720
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
8
draft 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:108
- Draft comment:
New messageId 'removeUnexpected' was added. Ensure the documentation and configuration examples are updated to reflect its intended usage. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that documentation and configuration examples are updated, which falls under the rule of not asking the author to ensure something is done. This is not a specific code suggestion or a request for a test to be written, so it should be removed.
2. src/rules/extensions.ts:196
- Draft comment:
The fixSource function uses a simple regex to replace the string literal. Consider if escaped quotes might be present and whether a more robust approach is warranted. - 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 regex could potentially fail if the import path contains escaped quotes. However, looking at the usage in the code, this function is only used to fix import paths. Import paths in JavaScript/TypeScript modules don't typically contain escaped quotes - they are simple string literals. The current approach seems sufficient for the actual use case. I could be wrong about import paths - there may be edge cases I'm not considering where escaped quotes are valid/needed in import paths. Even if there are edge cases, they would be extremely rare in real code. The current solution handles the 99.9% case correctly. While technically correct, this comment points out a theoretical issue that is extremely unlikely to occur in practice for this specific use case of handling import paths.
3. src/rules/extensions.ts:259
- Draft comment:
getModifier now accepts a string instead of a FileExtension. Verify that all configuration keys are provided as strings and that this change doesn’t inadvertently allow invalid values. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. src/rules/extensions.ts:403
- Draft comment:
In the branch handling unexpected extensions, fixedPathname is computed by slicing off the extension. Double‐check this logic for edge cases (e.g. filenames with multiple dots) to ensure it always produces the correct result. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to double-check the logic for edge cases, which violates the rule against asking for confirmation or double-checking. It does not provide a specific suggestion or point out a clear issue.
5. test/rules/extensions.spec.ts:176
- Draft comment:
The updated tests now include suggestions for 'removeUnexpected'. The test cases are quite comprehensive; consider adding additional edge cases (e.g. more complex filenames or unusual query strings) if not already covered. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
6. test/utils/__snapshots__/parse-path.spec.ts.snap:3
- Draft comment:
The snapshots for parsePath have been updated and now correctly reflect the separation of pathname, query, and hash. This appears correct; ensure that any future changes also update the snapshots. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is asking the author to ensure future changes are reflected in snapshots, which is a form of asking to double-check or ensure behavior. It doesn't provide a specific suggestion or point out a current issue.
7. test/utils/parse-path.spec.ts:14
- Draft comment:
The round‐trip tests for parsePath (parsing and then stringifying) cover the expected cases. This test ensures integrity of the parse/stringify logic. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it only states that the tests cover expected cases and ensure integrity. It does not provide any actionable feedback or suggestions for improvement.
8. src/rules/extensions.ts:318
- Draft comment:
Typographical: The comment has an extraneous closing parenthesis. Consider removing the trailing ')' to improve clarity. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_0IgAzLYOE507VYSF
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #329 +/- ##
==========================================
+ Coverage 96.04% 96.07% +0.03%
==========================================
Files 93 93
Lines 4802 4816 +14
Branches 1818 1800 -18
==========================================
+ Hits 4612 4627 +15
+ Misses 189 188 -1
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 |
5898310
to
8685327
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.
Important
Looks good to me! 👍
Reviewed 868532792258e96844e6ece42f1570d6f2645306 in 1 minute and 54 seconds. Click for details.
- Reviewed
731
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
7
draft 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:8
- Draft comment:
Removed the FileExtension type import – now extensions are treated as strings. Ensure all code using these values is updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to ensure that all code using theFileExtension
type is updated to handle strings instead. This is a request for confirmation and testing, which violates the rules.
2. src/rules/extensions.ts:196
- Draft comment:
New helper function fixSource preserves the original quoting style of the import literal. Consider verifying its behavior for edge cases (e.g. non-standard quotes or template literals). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment suggests verifying the behavior of a function for edge cases, which falls under asking the PR author to ensure the behavior is intended or tested. This violates the rules.
3. src/rules/extensions.ts:368
- Draft comment:
Using fixSource(source.raw, fixedImportPath) instead of JSON.stringify better preserves the quote style. Ensure that source.raw is always a valid string for all node types handled by the rule. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%
<= threshold50%
The comment suggests a specific code change to usefixSource(source.raw, fixedImportPath)
instead ofJSON.stringify
to preserve quote style, which is a valid suggestion. However, the second part of the comment asks the author to ensure thatsource.raw
is always a valid string, which violates the rule against asking the author to ensure behavior. The first part of the comment is useful, but the second part is not.
4. src/rules/extensions.ts:403
- Draft comment:
The computation of fixedPathname via slicing (importPath.slice(0, -(extension.length + 1))) assumes that query and hash components have been handled separately. Double-check that this slicing works correctly in all edge cases. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to double-check the slicing logic for edge cases, which violates the rule against asking the author to double-check things. It does not provide a specific suggestion or point out a clear issue with the code.
5. src/rules/extensions.ts:420
- Draft comment:
The new removal branch provides two suggestions (one for index case and one for regular case) via the removeUnexpected message. This comprehensive suggestion setup is good—ensure documentation reflects these distinct fixes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that documentation reflects certain changes. This falls under the rule of not asking the author to ensure something is done, which is not allowed.
6. test/rules/extensions.spec.ts:176
- Draft comment:
Test cases now include detailed suggestions for the 'removeUnexpected' error. The variety of expected outputs (e.g. 'a' vs 'a/index') confirms the new behavior is well‐covered. - 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.
7. src/rules/extensions.ts:251
- Draft comment:
In the newly added suggestion message forremoveUnexpected
, the use of the preposition "into" seems off. Consider revising it—using "to" or rephrasing the sentence might improve 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% While the suggestion is technically correct - "into" is a bit awkward here and "to" would be better - this is a very minor grammatical issue in an error message. The meaning is still clear either way. According to our rules, we should not make purely informative comments or comments about unimportant issues. Error message wording falls into this category unless it's actively confusing. The error message wording could impact developer experience and clarity of the tool's output. Poor grammar might make the tool seem less professional. While professional polish is good, this grammatical issue is extremely minor and doesn't impact understanding. The meaning is perfectly clear either way. Delete this comment. The grammatical suggestion, while technically correct, is too minor to warrant a PR comment according to our rules about not making purely informative comments about unimportant issues.
Workflow ID: wflow_o5VRxGt2lHEaugEr
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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: 2
♻️ Duplicate comments (1)
test/rules/extensions.spec.ts (1)
184-203
: High duplication in suggestion blocks – consider helper factoryThe added suggestion arrays are nearly identical across many cases (only the three
strings differ). A small helper (e.g.generateRemoveUnexpected('js', 'a/index.js', ['a', 'a/index'])
)
would collapse dozens of boilerplate lines and make future updates easier.
🧹 Nitpick comments (3)
src/rules/extensions.ts (1)
196-201
: Regex infixSource
still brittle – consider anchoring & using the 2ᵈ capture groupThe current pattern
/^(['"])(.+)\1$/
- Uses a greedy
.+
, which will happily swallow whitespace or trailing comma characters if they ever appear (e.g. in dynamic-import arrays).- You discard the second capture (the old path) anyway, so it’s clearer/safer to let the regex itself assert anything between the quotes without relying on greediness.
-return source.replace( - /^(['"])(.+)\1$/, - (_, quote: string) => `${quote}${importPath}${quote}`, -) +return source.replace( + /^(['"])[\s\S]*\1$/, + (_, quote: string) => `${quote}${importPath}${quote}`, +)This keeps the replacement logic identical while eliminating the greediness concern.
test/utils/parse-path.spec.ts (1)
5-11
: Consider adding test cases with file extensionsSince this PR relates to handling unexpected file extensions in import paths, it would be beneficial to include test cases that cover paths with various extensions (e.g., 'foo.js', 'foo.ts', 'foo.jsx') to ensure the parsing logic correctly handles the scenarios targeted by this feature.
const cases = [ 'foo', 'foo?query', 'foo#hash', 'foo?query#hash', 'foo#hash?query', + 'foo.js', + 'foo.ts?query', + 'foo.jsx#hash', + 'index.js', + './relative/path.ts', ].changeset/weak-points-pull.md (1)
5-5
: Summary cleanup: remove redundantfeat:
prefix
Changeset descriptions should be concise plain text. Thefeat:
prefix is unnecessary here. Consider updating to:Add suggestions support for `extensions` `unexpected` case
📜 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
📥 Commits
Reviewing files that changed from the base of the PR and between b80490e and 868532792258e96844e6ece42f1570d6f2645306.
⛔ Files ignored due to path filters (1)
test/utils/__snapshots__/parse-path.spec.ts.snap
is excluded by!**/*.snap
📒 Files selected for processing (4)
.changeset/weak-points-pull.md
(1 hunks)src/rules/extensions.ts
(9 hunks)test/rules/extensions.spec.ts
(19 hunks)test/utils/parse-path.spec.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-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 ubuntu-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.56 on ubuntu-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 9 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 18 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on ubuntu-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 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-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 24 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on ubuntu-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 24 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
🔇 Additional comments (1)
test/utils/parse-path.spec.ts (1)
15-15
: Great enhancement to snapshot testing!Adding the input string as the snapshot name improves test clarity and makes it easier to identify which test case is associated with each snapshot. This is especially helpful when reviewing snapshot diffs during test failures or updates.
8685327
to
5f6afba
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.
Important
Looks good to me! 👍
Reviewed 5f6afba4adf0548135a224e14992675303409cea in 1 minute and 33 seconds. Click for details.
- Reviewed
738
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
10
draft 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. .changeset/weak-points-pull.md:1
- Draft comment:
The changeset entry correctly documents the new 'removeUnexpected' suggestion support and version bump. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it only states that the changeset entry correctly documents a new feature and version bump. It does not provide any actionable feedback or suggestions for improvement.
2. src/rules/extensions.ts:1
- Draft comment:
Removed the unused FileExtension type from imports; using a string for extension improves flexibility. - 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.
3. src/rules/extensions.ts:108
- Draft comment:
Added 'removeUnexpected' to the MessageId union—this new message id clearly supports fix suggestions for unwanted file extensions. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, explaining what was added and why. It doesn't provide a suggestion, ask for confirmation, or point out a potential issue.
4. src/rules/extensions.ts:203
- Draft comment:
Good extraction of the fixSource helper function to preserve original quote style when replacing the import path. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the extraction of a helper function without offering any specific guidance or questions.
5. src/rules/extensions.ts:318
- Draft comment:
Utilizing parsePath to destructure the pathname, query, and hash improves handling of complex import strings with query/hash components. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it praises the use ofparsePath
without providing any actionable feedback or suggestions for improvement. It doesn't align with the rules for good comments, which should offer specific suggestions or highlight potential issues.
6. src/rules/extensions.ts:410
- Draft comment:
The branch handling 'removeUnexpected' provides alternative fix suggestions (including index file adjustments) and uses filter(Boolean) effectively to conditionally include suggestions. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the code without offering any constructive criticism or specific guidance.
7. test/rules/extensions.spec.ts:177
- Draft comment:
New tests for 'removeUnexpected' suggestion cover various scenarios and output formats, enhancing overall test coverage for auto-fix suggestions. - 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.
8. test/utils/__snapshots__/parse-path.spec.ts.snap:3
- Draft comment:
Snapshot updates for parsePath accurately reflect the expected parsing and stringification behavior of different path variants. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it only states that the snapshot updates reflect expected behavior. It does not provide any actionable feedback or suggestions for improvement.
9. test/utils/parse-path.spec.ts:1
- Draft comment:
The parse-path spec correctly verifies that parsePath and stringifyPath are inverses, ensuring consistent 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.
10. src/rules/extensions.ts:325
- Draft comment:
Typo: The comment includes an extraneous closing parenthesis. Consider changing "Likeimport Decimal from decimal.js
)" to "Likeimport Decimal from decimal.js
" or revising the comment for clarity. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_CJxDxarOZw2YVU2n
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
5f6afba
to
77562ad
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.
Important
Looks good to me! 👍
Reviewed 77562adb2529ebb2a5fc6f2c293046a6ee3b0db0 in 1 minute and 33 seconds. Click for details.
- Reviewed
738
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
8
draft 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. .changeset/weak-points-pull.md:1
- Draft comment:
Changeset entry is clear and concise. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. src/rules/extensions.ts:203
- Draft comment:
Good use of JSDoc in the new replaceImportPath function to preserve the original quote style. Consider verifying if the regex handles escaped quotes or edge cases. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. src/rules/extensions.ts:109
- Draft comment:
The MessageId union now includes 'removeUnexpected'. Ensure related documentation and rule integration are updated accordingly. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. src/rules/extensions.ts:412
- Draft comment:
The branch handling unexpected file extensions (with alternative fixes for index paths) is comprehensive. Verify that all edge cases are covered, especially when dealing with '/index' suffixes. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
5. test/rules/extensions.spec.ts:176
- Draft comment:
New test cases for 'removeUnexpected' suggestions are comprehensive. Ensure that multi-line output strings maintain consistent formatting. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
6. test/utils/__snapshots__/parse-path.spec.ts.snap:1
- Draft comment:
Snapshot keys have been updated to reflect the new test inputs and are clear. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
7. test/utils/parse-path.spec.ts:15
- Draft comment:
The parse-path tests use toMatchSnapshot(input) effectively to ensure full round-trip consistency. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
8. src/rules/extensions.ts:325
- Draft comment:
There appears to be an extra closing parenthesis in the comment (// Like
import Decimal from decimal.js)
). Consider removing it to avoid confusion. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_rxVVa2suG8PHiZub
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
77562ad
to
3758d66
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.
Important
Looks good to me! 👍
Reviewed 3758d668026954966f061f58a788dabef742d603 in 1 minute and 47 seconds. Click for details.
- Reviewed
745
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
10
draft 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. .changeset/weak-points-pull.md:5
- Draft comment:
Changeset file correctly documents the new feature; ensure release notes reference the updated suggestions behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is asking the PR author to ensure that release notes are updated, which is similar to asking them to update the PR description. It doesn't provide a specific code suggestion or point out a potential issue in the code itself.
2. src/rules/extensions.ts:5
- Draft comment:
Removed the unused 'FileExtension' import; now extensions are treated as strings. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, stating that an import was removed and how extensions are now treated. It doesn't provide a suggestion or ask for confirmation on any specific change.
3. src/rules/extensions.ts:112
- Draft comment:
Added new messageId 'removeUnexpected'; ensure documentation and user guides are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure documentation and user guides are updated, which falls under the rule of not asking the author to ensure something is done. It doesn't provide a specific code suggestion or ask for a specific test to be written.
4. src/rules/extensions.ts:203
- Draft comment:
Introduced helper 'replaceImportPath' to replace the import path while preserving the original quote style. Verify that the regex handles all valid quoted strings. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify that the regex handles all valid quoted strings, which is against the rules as it asks for verification. It does not provide a specific suggestion or point out a specific issue with the code.
5. src/rules/extensions.ts:297
- Draft comment:
Switched to using 'parsePath' to extract pathname, query, and hash from the import string. This is a robust improvement over using regex replacement. - 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.
6. src/rules/extensions.ts:374
- Draft comment:
Fixer now uses 'replaceImportPath(source.raw, fixedImportPath)' instead of JSON.stringify, preserving the original quoting style. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, explaining a change in the code without suggesting any action or asking for confirmation. It doesn't provide any actionable feedback or raise any concerns about the code change.
7. src/rules/extensions.ts:388
- Draft comment:
The unexpected branch now builds detailed suggestions using 'removeUnexpected', including an alternate fix when an index file suffix is detected. - 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.
8. test/rules/extensions.spec.ts:184
- Draft comment:
Test cases now include comprehensive 'removeUnexpected' suggestions for invalid import cases; this improves feedback and autofix coverage. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it praises the inclusion of comprehensive test cases without providing any actionable feedback or suggestions for improvement. It doesn't align with the rules for good comments, which should offer specific suggestions or highlight potential issues.
9. test/utils/__snapshots__/parse-path.spec.ts.snap:3
- Draft comment:
Snapshots updated reflect the new parsing of query and hash elements; ensure these outputs match the intended behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that the outputs match the intended behavior, which violates the rule against asking the author to confirm their intention or ensure behavior. It doesn't provide a specific suggestion or point out a specific issue.
10. test/utils/parse-path.spec.ts:15
- Draft comment:
Using 'expect(output).toMatchSnapshot(input)' improves snapshot naming and clarity. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it praises the use of a specific function without suggesting any changes or improvements. It doesn't provide actionable feedback or raise any concerns about the code.
Workflow ID: wflow_21UdnnR1gF5oPY6b
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
3758d66
to
f33d81a
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.
Important
Looks good to me! 👍
Reviewed f33d81ae4765a6db1ac5630adbde9253d5ff5db4 in 1 minute and 50 seconds. Click for details.
- Reviewed
806
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
9
draft 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. .changeset/weak-points-pull.md:5
- Draft comment:
The changeset message is clear and follows conventional commit style. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the clarity and style of the changeset message, which is not necessary for the PR review process.
2. src/rules/extensions.ts:5
- Draft comment:
Removed the unused FileExtension type import – good cleanup. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it only acknowledges the removal of an unused import without providing any actionable feedback or suggestions. It does not align with the rules for useful comments.
3. src/rules/extensions.ts:203
- Draft comment:
The new helper function replaceImportPath uses a regex to rebuild the quoted import path. Ensure that the input (source string) is always a fully quoted literal as assumed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that the input is always a fully quoted literal, which violates the rule against asking the author to ensure behavior is intended. It doesn't provide a specific suggestion or ask for a test to be written.
4. src/rules/extensions.ts:109
- Draft comment:
New MessageId 'removeUnexpected' has been added and is used in suggestions. This enriches the feedback – looks good. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it only states that a newMessageId
has been added and is used in suggestions, without providing any actionable feedback or suggestions for improvement.
5. src/rules/extensions.ts:425
- Draft comment:
In the branch for unexpected extensions, two alternative suggestions are provided when the path ends with '/index'. Verify that offering both alternatives is intentional and that users benefit from choosing between them. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify their intention regarding offering two alternatives. This violates the rule against asking the author to confirm their intention or ensure behavior is intended.
6. test/rules/extensions.spec.ts:175
- Draft comment:
Test cases have been expanded with detailed suggestions for both removal and addition of extensions. This robust coverage is very helpful. - 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.
7. test/utils/__snapshots__/parse-path.spec.ts.snap:3
- Draft comment:
Snapshot updates for parsePath outputs reflect the expected parsing behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, stating that the snapshot updates reflect expected behavior. It doesn't provide any actionable feedback or suggestions for improvement.
8. test/utils/parse-path.spec.ts:15
- Draft comment:
The test for parsePath correctly uses the input as a snapshot identifier and verifies round-trip behavior. Looks good. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the existing test without offering any constructive criticism or specific guidance.
9. src/rules/extensions.ts:325
- Draft comment:
Typo: The comment has an extraneous closing parenthesis at the end of the line. Consider removing it. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_9CCIlAy2C1GtKMVG
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
f33d81a
to
813a4a0
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.
Important
Looks good to me! 👍
Reviewed 813a4a0 in 1 minute and 49 seconds. Click for details.
- Reviewed
840
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
7
draft 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. .changeset/weak-points-pull.md:1
- Draft comment:
Changeset file looks good – the release note clearly indicates the new suggestion support for the 'unexpected' case. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. src/rules/extensions.ts:203
- Draft comment:
The new helper function 'replaceImportPath' correctly replaces the quoted import path. Consider adding a brief note that it assumes the source is a proper string literal with matching quotes. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. src/rules/extensions.ts:410
- Draft comment:
In the removal branch for unexpected extensions, the dual suggestion logic (one removing the '/index' and one preserving it) is clever. Consider adding an inline comment to clarify that when the fixed path ends with '/index', two suggestions are provided: one with the index removed (yielding a shorter path) and one preserving it. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. test/rules/extensions.spec.ts:176
- Draft comment:
Test cases are comprehensive – they cover various scenarios including multiple suggestions, query/hash handling, and package edge cases. Great work! - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
5. test/utils/__snapshots__/parse-path.spec.ts.snap:1
- Draft comment:
Snapshots for parse-path appear correct and reflect the updated behavior with query/hash components. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
6. test/utils/parse-path.spec.ts:1
- Draft comment:
The parse-path test uses each input as a snapshot identifier to ensure uniqueness. This is a clear and effective approach. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
7. src/rules/extensions.ts:325
- Draft comment:
Typographical error: There is an extra closing parenthesis in the comment on line 325. Consider removing the trailing ')' to clean up the comment. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_aildcrcKBW0fLw3o
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
* @param importPath - The new import path to replace the existing one. | ||
* @returns The updated source string with the replaced import path. | ||
*/ | ||
function replaceImportPath(source: string, importPath: string) { |
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.
@stephenjason89 You also need to change your fix
implementation for import-js/eslint-plugin-import#3177. JSON.stringify
is not correct for single quoted imports.
follow #327
Important
Adds suggestions for unexpected file extensions and improves import path handling in
extensions.ts
, with expanded test coverage.extensions.ts
, enhancing feedback and autofix capabilities.extensions.ts
to preserve quote styles and support query/hash components.extensions.spec.ts
with detailed autofix suggestions for invalid imports.parse-path.spec.ts
for better clarity.This description was created by
for 813a4a0. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Tests