Skip to content

Use JSDoc directly in syntactic classifier rather than reparsing #52795

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1380,6 +1380,7 @@ export function updateSourceFile(sourceFile: SourceFile, newText: string, textCh
}

/** @internal */
// Exposed only for testing.
export function parseIsolatedJSDocComment(content: string, start?: number, length?: number) {
const result = Parser.JSDocParser.parseIsolatedJSDocComment(content, start, length);
if (result && result.jsDoc) {
Expand Down
24 changes: 12 additions & 12 deletions src/services/classifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
getMeaningFromLocation,
getModuleInstanceState,
getTypeArgumentOrTypeParameterList,
HasJSDoc,
InterfaceDeclaration,
isAccessibilityModifier,
isConstTypeReference,
Expand Down Expand Up @@ -60,12 +59,10 @@ import {
Node,
nodeIsMissing,
ParameterDeclaration,
parseIsolatedJSDocComment,
Push,
Scanner,
ScriptTarget,
SemanticMeaning,
setParent,
some,
SourceFile,
Symbol,
Expand Down Expand Up @@ -727,7 +724,7 @@ export function getEncodedSyntacticClassifications(cancellationToken: Cancellati
case SyntaxKind.SingleLineCommentTrivia:
case SyntaxKind.MultiLineCommentTrivia:
// Only bother with the trivia if it at least intersects the span of interest.
classifyComment(token, kind, start, width);
classifyComment(kind, start, width);

// Classifying a comment might cause us to reuse the trivia scanner
// (because of jsdoc comments). So after we classify the comment make
Expand Down Expand Up @@ -762,15 +759,11 @@ export function getEncodedSyntacticClassifications(cancellationToken: Cancellati
}
}

function classifyComment(token: Node, kind: SyntaxKind, start: number, width: number) {
function classifyComment(kind: SyntaxKind, start: number, width: number) {
if (kind === SyntaxKind.MultiLineCommentTrivia) {
// See if this is a doc comment. If so, we'll classify certain portions of it
// specially.
const docCommentAndDiagnostics = parseIsolatedJSDocComment(sourceFile.text, start, width);
if (docCommentAndDiagnostics && docCommentAndDiagnostics.jsDoc) {
// TODO: This should be predicated on `token["kind"]` being compatible with `HasJSDoc["kind"]`
setParent(docCommentAndDiagnostics.jsDoc, token as HasJSDoc);
classifyJSDocComment(docCommentAndDiagnostics.jsDoc);
// If it's JSDoc, we will have handled its node explicitly as another Node's child; skip.
const text = sourceFile.text.substr(start, 3);
if (text === "/**") {
Comment on lines +765 to +766
Copy link
Member

@DanielRosenwasser DanielRosenwasser Feb 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately substr allocates so:

Suggested change
const text = sourceFile.text.substr(start, 3);
if (text === "/**") {
const sourceText = sourceFile.text;
if (start + 3 <= sourceText.length && sourceText.charCodeAt(start) === CharacterCodes.slash
&& sourceText.charCodeAt(start + 1) === CharacterCodes.asterisk && sourceText.charCodeAt(start + 2) === CharacterCodes.asterisk) {

return;
}
}
Expand Down Expand Up @@ -1014,6 +1007,7 @@ export function getEncodedSyntacticClassifications(cancellationToken: Cancellati
*/
function tryClassifyNode(node: Node): boolean {
if (isJSDoc(node)) {
classifyJSDocComment(node);
return true;
}

Expand All @@ -1028,6 +1022,12 @@ export function getEncodedSyntacticClassifications(cancellationToken: Cancellati

const tokenStart = node.kind === SyntaxKind.JsxText ? node.pos : classifyLeadingTriviaAndGetTokenStart(node);

if (node.kind === SyntaxKind.EndOfFileToken) {
// There's no token for the end of the file, but return
// false so that a potential JSDoc child node is visited.
return false;
}

const tokenWidth = node.end - tokenStart;
Debug.assert(tokenWidth >= 0);
if (tokenWidth > 0) {
Expand Down
11 changes: 11 additions & 0 deletions tests/cases/fourslash/jsdocWithBarInTypeLiteral.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path="fourslash.ts" />

// @filename: index.js
//// class I18n {
//// /**
//// * @param {{dot|fulltext}} [stringMode] - which mode our translation keys use
//// */
//// constructor(options = {}) {}
//// }

verify.encodedSyntacticClassificationsLength(69);