fix(linter): fix false positives reported when analyzing package imports

[taearls]

Closes #12220

I refined the logic for this rule to address false positives that were reported.

The key improvements include:

1. Path Alias Detection: Introduced a new is_package_import() helper function in the rule that correctly distinguishes between path aliases (@/, ~/, #/) and scoped npm packages (e.g., @org/pkg). Previously, the rule incorrectly treated single-character path aliases as package imports, leading to false positives where valid relative imports were flagged incorrectly.
2. Configuration Inheritance Fix: the ignorePackages setting wasn't properly interacting with per-extension configurations. The rule now correctly requires extensions for relative imports when per-extension configs inherit ignorePackages, unless explicitly overridden to never.
3. Package Subpath Handling: Improved detection of package subpaths (e.g., lodash/fp, @babel/core/lib) to treat them as packages rather than files requiring extensions, aligning with ESLint's behavior and JavaScript module resolution semantics.
4. Added new test cases validating the fixes across various import patterns, configuration combinations, and edge cases.

[codspeed-hq[bot]]

CodSpeed Performance Report

Merging #14602 will not alter performance

_{Comparing taearls:linter/fix/import-extensions-false-positive (3e79d4b) with main (e346426)}

Summary

✅ 4 untouched
⏩ 41 skipped[^skipped]

[^skipped]: 41 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

[taearls]

Would it be better to use use the module record for this.

code like:

    // Check for path aliases with single-char prefix followed by /
    // Examples: @/, ~/, #/
    if module_name.len() >= 2 {
        let chars: Vec<char> = module_name.chars().collect();
        if chars.len() >= 2 && chars[1] == '/' {
            // Single character before slash - this is a path alias
            return false;
        }
    }

feels flaky

yeah that makes sense. I'll work on refactoring this to use the module record.

Update:

after looking at it a bit more, I couldn't find any additional contextual information in the module record (aside from the module's import name that is already being used) that I could leverage to my benefit. I did update the helper function this snippet is in to be a little cleaner and more clear with its intentions.

as far as I can tell, the linter doesn't support custom resolver configurations in tsconfig.json yet, but I think this fixes the specific bugs that were reported in the issue. there are still some gaps though I believe. any custom aliases or typescript paths that are configured in a user's tsconfig.json will not be respected yet.

