From 64c7f3d38be6822c5ee58f88e5782ef8867df3b8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 28 May 2015 14:45:43 -0700 Subject: [PATCH 1/6] Add syntactic classification for doc comments. --- src/compiler/parser.ts | 12 +- src/harness/harness.ts | 2 +- src/services/services.ts | 122 +++++++++++++++++- tests/cases/fourslash/fourslash.ts | 4 + .../syntacticClassificationsDocComment1.ts | 17 +++ 5 files changed, 143 insertions(+), 14 deletions(-) create mode 100644 tests/cases/fourslash/syntacticClassificationsDocComment1.ts diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 28fd8184d7e..0595aa75959 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -363,9 +363,9 @@ module ts { } } - export function createSourceFile(fileName: string, sourceText: string, languageVersion: ScriptTarget, setParentNodes = false): SourceFile { + export function createSourceFile(fileName: string, sourceText: string, languageVersion: ScriptTarget, setParentNodes = false, includeDocComments = false): SourceFile { let start = new Date().getTime(); - let result = Parser.parseSourceFile(fileName, sourceText, languageVersion, /*syntaxCursor*/ undefined, setParentNodes); + let result = Parser.parseSourceFile(fileName, sourceText, languageVersion, /*syntaxCursor*/ undefined, setParentNodes, includeDocComments); parseTime += new Date().getTime() - start; return result; @@ -493,10 +493,10 @@ module ts { // attached to the EOF token. let parseErrorBeforeNextFinishedNode: boolean = false; - export function parseSourceFile(fileName: string, _sourceText: string, languageVersion: ScriptTarget, _syntaxCursor: IncrementalParser.SyntaxCursor, setParentNodes?: boolean): SourceFile { + export function parseSourceFile(fileName: string, _sourceText: string, languageVersion: ScriptTarget, _syntaxCursor: IncrementalParser.SyntaxCursor, setParentNodes?: boolean, includeDocComments?: boolean): SourceFile { initializeState(fileName, _sourceText, languageVersion, _syntaxCursor); - let result = parseSourceFileWorker(fileName, languageVersion, setParentNodes); + let result = parseSourceFileWorker(fileName, languageVersion, setParentNodes, includeDocComments); clearState(); @@ -535,7 +535,7 @@ module ts { sourceText = undefined; } - function parseSourceFileWorker(fileName: string, languageVersion: ScriptTarget, setParentNodes: boolean): SourceFile { + function parseSourceFileWorker(fileName: string, languageVersion: ScriptTarget, setParentNodes: boolean, includeDocComments: boolean): SourceFile { sourceFile = createSourceFile(fileName, languageVersion); // Prime the scanner. @@ -560,7 +560,7 @@ module ts { // If this is a javascript file, proactively see if we can get JSDoc comments for // relevant nodes in the file. We'll use these to provide typing informaion if they're // available. - if (isJavaScript(fileName)) { + if (includeDocComments || isJavaScript(fileName)) { addJSDocComments(); } diff --git a/src/harness/harness.ts b/src/harness/harness.ts index 90afab029eb..244c693ad12 100644 --- a/src/harness/harness.ts +++ b/src/harness/harness.ts @@ -167,7 +167,7 @@ module Utils { continue; } var child = (node)[childName]; - if (isNodeOrArray(child)) { + if (isNodeOrArray(child) && childName !== "jsDocComment") { assert.isFalse(childNodesAndArrays.indexOf(child) < 0, "Missing child when forEach'ing over node: " + (ts).SyntaxKind[node.kind] + "-" + childName); } diff --git a/src/services/services.ts b/src/services/services.ts index 1019c532192..8990a62f664 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1510,6 +1510,7 @@ module ts { public static typeParameterName = "type parameter name"; public static typeAliasName = "type alias name"; public static parameterName = "parameter name"; + public static docCommentTagName = "doc comment tag name"; } export const enum ClassificationType { @@ -1529,7 +1530,8 @@ module ts { moduleName = 14, typeParameterName = 15, typeAliasName = 16, - parameterName = 17 + parameterName = 17, + docCommentTagName = 18, } /// Language Service @@ -1813,7 +1815,8 @@ module ts { } export function createLanguageServiceSourceFile(fileName: string, scriptSnapshot: IScriptSnapshot, scriptTarget: ScriptTarget, version: string, setNodeParents: boolean): SourceFile { - let sourceFile = createSourceFile(fileName, scriptSnapshot.getText(0, scriptSnapshot.getLength()), scriptTarget, setNodeParents); + let text = scriptSnapshot.getText(0, scriptSnapshot.getLength()); + let sourceFile = createSourceFile(fileName, text, scriptTarget, setNodeParents, /*includeDocComments:*/ true); setSourceFileFields(sourceFile, scriptSnapshot, version); // after full parsing we can use table with interned strings as name table sourceFile.nameTable = sourceFile.identifiers; @@ -2832,6 +2835,7 @@ module ts { let typeChecker = program.getTypeChecker(); let syntacticStart = new Date().getTime(); let sourceFile = getValidSourceFile(fileName); + let isJavaScriptFile = isJavaScript(fileName); let start = new Date().getTime(); let currentToken = getTokenAtPosition(sourceFile, position); @@ -2932,13 +2936,29 @@ module ts { } let type = typeChecker.getTypeAtLocation(node); + addTypeProperties(type); + } + + function addTypeProperties(type: Type) { if (type) { // Filter private properties - forEach(type.getApparentProperties(), symbol => { + for (let symbol of type.getApparentProperties()) { if (typeChecker.isValidPropertyAccess((node.parent), symbol.name)) { symbols.push(symbol); } - }); + } + + if (isJavaScriptFile && type.flags & TypeFlags.Union) { + // In javascript files, for union types, we don't just get the members that + // the individual types have in common, we also include all the members that + // each individual type has. This is because we're going to add all identifiers + // anyways. So we might as well elevate the members that were at least part + // of the individual types to a higher status than since we know what they are. + let unionType = type; + for (let elementType of unionType.types) { + addTypeProperties(elementType); + } + } } } @@ -6031,6 +6051,7 @@ module ts { case ClassificationType.typeParameterName: return ClassificationTypeNames.typeParameterName; case ClassificationType.typeAliasName: return ClassificationTypeNames.typeAliasName; case ClassificationType.parameterName: return ClassificationTypeNames.parameterName; + case ClassificationType.docCommentTagName: return ClassificationTypeNames.docCommentTagName; } } @@ -6093,8 +6114,7 @@ module ts { // Only bother with the trivia if it at least intersects the span of interest. if (textSpanIntersectsWith(span, start, width)) { if (isComment(kind)) { - // Simple comment. Just add as is. - pushClassification(start, width, ClassificationType.comment); + classifyComment(token, kind, start, width); continue; } @@ -6118,6 +6138,90 @@ module ts { } } + function classifyComment(token: Node, 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. + let jsDocComment = parseIsolatedJSDocComment(sourceFile.text, start, width); + if (jsDocComment && jsDocComment.jsDocComment) { + jsDocComment.jsDocComment.parent = token; + classifyJSDocComment(jsDocComment.jsDocComment); + return; + } + } + + // Simple comment. Just add as is. + pushCommentRange(start, width); + } + + function pushCommentRange(start: number, width: number) { + pushClassification(start, width, ClassificationType.comment); + } + + function classifyJSDocComment(docComment: JSDocComment) { + let pos = docComment.pos; + + for (let tag of docComment.tags) { + if (tag.pos !== pos) { + pushCommentRange(pos, tag.pos - pos); + } + + pushClassification(tag.atToken.pos, tag.atToken.end - tag.atToken.pos, ClassificationType.punctuation); + pushClassification(tag.tagName.pos, tag.tagName.end - tag.tagName.pos, ClassificationType.docCommentTagName); + + pos = tag.tagName.end; + + switch (tag.kind) { + case SyntaxKind.JSDocParameterTag: + processJSDocParameterTag(tag); + break; + case SyntaxKind.JSDocTemplateTag: + processJSDocTemplateTag(tag); + break; + case SyntaxKind.JSDocTypeTag: + processElement((tag).typeExpression); + break; + case SyntaxKind.JSDocReturnTag: + processElement((tag).typeExpression); + break; + } + + pos = tag.end; + } + + if (pos !== docComment.end) { + pushCommentRange(pos, docComment.end - pos); + } + + return; + + function processJSDocParameterTag(tag: JSDocParameterTag) { + if (tag.preParameterName) { + pushCommentRange(pos, tag.preParameterName.pos - pos); + pushClassification(tag.preParameterName.pos, tag.preParameterName.end - tag.preParameterName.pos, ClassificationType.parameterName); + pos = tag.preParameterName.end; + } + + if (tag.typeExpression) { + pushCommentRange(pos, tag.typeExpression.pos - pos); + processElement(tag.typeExpression); + pos = tag.typeExpression.end; + } + + if (tag.postParameterName) { + pushCommentRange(pos, tag.postParameterName.pos - pos); + pushClassification(tag.postParameterName.pos, tag.postParameterName.end - tag.postParameterName.pos, ClassificationType.parameterName); + pos = tag.postParameterName.end; + } + } + } + + function processJSDocTemplateTag(tag: JSDocTemplateTag) { + for (let child of tag.getChildren()) { + processElement(child); + } + } + function classifyDisabledMergeCode(text: string, start: number, end: number) { // Classify the line that the ======= marker is on as a comment. Then just lex // all further tokens and add them to the result. @@ -6251,9 +6355,13 @@ module ts { } function processElement(element: Node) { + if (!element) { + return; + } + // Ignore nodes that don't intersect the original span to classify. if (textSpanIntersectsWith(span, element.getFullStart(), element.getFullWidth())) { - let children = element.getChildren(); + let children = element.getChildren(sourceFile); for (let child of children) { if (isToken(child)) { classifyToken(child); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 9c6fd814d9f..0d98c6e4869 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -639,6 +639,10 @@ module FourSlashInterface { return getClassification("punctuation", text, position); } + export function docCommentTagName(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } { + return getClassification("docCommentTagName", text, position); + } + export function className(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } { return getClassification("className", text, position); } diff --git a/tests/cases/fourslash/syntacticClassificationsDocComment1.ts b/tests/cases/fourslash/syntacticClassificationsDocComment1.ts new file mode 100644 index 00000000000..fe812b4c780 --- /dev/null +++ b/tests/cases/fourslash/syntacticClassificationsDocComment1.ts @@ -0,0 +1,17 @@ +/// + +//// /** @type {number} */ +//// var v; + +var c = classification; +verify.syntacticClassificationsAre( + c.comment("/** "), + c.punctuation("@"), + c.docCommentTagName("type"), + c.punctuation("{"), + c.keyword("number"), + c.punctuation("}"), + c.comment(" */"), + c.keyword("var"), + c.text("v"), + c.punctuation(";")); From caddec902a8d72f8a273bae09cbd46f5de792d81 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 28 May 2015 14:58:22 -0700 Subject: [PATCH 2/6] Remove uneeded code. --- src/compiler/parser.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 0595aa75959..bbc76c73b66 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -2,8 +2,6 @@ /// module ts { - export var throwOnJSDocErrors = false; - let nodeConstructors = new Array Node>(SyntaxKind.Count); /* @internal */ export let parseTime = 0; @@ -5049,13 +5047,6 @@ module ts { return finishNode(result); } - function setError(message: DiagnosticMessage) { - parseErrorAtCurrentToken(message); - if (throwOnJSDocErrors) { - throw new Error(message.key); - } - } - function parseJSDocTopLevelType(): JSDocType { var type = parseJSDocType(); if (token === SyntaxKind.BarToken) { From 0f3584e9cc5fbdd9ed9b556a24b9893ddb3d7565 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 28 May 2015 15:04:08 -0700 Subject: [PATCH 3/6] Removing unnecessary code. --- src/compiler/parser.ts | 12 ++++++------ src/services/services.ts | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index bbc76c73b66..f53c0799c9f 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -361,9 +361,9 @@ module ts { } } - export function createSourceFile(fileName: string, sourceText: string, languageVersion: ScriptTarget, setParentNodes = false, includeDocComments = false): SourceFile { + export function createSourceFile(fileName: string, sourceText: string, languageVersion: ScriptTarget, setParentNodes = false): SourceFile { let start = new Date().getTime(); - let result = Parser.parseSourceFile(fileName, sourceText, languageVersion, /*syntaxCursor*/ undefined, setParentNodes, includeDocComments); + let result = Parser.parseSourceFile(fileName, sourceText, languageVersion, /*syntaxCursor*/ undefined, setParentNodes); parseTime += new Date().getTime() - start; return result; @@ -491,10 +491,10 @@ module ts { // attached to the EOF token. let parseErrorBeforeNextFinishedNode: boolean = false; - export function parseSourceFile(fileName: string, _sourceText: string, languageVersion: ScriptTarget, _syntaxCursor: IncrementalParser.SyntaxCursor, setParentNodes?: boolean, includeDocComments?: boolean): SourceFile { + export function parseSourceFile(fileName: string, _sourceText: string, languageVersion: ScriptTarget, _syntaxCursor: IncrementalParser.SyntaxCursor, setParentNodes?: boolean): SourceFile { initializeState(fileName, _sourceText, languageVersion, _syntaxCursor); - let result = parseSourceFileWorker(fileName, languageVersion, setParentNodes, includeDocComments); + let result = parseSourceFileWorker(fileName, languageVersion, setParentNodes); clearState(); @@ -533,7 +533,7 @@ module ts { sourceText = undefined; } - function parseSourceFileWorker(fileName: string, languageVersion: ScriptTarget, setParentNodes: boolean, includeDocComments: boolean): SourceFile { + function parseSourceFileWorker(fileName: string, languageVersion: ScriptTarget, setParentNodes: boolean): SourceFile { sourceFile = createSourceFile(fileName, languageVersion); // Prime the scanner. @@ -558,7 +558,7 @@ module ts { // If this is a javascript file, proactively see if we can get JSDoc comments for // relevant nodes in the file. We'll use these to provide typing informaion if they're // available. - if (includeDocComments || isJavaScript(fileName)) { + if (isJavaScript(fileName)) { addJSDocComments(); } diff --git a/src/services/services.ts b/src/services/services.ts index 8990a62f664..350a7049728 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1816,7 +1816,7 @@ module ts { export function createLanguageServiceSourceFile(fileName: string, scriptSnapshot: IScriptSnapshot, scriptTarget: ScriptTarget, version: string, setNodeParents: boolean): SourceFile { let text = scriptSnapshot.getText(0, scriptSnapshot.getLength()); - let sourceFile = createSourceFile(fileName, text, scriptTarget, setNodeParents, /*includeDocComments:*/ true); + let sourceFile = createSourceFile(fileName, text, scriptTarget, setNodeParents); setSourceFileFields(sourceFile, scriptSnapshot, version); // after full parsing we can use table with interned strings as name table sourceFile.nameTable = sourceFile.identifiers; From 26103b8548cd03f23810ac8b70a01d046e4473f2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 28 May 2015 15:18:39 -0700 Subject: [PATCH 4/6] Remove unnecessary code. --- src/harness/harness.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/harness/harness.ts b/src/harness/harness.ts index 244c693ad12..90afab029eb 100644 --- a/src/harness/harness.ts +++ b/src/harness/harness.ts @@ -167,7 +167,7 @@ module Utils { continue; } var child = (node)[childName]; - if (isNodeOrArray(child) && childName !== "jsDocComment") { + if (isNodeOrArray(child)) { assert.isFalse(childNodesAndArrays.indexOf(child) < 0, "Missing child when forEach'ing over node: " + (ts).SyntaxKind[node.kind] + "-" + childName); } From 513183e7b1758a11d68a4f6a168da3180a39e911 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 2 Jun 2015 13:58:49 -0700 Subject: [PATCH 5/6] PR feedback. --- src/services/services.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 427c8db8cc9..cac897ed9ee 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2958,7 +2958,7 @@ module ts { // the individual types have in common, we also include all the members that // each individual type has. This is because we're going to add all identifiers // anyways. So we might as well elevate the members that were at least part - // of the individual types to a higher status than since we know what they are. + // of the individual types to a higher status since we know what they are. let unionType = type; for (let elementType of unionType.types) { addTypeProperties(elementType); @@ -6147,10 +6147,10 @@ module ts { if (kind === SyntaxKind.MultiLineCommentTrivia) { // See if this is a doc comment. If so, we'll classify certain portions of it // specially. - let jsDocComment = parseIsolatedJSDocComment(sourceFile.text, start, width); - if (jsDocComment && jsDocComment.jsDocComment) { - jsDocComment.jsDocComment.parent = token; - classifyJSDocComment(jsDocComment.jsDocComment); + let docCommentAndDiagnostics = parseIsolatedJSDocComment(sourceFile.text, start, width); + if (docCommentAndDiagnostics && docCommentAndDiagnostics.jsDocComment) { + docCommentAndDiagnostics.jsDocComment.parent = token; + classifyJSDocComment(docCommentAndDiagnostics.jsDocComment); return; } } @@ -6167,6 +6167,8 @@ module ts { let pos = docComment.pos; for (let tag of docComment.tags) { + // As we walk through each tag, classify the portion of text from the end of + // the last tag (or the start of the entire doc comment) as 'comment'. if (tag.pos !== pos) { pushCommentRange(pos, tag.pos - pos); } From 8fcd29f843f1948b967678f62bc6a1d539f0f2eb Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 2 Jun 2015 15:00:39 -0700 Subject: [PATCH 6/6] Adding tests. --- .../syntacticClassificationsDocComment2.ts | 26 +++++++++++++++++++ .../syntacticClassificationsDocComment3.ts | 20 ++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 tests/cases/fourslash/syntacticClassificationsDocComment2.ts create mode 100644 tests/cases/fourslash/syntacticClassificationsDocComment3.ts diff --git a/tests/cases/fourslash/syntacticClassificationsDocComment2.ts b/tests/cases/fourslash/syntacticClassificationsDocComment2.ts new file mode 100644 index 00000000000..201251dde6d --- /dev/null +++ b/tests/cases/fourslash/syntacticClassificationsDocComment2.ts @@ -0,0 +1,26 @@ +/// + +//// /** @param foo { function(x): string } */ +//// var v; + + +var c = classification; +verify.syntacticClassificationsAre( + c.comment("/** "), + c.punctuation("@"), + c.docCommentTagName("param"), + c.comment(" "), + c.parameterName("foo"), + c.comment(" "), + c.punctuation("{"), + c.keyword("function"), + c.punctuation("("), + c.text("x"), + c.punctuation(")"), + c.punctuation(":"), + c.keyword("string"), + c.punctuation("}"), + c.comment(" */"), + c.keyword("var"), + c.text("v"), + c.punctuation(";")); diff --git a/tests/cases/fourslash/syntacticClassificationsDocComment3.ts b/tests/cases/fourslash/syntacticClassificationsDocComment3.ts new file mode 100644 index 00000000000..bdc7faa470e --- /dev/null +++ b/tests/cases/fourslash/syntacticClassificationsDocComment3.ts @@ -0,0 +1,20 @@ +/// + +//// /** @param foo { number /* } */ +//// var v; + +var c = classification; +verify.syntacticClassificationsAre( + c.comment("/** "), + c.punctuation("@"), + c.docCommentTagName("param"), + c.comment(" "), + c.parameterName("foo"), + c.comment(" "), + c.punctuation("{"), + c.keyword("number"), + c.comment(" /* } */"), + c.comment("/* } */"), + c.keyword("var"), + c.text("v"), + c.punctuation(";"));