From 1130b681b97e640738c2dd72ec1c1a8772a40846 Mon Sep 17 00:00:00 2001 From: James Henry Date: Sat, 26 Jan 2019 19:40:08 -0500 Subject: [PATCH 1/3] perf(ts-estree): don't create Program in parse() --- packages/typescript-estree/src/parser.ts | 345 +++++++++++++---------- 1 file changed, 195 insertions(+), 150 deletions(-) diff --git a/packages/typescript-estree/src/parser.ts b/packages/typescript-estree/src/parser.ts index ff2784aa9963..4993dbc691b0 100644 --- a/packages/typescript-estree/src/parser.ts +++ b/packages/typescript-estree/src/parser.ts @@ -36,7 +36,10 @@ let extra: Extra; let warnedAboutTSVersion = false; /** - * Compute the filename based on the parser options + * Compute the filename based on the parser options. + * + * Even if jsx option is set in typescript compiler, filename still has to + * contain .tsx file extension. * * @param options Parser options */ @@ -106,8 +109,6 @@ function getASTAndDefaultProject(code: string, options: ParserOptions) { * @returns {{ast: ts.SourceFile, program: ts.Program}} Returns a new source file and program corresponding to the linted code */ function createNewProgram(code: string) { - // Even if jsx option is set in typescript compiler, filename still has to - // contain .tsx file extension const FILENAME = getFileName(extra); const compilerHost = { @@ -178,6 +179,98 @@ function getProgramAndAST( ); } +function applyParserOptionsToExtra(options: ParserOptions): void { + /** + * Track range information in the AST + */ + extra.range = typeof options.range === 'boolean' && options.range; + extra.loc = typeof options.loc === 'boolean' && options.loc; + /** + * Track tokens in the AST + */ + if (typeof options.tokens === 'boolean' && options.tokens) { + extra.tokens = []; + } + /** + * Track comments in the AST + */ + if (typeof options.comment === 'boolean' && options.comment) { + extra.comment = true; + extra.comments = []; + } + /** + * Enable JSX - note the applicable file extension is still required + */ + if (typeof options.jsx === 'boolean' && options.jsx) { + extra.jsx = true; + } + /** + * The JSX AST changed the node type for string literals + * inside a JSX Element from `Literal` to `JSXText`. + * + * When value is `true`, these nodes will be parsed as type `JSXText`. + * When value is `false`, these nodes will be parsed as type `Literal`. + */ + if (typeof options.useJSXTextNode === 'boolean' && options.useJSXTextNode) { + extra.useJSXTextNode = true; + } + /** + * Allow the user to cause the parser to error if it encounters an unknown AST Node Type + * (used in testing) + */ + if ( + typeof options.errorOnUnknownASTType === 'boolean' && + options.errorOnUnknownASTType + ) { + extra.errorOnUnknownASTType = true; + } + /** + * Allow the user to override the function used for logging + */ + if (typeof options.loggerFn === 'function') { + extra.log = options.loggerFn; + } else if (options.loggerFn === false) { + extra.log = Function.prototype; + } + + if (typeof options.project === 'string') { + extra.projects = [options.project]; + } else if ( + Array.isArray(options.project) && + options.project.every(projectPath => typeof projectPath === 'string') + ) { + extra.projects = options.project; + } + + if (typeof options.tsconfigRootDir === 'string') { + extra.tsconfigRootDir = options.tsconfigRootDir; + } + + if ( + Array.isArray(options.extraFileExtensions) && + options.extraFileExtensions.every(ext => typeof ext === 'string') + ) { + extra.extraFileExtensions = options.extraFileExtensions; + } +} + +function warnAboutTSVersion(): void { + if (!isRunningSupportedTypeScriptVersion && !warnedAboutTSVersion) { + const border = '============='; + const versionWarning = [ + border, + 'WARNING: You are currently running a version of TypeScript which is not officially supported by typescript-estree.', + 'You may find that it works just fine, or you may not.', + `SUPPORTED TYPESCRIPT VERSIONS: ${SUPPORTED_TYPESCRIPT_VERSIONS}`, + `YOUR TYPESCRIPT VERSION: ${ACTIVE_TYPESCRIPT_VERSION}`, + 'Please only submit bug reports when using the officially supported version.', + border + ]; + extra.log(versionWarning.join('\n\n')); + warnedAboutTSVersion = true; + } +} + //------------------------------------------------------------------------------ // Parser //------------------------------------------------------------------------------ @@ -187,143 +280,118 @@ type AST = Program & (T['tokens'] extends true ? { tokens: ESTreeToken[] } : {}) & (T['comment'] extends true ? { comments: ESTreeComment[] } : {}); -/** - * Parses the given source code to produce a valid AST - * @param {string} code TypeScript code - * @param {boolean} shouldGenerateServices Flag determining whether to generate ast maps and program or not - * @param {ParserOptions} options configuration object for the parser - * @returns {Object} the AST - */ -function generateAST( +interface ParseAndGenerateServicesResult { + ast: AST; + services: { + program: ts.Program | undefined; + esTreeNodeToTSNodeMap: WeakMap | undefined; + tsNodeToESTreeNodeMap: WeakMap | undefined; + }; +} + +//------------------------------------------------------------------------------ +// Public +//------------------------------------------------------------------------------ + +export const version: string = packageJSON.version; + +export function parse( code: string, - options: T = {} as T, - shouldGenerateServices = false -): { - estree: AST; - program: typeof shouldGenerateServices extends true - ? ts.Program - : (ts.Program | undefined); - astMaps: typeof shouldGenerateServices extends true - ? { - esTreeNodeToTSNodeMap: WeakMap; - tsNodeToESTreeNodeMap: WeakMap; - } - : { - esTreeNodeToTSNodeMap?: WeakMap; - tsNodeToESTreeNodeMap?: WeakMap; - }; -} { + options?: T +): AST { + /** + * Reset the parse configuration + */ + resetExtra(); + /** + * Ensure users do not attempt to use parse() when they need parseAndGenerateServices() + */ + if (options && options.errorOnTypeScriptSyntacticAndSemanticIssues) { + throw new Error( + `"errorOnTypeScriptSyntacticAndSemanticIssues" is only supported for parseAndGenerateServices()` + ); + } + /** + * Ensure the source code is a string, and store a reference to it + */ if (typeof code !== 'string' && !((code as any) instanceof String)) { code = String(code); } + extra.code = code; + /** + * Apply the given parser options + */ + if (typeof options !== 'undefined') { + applyParserOptionsToExtra(options); + } + /** + * Warn if the user is using an unsupported version of TypeScript + */ + warnAboutTSVersion(); + /** + * Create a ts.SourceFile directly, no ts.Program is needed for a simple + * parse + */ + const ast = ts.createSourceFile( + getFileName(extra), + code, + ts.ScriptTarget.Latest, + /* setParentNodes */ true + ); + /** + * Convert the TypeScript AST to an ESTree-compatible one + */ + const { estree } = convert(ast, extra, false); + return estree; +} +export function parseAndGenerateServices< + T extends ParserOptions = ParserOptions +>(code: string, options: T): ParseAndGenerateServicesResult { + /** + * Reset the parse configuration + */ resetExtra(); - + /** + * Ensure the source code is a string, and store a reference to it + */ + if (typeof code !== 'string' && !((code as any) instanceof String)) { + code = String(code); + } + extra.code = code; + /** + * Apply the given parser options + */ if (typeof options !== 'undefined') { - extra.range = typeof options.range === 'boolean' && options.range; - extra.loc = typeof options.loc === 'boolean' && options.loc; - - if (typeof options.tokens === 'boolean' && options.tokens) { - extra.tokens = []; - } - - if (typeof options.comment === 'boolean' && options.comment) { - extra.comment = true; - extra.comments = []; - } - - if (typeof options.jsx === 'boolean' && options.jsx) { - extra.jsx = true; - } - - /** - * Allow the user to cause the parser to error if it encounters an unknown AST Node Type - * (used in testing). - */ - if ( - typeof options.errorOnUnknownASTType === 'boolean' && - options.errorOnUnknownASTType - ) { - extra.errorOnUnknownASTType = true; - } - - /** - * Retrieve semantic and syntactic diagnostics from the underlying TypeScript Program - * and turn them into parse errors - */ + applyParserOptionsToExtra(options); if ( - shouldGenerateServices && typeof options.errorOnTypeScriptSyntacticAndSemanticIssues === 'boolean' && options.errorOnTypeScriptSyntacticAndSemanticIssues ) { extra.errorOnTypeScriptSyntacticAndSemanticIssues = true; } - - if (typeof options.useJSXTextNode === 'boolean' && options.useJSXTextNode) { - extra.useJSXTextNode = true; - } - - /** - * Allow the user to override the function used for logging - */ - if (typeof options.loggerFn === 'function') { - extra.log = options.loggerFn; - } else if (options.loggerFn === false) { - extra.log = Function.prototype; - } - - if (typeof options.project === 'string') { - extra.projects = [options.project]; - } else if ( - Array.isArray(options.project) && - options.project.every(projectPath => typeof projectPath === 'string') - ) { - extra.projects = options.project; - } - - if (typeof options.tsconfigRootDir === 'string') { - extra.tsconfigRootDir = options.tsconfigRootDir; - } - - if ( - Array.isArray(options.extraFileExtensions) && - options.extraFileExtensions.every(ext => typeof ext === 'string') - ) { - extra.extraFileExtensions = options.extraFileExtensions; - } } - - if (!isRunningSupportedTypeScriptVersion && !warnedAboutTSVersion) { - const border = '============='; - const versionWarning = [ - border, - 'WARNING: You are currently running a version of TypeScript which is not officially supported by typescript-estree.', - 'You may find that it works just fine, or you may not.', - `SUPPORTED TYPESCRIPT VERSIONS: ${SUPPORTED_TYPESCRIPT_VERSIONS}`, - `YOUR TYPESCRIPT VERSION: ${ACTIVE_TYPESCRIPT_VERSION}`, - 'Please only submit bug reports when using the officially supported version.', - border - ]; - extra.log(versionWarning.join('\n\n')); - warnedAboutTSVersion = true; - } - + /** + * Warn if the user is using an unsupported version of TypeScript + */ + warnAboutTSVersion(); + /** + * Generate a full ts.Program in order to be able to provide parser + * services, such as type-checking + */ const shouldProvideParserServices = - shouldGenerateServices && extra.projects && extra.projects.length > 0; + extra.projects && extra.projects.length > 0; const { ast, program } = getProgramAndAST( code, options, shouldProvideParserServices ); - - extra.code = code; - /** - * Convert the AST + * 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); - /** * 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. @@ -334,42 +402,19 @@ function generateAST( throw convertError(error); } } - - return { - estree, - program: shouldProvideParserServices ? program : undefined, - astMaps: shouldProvideParserServices - ? astMaps! - : { esTreeNodeToTSNodeMap: undefined, tsNodeToESTreeNodeMap: undefined } - }; -} - -//------------------------------------------------------------------------------ -// Public -//------------------------------------------------------------------------------ - -export const version: string = packageJSON.version; - -export function parse( - code: string, - options?: T -) { - if (options && options.errorOnTypeScriptSyntacticAndSemanticIssues) { - throw new Error( - `"errorOnTypeScriptSyntacticAndSemanticIssues" is only supported for parseAndGenerateServices()` - ); - } - return generateAST(code, options).estree; -} - -export function parseAndGenerateServices(code: string, options: ParserOptions) { - const result = generateAST(code, options, /*shouldGenerateServices*/ true); + /** + * Return the converted AST and additional parser services + */ return { - ast: result.estree, + ast: estree, services: { - program: result.program, - esTreeNodeToTSNodeMap: result.astMaps.esTreeNodeToTSNodeMap, - tsNodeToESTreeNodeMap: result.astMaps.tsNodeToESTreeNodeMap + program: shouldProvideParserServices ? program : undefined, + esTreeNodeToTSNodeMap: shouldProvideParserServices + ? astMaps!.esTreeNodeToTSNodeMap + : undefined, + tsNodeToESTreeNodeMap: shouldProvideParserServices + ? astMaps!.tsNodeToESTreeNodeMap + : undefined } }; } From d02e6b39c98c0654b51488f790ae6472f21515a8 Mon Sep 17 00:00:00 2001 From: James Henry Date: Mon, 28 Jan 2019 21:12:43 -0500 Subject: [PATCH 2/3] improve coverage --- .../tests/lib/__snapshots__/parse.ts.snap | 82 ++++++++++++++++++- packages/typescript-estree/tests/lib/parse.ts | 7 +- .../tests/lib/semanticInfo.ts | 18 ++++ .../tests/lib/warn-on-unsupported-ts.ts | 22 +++++ 4 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 packages/typescript-estree/tests/lib/warn-on-unsupported-ts.ts diff --git a/packages/typescript-estree/tests/lib/__snapshots__/parse.ts.snap b/packages/typescript-estree/tests/lib/__snapshots__/parse.ts.snap index 4509f7441037..38e378d9f7e0 100644 --- a/packages/typescript-estree/tests/lib/__snapshots__/parse.ts.snap +++ b/packages/typescript-estree/tests/lib/__snapshots__/parse.ts.snap @@ -189,7 +189,87 @@ Object { } `; -exports[`parse() non string code should correctly convert code to string 1`] = ` +exports[`parse() non string code should correctly convert code to a string for parse() 1`] = ` +Object { + "body": Array [ + Object { + "expression": Object { + "loc": Object { + "end": Object { + "column": 5, + "line": 1, + }, + "start": Object { + "column": 0, + "line": 1, + }, + }, + "range": Array [ + 0, + 5, + ], + "raw": "12345", + "type": "Literal", + "value": 12345, + }, + "loc": Object { + "end": Object { + "column": 5, + "line": 1, + }, + "start": Object { + "column": 0, + "line": 1, + }, + }, + "range": Array [ + 0, + 5, + ], + "type": "ExpressionStatement", + }, + ], + "comments": Array [], + "loc": Object { + "end": Object { + "column": 5, + "line": 1, + }, + "start": Object { + "column": 0, + "line": 1, + }, + }, + "range": Array [ + 0, + 5, + ], + "sourceType": "script", + "tokens": Array [ + Object { + "loc": Object { + "end": Object { + "column": 5, + "line": 1, + }, + "start": Object { + "column": 0, + "line": 1, + }, + }, + "range": Array [ + 0, + 5, + ], + "type": "Numeric", + "value": "12345", + }, + ], + "type": "Program", +} +`; + +exports[`parse() non string code should correctly convert code to a string for parseAndGenerateServices() 1`] = ` Object { "body": Array [ Object { diff --git a/packages/typescript-estree/tests/lib/parse.ts b/packages/typescript-estree/tests/lib/parse.ts index 74f02d03792c..fc7e8af99a10 100644 --- a/packages/typescript-estree/tests/lib/parse.ts +++ b/packages/typescript-estree/tests/lib/parse.ts @@ -57,9 +57,14 @@ describe('parse()', () => { }; it( - 'should correctly convert code to string', + 'should correctly convert code to a string for parse()', createSnapshotTestBlock(code, config) ); + + it( + 'should correctly convert code to a string for parseAndGenerateServices()', + createSnapshotTestBlock(code, config, true) + ); }); describe('loggerFn should be propagated to ast-converter', () => { diff --git a/packages/typescript-estree/tests/lib/semanticInfo.ts b/packages/typescript-estree/tests/lib/semanticInfo.ts index eb5af8677e97..e45cae843ed5 100644 --- a/packages/typescript-estree/tests/lib/semanticInfo.ts +++ b/packages/typescript-estree/tests/lib/semanticInfo.ts @@ -17,6 +17,7 @@ import { formatSnapshotName, parseCodeAndGenerateServices } from '../../tools/test-utils'; +import { parseAndGenerateServices } from '../../src/parser'; //------------------------------------------------------------------------------ // Setup @@ -59,6 +60,23 @@ describe('semanticInfo', () => { ); }); + it(`should handle "project": "./tsconfig.json" and "project": ["./tsconfig.json"] the same`, () => { + const filename = testFiles[0]; + const code = readFileSync(filename, 'utf8'); + const options = createOptions(filename); + const optionsProjectString = { + ...options, + project: './tsconfig.json' + }; + const optionsProjectArray = { + ...options, + project: ['./tsconfig.json'] + }; + expect(parseAndGenerateServices(code, optionsProjectString)).toEqual( + parseAndGenerateServices(code, optionsProjectArray) + ); + }); + // case-specific tests it('isolated-file tests', () => { const fileName = resolve(FIXTURES_DIR, 'isolated-file.src.ts'); diff --git a/packages/typescript-estree/tests/lib/warn-on-unsupported-ts.ts b/packages/typescript-estree/tests/lib/warn-on-unsupported-ts.ts new file mode 100644 index 000000000000..d984007bf757 --- /dev/null +++ b/packages/typescript-estree/tests/lib/warn-on-unsupported-ts.ts @@ -0,0 +1,22 @@ +import semver from 'semver'; +import * as parser from '../../src/parser'; + +jest.mock('semver'); + +describe('Warn on unsupported TypeScript version', () => { + afterEach(() => { + jest.resetModules(); + jest.resetAllMocks(); + }); + + it('should warn the user if they are using an unsupported TypeScript version', () => { + (semver.satisfies as jest.Mock).mockReturnValue(false); + console.log = jest.fn(); + parser.parse(''); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining( + 'WARNING: You are currently running a version of TypeScript which is not officially supported by typescript-estree' + ) + ); + }); +}); From 7a373fdef203633bd204be114b435a478c72ef1c Mon Sep 17 00:00:00 2001 From: James Henry Date: Mon, 28 Jan 2019 21:13:44 -0500 Subject: [PATCH 3/3] check for astMaps --- packages/typescript-estree/src/parser.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/typescript-estree/src/parser.ts b/packages/typescript-estree/src/parser.ts index 4993dbc691b0..225899a106f9 100644 --- a/packages/typescript-estree/src/parser.ts +++ b/packages/typescript-estree/src/parser.ts @@ -409,12 +409,14 @@ export function parseAndGenerateServices< ast: estree, services: { program: shouldProvideParserServices ? program : undefined, - esTreeNodeToTSNodeMap: shouldProvideParserServices - ? astMaps!.esTreeNodeToTSNodeMap - : undefined, - tsNodeToESTreeNodeMap: shouldProvideParserServices - ? astMaps!.tsNodeToESTreeNodeMap - : undefined + esTreeNodeToTSNodeMap: + shouldProvideParserServices && astMaps + ? astMaps.esTreeNodeToTSNodeMap + : undefined, + tsNodeToESTreeNodeMap: + shouldProvideParserServices && astMaps + ? astMaps.tsNodeToESTreeNodeMap + : undefined } }; }