Skip to content

Skip parsing JSDoc when not needed #52921

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 45 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
481a7dd
Pull in most of Nathan's change with tweaks
jakebailey Feb 22, 2023
170042c
Move to the scanner
jakebailey Feb 22, 2023
131a170
And ending
jakebailey Feb 22, 2023
3af4435
Get tests working
jakebailey Feb 22, 2023
ed54d9f
Remove commented out code
jakebailey Feb 22, 2023
85b29d7
Amend test
jakebailey Feb 22, 2023
6930f47
Don't use regex
jakebailey Feb 22, 2023
07e4a82
Fix incremental, other tweaks
jakebailey Feb 22, 2023
4bd24bc
Revert "Don't use regex"
jakebailey Feb 23, 2023
ae10cda
Add test for arrow function jsdoc
jakebailey Feb 23, 2023
ab63745
Ensure that arrow functions get their jsdoc
jakebailey Feb 23, 2023
f74bba3
Oops forgot the baselines
jakebailey Feb 23, 2023
bdd917c
Merge branch 'main' into skip-jsdoc
jakebailey Feb 23, 2023
988ab20
Merge branch 'main' into skip-jsdoc
jakebailey Feb 28, 2023
bc0ab70
Don't add the arg if it's already provided in sys.args
jakebailey Feb 28, 2023
ae56d5a
Merge branch 'main' into skip-jsdoc
jakebailey Mar 8, 2023
f9b120e
Fix bad merge
jakebailey Mar 8, 2023
3303914
Merge branch 'main' into skip-jsdoc
jakebailey Mar 8, 2023
4eea9ec
Merge branch 'main' into skip-jsdoc
jakebailey Mar 18, 2023
d0c18de
Merge branch 'main' into skip-jsdoc
jakebailey Jun 26, 2023
c25fe92
Merge branch 'main' into skip-jsdoc
jakebailey Jun 26, 2023
0f42a19
Merge branch 'main' into skip-jsdoc
jakebailey Jul 1, 2023
818162f
Merge branch 'main' into skip-jsdoc
jakebailey Aug 9, 2023
4732923
PR feedback, huge baseline changes
jakebailey Aug 9, 2023
536ee78
One missing baseline
jakebailey Aug 9, 2023
26b3ae8
Merge branch 'main' into skip-jsdoc
jakebailey Aug 30, 2023
a7bf1e0
Don't use a fake flag to set skipJSDocParsing in tsc, just set as an …
jakebailey Sep 7, 2023
e06e689
Remove CompilerOption in prep for cherry-pick
jakebailey Sep 12, 2023
f958d67
SkipJsDoc plumbing
sheetalkamat Sep 12, 2023
88c52b7
Add watchCompilerHost and solutionBuilder a way for overriding getSou…
sheetalkamat Sep 12, 2023
c8f6b91
Update baselines
jakebailey Sep 12, 2023
0527548
More baselines
jakebailey Sep 12, 2023
d7ddcd2
Update testing
jakebailey Sep 12, 2023
273b769
Rename variable when not inside parser
jakebailey Sep 12, 2023
406c471
Pull into global var
jakebailey Sep 12, 2023
f2a0317
Fix forgotten refactor
jakebailey Sep 12, 2023
7a5acb6
Undo some old changes from CompilerOptions
jakebailey Sep 12, 2023
a249aae
Oops
jakebailey Sep 12, 2023
371dbc2
Merge branch 'main' into skip-jsdoc
jakebailey Sep 12, 2023
829b10a
Undo harness changes
jakebailey Sep 12, 2023
16208fc
Remove unused baseline
jakebailey Sep 12, 2023
5b54f3c
Add missing param
jakebailey Sep 12, 2023
3b6c4dc
Remove comment
jakebailey Sep 12, 2023
1d38012
Rename flag to skipNonSemanticJSDoc and skipNonSemanticJSDocParsing
jakebailey Sep 13, 2023
b287a9f
Remove new option from public API for now
jakebailey Sep 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 30 additions & 14 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1324,7 +1324,10 @@ function setExternalModuleIndicator(sourceFile: SourceFile) {
sourceFile.externalModuleIndicator = isFileProbablyExternalModule(sourceFile);
}

