getSymbolsInScope() should return local symbols, not exported symbols

Author: Andy Hanson

src/compiler/checker.ts

@@ -22226,11 +22226,6 @@ namespace ts {
                     }
 
                     switch (location.kind) {
-                        case SyntaxKind.SourceFile:
-                            if (!isExternalOrCommonJsModule(<SourceFile>location)) {
-                                break;
-                            }
-                            // falls through
                         case SyntaxKind.ModuleDeclaration:
                             copySymbols(getSymbolOfNode(location).exports, meaning & SymbolFlags.ModuleMember);
                             break;
@@ -22282,7 +22277,7 @@ namespace ts {
              * @param meaning meaning of symbol to filter by before adding to symbol table
              */
             function copySymbol(symbol: Symbol, meaning: SymbolFlags): void {
-                if (symbol.flags & meaning) {
+                if (getCombinedLocalAndExportSymbolFlags(symbol) & meaning) {
                     const id = symbol.name;
                     // We will copy all symbol regardless of its reserved name because
                     // symbolsToArray will check whether the key is a reserved name and

src/compiler/utilities.ts

@@ -3549,6 +3549,11 @@ namespace ts {
         }
         return previous[previous.length - 1];
     }
+
+    /** See comment on `declareModuleMember` in `binder.ts`. */
+    export function getCombinedLocalAndExportSymbolFlags(symbol: Symbol): SymbolFlags {
+        return symbol.exportSymbol ? symbol.exportSymbol.flags | symbol.flags : symbol.flags;
+    }
 }
 
 namespace ts {

src/services/completions.ts

@@ -91,7 +91,7 @@ namespace ts.Completions {
 
             if (!uniqueNames.get(name)) {
                 uniqueNames.set(name, name);
-                const displayName = getCompletionEntryDisplayName(unescapeIdentifier(name), target, /*performCharacterChecks*/ true);
+                const displayName = getCompletionEntryDisplayName(name, target, /*performCharacterChecks*/ true);
                 if (displayName) {
                     const entry = {
                         name: displayName,
@@ -608,7 +608,7 @@ namespace ts.Completions {
                     // Extract module or enum members
                     const exportedSymbols = typeChecker.getExportsOfModule(symbol);
                     const isValidValueAccess = (symbol: Symbol) => typeChecker.isValidPropertyAccess(<PropertyAccessExpression>(node.parent), symbol.name);
-                    const isValidTypeAccess = (symbol: Symbol) => symbolCanbeReferencedAtTypeLocation(symbol);
+                    const isValidTypeAccess = (symbol: Symbol) => symbolCanBeReferencedAtTypeLocation(symbol);
                     const isValidAccess = isRhsOfImportDeclaration ?
                         // Any kind is allowed when dotting off namespace in internal import equals declaration
                         (symbol: Symbol) => isValidTypeAccess(symbol) || isValidValueAccess(symbol) :
@@ -768,12 +768,12 @@ namespace ts.Completions {
                         (!isContextTokenValueLocation(contextToken) &&
                             (isPartOfTypeNode(location) || isContextTokenTypeLocation(contextToken)))) {
                         // Its a type, but you can reach it by namespace.type as well
-                        return symbolCanbeReferencedAtTypeLocation(symbol);
+                        return symbolCanBeReferencedAtTypeLocation(symbol);
                     }
                 }
 
                 // expressions are value space (which includes the value namespaces)
-                return !!(symbol.flags & SymbolFlags.Value);
+                return !!(getCombinedLocalAndExportSymbolFlags(symbol) & SymbolFlags.Value);
             });
         }
 
@@ -803,7 +803,9 @@ namespace ts.Completions {
             }
         }
 
-        function symbolCanbeReferencedAtTypeLocation(symbol: Symbol): boolean {
+        function symbolCanBeReferencedAtTypeLocation(symbol: Symbol): boolean {
+            symbol = symbol.exportSymbol || symbol;
+
             // This is an alias, follow what it aliases
             if (symbol && symbol.flags & SymbolFlags.Alias) {
                 symbol = typeChecker.getAliasedSymbol(symbol);
@@ -817,7 +819,7 @@ namespace ts.Completions {
                 const exportedSymbols = typeChecker.getExportsOfModule(symbol);
                 // If the exported symbols contains type,
                 // symbol can be referenced at locations where type is allowed
-                return forEach(exportedSymbols, symbolCanbeReferencedAtTypeLocation);
+                return forEach(exportedSymbols, symbolCanBeReferencedAtTypeLocation);
             }
         }
 
@@ -1588,21 +1590,23 @@ namespace ts.Completions {
     /**
      * Get the name to be display in completion from a given symbol.
      *
-     * @return undefined if the name is of external module otherwise a name with striped of any quote
+     * @return undefined if the name is of external module
      */
-    function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean): string {
-        const displayName = symbol.name;
-        if (!displayName) return undefined;
+    function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean): string | undefined {
+        const name = symbol.name;
+        if (!name) return undefined;
 
-        const firstCharCode = displayName.charCodeAt(0);
         // First check of the displayName is not external module; if it is an external module, it is not valid entry
-        if ((symbol.flags & SymbolFlags.Namespace) && (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) {
-            // If the symbol is external module, don't show it in the completion list
-            // (i.e declare module "http" { const x; } | // <= request completion here, "http" should not be there)
-            return undefined;
+        if (symbol.flags & SymbolFlags.Namespace) {
+            const firstCharCode = name.charCodeAt(0);
+            if (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote) {
+                // If the symbol is external module, don't show it in the completion list
+                // (i.e declare module "http" { const x; } | // <= request completion here, "http" should not be there)
+                return undefined;
+            }
         }
 
-        return getCompletionEntryDisplayName(displayName, target, performCharacterChecks);
+        return getCompletionEntryDisplayName(name, target, performCharacterChecks);
     }
 
     /**
@@ -1618,7 +1622,7 @@ namespace ts.Completions {
             return undefined;
         }
 
-        return name;
+        return unescapeIdentifier(name);
     }
 
     // A cache of completion entries for keywords, these do not change between sessions

src/services/symbolDisplay.ts

@@ -2,7 +2,7 @@
 namespace ts.SymbolDisplay {
     // TODO(drosen): use contextual SemanticMeaning.
     export function getSymbolKind(typeChecker: TypeChecker, symbol: Symbol, location: Node): ScriptElementKind {
-        const { flags } = symbol;
+        const flags = getCombinedLocalAndExportSymbolFlags(symbol);
 
         if (flags & SymbolFlags.Class) {
             return getDeclarationOfKind(symbol, SyntaxKind.ClassExpression) ?
@@ -34,7 +34,7 @@ namespace ts.SymbolDisplay {
         if (location.kind === SyntaxKind.ThisKeyword && isExpression(location)) {
             return ScriptElementKind.parameterElement;
         }
-        const { flags } = symbol;
+        const flags = getCombinedLocalAndExportSymbolFlags(symbol);
         if (flags & SymbolFlags.Variable) {
             if (isFirstDeclarationOfSymbolParameter(symbol)) {
                 return ScriptElementKind.parameterElement;
@@ -96,7 +96,7 @@ namespace ts.SymbolDisplay {
         const displayParts: SymbolDisplayPart[] = [];
         let documentation: SymbolDisplayPart[];
         let tags: JSDocTagInfo[];
-        const symbolFlags = symbol.flags;
+        const symbolFlags = ts.getCombinedLocalAndExportSymbolFlags(symbol);
         let symbolKind = getSymbolKindOfConstructorPropertyMethodAccessorFunctionOrVar(typeChecker, symbol, location);
         let hasAddedSymbolInfo: boolean;
         const isThisExpression = location.kind === SyntaxKind.ThisKeyword && isExpression(location);
@@ -110,7 +110,7 @@ namespace ts.SymbolDisplay {
             }
 
             let signature: Signature;
-            type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol, location);
+            type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol.exportSymbol || symbol, location);
             if (type) {
                 if (location.parent && location.parent.kind === SyntaxKind.PropertyAccessExpression) {
                     const right = (<PropertyAccessExpression>location.parent).name;
@@ -198,7 +198,7 @@ namespace ts.SymbolDisplay {
                         hasAddedSymbolInfo = true;
                     }
                 }
-                else if ((isNameOfFunctionDeclaration(location) && !(symbol.flags & SymbolFlags.Accessor)) || // name of function declaration
+                else if ((isNameOfFunctionDeclaration(location) && !(symbolFlags & SymbolFlags.Accessor)) || // name of function declaration
                     (location.kind === SyntaxKind.ConstructorKeyword && location.parent.kind === SyntaxKind.Constructor)) { // At constructor keyword of constructor declaration
                     // get the signature from the declaration and write it
                     const functionDeclaration = <FunctionLikeDeclaration>location.parent;
@@ -429,7 +429,7 @@ namespace ts.SymbolDisplay {
         if (!documentation) {
             documentation = symbol.getDocumentationComment();
             tags = symbol.getJsDocTags();
-            if (documentation.length === 0 && symbol.flags & SymbolFlags.Property) {
+            if (documentation.length === 0 && symbolFlags & SymbolFlags.Property) {
                 // For some special property access expressions like `exports.foo = foo` or `module.exports.foo = foo`
                 // there documentation comments might be attached to the right hand side symbol of their declarations.
                 // The pattern of such special property access is that the parent symbol is the symbol of the file.

tests/cases/fourslash/commentsExternalModules.ts

@@ -35,7 +35,7 @@ goTo.file("commentsExternalModules_file0.ts");
 verify.quickInfoAt("1", "namespace m1", "Namespace comment");
 
 goTo.marker('2');
-verify.completionListContains("b", "var m1.b: number", "b's comment");
+verify.completionListContains("b", "var b: number", "b's comment");
 verify.completionListContains("foo", "function foo(): number", "foo's comment");
 
 goTo.marker('3');

tests/cases/fourslash/commentsModules.ts

@@ -99,7 +99,7 @@
 verify.quickInfoAt("1", "namespace m1", "Namespace comment");
 
 goTo.marker('2');
-verify.completionListContains("b", "var m1.b: number", "b's comment");
+verify.completionListContains("b", "var b: number", "b's comment");
 verify.completionListContains("foo", "function foo(): number", "foo's comment");
 
 goTo.marker('3');

tests/cases/fourslash/completionListForUnicodeEscapeName.ts

@@ -22,5 +22,5 @@ verify.completionListContains("B");
 verify.completionListContains("\u0042");
 verify.completionListContains("A");
 verify.completionListContains("\u0041");
-verify.not.completionListContains("C");
-verify.not.completionListContains("\u0043");
+verify.completionListContains("C");
+verify.completionListContains("\u0043");

tests/cases/fourslash/completionListInScope_doesNotIncludeAugmentations.ts

@@ -0,0 +1,13 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: /a.ts
+////import * as self from "./a";
+////
+////declare module "a" {
+////    export const a: number;
+////}
+////
+/////**/
+
+goTo.marker();
+verify.not.completionListContains("a");

tests/cases/fourslash/completionListOnAliases2.ts

@@ -41,19 +41,16 @@ function getVerify(isTypeLocation?: boolean) {
         verifyValueOrType: verify
     };
 }
-function typeLocationVerify(valueMarker: string, verify: (typeMarker: string) => void) {
-    verify(valueMarker + "Type");
-    return valueMarker;
-}
 
 function verifyModuleM(marker: string) {
-    const isTypeLocation = marker.indexOf("Type") !== -1;
-    const { verifyValue, verifyType, verifyValueOrType } = getVerify(isTypeLocation);
-    if (!isTypeLocation) {
-        marker = typeLocationVerify(marker, verifyModuleM);
-    }
+    verifyModuleMWorker(marker, /*isTypeLocation*/ false);
+    verifyModuleMWorker(`${marker}Type`, /*isTypeLocation*/ true);
+}
+
+function verifyModuleMWorker(marker: string, isTypeLocation: boolean): void {
     goTo.marker(marker);
 
+    const { verifyValue, verifyType, verifyValueOrType } = getVerify(isTypeLocation);
     verifyType.completionListContains("I");
     verifyValueOrType.completionListContains("C");
     verifyValueOrType.completionListContains("E");
@@ -63,8 +60,9 @@ function verifyModuleM(marker: string) {
     verifyValueOrType.completionListContains("A");
 }
 
-
 // Module m
+goTo.marker("1");
+verify.completionListContains("A");
 verifyModuleM("1");
 
 // Class C