Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(extensions): enhance import extension enforcement with autofix support #3177

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stephenjason89
Copy link

@stephenjason89 stephenjason89 commented Apr 8, 2025

Summary

This PR enhances the import extensions rule by introducing auto-fix support. With this change, ESLint can now automatically:

  • Append a missing file extension when one is required.
  • Remove an extension when its presence is forbidden and the module is resolvable without it.

Changes

  • New Configuration Option:
    Added a new fix option (defaulting to false) 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:

    • For missing file extensions, if auto-fix is enabled, the fixer appends the detected extension.
    • For extraneous file extensions, the fixer removes the extension from the import statement.
  • Code Adjustments:
    Updated the buildProperties function to handle the new fix 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 setting fix: true in their ESLint configuration for this rule.

Copy link
Member

@ljharb ljharb left a 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.

@stephenjason89 stephenjason89 force-pushed the feat/extensions-autofix branch from 262ab6c to d84fbfa Compare April 8, 2025 16:47
@stephenjason89 stephenjason89 requested a review from ljharb April 8, 2025 17:32
@stephenjason89
Copy link
Author

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.

@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!

@ljharb
Copy link
Member

ljharb commented Apr 8, 2025

Thanks for confirming.

The test failures are because your changes have broken the rule's behavior. One thing to look at is that until resolve supports exports, we can only safely autofix relative/local files.

I can't take ownership of this as-is, unfortunately.

@stephenjason89 stephenjason89 force-pushed the feat/extensions-autofix branch 3 times, most recently from 6f11cda to e5443f8 Compare April 8, 2025 20:16
@stephenjason89
Copy link
Author

stephenjason89 commented Apr 8, 2025

@ljharb Thanks for the reply. I have further reduced the failing tests to 12 now. I also need your feedback on reimplementing getModifier since it falls back to props.defaultConfig which is never.

Problem is, if we add a rule like

       "import/extensions": ["error", {
                "pattern": {
                    "js": "always",
                },
            }],

All the other extensions not defined in pattern falls back to never which is not ideal. If ts ,for example, is not defined then i would expect it to behave without this rule applied. I should explicitly opt in to this rule by defining always or never.

Please help on the remaining failing tests as well. I'm a bit stuck on the remaining tests.

Thank you

@stephenjason89 stephenjason89 force-pushed the feat/extensions-autofix branch from e5443f8 to 7d124ee Compare April 9, 2025 17:11
Copy link

codecov bot commented Apr 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.29%. Comparing base (da5f6ec) to head (b8e7370).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stephenjason89 stephenjason89 force-pushed the feat/extensions-autofix branch 3 times, most recently from 49381fd to e6458bf Compare April 9, 2025 17:58
@stephenjason89
Copy link
Author

@ljharb, I’ve reworked my pull request to include a fix prop for backward compatibility.
The first implementtion where autofixing was the default behavior was triggering several test failures—I’ve resolved nearly all of them, with nine remaining. But attempting to resolve them all bloated the PR.

This new version should be much more targeted and should be easier to review.

@stephenjason89 stephenjason89 force-pushed the feat/extensions-autofix branch from 93d3bfe to b8e7370 Compare April 10, 2025 15:48
@stephenjason89
Copy link
Author

@ljharb I've rebased and added tests to increase coverage. Please re-review and merge if it looks good.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants