From 8fd1c7915c28dd6f7bcef083f87fc27e53825b3a Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 9 Oct 2014 13:38:33 -0700 Subject: [PATCH 1/2] Undid changes where contextual semantics are taken into account. --- src/services/services.ts | 45 ++++++++++--------- .../fourslash/semanticClassification2.ts | 3 +- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index b0c69aa99fe..64504446119 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2663,16 +2663,12 @@ module ts { function getSymbolKind(symbol: Symbol, meaningAtLocation: SemanticMeaning): string { var flags = typeInfoResolver.getRootSymbol(symbol).getFlags(); + // TODO(drosen): use meaningAtLocation. if (flags & SymbolFlags.Class) return ScriptElementKind.classElement; if (flags & SymbolFlags.Enum) return ScriptElementKind.enumElement; - - // The following should only apply if encountered at a type position, - // and need to have precedence over other meanings if this is the case. - if (meaningAtLocation & SemanticMeaning.Type) { - if (flags & SymbolFlags.Interface) return ScriptElementKind.interfaceElement; - if (flags & SymbolFlags.TypeParameter) return ScriptElementKind.typeParameterElement; - } - + if (flags & SymbolFlags.Interface) return ScriptElementKind.interfaceElement; + if (flags & SymbolFlags.TypeParameter) return ScriptElementKind.typeParameterElement; + var result = getSymbolKindOfConstructorPropertyMethodAccessorFunctionOrVar(symbol, flags); if (result === ScriptElementKind.unknown) { if (flags & SymbolFlags.TypeParameter) return ScriptElementKind.typeParameterElement; @@ -2745,10 +2741,13 @@ module ts { : ScriptElementKindModifier.none; } - function getSymbolDisplayPartsDocumentationAndSymbolKind(symbol: Symbol, sourceFile: SourceFile, enclosingDeclaration: Node, - typeResolver: TypeChecker, location: Node, - // TODO(drosen): Currently completion entry details passes the SemanticMeaning.All instead of using semanticMeaning of location - semanticMeaning = getMeaningFromLocation(location)) { + // TODO(drosen): Currently completion entry details passes the SemanticMeaning.All instead of using semanticMeaning of location + function getSymbolDisplayPartsDocumentationAndSymbolKind(symbol: Symbol, + sourceFile: SourceFile, + enclosingDeclaration: Node, + typeResolver: TypeChecker, + location: Node, + semanticMeaning: SemanticMeaning) { var displayParts: SymbolDisplayPart[] = []; var documentation: SymbolDisplayPart[]; var symbolFlags = typeResolver.getRootSymbol(symbol).flags; @@ -2853,13 +2852,15 @@ module ts { } } } + + // TODO(drosen): use semanticMeaning. if (symbolFlags & SymbolFlags.Class && !hasAddedSymbolInfo) { displayParts.push(keywordPart(SyntaxKind.ClassKeyword)); displayParts.push(spacePart()); displayParts.push.apply(displayParts, symbolToDisplayParts(typeResolver, symbol, sourceFile, /*meaning*/ undefined, SymbolFormatFlags.WriteTypeParametersOrArguments)); writeTypeParametersOfSymbol(symbol, sourceFile); } - if ((symbolFlags & SymbolFlags.Interface) && (semanticMeaning & SemanticMeaning.Type)) { + if (symbolFlags & SymbolFlags.Interface) { addNewLineIfDisplayPartsExist(); displayParts.push(keywordPart(SyntaxKind.InterfaceKeyword)); displayParts.push(spacePart()); @@ -2878,7 +2879,7 @@ module ts { displayParts.push(spacePart()); displayParts.push.apply(displayParts, symbolToDisplayParts(typeResolver, symbol, sourceFile)); } - if ((symbolFlags & SymbolFlags.TypeParameter) && (semanticMeaning & SemanticMeaning.Type)) { + if (symbolFlags & SymbolFlags.TypeParameter) { addNewLineIfDisplayPartsExist(); displayParts.push(punctuationPart(SyntaxKind.OpenParenToken)); displayParts.push(textPart("type parameter")); @@ -3049,7 +3050,8 @@ module ts { return undefined; } - var displayPartsDocumentationsAndKind = getSymbolDisplayPartsDocumentationAndSymbolKind(symbol, sourceFile, getContainerNode(node), typeInfoResolver, node); + // TODO(drosen): Properly address 'semanticMeaning' parameter here + var displayPartsDocumentationsAndKind = getSymbolDisplayPartsDocumentationAndSymbolKind(symbol, sourceFile, getContainerNode(node), typeInfoResolver, node, SemanticMeaning.All); return { kind: displayPartsDocumentationsAndKind.symbolKind, kindModifiers: getSymbolModifiers(symbol), @@ -4666,19 +4668,18 @@ module ts { function classifySymbol(symbol: Symbol, meaningAtPosition: SemanticMeaning) { var flags = symbol.getFlags(); + // TODO(drosen): use meaningAtPosition. if (flags & SymbolFlags.Class) { return ClassificationTypeNames.className; } else if (flags & SymbolFlags.Enum) { return ClassificationTypeNames.enumName; } - else if (meaningAtPosition & SemanticMeaning.Type) { - if (flags & SymbolFlags.Interface) { - return ClassificationTypeNames.interfaceName; - } - else if (flags & SymbolFlags.TypeParameter) { - return ClassificationTypeNames.typeParameterName; - } + else if (flags & SymbolFlags.Interface) { + return ClassificationTypeNames.interfaceName; + } + else if (flags & SymbolFlags.TypeParameter) { + return ClassificationTypeNames.typeParameterName; } else if (flags & SymbolFlags.Module) { return ClassificationTypeNames.moduleName; diff --git a/tests/cases/fourslash/semanticClassification2.ts b/tests/cases/fourslash/semanticClassification2.ts index 810636dba3e..8d393140154 100644 --- a/tests/cases/fourslash/semanticClassification2.ts +++ b/tests/cases/fourslash/semanticClassification2.ts @@ -8,4 +8,5 @@ //// Thing.toExponential(); var c = classification; -verify.semanticClassificationsAre(c.interfaceName("Thing")); \ No newline at end of file +// NOTE: this is *wrong*, but will be fixed shortly. +verify.semanticClassificationsAre(c.interfaceName("Thing"), c.interfaceName("Thing"), c.interfaceName("Thing")); \ No newline at end of file From 8659dc5ca0692391d7b5771680bd060d58cc1fc3 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 9 Oct 2014 13:54:37 -0700 Subject: [PATCH 2/2] Addressed CR feedback. --- src/services/services.ts | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 64504446119..99a2dd2b65c 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2259,13 +2259,9 @@ module ts { return undefined; } - // TODO(drosen): Right now we just permit *all* semantic meanings when calling 'getSymbolKind' - // which is permissible given that it is backwards compatible; but really we should consider - // passing the meaning for the node so that we don't report that a suggestion for a value is an interface. - // We COULD also just do what 'getSymbolModifiers' does, which is to use the first declaration. return { name: displayName, - kind: getSymbolKind(symbol, SemanticMeaning.All), + kind: getSymbolKind(symbol), kindModifiers: getSymbolModifiers(symbol) }; } @@ -2617,7 +2613,7 @@ module ts { // which is permissible given that it is backwards compatible; but really we should consider // passing the meaning for the node so that we don't report that a suggestion for a value is an interface. // We COULD also just do what 'getSymbolModifiers' does, which is to use the first declaration. - var displayPartsDocumentationsAndSymbolKind = getSymbolDisplayPartsDocumentationAndSymbolKind(symbol, getSourceFile(filename), session.location, session.typeChecker, session.location, SemanticMeaning.All); + var displayPartsDocumentationsAndSymbolKind = getSymbolDisplayPartsDocumentationAndSymbolKind(symbol, getSourceFile(filename), session.location, session.typeChecker, session.location); return { name: entryName, kind: displayPartsDocumentationsAndSymbolKind.symbolKind, @@ -2660,10 +2656,10 @@ module ts { } } - function getSymbolKind(symbol: Symbol, meaningAtLocation: SemanticMeaning): string { + // TODO(drosen): use contextual SemanticMeaning. + function getSymbolKind(symbol: Symbol): string { var flags = typeInfoResolver.getRootSymbol(symbol).getFlags(); - // TODO(drosen): use meaningAtLocation. if (flags & SymbolFlags.Class) return ScriptElementKind.classElement; if (flags & SymbolFlags.Enum) return ScriptElementKind.enumElement; if (flags & SymbolFlags.Interface) return ScriptElementKind.interfaceElement; @@ -2741,13 +2737,12 @@ module ts { : ScriptElementKindModifier.none; } - // TODO(drosen): Currently completion entry details passes the SemanticMeaning.All instead of using semanticMeaning of location + // TODO(drosen): use contextual SemanticMeaning. function getSymbolDisplayPartsDocumentationAndSymbolKind(symbol: Symbol, sourceFile: SourceFile, enclosingDeclaration: Node, typeResolver: TypeChecker, - location: Node, - semanticMeaning: SemanticMeaning) { + location: Node) { var displayParts: SymbolDisplayPart[] = []; var documentation: SymbolDisplayPart[]; var symbolFlags = typeResolver.getRootSymbol(symbol).flags; @@ -2853,7 +2848,6 @@ module ts { } } - // TODO(drosen): use semanticMeaning. if (symbolFlags & SymbolFlags.Class && !hasAddedSymbolInfo) { displayParts.push(keywordPart(SyntaxKind.ClassKeyword)); displayParts.push(spacePart()); @@ -2959,7 +2953,7 @@ module ts { } } else { - symbolKind = getSymbolKind(symbol, semanticMeaning); + symbolKind = getSymbolKind(symbol); } } @@ -3050,8 +3044,7 @@ module ts { return undefined; } - // TODO(drosen): Properly address 'semanticMeaning' parameter here - var displayPartsDocumentationsAndKind = getSymbolDisplayPartsDocumentationAndSymbolKind(symbol, sourceFile, getContainerNode(node), typeInfoResolver, node, SemanticMeaning.All); + var displayPartsDocumentationsAndKind = getSymbolDisplayPartsDocumentationAndSymbolKind(symbol, sourceFile, getContainerNode(node), typeInfoResolver, node); return { kind: displayPartsDocumentationsAndKind.symbolKind, kindModifiers: getSymbolModifiers(symbol), @@ -3166,7 +3159,7 @@ module ts { var declarations = symbol.getDeclarations(); var symbolName = typeInfoResolver.symbolToString(symbol); // Do not get scoped name, just the name of the symbol - var symbolKind = getSymbolKind(symbol, getMeaningFromLocation(node)); + var symbolKind = getSymbolKind(symbol); var containerSymbol = symbol.parent; var containerName = containerSymbol ? typeInfoResolver.symbolToString(containerSymbol, node) : ""; @@ -5153,7 +5146,7 @@ module ts { // Only allow a symbol to be renamed if it actually has at least one declaration. if (symbol && symbol.getDeclarations() && symbol.getDeclarations().length > 0) { - var kind = getSymbolKind(symbol, getMeaningFromLocation(node)); + var kind = getSymbolKind(symbol); if (kind) { return getRenameInfo(symbol.name, typeInfoResolver.getFullyQualifiedName(symbol), kind, getSymbolModifiers(symbol),