refactor: migrate deprecated doctrine to comment-parser
#314
Conversation
🦋 Changeset detectedLatest commit: feb3e00 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 |
|
Warning Rate limit exceeded@JounQin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 8 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (48)
WalkthroughThis update migrates the codebase from using the deprecated Changes
Sequence Diagram(s)sequenceDiagram
participant SourceFile
participant ExportMap
participant CommentParser
SourceFile->>ExportMap: Load and analyze module
ExportMap->>SourceFile: Retrieve comments from AST
ExportMap->>CommentParser: Parse block comments
CommentParser-->>ExportMap: Return parsed Block(s)
ExportMap->>ExportMap: Extract tags (e.g., deprecated, module)
ExportMap-->>SourceFile: Provide documentation metadata with tags
Assessment against linked issues
Possibly related PRs
Poem
✨ 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:
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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the codebase from using the deprecated doctrine library to comment-parser for parsing JSDoc comments and deprecation metadata. Key changes include updating test expectations for deprecation tags, refactoring parsing functions and type annotations in the core utility files, and adjusting dependency imports to use comment-parser.
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/utils/export-map.spec.ts | Updated expectations to check for 'tag' instead of 'title' in deprecation tests |
| test/rules/no-extraneous-dependencies.spec.ts | Changed dependency import from doctrine to comment-parser |
| test.local.js | Added a test snippet demonstrating the usage of comment-parser |
| src/utils/resolve.ts | Refactored filename resolution to use import.meta.url |
| src/utils/export-map.ts | Replaced doctrine parsing with comment-parser and updated associated type annotations |
| src/rules/no-deprecated.ts | Updated deprecation rule to work with comment-parser's Spec type |
| src/meta.ts | Simplified CJS require resolution by relying on import.meta.url |
Files not reviewed (1)
- package.json: Language not supported
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
WalkthroughThis pull request refactors the codebase by migrating from the deprecated Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
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 e25a637 in 1 minute and 38 seconds. Click for details.
- Reviewed
427lines of code in8files - Skipped
1files when reviewing. - Skipped posting
9draft 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/meta.ts:5
- Draft comment:
The fallback check using import.meta.url may be redundant in pure ESM contexts. Verify if the ternary is truly needed or if the ESM environment always guarantees a non-falsy import.meta.url. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to verify if a fallback check is necessary, which violates the rule against asking the author to confirm or verify intentions. It does not provide a specific suggestion or ask for a specific test to be written.
2. src/utils/export-map.ts:71
- Draft comment:
The 'fixup' hack adding the name to the description for tags like 'deprecated' is clever, but consider adding comments to explain why this transformation is necessary for future maintainers. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. src/rules/no-deprecated.ts:28
- Draft comment:
The migration from doctrine's 'title' to comment-parser's 'tag' is consistently applied. Double-check that all cases (including edge cases) now reference 'tag' throughout for deprecation detection. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
4. test/utils/export-map.spec.ts:32
- Draft comment:
Good update: The deprecation metadata assertions now check for 'doc.tags[0].tag' (instead of the deprecated 'title') which correctly reflects the migration to comment-parser. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. test/utils/export-map.spec.ts:40
- Draft comment:
Default imports and variables tests correctly expect the new 'tag' property for deprecation metadata. This is consistent across tests. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. test/utils/export-map.spec.ts:143
- Draft comment:
The caching tests (including file modification simulation with fs.utimes and setTimeout) are robust and clearly validate that a modified file yields a new ExportMap instance. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. test/utils/export-map.spec.ts:389
- Draft comment:
Alternate parser and deep namespace tests are comprehensive. The alternate parsers block (including mocking tsconfig and verifying export names for TypeScript) effectively covers the migration. No issues found. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
8. src/rules/no-deprecated.ts:43
- Draft comment:
The deprecation message text on line 43 ('Deprecated: consider to find an alternative.') could be rephrased for better clarity. Consider revising it to something like 'Deprecated: consider using an alternative.' - 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.
9. test/rules/no-extraneous-dependencies.spec.ts:312
- Draft comment:
It appears that the filename 'foo.tes.js' (used in several invalid test cases, e.g., line 312) might be a typo. Consider verifying if this should be 'foo.test.js' for consistency and clarity. - 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.
Workflow ID: wflow_U1BeY16I417Ohlql
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/utils/export-map.ts (1)
1010-1023: 🛠️ Refactor suggestionReturn early only when a doc block was successfully parsed
captureJsDocnow relies onparseComment, which may yieldundefined.
Return the parsed block only when it actually exists; otherwise continue the search.- try { - return parseComment(comment.value) + try { + const doc = parseComment(comment.value) + if (doc) { + return doc + } } catch {This prevents accidental propagation of
undefinedthat some downstream code might not
expect, and it also keeps the original intent of “find the nearest valid JSDoc” intact.
♻️ Duplicate comments (2)
src/meta.ts (1)
5-7: Theimport.meta.urlchange looks good.The simplified approach to determining
cjsRequireis clean and aligns with modern module resolution in Node.js.src/utils/resolve.ts (1)
21-23: The change to prioritizeimport.meta.urlis appropriate.The updated conditional logic for determining
_filenameis consistent with the overall approach to prefer modern ESM-friendly module resolution methods.
🧹 Nitpick comments (1)
test.local.js (1)
1-12: Good test script for validating comment-parser functionality.This simple test script effectively demonstrates how to use the new
comment-parserlibrary to parse JSDoc comments and extract deprecated tags. It's a useful tool for manual verification during development.As a minor enhancement, consider adding some assertions to validate the expected output rather than just logging it, which would make this more useful as an automated test.
import { parse } from 'comment-parser' -console.log( - parse(` +const parsedComment = parse(` /** * So terrible * * @deprecated This is awful, use NotAsBadClass. */ export default class TerribleClass {} -`)[0].tags.find(tag => tag.tag === 'deprecated'), -) +`)[0] + +const deprecatedTag = parsedComment.tags.find(tag => tag.tag === 'deprecated') + +console.log('Parsed deprecated tag:', deprecatedTag) + +// Basic validation +if ( + deprecatedTag && + deprecatedTag.tag === 'deprecated' && + deprecatedTag.description === 'This is awful, use NotAsBadClass.' +) { + console.log('✅ Test passed: tag correctly parsed') +} else { + console.error('❌ Test failed: unexpected tag format', deprecatedTag) + process.exit(1) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
.changeset/kind-bananas-poke.md(1 hunks)package.json(4 hunks)src/meta.ts(1 hunks)src/rules/no-deprecated.ts(5 hunks)src/utils/export-map.ts(10 hunks)src/utils/resolve.ts(1 hunks)test.local.js(1 hunks)test/rules/no-extraneous-dependencies.spec.ts(1 hunks)test/utils/export-map.spec.ts(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
test.local.js (1)
src/utils/export-map.ts (1)
parse(164-717)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- 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 20 and ESLint 8.56 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 22 and ESLint 9 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 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.56 on ubuntu-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 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 18 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 18 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 18 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on windows-latest
🔇 Additional comments (5)
.changeset/kind-bananas-poke.md (1)
1-5: Proper changelog entry for the library migration.The changelog entry correctly documents this change as a minor version update, which is appropriate for replacing a deprecated dependency without changing the public API.
test/rules/no-extraneous-dependencies.spec.ts (1)
112-112: Updated test import to use the new dependency.Test properly updated to reference the new
comment-parserdependency instead of the deprecateddoctrinelibrary.test/utils/export-map.spec.ts (1)
34-34: Successfully migrated JSDoc tag property assertions.All assertions have been properly updated to check for
taginstead oftitle, which correctly aligns with thecomment-parserAPI structure. The changes are consistent across all test cases for deprecated imports.Also applies to: 45-45, 56-56, 68-68, 78-78, 85-85
src/rules/no-deprecated.ts (1)
2-2: Clean migration from doctrine to comment-parser.The changes correctly update types and property accesses to match the
comment-parserAPI:
- Changed import from
TagtoSpectype- Updated function signatures and the
deprecatedmap type- Modified property accesses from
titletotagfor finding deprecated tagsAll changes are consistent and maintain the same functionality while adapting to the new library's structure.
Also applies to: 12-12, 28-28, 48-48, 69-69
package.json (1)
75-76:✅ Verification successful
Successfully updated dependencies for the migration.
The package.json changes appropriately add
comment-parserwhile removing the deprecateddoctrinedependencies. The version updates for other packages keep the project current with the latest compatible versions.Also applies to: 85-85
🏁 Script executed:
#!/bin/bash # Verify all references to doctrine have been removed from the codebase echo "Checking for any remaining references to doctrine..." rg -i "doctrine" --type ts --type js --type json # Check if comment-parser is being properly used echo "Verifying comment-parser usage..." rg -i "comment-parser" --type ts --type js --type jsonLength of output: 841
Migration to
comment-parserverified; nodoctrineremnants found
- Ripgrep confirms zero remaining references to “doctrine” in
.ts,.js, or.jsonfiles.comment-parseris imported and used in:
•test.local.js
•test/rules/no-extraneous-dependencies.spec.ts
•src/utils/export-map.ts
•src/rules/no-deprecated.tsAll package.json updates align with the migration. Approving these changes.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
JounQin
force-pushed
the
refactor/comment-parser
branch
from
60aa868 to
15a7171
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 15a7171 in 1 minute and 49 seconds. Click for details.
- Reviewed
433lines of code in8files - Skipped
2files when reviewing. - Skipped posting
15draft 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. package.json:130
- Draft comment:
Potential leftover: the "doctrine" package still appears in devDependencies. Confirm if it's needed after migration to comment-parser. - 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 comment points out a real potential issue - if doctrine was replaced by comment-parser, it might be unnecessary. However, without knowing the codebase, we can't be sure if doctrine is still used in tests or other dev scenarios. The comment is speculative and asks for confirmation rather than making a clear statement about a problem. The comment identifies a real potential cleanup opportunity. Moving a package from dependencies to devDependencies could indicate an incomplete migration. However, the comment violates our rules by being speculative and asking for confirmation rather than identifying a clear issue. We don't have enough context to know if doctrine is still needed in devDependencies. The comment should be deleted because it's speculative and asks for confirmation rather than identifying a clear problem that needs fixing.
2. src/meta.ts:5
- Draft comment:
Simplified cjsRequire initialization using import.meta.url is fine, but ensure fallback remains valid in non-ESM environments. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. src/rules/no-deprecated.ts:2
- Draft comment:
Good update: using the 'Spec' type from comment-parser and checking t.tag instead of t.title. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. src/utils/export-map.ts:70
- Draft comment:
New parseComment function correctly wraps comment text and applies a fixup mapping. Verify that concatenation of name with description for fixed tags meets your docs conventions. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. src/utils/export-map.ts:689
- Draft comment:
Updated logic for obtaining module doc now checks for t.tag === 'module' instead of t.title. This change is consistent with usage of comment-parser. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. src/utils/resolve.ts:21
- Draft comment:
Using import.meta.url to determine _filename is consistent; ensure fallback to __filename works in legacy environments. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. test/rules/no-extraneous-dependencies.spec.ts:112
- Draft comment:
Test updated to import 'comment-parser' instead of the deprecated package. Ensure tests cover all migration paths. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
8. test/utils/export-map.spec.ts:34
- Draft comment:
Test expectations updated to check for 'doc.tags[0].tag' instead of 'title'. Looks good. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
9. .changeset/kind-bananas-poke.md:5
- Draft comment:
The changelog message clearly documents the migration. Ensure that your release notes mention the full removal of doctrine from production dependencies. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
10. package.json:74
- Draft comment:
Dependencies now include comment-parser instead of doctrine. Verify that 'doctrine' is fully removed from production dependencies to avoid bundling an unnecessary library. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
11. src/meta.ts:5
- Draft comment:
Using import.meta.url with createRequire is a clean approach for ESM. Note that in typical ESM environments, the fallback branch may never be used. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
12. src/rules/no-deprecated.ts:22
- Draft comment:
Refactoring to use comment-parser's Spec (using 'tag' instead of 'title') looks good. Confirm that all deprecation tags in documentation now consistently use the 'tag' property. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
13. src/utils/export-map.ts:73
- Draft comment:
The new parseComment function wraps comment-parser with custom fixup logic for 'deprecated' and 'module' tags. Consider adding extra unit tests to cover edge cases with unusual comment formatting. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
14. test/utils/export-map.spec.ts:34
- Draft comment:
Test expectations have been updated to check for the 'tag' property instead of 'title'. This aligns with the migration. Double-check that all tests now consistently reflect the new schema. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
15. src/rules/no-deprecated.ts:43
- Draft comment:
The message string 'Deprecated: consider to find an alternative.' is a bit awkward. Consider rephrasing it to, for example, 'Deprecated: consider finding an alternative.' for improved readability. - 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.
Workflow ID: wflow_KHr1ndXuZXH3gWCU
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 |
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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #314 +/- ##
==========================================
+ Coverage 95.99% 96.02% +0.03%
==========================================
Files 91 92 +1
Lines 4744 4756 +12
Branches 1784 1791 +7
==========================================
+ Hits 4554 4567 +13
+ Misses 189 188 -1
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
A lot of related ESLint plugins exist, EvgenyOrekhov/eslint-config-hardcore#732, most of them are using |
JounQin
force-pushed
the
refactor/comment-parser
branch
from
15a7171 to
d5fa20d
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed d5fa20d in 1 minute and 58 seconds. Click for details.
- Reviewed
425lines of code in8files - Skipped
2files when reviewing. - Skipped posting
17draft 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/kind-bananas-poke.md:5
- Draft comment:
Good changelog entry: It clearly indicates the migration fromdoctrinetocomment-parser. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. package.json:74
- Draft comment:
Dependencies updated: Removal ofdoctrineand addition ofcomment-parseris correctly reflected. Verify that no residual references to doctrine remain. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. src/meta.ts:5
- Draft comment:
Refactored CJS require assignment using import.meta.url. Ensure fallback behavior meets runtime environments. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. src/rules/no-deprecated.ts:27
- Draft comment:
Updated search for deprecation tag from using t.title to t.tag. This fits comment-parser's API; please verify consistency across deprecation documentation. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. src/utils/export-map.ts:70
- Draft comment:
The new parseComment function uses comment-parser and applies a fixup for tags. Consider adding error handling if parseComment_ returns undefined, though this is likely rare. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
6. test/rules/no-extraneous-dependencies.spec.ts:112
- Draft comment:
Test updated to import 'comment-parser' instead of the old library. This ensures tests use updated dependency. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. test/utils/export-map.spec.ts:34
- Draft comment:
Test expectations updated to check property 'doc.tags[0].tag'. Confirm that all tests now check against the new property name from comment-parser. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
8. .changeset/kind-bananas-poke.md:5
- Draft comment:
Changeset message clearly reflects the migration from doctrine to comment-parser. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
9. package.json:75
- Draft comment:
Removal of doctrine dependencies and addition of comment-parser (with version bumps) is consistent with the migration. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
10. src/meta.ts:5
- Draft comment:
Refactored cjsRequire using import.meta.url – while the ternary check may be redundant in an ES module, the fallback is a safe guard. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
11. src/rules/no-deprecated.ts:28
- Draft comment:
Migration from doctrine to comment-parser is reflected by switching from 'title' to 'tag' when identifying deprecated annotations. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
12. src/utils/export-map.ts:73
- Draft comment:
The parseComment function now formats block comments and maps tag properties correctly; consider edge cases in varied comment formats. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%None
13. src/utils/export-map.ts:1020
- Draft comment:
Capture functions (captureJsDoc and captureTomDoc) have been updated to use comment-parser; ensure that the new structure with 'tag' works across all use cases. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
14. src/utils/resolve.ts:21
- Draft comment:
Using import.meta.url with fileURLToPath is a clean modern approach for filename resolution. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
15. test/rules/no-extraneous-dependencies.spec.ts:112
- Draft comment:
Test updated to import 'comment-parser' instead of the deprecated library; this aligns well with the migration. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
16. test/utils/export-map.spec.ts:34
- Draft comment:
Test expectations now check for the property 'doc.tags[0].tag', ensuring compatibility with the comment-parser output. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
17. src/utils/export-map.ts:83
- Draft comment:
In the parseComment function, the condition usest.name && fixup.has(t.tag). Since the comment-parser returns tag information under thetagproperty (as used elsewhere in the file), it seems more appropriate to checkt.tag && fixup.has(t.tag)(or otherwise verify the property to be checked). This could be a typographical error and might lead to unexpected results. Consider revising this condition for consistency. - 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% Looking at the code, this is processing JSDoc tags. The comment-parser library's Block type is being used. The code is checking t.name before checking if t.tag is in the fixup set. The fixup set contains 'deprecated' and 'module' which are JSDoc tag names. The code appears to be intentionally checking t.name first before combining it with the description for certain tag types. I could be wrong about the comment-parser library's API - maybe t.name is not a valid property to check. I should verify the library's type definitions. Looking at the code's type imports and usage, it's clearly using the comment-parser library's Block type, and the code is handling JSDoc tag parsing. The current code appears intentional in checking t.name before combining it with the description. The comment appears to be incorrect. The code is intentionally checking t.name before combining it with the description for certain JSDoc tags. The current logic appears to be working as intended.
Workflow ID: wflow_YkOwyXIxNUR8IzQJ
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 23d0e08 in 1 minute and 1 seconds. Click for details.
- Reviewed
84lines of code in1files - Skipped
0files when reviewing. - Skipped posting
7draft 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/export-map.ts:4
- Draft comment:
Good refactoring: importing the entire module as 'commentParser' ensures that all type usages (e.g. commentParser.Block) are consistent. Verify that the new typings match the expected API. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. src/utils/export-map.ts:73
- Draft comment:
The lazy initialization with 'cjsRequire' in parseComment is neat. Consider adding error handling if 'cjsRequire' fails to load the module. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. src/utils/export-map.ts:37
- Draft comment:
Updated DocStyleParsers type to use commentParser.Block. Ensure that all parsers in availableDocStyleParsers return the expected type. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. src/utils/export-map.ts:1030
- Draft comment:
Capturing TomDoc now casts as commentParser.Block; review that the tag objects and structure conform to the new library’s expectations. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. src/utils/export-map.ts:73
- Draft comment:
Using a lazy, dynamic require for the comment-parser provides good ESM/CJS compatibility. Ensure that any failure (e.g. module missing) is handled appropriately. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. src/utils/export-map.ts:38
- Draft comment:
Updating Block types to commentParser.Block is consistent. Make sure these new types fully match the expected structure from comment-parser. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. src/utils/export-map.ts:1035
- Draft comment:
The updated captureTomDoc signature now returns commentParser.Block. Verify that the casted object meets all of comment-parser’s expectations. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_klBFDN5rau1RXegT
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 60d4b96 in 38 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
1files 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. package.json:122
- Draft comment:
Updated @types/node version. Ensure that the new version doesn’t introduce breaking TypeScript type changes in your environment. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is related to a dependency change, specifically the@types/nodeversion. The comment asks the PR author to ensure that the new version doesn't introduce breaking changes, which violates the rule against asking the author to ensure compatibility or test for issues. Therefore, this comment should be removed.
2. package.json:122
- Draft comment:
Updated @types/node version bump looks fine; please confirm it's compatible with all Node versions supported by the project. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_HtjtFulPC33XeoZW
You can customize
JounQin
force-pushed
the
refactor/comment-parser
branch
from
60d4b96 to
5cb8aa1
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 5cb8aa1 in 57 seconds. Click for details.
- Reviewed
25lines of code in2files - Skipped
1files 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. eslint.config.js:152
- Draft comment:
Ensure added ignore pattern 'test.local.*' is intentional and follows project convention. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. package.json:122
- Draft comment:
Dependency '@types/node' updated from ^22.14.1 to ^22.15.2; verify compatibility with project. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. eslint.config.js:149
- Draft comment:
Ensure adding 'test.local.*' to the ignore list is intentional and doesn't create redundant overlap with other patterns. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. package.json:119
- Draft comment:
The @types/node version bump to '^22.15.2' is appropriate; ensure compatibility with other dependencies. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_qNC4B7s3UC1kvlXv
You can customize
JounQin
force-pushed
the
refactor/comment-parser
branch
from
5cb8aa1 to
2f4e05e
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 2f4e05e in 59 seconds. Click for details.
- Reviewed
38lines of code in3files - Skipped
1files when reviewing. - Skipped posting
8draft 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. eslint.config.js:152
- Draft comment:
Added 'test.local.*' to ignore patterns. Ensure that this pattern correctly matches your local test files. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. package.json:119
- Draft comment:
Updated '@types/node' version from ^22.14.1 to ^22.15.2. Verify compatibility with your existing codebase. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. src/rules/no-duplicates.ts:15
- Draft comment:
Using generic parameter with cjsRequire improves type safety. Confirm that this change adequately replaces the cast. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. eslint.config.js:150
- Draft comment:
Verify that 'test.local.*' matches the intended test files. Consider including a directory path if needed. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. package.json:122
- Draft comment:
The @types/node version bump to ^22.15.2 is appropriate. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. src/rules/no-duplicates.ts:15
- Draft comment:
Using generics with cjsRequire improves type safety over a type cast. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. src/rules/no-duplicates.ts:134
- Draft comment:
Typo in the comment: 'pre-caculate' should be corrected to 'pre-calculate'. - 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. src/rules/no-duplicates.ts:382
- Draft comment:
Typo in the comment: 'we're no looking for comments' should be corrected to 'we're not looking for comments'. - 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.
Workflow ID: wflow_fgaYzPCN6XkE0IQi
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 6d0b020 in 1 minute and 34 seconds. Click for details.
- Reviewed
783lines of code in47files - Skipped
1files when reviewing. - Skipped posting
30draft 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. test/utils/resolve.spec.ts:1
- Draft comment:
The tests here are very comprehensive—they cover multiple resolver interface versions (v1, v2, v3 and the resolver‐compat helper), error reporting via context.report, caching behavior, and handling of extensions and case sensitivity. Very thorough! - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. test/utils/resolve.spec.ts:479
- Draft comment:
The test for rename cache correctness uses async functions with setTimeout and fs.promises.rename. It’s well implemented; consider adding inline comments to clarify the sequence of cache invalidation and file rename, though it’s already clear. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. test/utils/resolve.spec.ts:600
- Draft comment:
Multiple tests use various forms of resolver configurations (as package names, file paths, as records, and as objects). The diversity of test cases is impressive. A minor suggestion is to add brief inline comments explaining the purpose of each configuration form for clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. test/utils/resolve.spec.ts:860
- Draft comment:
The case sensitivity tests are well done, using both exists and detecting mismatches in parent folder paths. The logic using fileExistsWithCaseSync is clear. No changes required. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. test/utils/resolve.spec.ts:557
- Draft comment:
The tests also check that invalid resolver configs and load exceptions in user resolvers are properly reported. The use of context.report to capture error messages is correct. Consider adding a single inline comment above these tests to note that these tests ensure proper error reporting for misconfiguration. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. test/utils/resolve.spec.ts:1
- Draft comment:
Very thorough tests for the resolve() function. The various custom resolver interfaces (v1, v2, v3, and fallback cases) and error reporting scenarios are well‐covered; excellent use of context.report to capture error messages and check location details. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. test/utils/resolve.spec.ts:77
- Draft comment:
Good test logic for interface version 2: The use of a temporary report array and verifying that the proper error message (including location) is reported is robust. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
8. test/utils/resolve.spec.ts:111
- Draft comment:
The test for interface version 3 using 'import-x/resolver-next' and passing a resolver function (with foobarResolver) is clear and covers the case well. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
9. test/utils/resolve.spec.ts:165
- Draft comment:
Tests for invalid resolver configuration and for resolvers with load exceptions are well-handled. This ensures users are notified when the resolver config is misconfigured or throws. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
10. test/utils/resolve.spec.ts:438
- Draft comment:
The case sensitivity tests using CASE_SENSITIVE_FS and fileExistsWithCaseSync are a nice touch to ensure proper behavior on case-sensitive filesystems. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
11. test/utils/resolve.spec.ts:478
- Draft comment:
The caching tests (rename cache correctness) are well designed: they simulate file renaming, use setTimeout to let cache expire, and then check that the new file is resolved. This ensures the caching mechanism works as expected. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
12. test/utils/resolve.spec.ts:558
- Draft comment:
Tests for different formats of specifying the resolver (as package names, file paths, records, and objects) are comprehensive, and the use of multiple describe blocks aids clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
13. test/utils/resolve.spec.ts:660
- Draft comment:
Tests in the nested 'getPhysicalFilename()' describe block further verify consistent behavior when using different settings and parserPath values. The tests are methodical and cover edge cases. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
14. test/utils/resolve.spec.ts:758
- Draft comment:
The tests that specify the resolver using object and record forms (including asynchronous imports of resolver modules) are clear and confirm that various configurations work as expected. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
15. test/utils/resolve.spec.ts:900
- Draft comment:
The error tests that simulate load errors and invalid interfaces in user resolvers are precise. The expected messages and location checks help validate robust error handling. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
16. test/utils/resolve.spec.ts:940
- Draft comment:
Overall, the 'resolve' test suite is well structured and comprehensive. The consistently used helper functions (testContext, testFilePath) and clear separation of different test concerns make it easy to follow. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
17. test/utils/resolve.spec.ts:506
- Draft comment:
Minor suggestion: In several tests that use 'expect(...).toBeUndefined()', you might consider also checking for explicit 'undefined' rather than relying on falsiness; however, this is a matter of style and the current usage is acceptable. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%None
18. CHANGELOG.md:435
- Draft comment:
Typographical error: The rule 'no-defualt-export' is misspelled. It should be 'no-default-export' for consistency. - 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.
19. src/rules/no-duplicates.ts:134
- Draft comment:
There's a typographical error in the comment: 'pre-caculate preferInline before actual fix function'. It should be 'pre-calculate preferInline before actual fix function'. - 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.
20. src/utils/legacy-resolver-settings.ts:1
- Draft comment:
The comment on line 1 ends with 'legacy~'. Consider removing or revising the tilde for clearer, more conventional text. - 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.
21. test/rules/export.spec.ts:137
- Draft comment:
Typo in comment on line 137: "can'use rule types" should be corrected to "can use rule types". - 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.
22. test/rules/namespace.spec.ts:145
- Draft comment:
Typo detected: 'defport' on line 145. Did you mean 'default'? - 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.
23. test/rules/namespace.spec.ts:175
- Draft comment:
Typo detected: 'defport' on line 175. Did you mean 'default'? - 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.
24. test/rules/newline-after-import.spec.ts:388
- Draft comment:
Typo detected: The import statement at line 388 uses 'ExecbReturnValue' from 'execb'. Did you mean 'ExecaReturnValue' from 'execa'? Please verify if this is a typo or intended. - 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.
25. test/rules/no-cycle.spec.ts:243
- Draft comment:
Typo detected in the comment on line 243: 'caracterized' should be spelled as 'characterized'. - 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.
26. test/rules/no-default-export.spec.ts:190
- Draft comment:
Typographical issue: the property 'ecmaversion' on line 190 should be corrected to 'ecmaVersion' (with a capital 'V') to properly configure the parser options. - 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.
27. test/rules/no-unused-modules.spec.ts:1415
- Draft comment:
Typo: In the test block for ES2022 Arbitrary module namespace identifier names (starting around line 1415), the rule name is given as "no-unused-module". For consistency with the imported rule name "no-unused-modules", it should be updated to "no-unused-modules". - 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.
28. test/rules/prefer-default-export.spec.ts:106
- Draft comment:
It looks like the test case code snippet 'export const [CounterProvider,, withCounter] = func();;' has a double semicolon at the end. Unless this is intentional for testing purposes, it might be a typographical error that should be cleaned up. - 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.
29. test/utils/ignore.spec.ts:78
- Draft comment:
Typographical error: In the test description on line 78, 'import-x/extension' should be corrected to 'import-x/extensions' to match the configuration key used elsewhere. - 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.
30. test/utils/ignore.spec.ts:89
- Draft comment:
Typographical error: In the test description on line 89, 'import-x/extension' should be corrected to 'import-x/extensions' for consistency with the configuration used. - 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.
Workflow ID: wflow_kDHIwISNYKgYpBwb
You can customize
JounQin
force-pushed
the
refactor/comment-parser
branch
2 times, most recently
from
a24cc6f to
5f37835
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed a24cc6f in 5 minutes and 15 seconds. Click for details.
- Reviewed
783lines of code in47files - Skipped
1files when reviewing. - Skipped posting
45draft 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. test/utils/resolve.spec.ts:23
- Draft comment:
Consider adding additional validation for null/undefined parameters in resolve(). The test for bad parameters (lines 21-31) is good, but ensure that the error message is consistent across different invalid inputs. - 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. test/utils/resolve.spec.ts:41
- Draft comment:
Tests use different custom resolvers (v1, no-version, v2, v3). Verify that the custom resolvers’ error reporting is consistent. Fragile error message assertions may require maintenance. - 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. test/utils/resolve.spec.ts:406
- Draft comment:
In the case sensitivity tests, ensure that fileExistsWithCaseSync is well-documented and its behavior with parent folder path checking is understood. - 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.
4. test/utils/export-map.spec.ts:15
- Draft comment:
The tests for ExportMap include multiple scenarios. Consider extracting common assertions into helper functions to minimize duplication. - 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/utils/export-map.spec.ts:144
- Draft comment:
The caching tests rely on file system modifications using fs.utimes and rename. Be cautious of timing issues on slower filesystems. - 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. test/utils/ignore.spec.ts:12
- Draft comment:
Tests for isIgnored, hasValidExtension, and getFileExtensions are clear. Ensure default settings are documented in plugin options for future clarity. - 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/parse.spec.ts:52
- Draft comment:
Good test coverage for parse() options. Consider adding comments on why a clone of parserOptions is used. - 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/parse.spec.ts:112
- Draft comment:
Tests that assert error-throwing for invalid languageOptions are concise. Ensure expected error messages are documented in the public docs. - 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.
9. test/utils/resolve.spec.ts:158
- Draft comment:
Tests for importXResolverCompat() correctly verify compatibility. Consider adding a comment explaining the usage of v1 and v2 resolvers interchangeably. - 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. test/utils/resolve.spec.ts:600
- Draft comment:
For tests that pass configuration as various types (package names, file paths, etc.), ensure all supported configurations are clearly documented. This extensive test coverage requires maintenance if new configuration options are added. - 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.
11. test/utils/resolve.spec.ts:434
- Draft comment:
When testing caching behavior, note that file renames and utimes can be subject to timing issues. Consider additional error handling or delays if tests become flaky. - 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.
12. test/utils/resolve.spec.ts:21
- Draft comment:
Overall, this file thoroughly tests the resolve() function across various custom resolver interfaces (v1, v2, v3 and resolver compat) as well as error reporting, caching, and edge‐cases like file extension resolution and case sensitivity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
13. test/utils/resolve.spec.ts:33
- Draft comment:
The test for a custom resolver (interface version 1) is well written. It verifies that resolving '../fixtures/foo' returns the correct file path and handles exceptions properly. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
14. test/utils/resolve.spec.ts:77
- Draft comment:
The tests for interface version 2 properly capture error reports via a custom report function and check that the error message and location are as expected. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
15. test/utils/resolve.spec.ts:111
- Draft comment:
The interface version 3 tests use the new 'import-x/resolver-next' setting with a local foobarResolver; they mirror the earlier tests and ensure that load errors are captured appropriately. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
16. test/utils/resolve.spec.ts:149
- Draft comment:
The test using importXResolverCompat() confirms that wrapping an existing resolver module yields the correct behavior. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
17. test/utils/resolve.spec.ts:165
- Draft comment:
Tests reporting an invalid resolver config are clear—when a non‐object value is provided, resolve() returns undefined and reports an error with an appropriate message and location. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
18. test/utils/resolve.spec.ts:186
- Draft comment:
The test for a loaded resolver with an invalid interface is well structured. It confirms that the error message includes the resolver name and mentions the invalid interface. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
19. test/utils/resolve.spec.ts:206
- Draft comment:
The resolve() call that honors 'import-x/resolve' extensions is clearly tested (expecting a resolution to a .jsx file). - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
20. test/utils/resolve.spec.ts:216
- Draft comment:
The test for a load exception in a user resolver is robust, checking that a SyntaxError is reported along with correct location data. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
21. test/utils/resolve.spec.ts:240
- Draft comment:
The 'getPhysicalFilename()' tests verify that caching behavior is respected and that physical filenames are correctly retrieved in different contexts. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
22. test/utils/resolve.spec.ts:438
- Draft comment:
The conditional 'case sensitivity' tests (using CASE_SENSITIVE_FS) appropriately skip tests when the filesystem is case‐sensitive, and they validate that fileExistsWithCaseSync() behaves as expected. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
23. test/utils/resolve.spec.ts:478
- Draft comment:
The 'rename cache correctness' section is notably thorough. It renames a file, waits for the cache lifetime to expire, and then asserts updated values. This helps ensure that caching correctly reflects filesystem changes. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
24. test/utils/resolve.spec.ts:558
- Draft comment:
The tests under 'respects import-x/resolver' cover various configurations (package names, file paths, record objects, and object arrays) and verify that the resolve() function correctly handles each format. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
25. test/utils/resolve.spec.ts:660
- Draft comment:
The tests for alternate resolver objects (via dynamic import) ensure that both resolver records and objects are correctly interpreted and that getPhysicalFilename() works as expected with each. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
26. test/utils/resolve.spec.ts:861
- Draft comment:
The last section testing 'respects import-x/resolver' with various forms is comprehensive. It even tests error-reporting for load exceptions and checking that error messages include proper details and locations. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
27. CHANGELOG.md:435
- Draft comment:
Typo detected: In the commit message for [refactor: backport a few upstream changes #171], 'no-defualt-export' appears, which is likely a typo. Please consider changing it to 'no-default-export'. - 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.
28. package.json:7
- Draft comment:
Typo Alert: The author's name appears as 'JounQin'. Please verify if this is the intended spelling or if it should be corrected. - 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.
29. src/rules/no-duplicates.ts:134
- Draft comment:
Typo found: 'pre-caculate' should be 'pre-calculate' for clarity. - 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.
30. src/rules/no-duplicates.ts:382
- Draft comment:
Typo found in comment: "we're no looking for comments before" should be "we're not looking for comments before". - 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.
31. src/utils/legacy-resolver-settings.ts:1
- Draft comment:
Line 1: Consider changing "the new import resolver settings is still" to "the new import resolver settings are still" to maintain subject-verb agreement. - 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.
32. src/utils/legacy-resolver-settings.ts:56
- Draft comment:
Line 56: The comment "// Any object satisfied Resolver type" could be revised to "// Any object that satisfies the Resolver type" for improved clarity. - 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.
33. src/utils/module-require.ts:9
- Draft comment:
Typo in comment: it states '_nodeModulesPaths' but the code uses '_nodeModulePaths'. Consider updating the comment for consistency. - 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.
34. test/rules/export.spec.ts:137
- Draft comment:
Typographical error: In the comment at line 137, "can'use rule types" should be corrected to "can use rule types". - 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.
35. test/rules/named.spec.ts:66
- Draft comment:
The test case uses the identifiers 'destructingAssign' and 'destructingRenamedAssign', which might be a typographical error. Typically, the term 'destructuring' is used in this context (e.g. 'destructuringAssign'). Please verify if these names should be updated for clarity. - 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.
36. test/rules/namespace.spec.ts:145
- Draft comment:
The term 'defport' in the export statement (line 145) appears to be a typographical error. If this was not intentional for testing purposes, please consider correcting it to 'default' for clarity. - 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.
37. test/rules/namespace.spec.ts:175
- Draft comment:
Similar to the export on line 145, the export statement on line 175 uses 'defport'. If this is not intentional for testing, please consider correcting it to 'default'. - 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.
38. test/rules/no-cycle.spec.ts:243
- Draft comment:
Typo: Consider changing 'caracterized' to 'characterized' in the comment on line 243 to improve clarity. - 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.
39. test/rules/no-default-export.spec.ts:190
- Draft comment:
Typo: In the languageOptions.parserOptions, 'ecmaversion' should be corrected to 'ecmaVersion' (with a capital 'V') to match the expected option name. - 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.
40. test/rules/no-extraneous-dependencies.spec.ts:200
- Draft comment:
There appears to be a typographical error in the comment around line 200. The comment reads: "// check if "rxjs" dependency declaration fix the "rxjs/operators subpackage". Consider rephrasing it to something like "// Check if the 'rxjs' dependency declaration correctly addresses the 'rxjs/operators' subpackage" to improve clarity. - 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.
41. test/rules/no-unused-modules.spec.ts:1415
- Draft comment:
There appears to be a typographical error in the rule name passed toruleTester.runfor the ES2022 Arbitrary module namespace identifier names tests (line ~1415). It currently uses 'no-unused-module', but it should likely be 'no-unused-modules' to match the rest of the suite and the intended rule name. - 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.
42. test/rules/prefer-default-export.spec.ts:106
- Draft comment:
There appears to be a duplicate semicolon in the test code for the valid case at line 106 ('export const [CounterProvider,, withCounter] = func();;'). Consider removing the extra semicolon to improve clarity. - 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.
43. test/utils/ignore.spec.ts:78
- Draft comment:
Typographical inconsistency: The test description at line 78 references 'import-x/extension' (singular), but the configuration key used elsewhere is 'import-x/extensions' (plural). Consider updating the description for consistency. - 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.
44. test/utils/ignore.spec.ts:89
- Draft comment:
Typographical inconsistency: The test description at line 89 uses 'import-x/extension' (singular) whereas the actual configuration key and other test descriptions use 'import-x/extensions' (plural). Updating it would avoid confusion. - 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.
45. test/utils/parse.spec.ts:232
- Draft comment:
The test description on line 232 reads 'throws on non-function languageOptions.parser.parse' but the test is actually checking the behavior forparseForESLint(see the object on line 238). Consider renaming it to 'throws on non-function languageOptions.parser.parseForESLint' to avoid confusion. - 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.
Workflow ID: wflow_Vcd6slyrjWzDOC6e
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 5f37835 in 4 minutes and 4 seconds. Click for details.
- Reviewed
795lines of code in48files - Skipped
1files when reviewing. - Skipped posting
39draft 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. test/utils/parse.spec.ts:67
- Draft comment:
The test correctly verifies that the custom parser is deep‐cloned before use. Consider adding a comment explaining why we check that the parserOptions object isn’t the same reference. - 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. test/utils/resolve.spec.ts:224
- Draft comment:
Test for reporting load exception in a user resolver is thorough. Verify that the thrown error message remains stable. - 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. test/utils/resolve.spec.ts:438
- Draft comment:
The case sensitivity tests are clever. Verify that fileExistsWithCaseSync works consistently across different platforms. - 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.
4. test/utils/ignore.spec.ts:26
- Draft comment:
Tests for ignore and file extension filtering look good. The configuration using 'import-x/extensions' is clear. - 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/utils/export-map.spec.ts:148
- Draft comment:
The caching and renaming tests are thorough. Consider documenting the cache lifetime in a comment for clarity. - 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. test/utils/export-map.spec.ts:254
- Draft comment:
Tests for deprecation metadata using JSDoc check the tag and description. Ensure any structural changes in metadata update these tests. - 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/resolve.spec.ts:604
- Draft comment:
Tests for various resolver configuration types are detailed. Consider adding a comment describing the multiple methods to configure a resolver. - 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/resolve.spec.ts:645
- Draft comment:
Tests verifying that a resolver provided as a record or an array returns expected file paths are well‐structured. Confirm that testFilePath remains accurate if the fixture structure changes. - 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.
9. test/utils/resolve.spec.ts:438
- Draft comment:
Ensure that using process.cwd().toUpperCase() for parent folders works consistently across operating systems with different case sensitivity. - 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. test/utils/parse.spec.ts:151
- Draft comment:
When using createRequire with import.meta.url, verify that the Node version used supports this API to avoid compatibility issues. - 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.
11. test/utils/ignore.spec.ts:60
- Draft comment:
Tests for getFileExtensions correctly merge extensions from 'import-x/parsers'. Verify the default inclusion of '.js' when not configured. - 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.
12. test/utils/export-map.spec.ts:497
- Draft comment:
Caching tests for deep namespace renaming are comprehensive. Consider adding a comment to explain the wait period before cache refresh. - 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.
13. test/utils/parse.spec.ts:269
- Draft comment:
Group the tests for invalid languageOptions together for clarity; these tests are comprehensive and ensure erroneous parser options are caught. - 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.
14. test/utils/resolve.spec.ts:615
- Draft comment:
Tests for different resolver configurations (package name, file path, etc.) are well-detailed. Ensure that expected file paths remain correct with any updates. - 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.
15. test/utils/resolve.spec.ts:645
- Draft comment:
Test verifies that a resolver loaded with an invalid interface reports a clear error message. Ensure the error text is maintained in sync with the implementation. - 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.
16. test/utils/resolve.spec.ts:80
- Draft comment:
The tests for the various custom resolver interface versions (v1, v2, v3) are very comprehensive. Consider adding a brief comment above each block summarizing the resolver type being tested to improve readability. - 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.
17. test/utils/resolve.spec.ts:81
- Draft comment:
In tests where the context’s report function is mutated (e.g. resetting the report array), consider reassigning a new empty array instead of manually setting length = 0 to avoid potential side effects. - 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.
18. test/utils/resolve.spec.ts:478
- Draft comment:
The cache tests using file renames are comprehensive. For extra robustness, consider wrapping asynchronous file operations in try/catch blocks or using more robust cleanup mechanisms. - 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.
19. test/utils/resolve.spec.ts:457
- Draft comment:
The case sensitivity tests are good, but adding inline comments explaining why the file existence check returns false in some cases would help clarify the platform‐specific 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.
20. test/utils/resolve.spec.ts:165
- Draft comment:
When asserting resolver error messages, consider using a regex or a less brittle substring check to accommodate minor formatting changes in the error output. - 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.
21. test/utils/resolve.spec.ts:600
- Draft comment:
The tests covering different forms of the 'import-x/resolver' configuration are extensive. Grouping these tests into separate describe blocks or adding inline comments would improve navigation and maintainability. - 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.
22. test/utils/resolve.spec.ts:407
- Draft comment:
Consider adding a brief comment explaining how 'testFilePath' is used to resolve paths relative to the fixtures directory for clarity and consistency. - 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.
23. CHANGELOG.md:435
- Draft comment:
Typo detected in the commit message for version 4.4.0: 'no-defualt-export' should be corrected to 'no-default-export'. - 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.
24. src/rules/no-duplicates.ts:134
- Draft comment:
Typo: In the comment 'pre-caculate preferInline before actual fix function', please change 'caculate' to 'calculate' for clarity. - 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.
25. src/utils/module-require.ts:9
- Draft comment:
Typographical inconsistency: the comment on line 9 refers to_nodeModulesPathswhereas the actual property used is_nodeModulePaths. Consider aligning the comment with the code for clarity. - 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.
26. test/rules/export.spec.ts:137
- Draft comment:
Typo detected in the comment: "can'use rule types". It likely should be corrected to "can't use rule types". Please fix for clarity. - 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.
27. test/rules/named.spec.ts:66
- Draft comment:
Consider renaming 'destructingAssign' and 'destructingRenamedAssign' to 'destructuringAssign' and 'destructuringRenamedAssign' (if this is not intentional) to reflect the common terminology and improve clarity. - 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.
28. test/rules/named.spec.ts:299
- Draft comment:
It looks like 'ActionTypess' might be a typographical error. If this is unintended, please correct it to 'ActionTypes' for consistency. - 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.
29. test/rules/namespace.spec.ts:146
- Draft comment:
Typo detected: 'defport' should likely be corrected to 'default'. This occurs in the valid test case for namespace exports. - 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.
30. test/rules/namespace.spec.ts:175
- Draft comment:
Typo detected: 'defport' should likely be corrected to 'default' in this export statement. - 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.
31. test/rules/no-cycle.spec.ts:243
- Draft comment:
Typographical error: The comment on line 243 uses 'caracterized', which appears to be a typo. It should be corrected to 'characterized'. - 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.
32. test/rules/no-default-export.spec.ts:190
- Draft comment:
Typographical error: The property 'ecmaversion' on line 190 should be corrected to 'ecmaVersion' to match ESLint configuration standards. - 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.
33. test/rules/no-empty-named-blocks.spec.ts:48
- Draft comment:
Typo: Consider capitalizing 'Typescript' as 'TypeScript' for consistency and correctness in the comment on the test cases section. - 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.
34. test/rules/no-extraneous-dependencies.spec.ts:200
- Draft comment:
Typo in the comment: the sentence 'check if "rxjs" dependency declaration fix the "rxjs/operators subpackage' seems to be missing a closing quote and may be grammatically off. Consider revising it to something like 'check if the "rxjs" dependency declaration fixes the "rxjs/operators" subpackage issue'. - 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.
35. test/rules/no-unused-modules.spec.ts:1415
- Draft comment:
Typographical Inconsistency: In the test block for 'support ES2022 Arbitrary module namespace identifier names', the rule name is passed as 'no-unused-module', whereas all other tests use 'no-unused-modules'. Consider updating it for consistency. - 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.
36. test/rules/no-unused-modules.spec.ts:1013
- Draft comment:
Typographical Issue: The file name 'file-added-4.js.js' appears to have a duplicate '.js' extension. If this is unintentional, please consider renaming it to 'file-added-4.js' for consistency. - 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.
37. test/rules/no-useless-path-segments.spec.ts:244
- Draft comment:
There's a typographical error in the trailing comment of the test case forrequire("./importType/index"). The comment says "// ./importPath.js does not exist", but it should reference 'importType' instead of 'importPath'. Please update the comment to match the test case. - 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.
38. test/utils/ignore.spec.ts:78
- Draft comment:
There is an inconsistency in the test descriptions regarding the configuration option name. In Lines 78 and 89, the test description refers to 'import-x/extension', whereas the actual configuration property used in the tests is 'import-x/extensions'. For clarity and consistency, consider updating the test description to 'import-x/extensions'. - 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.
39. test/utils/parse.spec.ts:232
- Draft comment:
Typo: The test description is identical to the previous one ('throws on non-function languageOptions.parser.parse') but the test is actually checking for a non-functionparseForESLintproperty. Please update the description to reflect that it should say 'throws on non-function languageOptions.parser.parseForESLint'. - 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.
Workflow ID: wflow_TEfDavZITJSep4Oj
You can customize
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/utils/export-map.ts (1)
1036-1062: 🛠️ Refactor suggestionUnsound cast to
commentParser.Blockin TomDoc parsing
captureTomDocfabricates aBlockwith onlydescriptionandtags.
TheBlockinterface also contains properties likeline,source,
postfix, etc. For TypeScript users consuming these objects, the cast can
lead toundefinedproperty accesses.Consider returning
Pick<Block, 'description' | 'tags'>or defining a
MinimalBlockinterface that matches what you actually create, then
up-cast where the full shape is guaranteed.-function captureTomDoc(...): commentParser.Block | undefined { +function captureTomDoc(...): Pick<commentParser.Block, 'description' | 'tags'> | undefined {
🧹 Nitpick comments (5)
test/fixtures/foo-bar-resolver-v3.js (1)
1-2: Prefer@typedef {import(...)}over a non-standard@importtag
comment-parserand most JSDoc tooling recognise@typedef {import('pkg').Type} Aliasor@type {import('pkg').Type}.
The custom@importtag may be parsed as an unknown tag and lose type-checking benefits.-/** @import {NewResolver} from 'eslint-plugin-import-x' */ +/** @typedef {import('eslint-plugin-import-x').NewResolver} NewResolver */Unless the fixture intentionally tests unsupported tags, consider switching to the canonical form for better editor/TS integration.
src/rules/no-deprecated.ts (2)
12-21: Minor: trim long descriptions to keep diagnostics conciseWhen a
@deprecatedtag contains a very long description, the entire text is forwarded to the ESLint diagnostic unchanged.
Displaying hundreds of characters in the Problems pane can be noisy.A small helper could truncate or wrap the text (e.g.
slice(0, 120) + '…') while keeping the full text indeprecation.descriptionfor those who want it.
28-29: Use a shared constant for'deprecated'The literal
'deprecated'appears in several places in this rule and inexport-map.ts.
Exporting aconst DEPRECATED_TAG = 'deprecated'from a shared utility would:• avoid typos
• make future renames trivial
• allow tools likeeslint-plugin/unicorn/prefer-export-fromto flag duplicationsrc/utils/export-map.ts (2)
73-84: Lazy-loadingcomment-parsercan be simplifiedYou already cache
parseComment_, so constructingrestoredfor
every call is now the dominant cost. Consider a fast-path when the
original comment is already a well-formed JSDoc block:-const restored = `/**${comment.split(/\r?\n/)...` -const [doc] = parseComment_(restored) +const source = comment.startsWith('*') ? `/*${comment}*/` : `/**${comment}*/` +const [doc] = parseComment_(source)This avoids the per-line
reduceand keeps identical semantics.
687-704: Performance: stop iterating once module doc is foundAfter
doc.tags.some(t => t.tag === 'module')succeeds, the current
implementation returns the doc, but the outerforloop continues in the
next lazy evaluation call becausereturnexits only thetryblock,
then execution proceeds to the next comment. Wrapping the loop in its own
function or adding abreakafter thereturnis unnecessary – but the
compiler keeps the unreachable code. A tiny refactor eliminates dead code
and clarifies intent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (52)
.changeset/kind-bananas-poke.md(1 hunks).gitignore(1 hunks)CHANGELOG.md(1 hunks)eslint.config.js(1 hunks)index.d.cts(1 hunks)package.json(4 hunks)src/index.ts(3 hunks)src/meta.ts(1 hunks)src/require.ts(1 hunks)src/rules/no-deprecated.ts(5 hunks)src/rules/no-duplicates.ts(2 hunks)src/rules/no-import-module-exports.ts(1 hunks)src/types.ts(1 hunks)src/utils/export-map.ts(10 hunks)src/utils/legacy-resolver-settings.ts(1 hunks)src/utils/module-require.ts(1 hunks)src/utils/resolve.ts(1 hunks)test/fixtures/foo-bar-resolver-v3.js(1 hunks)test/rules/consistent-type-specifier-style.spec.ts(1 hunks)test/rules/default.spec.ts(1 hunks)test/rules/dynamic-import-chunkname.spec.ts(1 hunks)test/rules/export.spec.ts(1 hunks)test/rules/first.spec.ts(1 hunks)test/rules/group-exports.spec.ts(1 hunks)test/rules/max-dependencies.spec.ts(1 hunks)test/rules/named.spec.ts(1 hunks)test/rules/namespace.spec.ts(1 hunks)test/rules/newline-after-import.spec.ts(1 hunks)test/rules/no-absolute-path.spec.ts(1 hunks)test/rules/no-anonymous-default-export.spec.ts(1 hunks)test/rules/no-cycle.spec.ts(1 hunks)test/rules/no-default-export.spec.ts(1 hunks)test/rules/no-duplicates.spec.ts(1 hunks)test/rules/no-dynamic-require.spec.ts(1 hunks)test/rules/no-empty-named-blocks.spec.ts(1 hunks)test/rules/no-extraneous-dependencies.spec.ts(2 hunks)test/rules/no-mutable-exports.spec.ts(1 hunks)test/rules/no-named-as-default-member.spec.ts(1 hunks)test/rules/no-named-as-default.spec.ts(1 hunks)test/rules/no-named-default.spec.ts(1 hunks)test/rules/no-named-export.spec.ts(1 hunks)test/rules/no-relative-parent-imports.spec.ts(1 hunks)test/rules/no-unused-modules.spec.ts(1 hunks)test/rules/no-useless-path-segments.spec.ts(1 hunks)test/rules/order.spec.ts(1 hunks)test/rules/prefer-default-export.spec.ts(1 hunks)test/rules/unambiguous.spec.ts(1 hunks)test/utils.ts(1 hunks)test/utils/export-map.spec.ts(6 hunks)test/utils/ignore.spec.ts(1 hunks)test/utils/parse.spec.ts(1 hunks)test/utils/resolve.spec.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
src/index.ts (1)
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#0
File: :0-0
Timestamp: 2025-03-30T09:56:52.604Z
Learning: The project supports both ESM and CommonJS module systems simultaneously, even as it migrates to use ESM as the primary module system. Using `cjsRequire` is part of this strategy to maintain compatibility with both systems.
src/meta.ts (1)
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#0
File: :0-0
Timestamp: 2025-03-30T09:56:52.604Z
Learning: The project supports both ESM and CommonJS module systems simultaneously, even as it migrates to use ESM as the primary module system. Using `cjsRequire` is part of this strategy to maintain compatibility with both systems.
src/require.ts (1)
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#0
File: :0-0
Timestamp: 2025-03-30T09:56:52.604Z
Learning: The project supports both ESM and CommonJS module systems simultaneously, even as it migrates to use ESM as the primary module system. Using `cjsRequire` is part of this strategy to maintain compatibility with both systems.
src/utils/export-map.ts (1)
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#314
File: src/utils/export-map.ts:681-696
Timestamp: 2025-04-24T16:00:37.936Z
Learning: The `parseComment` function in the `src/utils/export-map.ts` file will not return `undefined`. It always returns a `Block` object, as indicated by its explicit return type, and there's no need to check for `undefined` before accessing its properties.
🧬 Code Graph Analysis (3)
src/rules/no-duplicates.ts (1)
src/require.ts (1)
cjsRequire(7-9)
src/require.ts (3)
src/meta.ts (1)
cjsRequire(3-5)src/index.ts (1)
cjsRequire(197-197)src/types.ts (1)
CjsRequire(239-241)
src/utils/export-map.ts (1)
src/require.ts (1)
cjsRequire(7-9)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Lint and Test with Node.js 18 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 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.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 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 22 and ESLint 9 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 20 and ESLint 8 on ubuntu-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 9 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 22 and ESLint 8 on windows-latest
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (53)
src/types.ts (1)
238-241: Well-defined interface for the new internalcjsRequirefunction.The generic CjsRequire interface extends NodeJS.Require appropriately, ensuring type safety when requiring modules with specific return types.
test/rules/unambiguous.spec.ts (1)
5-5: Import source updated correctly.The import source for
cjsRequirehas been updated from the external@pkgr/corepackage to the project's own implementation ineslint-plugin-import-x, aligning with the refactoring strategy.src/utils/legacy-resolver-settings.ts (1)
8-8: Import source updated correctly.The import source for
cjsRequirehas been updated from the external@pkgr/corepackage to a local relative import../require.js, aligning with the refactoring strategy to centralize this utility.src/utils/module-require.ts (1)
4-4: Import source updated correctly.The import source for
cjsRequirehas been updated from the external@pkgr/corepackage to a local relative import../require.js, consistent with the centralization of this utility within the project.src/rules/no-import-module-exports.ts (1)
6-6: LGTM: Successfully migrated to local cjsRequire importThe import source for
cjsRequirehas been properly updated to use the new local module instead of the external dependency.test/rules/first.spec.ts (1)
7-7: LGTM: Successfully migrated to package cjsRequire importThe import source for
cjsRequirehas been properly updated to use the main package instead of the external dependency.test/rules/consistent-type-specifier-style.spec.ts (1)
7-7: LGTM: Successfully migrated to package cjsRequire importThe import source for
cjsRequirehas been properly updated to use the main package instead of the external dependency.test/rules/no-named-as-default-member.spec.ts (1)
12-12: LGTM: Successfully migrated and maintained alias patternThe import source for
cjsRequirehas been properly updated while preserving the alias pattern, ensuring consistent behavior with the rest of the codebase..changeset/kind-bananas-poke.md (1)
1-5: LGTM! The changeset file is properly formatted.The changeset correctly documents the migration from the deprecated
doctrinelibrary tocomment-parserwith appropriate versioning information. This aligns perfectly with the PR objectives.test/rules/no-anonymous-default-export.spec.ts (1)
11-11: Import source change looks good.The import for
cjsRequirehas been updated to use the local implementation from 'eslint-plugin-import-x' instead of an external package, which aligns with the overall refactoring goal of centralizing utilities.test/utils/ignore.spec.ts (1)
3-3: Type import consolidation looks good.The
PluginSettingstype is now imported directly from the main package rather than a subpath, which follows the standardization pattern implemented throughout the codebase.test/rules/export.spec.ts (1)
11-11: Import source change looks good.The import for
cjsRequirehas been updated to use the local implementation from 'eslint-plugin-import-x' instead of an external package, maintaining consistency with other files in this PR.test/rules/group-exports.spec.ts (1)
5-5: Import source updated correctly.The import source for
cjsRequirehas been changed from@pkgr/coretoeslint-plugin-import-x, aligning with the PR's goal of migrating to an internal implementation of the CommonJS require helper.test/rules/no-cycle.spec.ts (1)
7-7: Import source updated correctly.The import source for
cjsRequirehas been updated from@pkgr/coretoeslint-plugin-import-xand aliased asrequire. This change is consistent with the migration to using the internal implementation of the CommonJS require helper.test/rules/no-relative-parent-imports.spec.ts (1)
7-7: Import source updated correctly.The import source for
cjsRequirehas been changed from@pkgr/coretoeslint-plugin-import-x, consistent with the other changes in this PR to use the new internal implementation.test/rules/no-named-default.spec.ts (1)
11-11: Import source updated correctly.The import source for
cjsRequirehas been changed from@pkgr/coretoeslint-plugin-import-xand appropriately aliased asrequire. This change aligns with the PR's objective to migrate to the internal implementation of the CommonJS require helper.test/rules/max-dependencies.spec.ts (1)
5-5: Import source updated correctly.The import for
cjsRequirehas been properly updated to source fromeslint-plugin-import-xinstead of@pkgr/core, aligning with the broader refactor to use the newly introduced local implementation.test/rules/named.spec.ts (1)
16-16: Import source updated correctly.The import for
cjsRequirehas been properly updated to source from the localeslint-plugin-import-xpackage rather than the external@pkgr/corepackage, consistent with the module loading refactor.test/rules/prefer-default-export.spec.ts (1)
12-12: Import source updated correctly.The import for
cjsRequirehas been properly updated to source from the local implementation ineslint-plugin-import-xinstead of from@pkgr/core, consistent with the introduction of the newcjsRequirehelper.test/rules/no-named-as-default.spec.ts (1)
12-12: Import source updated correctly.The import for
cjsRequirehas been properly updated to source fromeslint-plugin-import-xinstead of@pkgr/core, aligning with the broader refactor to centralize the CommonJS require helper implementation.test/rules/namespace.spec.ts (1)
13-13: Import source updated correctly.The import for
cjsRequirehas been properly updated to source fromeslint-plugin-import-xinstead of@pkgr/core, consistent with the migration to use the new local implementation across the codebase.test/rules/no-useless-path-segments.spec.ts (1)
5-5: Import source updated correctly.The import source for
cjsRequirehas been changed from@pkgr/coretoeslint-plugin-import-x, which aligns with the broader refactoring to use a local implementation of the require helper.test/rules/no-empty-named-blocks.spec.ts (1)
6-6: Import source updated correctly.The import source for
cjsRequirehas been updated from@pkgr/coretoeslint-plugin-import-xas part of the broader refactoring to centralize the require helper implementation.test/rules/newline-after-import.spec.ts (1)
7-7: Import source updated correctly.The import source for
cjsRequirehas been changed from@pkgr/coretoeslint-plugin-import-x, consistent with the other test files in this PR.test/rules/default.spec.ts (1)
13-13: Import source updated correctly.The import source for
cjsRequirehas been updated from@pkgr/coretoeslint-plugin-import-x, maintaining consistency with the other updated test files.test/rules/no-absolute-path.spec.ts (1)
7-7: Import source updated correctly.The import source for
cjsRequirehas been changed from@pkgr/coretoeslint-plugin-import-x, consistent with the broader refactor to use the locally implemented require helper.test/rules/no-dynamic-require.spec.ts (1)
6-6: Properly migrated cjsRequire import sourceThe import source for
cjsRequirehas been correctly updated to use the new internal module implementation rather than the external@pkgr/corepackage. This aligns with the PR objective of introducing a dedicated module loading helper.src/index.ts (2)
22-22: Import of new cjsRequire module added correctlyThe new local
cjsRequirehelper is properly imported from./require.js, implementing the centralized module loading helper as outlined in the PR objectives.
187-187: Properly exported cjsRequire from index moduleThe
cjsRequireutility is correctly exposed in both the default export and named exports, making it accessible throughout the codebase. This supports the dual ESM/CommonJS compatibility approach used in the project.Also applies to: 197-197
.gitignore (1)
30-31: Added appropriate gitignore rule for local test filesThe addition of the
test.local.*pattern to gitignore properly implements the PR's objective to exclude local test files from version control.test/rules/order.spec.ts (1)
13-13: Properly migrated cjsRequire import source with aliasThe import source for
cjsRequire(aliased asrequire) has been correctly updated to use the new internal implementation rather than the external@pkgr/corepackage. The alias maintains backward compatibility with existing code.test/rules/no-duplicates.spec.ts (1)
14-16: ConsistentcjsRequireimport source
ThecjsRequirehelper is now imported from the packaged entry (eslint-plugin-import-x), matching the refactoredsrc/require.ts. This aligns test imports with the new module layout without affecting test logic.src/meta.ts (1)
1-1: ImportcjsRequirefrom local helper
Replacing the inline conditional resolver with a direct import from./require.jscentralizes CJS loading logic and improves maintainability.test/rules/no-unused-modules.spec.ts (1)
14-14: UpdatecjsRequireimport path
Switching the test’scjsRequireimport toeslint-plugin-import-xaligns it with the new packaged helper location. No change to test behavior.test/rules/no-named-export.spec.ts (1)
6-6: AligncjsRequireimport to plugin entry
The test now sourcescjsRequirefromeslint-plugin-import-x, reflecting the new helper extraction. Functionality remains unchanged.test/rules/no-default-export.spec.ts (1)
8-8: SwitchcjsRequireimport to plugin entry
ImportingcjsRequirefromeslint-plugin-import-xstandardizes test dependencies with the refactored helper module. Tests continue to run without modification.test/rules/no-extraneous-dependencies.spec.ts (2)
10-10: Import source change looks good.The update to import
CjsRequiretype from 'eslint-plugin-import-x' aligns with the migration from external dependencies to local module helpers.
112-112: Test dependency correctly updated.Replaced
"doctrine"with"comment-parser"to reflect the migration from the deprecated library, ensuring tests validate against the new dependency.test/rules/no-mutable-exports.spec.ts (1)
7-7: Import source change looks good.The update to import
cjsRequirefrom 'eslint-plugin-import-x' aligns with the new module loading helper approach implemented throughout the codebase.eslint.config.js (1)
152-152: ESLint configuration update looks good.Added
'test.local.*'to the list of files exempt from import-related rules, which complements the.gitignorechanges to exclude local test files from version control.test/rules/dynamic-import-chunkname.spec.ts (1)
8-8: Import source change looks good.The update to import
cjsRequirefrom 'eslint-plugin-import-x' aligns with the new centralized module loading helper approach.CHANGELOG.md (1)
320-320: Updated NewResolver import path correctlyThe documentation now correctly indicates that users can import
NewResolverdirectly fromeslint-plugin-import-xrather than from a subpath. This aligns with the broader refactoring to consolidate imports across the codebase.test/utils/parse.spec.ts (1)
12-16: Simplified types import pathThe import path for types has been updated to import directly from the main package instead of from a subpath, which aligns with the refactoring across the codebase to consolidate import sources.
src/rules/no-duplicates.ts (2)
5-5: Updated cjsRequire import sourceThe import source for
cjsRequirehas been changed from an external package to a local module, reducing external dependencies and aligning with the PR objective of introducing a new local module loading helper.
15-15: Improved type safety with generic type parameterUpdated the usage of
cjsRequireto use a generic type parameter<PackageJson>instead of a type assertion, which provides better type safety and maintainability.index.d.cts (1)
1-1: Updated import style for better named exports accessChanged from a default import to a namespace import, which allows access to all named exports including the new
cjsRequirehelper. This is consistent with the broader refactoring to make utilities available from the main package export.test/utils.ts (1)
12-13: Import paths updated to reflect internal structure changesThe imports have been updated to source
cjsRequiredirectly from 'eslint-plugin-import-x' instead of '@pkgr/core', and to import types directly from the main package rather than from a '/types' subpath.test/utils/resolve.spec.ts (1)
12-12: Consolidated type imports from main packageThe types
CjsRequireandNewResolverare now imported directly from 'eslint-plugin-import-x' rather than from separate locations, which aligns with the refactoring to localize thecjsRequireutility.test/utils/export-map.spec.ts (2)
9-9: Updated type import locationTypes are now being imported from the main package rather than the '/types' subpath, for consistency with other files in the codebase.
34-34: Updated property access for comment parser compatibilityJSDoc tag assertions have been updated from
doc.tags[0].titletodoc.tags[0].tagto align with the structure provided by the newcomment-parserlibrary, which has replaced the deprecateddoctrinelibrary.Also applies to: 45-46, 56-57, 68-69, 78-79, 85-86
src/utils/resolve.ts (1)
21-25: Improved module resolution logic with better fallback handlingThe code now first stores
import.meta.urlin a variable before using it to determine_filename, making the condition clearer and adding proper coverage annotation for the fallback case.This approach for handling ESM/CommonJS interoperability is important since the project supports both module systems simultaneously, as noted in previous discussions. The
import.meta.urlwill be properly transformed by rollup during build.src/require.ts (1)
1-10: New module for centralized CommonJS require functionalityThis new module exports a typed
cjsRequireutility that's compatible with both ESM and CommonJS environments. It uses Node.js'screateRequirewith the current module's URL to create a properly scoped require function.The implementation:
- Handles the ESM case by using
createRequire(import.meta.url)- Provides a fallback to global
requirefor CommonJS environments- Correctly marks the fallback case for code coverage tools to ignore
- Uses proper typing via the
CjsRequireinterfaceThis centralized approach eliminates the need for the external
@pkgr/coredependency and ensures consistent require functionality throughout the codebase.package.json (1)
70-76: Verify thatcomment-parser@^1.4.1is ESM-compatible with Jest
comment-parseris CommonJS only up to v1.x.
While you circumvent that at runtime viacjsRequire, Jest still hoists
ESM mocks beforerequireruns, which can trip test environments that
assume ESM harmony. Please run the test suite withNODE_OPTIONS="--experimental-vm-modules"
(one of your npm scripts already does) to ensure there are no resolution
issues.No action needed if CI is green – this is just a heads-up.
Also applies to: 84-85
JounQin
force-pushed
the
refactor/comment-parser
branch
from
5f37835 to
feb3e00
Compare
|
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed feb3e00 in 5 minutes and 1 seconds. Click for details.
- Reviewed
804lines of code in48files - Skipped
1files when reviewing. - Skipped posting
31draft 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. test/utils/export-map.spec.ts:15
- Draft comment:
The test code for parsing deprecated JSDoc comments looks clear. Verify that the expected properties (e.g. doc.tags[0] for 'deprecated') correctly reflect the transition from the old doctrine-based parser to the new comment-parser implementation. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. test/utils/ignore.spec.ts:11
- Draft comment:
Test cases for ignore functions correctly cover the default and configured extensions. No issues found. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. test/utils/parse.spec.ts:52
- Draft comment:
The custom parser tests ensure that parserOptions are cloned and augmented (attachComment, tokens, range, filePath). Verify that these options work as intended with the new comment-parser integration. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. test/utils/resolve.spec.ts:21
- Draft comment:
The resolve tests with multiple custom resolvers (versions 1, 2, and 3) and caching look comprehensive. Ensure that error reporting via context.report is displayed as expected with the new resolver implementations. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. test/utils/resolve.spec.ts:436
- Draft comment:
The case sensitivity tests use a conditional describe. Confirm that the helper fileExistsWithCaseSync behaves correctly when comparing file system case-sensitivity on different OS's. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. test/utils/resolve.spec.ts:478
- Draft comment:
Tests for caching behavior using fs.utimes and renaming files are thorough; ensure that the cache invalidation logic works as expected. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. test/utils/export-map.spec.ts:498
- Draft comment:
The tests for ExportMap caching and self-reference ("narcissist") are detailed. Verify that the new comment parser output (if any) is preserved in the ExportMap’s doc property. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
8. test/utils/ignore.spec.ts:56
- Draft comment:
The tests covering getFileExtensions show that both 'import-x/extensions' and 'import-x/parsers' settings are merged correctly. No issues detected. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
9. test/utils/parse.spec.ts:113
- Draft comment:
There are several tests checking error handling when languageOptions.parser is not valid. Ensure error messages are consistent after migration. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
10. test/utils/resolve.spec.ts:606
- Draft comment:
Tests for various resolver configuration formats (package names, file paths, object records) look good; verify that the migration cleanly supports all these formats. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
11. test/utils/resolve.spec.ts:812
- Draft comment:
The test 'reports load exception in a user resolver' correctly simulates a syntax error in a custom resolver. Ensure that exceptions are wrapped and reported without exposing sensitive details. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
12. test/utils/parse.spec.ts:369
- Draft comment:
The tests confirm that languageOptions.parserOptions from languageOptions override the top-level parserOptions. This is important for ensuring backward compatibility. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
13. test/utils/export-map.spec.ts:169
- Draft comment:
The ExportMap tests for deep namespace caching and self-reference appear comprehensive. Please verify that the caching logic and file system timestamp comparisons continue to work with the revised comment parsing. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
14. test/utils/resolve.spec.ts:259
- Draft comment:
Test cases for 'getPhysicalFilename()' confirm that the resolver returns updated results when the file is modified. Ensure that any adjustments in resolution due to internal changes are covered. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
15. test/utils/resolve.spec.ts:88
- Draft comment:
Instead of resetting the testContextReports array by setting its length to 0, consider using splice(0) for clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
16. test/utils/resolve.spec.ts:480
- Draft comment:
The rename cache correctness tests rely on renaming fixture files and using fixed delays with setTimeout. Consider adding documentation to explain the cache lifetime logic and ensuring these file operations run in isolation. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
17. test/utils/resolve.spec.ts:380
- Draft comment:
In the case sensitivity tests, the file path is derived using process.cwd().toUpperCase(). Ensure this logic behaves consistently across different operating systems to avoid false positives or negatives. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
18. test/utils/resolve.spec.ts:240
- Draft comment:
The tests for custom resolver interfaces (v1, v2, v3) are comprehensive. Consider adding brief comments above each block summarizing the intended behavior for added clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
19. test/utils/resolve.spec.ts:396
- Draft comment:
The tests for various forms of 'import-x/resolver' configuration (package names, file paths, object records) are thorough. No major issues found here. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
20. test/utils/resolve.spec.ts:560
- Draft comment:
Overall, the test suite in this file is comprehensive and covers many edge cases for module resolution and caching. Good job ensuring that error reporting (including message matching and location) is validated. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
21. CHANGELOG.md:435
- Draft comment:
Typo detected: 'no-defualt-export' should be corrected to 'no-default-export'. - 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.
22. src/rules/no-duplicates.ts:134
- Draft comment:
Typographical error: In the comment on line 134, 'pre-caculate' should be corrected to 'pre-calculate'. - 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.
23. test/rules/export.spec.ts:137
- Draft comment:
Typographical error: In the comment at line 137, "can'use" appears to be a typo. Consider changing it to "can't use" for clarity. - 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.
24. test/rules/namespace.spec.ts:145
- Draft comment:
It appears that 'defport' on line 145 might be a typo for 'default'. Please double-check if this was intended, otherwise consider correcting it. - 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.
25. test/rules/namespace.spec.ts:175
- Draft comment:
It appears that 'defport' on line 175 might be a typo for 'default'. Please check if this is correct, or update it to the intended term. - 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.
26. test/rules/no-cycle.spec.ts:243
- Draft comment:
Typographical error: In the comment on line 243, consider replacing 'caracterized' with 'characterized' for clarity. - 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.
27. test/rules/no-default-export.spec.ts:190
- Draft comment:
Typographical error: 'ecmaversion' on line 190 should be corrected to 'ecmaVersion' to match the expected property name in parserOptions. - 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.
28. test/rules/no-unused-modules.spec.ts:1415
- Draft comment:
It appears that the rule name passed toruleTester.runis 'no-unused-module' (singular) whereas the imported rule is named 'no-unused-modules'. This discrepancy seems like a typographical error. Please update the rule name to 'no-unused-modules' to ensure consistency. - 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.
29. test/utils/ignore.spec.ts:78
- Draft comment:
Typographical error: The test description in this block refers to "import-x/extension", but the configuration key used in the code is "import-x/extensions". Consider updating the description for consistency. - 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.
30. test/utils/ignore.spec.ts:89
- Draft comment:
Typographical error: The test description here mentions "import-x/extension" (singular) whereas the code uses "import-x/extensions" (plural). Please update this string to maintain consistency with the configuration property. - 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.
31. test/utils/parse.spec.ts:217
- Draft comment:
The test descriptions for the non-function parser methods are identical. The first test correctly describes a failure when a non-function is provided forlanguageOptions.parser.parse, while the second test (lines 232-244) is intended to verify a failure when a non-function is provided forlanguageOptions.parser.parseForESLint. Please update the second test description to accurately reflect that it's testingparseForESLintinstead of repeating 'parse'. - 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.
Workflow ID: wflow_MzwoMv3jNPfrrQK9
You can customize
close #313
Important
Migrated from
doctrinetocomment-parserfor comment parsing, updated test cases, and introduced a new CommonJS require helper.doctrinetocomment-parserfor comment parsing insrc/rules/no-deprecated.ts,src/utils/export-map.ts, andsrc/rules/no-duplicates.ts.cjsRequireinsrc/require.tsfor consistent module loading.test/rules/no-deprecated.tsandtest/utils/export-map.spec.tsto supportcomment-parser.doctrineand addedcomment-parserinpackage.jsondependencies..gitignoreand ESLint configuration ineslint.config.jsto exclude local test files.This description was created by
for feb3e00. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Refactor
Bug Fixes
Chores