From 3ebed5aab987744f41b0d6d3fa5b1ad94abc3bf1 Mon Sep 17 00:00:00 2001 From: James Henry Date: Tue, 30 Apr 2019 20:04:45 -0400 Subject: [PATCH] feat(ts-estree): add preserveNodeMaps option --- packages/typescript-estree/README.md | 14 ++- .../typescript-estree/src/ast-converter.ts | 8 +- packages/typescript-estree/src/convert.ts | 6 +- .../typescript-estree/src/parser-options.ts | 2 + packages/typescript-estree/src/parser.ts | 27 ++++- .../typescript-estree/tests/lib/convert.ts | 18 ++-- packages/typescript-estree/tests/lib/parse.ts | 101 ++++++++++++++++++ 7 files changed, 155 insertions(+), 21 deletions(-) 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/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(); + }); + }); });