Don't bother with a predicate. It doesn't provide enough of a perf savings.

CyrusNajmabadi · CyrusNajmabadi · commit 63278ca037d7 · 2015-03-24T15:30:32.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -10556,7 +10556,7 @@ module ts {
             return false;
         }
 
-        function getSymbolsInScope(location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean): Symbol[] {
+        function getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[] {
             let symbols: SymbolTable = {};
             let memberFlags: NodeFlags = 0;
 
@@ -10572,85 +10572,59 @@ module ts {
             function populateSymbols() {
                 while (location) {
                     if (location.locals && !isGlobalSourceFile(location)) {
-                        if (copySymbols(location.locals, meaning)) {
-                            return;
-                        }
+                        copySymbols(location.locals, meaning);
                     }
+
                     switch (location.kind) {
                         case SyntaxKind.SourceFile:
                             if (!isExternalModule(<SourceFile>location)) {
                                 break;
                             }
                         case SyntaxKind.ModuleDeclaration:
-                            if (copySymbols(getSymbolOfNode(location).exports, meaning & SymbolFlags.ModuleMember)) {
-                                return;
-                            }
+                            copySymbols(getSymbolOfNode(location).exports, meaning & SymbolFlags.ModuleMember);
                             break;
                         case SyntaxKind.EnumDeclaration:
-                            if (copySymbols(getSymbolOfNode(location).exports, meaning & SymbolFlags.EnumMember)) {
-                                return;
-                            }
+                            copySymbols(getSymbolOfNode(location).exports, meaning & SymbolFlags.EnumMember);
                             break;
                         case SyntaxKind.ClassDeclaration:
                         case SyntaxKind.InterfaceDeclaration:
                             if (!(memberFlags & NodeFlags.Static)) {
-                                if (copySymbols(getSymbolOfNode(location).members, meaning & SymbolFlags.Type)) {
-                                    return;
-                                }
+                                copySymbols(getSymbolOfNode(location).members, meaning & SymbolFlags.Type);
                             }
                             break;
                         case SyntaxKind.FunctionExpression:
                             if ((<FunctionExpression>location).name) {
-                                if (copySymbol(location.symbol, meaning)) {
-                                    return;
-                                }
+                                copySymbol(location.symbol, meaning);
                             }
                             break;
                     }
+
                     memberFlags = location.flags;
                     location = location.parent;
                 }
-                if (copySymbols(globals, meaning)) {
-                    return;
-                }
+
+                copySymbols(globals, meaning);
             }
 
             // Returns 'true' if we should stop processing symbols.
-            function copySymbol(symbol: Symbol, meaning: SymbolFlags): boolean {
+            function copySymbol(symbol: Symbol, meaning: SymbolFlags): void {
                 if (symbol.flags & meaning) {
                     let id = symbol.name;
                     if (!isReservedMemberName(id) && !hasProperty(symbols, id)) {
-                        if (predicate) {
-                            // If we were supplied a predicate function, then check if this symbol 
-                            // matches with it.  If so, we're done and can immediately return.
-                            // Otherwise, just ignore this symbol and keep going.
-                            if (predicate(symbol)) {
-                                symbols[id] = symbol;
-                                return true;
-                            }
-                        }
-                        else {
-                            // If no predicate was supplied, then just add the symbol as is.
-                            symbols[id] = symbol;
-                        }
+                        // If no predicate was supplied, then just add the symbol as is.
+                        symbols[id] = symbol;
                     }
                 }
-
-                return false;
             }
 
-            function copySymbols(source: SymbolTable, meaning: SymbolFlags): boolean {
+            function copySymbols(source: SymbolTable, meaning: SymbolFlags): void {
                 if (meaning) {
                     for (let id in source) {
                         if (hasProperty(source, id)) {
-                            if (copySymbol(source[id], meaning)) {
-                                return true;
-                            }
+                            copySymbol(source[id], meaning);
                         }
                     }
                 }
-
-                return false;
             }
         }
 
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -1095,7 +1095,7 @@ module ts {
 
         // If 'predicate' is supplied, then only the first symbol in scope matching the predicate 
         // will be returned.  Otherwise, all symbols in scope will be returned.
-        getSymbolsInScope(location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean): Symbol[];
+        getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
         getSymbolAtLocation(node: Node): Symbol;
         getShorthandAssignmentValueSymbol(location: Node): Symbol;
         getTypeAtLocation(node: Node): Type;
diff --git a/src/services/services.ts b/src/services/services.ts
@@ -2398,42 +2398,51 @@ module ts {
         }
 
         /// Completion
-        function getValidCompletionEntryDisplayName(symbol: Symbol, target: ScriptTarget): string {
+        function getCompletionEntryDisplayName(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean): string {
             let displayName = symbol.getName();
-            if (displayName && displayName.length > 0) {
-                let 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" { let x; } | // <= request completion here, "http" should not be there)
-                    return undefined;
-                }
+            if (!displayName) {
+                return undefined;
+            }
 
-                if (displayName && displayName.length >= 2 && firstCharCode === displayName.charCodeAt(displayName.length - 1) &&
-                    (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) {
-                    // If the user entered name for the symbol was quoted, removing the quotes is not enough, as the name could be an
-                    // invalid identifier name. We need to check if whatever was inside the quotes is actually a valid identifier name.
-                    displayName = displayName.substring(1, displayName.length - 1);
-                }
+            let 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" { let x; } | // <= request completion here, "http" should not be there)
+                return undefined;
+            }
 
-                let isValid = isIdentifierStart(displayName.charCodeAt(0), target);
-                for (let i = 1, n = displayName.length; isValid && i < n; i++) {
-                    isValid = isIdentifierPart(displayName.charCodeAt(i), target);
+            if (displayName && displayName.length >= 2 && firstCharCode === displayName.charCodeAt(displayName.length - 1) &&
+                (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) {
+                // If the user entered name for the symbol was quoted, removing the quotes is not enough, as the name could be an
+                // invalid identifier name. We need to check if whatever was inside the quotes is actually a valid identifier name.
+                displayName = displayName.substring(1, displayName.length - 1);
+            }
+
+            if (!displayName) {
+                return undefined;
+            }
+
+            if (performCharacterChecks) {
+                if (!isIdentifierStart(displayName.charCodeAt(0), target)) {
+                    return undefined;
                 }
 
-                if (isValid) {
-                    return unescapeIdentifier(displayName);
+                for (let i = 1, n = displayName.length; i < n; i++) {
+                    if (!isIdentifierPart(displayName.charCodeAt(i), target)) {
+                        return undefined;
+                    }
                 }
             }
 
-            return undefined;
+            return unescapeIdentifier(displayName);
         }
 
         function createCompletionEntry(symbol: Symbol, typeChecker: TypeChecker, location: Node): CompletionEntry {
             // Try to get a valid display name for this symbol, if we could not find one, then ignore it. 
             // We would like to only show things that can be added after a dot, so for instance numeric properties can
             // not be accessed with a dot (a.1 <- invalid)
-            let displayName = getValidCompletionEntryDisplayName(symbol, program.getCompilerOptions().target);
+            let displayName = getCompletionEntryDisplayName(symbol, program.getCompilerOptions().target, /*performCharacterChecks:*/ true);
             if (!displayName) {
                 return undefined;
             }
@@ -2449,26 +2458,7 @@ module ts {
             };
         }
 
-        // If symbolName is undefined, all symbols at the specified are returned.  If symbolName 
-        // is not undefined, then the first symbol with that name at the specified position
-        // will be returned.  Calling without symbolName is useful when you want the entire
-        // list of symbols (like for getCompletionsAtPosition).  Calling with a symbolName is
-        // useful when you want information about a single symbol (like for getCompletionEntryDetails).
-        function getCompletionData(fileName: string, position: number, symbolName?: string) {
-            let result = getCompletionDataWorker(fileName, position, symbolName);
-            if (!result) {
-                return undefined;
-            }
-
-            if (result.symbols && symbolName) {
-                var target = program.getCompilerOptions().target;
-                result.symbols = filter(result.symbols, s => getValidCompletionEntryDisplayName(s, target) === symbolName);
-            }
-
-            return result;
-        }
-
-        function getCompletionDataWorker(fileName: string, position: number, symbolName: string) {
+        function getCompletionData(fileName: string, position: number) {
             let syntacticStart = new Date().getTime();
             let sourceFile = getValidSourceFile(fileName);
 
@@ -2600,11 +2590,7 @@ module ts {
                     let scopeNode = getScopeNode(previousToken, position, sourceFile);
                     let symbolMeanings = SymbolFlags.Type | SymbolFlags.Value | SymbolFlags.Namespace | SymbolFlags.Alias;
                     
-                    // Filter down to the symbol that matches the symbolName if we were given one.
-                    let predicate = symbolName !== undefined
-                        ? (s: Symbol) => getValidCompletionEntryDisplayName(s, target) === symbolName
-                        : undefined;
-                    symbols = typeInfoResolver.getSymbolsInScope(scopeNode, symbolMeanings, predicate);
+                    symbols = typeInfoResolver.getSymbolsInScope(scopeNode, symbolMeanings);
                 }
             }
 
@@ -2955,12 +2941,16 @@ module ts {
         function getCompletionEntryDetails(fileName: string, position: number, entryName: string): CompletionEntryDetails {
             synchronizeHostData();
 
-            // Look up a completion symbol with this name.
-            let completionData = getCompletionData(fileName, position, entryName);
+            // Compute all the completion symbols again.
+            let completionData = getCompletionData(fileName, position);
             if (completionData) {
                 let { symbols, location } = completionData;
-                if (symbols && symbols.length > 0) {
-                    let symbol = symbols[0];
+
+                // Find the symbol with the matching entry name.
+                let target = program.getCompilerOptions().target;
+                let symbol = forEach(symbols, s => getCompletionEntryDisplayName(s, target, /*performCharacterChecks:*/ false) === entryName ? s : undefined);
+
+                if (symbol) {
                     let displayPartsDocumentationsAndSymbolKind = getSymbolDisplayPartsDocumentationAndSymbolKind(symbol, getValidSourceFile(fileName), location, typeInfoResolver, location, SemanticMeaning.All);
                     return {
                         name: entryName,
diff --git a/tests/baselines/reference/APISample_compile.js b/tests/baselines/reference/APISample_compile.js
@@ -860,7 +860,7 @@ declare module "typescript" {
         getSignaturesOfType(type: Type, kind: SignatureKind): Signature[];
         getIndexTypeOfType(type: Type, kind: IndexKind): Type;
         getReturnTypeOfSignature(signature: Signature): Type;
-        getSymbolsInScope(location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean): Symbol[];
+        getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
         getSymbolAtLocation(node: Node): Symbol;
         getShorthandAssignmentValueSymbol(location: Node): Symbol;
         getTypeAtLocation(node: Node): Type;
diff --git a/tests/baselines/reference/APISample_compile.types b/tests/baselines/reference/APISample_compile.types
@@ -2632,15 +2632,12 @@ declare module "typescript" {
 >Signature : Signature
 >Type : Type
 
-        getSymbolsInScope(location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean): Symbol[];
->getSymbolsInScope : (location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean) => Symbol[]
+        getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
+>getSymbolsInScope : (location: Node, meaning: SymbolFlags) => Symbol[]
 >location : Node
 >Node : Node
 >meaning : SymbolFlags
 >SymbolFlags : SymbolFlags
->predicate : (symbol: Symbol) => boolean
->symbol : Symbol
->Symbol : Symbol
 >Symbol : Symbol
 
         getSymbolAtLocation(node: Node): Symbol;
diff --git a/tests/baselines/reference/APISample_linter.js b/tests/baselines/reference/APISample_linter.js
@@ -891,7 +891,7 @@ declare module "typescript" {
         getSignaturesOfType(type: Type, kind: SignatureKind): Signature[];
         getIndexTypeOfType(type: Type, kind: IndexKind): Type;
         getReturnTypeOfSignature(signature: Signature): Type;
-        getSymbolsInScope(location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean): Symbol[];
+        getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
         getSymbolAtLocation(node: Node): Symbol;
         getShorthandAssignmentValueSymbol(location: Node): Symbol;
         getTypeAtLocation(node: Node): Type;
diff --git a/tests/baselines/reference/APISample_linter.types b/tests/baselines/reference/APISample_linter.types
@@ -2778,15 +2778,12 @@ declare module "typescript" {
 >Signature : Signature
 >Type : Type
 
-        getSymbolsInScope(location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean): Symbol[];
->getSymbolsInScope : (location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean) => Symbol[]
+        getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
+>getSymbolsInScope : (location: Node, meaning: SymbolFlags) => Symbol[]
 >location : Node
 >Node : Node
 >meaning : SymbolFlags
 >SymbolFlags : SymbolFlags
->predicate : (symbol: Symbol) => boolean
->symbol : Symbol
->Symbol : Symbol
 >Symbol : Symbol
 
         getSymbolAtLocation(node: Node): Symbol;
diff --git a/tests/baselines/reference/APISample_transform.js b/tests/baselines/reference/APISample_transform.js
@@ -892,7 +892,7 @@ declare module "typescript" {
         getSignaturesOfType(type: Type, kind: SignatureKind): Signature[];
         getIndexTypeOfType(type: Type, kind: IndexKind): Type;
         getReturnTypeOfSignature(signature: Signature): Type;
-        getSymbolsInScope(location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean): Symbol[];
+        getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
         getSymbolAtLocation(node: Node): Symbol;
         getShorthandAssignmentValueSymbol(location: Node): Symbol;
         getTypeAtLocation(node: Node): Type;
diff --git a/tests/baselines/reference/APISample_transform.types b/tests/baselines/reference/APISample_transform.types
@@ -2728,15 +2728,12 @@ declare module "typescript" {
 >Signature : Signature
 >Type : Type
 
-        getSymbolsInScope(location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean): Symbol[];
->getSymbolsInScope : (location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean) => Symbol[]
+        getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
+>getSymbolsInScope : (location: Node, meaning: SymbolFlags) => Symbol[]
 >location : Node
 >Node : Node
 >meaning : SymbolFlags
 >SymbolFlags : SymbolFlags
->predicate : (symbol: Symbol) => boolean
->symbol : Symbol
->Symbol : Symbol
 >Symbol : Symbol
 
         getSymbolAtLocation(node: Node): Symbol;
diff --git a/tests/baselines/reference/APISample_watcher.js b/tests/baselines/reference/APISample_watcher.js
@@ -929,7 +929,7 @@ declare module "typescript" {
         getSignaturesOfType(type: Type, kind: SignatureKind): Signature[];
         getIndexTypeOfType(type: Type, kind: IndexKind): Type;
         getReturnTypeOfSignature(signature: Signature): Type;
-        getSymbolsInScope(location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean): Symbol[];
+        getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
         getSymbolAtLocation(node: Node): Symbol;
         getShorthandAssignmentValueSymbol(location: Node): Symbol;
         getTypeAtLocation(node: Node): Type;
diff --git a/tests/baselines/reference/APISample_watcher.types b/tests/baselines/reference/APISample_watcher.types
@@ -2901,15 +2901,12 @@ declare module "typescript" {
 >Signature : Signature
 >Type : Type
 
-        getSymbolsInScope(location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean): Symbol[];
->getSymbolsInScope : (location: Node, meaning: SymbolFlags, predicate?: (symbol: Symbol) => boolean) => Symbol[]
+        getSymbolsInScope(location: Node, meaning: SymbolFlags): Symbol[];
+>getSymbolsInScope : (location: Node, meaning: SymbolFlags) => Symbol[]
 >location : Node
 >Node : Node
 >meaning : SymbolFlags
 >SymbolFlags : SymbolFlags
->predicate : (symbol: Symbol) => boolean
->symbol : Symbol
->Symbol : Symbol
 >Symbol : Symbol
 
         getSymbolAtLocation(node: Node): Symbol;
