From 55f6277def88835a09db408015b4f1ada4f32349 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 18 Jun 2019 13:15:32 -0700 Subject: [PATCH 1/6] feat(eslint-plugin): added no-reference-import rule --- packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-reference-import.ts | 73 +++++++++++++++++++ .../tests/rules/no-reference-import.test.ts | 49 +++++++++++++ 3 files changed, 124 insertions(+) create mode 100644 packages/eslint-plugin/src/rules/no-reference-import.ts create mode 100644 packages/eslint-plugin/tests/rules/no-reference-import.test.ts diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 3b799b1cc80..d12e6542a10 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -31,6 +31,7 @@ import noNamespace from './no-namespace'; import noNonNullAssertion from './no-non-null-assertion'; import noObjectLiteralTypeAssertion from './no-object-literal-type-assertion'; import noParameterProperties from './no-parameter-properties'; +import noReferenceImport from './no-reference-import'; import noRequireImports from './no-require-imports'; import noThisAlias from './no-this-alias'; import noTripleSlashReference from './no-triple-slash-reference'; @@ -90,6 +91,7 @@ export default { 'no-non-null-assertion': noNonNullAssertion, 'no-object-literal-type-assertion': noObjectLiteralTypeAssertion, 'no-parameter-properties': noParameterProperties, + 'no-reference-import': noReferenceImport, 'no-require-imports': noRequireImports, 'no-this-alias': noThisAlias, 'no-triple-slash-reference': noTripleSlashReference, diff --git a/packages/eslint-plugin/src/rules/no-reference-import.ts b/packages/eslint-plugin/src/rules/no-reference-import.ts new file mode 100644 index 00000000000..1a606486c41 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-reference-import.ts @@ -0,0 +1,73 @@ +import * as util from '../util'; +import { AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; +import { + Literal, + Node, + TSExternalModuleReference, +} from '@typescript-eslint/typescript-estree/dist/ts-estree/ts-estree'; + +export default util.createRule({ + name: 'no-reference-import', + meta: { + type: 'suggestion', + docs: { + description: + 'Disallow `/// ` comments when already using an import declaration.', + category: 'Best Practices', + recommended: 'error', + }, + schema: [], + messages: { + noReferenceImport: 'Do not reference {{module}} if importing it anyway.', + }, + }, + defaultOptions: [], + create(context) { + let programNode: Node; + + function hasMatchingReference(source: Literal) { + const referenceRegExp = /^\/\s* { + if (comment.type !== 'Line') { + return; + } + const referenceResult = referenceRegExp.exec(comment.value); + + if (referenceResult && source.value === referenceResult[1]) { + context.report({ + node: comment, + messageId: 'noReferenceImport', + data: { + module: referenceResult[1], + }, + }); + } + }); + } + return { + ImportDeclaration(node) { + if (node.type === AST_NODE_TYPES.ImportDeclaration) { + if (programNode) { + let source = node.source as Literal; + hasMatchingReference(source); + } + } + }, + TSImportEqualsDeclaration(node) { + if (node.type === AST_NODE_TYPES.TSImportEqualsDeclaration) { + if (programNode) { + let source = (node.moduleReference as TSExternalModuleReference) + .expression as Literal; + hasMatchingReference(source); + } + } + }, + Program(node) { + programNode = node; + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-reference-import.test.ts b/packages/eslint-plugin/tests/rules/no-reference-import.test.ts new file mode 100644 index 00000000000..2bfc15d0476 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-reference-import.test.ts @@ -0,0 +1,49 @@ +import rule from '../../src/rules/no-reference-import'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parserOptions: { + sourceType: 'module', + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-reference-import', rule, { + valid: [ + `/// + var foo = require("foo");`, + `/// + import * from "foo"`, + ], + invalid: [ + { + code: ` +/// +import * from "foo" + `, + + parser: '@typescript-eslint/parser', + errors: [ + { + messageId: 'noReferenceImport', + line: 2, + column: 1, + }, + ], + }, + { + code: ` +/// +import foo = require("foo"); + `, + parser: '@typescript-eslint/parser', + errors: [ + { + messageId: 'noReferenceImport', + line: 2, + column: 1, + }, + ], + }, + ], +}); From b675bc8478e5c589df7e2d16585ec62928f9f887 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 18 Jun 2019 16:43:50 -0700 Subject: [PATCH 2/6] docs: added docs for no-reference-import --- packages/eslint-plugin/README.md | 1 + .../docs/rules/no-reference-import.md | 37 +++++++++++++++++++ .../src/rules/no-reference-import.ts | 4 +- .../tests/rules/no-reference-import.test.ts | 4 +- 4 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/no-reference-import.md diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 68ff8434d8a..4d3e4c308e8 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -157,6 +157,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator | :heavy_check_mark: | | | | [`@typescript-eslint/no-object-literal-type-assertion`](./docs/rules/no-object-literal-type-assertion.md) | Forbids an object literal to appear in a type assertion expression | :heavy_check_mark: | | | | [`@typescript-eslint/no-parameter-properties`](./docs/rules/no-parameter-properties.md) | Disallow the use of parameter properties in class constructors | :heavy_check_mark: | | | +| [`@typescript-eslint/no-reference-import`](./docs/rules/no-reference-import.md) | Disallow simultaneous use of `/// ` comments and ES6 style imports for the same module | | | | | [`@typescript-eslint/no-require-imports`](./docs/rules/no-require-imports.md) | Disallows invocation of `require()` | | | | | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` | | | | | [`@typescript-eslint/no-triple-slash-reference`](./docs/rules/no-triple-slash-reference.md) | Disallow `/// ` comments | :heavy_check_mark: | | | diff --git a/packages/eslint-plugin/docs/rules/no-reference-import.md b/packages/eslint-plugin/docs/rules/no-reference-import.md new file mode 100644 index 00000000000..05a7db9dcae --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-reference-import.md @@ -0,0 +1,37 @@ +# Disallow simultaneous use of `/// ` comments and ES6 style imports for the same module. (no-reference-import) + +Use of triple-slash directives is discouraged in favor of the newer `import` style. In cases where both styles might occur, this rule prevents use of triple-slash references for modules which are otherwise imported. + +Use `no-triple-slash-reference` instead if you intend to ban triple slash directives entirely. + +## Rule Details + +Examples of **incorrect** code for this rule: + +```ts +/// +import * as foo from 'foo'; +``` + +```ts +/// +import foo = require('foo'); +``` + +Examples of **correct** code for this rule: + +```ts +import * as foo from 'foo'; +``` + +```ts +import foo = require('foo'); +``` + +## When To Use It + +Any time you might use triple-slash directives and ES6 import declarations in the same file. + +## When Not To Use It + +If you intend to ban triple slash directives entirely. diff --git a/packages/eslint-plugin/src/rules/no-reference-import.ts b/packages/eslint-plugin/src/rules/no-reference-import.ts index 1a606486c41..b7bb7036766 100644 --- a/packages/eslint-plugin/src/rules/no-reference-import.ts +++ b/packages/eslint-plugin/src/rules/no-reference-import.ts @@ -12,9 +12,9 @@ export default util.createRule({ type: 'suggestion', docs: { description: - 'Disallow `/// ` comments when already using an import declaration.', + 'Disallow simultaneous use of `/// ` comments and ES6 style imports for the same module', category: 'Best Practices', - recommended: 'error', + recommended: false, }, schema: [], messages: { diff --git a/packages/eslint-plugin/tests/rules/no-reference-import.test.ts b/packages/eslint-plugin/tests/rules/no-reference-import.test.ts index 2bfc15d0476..20ddd2b94c6 100644 --- a/packages/eslint-plugin/tests/rules/no-reference-import.test.ts +++ b/packages/eslint-plugin/tests/rules/no-reference-import.test.ts @@ -13,13 +13,13 @@ ruleTester.run('no-reference-import', rule, { `/// var foo = require("foo");`, `/// - import * from "foo"`, + import * as foo from "foo"`, ], invalid: [ { code: ` /// -import * from "foo" +import * as foo from "foo" `, parser: '@typescript-eslint/parser', From ac3cd5da697ba7d38780fd2cd283bd50d9bcda1d Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Wed, 19 Jun 2019 14:51:45 -0700 Subject: [PATCH 3/6] fix(eslint-plugin): collect references when visiting the program node --- .../src/rules/no-reference-import.ts | 55 ++++++++++--------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-reference-import.ts b/packages/eslint-plugin/src/rules/no-reference-import.ts index b7bb7036766..7a7fee23a22 100644 --- a/packages/eslint-plugin/src/rules/no-reference-import.ts +++ b/packages/eslint-plugin/src/rules/no-reference-import.ts @@ -1,10 +1,10 @@ import * as util from '../util'; -import { AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; import { Literal, Node, TSExternalModuleReference, } from '@typescript-eslint/typescript-estree/dist/ts-estree/ts-estree'; +import { TSESTree } from '@typescript-eslint/typescript-estree'; export default util.createRule({ name: 'no-reference-import', @@ -24,24 +24,20 @@ export default util.createRule({ defaultOptions: [], create(context) { let programNode: Node; + const sourceCode = context.getSourceCode(); + const references: ({ + comment: TSESTree.Comment; + importName: string; + })[] = []; function hasMatchingReference(source: Literal) { - const referenceRegExp = /^\/\s* { - if (comment.type !== 'Line') { - return; - } - const referenceResult = referenceRegExp.exec(comment.value); - - if (referenceResult && source.value === referenceResult[1]) { + references.forEach(reference => { + if (reference.importName === source.value) { context.report({ - node: comment, + node: reference.comment, messageId: 'noReferenceImport', data: { - module: referenceResult[1], + module: reference.importName, }, }); } @@ -49,24 +45,33 @@ export default util.createRule({ } return { ImportDeclaration(node) { - if (node.type === AST_NODE_TYPES.ImportDeclaration) { - if (programNode) { - let source = node.source as Literal; - hasMatchingReference(source); - } + if (programNode) { + const source = node.source as Literal; + hasMatchingReference(source); } }, TSImportEqualsDeclaration(node) { - if (node.type === AST_NODE_TYPES.TSImportEqualsDeclaration) { - if (programNode) { - let source = (node.moduleReference as TSExternalModuleReference) - .expression as Literal; - hasMatchingReference(source); - } + if (programNode) { + const source = (node.moduleReference as TSExternalModuleReference) + .expression as Literal; + hasMatchingReference(source); } }, Program(node) { programNode = node; + const referenceRegExp = /^\/\s* { + if (comment.type !== 'Line') { + return; + } + const referenceResult = referenceRegExp.exec(comment.value); + + if (referenceResult && referenceResult[1]) { + references.push({ comment, importName: referenceResult[1] }); + } + }); }, }; }, From ecc65e11b61a2c2ff845558ea07722bdeb4ddd0e Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 21 Jun 2019 17:19:48 -0700 Subject: [PATCH 4/6] feat(eslint-plugin): updated rule to cover more reference directives --- packages/eslint-plugin/README.md | 2 +- .../docs/rules/no-reference-import.md | 37 ----- .../docs/rules/triple-slash-reference.md | 58 ++++++++ packages/eslint-plugin/src/rules/index.ts | 4 +- .../src/rules/no-reference-import.ts | 78 ----------- .../src/rules/no-triple-slash-reference.ts | 4 +- .../src/rules/triple-slash-reference.ts | 129 ++++++++++++++++++ .../tests/rules/no-reference-import.test.ts | 49 ------- .../rules/no-triple-slash-reference.test.ts | 4 +- .../rules/triple-slash-reference.test.ts | 122 +++++++++++++++++ 10 files changed, 316 insertions(+), 171 deletions(-) delete mode 100644 packages/eslint-plugin/docs/rules/no-reference-import.md create mode 100644 packages/eslint-plugin/docs/rules/triple-slash-reference.md delete mode 100644 packages/eslint-plugin/src/rules/no-reference-import.ts create mode 100644 packages/eslint-plugin/src/rules/triple-slash-reference.ts delete mode 100644 packages/eslint-plugin/tests/rules/no-reference-import.test.ts create mode 100644 packages/eslint-plugin/tests/rules/triple-slash-reference.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 4d3e4c308e8..05d0a88ebcd 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -157,7 +157,6 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator | :heavy_check_mark: | | | | [`@typescript-eslint/no-object-literal-type-assertion`](./docs/rules/no-object-literal-type-assertion.md) | Forbids an object literal to appear in a type assertion expression | :heavy_check_mark: | | | | [`@typescript-eslint/no-parameter-properties`](./docs/rules/no-parameter-properties.md) | Disallow the use of parameter properties in class constructors | :heavy_check_mark: | | | -| [`@typescript-eslint/no-reference-import`](./docs/rules/no-reference-import.md) | Disallow simultaneous use of `/// ` comments and ES6 style imports for the same module | | | | | [`@typescript-eslint/no-require-imports`](./docs/rules/no-require-imports.md) | Disallows invocation of `require()` | | | | | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` | | | | | [`@typescript-eslint/no-triple-slash-reference`](./docs/rules/no-triple-slash-reference.md) | Disallow `/// ` comments | :heavy_check_mark: | | | @@ -178,6 +177,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/require-array-sort-compare`](./docs/rules/require-array-sort-compare.md) | Enforce giving `compare` argument to `Array#sort` | | | :thought_balloon: | | [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string | | | :thought_balloon: | | [`@typescript-eslint/semi`](./docs/rules/semi.md) | Require or disallow semicolons instead of ASI | | :wrench: | | +| [`@typescript-eslint/triple-slash-reference`](./docs/rules/triple-slash-reference.md) | Sets preference level for triple slash directives versus ES6-style import declarations | | | | | [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/unbound-method`](./docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | | | :thought_balloon: | | [`@typescript-eslint/unified-signatures`](./docs/rules/unified-signatures.md) | Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter | | | | diff --git a/packages/eslint-plugin/docs/rules/no-reference-import.md b/packages/eslint-plugin/docs/rules/no-reference-import.md deleted file mode 100644 index 05a7db9dcae..00000000000 --- a/packages/eslint-plugin/docs/rules/no-reference-import.md +++ /dev/null @@ -1,37 +0,0 @@ -# Disallow simultaneous use of `/// ` comments and ES6 style imports for the same module. (no-reference-import) - -Use of triple-slash directives is discouraged in favor of the newer `import` style. In cases where both styles might occur, this rule prevents use of triple-slash references for modules which are otherwise imported. - -Use `no-triple-slash-reference` instead if you intend to ban triple slash directives entirely. - -## Rule Details - -Examples of **incorrect** code for this rule: - -```ts -/// -import * as foo from 'foo'; -``` - -```ts -/// -import foo = require('foo'); -``` - -Examples of **correct** code for this rule: - -```ts -import * as foo from 'foo'; -``` - -```ts -import foo = require('foo'); -``` - -## When To Use It - -Any time you might use triple-slash directives and ES6 import declarations in the same file. - -## When Not To Use It - -If you intend to ban triple slash directives entirely. diff --git a/packages/eslint-plugin/docs/rules/triple-slash-reference.md b/packages/eslint-plugin/docs/rules/triple-slash-reference.md new file mode 100644 index 00000000000..b09fc741097 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/triple-slash-reference.md @@ -0,0 +1,58 @@ +# Sets preference level for triple slash directives versus ES6-style import declarations. (triple-slash-reference) + +Use of triple-slash reference type directives is discouraged in favor of the newer `import` style. This rule allows you to ban use of `/// `, `/// `, or `/// ` directives. + +If you use the `no-triple-slash-reference` rule, consider using this rule instead. + +## Rule Details + +With `{ "path": "never", "types": "never", "lib": "never" }` options set, the following will all be **incorrect** usage: + +```ts +/// +/// +/// +``` + +Examples of **incorrect** code for the `{ "types": "prefer-import" }` option. Note that these are only errors when **both** stlyes are used for the **same** module: + +```ts +/// +import * as foo from 'foo'; +``` + +```ts +/// +import foo = require('foo'); +``` + +With `{ "path": "always", "types": "always", "lib": "always" }` options set, the following will all be **correct** usage: + +```ts +/// +/// +/// +``` + +Examples of **correct** code for the `{ "types": "prefer-import" }` option: + +```ts +import * as foo from 'foo'; +``` + +```ts +import foo = require('foo'); +``` + +## When To Use It + +If you want to ban use of one or all of the triple slash reference directives, or any time you might use triple-slash type reference directives and ES6 import declarations in the same file. + +## When Not To Use It + +If you want to use all flavors of triple slash reference directives. + +## Compatibility + +- TSLint: [no-reference](http://palantir.github.io/tslint/rules/no-reference/) +- TSLint: [no-reference-import](https://palantir.github.io/tslint/rules/no-reference-import/) diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index d12e6542a10..2c16956758f 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -31,7 +31,6 @@ import noNamespace from './no-namespace'; import noNonNullAssertion from './no-non-null-assertion'; import noObjectLiteralTypeAssertion from './no-object-literal-type-assertion'; import noParameterProperties from './no-parameter-properties'; -import noReferenceImport from './no-reference-import'; import noRequireImports from './no-require-imports'; import noThisAlias from './no-this-alias'; import noTripleSlashReference from './no-triple-slash-reference'; @@ -53,6 +52,7 @@ import promiseFunctionAsync from './promise-function-async'; import requireArraySortCompare from './require-array-sort-compare'; import restrictPlusOperands from './restrict-plus-operands'; import semi from './semi'; +import tripleSlashReference from './triple-slash-reference'; import typeAnnotationSpacing from './type-annotation-spacing'; import unboundMethod from './unbound-method'; import unifiedSignatures from './unified-signatures'; @@ -91,7 +91,6 @@ export default { 'no-non-null-assertion': noNonNullAssertion, 'no-object-literal-type-assertion': noObjectLiteralTypeAssertion, 'no-parameter-properties': noParameterProperties, - 'no-reference-import': noReferenceImport, 'no-require-imports': noRequireImports, 'no-this-alias': noThisAlias, 'no-triple-slash-reference': noTripleSlashReference, @@ -113,6 +112,7 @@ export default { 'require-array-sort-compare': requireArraySortCompare, 'restrict-plus-operands': restrictPlusOperands, semi: semi, + 'triple-slash-reference': tripleSlashReference, 'type-annotation-spacing': typeAnnotationSpacing, 'unbound-method': unboundMethod, 'unified-signatures': unifiedSignatures, diff --git a/packages/eslint-plugin/src/rules/no-reference-import.ts b/packages/eslint-plugin/src/rules/no-reference-import.ts deleted file mode 100644 index 7a7fee23a22..00000000000 --- a/packages/eslint-plugin/src/rules/no-reference-import.ts +++ /dev/null @@ -1,78 +0,0 @@ -import * as util from '../util'; -import { - Literal, - Node, - TSExternalModuleReference, -} from '@typescript-eslint/typescript-estree/dist/ts-estree/ts-estree'; -import { TSESTree } from '@typescript-eslint/typescript-estree'; - -export default util.createRule({ - name: 'no-reference-import', - meta: { - type: 'suggestion', - docs: { - description: - 'Disallow simultaneous use of `/// ` comments and ES6 style imports for the same module', - category: 'Best Practices', - recommended: false, - }, - schema: [], - messages: { - noReferenceImport: 'Do not reference {{module}} if importing it anyway.', - }, - }, - defaultOptions: [], - create(context) { - let programNode: Node; - const sourceCode = context.getSourceCode(); - const references: ({ - comment: TSESTree.Comment; - importName: string; - })[] = []; - - function hasMatchingReference(source: Literal) { - references.forEach(reference => { - if (reference.importName === source.value) { - context.report({ - node: reference.comment, - messageId: 'noReferenceImport', - data: { - module: reference.importName, - }, - }); - } - }); - } - return { - ImportDeclaration(node) { - if (programNode) { - const source = node.source as Literal; - hasMatchingReference(source); - } - }, - TSImportEqualsDeclaration(node) { - if (programNode) { - const source = (node.moduleReference as TSExternalModuleReference) - .expression as Literal; - hasMatchingReference(source); - } - }, - Program(node) { - programNode = node; - const referenceRegExp = /^\/\s* { - if (comment.type !== 'Line') { - return; - } - const referenceResult = referenceRegExp.exec(comment.value); - - if (referenceResult && referenceResult[1]) { - references.push({ comment, importName: referenceResult[1] }); - } - }); - }, - }; - }, -}); diff --git a/packages/eslint-plugin/src/rules/no-triple-slash-reference.ts b/packages/eslint-plugin/src/rules/no-triple-slash-reference.ts index c7780a99bf3..5d24bb48e2c 100644 --- a/packages/eslint-plugin/src/rules/no-triple-slash-reference.ts +++ b/packages/eslint-plugin/src/rules/no-triple-slash-reference.ts @@ -11,7 +11,7 @@ export default util.createRule({ }, schema: [], messages: { - tripleSlashReference: 'Do not use a triple slash reference.', + noTripleSlashReference: 'Do not use a triple slash reference.', }, }, defaultOptions: [], @@ -30,7 +30,7 @@ export default util.createRule({ if (referenceRegExp.test(comment.value)) { context.report({ node: comment, - messageId: 'tripleSlashReference', + messageId: 'noTripleSlashReference', }); } }); diff --git a/packages/eslint-plugin/src/rules/triple-slash-reference.ts b/packages/eslint-plugin/src/rules/triple-slash-reference.ts new file mode 100644 index 00000000000..286f445c966 --- /dev/null +++ b/packages/eslint-plugin/src/rules/triple-slash-reference.ts @@ -0,0 +1,129 @@ +import * as util from '../util'; +import { + Literal, + Node, + TSExternalModuleReference, +} from '@typescript-eslint/typescript-estree/dist/ts-estree/ts-estree'; +import { TSESTree } from '@typescript-eslint/typescript-estree'; + +type Options = [ + { + lib?: 'always' | 'never'; + path?: 'always' | 'never'; + types?: 'always' | 'never' | 'prefer-import'; + } +]; +type MessageIds = 'tripleSlashReference'; + +export default util.createRule({ + name: 'triple-slash-reference', + meta: { + type: 'suggestion', + docs: { + description: + 'Sets preference level for triple slash directives versus ES6-style import declarations', + category: 'Best Practices', + recommended: false, + }, + messages: { + tripleSlashReference: + 'Do not use a triple slash reference for {{module}}, use `import` style instead.', + }, + schema: [ + { + type: 'object', + properties: { + lib: { + enum: ['always', 'never'], + }, + path: { + enum: ['always', 'never'], + }, + types: { + enum: ['always', 'never', 'prefer-import'], + }, + }, + additionalProperties: false, + }, + ], + }, + defaultOptions: [ + { + lib: 'always', + path: 'never', + types: 'prefer-import', + }, + ], + create(context, [{ lib, path, types }]) { + let programNode: Node; + const sourceCode = context.getSourceCode(); + const references: ({ + comment: TSESTree.Comment; + importName: string; + })[] = []; + + function hasMatchingReference(source: Literal) { + references.forEach(reference => { + if (reference.importName === source.value) { + context.report({ + node: reference.comment, + messageId: 'tripleSlashReference', + data: { + module: reference.importName, + }, + }); + } + }); + } + return { + ImportDeclaration(node) { + if (programNode) { + const source = node.source as Literal; + hasMatchingReference(source); + } + }, + TSImportEqualsDeclaration(node) { + if (programNode) { + const source = (node.moduleReference as TSExternalModuleReference) + .expression as Literal; + hasMatchingReference(source); + } + }, + Program(node) { + if (lib === 'always' && path === 'always' && types == 'always') { + return; + } + programNode = node; + const referenceRegExp = /^\/\s* { + if (comment.type !== 'Line') { + return; + } + const referenceResult = referenceRegExp.exec(comment.value); + + if (referenceResult) { + if ( + (referenceResult[1] === 'types' && types === 'never') || + (referenceResult[1] === 'path' && path === 'never') || + (referenceResult[1] === 'lib' && lib === 'never') + ) { + context.report({ + node: comment, + messageId: 'tripleSlashReference', + data: { + module: referenceResult[2], + }, + }); + return; + } + if (referenceResult[1] === 'types' && types === 'prefer-import') { + references.push({ comment, importName: referenceResult[2] }); + } + } + }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-reference-import.test.ts b/packages/eslint-plugin/tests/rules/no-reference-import.test.ts deleted file mode 100644 index 20ddd2b94c6..00000000000 --- a/packages/eslint-plugin/tests/rules/no-reference-import.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -import rule from '../../src/rules/no-reference-import'; -import { RuleTester } from '../RuleTester'; - -const ruleTester = new RuleTester({ - parserOptions: { - sourceType: 'module', - }, - parser: '@typescript-eslint/parser', -}); - -ruleTester.run('no-reference-import', rule, { - valid: [ - `/// - var foo = require("foo");`, - `/// - import * as foo from "foo"`, - ], - invalid: [ - { - code: ` -/// -import * as foo from "foo" - `, - - parser: '@typescript-eslint/parser', - errors: [ - { - messageId: 'noReferenceImport', - line: 2, - column: 1, - }, - ], - }, - { - code: ` -/// -import foo = require("foo"); - `, - parser: '@typescript-eslint/parser', - errors: [ - { - messageId: 'noReferenceImport', - line: 2, - column: 1, - }, - ], - }, - ], -}); diff --git a/packages/eslint-plugin/tests/rules/no-triple-slash-reference.test.ts b/packages/eslint-plugin/tests/rules/no-triple-slash-reference.test.ts index 5beaf104e52..f120d526243 100644 --- a/packages/eslint-plugin/tests/rules/no-triple-slash-reference.test.ts +++ b/packages/eslint-plugin/tests/rules/no-triple-slash-reference.test.ts @@ -22,7 +22,7 @@ let a code: '/// ', errors: [ { - messageId: 'tripleSlashReference', + messageId: 'noTripleSlashReference', line: 1, column: 1, }, @@ -36,7 +36,7 @@ let a parser: '@typescript-eslint/parser', errors: [ { - messageId: 'tripleSlashReference', + messageId: 'noTripleSlashReference', line: 2, column: 1, }, diff --git a/packages/eslint-plugin/tests/rules/triple-slash-reference.test.ts b/packages/eslint-plugin/tests/rules/triple-slash-reference.test.ts new file mode 100644 index 00000000000..2e235f1751c --- /dev/null +++ b/packages/eslint-plugin/tests/rules/triple-slash-reference.test.ts @@ -0,0 +1,122 @@ +import rule from '../../src/rules/triple-slash-reference'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parserOptions: { + sourceType: 'module', + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('triple-slash-reference', rule, { + valid: [ + { + code: ` + /// + import * as foo from "foo" + `, + options: [{ path: 'always' }], + }, + { + code: ` + /// + import * as foo from "foo" + `, + options: [{ types: 'always' }], + }, + { + code: ` + /// + import * as foo from "foo" + `, + options: [{ lib: 'always' }], + }, + { + code: ` + import * as foo from "foo" + `, + options: [{ path: 'never' }], + }, + { + code: ` + import * as foo from "foo" + `, + options: [{ types: 'never' }], + }, + { + code: ` + import * as foo from "foo" + `, + options: [{ lib: 'never' }], + }, + { + code: ` + import * as foo from "foo" + `, + options: [{ types: 'prefer-import' }], + }, + ], + invalid: [ + { + code: ` +/// +import * as foo from "foo" + `, + options: [{ types: 'prefer-import' }], + errors: [ + { + messageId: 'tripleSlashReference', + line: 2, + column: 1, + }, + ], + }, + { + code: ` +/// +import foo = require("foo"); + `, + options: [{ types: 'prefer-import' }], + errors: [ + { + messageId: 'tripleSlashReference', + line: 2, + column: 1, + }, + ], + }, + { + code: `/// `, + options: [{ path: 'never' }], + errors: [ + { + messageId: 'tripleSlashReference', + line: 1, + column: 1, + }, + ], + }, + { + code: `/// `, + options: [{ types: 'never' }], + errors: [ + { + messageId: 'tripleSlashReference', + line: 1, + column: 1, + }, + ], + }, + { + code: `/// `, + options: [{ lib: 'never' }], + errors: [ + { + messageId: 'tripleSlashReference', + line: 1, + column: 1, + }, + ], + }, + ], +}); From 6fca1464213d5bbf25cb99c6a2ea27b0c752fc0b Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 28 Jun 2019 16:13:31 -0700 Subject: [PATCH 5/6] fix(eslint-plugin): deprecated rule and added coverage --- .../docs/rules/no-triple-slash-reference.md | 2 ++ .../docs/rules/triple-slash-reference.md | 2 +- packages/eslint-plugin/src/configs/all.json | 2 +- .../src/rules/no-triple-slash-reference.ts | 2 ++ .../rules/triple-slash-reference.test.ts | 26 ++++++++++++------- 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-triple-slash-reference.md b/packages/eslint-plugin/docs/rules/no-triple-slash-reference.md index be090acea26..409ff6093ca 100644 --- a/packages/eslint-plugin/docs/rules/no-triple-slash-reference.md +++ b/packages/eslint-plugin/docs/rules/no-triple-slash-reference.md @@ -12,6 +12,8 @@ A triple-slash reference directive is a comment beginning with three slashes fol ES6 Modules handle this now: `import animal from "./Animal"` +## DEPRECATED - this rule has been deprecated in favour of [`triple-slash-reference`](./triple-slash-reference.md) + ## Rule Details Does not allow the use of `/// ` comments. diff --git a/packages/eslint-plugin/docs/rules/triple-slash-reference.md b/packages/eslint-plugin/docs/rules/triple-slash-reference.md index b09fc741097..3dccaa925e9 100644 --- a/packages/eslint-plugin/docs/rules/triple-slash-reference.md +++ b/packages/eslint-plugin/docs/rules/triple-slash-reference.md @@ -2,7 +2,7 @@ Use of triple-slash reference type directives is discouraged in favor of the newer `import` style. This rule allows you to ban use of `/// `, `/// `, or `/// ` directives. -If you use the `no-triple-slash-reference` rule, consider using this rule instead. +Consider using this rule in place of [`no-triple-slash-reference`](./no-triple-slash-reference.md) which has been deprecated. ## Rule Details diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 2efc8d24323..93576484596 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -42,7 +42,6 @@ "@typescript-eslint/no-parameter-properties": "error", "@typescript-eslint/no-require-imports": "error", "@typescript-eslint/no-this-alias": "error", - "@typescript-eslint/no-triple-slash-reference": "error", "@typescript-eslint/no-type-alias": "error", "@typescript-eslint/no-unnecessary-qualifier": "error", "@typescript-eslint/no-unnecessary-type-assertion": "error", @@ -64,6 +63,7 @@ "@typescript-eslint/restrict-plus-operands": "error", "semi": "off", "@typescript-eslint/semi": "error", + "@typescript-eslint/triple-slash-reference": "error", "@typescript-eslint/type-annotation-spacing": "error", "@typescript-eslint/unbound-method": "error", "@typescript-eslint/unified-signatures": "error" diff --git a/packages/eslint-plugin/src/rules/no-triple-slash-reference.ts b/packages/eslint-plugin/src/rules/no-triple-slash-reference.ts index 5d24bb48e2c..49f16420166 100644 --- a/packages/eslint-plugin/src/rules/no-triple-slash-reference.ts +++ b/packages/eslint-plugin/src/rules/no-triple-slash-reference.ts @@ -10,6 +10,8 @@ export default util.createRule({ recommended: 'error', }, schema: [], + deprecated: true, + replacedBy: ['triple-slash-reference'], messages: { noTripleSlashReference: 'Do not use a triple slash reference.', }, diff --git a/packages/eslint-plugin/tests/rules/triple-slash-reference.test.ts b/packages/eslint-plugin/tests/rules/triple-slash-reference.test.ts index 2e235f1751c..0785bf7a02e 100644 --- a/packages/eslint-plugin/tests/rules/triple-slash-reference.test.ts +++ b/packages/eslint-plugin/tests/rules/triple-slash-reference.test.ts @@ -13,47 +13,53 @@ ruleTester.run('triple-slash-reference', rule, { { code: ` /// + /// + /// import * as foo from "foo" + import * as bar from "bar" + import * as baz from "baz" `, - options: [{ path: 'always' }], + options: [{ path: 'always', types: 'always', lib: 'always' }], }, { code: ` - /// import * as foo from "foo" `, - options: [{ types: 'always' }], + options: [{ path: 'never' }], }, { code: ` - /// import * as foo from "foo" `, - options: [{ lib: 'always' }], + options: [{ types: 'never' }], }, { code: ` import * as foo from "foo" `, - options: [{ path: 'never' }], + options: [{ lib: 'never' }], }, { code: ` import * as foo from "foo" `, - options: [{ types: 'never' }], + options: [{ types: 'prefer-import' }], }, { code: ` - import * as foo from "foo" + /// + import * as bar from "bar" `, - options: [{ lib: 'never' }], + options: [{ types: 'prefer-import' }], }, { code: ` + /* + /// + */ import * as foo from "foo" `, - options: [{ types: 'prefer-import' }], + options: [{ path: 'never', types: 'never', lib: 'never' }], }, ], invalid: [ From 1549cf106365d6f6cbbdee599ca35398fee68bdb Mon Sep 17 00:00:00 2001 From: Jesse Trinity <42591254+jessetrinity@users.noreply.github.com> Date: Mon, 1 Jul 2019 18:21:54 -0700 Subject: [PATCH 6/6] Update README.md --- packages/eslint-plugin/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index bb6cbf94fa6..c436e32be6c 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -159,7 +159,6 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-parameter-properties`](./docs/rules/no-parameter-properties.md) | Disallow the use of parameter properties in class constructors | :heavy_check_mark: | | | | [`@typescript-eslint/no-require-imports`](./docs/rules/no-require-imports.md) | Disallows invocation of `require()` | | | | | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` | | | | -| [`@typescript-eslint/no-triple-slash-reference`](./docs/rules/no-triple-slash-reference.md) | Disallow `/// ` comments | :heavy_check_mark: | | | | [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases | | | | | [`@typescript-eslint/no-unnecessary-qualifier`](./docs/rules/no-unnecessary-qualifier.md) | Warns when a namespace qualifier is unnecessary | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression | | :wrench: | :thought_balloon: |