-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: add internal eslint plugin for repo-specific lint rules (#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.
- Loading branch information
1 parent
a78b194
commit 3a15413
Showing
51 changed files
with
401 additions
and
47 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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'], | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
{ | ||
"name": "@typescript-eslint/eslint-plugin-internal", | ||
"version": "2.13.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.13.0" | ||
}, | ||
"devDependencies": {} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import rules from './rules'; | ||
|
||
export = { | ||
rules, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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, | ||
}; |
80 changes: 80 additions & 0 deletions
80
packages/eslint-plugin-internal/src/rules/no-typescript-default-import.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 compatibility 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';", | ||
); | ||
}, | ||
}); | ||
} | ||
}, | ||
}; | ||
}, | ||
}); |
52 changes: 52 additions & 0 deletions
52
packages/eslint-plugin-internal/src/rules/no-typescript-estree-import.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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, | ||
); | ||
}, | ||
}); | ||
} | ||
}, | ||
}; | ||
}, | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from './createRule'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 }; |
45 changes: 45 additions & 0 deletions
45
packages/eslint-plugin-internal/tests/rules/no-typescript-default-import.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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, | ||
}, | ||
], | ||
}), | ||
}); |
Oops, something went wrong.