Address code review

Author: Yui T

src/services/services.ts

@@ -1681,9 +1681,17 @@ module ts {
         }
 
         /// Completion
-        function getValidCompletionEntryDisplayName(displayName: string, target: ScriptTarget): string {
+        function getValidCompletionEntryDisplayName(symbol: Symbol, target: ScriptTarget): string {
+            var displayName = symbol.getName();
             if (displayName && displayName.length > 0) {
                 var 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" { var x; } | // <= request completion here, "http" should not be there)
+                    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
@@ -1696,6 +1704,7 @@ module ts {
                     isValid = isIdentifierPart(displayName.charCodeAt(i), target);
                 }
 
+
                 if (isValid) {
                     return displayName;
                 }
@@ -1708,14 +1717,7 @@ module ts {
             // 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)
-            var firstCharCode = symbol.name.charCodeAt(0);
-            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" { var x; } | // <= request completion here, "http" should not be there)
-                return undefined;
-            }
-
-            var displayName = getValidCompletionEntryDisplayName(symbol.getName(), program.getCompilerOptions().target);
+            var displayName = getValidCompletionEntryDisplayName(symbol, program.getCompilerOptions().target);
             if (!displayName) {
                 return undefined;
             }

tests/cases/fourslash/completionListWithAmbientDeclaration.ts

@@ -6,4 +6,4 @@
 //// /**/
 
 goTo.marker();
-verifyNegatable.completionListContains("http");
+verify.not.completionListContains("http");

tests/cases/fourslash/fourslash.ts

@@ -630,7 +630,6 @@ function verifyOperationIsCancelled(f) {
 var test = new FourSlashInterface.test_();
 var goTo = new FourSlashInterface.goTo();
 var verify = new FourSlashInterface.verify();
-var verifyNegatable = new FourSlashInterface.verifyNegatable(true);
 var edit = new FourSlashInterface.edit();
 var debug = new FourSlashInterface.debug();
 var format = new FourSlashInterface.format();