From 06c2d9ba5442330f56637ecb14fae7e41696699c Mon Sep 17 00:00:00 2001 From: James Henry Date: Fri, 11 Jun 2021 23:44:46 +0400 Subject: [PATCH] feat(typescript-estree): add opt-in inference for single runs and create programs for projects up front (#3512) --- .eslintrc.js | 1 + packages/typescript-estree/README.md | 14 + .../src/create-program/createWatchProgram.ts | 4 +- .../src/create-program/useProvidedPrograms.ts | 2 +- packages/typescript-estree/src/index.ts | 2 +- .../typescript-estree/src/parser-options.ts | 21 +- packages/typescript-estree/src/parser.ts | 109 +++++++- .../tests/lib/persistentParse.test.ts | 5 +- .../tests/lib/semanticInfo-singleRun.test.ts | 248 ++++++++++++++++++ .../tests/lib/semanticInfo.test.ts | 18 +- 10 files changed, 399 insertions(+), 25 deletions(-) create mode 100644 packages/typescript-estree/tests/lib/semanticInfo-singleRun.test.ts diff --git a/.eslintrc.js b/.eslintrc.js index 41f25f55671..b39e5187bc7 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -25,6 +25,7 @@ module.exports = { './tests/integration/utils/jsconfig.json', './packages/*/tsconfig.json', ], + allowAutomaticSingleRunInference: true, tsconfigRootDir: __dirname, warnOnUnsupportedTypeScriptVersion: false, EXPERIMENTAL_useSourceOfProjectReferenceRedirect: false, diff --git a/packages/typescript-estree/README.md b/packages/typescript-estree/README.md index fca834e2209..31084150646 100644 --- a/packages/typescript-estree/README.md +++ b/packages/typescript-estree/README.md @@ -225,6 +225,20 @@ interface ParseAndGenerateServicesOptions extends ParseOptions { * it will not error, but will instead parse the file and its dependencies in a new program. */ createDefaultProgram?: boolean; + + /** + * ESLint (and therefore typescript-eslint) is used in both "single run"/one-time contexts, + * such as an ESLint CLI invocation, and long-running sessions (such as continuous feedback + * on a file in an IDE). + * + * When typescript-eslint handles TypeScript Program management behind the scenes, this distinction + * is important because there is significant overhead to managing the so called Watch Programs + * needed for the long-running use-case. + * + * When allowAutomaticSingleRunInference is enabled, we will use common heuristics to infer + * whether or not ESLint is being used as part of a single run. + */ + allowAutomaticSingleRunInference?: boolean; } interface ParserServices { diff --git a/packages/typescript-estree/src/create-program/createWatchProgram.ts b/packages/typescript-estree/src/create-program/createWatchProgram.ts index e2bf060050c..f10f2a2295f 100644 --- a/packages/typescript-estree/src/create-program/createWatchProgram.ts +++ b/packages/typescript-estree/src/create-program/createWatchProgram.ts @@ -50,7 +50,7 @@ const parsedFilesSeenHash = new Map(); * Clear all of the parser caches. * This should only be used in testing to ensure the parser is clean between tests. */ -function clearCaches(): void { +function clearWatchCaches(): void { knownWatchProgramMap.clear(); fileWatchCallbackTrackingMap.clear(); folderWatchCallbackTrackingMap.clear(); @@ -530,4 +530,4 @@ function maybeInvalidateProgram( return null; } -export { clearCaches, createWatchProgram, getProgramsForProjects }; +export { clearWatchCaches, createWatchProgram, getProgramsForProjects }; diff --git a/packages/typescript-estree/src/create-program/useProvidedPrograms.ts b/packages/typescript-estree/src/create-program/useProvidedPrograms.ts index d807a0f4675..23b56497dc6 100644 --- a/packages/typescript-estree/src/create-program/useProvidedPrograms.ts +++ b/packages/typescript-estree/src/create-program/useProvidedPrograms.ts @@ -12,7 +12,7 @@ import { const log = debug('typescript-eslint:typescript-estree:useProvidedProgram'); function useProvidedPrograms( - programInstances: ts.Program[], + programInstances: Iterable, extra: Extra, ): ASTAndProgram | undefined { log( diff --git a/packages/typescript-estree/src/index.ts b/packages/typescript-estree/src/index.ts index 3345d4d46ce..b2a0581dc92 100644 --- a/packages/typescript-estree/src/index.ts +++ b/packages/typescript-estree/src/index.ts @@ -2,7 +2,7 @@ export * from './parser'; export { ParserServices, TSESTreeOptions } from './parser-options'; export { simpleTraverse } from './simple-traverse'; export * from './ts-estree'; -export { clearCaches } from './create-program/createWatchProgram'; +export { clearWatchCaches as clearCaches } from './create-program/createWatchProgram'; export { createProgramFromConfigFile as createProgram } from './create-program/useProvidedPrograms'; // re-export for backwards-compat diff --git a/packages/typescript-estree/src/parser-options.ts b/packages/typescript-estree/src/parser-options.ts index 9e33627ac9a..0bde1d9176a 100644 --- a/packages/typescript-estree/src/parser-options.ts +++ b/packages/typescript-estree/src/parser-options.ts @@ -1,7 +1,7 @@ import { DebugLevel } from '@typescript-eslint/types'; -import { Program } from 'typescript'; -import { TSESTree, TSNode, TSESTreeToTSNode, TSToken } from './ts-estree'; +import type { Program } from 'typescript'; import { CanonicalPath } from './create-program/shared'; +import { TSESTree, TSESTreeToTSNode, TSNode, TSToken } from './ts-estree'; type DebugModule = 'typescript-eslint' | 'eslint' | 'typescript'; @@ -18,9 +18,10 @@ export interface Extra { filePath: string; jsx: boolean; loc: boolean; + singleRun: boolean; log: (message: string) => void; preserveNodeMaps?: boolean; - programs: null | Program[]; + programs: null | Iterable; projects: CanonicalPath[]; range: boolean; strict: boolean; @@ -187,6 +188,20 @@ interface ParseAndGenerateServicesOptions extends ParseOptions { * it will not error, but will instead parse the file and its dependencies in a new program. */ createDefaultProgram?: boolean; + + /** + * ESLint (and therefore typescript-eslint) is used in both "single run"/one-time contexts, + * such as an ESLint CLI invocation, and long-running sessions (such as continuous feedback + * on a file in an IDE). + * + * When typescript-eslint handles TypeScript Program management behind the scenes, this distinction + * is important because there is significant overhead to managing the so called Watch Programs + * needed for the long-running use-case. + * + * When allowAutomaticSingleRunInference is enabled, we will use common heuristics to infer + * whether or not ESLint is being used as part of a single run. + */ + allowAutomaticSingleRunInference?: boolean; } export type TSESTreeOptions = ParseAndGenerateServicesOptions; diff --git a/packages/typescript-estree/src/parser.ts b/packages/typescript-estree/src/parser.ts index 7fe00818959..904bb054fdf 100644 --- a/packages/typescript-estree/src/parser.ts +++ b/packages/typescript-estree/src/parser.ts @@ -2,6 +2,7 @@ import debug from 'debug'; import { sync as globSync } from 'globby'; import isGlob from 'is-glob'; import semver from 'semver'; +import { normalize } from 'path'; import * as ts from 'typescript'; import { astConverter } from './ast-converter'; import { convertError } from './convert'; @@ -18,8 +19,10 @@ import { ensureAbsolutePath, getCanonicalFileName, } from './create-program/shared'; -import { Program } from 'typescript'; -import { useProvidedPrograms } from './create-program/useProvidedPrograms'; +import { + createProgramFromConfigFile, + useProvidedPrograms, +} from './create-program/useProvidedPrograms'; const log = debug('typescript-eslint:typescript-estree:parser'); @@ -44,6 +47,16 @@ const isRunningSupportedTypeScriptVersion = semver.satisfies( let extra: Extra; let warnedAboutTSVersion = false; +/** + * Cache existing programs for the single run use-case. + * + * clearProgramCache() is only intended to be used in testing to ensure the parser is clean between tests. + */ +const existingPrograms = new Map(); +function clearProgramCache(): void { + existingPrograms.clear(); +} + function enforceString(code: unknown): string { /** * Ensure the source code is a string @@ -57,20 +70,19 @@ function enforceString(code: unknown): string { /** * @param code The code of the file being linted - * @param programInstances One or more existing programs to use + * @param programInstances One or more (potentially lazily constructed) existing programs to use * @param shouldProvideParserServices True if the program should be attempted to be calculated from provided tsconfig files * @param shouldCreateDefaultProgram True if the program should be created from compiler host * @returns Returns a source file and program corresponding to the linted code */ function getProgramAndAST( code: string, - programInstances: Program[] | null, + programInstances: Iterable | null, shouldProvideParserServices: boolean, shouldCreateDefaultProgram: boolean, ): ASTAndProgram { return ( - (programInstances?.length && - useProvidedPrograms(programInstances, extra)) || + (programInstances && useProvidedPrograms(programInstances, extra)) || (shouldProvideParserServices && createProjectProgram(code, shouldCreateDefaultProgram, extra)) || (shouldProvideParserServices && @@ -118,6 +130,11 @@ function resetExtra(): void { tokens: null, tsconfigRootDir: process.cwd(), useJSXTextNode: false, + /** + * Unless we can reliably infer otherwise, we default to assuming that this run could be part + * of a long-running session (e.g. in an IDE) and watch programs will therefore be required + */ + singleRun: false, }; } @@ -347,6 +364,47 @@ function warnAboutTSVersion(): void { } } +/** + * ESLint (and therefore typescript-eslint) is used in both "single run"/one-time contexts, + * such as an ESLint CLI invocation, and long-running sessions (such as continuous feedback + * on a file in an IDE). + * + * When typescript-eslint handles TypeScript Program management behind the scenes, this distinction + * is important because there is significant overhead to managing the so called Watch Programs + * needed for the long-running use-case. We therefore use the following logic to figure out which + * of these contexts applies to the current execution. + */ +function inferSingleRun(options: TSESTreeOptions | undefined): void { + // Allow users to explicitly inform us of their intent to perform a single run (or not) with TSESTREE_SINGLE_RUN + if (process.env.TSESTREE_SINGLE_RUN === 'false') { + extra.singleRun = false; + return; + } + if (process.env.TSESTREE_SINGLE_RUN === 'true') { + extra.singleRun = true; + return; + } + + // Currently behind a flag while we gather real-world feedback + if (options?.allowAutomaticSingleRunInference) { + if ( + // Default to single runs for CI processes. CI=true is set by most CI providers by default. + process.env.CI === 'true' || + // This will be true for invocations such as `npx eslint ...` and `./node_modules/.bin/eslint ...` + process.argv[1].endsWith(normalize('node_modules/.bin/eslint')) + ) { + extra.singleRun = true; + return; + } + } + + /** + * We default to assuming that this run could be part of a long-running session (e.g. in an IDE) + * and watch programs will therefore be required + */ + extra.singleRun = false; +} + // eslint-disable-next-line @typescript-eslint/no-empty-interface interface EmptyObject {} type AST = TSESTree.Program & @@ -408,6 +466,11 @@ function parseWithNodeMapsInternal( */ warnAboutTSVersion(); + /** + * Figure out whether this is a single run or part of a long-running process + */ + inferSingleRun(options); + /** * Create a ts.SourceFile directly, no ts.Program is needed for a simple * parse @@ -468,7 +531,38 @@ function parseAndGenerateServices( warnAboutTSVersion(); /** - * Generate a full ts.Program or offer provided instance in order to be able to provide parser services, such as type-checking + * Figure out whether this is a single run or part of a long-running process + */ + inferSingleRun(options); + + /** + * If this is a single run in which the user has not provided any existing programs but there + * are programs which need to be created from the provided "project" option, + * create an Iterable which will lazily create the programs as needed by the iteration logic + */ + if (extra.singleRun && !extra.programs && extra.projects?.length > 0) { + extra.programs = { + *[Symbol.iterator](): Iterator { + for (const configFile of extra.projects) { + const existingProgram = existingPrograms.get(configFile); + if (existingProgram) { + yield existingProgram; + } else { + log( + 'Detected single-run/CLI usage, creating Program once ahead of time for project: %s', + configFile, + ); + const newProgram = createProgramFromConfigFile(configFile); + existingPrograms.set(configFile, newProgram); + yield newProgram; + } + } + }, + }; + } + + /** + * Generate a full ts.Program or offer provided instances in order to be able to provide parser services, such as type-checking */ const shouldProvideParserServices = extra.programs != null || (extra.projects && extra.projects.length > 0); @@ -519,4 +613,5 @@ export { parseWithNodeMaps, ParseAndGenerateServicesResult, ParseWithNodeMapsResult, + clearProgramCache, }; diff --git a/packages/typescript-estree/tests/lib/persistentParse.test.ts b/packages/typescript-estree/tests/lib/persistentParse.test.ts index 7e751ed4248..50ab0351c16 100644 --- a/packages/typescript-estree/tests/lib/persistentParse.test.ts +++ b/packages/typescript-estree/tests/lib/persistentParse.test.ts @@ -1,7 +1,8 @@ import fs from 'fs'; import path from 'path'; import tmp from 'tmp'; -import { clearCaches, parseAndGenerateServices } from '../../src'; +import { clearWatchCaches } from '../../src/create-program/createWatchProgram'; +import { parseAndGenerateServices } from '../../src/parser'; const CONTENTS = { foo: 'console.log("foo")', @@ -17,7 +18,7 @@ const cwdCopy = process.cwd(); const tmpDirs = new Set(); afterEach(() => { // stop watching the files and folders - clearCaches(); + clearWatchCaches(); // clean up the temporary files and folders tmpDirs.forEach(t => t.removeCallback()); diff --git a/packages/typescript-estree/tests/lib/semanticInfo-singleRun.test.ts b/packages/typescript-estree/tests/lib/semanticInfo-singleRun.test.ts new file mode 100644 index 00000000000..b8e778e0fb0 --- /dev/null +++ b/packages/typescript-estree/tests/lib/semanticInfo-singleRun.test.ts @@ -0,0 +1,248 @@ +import glob from 'glob'; +import * as path from 'path'; +import { clearProgramCache, parseAndGenerateServices } from '../../src'; +import { getCanonicalFileName } from '../../src/create-program/shared'; + +const mockProgram = { + getSourceFile(): void { + return; + }, + getTypeChecker(): void { + return; + }, +}; + +jest.mock('../../src/ast-converter', () => { + return { + astConverter(): unknown { + return { estree: {}, astMaps: {} }; + }, + }; +}); + +interface MockProgramWithConfigFile { + __FROM_CONFIG_FILE__?: string; +} + +jest.mock('../../src/create-program/shared.ts', () => { + return { + ...jest.requireActual('../../src/create-program/shared.ts'), + getAstFromProgram(program: MockProgramWithConfigFile): unknown { + if ( + program.__FROM_CONFIG_FILE__?.endsWith('non-matching-tsconfig.json') + ) { + return null; + } + // Remove temporary tracking value for the config added by mock createProgramFromConfigFile() below + delete program.__FROM_CONFIG_FILE__; + return { ast: {}, program }; + }, + }; +}); + +jest.mock('../../src/create-program/useProvidedPrograms.ts', () => { + return { + ...jest.requireActual('../../src/create-program/useProvidedPrograms.ts'), + createProgramFromConfigFile: jest + .fn() + .mockImplementation((configFile): MockProgramWithConfigFile => { + return { + // So we can differentiate our mock return values based on which tsconfig this is + __FROM_CONFIG_FILE__: configFile, + ...mockProgram, + }; + }), + }; +}); + +jest.mock('../../src/create-program/createWatchProgram', () => { + return { + ...jest.requireActual('../../src/create-program/createWatchProgram'), + getProgramsForProjects: jest.fn(() => [mockProgram]), + }; +}); + +const { + createProgramFromConfigFile, +} = require('../../src/create-program/useProvidedPrograms'); + +const FIXTURES_DIR = './tests/fixtures/semanticInfo'; +const testFiles = glob.sync(`**/*.src.ts`, { + cwd: FIXTURES_DIR, +}); + +const code = 'const foo = 5;'; +// File will not be found in the first Program, but will be in the second +const tsconfigs = ['./non-matching-tsconfig.json', './tsconfig.json']; +const options = { + filePath: testFiles[0], + tsconfigRootDir: path.join(process.cwd(), FIXTURES_DIR), + loggerFn: false, + project: tsconfigs, + allowAutomaticSingleRunInference: true, +} as const; + +const resolvedProject = (p: string): string => + getCanonicalFileName(path.resolve(path.join(process.cwd(), FIXTURES_DIR), p)); + +describe('semanticInfo - singleRun', () => { + beforeEach(() => { + // ensure caches are clean for each test + clearProgramCache(); + // ensure invocations of mock are clean for each test + (createProgramFromConfigFile as jest.Mock).mockClear(); + }); + + it('should not create any programs ahead of time by default when there is no way to infer singleRun=true', () => { + // For when these tests themselves are running in CI, we need to ignore that for this particular spec + const originalEnvCI = process.env.CI; + process.env.CI = 'false'; + + /** + * At this point there is nothing to indicate it is a single run, so createProgramFromConfigFile should + * never be called + */ + parseAndGenerateServices(code, options); + expect(createProgramFromConfigFile).not.toHaveBeenCalled(); + + // Restore process data + process.env.CI = originalEnvCI; + }); + + it('should not create any programs ahead of time when when TSESTREE_SINGLE_RUN=false, even if other inferrence criteria apply', () => { + const originalTSESTreeSingleRun = process.env.TSESTREE_SINGLE_RUN; + process.env.TSESTREE_SINGLE_RUN = 'false'; + + // Normally CI=true would be used to infer singleRun=true, but TSESTREE_SINGLE_RUN is explicitly set to false + const originalEnvCI = process.env.CI; + process.env.CI = 'true'; + + parseAndGenerateServices(code, options); + expect(createProgramFromConfigFile).not.toHaveBeenCalled(); + + // Restore process data + process.env.TSESTREE_SINGLE_RUN = originalTSESTreeSingleRun; + process.env.CI = originalEnvCI; + }); + + it('should lazily create the required program out of the provided "parserOptions.project" one time when TSESTREE_SINGLE_RUN=true', () => { + /** + * Single run because of explicit environment variable TSESTREE_SINGLE_RUN + */ + const originalTSESTreeSingleRun = process.env.TSESTREE_SINGLE_RUN; + process.env.TSESTREE_SINGLE_RUN = 'true'; + + const resultProgram = parseAndGenerateServices(code, options).services + .program; + expect(resultProgram).toEqual(mockProgram); + + // Call parseAndGenerateServices() again to ensure caching of Programs is working correctly... + parseAndGenerateServices(code, options); + // ...by asserting this was only called once per project + expect(createProgramFromConfigFile).toHaveBeenCalledTimes(tsconfigs.length); + + expect(createProgramFromConfigFile).toHaveBeenNthCalledWith( + 1, + resolvedProject(tsconfigs[0]), + ); + expect(createProgramFromConfigFile).toHaveBeenNthCalledWith( + 2, + resolvedProject(tsconfigs[1]), + ); + + // Restore process data + process.env.TSESTREE_SINGLE_RUN = originalTSESTreeSingleRun; + }); + + it('should lazily create the required program out of the provided "parserOptions.project" one time when singleRun is inferred from CI=true', () => { + /** + * Single run because of CI=true (we need to make sure we respect the original value + * so that we won't interfere with our own usage of the variable) + */ + const originalEnvCI = process.env.CI; + process.env.CI = 'true'; + + const resultProgram = parseAndGenerateServices(code, options).services + .program; + expect(resultProgram).toEqual(mockProgram); + + // Call parseAndGenerateServices() again to ensure caching of Programs is working correctly... + parseAndGenerateServices(code, options); + // ...by asserting this was only called once per project + expect(createProgramFromConfigFile).toHaveBeenCalledTimes(tsconfigs.length); + + expect(createProgramFromConfigFile).toHaveBeenNthCalledWith( + 1, + resolvedProject(tsconfigs[0]), + ); + expect(createProgramFromConfigFile).toHaveBeenNthCalledWith( + 2, + resolvedProject(tsconfigs[1]), + ); + + // Restore process data + process.env.CI = originalEnvCI; + }); + + it('should lazily create the required program out of the provided "parserOptions.project" one time when singleRun is inferred from process.argv', () => { + /** + * Single run because of process.argv + */ + const originalProcessArgv = process.argv; + process.argv = ['', 'node_modules/.bin/eslint', '']; + + const resultProgram = parseAndGenerateServices(code, options).services + .program; + expect(resultProgram).toEqual(mockProgram); + + // Call parseAndGenerateServices() again to ensure caching of Programs is working correctly... + parseAndGenerateServices(code, options); + // ...by asserting this was only called once per project + expect(createProgramFromConfigFile).toHaveBeenCalledTimes(tsconfigs.length); + + expect(createProgramFromConfigFile).toHaveBeenNthCalledWith( + 1, + resolvedProject(tsconfigs[0]), + ); + expect(createProgramFromConfigFile).toHaveBeenNthCalledWith( + 2, + resolvedProject(tsconfigs[1]), + ); + + // Restore process data + process.argv = originalProcessArgv; + }); + + it('should stop iterating through and lazily creating programs for the given "parserOptions.project" once a matching one has been found', () => { + /** + * Single run because of explicit environment variable TSESTREE_SINGLE_RUN + */ + const originalTSESTreeSingleRun = process.env.TSESTREE_SINGLE_RUN; + process.env.TSESTREE_SINGLE_RUN = 'true'; + + const optionsWithReversedTsconfigs = { + ...options, + // Now the matching tsconfig comes first + project: options.project.reverse(), + }; + + const resultProgram = parseAndGenerateServices( + code, + optionsWithReversedTsconfigs, + ).services.program; + expect(resultProgram).toEqual(mockProgram); + + // Call parseAndGenerateServices() again to ensure caching of Programs is working correctly... + parseAndGenerateServices(code, options); + // ...by asserting this was only called only once + expect(createProgramFromConfigFile).toHaveBeenCalledTimes(1); + + expect(createProgramFromConfigFile).toHaveBeenNthCalledWith( + 1, + resolvedProject(tsconfigs[0]), + ); + + // Restore process data + process.env.TSESTREE_SINGLE_RUN = originalTSESTreeSingleRun; + }); +}); diff --git a/packages/typescript-estree/tests/lib/semanticInfo.test.ts b/packages/typescript-estree/tests/lib/semanticInfo.test.ts index 397fb4ddad4..d099342c72e 100644 --- a/packages/typescript-estree/tests/lib/semanticInfo.test.ts +++ b/packages/typescript-estree/tests/lib/semanticInfo.test.ts @@ -2,19 +2,19 @@ import * as fs from 'fs'; import glob from 'glob'; import * as path from 'path'; import * as ts from 'typescript'; +import { clearWatchCaches } from '../../src/create-program/createWatchProgram'; +import { createProgramFromConfigFile as createProgram } from '../../src/create-program/useProvidedPrograms'; +import { + parseAndGenerateServices, + ParseAndGenerateServicesResult, +} from '../../src/parser'; import { TSESTreeOptions } from '../../src/parser-options'; +import { TSESTree } from '../../src/ts-estree'; import { createSnapshotTestBlock, formatSnapshotName, parseCodeAndGenerateServices, } from '../../tools/test-utils'; -import { - clearCaches, - createProgram, - parseAndGenerateServices, - ParseAndGenerateServicesResult, -} from '../../src'; -import { TSESTree } from '../../src/ts-estree'; const FIXTURES_DIR = './tests/fixtures/semanticInfo'; const testFiles = glob.sync(`**/*.src.ts`, { @@ -37,8 +37,8 @@ function createOptions(fileName: string): TSESTreeOptions & { cwd?: string } { }; } -// ensure tsconfig-parser caches are clean for each test -beforeEach(() => clearCaches()); +// ensure tsconfig-parser watch caches are clean for each test +beforeEach(() => clearWatchCaches()); describe('semanticInfo', () => { // test all AST snapshots