Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat: speed up non-type-aware linting with project service#8322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
JoshuaKGoldberg merged 7 commits intotypescript-eslint:v8fromyepitschunked:update_watch_programs_for_non_type_awareJul 21, 2024
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes fromall commits
Commits
Show all changes
7 commits Select commitHold shift + click to select a range
b1662e2 speed up non-type-aware linting with project service
06075cc fix ts error
667fd5b Merge branch 'v8'
JoshuaKGoldbergdd18a1d fix: account for the default project
JoshuaKGoldberg6d5ceb0 fix: account for the default project in website
JoshuaKGoldbergeadbfb5 Apply suggestions from code review
JoshuaKGoldberg80dbb38 chore: formatting
JoshuaKGoldbergFile filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
4 changes: 2 additions & 2 deletionspackages/typescript-estree/src/create-program/createProjectProgram.ts
This file contains hidden or 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 |
|---|---|---|
| @@ -52,10 +52,10 @@ function createProjectProgram( | ||
| ); | ||
| const describedPrograms = | ||
| relativeProjects.length === 1 | ||
| ?` ${relativeProjects[0]}` | ||
| : `\n${relativeProjects.map(project => `- ${project}`).join('\n')}`; | ||
| const errorLines = [ | ||
| `ESLint was configured to run on \`${describedFilePath}\` using \`parserOptions.project\`:${describedPrograms}`, | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Not strictly needed for this PR, but it wasso annoying having Prettier auto-save the snapshot to no longer have a trailing slash... | ||
| ]; | ||
| let hasMatchedAnError = false; | ||
7 changes: 3 additions & 4 deletionspackages/typescript-estree/src/parser.ts
This file contains hidden or 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 |
|---|---|---|
| @@ -195,11 +195,10 @@ function parseAndGenerateServices<T extends TSESTreeOptions = TSESTreeOptions>( | ||
| }; | ||
| } | ||
| const hasFullTypeInformation = | ||
| parseSettings.programs != null || | ||
| parseSettings.projects.size > 0 || | ||
| !!parseSettings.projectService; | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. @yepitschunked following up on | ||
| if ( | ||
| typeof tsestreeOptions.errorOnTypeScriptSyntacticAndSemanticIssues === | ||
245 changes: 173 additions & 72 deletionspackages/typescript-estree/src/useProgramFromProjectService.ts
This file contains hidden or 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 |
|---|---|---|
| @@ -7,7 +7,12 @@ import * as ts from 'typescript'; | ||
| import { createProjectProgram } from './create-program/createProjectProgram'; | ||
| import type { ProjectServiceSettings } from './create-program/createProjectService'; | ||
| import { createNoProgram } from './create-program/createSourceFile'; | ||
| import type { | ||
| ASTAndDefiniteProgram, | ||
| ASTAndNoProgram, | ||
| ASTAndProgram, | ||
| } from './create-program/shared'; | ||
| import { DEFAULT_PROJECT_FILES_ERROR_EXPLANATION } from './create-program/validateDefaultProjectForFilesGlob'; | ||
| import type { MutableParseSettings } from './parseSettings'; | ||
| @@ -42,70 +47,107 @@ const updateExtraFileExtensions = ( | ||
| } | ||
| }; | ||
| function openClientFileFromProjectService( | ||
| defaultProjectMatchedFiles: Set<string>, | ||
| isDefaultProjectAllowed: boolean, | ||
| filePathAbsolute: string, | ||
| parseSettings: Readonly<MutableParseSettings>, | ||
| serviceSettings: ProjectServiceSettings, | ||
| ): ts.server.OpenConfiguredProjectResult { | ||
| const opened = serviceSettings.service.openClientFile( | ||
| filePathAbsolute, | ||
| parseSettings.codeFullText, | ||
| /* scriptKind */ undefined, | ||
| parseSettings.tsconfigRootDir, | ||
| ); | ||
| log( | ||
| 'Project service type information enabled; checking for file path match on: %o', | ||
| serviceSettings.allowDefaultProject, | ||
| ); | ||
| log( | ||
| 'Default project allowed path: %s, based on config file: %s', | ||
| isDefaultProjectAllowed, | ||
| opened.configFileName, | ||
| ); | ||
| if (opened.configFileName) { | ||
| if (isDefaultProjectAllowed) { | ||
| throw new Error( | ||
| `${parseSettings.filePath} was included by allowDefaultProject but also was found in the project service. Consider removing it from allowDefaultProject.`, | ||
| ); | ||
| } | ||
| } else if (!isDefaultProjectAllowed) { | ||
| throw new Error( | ||
| `${parseSettings.filePath} was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProject.`, | ||
| ); | ||
| } | ||
| // No a configFileName indicates this file wasn't included in a TSConfig. | ||
| // That means it must get its type information from the default project. | ||
| if (!opened.configFileName) { | ||
| defaultProjectMatchedFiles.add(filePathAbsolute); | ||
| if ( | ||
| defaultProjectMatchedFiles.size > | ||
| serviceSettings.maximumDefaultProjectFileMatchCount | ||
| ) { | ||
| const filePrintLimit = 20; | ||
| const filesToPrint = Array.from(defaultProjectMatchedFiles).slice( | ||
| 0, | ||
| filePrintLimit, | ||
| ); | ||
| const truncatedFileCount = | ||
| defaultProjectMatchedFiles.size - filesToPrint.length; | ||
| throw new Error( | ||
| `Too many files (>${serviceSettings.maximumDefaultProjectFileMatchCount}) have matched the default project.${DEFAULT_PROJECT_FILES_ERROR_EXPLANATION} | ||
| Matching files: | ||
| ${filesToPrint.map(file => `- ${file}`).join('\n')} | ||
| ${truncatedFileCount ? `...and ${truncatedFileCount} more files\n` : ''} | ||
| If you absolutely need more files included, set parserOptions.projectService.maximumDefaultProjectFileMatchCount_THIS_WILL_SLOW_DOWN_LINTING to a larger value. | ||
| `, | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Note that I've moved this"Too many files ..." notice toinside the | ||
| ); | ||
| } | ||
| } | ||
| return opened; | ||
| } | ||
| function createNoProgramWithProjectService( | ||
| filePathAbsolute: string, | ||
| parseSettings: Readonly<MutableParseSettings>, | ||
| service: ts.server.ProjectService, | ||
| ): ASTAndNoProgram { | ||
| log('No project service information available. Creating no program.'); | ||
| // If the project service knows about this file, this informs if of changes. | ||
| // Doing so ensures that: | ||
| // - if the file is not part of a project, we don't waste time creating a program (fast non-type-aware linting) | ||
| // - otherwise, we refresh the file in the project service (moderately fast, since the project is already loaded) | ||
| if (service.getScriptInfo(filePathAbsolute)) { | ||
| log('Script info available. Opening client file in project service.'); | ||
| service.openClientFile( | ||
| filePathAbsolute, | ||
| parseSettings.codeFullText, | ||
| /* scriptKind */ undefined, | ||
| parseSettings.tsconfigRootDir, | ||
| ); | ||
| } | ||
| return createNoProgram(parseSettings); | ||
| } | ||
| function retrieveASTAndProgramFor( | ||
| filePathAbsolute: string, | ||
| parseSettings: Readonly<MutableParseSettings>, | ||
| serviceSettings: ProjectServiceSettings, | ||
| ): ASTAndDefiniteProgram | undefined { | ||
| log('Retrieving script info and then program for: %s', filePathAbsolute); | ||
| const scriptInfo =serviceSettings.service.getScriptInfo(filePathAbsolute); | ||
| /* eslint-disable @typescript-eslint/no-non-null-assertion */ | ||
| const program =serviceSettings.service | ||
| .getDefaultProjectForFile(scriptInfo!.fileName, true)! | ||
| .getLanguageService(/*ensureSynchronized*/ true) | ||
| .getProgram(); | ||
| @@ -116,37 +158,96 @@ export function useProgramFromProjectService( | ||
| return undefined; | ||
| } | ||
| log('Found project service program for: %s', filePathAbsolute); | ||
| return createProjectProgram(parseSettings, [program]); | ||
| } | ||
| export function useProgramFromProjectService( | ||
| settings: ProjectServiceSettings, | ||
| parseSettings: Readonly<MutableParseSettings>, | ||
| hasFullTypeInformation: boolean, | ||
| defaultProjectMatchedFiles: Set<string>, | ||
| ): ASTAndProgram | undefined; | ||
| export function useProgramFromProjectService( | ||
| settings: ProjectServiceSettings, | ||
| parseSettings: Readonly<MutableParseSettings>, | ||
| hasFullTypeInformation: true, | ||
| defaultProjectMatchedFiles: Set<string>, | ||
| ): ASTAndDefiniteProgram | undefined; | ||
| export function useProgramFromProjectService( | ||
| settings: ProjectServiceSettings, | ||
| parseSettings: Readonly<MutableParseSettings>, | ||
| hasFullTypeInformation: false, | ||
| defaultProjectMatchedFiles: Set<string>, | ||
| ): ASTAndNoProgram | undefined; | ||
| export function useProgramFromProjectService( | ||
| serviceSettings: ProjectServiceSettings, | ||
| parseSettings: Readonly<MutableParseSettings>, | ||
| hasFullTypeInformation: boolean, | ||
| defaultProjectMatchedFiles: Set<string>, | ||
| ): ASTAndProgram | undefined { | ||
| // NOTE: triggers a full project reload when changes are detected | ||
| updateExtraFileExtensions( | ||
| serviceSettings.service, | ||
| parseSettings.extraFileExtensions, | ||
| ); | ||
| // We don't canonicalize the filename because it caused a performance regression. | ||
| // See https://github.com/typescript-eslint/typescript-eslint/issues/8519 | ||
| const filePathAbsolute = absolutify(parseSettings.filePath, serviceSettings); | ||
| log( | ||
| 'Opening project service file for: %s at absolute path %s', | ||
| parseSettings.filePath, | ||
| filePathAbsolute, | ||
| ); | ||
| const isDefaultProjectAllowed = filePathMatchedBy( | ||
| parseSettings.filePath, | ||
| serviceSettings.allowDefaultProject, | ||
| ); | ||
| // Type-aware linting is disabled for this file. | ||
| // However, type-aware lint rules might still rely on its contents. | ||
| if (!hasFullTypeInformation && !isDefaultProjectAllowed) { | ||
| return createNoProgramWithProjectService( | ||
| filePathAbsolute, | ||
| parseSettings, | ||
| serviceSettings.service, | ||
| ); | ||
| } | ||
| // If type info was requested, we attempt to open it in the project service. | ||
| // By now, the file is known to be one of: | ||
| // - in the project service (valid configuration) | ||
| // - allowlisted in the default project (valid configuration) | ||
| // - neither, which openClientFileFromProjectService will throw an error for | ||
| const opened = | ||
| hasFullTypeInformation && | ||
| openClientFileFromProjectService( | ||
| defaultProjectMatchedFiles, | ||
| isDefaultProjectAllowed, | ||
| filePathAbsolute, | ||
| parseSettings, | ||
| serviceSettings, | ||
| ); | ||
| log('Opened project service file: %o', opened); | ||
| return retrieveASTAndProgramFor( | ||
| filePathAbsolute, | ||
| parseSettings, | ||
| serviceSettings, | ||
| ); | ||
| } | ||
| function absolutify( | ||
| filePath: string, | ||
| serviceSettings: ProjectServiceSettings, | ||
| ): string { | ||
| return path.isAbsolute(filePath) | ||
| ? filePath | ||
| : path.join(serviceSettings.service.host.getCurrentDirectory(), filePath); | ||
| } | ||
| function filePathMatchedBy( | ||
29 changes: 10 additions & 19 deletionspackages/typescript-estree/tests/lib/parse.test.ts
This file contains hidden or 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
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.