eslint-plugin-import icon indicating copy to clipboard operation
eslint-plugin-import copied to clipboard

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

Open stephenjason89 opened this issue 8 months ago • 9 comments

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.

stephenjason89 avatar Apr 08 '25 10:04 stephenjason89

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!

stephenjason89 avatar Apr 08 '25 17:04 stephenjason89

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.

ljharb avatar Apr 08 '25 17:04 ljharb

@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 avatar Apr 08 '25 20:04 stephenjason89

Codecov Report

:x: Patch coverage is 92.59259% with 2 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 95.34%. Comparing base (da5f6ec) to head (8cd047b). :warning: Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/rules/extensions.js 92.59% 2 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3177      +/-   ##
==========================================
- Coverage   95.52%   95.34%   -0.19%     
==========================================
  Files          83       83              
  Lines        3690     3715      +25     
  Branches     1333     1346      +13     
==========================================
+ Hits         3525     3542      +17     
- Misses        165      173       +8     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

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

codecov[bot] avatar Apr 09 '25 17:04 codecov[bot]

@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 avatar Apr 09 '25 19:04 stephenjason89

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

Thanks!

stephenjason89 avatar Apr 10 '25 15:04 stephenjason89

@ljharb may I gently follow up on this PR, would love to hear you feedback.

Thanks

stephenjason89 avatar Apr 20 '25 19:04 stephenjason89

@stephenjason89 Any chance you could contribute this to https://github.com/un-ts/eslint-plugin-import-x instead?

If you look at the open PRs on this project, this PR has low chance of getting prioritized.

lucgagan avatar May 01 '25 13:05 lucgagan

@lucgagan This PR has an absolute certainty of being prioritized, because it is already prioritized. I’m just busy. I will get back to it once i have time. Please be less entitled.

ljharb avatar May 02 '25 04:05 ljharb