From 4047233fb80fa36b7c7fb9580d8c90f1881a2e85 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 23 Apr 2018 13:26:34 -0700 Subject: [PATCH 1/2] Ensure getSymbolKind matches getSymbolDisplayPartsDocumentationAndSymbolKind --- src/harness/fourslash.ts | 3 +- src/services/symbolDisplay.ts | 35 +++++++++++-------- .../completionsRecommended_import.ts | 2 +- .../fourslash/completionsRecommended_local.ts | 4 +-- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index a29a25af19bd3..68e20c4eabf22 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -921,8 +921,7 @@ namespace FourSlash { const actualDetails = this.getCompletionEntryDetails(actual.name, actual.source); assert.equal(ts.displayPartsToString(actualDetails.displayParts), text); assert.equal(ts.displayPartsToString(actualDetails.documentation), documentation || ""); - // TODO: GH#23587 - // assert.equal(actualDetails.kind, actual.kind); + assert.equal(actualDetails.kind, actual.kind); assert.equal(actualDetails.kindModifiers, actual.kindModifiers); assert.equal(actualDetails.source && ts.displayPartsToString(actualDetails.source), sourceDisplay); } diff --git a/src/services/symbolDisplay.ts b/src/services/symbolDisplay.ts index 7abb89befd7a7..b0c039cf15d37 100644 --- a/src/services/symbolDisplay.ts +++ b/src/services/symbolDisplay.ts @@ -9,8 +9,9 @@ namespace ts.SymbolDisplay { const flags = getCombinedLocalAndExportSymbolFlags(symbol); if (flags & SymbolFlags.Class) { - return getDeclarationOfKind(symbol, SyntaxKind.ClassExpression) ? - ScriptElementKind.localClassElement : ScriptElementKind.classElement; + const callExpressionLike = getCallExpressionLike(location, symbol); + return callExpressionLike && shouldUseConstructSignatures(callExpressionLike) ? ScriptElementKind.constructorImplementationElement + : getDeclarationOfKind(symbol, SyntaxKind.ClassExpression) ? ScriptElementKind.localClassElement : ScriptElementKind.classElement; } if (flags & SymbolFlags.Enum) return ScriptElementKind.enumElement; if (flags & SymbolFlags.TypeAlias) return ScriptElementKind.typeElement; @@ -155,22 +156,12 @@ namespace ts.SymbolDisplay { } // try get the call/construct signature from the type if it matches - let callExpressionLike: CallExpression | NewExpression | JsxOpeningLikeElement; - if (isCallOrNewExpression(location)) { - callExpressionLike = location; - } - else if (isCallExpressionTarget(location) || isNewExpressionTarget(location)) { - callExpressionLike = location.parent; - } - else if (location.parent && isJsxOpeningLikeElement(location.parent) && isFunctionLike(symbol.valueDeclaration)) { - callExpressionLike = location.parent; - } - + const callExpressionLike = getCallExpressionLike(location, symbol); if (callExpressionLike) { const candidateSignatures: Signature[] = []; signature = typeChecker.getResolvedSignature(callExpressionLike, candidateSignatures); - const useConstructSignatures = callExpressionLike.kind === SyntaxKind.NewExpression || (isCallExpression(callExpressionLike) && callExpressionLike.expression.kind === SyntaxKind.SuperKeyword); + const useConstructSignatures = shouldUseConstructSignatures(callExpressionLike); const allSignatures = useConstructSignatures ? type.getConstructSignatures() : type.getCallSignatures(); @@ -648,4 +639,20 @@ namespace ts.SymbolDisplay { return true; }); } + + type CallExpressionLike = CallExpression | NewExpression | JsxOpeningLikeElement | undefined; + function getCallExpressionLike(location: Node, symbol: Symbol): CallExpressionLike { + if (isCallOrNewExpression(location)) { + return location; + } + else if (isCallExpressionTarget(location) || isNewExpressionTarget(location)) { + return location.parent; + } + else if (location.parent && isJsxOpeningLikeElement(location.parent) && isFunctionLike(symbol.valueDeclaration)) { + return location.parent; + } + } + function shouldUseConstructSignatures(callExpressionLike: CallExpressionLike): boolean { + return callExpressionLike.kind === SyntaxKind.NewExpression || (isCallExpression(callExpressionLike) && callExpressionLike.expression.kind === SyntaxKind.SuperKeyword); + } } diff --git a/tests/cases/fourslash/completionsRecommended_import.ts b/tests/cases/fourslash/completionsRecommended_import.ts index 158221f03a8c8..4bef4452ff51f 100644 --- a/tests/cases/fourslash/completionsRecommended_import.ts +++ b/tests/cases/fourslash/completionsRecommended_import.ts @@ -23,7 +23,7 @@ goTo.eachMarker(["b0", "b1"], (_, idx) => { { name: "Cls", source: "/a" }, idx === 0 ? "constructor Cls(): Cls" : "class Cls", "", - "class", + idx === 0 ? "constructor" : "class", undefined, /*hasAction*/ true, { includeExternalModuleExports: true, diff --git a/tests/cases/fourslash/completionsRecommended_local.ts b/tests/cases/fourslash/completionsRecommended_local.ts index 768db8c0c9120..2507b5cf328b0 100644 --- a/tests/cases/fourslash/completionsRecommended_local.ts +++ b/tests/cases/fourslash/completionsRecommended_local.ts @@ -24,7 +24,7 @@ goTo.eachMarker(["c0", "c1"], (_, idx) => { "Cls", idx === 0 ? "constructor Cls(): Cls" : "class Cls", "", - "class", + idx === 0 ? "constructor" : "class", undefined, undefined, { isRecommended: true, @@ -33,5 +33,5 @@ goTo.eachMarker(["c0", "c1"], (_, idx) => { goTo.eachMarker(["a0", "a1"], (_, idx) => { // Not recommended, because it's an abstract class - verify.completionListContains("Abs", idx == 0 ? "constructor Abs(): Abs" : "class Abs", "", "class"); + verify.completionListContains("Abs", idx == 0 ? "constructor Abs(): Abs" : "class Abs", "", idx === 0 ? "constructor" : "class"); }); From bdca6d03c3e8bd14f3613296e9d3dc6e6dbe99ee Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 23 Apr 2018 15:31:13 -0700 Subject: [PATCH 2/2] Fix for property access --- src/services/symbolDisplay.ts | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/services/symbolDisplay.ts b/src/services/symbolDisplay.ts index b0c039cf15d37..bde546e7e7b74 100644 --- a/src/services/symbolDisplay.ts +++ b/src/services/symbolDisplay.ts @@ -9,7 +9,7 @@ namespace ts.SymbolDisplay { const flags = getCombinedLocalAndExportSymbolFlags(symbol); if (flags & SymbolFlags.Class) { - const callExpressionLike = getCallExpressionLike(location, symbol); + const callExpressionLike = getCallExpressionLike(climbParentPropertyAccesses(location), symbol); return callExpressionLike && shouldUseConstructSignatures(callExpressionLike) ? ScriptElementKind.constructorImplementationElement : getDeclarationOfKind(symbol, SyntaxKind.ClassExpression) ? ScriptElementKind.localClassElement : ScriptElementKind.classElement; } @@ -147,13 +147,7 @@ namespace ts.SymbolDisplay { let signature: Signature; type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol.exportSymbol || symbol, location); - if (location.parent && location.parent.kind === SyntaxKind.PropertyAccessExpression) { - const right = (location.parent).name; - // Either the location is on the right of a property access, or on the left and the right is missing - if (right === location || (right && right.getFullWidth() === 0)) { - location = location.parent; - } - } + location = climbParentPropertyAccesses(location); // try get the call/construct signature from the type if it matches const callExpressionLike = getCallExpressionLike(location, symbol); @@ -655,4 +649,15 @@ namespace ts.SymbolDisplay { function shouldUseConstructSignatures(callExpressionLike: CallExpressionLike): boolean { return callExpressionLike.kind === SyntaxKind.NewExpression || (isCallExpression(callExpressionLike) && callExpressionLike.expression.kind === SyntaxKind.SuperKeyword); } + + function climbParentPropertyAccesses(node: Node): Node { + if (node.parent && isPropertyAccessExpression(node.parent)) { + const { name } = node.parent; + // Either the location is on the right of a property access, or on the left and the right is missing + if (node === name || (name && name.getFullWidth() === 0)) { + return node.parent; + } + } + return node; + } }