feat: add internal eslint plugin for repo-specific lint rules

.eslintrc.js

@@ -6,6 +6,7 @@ module.exports = {
     'jest',
     'import',
     'eslint-comments',
+    '@typescript-eslint/internal',
   ],
   env: {
     es6: true,
@@ -117,6 +118,11 @@ module.exports = {
     'import/no-self-import': 'error',
     // Require modules with a single export to use a default export
     'import/prefer-default-export': 'off', // we want everything to be named
+
+    //
+    // Internal repo rules
+    //
+    '@typescript-eslint/internal/no-typescript-default-import': 'error',
   },
   parserOptions: {
     sourceType: 'module',
@@ -127,8 +133,10 @@ module.exports = {
     tsconfigRootDir: __dirname,
   },
   overrides: [
+    // all test files
     {
       files: [
+        'packages/eslint-plugin-internal/tests/**/*.test.ts',
         'packages/eslint-plugin-tslint/tests/**/*.ts',
         'packages/eslint-plugin/tests/**/*.test.ts',
         'packages/parser/tests/**/*.ts',
@@ -138,6 +146,7 @@ module.exports = {
         'jest/globals': true,
       },
       rules: {
+        'eslint-plugin/no-identical-tests': 'error',
         'jest/no-disabled-tests': 'warn',
         'jest/no-focused-tests': 'error',
         'jest/no-alias-methods': 'error',
@@ -152,26 +161,33 @@ module.exports = {
         'jest/valid-expect': 'error',
       },
     },
+    // plugin source files
     {
       files: [
-        'packages/eslint-plugin/tests/**/*.test.ts',
-        'packages/eslint-plugin-tslint/tests/**/*.spec.ts',
+        'packages/eslint-plugin-internal/**/*.ts',
+        'packages/eslint-plugin-tslint/**/*.ts',
+        'packages/eslint-plugin/**/*.ts',
       ],
       rules: {
-        'eslint-plugin/no-identical-tests': 'error',
+        '@typescript-eslint/internal/no-typescript-estree-import': 'error',
+        // plugins don't have any generated types, so it's safe to use
+        '@typescript-eslint/internal/no-typescript-default-import': 'off',
       },
     },
+    // rule source files
     {
       files: [
-        'packages/eslint-plugin/src/rules/**/*.ts',
-        'packages/eslint-plugin/src/configs/**/*.ts',
+        'packages/eslint-plugin-internal/src/rules/**/*.ts',
         'packages/eslint-plugin-tslint/src/rules/**/*.ts',
+        'packages/eslint-plugin/src/configs/**/*.ts',
+        'packages/eslint-plugin/src/rules/**/*.ts',
       ],
       rules: {
         // specifically for rules - default exports makes the tooling easier
         'import/no-default-export': 'off',
       },
     },
+    // tools and tests
     {
       files: ['**/tools/**/*.ts', '**/tests/**/*.ts'],
       rules: {

.vscode/launch.json

@@ -20,6 +20,22 @@
           "console": "integratedTerminal",
           "internalConsoleOptions": "neverOpen"
       },
+      {
+          "type": "node",
+          "request": "launch",
+          "name": "Jest Test Current eslint-plugin-internal Rule",
+          "cwd": "${workspaceFolder}/packages/eslint-plugin-internal/",
+          "program": "${workspaceFolder}/node_modules/jest/bin/jest.js",
+          "args": [
+              "--runInBand",
+              "--no-coverage",
+              // needs the '' around it so that the () are properly handled
+              "'tests/(.+/)?${fileBasenameNoExtension}'"
+          ],
+          "sourceMaps": true,
+          "console": "integratedTerminal",
+          "internalConsoleOptions": "neverOpen"
+      },
       {
         "type": "node",
         "request": "launch",

packages/eslint-plugin-internal/README.md

@@ -0,0 +1,5 @@
+# `eslint-plugin-internal`
+
+This is just a collection of internal lint rules to help enforce some guidelines specific to this repository.
+
+These are not intended to be used externally.

packages/eslint-plugin-internal/jest.config.js

@@ -0,0 +1,13 @@
+'use strict';
+
+module.exports = {
+  testEnvironment: 'node',
+  transform: {
+    '^.+\\.tsx?$': 'ts-jest',
+  },
+  testRegex: './tests/.+\\.test\\.ts$',
+  collectCoverage: false,
+  collectCoverageFrom: ['src/**/*.{js,jsx,ts,tsx}'],
+  moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'],
+  coverageReporters: ['text-summary', 'lcov'],
+};

packages/eslint-plugin-internal/package.json

@@ -0,0 +1,18 @@
+{
+  "name": "@typescript-eslint/eslint-plugin-internal",
+  "version": "2.12.0",
+  "private": true,
+  "main": "dist/index.js",
+  "scripts": {
+    "build": "tsc -b tsconfig.build.json",
+    "clean": "tsc -b tsconfig.build.json --clean",
+    "format": "prettier --write \"./**/*.{ts,js,json,md}\" --ignore-path ../../.prettierignore",
+    "lint": "eslint . --ext .js,.ts --ignore-path='../../.eslintignore'",
+    "test": "jest --coverage",
+    "typecheck": "tsc -p tsconfig.json --noEmit"
+  },
+  "dependencies": {
+    "@typescript-eslint/experimental-utils": "2.12.0"
+  },
+  "devDependencies": {}
+}

packages/eslint-plugin-internal/src/index.ts

@@ -0,0 +1,5 @@
+import rules from './rules';
+
+export = {
+  rules,
+};

packages/eslint-plugin-internal/src/rules/index.ts

@@ -0,0 +1,7 @@
+import noTypescriptDefaultImport from './no-typescript-default-import';
+import noTypescriptEstreeImport from './no-typescript-estree-import';
+
+export default {
+  'no-typescript-default-import': noTypescriptDefaultImport,
+  'no-typescript-estree-import': noTypescriptEstreeImport,
+};

packages/eslint-plugin-internal/src/rules/no-typescript-default-import.ts

@@ -0,0 +1,80 @@
+import {
+  TSESTree,
+  AST_NODE_TYPES,
+} from '@typescript-eslint/experimental-utils';
+import { createRule } from '../util';
+
+/*
+We have `allowSyntheticDefaultImports` turned on in this project, so there are two problems that arise:
+- TypeScript's auto import will suggest `import ts = require('typescript');` if you type `ts`
+- VSCode's suggestion feature will suggest changing `import * as ts from 'typescript'` to `import ts from 'typescript'`
+
+In order to keep compat with a wide range of consumers, some of whom don't use `allowSyntheticDefaultImports`, we should
+always use either:
+- `import * as ts from 'typescript';`
+- `import { SyntaxKind } from 'typescript';`
+*/
+
+export default createRule({
+  name: 'no-typescript-default-import',
+  meta: {
+    type: 'problem',
+    docs: {
+      description:
+        "Enforces that packages rules don't do `import ts from 'typescript';`",
+      category: 'Possible Errors',
+      recommended: 'error',
+    },
+    fixable: 'code',
+    schema: [],
+    messages: {
+      noTSDefaultImport: [
+        "Do not use the default import for typescript. Doing so will cause the package's type definitions to do the same.",
+        "This causes errors for consumers if they don't use the allowSyntheticDefaultImports compiler option.",
+      ].join('\n'),
+    },
+  },
+  defaultOptions: [],
+  create(context) {
+    return {
+      'ImportDeclaration > ImportDefaultSpecifier'(
+        node: TSESTree.ImportDefaultSpecifier,
+      ): void {
+        const importStatement = node.parent as TSESTree.ImportDeclaration;
+        if (importStatement.source.value === 'typescript') {
+          context.report({
+            node,
+            messageId: 'noTSDefaultImport',
+            fix(fixer) {
+              if (importStatement.specifiers.length === 1) {
+                return fixer.replaceText(node, '* as ts');
+              }
+
+              return null;
+            },
+          });
+        }
+      },
+      'TSImportEqualsDeclaration > TSExternalModuleReference'(
+        node: TSESTree.TSExternalModuleReference,
+      ): void {
+        const parent = node.parent as TSESTree.TSImportEqualsDeclaration;
+        if (
+          node.expression.type === AST_NODE_TYPES.Literal &&
+          node.expression.value === 'typescript'
+        ) {
+          context.report({
+            node,
+            messageId: 'noTSDefaultImport',
+            fix(fixer) {
+              return fixer.replaceText(
+                parent,
+                "import * as ts from 'typescript';",
+              );
+            },
+          });
+        }
+      },
+    };
+  },
+});

packages/eslint-plugin-internal/src/rules/no-typescript-estree-import.ts

@@ -0,0 +1,52 @@
+import { createRule } from '../util';
+
+const TSESTREE_NAME = '@typescript-eslint/typescript-estree';
+const UTILS_NAME = '@typescript-eslint/experimental-utils';
+
+/*
+Typescript will not error if people use typescript-estree within eslint-plugin.
+This is because it's an indirect dependency.
+We don't want people to import it, instead we want them to import from the utils package.
+*/
+
+export default createRule({
+  name: 'no-typescript-estree-import',
+  meta: {
+    type: 'problem',
+    docs: {
+      description: `Enforces that eslint-plugin rules don't require anything from ${TSESTREE_NAME}`,
+      category: 'Possible Errors',
+      recommended: 'error',
+    },
+    fixable: 'code',
+    schema: [],
+    messages: {
+      dontImportTSEStree: [
+        `Don't import from ${TSESTREE_NAME}. Everything you need should be available in ${UTILS_NAME}.`,
+        `${TSESTREE_NAME} is an indirect dependency of this package, and thus should not be used directly.`,
+      ].join('\n'),
+    },
+  },
+  defaultOptions: [],
+  create(context) {
+    return {
+      ImportDeclaration(node): void {
+        if (
+          typeof node.source.value === 'string' &&
+          node.source.value.startsWith(TSESTREE_NAME)
+        ) {
+          context.report({
+            node,
+            messageId: 'dontImportTSEStree',
+            fix(fixer) {
+              return fixer.replaceTextRange(
+                [node.source.range[0] + 1, node.source.range[1] - 1],
+                UTILS_NAME,
+              );
+            },
+          });
+        }
+      },
+    };
+  },
+});

packages/eslint-plugin-internal/src/util/createRule.ts

@@ -0,0 +1,11 @@
+import { ESLintUtils } from '@typescript-eslint/experimental-utils';
+
+// note - cannot migrate this to an import statement because it will make TSC copy the package.json to the dist folder
+const version = require('../../package.json').version;
+
+const createRule = ESLintUtils.RuleCreator(
+  name =>
+    `https://github.com/typescript-eslint/typescript-eslint/blob/v${version}/packages/eslint-plugin-internal/src/rules/${name}.ts`,
+);
+
+export { createRule };

packages/eslint-plugin-internal/src/util/index.ts

@@ -0,0 +1 @@
+export * from './createRule';

packages/eslint-plugin-internal/tests/RuleTester.ts

@@ -0,0 +1,22 @@
+import { TSESLint, ESLintUtils } from '@typescript-eslint/experimental-utils';
+
+const { batchedSingleLineTests } = ESLintUtils;
+
+const parser = '@typescript-eslint/parser';
+
+type RuleTesterConfig = Omit<TSESLint.RuleTesterConfig, 'parser'> & {
+  parser: typeof parser;
+};
+class RuleTester extends TSESLint.RuleTester {
+  // as of eslint 6 you have to provide an absolute path to the parser
+  // but that's not as clean to type, this saves us trying to manually enforce
+  // that contributors require.resolve everything
+  constructor(options: RuleTesterConfig) {
+    super({
+      ...options,
+      parser: require.resolve(options.parser),
+    });
+  }
+}
+
+export { RuleTester, batchedSingleLineTests };

packages/eslint-plugin-internal/tests/rules/no-typescript-default-import.test.ts

@@ -0,0 +1,45 @@
+import rule from '../../src/rules/no-typescript-default-import';
+import { RuleTester, batchedSingleLineTests } from '../RuleTester';
+
+const ruleTester = new RuleTester({
+  parser: '@typescript-eslint/parser',
+  parserOptions: {
+    sourceType: 'module',
+  },
+});
+
+ruleTester.run('no-typescript-default-import', rule, {
+  valid: [
+    "import { foo } from 'typescript';",
+    "import ts from 'nottypescript';",
+    "import * as foo from 'typescript';",
+    'import ts = foo;',
+    "import ts = require('nottypescript');",
+  ],
+  invalid: batchedSingleLineTests({
+    code: `
+import ts from 'typescript';
+import ts, { SyntaxKind } from 'typescript';
+import ts = require('typescript');
+    `,
+    output: `
+import * as ts from 'typescript';
+import ts, { SyntaxKind } from 'typescript';
+import * as ts from 'typescript';
+    `,
+    errors: [
+      {
+        messageId: 'noTSDefaultImport',
+        line: 2,
+      },
+      {
+        messageId: 'noTSDefaultImport',
+        line: 3,
+      },
+      {
+        messageId: 'noTSDefaultImport',
+        line: 4,
+      },
+    ],
+  }),
+});