From fde4c188ac7976b0d4500e1ccc4ef0b7b680f1b7 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 26 Jul 2017 10:57:29 -0700 Subject: [PATCH] Address more PR comments --- src/compiler/checker.ts | 4 +- src/compiler/parser.ts | 162 +++++++++++++++++++------------------- src/compiler/types.ts | 7 +- src/compiler/utilities.ts | 19 +++-- src/services/jsDoc.ts | 7 +- 5 files changed, 105 insertions(+), 94 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b7e08ea41a0..c4ccbd17f5c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4493,8 +4493,8 @@ namespace ts { if (declaration.kind === SyntaxKind.ExportAssignment) { return links.type = checkExpression((declaration).expression); } - if (isInJavaScriptFile(declaration) && declaration.kind === SyntaxKind.JSDocPropertyTag && (declaration).typeExpression) { - return links.type = getTypeFromTypeNode((declaration).typeExpression.type); + if (isInJavaScriptFile(declaration) && isJSDocPropertyLikeTag(declaration) && declaration.typeExpression) { + return links.type = getTypeFromTypeNode(declaration.typeExpression.type); } // Handle variable, parameter or property if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) { diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index a0bd6c02c51..afc7b0e97b8 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -412,12 +412,12 @@ namespace ts { case SyntaxKind.JSDocParameterTag: case SyntaxKind.JSDocPropertyTag: if ((node as JSDocPropertyLikeTag).isNameFirst) { - return visitNode(cbNode, (node).fullName) || + return visitNode(cbNode, (node).name) || visitNode(cbNode, (node).typeExpression); } else { return visitNode(cbNode, (node).typeExpression) || - visitNode(cbNode, (node).fullName); + visitNode(cbNode, (node).name); } case SyntaxKind.JSDocReturnTag: return visitNode(cbNode, (node).typeExpression); @@ -438,7 +438,10 @@ namespace ts { visitNode(cbNode, (node).typeExpression); } case SyntaxKind.JSDocTypeLiteral: - return visitNodes(cbNode, cbNodes, (node).jsDocPropertyTags); + for (const tag of (node as JSDocTypeLiteral).jsDocPropertyTags) { + visitNode(cbNode, tag); + } + return; case SyntaxKind.PartiallyEmittedExpression: return visitNode(cbNode, (node).expression); } @@ -1943,14 +1946,18 @@ namespace ts { break; } dotPos = scanner.getStartPos(); - const node: QualifiedName = createNode(SyntaxKind.QualifiedName, entity.pos); - node.left = entity; - node.right = parseRightSideOfDot(allowReservedWords); - entity = finishNode(node); + entity = createQualifiedName(entity, parseRightSideOfDot(allowReservedWords)); } return entity; } + function createQualifiedName(entity: EntityName, name: Identifier): QualifiedName { + const node = createNode(SyntaxKind.QualifiedName, entity.pos) as QualifiedName; + node.left = entity; + node.right = name; + return finishNode(node); + } + function parseRightSideOfDot(allowIdentifierNames: boolean): Identifier { // Technically a keyword is valid here as all identifiers and keywords are identifier names. // However, often we'll encounter this in error situations when the identifier or keyword @@ -6473,10 +6480,10 @@ namespace ts { }); } - function parseBracketNameInPropertyAndParamTag(): { fullName: EntityName, isBracketed: boolean } { + function parseBracketNameInPropertyAndParamTag(): { name: EntityName, isBracketed: boolean } { // Looking for something like '[foo]', 'foo', '[foo.bar]' or 'foo.bar' const isBracketed = parseOptional(SyntaxKind.OpenBracketToken); - const fullName = parseJSDocEntityName(/*createIfMissing*/ true); + const name = parseJSDocEntityName(); if (isBracketed) { skipWhitespace(); @@ -6488,68 +6495,68 @@ namespace ts { parseExpected(SyntaxKind.CloseBracketToken); } - return { fullName, isBracketed }; + return { name, isBracketed }; } function isObjectOrObjectArrayTypeReference(node: TypeNode): boolean { - return node.kind === SyntaxKind.ObjectKeyword || - isTypeReferenceNode(node) && ts.isIdentifier(node.typeName) && node.typeName.text === "Object" || - node.kind === SyntaxKind.ArrayType && isObjectOrObjectArrayTypeReference((node as ArrayTypeNode).elementType); + switch (node.kind) { + case SyntaxKind.ObjectKeyword: + return true; + case SyntaxKind.ArrayType: + return isObjectOrObjectArrayTypeReference((node as ArrayTypeNode).elementType); + default: + return isTypeReferenceNode(node) && ts.isIdentifier(node.typeName) && node.typeName.text === "Object"; + } } function parseParameterOrPropertyTag(atToken: AtToken, tagName: Identifier, target: PropertyLikeParse.Parameter): JSDocParameterTag; function parseParameterOrPropertyTag(atToken: AtToken, tagName: Identifier, target: PropertyLikeParse.Property): JSDocPropertyTag; function parseParameterOrPropertyTag(atToken: AtToken, tagName: Identifier, target: PropertyLikeParse): JSDocPropertyLikeTag { let typeExpression = tryParseTypeExpression(); + let isNameFirst = !typeExpression; skipWhitespace(); - const { fullName, isBracketed } = parseBracketNameInPropertyAndParamTag(); + const { name, isBracketed } = parseBracketNameInPropertyAndParamTag(); skipWhitespace(); - let preName: EntityName, postName: EntityName; - if (typeExpression) { - postName = fullName; - } - else { - preName = fullName; + if (isNameFirst) { typeExpression = tryParseTypeExpression(); } - const result: JSDocPropertyLikeTag = target ? + const result: JSDocPropertyLikeTag = target === PropertyLikeParse.Parameter ? createNode(SyntaxKind.JSDocParameterTag, atToken.pos) : createNode(SyntaxKind.JSDocPropertyTag, atToken.pos); - const nestedTypeLiteral = parseNestedTypeLiteral(typeExpression, fullName); + const nestedTypeLiteral = parseNestedTypeLiteral(typeExpression, name); if (nestedTypeLiteral) { typeExpression = nestedTypeLiteral; + isNameFirst = true; } result.atToken = atToken; result.tagName = tagName; result.typeExpression = typeExpression; - if (typeExpression) { - result.type = typeExpression.type; - } - result.fullName = postName || preName; - result.name = ts.isIdentifier(result.fullName) ? result.fullName : result.fullName.right; - result.isNameFirst = !!nestedTypeLiteral || (postName ? false : !!preName); + result.name = name; + result.isNameFirst = isNameFirst; result.isBracketed = isBracketed; return finishNode(result); } - function parseNestedTypeLiteral(typeExpression: JSDocTypeExpression, fullName: EntityName) { + function parseNestedTypeLiteral(typeExpression: JSDocTypeExpression, name: EntityName) { if (typeExpression && isObjectOrObjectArrayTypeReference(typeExpression.type)) { const typeLiteralExpression = createNode(SyntaxKind.JSDocTypeExpression, scanner.getTokenPos()); - let child: JSDocPropertyLikeTag | false; + let child: JSDocParameterTag | false; let jsdocTypeLiteral: JSDocTypeLiteral; const start = scanner.getStartPos(); - while (child = tryParse(() => parseChildParameterOrPropertyTag(PropertyLikeParse.Parameter, fullName))) { - if (!jsdocTypeLiteral) { - jsdocTypeLiteral = createNode(SyntaxKind.JSDocTypeLiteral, start); - jsdocTypeLiteral.jsDocPropertyTags = [] as MutableNodeArray; + let children: JSDocParameterTag[]; + while (child = tryParse(() => parseChildParameterOrPropertyTag(PropertyLikeParse.Parameter, name))) { + if (!children) { + children = []; } - (jsdocTypeLiteral.jsDocPropertyTags as MutableNodeArray).push(child as JSDocPropertyTag); + children.push(child); } - if (jsdocTypeLiteral) { + if (children) { + jsdocTypeLiteral = createNode(SyntaxKind.JSDocTypeLiteral, start); + jsdocTypeLiteral.jsDocPropertyTags = children; if (typeExpression.type.kind === SyntaxKind.ArrayType) { jsdocTypeLiteral.isArrayType = true; } @@ -6678,61 +6685,58 @@ namespace ts { } } - function textsEqual(parent: EntityName, name: EntityName): boolean { - while (!ts.isIdentifier(parent) || !ts.isIdentifier(name)) { - if (!ts.isIdentifier(parent) && !ts.isIdentifier(name) && parent.right.text === name.right.text) { - parent = parent.left; - name = name.left; + function textsEqual(a: EntityName, b: EntityName): boolean { + while (!ts.isIdentifier(a) || !ts.isIdentifier(b)) { + if (!ts.isIdentifier(a) && !ts.isIdentifier(b) && a.right.text === b.right.text) { + a = a.left; + b = b.left; } else { return false; } } - return parent.text === name.text; + return a.text === b.text; } function parseChildParameterOrPropertyTag(target: PropertyLikeParse.Property): JSDocTypeTag | JSDocPropertyTag | false; - function parseChildParameterOrPropertyTag(target: PropertyLikeParse.Parameter, fullName: EntityName): JSDocPropertyTag | JSDocParameterTag | false; - function parseChildParameterOrPropertyTag(target: PropertyLikeParse, fullName?: EntityName): JSDocTypeTag | JSDocPropertyTag | JSDocParameterTag | false { - let resumePos = scanner.getStartPos(); + function parseChildParameterOrPropertyTag(target: PropertyLikeParse.Parameter, name: EntityName): JSDocParameterTag | false; + function parseChildParameterOrPropertyTag(target: PropertyLikeParse, name?: EntityName): JSDocTypeTag | JSDocPropertyTag | JSDocParameterTag | false { let canParseTag = true; let seenAsterisk = false; - while (token() !== SyntaxKind.EndOfFileToken) { + while (true) { nextJSDocToken(); switch (token()) { - case SyntaxKind.AtToken: - if (canParseTag) { - const child = tryParseChildTag(target); - if (child && child.kind === SyntaxKind.JSDocParameterTag && - (ts.isIdentifier(child.fullName) || !textsEqual(fullName, child.fullName.left))) { - break; + case SyntaxKind.AtToken: + if (canParseTag) { + const child = tryParseChildTag(target); + if (child && child.kind === SyntaxKind.JSDocParameterTag && + (ts.isIdentifier(child.name) || !textsEqual(name, child.name.left))) { + return false; + } + return child; } - return child; - } - seenAsterisk = false; - break; - case SyntaxKind.NewLineTrivia: - resumePos = scanner.getStartPos() - 1; - canParseTag = true; - seenAsterisk = false; - break; - case SyntaxKind.AsteriskToken: - if (seenAsterisk) { + seenAsterisk = false; + break; + case SyntaxKind.NewLineTrivia: + canParseTag = true; + seenAsterisk = false; + break; + case SyntaxKind.AsteriskToken: + if (seenAsterisk) { + canParseTag = false; + } + seenAsterisk = true; + break; + case SyntaxKind.Identifier: canParseTag = false; - } - seenAsterisk = true; - break; - case SyntaxKind.Identifier: - canParseTag = false; - break; - case SyntaxKind.EndOfFileToken: - break; + break; + case SyntaxKind.EndOfFileToken: + return false; } } - scanner.setTextPos(resumePos); } - function tryParseChildTag(target: PropertyLikeParse, alreadyHasTypeTag?: boolean): JSDocTypeTag | JSDocPropertyTag | JSDocParameterTag | false { + function tryParseChildTag(target: PropertyLikeParse): JSDocTypeTag | JSDocPropertyTag | JSDocParameterTag | false { Debug.assert(token() === SyntaxKind.AtToken); const atToken = createNode(SyntaxKind.AtToken, scanner.getStartPos()); atToken.end = scanner.getTextPos(); @@ -6745,7 +6749,7 @@ namespace ts { } switch (tagName.text) { case "type": - return !alreadyHasTypeTag && target === PropertyLikeParse.Property && parseTypeTag(atToken, tagName); + return target === PropertyLikeParse.Property && parseTypeTag(atToken, tagName); case "prop": case "property": return target === PropertyLikeParse.Property && parseParameterOrPropertyTag(atToken, tagName, target); @@ -6801,8 +6805,8 @@ namespace ts { return currentToken = scanner.scanJSDocToken(); } - function parseJSDocEntityName(createIfMissing = false): EntityName { - let entity: EntityName = parseJSDocIdentifierName(createIfMissing); + function parseJSDocEntityName(): EntityName { + let entity: EntityName = parseJSDocIdentifierName(/*createIfMissing*/ true); if (parseOptional(SyntaxKind.OpenBracketToken)) { parseExpected(SyntaxKind.CloseBracketToken); // Note that y[] is accepted as an entity name, but the postfix brackets are not saved for checking. @@ -6810,13 +6814,11 @@ namespace ts { // but it's not worth it to enforce that restriction. } while (parseOptional(SyntaxKind.DotToken)) { - const node: QualifiedName = createNode(SyntaxKind.QualifiedName, entity.pos) as QualifiedName; - node.left = entity; - node.right = parseJSDocIdentifierName(createIfMissing); + const name = parseJSDocIdentifierName(/*createIfMissing*/ true); if (parseOptional(SyntaxKind.OpenBracketToken)) { parseExpected(SyntaxKind.CloseBracketToken); } - entity = finishNode(node); + entity = createQualifiedName(entity, name); } return entity; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index f608684a136..f8170d62238 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2129,10 +2129,9 @@ namespace ts { typeExpression?: JSDocTypeExpression | JSDocTypeLiteral; } - export interface JSDocPropertyLikeTag extends JSDocTag, VariableLikeDeclaration { + export interface JSDocPropertyLikeTag extends JSDocTag, Declaration { parent: JSDoc; - fullName?: EntityName; - name: Identifier; + name: EntityName; typeExpression: JSDocTypeExpression; /** Whether the property name came before the type -- non-standard for JSDoc, but Typescript-like */ isNameFirst: boolean; @@ -2149,7 +2148,7 @@ namespace ts { export interface JSDocTypeLiteral extends JSDocType { kind: SyntaxKind.JSDocTypeLiteral; - jsDocPropertyTags?: NodeArray; + jsDocPropertyTags?: ReadonlyArray; jsDocTypeTag?: JSDocTypeTag; /** If true, then this type literal represents an *array* of its type. */ isArrayType?: boolean; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index f20a2d422fd..1db25a0d211 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1542,13 +1542,10 @@ namespace ts { export function getJSDocParameterTags(param: ParameterDeclaration): JSDocParameterTag[] | undefined { if (param.name && isIdentifier(param.name)) { const name = param.name.text; - return getJSDocTags(param.parent).filter((tag): tag is JSDocParameterTag => isJSDocParameterTag(tag) && tag.name.text === name) as JSDocParameterTag[]; - } - else { - // TODO: it's a destructured parameter, so it should look up an "object type" series of multiple lines - // But multi-line object types aren't supported yet either - return undefined; + return getJSDocTags(param.parent).filter((tag): tag is JSDocParameterTag => isJSDocParameterTag(tag) && isIdentifier(tag.name) && tag.name.text === name) as JSDocParameterTag[]; } + // a binding pattern doesn't have a name, so it's not possible to match it a jsdoc parameter, which is identified by name + return undefined; } /** Does the opposite of `getJSDocParameterTags`: given a JSDoc parameter, finds the parameter corresponding to it. */ @@ -1556,6 +1553,9 @@ namespace ts { if (node.symbol) { return node.symbol; } + if (!isIdentifier(node.name)) { + return undefined; + } const name = node.name.text; Debug.assert(node.parent!.kind === SyntaxKind.JSDocComment); const func = node.parent!.parent!; @@ -4049,6 +4049,9 @@ namespace ts { if (!declaration) { return undefined; } + if (isJSDocPropertyLikeTag(declaration) && declaration.name.kind === SyntaxKind.QualifiedName) { + return declaration.name.right; + } if (declaration.kind === SyntaxKind.BinaryExpression) { const expr = declaration as BinaryExpression; switch (getSpecialPropertyAssignmentKind(expr)) { @@ -4707,6 +4710,10 @@ namespace ts { return node.kind === SyntaxKind.JSDocPropertyTag; } + export function isJSDocPropertyLikeTag(node: Node): node is JSDocPropertyLikeTag { + return node.kind === SyntaxKind.JSDocPropertyTag || node.kind === SyntaxKind.JSDocParameterTag; + } + export function isJSDocTypeLiteral(node: Node): node is JSDocTypeLiteral { return node.kind === SyntaxKind.JSDocTypeLiteral; } diff --git a/src/services/jsDoc.ts b/src/services/jsDoc.ts index 464d251d4c2..2575d26cddd 100644 --- a/src/services/jsDoc.ts +++ b/src/services/jsDoc.ts @@ -120,7 +120,10 @@ namespace ts.JsDoc { } export function getJSDocParameterNameCompletions(tag: JSDocParameterTag): CompletionEntry[] { - const nameThusFar = isIdentifier(tag.fullName) ? unescapeLeadingUnderscores(tag.name.text) : undefined; + if (!isIdentifier(tag.name)) { + return emptyArray; + } + const nameThusFar = unescapeLeadingUnderscores(tag.name.text); const jsdoc = tag.parent; const fn = jsdoc.parent; if (!ts.isFunctionLike(fn)) return []; @@ -129,7 +132,7 @@ namespace ts.JsDoc { if (!isIdentifier(param.name)) return undefined; const name = unescapeLeadingUnderscores(param.name.text); - if (jsdoc.tags.some(t => t !== tag && isJSDocParameterTag(t) && isIdentifier(t.fullName) && t.name.text === name) + if (jsdoc.tags.some(t => t !== tag && isJSDocParameterTag(t) && isIdentifier(t.name) && t.name.text === name) || nameThusFar !== undefined && !startsWith(name, nameThusFar)) { return undefined; }