Improve suggestions for duplicate exports and imports rules #4
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the duplicate imports and exports rules by adding automatic merge suggestions. The changes transform these rules from simple detection-only rules to ones that can suggest concrete fixes by merging duplicate import/export statements.
Key Changes
- Added suggestion generation with automatic merge functionality for both duplicate imports and exports
- Refactored data tracking from simple Sets to structured objects that store full import/export information
- Updated error messages to be more concise and accurate
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/tsl-module/src/rules/no-duplicate-imports.ts | Refactored to track full import details and added buildMergedImport function to generate merge suggestions |
| packages/tsl-module/src/rules/no-duplicate-imports.test.ts | Added test expectations for merge suggestions including complex cases with inline type modifiers |
| packages/tsl-module/src/rules/no-duplicate-exports.ts | Refactored to track export declarations and added buildSuggestions function for generating merge proposals |
| packages/tsl-module/src/rules/no-duplicate-exports.test.ts | Added test expectations for merge suggestions |
| packages/tsl-module/docs/variables/noDuplicateImports.md | Updated documentation to reflect "detect and merge" functionality |
| packages/tsl-module/docs/variables/noDuplicateExports.md | Updated documentation to reflect "detect and merge" functionality |
| packages/tsl-module/docs/README.md | Updated rule descriptions in the main documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| && ts.isNamedImports(namedBindings) | ||
| ? namedBindings.elements.map((el) => el.getText()) | ||
| : [], | ||
| namespaceImport: namedBindings != null | ||
| && ts.isNamespaceImport(namedBindings) |
There was a problem hiding this comment.
There's inconsistent indentation in this conditional expression. Lines 112-113 use different indentation than line 114, making the code harder to read. The logical AND operator on line 113 should align consistently with the ternary expression.
| && ts.isNamedImports(namedBindings) | |
| ? namedBindings.elements.map((el) => el.getText()) | |
| : [], | |
| namespaceImport: namedBindings != null | |
| && ts.isNamespaceImport(namedBindings) | |
| && ts.isNamedImports(namedBindings) | |
| ? namedBindings.elements.map((el) => el.getText()) | |
| : [], | |
| namespaceImport: namedBindings != null | |
| && ts.isNamespaceImport(namedBindings) |
| function decodeImportClause(node: AST.ImportClause) { | ||
| const { name, namedBindings } = node; | ||
| return { | ||
| defaultImport: name?.getText(), |
There was a problem hiding this comment.
The decodeImportClause function returns unit for the namespaceImport field when not present (line 119), but returns undefined for the defaultImport field when name is absent (line 111). This inconsistency means the types don't match the ImportInfo interface which expects both fields to be string | unit. The defaultImport should also return unit when not present.
| defaultImport: name?.getText(), | |
| defaultImport: name != null ? name.getText() : unit, |
No description provided.