export function createSourceFile(fileName: string, sourceText: string, languageVersionOrOptions: ScriptTarget | CreateSourceFileOptions, setParentNodes = false, scriptKind?: ScriptKind): SourceFile {
export function createSourceFile(fileName: string, sourceText: string, languageVersionOrOptions: ScriptTarget | CreateSourceFileOptions, setParentNodes?: boolean, scriptKind?: ScriptKind): SourceFile;
/** @internal */
export function createSourceFile(fileName: string, sourceText: string, languageVersionOrOptions: ScriptTarget | CreateSourceFileOptions, setParentNodes?: boolean, scriptKind?: ScriptKind, skipNonSemanticJSDoc?: boolean): SourceFile;
export function createSourceFile(fileName: string, sourceText: string, languageVersionOrOptions: ScriptTarget | CreateSourceFileOptions, setParentNodes = false, scriptKind?: ScriptKind, skipNonSemanticJSDoc?: boolean): SourceFile {
tracing?.push(tracing.Phase.Parse, "createSourceFile", { path: fileName }, /*separateBeginAndEnd*/ true);
performance.mark("beforeParse");
let result: SourceFile;
Expand All @@ -1336,14 +1339,14 @@ export function createSourceFile(fileName: string, sourceText: string, languageV
impliedNodeFormat: format,
} = typeof languageVersionOrOptions === "object" ? languageVersionOrOptions : ({ languageVersion: languageVersionOrOptions } as CreateSourceFileOptions);
if (languageVersion === ScriptTarget.JSON) {
result = Parser.parseSourceFile(fileName, sourceText, languageVersion, /*syntaxCursor*/ undefined, setParentNodes, ScriptKind.JSON, noop);
result = Parser.parseSourceFile(fileName, sourceText, languageVersion, /*syntaxCursor*/ undefined, setParentNodes, ScriptKind.JSON, noop, skipNonSemanticJSDoc);
}
else {
const setIndicator = format === undefined ? overrideSetExternalModuleIndicator : (file: SourceFile) => {
file.impliedNodeFormat = format;
return (overrideSetExternalModuleIndicator || setExternalModuleIndicator)(file);
};
result = Parser.parseSourceFile(fileName, sourceText, languageVersion, /*syntaxCursor*/ undefined, setParentNodes, scriptKind, setIndicator);
result = Parser.parseSourceFile(fileName, sourceText, languageVersion, /*syntaxCursor*/ undefined, setParentNodes, scriptKind, setIndicator, skipNonSemanticJSDoc);
}
perfLogger?.logStopParseSourceFile();

Expand Down Expand Up @@ -1575,7 +1578,16 @@ namespace Parser {
var parseErrorBeforeNextFinishedNode = false;
/* eslint-enable no-var */

export function parseSourceFile(fileName: string, sourceText: string, languageVersion: ScriptTarget, syntaxCursor: IncrementalParser.SyntaxCursor | undefined, setParentNodes = false, scriptKind?: ScriptKind, setExternalModuleIndicatorOverride?: (file: SourceFile) => void): SourceFile {
export function parseSourceFile(
fileName: string,
sourceText: string,
languageVersion: ScriptTarget,
syntaxCursor: IncrementalParser.SyntaxCursor | undefined,
setParentNodes = false,
scriptKind?: ScriptKind,
setExternalModuleIndicatorOverride?: (file: SourceFile) => void,
skipNonSemanticJSDoc?: boolean,
): SourceFile {
scriptKind = ensureScriptKind(fileName, scriptKind);
if (scriptKind === ScriptKind.JSON) {
const result = parseJsonText(fileName, sourceText, languageVersion, syntaxCursor, setParentNodes);
Expand All @@ -1589,9 +1601,10 @@ namespace Parser {
return result;
}

initializeState(fileName, sourceText, languageVersion, syntaxCursor, scriptKind);
skipNonSemanticJSDoc = !!skipNonSemanticJSDoc && scriptKind !== ScriptKind.JS && scriptKind !== ScriptKind.JSX;
initializeState(fileName, sourceText, languageVersion, syntaxCursor, scriptKind, skipNonSemanticJSDoc);

const result = parseSourceFileWorker(languageVersion, setParentNodes, scriptKind, setExternalModuleIndicatorOverride || setExternalModuleIndicator);
const result = parseSourceFileWorker(languageVersion, setParentNodes, scriptKind, setExternalModuleIndicatorOverride || setExternalModuleIndicator, skipNonSemanticJSDoc);

clearState();

Expand All @@ -1600,7 +1613,7 @@ namespace Parser {

export function parseIsolatedEntityName(content: string, languageVersion: ScriptTarget): EntityName | undefined {
// Choice of `isDeclarationFile` should be arbitrary
initializeState("", content, languageVersion, /*syntaxCursor*/ undefined, ScriptKind.JS);
initializeState("", content, languageVersion, /*syntaxCursor*/ undefined, ScriptKind.JS, /*skipNonSemanticJSDoc*/ false);
// Prime the scanner.
nextToken();
const entityName = parseEntityName(/*allowReservedWords*/ true);
Expand All @@ -1610,7 +1623,7 @@ namespace Parser {
}

export function parseJsonText(fileName: string, sourceText: string, languageVersion: ScriptTarget = ScriptTarget.ES2015, syntaxCursor?: IncrementalParser.SyntaxCursor, setParentNodes = false): JsonSourceFile {
initializeState(fileName, sourceText, languageVersion, syntaxCursor, ScriptKind.JSON);
initializeState(fileName, sourceText, languageVersion, syntaxCursor, ScriptKind.JSON, /*skipNonSemanticJSDoc*/ false);
sourceFlags = contextFlags;

// Prime the scanner.
Expand Down Expand Up @@ -1698,7 +1711,7 @@ namespace Parser {
return result;
}

function initializeState(_fileName: string, _sourceText: string, _languageVersion: ScriptTarget, _syntaxCursor: IncrementalParser.SyntaxCursor | undefined, _scriptKind: ScriptKind) {
function initializeState(_fileName: string, _sourceText: string, _languageVersion: ScriptTarget, _syntaxCursor: IncrementalParser.SyntaxCursor | undefined, _scriptKind: ScriptKind, _skipNonSemanticJSDoc: boolean) {
NodeConstructor = objectAllocator.getNodeConstructor();
TokenConstructor = objectAllocator.getTokenConstructor();
IdentifierConstructor = objectAllocator.getIdentifierConstructor();
Expand Down Expand Up @@ -1739,13 +1752,15 @@ namespace Parser {
scanner.setOnError(scanError);
scanner.setScriptTarget(languageVersion);
scanner.setLanguageVariant(languageVariant);
scanner.setSkipNonSemanticJSDoc(_skipNonSemanticJSDoc);
}

function clearState() {
// Clear out the text the scanner is pointing at, so it doesn't keep anything alive unnecessarily.
scanner.clearCommentDirectives();
scanner.setText("");
scanner.setOnError(undefined);
scanner.setSkipNonSemanticJSDoc(false);

// Clear any data. We don't want to accidentally hold onto it for too long.
sourceText = undefined!;
Expand All @@ -1762,7 +1777,7 @@ namespace Parser {
topLevel = true;
}

function parseSourceFileWorker(languageVersion: ScriptTarget, setParentNodes: boolean, scriptKind: ScriptKind, setExternalModuleIndicator: (file: SourceFile) => void): SourceFile {
function parseSourceFileWorker(languageVersion: ScriptTarget, setParentNodes: boolean, scriptKind: ScriptKind, setExternalModuleIndicator: (file: SourceFile) => void, skipNonSemanticJSDoc: boolean): SourceFile {
const isDeclarationFile = isDeclarationFileName(fileName);
if (isDeclarationFile) {
contextFlags |= NodeFlags.Ambient;
Expand All @@ -1789,6 +1804,7 @@ namespace Parser {
sourceFile.identifierCount = identifierCount;
sourceFile.identifiers = identifiers;
sourceFile.parseDiagnostics = attachFileToDiagnostics(parseDiagnostics, sourceFile);
sourceFile.skipNonSemanticJSDoc = skipNonSemanticJSDoc;
if (jsDocDiagnostics) {
sourceFile.jsDocDiagnostics = attachFileToDiagnostics(jsDocDiagnostics, sourceFile);
}
Expand Down Expand Up @@ -8670,7 +8686,7 @@ namespace Parser {

export namespace JSDocParser {
export function parseJSDocTypeExpressionForTests(content: string, start: number | undefined, length: number | undefined): { jsDocTypeExpression: JSDocTypeExpression; diagnostics: Diagnostic[]; } | undefined {
initializeState("file.js", content, ScriptTarget.Latest, /*syntaxCursor*/ undefined, ScriptKind.JS);
initializeState("file.js", content, ScriptTarget.Latest, /*syntaxCursor*/ undefined, ScriptKind.JS, /*skipNonSemanticJSDoc*/ false);
scanner.setText(content, start, length);
currentToken = scanner.scan();
const jsDocTypeExpression = parseJSDocTypeExpression();
Expand Down Expand Up @@ -8720,7 +8736,7 @@ namespace Parser {
}

export function parseIsolatedJSDocComment(content: string, start: number | undefined, length: number | undefined): { jsDoc: JSDoc; diagnostics: Diagnostic[]; } | undefined {
initializeState("", content, ScriptTarget.Latest, /*syntaxCursor*/ undefined, ScriptKind.JS);
initializeState("", content, ScriptTarget.Latest, /*syntaxCursor*/ undefined, ScriptKind.JS, /*skipNonSemanticJSDoc*/ false);
const jsDoc = doInsideOfContext(NodeFlags.JSDoc, () => parseJSDocCommentWorker(start, length));

const sourceFile = { languageVariant: LanguageVariant.Standard, text: content } as SourceFile;
Expand Down Expand Up @@ -9788,7 +9804,7 @@ namespace IncrementalParser {
if (sourceFile.statements.length === 0) {
// If we don't have any statements in the current source file, then there's no real
// way to incrementally parse. So just do a full parse instead.
return Parser.parseSourceFile(sourceFile.fileName, newText, sourceFile.languageVersion, /*syntaxCursor*/ undefined, /*setParentNodes*/ true, sourceFile.scriptKind, sourceFile.setExternalModuleIndicator);
return Parser.parseSourceFile(sourceFile.fileName, newText, sourceFile.languageVersion, /*syntaxCursor*/ undefined, /*setParentNodes*/ true, sourceFile.scriptKind, sourceFile.setExternalModuleIndicator, sourceFile.skipNonSemanticJSDoc);
}

// Make sure we're not trying to incrementally update a source file more than once. Once
Expand Down Expand Up @@ -9851,7 +9867,7 @@ namespace IncrementalParser {
// inconsistent tree. Setting the parents on the new tree should be very fast. We
// will immediately bail out of walking any subtrees when we can see that their parents
// are already correct.
const result = Parser.parseSourceFile(sourceFile.fileName, newText, sourceFile.languageVersion, syntaxCursor, /*setParentNodes*/ true, sourceFile.scriptKind, sourceFile.setExternalModuleIndicator);
const result = Parser.parseSourceFile(sourceFile.fileName, newText, sourceFile.languageVersion, syntaxCursor, /*setParentNodes*/ true, sourceFile.scriptKind, sourceFile.setExternalModuleIndicator, sourceFile.skipNonSemanticJSDoc);
result.commentDirectives = getNewCommentDirectives(
sourceFile.commentDirectives,
result.commentDirectives,
Expand Down
12 changes: 9 additions & 3 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ export function createGetSourceFile(
readFile: ProgramHost<any>["readFile"],
getCompilerOptions: () => CompilerOptions,
setParentNodes: boolean | undefined,
skipNonSemanticJSDocParsing: boolean | undefined,
): CompilerHost["getSourceFile"] {
return (fileName, languageVersionOrOptions, onError) => {
let text: string | undefined;
Expand All @@ -417,7 +418,7 @@ export function createGetSourceFile(
}
text = "";
}
return text !== undefined ? createSourceFile(fileName, text, languageVersionOrOptions, setParentNodes) : undefined;
return text !== undefined ? createSourceFile(fileName, text, languageVersionOrOptions, setParentNodes, /*scriptKind*/ undefined, skipNonSemanticJSDocParsing) : undefined;
};
}

Expand Down Expand Up @@ -455,7 +456,12 @@ export function createWriteFileMeasuringIO(
}

/** @internal */
export function createCompilerHostWorker(options: CompilerOptions, setParentNodes?: boolean, system: System = sys): CompilerHost {
export function createCompilerHostWorker(
options: CompilerOptions,
setParentNodes?: boolean,
skipNonSemanticJSDocParsing?: boolean,
system: System = sys,
): CompilerHost {
const existingDirectories = new Map<string, boolean>();
const getCanonicalFileName = createGetCanonicalFileName(system.useCaseSensitiveFileNames);
function directoryExists(directoryPath: string): boolean {
Expand All @@ -476,7 +482,7 @@ export function createCompilerHostWorker(options: CompilerOptions, setParentNode
const newLine = getNewLineCharacter(options);
const realpath = system.realpath && ((path: string) => system.realpath!(path));
const compilerHost: CompilerHost = {
getSourceFile: createGetSourceFile(fileName => compilerHost.readFile(fileName), () => options, setParentNodes),
getSourceFile: createGetSourceFile(fileName => compilerHost.readFile(fileName), () => options, setParentNodes, skipNonSemanticJSDocParsing),
getDefaultLibLocation,
getDefaultLibFileName: options => combinePaths(getDefaultLibLocation(), getDefaultLibFileName(options)),
writeFile: createWriteFileMeasuringIO(
Expand Down
22 changes: 19 additions & 3 deletions src/compiler/scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ export interface Scanner {
// callback returns something truthy, then the scanner state is not rolled back. The result
// of invoking the callback is returned from this function.
tryScan<T>(callback: () => T): T;
/** @internal */
setSkipNonSemanticJSDoc(skip: boolean): void;
}

/** @internal */
Expand Down Expand Up @@ -343,6 +345,11 @@ const commentDirectiveRegExSingleLine = /^\/\/\/?\s*@(ts-expect-error|ts-ignore)
*/
const commentDirectiveRegExMultiLine = /^(?:\/|\*)*\s*@(ts-expect-error|ts-ignore)/;

/**
* Test for whether a comment contains a JSDoc tag needed by the checker when run in tsc.
*/
const semanticJSDocTagRegEx = /@(?:see|link)/i;

function lookupInUnicodeMap(code: number, map: readonly number[]): boolean {
// Bail out quickly if it couldn't possibly be in the map.
if (code < map[0]) {
Expand Down Expand Up @@ -1001,6 +1008,8 @@ export function createScanner(languageVersion: ScriptTarget, skipTrivia: boolean
var commentDirectives: CommentDirective[] | undefined;
var inJSDocType = 0;

var skipNonSemanticJSDoc = false;

setText(text, start, length);

var scanner: Scanner = {
Expand Down Expand Up @@ -1052,6 +1061,7 @@ export function createScanner(languageVersion: ScriptTarget, skipTrivia: boolean
tryScan,
lookAhead,
scanRange,
setSkipNonSemanticJSDoc,
};
/* eslint-enable no-var */

Expand Down Expand Up @@ -1971,9 +1981,7 @@ export function createScanner(languageVersion: ScriptTarget, skipTrivia: boolean
// Multi-line comment
if (text.charCodeAt(pos + 1) === CharacterCodes.asterisk) {
pos += 2;
if (text.charCodeAt(pos) === CharacterCodes.asterisk && text.charCodeAt(pos + 1) !== CharacterCodes.slash) {
tokenFlags |= TokenFlags.PrecedingJSDocComment;
}
const isJSDoc = text.charCodeAt(pos) === CharacterCodes.asterisk && text.charCodeAt(pos + 1) !== CharacterCodes.slash;

let commentClosed = false;
let lastLineStart = tokenStart;
Expand All @@ -1994,6 +2002,10 @@ export function createScanner(languageVersion: ScriptTarget, skipTrivia: boolean
}
}

if (isJSDoc && (!skipNonSemanticJSDoc || semanticJSDocTagRegEx.test(text.slice(fullStartPos, pos)))) {
tokenFlags |= TokenFlags.PrecedingJSDocComment;
}

commentDirectives = appendIfCommentDirective(commentDirectives, text.slice(lastLineStart, pos), commentDirectiveRegExMultiLine, lastLineStart);

if (!commentClosed) {
Expand Down Expand Up @@ -2775,6 +2787,10 @@ export function createScanner(languageVersion: ScriptTarget, skipTrivia: boolean
languageVariant = variant;
}

function setSkipNonSemanticJSDoc(skip: boolean) {
skipNonSemanticJSDoc = skip;
}

function resetTokenState(position: number) {
Debug.assert(position >= 0);
pos = position;
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4297,6 +4297,8 @@ export interface SourceFile extends Declaration, LocalsContainer {

/** @internal */ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;
/** @internal */ endFlowNode?: FlowNode;

/** @internal */ skipNonSemanticJSDoc?: boolean;
}

/** @internal */
Expand Down
1 change: 1 addition & 0 deletions src/compiler/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,7 @@ export function createCompilerHostFromProgramHost(host: ProgramHost<any>, getCom
(fileName, encoding) => !encoding ? compilerHost.readFile(fileName) : host.readFile(fileName, encoding),
getCompilerOptions,
/*setParentNodes*/ undefined,
host.skipNonSemanticJSDocParsing,
),
getDefaultLibLocation: maybeBind(host, host.getDefaultLibLocation),
getDefaultLibFileName: options => host.getDefaultLibFileName(options),
Expand Down
14 changes: 12 additions & 2 deletions src/compiler/watchPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,11 @@ export function readBuilderProgram(compilerOptions: CompilerOptions, host: ReadB
return createBuilderProgramUsingProgramBuildInfo(buildInfo, buildInfoPath, host);
}

export function createIncrementalCompilerHost(options: CompilerOptions, system = sys): CompilerHost {
const host = createCompilerHostWorker(options, /*setParentNodes*/ undefined, system);
export function createIncrementalCompilerHost(options: CompilerOptions, system?: System): CompilerHost;
/** @internal */
export function createIncrementalCompilerHost(options: CompilerOptions, system?: System, skipNonSemanticJSDocParsing?: boolean): CompilerHost;
export function createIncrementalCompilerHost(options: CompilerOptions, system = sys, skipNonSemanticJSDocParsing?: boolean): CompilerHost {
const host = createCompilerHostWorker(options, /*setParentNodes*/ undefined, skipNonSemanticJSDocParsing, system);
host.createHash = maybeBind(system, system.createHash);
host.storeFilesChangingSignatureDuringEmit = system.storeFilesChangingSignatureDuringEmit;
setGetSourceFileAsHashVersioned(host);
Expand Down Expand Up @@ -254,6 +257,13 @@ export interface ProgramHost<T extends BuilderProgram> {
* Returns the module resolution cache used by a provided `resolveModuleNames` implementation so that any non-name module resolution operations (eg, package.json lookup) can reuse it
*/
getModuleResolutionCache?(): ModuleResolutionCache | undefined;

/**
* True if it's safe for the parser to skip parsing non-semantic JSDoc tags.
*
* @internal
*/
skipNonSemanticJSDocParsing?: boolean;
}
/**
* Internal interface used to wire emit through same host
Expand Down
Loading