10000 feat: add internal eslint plugin for repo-specific lint rules (#1373) · invalidred/typescript-eslint@3a15413 · GitHub
[go: up one dir, main page]
Skip to content

Commit 3a15413

Browse files
bradzacherbradzacher
authored
feat: add internal eslint plugin for repo-specific lint rules (typescript-eslint#1373)
There's a few things I want to enforce internally. Save us from having to communicate these things in PR reviews. ensuring people don't do import ts from 'typescript' in packages. this breaks compat with users that don't use allowSyntheticDefaultImports ensuring people don't accidentally do import {} from '@typescript-eslint/typescript-estree' from the plugins, where the package isn't a dependency. this breaks encapsulation, and will cause problems if we move things around in future Adding this in now reduces the barrier to entry, meaning we can easily add rules to warn against patterns we see people do in the future.
1 parent a78b194 commit 3a15413

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+401
-47
lines changed

‎.eslintrc.js

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ module.exports = {
66
'jest',
77
'import',
88
'eslint-comments',
9+
'@typescript-eslint/internal',
910
],
1011
env: {
1112
es6: true,
@@ -117,6 +118,11 @@ module.exports = {
117118
'import/no-self-import': 'error',
118119
// Require modules with a single export to use a default export
119120
'import/prefer-default-export': 'off', // we want everything to be named
121+
122+
//
123+
// Internal repo rules
124+
//
125+
'@typescript-eslint/internal/no-typescript-default-import': 'error',
120126
},
121127
parserOptions: {
122128
sourceType: 'module',
@@ -127,8 +133,10 @@ module.exports = {
127133
tsconfigRootDir: __dirname,
128134
},
129135
overrides: [
136+
// all test files
130137
{
131138
files: [
139+
'packages/eslint-plugin-internal/tests/**/*.test.ts',
132140
'packages/eslint-plugin-tslint/tests/**/*.ts',
133141
'packages/eslint-plugin/tests/**/*.test.ts',
134142
'packages/parser/tests/**/*.ts',
@@ -138,6 +146,7 @@ module.exports = {
138146
'jest/globals': true,
139147
},
140148
rules: {
149+
'eslint-plugin/no-identical-tests': 'error',
141150
'jest/no-disabled-tests': 'warn',
142151
'jest/no-focused-tests': 'error',
143152
'jest/no-alias-methods': 'error',
@@ -152,26 +161,31 @@ module.exports = {
152161
'jest/valid-expect': 'error',
153162
},
154163
},
164+
// plugin source files
155165
{
156166
files: [
157-
'packages/eslint-plugin/tests/**/*.test.ts',
158-
'packages/eslint-plugin-tslint/tests/**/*.spec.ts',
167+
'packages/eslint-plugin-internal/**/*.ts',
168+
'packages/eslint-plugin-tslint/**/*.ts',
169+
'packages/eslint-plugin/**/*.ts',
159170
],
160171
rules: {
161-
'eslint-plugin/no-identical-tests': 'error',
172+
'@typescript-eslint/internal/no-typescript-estree-import': 'error',
162173
},
163174
},
175+
// rule source files
164176
{
165177
files: [
166-
'packages/eslint-plugin/src/rules/**/*.ts',
167-
'packages/eslint-plugin/src/configs/**/*.ts',
178+
'packages/eslint-plugin-internal/src/rules/**/*.ts',
168179
'packages/eslint-plugin-tslint/src/rules/**/*.ts',
180+
'packages/eslint-plugin/src/configs/**/*.ts',
181+
'packages/eslint-plugin/src/rules/**/*.ts',
169182
],
170183
rules: {
171184
// specifically for rules - default exports makes the tooling easier
172185
'import/no-default-export': 'off',
173186
},
174187
},
188+
// tools and tests
175189
{
176190
files: ['**/tools/**/*.ts', '**/tests/**/*.ts'],
177191
rules: {

‎.vscode/launch.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@
2020
"console": "integratedTerminal",
2121
"internalConsoleOptions": "neverOpen"
2222
},
23+
{
24+
"type": "node",
25+
"request": "launch",
26+
"name": "Jest Test Current eslint-plugin-internal Rule",
27+
"cwd": "${workspaceFolder}/packages/eslint-plugin-internal/",
28+
"program": "${workspaceFolder}/node_modules/jest/bin/jest.js",
29+
"args": [
30+
"--runInBand",
31+
"--no-coverage",
32+
// needs the '' around it so that the () are properly handled
33+
"'tests/(.+/)?${fileBasenameNoExtension}'"
34+
],
35+
"sourceMaps": true,
36+
"console": "integratedTerminal",
37+
"internalConsoleOptions": "neverOpen"
38+
},
2339
{
2440
"type": "node",
2541
"request": "launch",

‎packages/eslint-plugin-internal/README.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# `eslint-plugin-internal`
2+
3+
This is just a collection of internal lint rules to help enforce some guidelines specific to this repository.
4+
5+
These are not intended to be used externally.

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict';
2+
3+
module.exports = {
4+
testEnvironment: 'node',
5+
transform: {
6+
'^.+\\.tsx?$': 'ts-jest',
7+
},
8+
testRegex: './tests/.+\\.test\\.ts$',
9+
collectCoverage: false,
10+
collectCoverageFrom: ['src/**/*.{js,jsx,ts,tsx}'],
11+
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'],
12+
coverageReporters: ['text-summary', 'lcov'],
13+
};

‎packages/eslint-plugin-internal/package.json

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"name": "@typescript-eslint/eslint-plugin-internal",
3+
"version": "2.13.0",
4+
"private": true,
5+
"main": "dist/index.js",
6+
"scripts": {
7+
"build": "tsc -b tsconfig.build.json",
8+
"clean": "tsc -b tsconfig.build.json --clean",
9+
"format": "prettier --write \"./**/*.{ts,js,json,md}\" --ignore-path ../../.prettierignore",
10+
"lint": "eslint . --ext .js,.ts --ignore-path='../../.eslintignore'",
11+
"test": "jest --coverage",
12+
"typecheck": "tsc -p tsconfig.json --noEmit"
13+
},
14+
"dependencies": {
15+
"@typescript-eslint/experimental-utils": "2.13.0"
16+
},
17+
"devDependencies": {}
18+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import rules from './rules';
2+
3+
export = {
4+
rules,
5+
};

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import noTypescriptDefaultImport from './no-typescript-default-import';
2+
import noTypescriptEstreeImport from './no-typescript-estree-import';
3+
4+
export default {
5+
'no-typescript-default-import': noTypescriptDefaultImport,
6+
'no-typescript-estree-import': noTypescriptEstreeImport,
7+
};

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import {
2+
TSESTree,
3+
AST_NODE_TYPES,
4+
} from '@typescript-eslint/experimental-utils';
5+
import { createRule } from '../util';
6+
7+
/*
8+
We have `allowSyntheticDefaultImports` turned on in this project, so there are two problems that arise:
9+
- TypeScript's auto import will suggest `import ts = require('typescript');` if you type `ts`
10+
- VSCode's suggestion feature will suggest changing `import * as ts from 'typescript'` to `import ts from 'typescript'`
11+
12+
In order to keep compatibility with a wide range of consumers, some of whom don't use `allowSyntheticDefaultImports`, we should
13+
always use either:
14+
- `import * as ts from 'typescript';`
15+
- `import { SyntaxKind } from 'typescript';`
16+
*/
17+
18+
export default createRule({
19+
name: 'no-typescript-default-import',
20+
meta: {
21+
type: 'problem',
22+
docs: {
23+
description:
24+
"Enforces that packages rules don't do `import ts from 'typescript';`",
25+
category: 'Possible Errors',
26+
recommended: 'error',
27+
},
28+
fixable: 'code',
29+
schema: [],
30+
messages: {
31+
noTSDefaultImport: [
32+
"Do not use the default import for typescript. Doing so will cause the package's type definitions to do the same.",
33+
"This causes errors for consumers if they don't use the allowSyntheticDefaultImports compiler option.",
34+
].join('\n'),
35+
},
36+
},
37+
defaultOptions: [],
38+
create(context) {
39+
return {
40+
'ImportDeclaration > ImportDefaultSpecifier'(
41+
node: TSESTree.ImportDefaultSpecifier,
42+
): void {
43+
const importStatement = node.parent as TSESTree.ImportDeclaration;
44+
if (importStatement.source.value === 'typescript') {
45+
context.report({
46+
node,
47+
messageId: 'noTSDefaultImport',
48+
fix(fixer) {
49+
if (importStatement.specifiers.length === 1) {
50+
return fixer.replaceText(node, '* as ts');
51+
}
52+
53+
return null;
54+
},
55+
});
56+
}
57+
},
58+
'TSImportEqualsDeclaration > TSExternalModuleReference'(
59+
node: TSESTree.TSExternalModuleReference,
60+
): void {
61+
const parent = node.parent as TSESTree.TSImportEqualsDeclaration;
62+
if (
63+
node.expression.type === AST_NODE_TYPES.Literal &&
64+
node.expression.value === 'typescript'
65+
) {
66+
context.report({
67+
node,
68+
messageId: 'noTSDefaultImport',
69+
fix(fixer) {
70+
return fixer.replaceText(
71+
parent,
72+
"import * as ts from 'typescript';",
73+
);
74+
},
75+
});
76+
}
77+
},
78+
};
79+
},
80+
});

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { createRule } from '../util';
2+
3+
const TSESTREE_NAME = '@typescript-eslint/typescript-estree';
4+
const UTILS_NAME = '@typescript-eslint/experimental-utils';
5+
6+
/*
7+
Typescript will not error if people use typescript-estree within eslint-plugin.
8+
This is because it's an indirect dependency.
9+
We don't want people to import it, instead we want them to import from the utils package.
10+
*/
11+
12+
export default createRule({
13+
name: 'no-typescript-estree-import',
14+
meta: {
15+
type: 'problem',
16+
docs: {
17+
description: F438 `Enforces that eslint-plugin rules don't require anything from ${TSESTREE_NAME}`,
18+
category: 'Possible Errors',
19+
recommended: 'error',
20+
},
21+
fixable: 'code',
22+
schema: [],
23+
messages: {
24+
dontImportTSEStree: [
25+
`Don't import from ${TSESTREE_NAME}. Everything you need should be available in ${UTILS_NAME}.`,
26+
`${TSESTREE_NAME} is an indirect dependency of this package, and thus should not be used directly.`,
27+
].join('\n'),
28+
},
29+
},
30+
defaultOptions: [],
31+
create(context) {
32+
return {
33+
ImportDeclaration(node): void {
34+
if (
35+
typeof node.source.value === 'string' &&
36+
node.source.value.startsWith(TSESTREE_NAME)
37+
) {
38+
context.report({
39+
node,
40+
messageId: 'dontImportTSEStree',
41+
fix(fixer) {
42+
return fixer.replaceTextRange(
43+
[node.source.range[0] + 1, node.source.range[1] - 1],
44+
UTILS_NAME,
45+
);
46+
},
47+
});
48+
}
49+
},
50+
};
51+
},
52+
});

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { ESLintUtils } from '@typescript-eslint/experimental-utils';
2+
3+
// note - cannot migrate this to an import statement because it will make TSC copy the package.json to the dist folder
4+
const version = require('../../package.json').version;
5+
6+
const createRule = ESLintUtils.RuleCreator(
7+
name =>
8+
`https://github.com/typescript-eslint/typescript-eslint/blob/v${version}/packages/eslint-plugin-internal/src/rules/${name}.ts`,
9+
);
10+
11+
export { createRule };

0 commit comments

Comments
 (0)
0