however, I think adding full resolver config support deserves its own dedicated issue / PR because the filesystem lookups to resolve the tsconfig.json (and to safely handle if it doesn't exist) would have performance implications.

maybe I'm missing something though. let me know if you have any suggestions in terms of how I can use the module record better here

[connorshea]

This MR does fix various issues in the reproduction repo here.

The only things it doesn't fix are main4.js and main5.js but we can try to handle those in a different PR.

main4.js should fail because I have the config set as "import/extensions": ["error", "always", { "ignorePackages": true }], and main4 has import foo from "./foo"; in it. Weirdly, if it doesn't have the console.log(foo) line after the import, it does trigger the lint error. So that's weird.

linting output on the repro repo for eslint vs main vs this branch

oxlint 1.26.0 (3 errors, and 1 error missing that is expected, based on what ESLint does):

connorshea@Connors-MacBook-Pro oxlint-extensions-reproduction % pnpm oxlint

  × eslint-plugin-import(extensions): File extension "JS" should not be included in the import declaration.
   ╭─[main5.js:3:1]
 2 │ // ESLint passes on this, oxlint does not currently.
 3 │ import foo from './foo.JS';
   · ───────────────────────────
 4 │ 
   ╰────
  help: Remove the file extension from this import.

  × eslint-plugin-import(extensions): Missing file extension in import declaration
   ╭─[main1.js:4:1]
 3 │ // We have `ignorePackages` set to true, so this should not error.
 4 │ import { defineConfig } from "vitest/config";
   · ─────────────────────────────────────────────
 5 │ 
   ╰────
  help: Add a file extension to this import.

  × eslint-plugin-import(extensions): Missing file extension in import declaration
   ╭─[main2.js:3:1]
 2 │ // This behavior is not correct because I have `ignorePackages` set to true, so it should be... ignored.
 3 │ import { defineConfig } from "vitest/config";
   · ─────────────────────────────────────────────
 4 │ 
   ╰────
  help: Add a file extension to this import.

Found 0 warnings and 3 errors.
Finished in 49ms on 7 files using 8 threads.

This PR (after being rebased on top of main, 1 error but it's different from ESLint's error):

connorshea@Connors-MacBook-Pro oxlint % node dist/cli.js -c ../../../oxlint-extensions-reproduction/.oxlintrc.json ~/code/oxlint-extensions-reproduction/

  × eslint-plugin-import(extensions): File extension "JS" should not be included in the import declaration.
   ╭─[/Users/connorshea/code/oxlint-extensions-reproduction/main5.js:3:1]
 2 │ // ESLint passes on this, oxlint does not currently.
 3 │ import foo from './foo.JS';
   · ───────────────────────────
 4 │ 
   ╰────
  help: Remove the file extension from this import.

Found 0 warnings and 1 error.
Finished in 324ms on 7 files with 1 rules using 8 threads.

ESLint (1 error):

connorshea@Connors-MacBook-Pro oxlint-extensions-reproduction % pnpm eslint

/Users/connorshea/code/oxlint-extensions-reproduction/main4.js
  5:17  error  Missing file extension "js" for "./foo"  import/extensions

✖ 1 problem (1 error, 0 warnings)

---

Aside on main5.js:

I'm not sure if we should necessarily downcase the extension filename to fix main5,js there? And technically the difference may be entirely based on eslint allowing any extensions not-defined-by-the-config by default, while oxlint doesn't? So if it's because 'JS' is technically a distinct extension from 'js', then maybe we would fix this behavior in a different way. :shrug:

Regardless, I don't think that edge case needs to be fixed in this MR. I do think we should consider adding a bunch of fixtures for this rule, to confirm various complex behaviors, but that can also be done in a separate PR imo.

[connorshea]

This definitely seems to improve the number of discrepancies between oxlint and ESLint, based on testing it on the Mastodon repo.

1.26.0 on mastodon: 169 errors this PR (after rebase) on mastodon: 120 errors

However, I think more work will need to be done to fix further issues with this rule, as things like this are showing up as violations still after this PR, despite not being violations (config is always at the top-level, then never for importing tsx files):

x eslint-plugin-import(extensions): Missing file extension in import declaration
   ,-[app/javascript/mastodon/features/emoji/emoji_picker.tsx:7:1]
 6 | 
 7 | import { EMOJI_MODE_NATIVE } from './constants';
   : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 8 | import EmojiData from './emoji_data.json';
   `----
  help: Add a file extension to this import.

See emoji_picker.tsx. The constants file being imported is at app/javascript/mastodon/features/emoji/constants.ts, and so should be enforced to never have an extension. Yet the rule still enforces it.

I'm wondering if this rule actually correctly has the ability to check the source file's type when it's being imported?

The .oxlintrc.json in mastodon I'm testing with is this, if anyone wants to reproduce this behavior:

oxlintrc.json

{
  "$schema": "./node_modules/oxlint/configuration_schema.json",
  "plugins": [
    "import"
  ],
  "categories": {
    "correctness": "off"
  },
  "env": {
    "builtin": true,
    "es2021": true,
    "browser": true,
    "shared-node-browser": true
  },
  "rules": {
    "import/extensions": [
      "error",
      "always",
      {
        "js": "never",
        "jsx": "never",
        "ts": "never",
        "tsx": "never"
      }
    ]
  },
  "ignorePatterns": [
    "build/**/*",
    "coverage/**/*",
    "db/**/*",
    "lib/**/*",
    "log/**/*",
    "node_modules/**/*",
    "public/**/*",
    "!public/embed.js",
    "spec/**/*",
    "tmp/**/*",
    "vendor/**/*",
    "streaming/**/*",
    ".bundle/**/*",
    "storybook-static/**/*"
  ]
}

[taearls]

This definitely seems to improve the number of discrepancies between oxlint and ESLint, based on testing it on the Mastodon repo.

1.26.0 on mastodon: 169 errors this PR (after rebase) on mastodon: 120 errors

However, I think more work will need to be done to fix further issues with this rule, as things like this are showing up as violations still after this PR, despite not being violations (config is always at the top-level, then never for importing tsx files):

x eslint-plugin-import(extensions): Missing file extension in import declaration
   ,-[app/javascript/mastodon/features/emoji/emoji_picker.tsx:7:1]
 6 | 
 7 | import { EMOJI_MODE_NATIVE } from './constants';
   : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 8 | import EmojiData from './emoji_data.json';
   `----
  help: Add a file extension to this import.

See emoji_picker.tsx. The constants file being imported is at app/javascript/mastodon/features/emoji/constants.ts, and so should be enforced to never have an extension. Yet the rule still enforces it.

I'm wondering if this rule actually correctly has the ability to check the source file's type when it's being imported?

The .oxlintrc.json in mastodon I'm testing with is this, if anyone wants to reproduce this behavior:

oxlintrc.json

{
  "$schema": "./node_modules/oxlint/configuration_schema.json",
  "plugins": [
    "import"
  ],
  "categories": {
    "correctness": "off"
  },
  "env": {
    "builtin": true,
    "es2021": true,
    "browser": true,
    "shared-node-browser": true
  },
  "rules": {
    "import/extensions": [
      "error",
      "always",
      {
        "js": "never",
        "jsx": "never",
        "ts": "never",
        "tsx": "never"
      }
    ]
  },
  "ignorePatterns": [
    "build/**/*",
    "coverage/**/*",
    "db/**/*",
    "lib/**/*",
    "log/**/*",
    "node_modules/**/*",
    "public/**/*",
    "!public/embed.js",
    "spec/**/*",
    "tmp/**/*",
    "vendor/**/*",
    "streaming/**/*",
    ".bundle/**/*",
    "storybook-static/**/*"
  ]
}

Good to know! Thanks for the detailed report. I can take a look this week into this further, but I'll probably do it in a separate development branch because I suspect it will be a more significant refactor to get all those things working.

I don't think the file extension parsing is working how we would expect. When I initially implemented this rule, I basically just finished once I got all the tests to pass, but there have been a few problems with this rule.

I agree that we should add more testing to verify the behavior. It deserves a closer look for sure.

[connorshea]

See also #15009, not sure which is the best to go with as the solution here but I definitely think we need to prioritize fixing this rule.

[taearls]

I'm not sure either. From the discord conversation I'm also working on a separate branch that I was planning to point to this one when it's ready that will support custom extensions (i.e., not hard coded ones).

[taearls]

I discussed and tested this rule with @connorshea on discord and pushed up some changes that will significantly improve the behavior of this rule significantly.

the key highlights:

• I updated the parsing logic so that if a file extension is not included in the configuration, and the global setting is NOT never, then this rule will not emit diagnostics about files with that extension. this makes it so users get custom file extension support without needing to manually configure potentially dozens of file extensions in their config. Users can incrementally flag what they want and have fine control over what extensions are allowed.

• the parsing logic uses the ModuleRecord struct rather than string matching against the source code. this means that we'll be able to check against the actual file extension in in the filesystem rather than simply performing static analysis on source code.

• I removed the hard coded file extension configurations and am using an FxHashMap to build up a mapping based on the user's configuration.

• I used bitflags to denote always | never | ignorePackages so that the enum is memory efficient.

• added support for the pathGroupOverrides configuration option

I'm planning to review it more thoroughly over the next couple of days to make sure everything looks good and that I didn't miss anything, but meanwhile I think it's in a good enough place for review.

[taearls]

I discussed and tested this rule with @connorshea on discord and pushed up some changes that will significantly improve the behavior of this rule significantly.

the key highlights:

• I updated the parsing logic so that if a file extension is not included in the configuration, and the global setting is NOT never, then this rule will not emit diagnostics about files with that extension. this makes it so users get custom file extension support without needing to manually configure potentially dozens of file extensions in their config. Users can incrementally flag what they want and have fine control over what extensions are allowed.
• the parsing logic uses the ModuleRecord struct rather than string matching against the source code. this means that we'll be able to check against the actual file extension in in the filesystem rather than simply performing static analysis on source code.
• I removed the hard coded file extension configurations and am using an FxHashMap to build up a mapping based on the user's configuration.
• I used bitflags to denote always | never | ignorePackages so that the enum is memory efficient.
• added support for the pathGroupOverrides configuration option

I'm planning to review it more thoroughly over the next couple of days to make sure everything looks good and that I didn't miss anything, but meanwhile I think it's in a good enough place for review.

I'm feeling good about these changes after giving them a quick review and updating the PR description.