From af70a5932f8032cde13f4a0c98d6657cdb40b441 Mon Sep 17 00:00:00 2001
From: Jesse Trinity <42591254+jessetrinity@users.noreply.github.com>
Date: Mon, 1 Jul 2019 18:52:02 -0700
Subject: [PATCH] feat(eslint-plugin): Add rule no-reference-import (#625)
* feat(eslint-plugin): added no-reference-import rule
* docs: added docs for no-reference-import
* fix(eslint-plugin): collect references when visiting the program node
* feat(eslint-plugin): updated rule to cover more reference directives
* fix(eslint-plugin): deprecated rule and added coverage
* Update README.md
---
packages/eslint-plugin/README.md | 2 +-
.../docs/rules/no-triple-slash-reference.md | 2 +
.../docs/rules/triple-slash-reference.md | 58 ++++++++
packages/eslint-plugin/src/configs/all.json | 2 +-
packages/eslint-plugin/src/rules/index.ts | 2 +
.../src/rules/no-triple-slash-reference.ts | 6 +-
.../src/rules/triple-slash-reference.ts | 129 ++++++++++++++++++
.../rules/no-triple-slash-reference.test.ts | 4 +-
.../rules/triple-slash-reference.test.ts | 128 +++++++++++++++++
9 files changed, 327 insertions(+), 6 deletions(-)
create mode 100644 packages/eslint-plugin/docs/rules/triple-slash-reference.md
create mode 100644 packages/eslint-plugin/src/rules/triple-slash-reference.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 875f89bbebe..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: |
@@ -179,6 +178,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
| [`@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/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md) | Restricts the types allowed in boolean expressions | | | :thought_balloon: |
+| [`@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-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
new file mode 100644
index 00000000000..3dccaa925e9
--- /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.
+
+Consider using this rule in place of [`no-triple-slash-reference`](./no-triple-slash-reference.md) which has been deprecated.
+
+## 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/configs/all.json b/packages/eslint-plugin/src/configs/all.json
index 7dc471fdd5d..cd5b2bb9cbe 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",
@@ -65,6 +64,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/index.ts b/packages/eslint-plugin/src/rules/index.ts
index f93cb788cf7..00107aa57a8 100644
--- a/packages/eslint-plugin/src/rules/index.ts
+++ b/packages/eslint-plugin/src/rules/index.ts
@@ -54,6 +54,7 @@ import requireArraySortCompare from './require-array-sort-compare';
import restrictPlusOperands from './restrict-plus-operands';
import semi from './semi';
import strictBooleanExpressions from './strict-boolean-expressions';
+import tripleSlashReference from './triple-slash-reference';
import typeAnnotationSpacing from './type-annotation-spacing';
import unboundMethod from './unbound-method';
import unifiedSignatures from './unified-signatures';
@@ -115,6 +116,7 @@ export default {
'restrict-plus-operands': restrictPlusOperands,
semi: semi,
'strict-boolean-expressions': strictBooleanExpressions,
+ 'triple-slash-reference': tripleSlashReference,
'type-annotation-spacing': typeAnnotationSpacing,
'unbound-method': unboundMethod,
'unified-signatures': unifiedSignatures,
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..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,8 +10,10 @@ export default util.createRule({
recommended: 'error',
},
schema: [],
+ deprecated: true,
+ replacedBy: ['triple-slash-reference'],
messages: {
- tripleSlashReference: 'Do not use a triple slash reference.',
+ noTripleSlashReference: 'Do not use a triple slash reference.',
},
},
defaultOptions: [],
@@ -30,7 +32,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-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..0785bf7a02e
--- /dev/null
+++ b/packages/eslint-plugin/tests/rules/triple-slash-reference.test.ts
@@ -0,0 +1,128 @@
+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"
+ import * as bar from "bar"
+ import * as baz from "baz"
+ `,
+ options: [{ path: 'always', types: 'always', 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' }],
+ },
+ {
+ code: `
+ ///
+ import * as bar from "bar"
+ `,
+ options: [{ types: 'prefer-import' }],
+ },
+ {
+ code: `
+ /*
+ ///
+ */
+ import * as foo from "foo"
+ `,
+ options: [{ path: 'never', types: 'never', lib: 'never' }],
+ },
+ ],
+ 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,
+ },
+ ],
+ },
+ ],
+});