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(eslint-plugin): [no-restricted-imports] support import = require #7709

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 22 additions & 12 deletions packages/eslint-plugin/docs/rules/no-restricted-imports.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ description: 'Disallow specified modules when loaded by `import`.'
>
> See **https://typescript-eslint.io/rules/no-restricted-imports** for documentation.

This rule extends the base [`eslint/no-restricted-imports`](https://eslint.org/docs/rules/no-restricted-imports) rule.
This rule extends the base [`eslint/no-restricted-imports`](https://eslint.org/docs/rules/no-restricted-imports) rule. It adds support for the type import (`import type X from "..."`, `import { type X } from "..."`) and `import x = require("...")` syntaxes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Praise] Yes! I've started souring on extension docs that don't explicitly say what gets added.


## Options

Expand All @@ -19,17 +19,27 @@ This rule adds the following options:
You can specify this option for a specific path or pattern as follows:

```jsonc
"@typescript-eslint/no-restricted-imports": ["error", {
"paths": [{
"name": "import-foo",
"message": "Please use import-bar instead.",
"allowTypeImports": true
}, {
"name": "import-baz",
"message": "Please use import-quux instead.",
"allowTypeImports": true
}]
}]
{
"rules": {
"@typescript-eslint/no-restricted-imports": [
"error",
{
"paths": [
{
"name": "import-foo",
"message": "Please use import-bar instead.",
"allowTypeImports": true
},
{
"name": "import-baz",
"message": "Please use import-quux instead.",
"allowTypeImports": true
}
]
}
]
}
}
```

When set to `true`, the rule will allow [Type-Only Imports](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export).
Expand Down
61 changes: 44 additions & 17 deletions packages/eslint-plugin/src/rules/no-restricted-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,28 +269,55 @@ export default createRule<Options, MessageIds>({
);
}

return {
ImportDeclaration(node: TSESTree.ImportDeclaration): void {
function checkImportNode(node: TSESTree.ImportDeclaration): void {
if (
node.importKind === 'type' ||
(node.specifiers.length > 0 &&
node.specifiers.every(
specifier =>
specifier.type === AST_NODE_TYPES.ImportSpecifier &&
specifier.importKind === 'type',
))
) {
const importSource = node.source.value.trim();
if (
node.importKind === 'type' ||
(node.specifiers.length > 0 &&
node.specifiers.every(
specifier =>
specifier.type === AST_NODE_TYPES.ImportSpecifier &&
specifier.importKind === 'type',
))
!isAllowedTypeImportPath(importSource) &&
!isAllowedTypeImportPattern(importSource)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Cleanup] Nit: there's a teeny bit of duplication with calling !isAllowedTypeImportPath(importSource) && !isAllowedTypeImportPattern(importSource). Maybe worth a helper function? It's only two places, so maybe not...

) {
const importSource = node.source.value.trim();
if (
!isAllowedTypeImportPath(importSource) &&
!isAllowedTypeImportPattern(importSource)
) {
return rules.ImportDeclaration(node);
}
} else {
return rules.ImportDeclaration(node);
}
} else {
return rules.ImportDeclaration(node);
}
}

return {
TSImportEqualsDeclaration(
node: TSESTree.TSImportEqualsDeclaration,
): void {
if (
node.moduleReference.type ===
AST_NODE_TYPES.TSExternalModuleReference &&
node.moduleReference.expression.type === AST_NODE_TYPES.Literal &&
typeof node.moduleReference.expression.value === 'string'
) {
const synthesizedImport = {
...node,
type: AST_NODE_TYPES.ImportDeclaration,
source: node.moduleReference.expression,
assertions: [],
specifiers: [
{
...node.id,
type: AST_NODE_TYPES.ImportDefaultSpecifier,
local: node.id,
},
],
} satisfies TSESTree.ImportDeclaration;
return checkImportNode(synthesizedImport);
}
},
ImportDeclaration: checkImportNode,
'ExportNamedDeclaration[source]'(
node: TSESTree.ExportNamedDeclaration & {
source: NonNullable<TSESTree.ExportNamedDeclaration['source']>;
Expand Down
61 changes: 61 additions & 0 deletions packages/eslint-plugin/tests/rules/no-restricted-imports.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,16 @@ const ruleTester = new RuleTester({
ruleTester.run('no-restricted-imports', rule, {
valid: [
"import foo from 'foo';",
"import foo = require('foo');",
"import 'foo';",
{
code: "import foo from 'foo';",
options: ['import1', 'import2'],
},
{
code: "import foo = require('foo');",
options: ['import1', 'import2'],
},
{
code: "export { foo } from 'foo';",
options: ['import1', 'import2'],
Expand Down Expand Up @@ -147,6 +152,20 @@ ruleTester.run('no-restricted-imports', rule, {
},
],
},
{
code: "import foo = require('foo');",
options: [
{
paths: [
{
name: 'foo',
importNames: ['foo'],
message: 'Please use Bar from /import-bar/baz/ instead.',
},
],
},
],
},
{
code: "import type foo from 'import-foo';",
options: [
Expand All @@ -161,6 +180,20 @@ ruleTester.run('no-restricted-imports', rule, {
},
],
},
{
code: "import type _ = require('import-foo');",
options: [
{
paths: [
{
name: 'import-foo',
message: 'Please use import-bar instead.',
allowTypeImports: true,
},
],
},
],
},
{
code: "import type { Bar } from 'import-foo';",
options: [
Expand Down Expand Up @@ -301,6 +334,15 @@ import type { foo } from 'import2/private/bar';
},
],
},
{
code: "import foo = require('import1');",
options: ['import1', 'import2'],
errors: [
{
messageId: 'path',
},
],
},
{
code: "export { foo } from 'import1';",
options: ['import1', 'import2'],
Expand Down Expand Up @@ -552,6 +594,25 @@ import type { foo } from 'import2/private/bar';
},
],
},
{
code: "import foo = require('import-foo');",
options: [
{
paths: [
{
name: 'import-foo',
message: 'Please use import-bar instead.',
allowTypeImports: true,
},
],
},
],
errors: [
{
messageId: 'pathWithCustomMessage',
},
],
},
{
code: "import { Bar } from 'import-foo';",
options: [
Expand Down