diff --git a/packages/eslint-plugin/src/configs/README.md b/packages/eslint-plugin/src/configs/README.md index c9e8d9769f9..3ba76f661e6 100644 --- a/packages/eslint-plugin/src/configs/README.md +++ b/packages/eslint-plugin/src/configs/README.md @@ -6,6 +6,19 @@ These configs exist for your convenience. They contain configuration intended to TODO when all config is added. +## eslint-recommended + +The `eslint-recommended` ruleset is meant to be used after extending `eslint:recommended`. It disables rules that are already checked by the Typescript compiler and enables rules that promote using more the more modern constructs Typescript allows for. + +```cjson +{ + "extends": [ + "eslint:recommended", + "plugin:@typescript-eslint/eslint-recommended" + ] +} +``` + ## Recommended The recommended set is an **_opinionated_** set of rules that we think you should use because: diff --git a/packages/eslint-plugin/src/configs/eslint-recommended.ts b/packages/eslint-plugin/src/configs/eslint-recommended.ts new file mode 100644 index 00000000000..3e47d860173 --- /dev/null +++ b/packages/eslint-plugin/src/configs/eslint-recommended.ts @@ -0,0 +1,49 @@ +/** + * The goal of this ruleset is to update the eslint:recommended config to better + * suit Typescript. There are two main reasons to change the configuration: + * 1. The Typescript compiler natively checks some things that therefore don't + * need extra rules anymore. + * 2. Typescript allows for more modern Javascript code that can thus be + * enabled. + */ +export default { + overrides: [ + { + files: ['*.ts', '*.tsx'], + rules: { + /** + * 1. Disable things that are checked by Typescript + */ + //Checked by Typescript - ts(2378) + 'getter-return': 'off', + // Checked by Typescript - ts(2300) + 'no-dupe-args': 'off', + // Checked by Typescript - ts(1117) + 'no-dupe-keys': 'off', + // Checked by Typescript - ts(7027) + 'no-unreachable': 'off', + // Checked by Typescript - ts(2367) + 'valid-typeof': 'off', + // Checked by Typescript - ts(2588) + 'no-const-assign': 'off', + // Checked by Typescript - ts(2588) + 'no-new-symbol': 'off', + // Checked by Typescript - ts(2376) + 'no-this-before-super': 'off', + // This is checked by Typescript using the option `strictNullChecks`. + 'no-undef': 'off', + /** + * 2. Enable more ideomatic code + */ + // Typescript allows const and let instead of var. + 'no-var': 'error', + 'prefer-const': 'error', + // The spread operator/rest parameters should be prefered in Typescript. + 'prefer-rest-params': 'error', + 'prefer-spread': 'error', + // This is already checked by Typescript. + 'no-dupe-class-members': 'error', + }, + }, + ], +}; diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts index a69361543aa..0b63396576a 100644 --- a/packages/eslint-plugin/src/index.ts +++ b/packages/eslint-plugin/src/index.ts @@ -2,6 +2,7 @@ import requireIndex from 'requireindex'; import path from 'path'; import recommended from './configs/recommended.json'; +import eslintRecommended from './configs/eslint-recommended'; const rules = requireIndex(path.join(__dirname, 'rules')); // eslint expects the rule to be on rules[name], not rules[name].default @@ -18,5 +19,6 @@ export = { rules: rulesWithoutDefault, configs: { recommended, + eslintRecommended, }, }; diff --git a/packages/typescript-estree/README.md b/packages/typescript-estree/README.md index 37ccdd18fc9..8cb7d48b3e0 100644 --- a/packages/typescript-estree/README.md +++ b/packages/typescript-estree/README.md @@ -66,7 +66,19 @@ Parses the given string of code with the options provided and returns an ESTree- * When value is `false`, no logging will occur. * When value is not provided, `console.log()` will be used. */ - loggerFn: undefined + loggerFn: undefined, + + /** + * Allows the user to control whether or not two-way AST node maps are preserved + * during the AST conversion process. + * + * By default: the AST node maps are NOT preserved, unless `project` has been specified, + * in which case the maps are made available on the returned `parserServices`. + * + * NOTE: If `preserveNodeMaps` is explicitly set by the user, it will be respected, + * regardless of whether or not `project` is in use. + */ + preserveNodeMaps: undefined } ``` diff --git a/packages/typescript-estree/src/ast-converter.ts b/packages/typescript-estree/src/ast-converter.ts index ac966c0889b..72ff381f87e 100644 --- a/packages/typescript-estree/src/ast-converter.ts +++ b/packages/typescript-estree/src/ast-converter.ts @@ -7,7 +7,7 @@ import { Extra } from './parser-options'; export default function astConverter( ast: ts.SourceFile, extra: Extra, - shouldProvideParserServices: boolean, + shouldPreserveNodeMaps: boolean, ) { /** * The TypeScript compiler produced fundamental parse errors when parsing the @@ -23,7 +23,7 @@ export default function astConverter( const instance = new Converter(ast, { errorOnUnknownASTType: extra.errorOnUnknownASTType || false, useJSXTextNode: extra.useJSXTextNode || false, - shouldProvideParserServices, + shouldPreserveNodeMaps, }); const estree = instance.convertProgram(); @@ -42,9 +42,7 @@ export default function astConverter( estree.comments = convertComments(ast, extra.code); } - const astMaps = shouldProvideParserServices - ? instance.getASTMaps() - : undefined; + const astMaps = shouldPreserveNodeMaps ? instance.getASTMaps() : undefined; return { estree, astMaps }; } diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index d9f4486159e..1c70dbf8642 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -26,7 +26,7 @@ const SyntaxKind = ts.SyntaxKind; interface ConverterOptions { errorOnUnknownASTType: boolean; useJSXTextNode: boolean; - shouldProvideParserServices: boolean; + shouldPreserveNodeMaps: boolean; } /** @@ -168,7 +168,7 @@ export class Converter { node: ts.Node, result: TSESTree.BaseNode | null, ) { - if (result && this.options.shouldProvideParserServices) { + if (result && this.options.shouldPreserveNodeMaps) { if (!this.tsNodeToESTreeNodeMap.has(node)) { this.tsNodeToESTreeNodeMap.set(node, result); } @@ -217,7 +217,7 @@ export class Converter { result.loc = getLocFor(result.range[0], result.range[1], this.ast); } - if (result && this.options.shouldProvideParserServices) { + if (result && this.options.shouldPreserveNodeMaps) { this.esTreeNodeToTSNodeMap.set(result, node); } return result as T; diff --git a/packages/typescript-estree/src/parser-options.ts b/packages/typescript-estree/src/parser-options.ts index 9dadb456483..5d1821bd50c 100644 --- a/packages/typescript-estree/src/parser-options.ts +++ b/packages/typescript-estree/src/parser-options.ts @@ -18,6 +18,7 @@ export interface Extra { projects: string[]; tsconfigRootDir: string; extraFileExtensions: string[]; + preserveNodeMaps?: boolean; } export interface ParserOptions { @@ -34,6 +35,7 @@ export interface ParserOptions { filePath?: string; tsconfigRootDir?: string; extraFileExtensions?: string[]; + preserveNodeMaps?: boolean; } export interface ParserWeakMap { diff --git a/packages/typescript-estree/src/parser.ts b/packages/typescript-estree/src/parser.ts index 8504513ef2e..2f4003c8e3d 100644 --- a/packages/typescript-estree/src/parser.ts +++ b/packages/typescript-estree/src/parser.ts @@ -57,6 +57,7 @@ function resetExtra(): void { code: '', tsconfigRootDir: process.cwd(), extraFileExtensions: [], + preserveNodeMaps: undefined, }; } @@ -241,6 +242,18 @@ function applyParserOptionsToExtra(options: ParserOptions): void { ) { extra.extraFileExtensions = options.extraFileExtensions; } + /** + * Allow the user to enable or disable the preservation of the AST node maps + * during the conversion process. + * + * NOTE: For backwards compatibility we also preserve node maps in the case where `project` is set, + * and `preserveNodeMaps` is not explicitly set to anything. + */ + extra.preserveNodeMaps = + typeof options.preserveNodeMaps === 'boolean' && options.preserveNodeMaps; + if (options.preserveNodeMaps === undefined && extra.projects.length > 0) { + extra.preserveNodeMaps = true; + } } function warnAboutTSVersion(): void { @@ -372,11 +385,19 @@ export function parseAndGenerateServices< options, shouldProvideParserServices, ); + /** + * Determine whether or not two-way maps of converted AST nodes should be preserved + * during the conversion process + */ + const shouldPreserveNodeMaps = + extra.preserveNodeMaps !== undefined + ? extra.preserveNodeMaps + : shouldProvideParserServices; /** * Convert the TypeScript AST to an ESTree-compatible one, and optionally preserve * mappings between converted and original AST nodes */ - const { estree, astMaps } = convert(ast, extra, shouldProvideParserServices); + const { estree, astMaps } = convert(ast, extra, shouldPreserveNodeMaps); /** * Even if TypeScript parsed the source code ok, and we had no problems converting the AST, * there may be other syntactic or semantic issues in the code that we can optionally report on. @@ -395,11 +416,11 @@ export function parseAndGenerateServices< services: { program: shouldProvideParserServices ? program : undefined, esTreeNodeToTSNodeMap: - shouldProvideParserServices && astMaps + shouldPreserveNodeMaps && astMaps ? astMaps.esTreeNodeToTSNodeMap : undefined, tsNodeToESTreeNodeMap: - shouldProvideParserServices && astMaps + shouldPreserveNodeMaps && astMaps ? astMaps.tsNodeToESTreeNodeMap : undefined, }, diff --git a/packages/typescript-estree/src/tsconfig-parser.ts b/packages/typescript-estree/src/tsconfig-parser.ts index 6ab8047b0be..0f4a9b7c35e 100644 --- a/packages/typescript-estree/src/tsconfig-parser.ts +++ b/packages/typescript-estree/src/tsconfig-parser.ts @@ -89,7 +89,10 @@ export function calculateProjectParserOptions( if (typeof existingWatch !== 'undefined') { // get new program (updated if necessary) - results.push(existingWatch.getProgram().getProgram()); + const updatedProgram = existingWatch.getProgram().getProgram(); + updatedProgram.getTypeChecker(); // sets parent pointers in source files + results.push(updatedProgram); + continue; } diff --git a/packages/typescript-estree/tests/lib/convert.ts b/packages/typescript-estree/tests/lib/convert.ts index b173885d040..d4c694379cb 100644 --- a/packages/typescript-estree/tests/lib/convert.ts +++ b/packages/typescript-estree/tests/lib/convert.ts @@ -18,7 +18,7 @@ describe('convert', () => { const instance = new Converter(ast, { errorOnUnknownASTType: false, useJSXTextNode: false, - shouldProvideParserServices: false, + shouldPreserveNodeMaps: false, }); expect(instance.convertProgram()).toMatchSnapshot(); }); @@ -29,7 +29,7 @@ describe('convert', () => { const instance = new Converter(ast, { errorOnUnknownASTType: false, useJSXTextNode: false, - shouldProvideParserServices: false, + shouldPreserveNodeMaps: false, }); expect((instance as any).deeplyCopy(ast.statements[0])).toMatchSnapshot(); }); @@ -40,7 +40,7 @@ describe('convert', () => { const instance = new Converter(ast, { errorOnUnknownASTType: false, useJSXTextNode: false, - shouldProvideParserServices: false, + shouldPreserveNodeMaps: false, }); expect((instance as any).deeplyCopy(ast.statements[0])).toMatchSnapshot(); }); @@ -51,7 +51,7 @@ describe('convert', () => { const instance = new Converter(ast, { errorOnUnknownASTType: false, useJSXTextNode: false, - shouldProvideParserServices: false, + shouldPreserveNodeMaps: false, }); expect( (instance as any).deeplyCopy((ast.statements[0] as any).expression), @@ -64,7 +64,7 @@ describe('convert', () => { const instance = new Converter(ast, { errorOnUnknownASTType: false, useJSXTextNode: false, - shouldProvideParserServices: false, + shouldPreserveNodeMaps: false, }); expect((instance as any).deeplyCopy(ast)).toMatchSnapshot(); }); @@ -75,7 +75,7 @@ describe('convert', () => { const instance = new Converter(ast, { errorOnUnknownASTType: true, useJSXTextNode: false, - shouldProvideParserServices: false, + shouldPreserveNodeMaps: false, }); expect(() => instance.convertProgram()).toThrow( 'Unknown AST_NODE_TYPE: "TSJSDocNullableType"', @@ -93,7 +93,7 @@ describe('convert', () => { const instance = new Converter(ast, { errorOnUnknownASTType: false, useJSXTextNode: false, - shouldProvideParserServices: true, + shouldPreserveNodeMaps: true, }); instance.convertProgram(); const maps = instance.getASTMaps(); @@ -127,7 +127,7 @@ describe('convert', () => { const instance = new Converter(ast, { errorOnUnknownASTType: false, useJSXTextNode: false, - shouldProvideParserServices: true, + shouldPreserveNodeMaps: true, }); instance.convertProgram(); const maps = instance.getASTMaps(); @@ -160,7 +160,7 @@ describe('convert', () => { const instance = new Converter(ast, { errorOnUnknownASTType: false, useJSXTextNode: false, - shouldProvideParserServices: true, + shouldPreserveNodeMaps: true, }); const program = instance.convertProgram(); const maps = instance.getASTMaps(); diff --git a/packages/typescript-estree/tests/lib/parse.ts b/packages/typescript-estree/tests/lib/parse.ts index c7ed96390a1..c757bad7cde 100644 --- a/packages/typescript-estree/tests/lib/parse.ts +++ b/packages/typescript-estree/tests/lib/parse.ts @@ -88,6 +88,7 @@ describe('parse()', () => { tokens: expect.any(Array), tsconfigRootDir: expect.any(String), useJSXTextNode: false, + preserveNodeMaps: false, }, false, ); @@ -126,4 +127,104 @@ describe('parse()', () => { }).toThrow('Decorators are not valid here.'); }); }); + + describe('preserveNodeMaps', () => { + const code = 'var a = true'; + const baseConfig: ParserOptions = { + comment: true, + tokens: true, + range: true, + loc: true, + }; + + it('should not impact the use of parse()', () => { + const resultWithNoOptionSet = parser.parse(code, baseConfig); + const resultWithOptionSetToTrue = parser.parse(code, { + ...baseConfig, + preserveNodeMaps: true, + }); + const resultWithOptionSetToFalse = parser.parse(code, { + ...baseConfig, + preserveNodeMaps: false, + }); + const resultWithOptionSetExplicitlyToUndefined = parser.parse(code, { + ...baseConfig, + preserveNodeMaps: undefined, + }); + + expect(resultWithNoOptionSet).toMatchObject(resultWithOptionSetToTrue); + expect(resultWithNoOptionSet).toMatchObject(resultWithOptionSetToFalse); + expect(resultWithNoOptionSet).toMatchObject( + resultWithOptionSetExplicitlyToUndefined, + ); + }); + + it('should not preserve node maps by default for parseAndGenerateServices(), unless `project` is set', () => { + const noOptionSet = parser.parseAndGenerateServices(code, baseConfig); + + expect(noOptionSet.services.esTreeNodeToTSNodeMap).toBeUndefined(); + expect(noOptionSet.services.tsNodeToESTreeNodeMap).toBeUndefined(); + + const withProjectNoOptionSet = parser.parseAndGenerateServices(code, { + ...baseConfig, + project: './tsconfig.json', + }); + + expect(withProjectNoOptionSet.services.esTreeNodeToTSNodeMap).toEqual( + expect.any(WeakMap), + ); + expect(withProjectNoOptionSet.services.tsNodeToESTreeNodeMap).toEqual( + expect.any(WeakMap), + ); + }); + + it('should preserve node maps for parseAndGenerateServices() when option is `true`, regardless of `project` config', () => { + const optionSetToTrue = parser.parseAndGenerateServices(code, { + ...baseConfig, + preserveNodeMaps: true, + }); + + expect(optionSetToTrue.services.esTreeNodeToTSNodeMap).toEqual( + expect.any(WeakMap), + ); + expect(optionSetToTrue.services.tsNodeToESTreeNodeMap).toEqual( + expect.any(WeakMap), + ); + + const withProjectOptionSetToTrue = parser.parseAndGenerateServices(code, { + ...baseConfig, + preserveNodeMaps: true, + project: './tsconfig.json', + }); + + expect(withProjectOptionSetToTrue.services.esTreeNodeToTSNodeMap).toEqual( + expect.any(WeakMap), + ); + expect(withProjectOptionSetToTrue.services.tsNodeToESTreeNodeMap).toEqual( + expect.any(WeakMap), + ); + }); + + it('should not preserve node maps for parseAndGenerateServices() when option is `false`, regardless of `project` config', () => { + const optionSetToFalse = parser.parseAndGenerateServices(code, { + ...baseConfig, + preserveNodeMaps: false, + }); + + expect(optionSetToFalse.services.esTreeNodeToTSNodeMap).toBeUndefined(); + expect(optionSetToFalse.services.tsNodeToESTreeNodeMap).toBeUndefined(); + + const withProjectOptionSetToFalse = parser.parseAndGenerateServices( + code, + { ...baseConfig, preserveNodeMaps: false, project: './tsconfig.json' }, + ); + + expect( + withProjectOptionSetToFalse.services.esTreeNodeToTSNodeMap, + ).toBeUndefined(); + expect( + withProjectOptionSetToFalse.services.tsNodeToESTreeNodeMap, + ).toBeUndefined(); + }); + }); });