Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.6k
feat(extensions): enhance import extension enforcement with autofix support#3177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you confirm you did not use an LLM to generate any of this code? LLM-generated code has unclear provenance and can't be safely licensed to an open source project.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
262ab6c
tod84fbfa
Compare
@ljharb I've reverted the parts where it was LLM generated. Didn't know what to put in type, category, and description for meta. Also, my PR description is LLM generated, i have reviewed it though, I hope that's alright. The rest is my work. Thanks for taking the time to review this PR! I'm a bit stuck on the remaining failing tests and not sure how to proceed. Would you be open to taking ownership of this PR to get it across the finish line? Again, thank you! |
Thanks for confirming. The test failures are because your changes have broken the rule's behavior. One thing to look at is that until I can't take ownership of this as-is, unfortunately. |
6f11cda
toe5443f8
Comparestephenjason89 commentedApr 8, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@ljharb Thanks for the reply. I have further reduced the failing tests to 12 now. I also need your feedback on reimplementing Problem is, if we add a rule like
All the other extensions not defined in pattern falls back to Please help on the remaining failing tests as well. I'm a bit stuck on the remaining tests. Thank you |
e5443f8
to7d124ee
Comparecodecovbot commentedApr 9, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #3177 +/- ##==========================================- Coverage 95.52% 95.29% -0.24%========================================== Files 83 83 Lines 3690 3695 +5 Branches 1333 1337 +4 ==========================================- Hits 3525 3521 -4- Misses 165 174 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
49381fd
toe6458bf
Compare@ljharb, I’ve reworked my pull request to include a This new version should be much more targeted and should be easier to review. |
93d3bfe
tob8e7370
Compare@ljharb I've rebased and added tests to increase coverage. Please re-review and merge if it looks good. Thanks! |
@ljharb may I gently follow up on this PR, would love to hear you feedback. Thanks |
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as off-topic.
This comment was marked as off-topic.
fix(fixer) { | ||
return fixer.replaceText( | ||
source, | ||
JSON.stringify(`${importPathWithQueryString}.${extension}`), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seehttps://github.com/un-ts/eslint-plugin-import-x/pull/329/files#r2094349587
JSON.stringify
is not correct for single quoted imports.
https://github.com/un-ts/eslint-plugin-import-x/pull/327/files#r2094298890
importPathWithQueryString
could containsquery
andhash
, and could also ends with.
,..
or/
.
return fixer.replaceText( | ||
source, | ||
JSON.stringify( | ||
importPath.slice(0, -(extension.length + 1)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seehttps://github.com/un-ts/eslint-plugin-import-x/pull/327/files#r2094298890
extension
is not always at the end.
Uh oh!
There was an error while loading.Please reload this page.
Summary
This PR enhances the import extensions rule by introducing auto-fix support. With this change, ESLint can now automatically:
Changes
New Configuration Option:
Added a new
fix
option (defaulting tofalse
) in the rule's configuration. When enabled, the rule will attempt to fix detected issues automatically.Meta Update:
Marked the rule as fixable by adding
fixable: 'code'
in the meta information.Auto-fix Implementation:
Extended the reporting logic in
checkFileExtension
with fixer functions:Code Adjustments:
Updated the
buildProperties
function to handle the newfix
property from the user configuration.Motivation
This update improves developer experience by allowing ESLint to automatically correct common mistakes regarding file extensions in import paths, reducing manual intervention during code review or development.
Backward Compatibility
All existing functionality remains unchanged when the
fix
option is disabled. Users can enable auto-fixing by settingfix: true
in their ESLint configuration for this rule.