feat(eslint-plugin): [naming-convention] add support for default and namespace imports

[binoche9]

PR Checklist

• Addresses an existing open issue: fixes [naming-convention] Add the ability to lint imports #2106
• That issue was marked as accepting prs
• Steps in Contributing were taken

Overview

Currently the naming-convention rule cannot be used to enforce conventions on default and namespace imports. This PR introduces that capability by introducing an import selector and default and namespace modifiers.

Named imports are not checked since they have to match the name from the imported file.

[typescript-eslint]

Thanks for the PR, @binoche9!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

---

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

[netlify]

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	bc92533
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/64d19e5a6f988700073ad170
😎 Deploy Preview	https://deploy-preview-7269--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit bc92533. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 37 targets

• nx test eslint-plugin --coverage=false
• Node 20 - nx test eslint-plugin
• Node 16 - nx test eslint-plugin --coverage=false
• nx build website
• nx run-many --target=typecheck --parallel
• nx run-many --target=lint --parallel
• nx test typescript-estree --coverage=false
• Node 20 - nx test visitor-keys
• nx run types:build
• Node 20 - nx run types:build
• nx test eslint-plugin-tslint --coverage=false
• Node 20 - nx test typescript-estree
• Node 20 - nx test utils
• Node 20 - nx test rule-schema-to-typescript-types
• nx test eslint-plugin-internal --coverage=false
• Node 20 - nx test scope-manager
• Node 20 - nx test type-utils
• Node 16 - nx test scope-manager --coverage=false
• Node 16 - nx run types:build
• Node 16 - nx test repo-tools --coverage=false
• Node 20 - nx test repo-tools
• Node 16 - nx test ast-spec --coverage=false
• Node 16 - nx test typescript-estree --coverage=false
• Node 16 - nx test rule-schema-to-typescript-types --coverage=false
• Node 16 - nx test visitor-keys --coverage=false
• Node 16 - nx test eslint-plugin-tslint --coverage=false
• Node 20 - nx test eslint-plugin-tslint
• Node 16 - nx test type-utils --coverage=false
• nx run integration-tests:test
• Node 20 - nx test parser
• Node 20 - nx test eslint-plugin-internal
• Node 20 - nx test ast-spec
• Node 16 - nx test parser --coverage=false
• Node 16 - nx test utils --coverage=false
• Node 16 - nx test eslint-plugin-internal --coverage=false
• nx run eslint-plugin:generate:configs
• nx run-many --target=build --parallel --exclude=website --exclude=website-eslint

---

Sent with 💌 from NxCloud.

[binoche9]

Let me know if there's any other place I should be making changes.

I'm also not sure if this counts as a breaking change since for people who have a default style it may break for them since imports that previously weren't matched will start getting the default style enforced.

[binoche9]

I decided it didn't belong to any selector group (I guess variableLike comes closest) but feel free to disagree

[binoche9]

I decided import { default as Foo } counts as a default import, but feel free to disagree

[JoshuaKGoldberg]

Generally looks great to me, thanks for sending in! 🙌

Just requesting changes on splitting the tests up. It all strikes me as reasonable otherwise though.

cc @bradzacher / will defer to you since you'd interacted with the feature request in #2106.

[codecov]

Codecov Report

Merging #7269 (bc92533) into main (0a1de14) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7269      +/-   ##
==========================================
+ Coverage   87.41%   87.42%   +0.01%     
==========================================
  Files         381      381              
  Lines       13321    13332      +11     
  Branches     3938     3940       +2     
==========================================
+ Hits        11645    11656      +11     
  Misses       1298     1298              
  Partials      378      378              

Flag	Coverage Δ
unittest	87.42% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
...-plugin/src/rules/naming-convention-utils/enums.ts	100.00% <ø> (ø)
...plugin/src/rules/naming-convention-utils/schema.ts	100.00% <ø> (ø)
...kages/eslint-plugin/src/rules/naming-convention.ts	90.68% <100.00%> (+0.53%)	⬆️

[binoche9]

I split up the tests as requested 👍

[binoche9]

Friendly bump here!

[JoshuaKGoldberg]

Sorry for the delay! I just re-reviewed and it looks great. Thanks again for sending!

[karlhorky]

Just upgraded to @​typescript-eslint/eslint-plugin@6.9.0 and received a number of new warnings, such as this one:

checkbox/index.js
Warning:   1:8  warning  Import name `React` must match one of the following formats: camelCase  @typescript-eslint/naming-convention

With this code:

import React from 'react';

This is using the default rule configuration mentioned in the docs:

// the default config is similar to ESLint's camelcase rule but more strict
const defaultOptions: Options = [
  {
    selector: 'default',
    format: ['camelCase'],
    leadingUnderscore: 'allow',
    trailingUnderscore: 'allow',
  },

  {
    selector: 'variable',
    format: ['camelCase', 'UPPER_CASE'],
    leadingUnderscore: 'allow',
    trailingUnderscore: 'allow',
  },

  {
    selector: 'typeLike',
    format: ['PascalCase'],
  },
];

I guess this is going to be a pretty common pattern that gets rejected now. 😬

Maybe the default configuration should accept every format as an import? (format: null)

Alternative: expect all users with default PascalCase imports (pretty common across the npm ecosystem, incl. React) to add the following to their config:

// the default config is similar to ESLint's camelcase rule but more strict
const defaultOptions: Options = [
  // ...

+  // Disable @typescript-eslint/naming-convention format for imports
+  // https://github.com/typescript-eslint/typescript-eslint/pull/7269#issuecomment-1777628591
+  {
+    selector: 'import',
+    format: null,
+  },
];

[JoshuaKGoldberg]

Hm good point @binoche9. Could I ask you to file a new issue for this please, for tracking and attribution?

[karlhorky]

@JoshuaKGoldberg @binoche9 opened an issue here:

• Bug: [naming-convention] Default import config clashes with common npm import patterns (PascalCase and other-cased imports) #7838