Cleaning up the completion code and tests

Author: riknoll

src/harness/fourslash.ts

@@ -598,38 +598,6 @@ namespace FourSlash {
             }
         }
 
-        public verifyImportModuleCompletionListItemsCountIsGreaterThan(count: number, negative: boolean) {
-            const completions = this.getImportModuleCompletionListAtCaret();
-            const itemsCount = completions.entries.length;
-
-            if (negative) {
-                if (itemsCount > count) {
-                    this.raiseError(`Expected import module completion list items count to not be greater than ${count}, but is actually ${itemsCount}`);
-                }
-            }
-            else {
-                if (itemsCount <= count) {
-                    this.raiseError(`Expected import module completion list items count to be greater than ${count}, but is actually ${itemsCount}`);
-                }
-            }
-        }
-
-        public verifyImportModuleCompletionListIsEmpty(negative: boolean) {
-            const completions = this.getImportModuleCompletionListAtCaret();
-            if ((!completions || completions.entries.length === 0) && negative) {
-                this.raiseError("Completion list is empty at caret at position " + this.activeFile.fileName + " " + this.currentCaretPosition);
-            }
-            else if (completions && completions.entries.length !== 0 && !negative) {
-                let errorMsg = "\n" + "Completion List contains: [" + completions.entries[0].name;
-                for (let i = 1; i < completions.entries.length; i++) {
-                    errorMsg += ", " + completions.entries[i].name;
-                }
-                errorMsg += "]\n";
-
-                this.raiseError("Completion list is not empty at caret at position " + this.activeFile.fileName + " " + this.currentCaretPosition + errorMsg);
-            }
-        }
-
         public verifyCompletionListStartsWithItemsInOrder(items: string[]): void {
             if (items.length === 0) {
                 return;
@@ -701,10 +669,10 @@ namespace FourSlash {
             }
         }
 
-        public verifyCompletionListContains(symbol: string, text?: string, documentation?: string, kind?: string) {
+        public verifyCompletionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) {
             const completions = this.getCompletionListAtCaret();
             if (completions) {
-                this.assertItemInCompletionList(completions.entries, symbol, text, documentation, kind);
+                this.assertItemInCompletionList(completions.entries, symbol, text, documentation, kind, spanIndex);
             }
             else {
                 this.raiseError(`No completions at position '${this.currentCaretPosition}' when looking for '${symbol}'.`);
@@ -720,25 +688,32 @@ namespace FourSlash {
          * @param expectedText the text associated with the symbol
          * @param expectedDocumentation the documentation text associated with the symbol
          * @param expectedKind the kind of symbol (see ScriptElementKind)
+         * @param spanIndex the index of the range that the completion item's replacement text span should match
          */
-        public verifyCompletionListDoesNotContain(symbol: string, expectedText?: string, expectedDocumentation?: string, expectedKind?: string) {
+        public verifyCompletionListDoesNotContain(symbol: string, expectedText?: string, expectedDocumentation?: string, expectedKind?: string, spanIndex?: number) {
             const that = this;
+            let replacementSpan: ts.TextSpan;
+            if (spanIndex !== undefined) {
+                replacementSpan = this.getTextSpanForRangeAtIndex(spanIndex);
+            }
+
             function filterByTextOrDocumentation(entry: ts.CompletionEntry) {
                 const details = that.getCompletionEntryDetails(entry.name);
                 const documentation = ts.displayPartsToString(details.documentation);
                 const text = ts.displayPartsToString(details.displayParts);
-                if (expectedText && expectedDocumentation) {
-                    return (documentation === expectedDocumentation && text === expectedText) ? true : false;
+
+                // If any of the expected values are undefined, assume that users don't
+                // care about them.
+                if (replacementSpan && !TestState.textSpansEqual(replacementSpan, entry.replacementSpan)) {
+                    return false;
                 }
-                else if (expectedText && !expectedDocumentation) {
-                    return text === expectedText ? true : false;
+                else if (expectedText && text !== expectedText) {
+                    return false;
                 }
-                else if (expectedDocumentation && !expectedText) {
-                    return documentation === expectedDocumentation ? true : false;
+                else if (expectedDocumentation && documentation !== expectedDocumentation) {
+                    return false;
                 }
-                // Because expectedText and expectedDocumentation are undefined, we assume that
-                // users don"t care to compare them so we will treat that entry as if the entry has matching text and documentation
-                // and keep it in the list of filtered entry.
+
                 return true;
             }
 
@@ -762,45 +737,11 @@ namespace FourSlash {
                     if (expectedKind) {
                         error += "Expected kind: " + expectedKind + " to equal: " + filterCompletions[0].kind + ".";
                     }
-                    this.raiseError(error);
-                }
-            }
-        }
-
-        public verifyImportModuleCompletionListContains(symbol: string, rangeIndex?: number) {
-            const completions = this.getImportModuleCompletionListAtCaret();
-            if (completions) {
-                const completion = ts.forEach(completions.entries, completion => completion.name === symbol ? completion : undefined);
-                if (!completion) {
-                    const itemsString = completions.entries.map(item => item.name).join(",\n");
-                    this.raiseError(`Expected "${symbol}" to be in list [${itemsString}]`);
-                }
-                else if (rangeIndex !== undefined) {
-                    const ranges = this.getRanges();
-                    if (ranges && ranges.length > rangeIndex) {
-                        const range = ranges[rangeIndex];
-
-                        const start = completion.replacementSpan.start;
-                        const end = start + completion.replacementSpan.length;
-                        if (range.start !== start || range.end !== end) {
-                            this.raiseError(`Expected completion span for '${symbol}', ${stringify(completion.replacementSpan)}, to cover range ${stringify(range)}`);
-                        }
+                    if (replacementSpan) {
+                        const spanText = filterCompletions[0].replacementSpan ? stringify(filterCompletions[0].replacementSpan) : undefined;
+                        error += "Expected replacement span: " + stringify(replacementSpan) + " to equal: " + spanText + ".";
                     }
-                    else {
-                        this.raiseError(`Expected completion span for '${symbol}' to cover range at index ${rangeIndex}, but no range was found at that index`);
-                    }
-                }
-            }
-            else {
-                this.raiseError(`No import module completions at position '${this.currentCaretPosition}' when looking for '${symbol}'.`);
-            }
-        }
-
-        public verifyImportModuleCompletionListDoesNotContain(symbol: string) {
-            const completions = this.getImportModuleCompletionListAtCaret();
-            if (completions) {
-                if (ts.forEach(completions.entries, completion => completion.name === symbol)) {
-                    this.raiseError(`Import module completion list did contain ${symbol}`);
+                    this.raiseError(error);
                 }
             }
         }
@@ -891,10 +832,6 @@ namespace FourSlash {
             return this.languageService.getCompletionsAtPosition(this.activeFile.fileName, this.currentCaretPosition);
         }
 
-        private getImportModuleCompletionListAtCaret() {
-            return this.languageService.getCompletionsAtPosition(this.activeFile.fileName, this.currentCaretPosition);
-        }
-
         private getCompletionEntryDetails(entryName: string) {
             return this.languageService.getCompletionEntryDetails(this.activeFile.fileName, this.currentCaretPosition, entryName);
         }
@@ -2241,7 +2178,7 @@ namespace FourSlash {
             return text.substring(startPos, endPos);
         }
 
-        private assertItemInCompletionList(items: ts.CompletionEntry[], name: string, text?: string, documentation?: string, kind?: string) {
+        private assertItemInCompletionList(items: ts.CompletionEntry[], name: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) {
             for (let i = 0; i < items.length; i++) {
                 const item = items[i];
                 if (item.name === name) {
@@ -2260,6 +2197,11 @@ namespace FourSlash {
                         assert.equal(item.kind, kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + name));
                     }
 
+                    if (spanIndex !== undefined) {
+                        const span = this.getTextSpanForRangeAtIndex(spanIndex);
+                        assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + name));
+                    }
+
                     return;
                 }
             }
@@ -2316,6 +2258,17 @@ namespace FourSlash {
             return `line ${(pos.line + 1)}, col ${pos.character}`;
         }
 
+        private getTextSpanForRangeAtIndex(index: number): ts.TextSpan {
+            const ranges = this.getRanges();
+            if (ranges && ranges.length > index) {
+                const range = ranges[index];
+                return { start: range.start, length: range.end - range.start };
+            }
+            else {
+                this.raiseError("Supplied span index: " + index + " does not exist in range list of size: " + (ranges ? 0 : ranges.length));
+            }
+        }
+
         public getMarkerByName(markerName: string) {
             const markerPos = this.testData.markerPositions[markerName];
             if (markerPos === undefined) {
@@ -2339,6 +2292,10 @@ namespace FourSlash {
         public resetCancelled(): void {
             this.cancellationToken.resetCancelled();
         }
+
+        private static textSpansEqual(a: ts.TextSpan, b: ts.TextSpan) {
+            return a && b && a.start === b.start && a.length === b.length;
+        }
     }
 
     export function runFourSlashTest(basePath: string, testType: FourSlashTestType, fileName: string) {
@@ -2909,12 +2866,12 @@ namespace FourSlashInterface {
 
         // Verifies the completion list contains the specified symbol. The
         // completion list is brought up if necessary
-        public completionListContains(symbol: string, text?: string, documentation?: string, kind?: string) {
+        public completionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) {
             if (this.negative) {
-                this.state.verifyCompletionListDoesNotContain(symbol, text, documentation, kind);
+                this.state.verifyCompletionListDoesNotContain(symbol, text, documentation, kind, spanIndex);
             }
             else {
-                this.state.verifyCompletionListContains(symbol, text, documentation, kind);
+                this.state.verifyCompletionListContains(symbol, text, documentation, kind, spanIndex);
             }
         }
 
@@ -2924,23 +2881,6 @@ namespace FourSlashInterface {
             this.state.verifyCompletionListItemsCountIsGreaterThan(count, this.negative);
         }
 
-        public importModuleCompletionListContains(symbol: string, rangeIndex?: number): void {
-            if (this.negative) {
-                this.state.verifyImportModuleCompletionListDoesNotContain(symbol);
-            }
-            else {
-                this.state.verifyImportModuleCompletionListContains(symbol, rangeIndex);
-            }
-        }
-
-        public importModuleCompletionListItemsCountIsGreaterThan(count: number): void {
-            this.state.verifyImportModuleCompletionListItemsCountIsGreaterThan(count, this.negative);
-        }
-
-        public importModuleCompletionListIsEmpty(): void {
-            this.state.verifyImportModuleCompletionListIsEmpty(this.negative);
-        }
-
         public assertHasRanges(ranges: FourSlash.Range[]) {
             assert(ranges.length !== 0, "Array of ranges is expected to be non-empty");
         }

src/services/services.ts

@@ -4236,9 +4236,8 @@ namespace ts {
 
             const sourceFile = getValidSourceFile(fileName);
 
-            const importCompletionInfo = getImportModuleCompletionsAtPosition(fileName, position);
-            if (importCompletionInfo && importCompletionInfo.entries.length !== 0) {
-                return importCompletionInfo;
+            if (isInReferenceComment(sourceFile, position)) {
+                return getTripleSlashReferenceCompletion(sourceFile, position);
             }
 
             if (isInString(sourceFile, position)) {
@@ -4410,6 +4409,13 @@ namespace ts {
                     // a['/*completion position*/']
                     return getStringLiteralCompletionEntriesFromElementAccess(node.parent);
                 }
+                else if (node.parent.kind === SyntaxKind.ImportDeclaration || isExpressionOfExternalModuleImportEqualsDeclaration(node) || isRequireCall(node.parent, false)) {
+                    // Get all known external module names or complete a path to a module
+                    // i.e. import * as ns from "/*completion position*/";
+                    //      import x = require("/*completion position*/");
+                    //      var y = require("/*completion position*/");
+                    return getStringLiteralCompletionEntriesFromModuleNames(<StringLiteral>node);
+                }
                 else {
                     const argumentInfo = SignatureHelp.getContainingArgumentInfo(node, position, sourceFile);
                     if (argumentInfo) {
@@ -4502,55 +4508,35 @@ namespace ts {
                     }
                 }
             }
-        }
-
-        function getImportModuleCompletionsAtPosition(fileName: string, position: number): CompletionInfo {
-            synchronizeHostData();
-
-            const sourceFile = getValidSourceFile(fileName);
-            if (isInReferenceComment(sourceFile, position)) {
-                return getTripleSlashReferenceCompletion(sourceFile, position);
-            }
-            else if (isInString(sourceFile, position)) {
-                const node = findPrecedingToken(position, sourceFile);
-                if (!node || node.kind !== SyntaxKind.StringLiteral) {
-                    return undefined;
-                }
-
-                if (node.parent.kind === SyntaxKind.ImportDeclaration || isExpressionOfExternalModuleImportEqualsDeclaration(node) || isRequireCall(node.parent, false)) {
-                    // Get all known external module names or complete a path to a module
-                    return {
-                        isMemberCompletion: false,
-                        isNewIdentifierLocation: true,
-                        entries: getStringLiteralCompletionEntriesFromModuleNames(<StringLiteral>node)
-                    };
-                }
-            }
 
-            return undefined;
-
-            function getStringLiteralCompletionEntriesFromModuleNames(node: StringLiteral) {
+            function getStringLiteralCompletionEntriesFromModuleNames(node: StringLiteral): CompletionInfo {
                 const literalValue = normalizeSlashes(node.text);
 
                 const scriptPath = node.getSourceFile().path;
                 const scriptDirectory = getDirectoryPath(scriptPath);
 
                 const span = getDirectoryFragmentTextSpan((<StringLiteral>node).text, node.getStart() + 1);
+                let entries: CompletionEntry[];
                 if (isPathRelativeToScript(literalValue) || isRootedDiskPath(literalValue)) {
                     const compilerOptions = program.getCompilerOptions();
                     if (compilerOptions.rootDirs) {
-                        return getCompletionEntriesForDirectoryFragmentWithRootDirs(
+                        entries = getCompletionEntriesForDirectoryFragmentWithRootDirs(
                             compilerOptions.rootDirs, literalValue, scriptDirectory, getSupportedExtensions(program.getCompilerOptions()), /*includeExtensions*/false, span, scriptPath);
                     }
                     else {
-                        return getCompletionEntriesForDirectoryFragment(
+                        entries = getCompletionEntriesForDirectoryFragment(
                             literalValue, scriptDirectory, getSupportedExtensions(program.getCompilerOptions()), /*includeExtensions*/false, span, scriptPath);
                     }
                 }
                 else {
                     // Check for node modules
-                    return getCompletionEntriesForNonRelativeModules(literalValue, scriptDirectory, span);
+                    entries = getCompletionEntriesForNonRelativeModules(literalValue, scriptDirectory, span);
                 }
+                return {
+                    isMemberCompletion: false,
+                    isNewIdentifierLocation: true,
+                    entries
+                };
             }
 
             /**

tests/cases/fourslash/completionForStringLiteralImport1.ts

@@ -21,13 +21,13 @@
 //// export var x = 9;
 
 goTo.marker("0");
-verify.importModuleCompletionListContains("someFile1", 0);
+verify.completionListContains("someFile1", undefined, undefined, undefined, 0);
 
 goTo.marker("1");
-verify.importModuleCompletionListContains("someFile2", 1);
+verify.completionListContains("someFile2", undefined, undefined, undefined, 1);
 
 goTo.marker("2");
-verify.importModuleCompletionListContains("some-module", 2);
+verify.completionListContains("some-module", undefined, undefined, undefined, 2);
 
 goTo.marker("3");
-verify.importModuleCompletionListContains("fourslash", 3);
+verify.completionListContains("fourslash", undefined, undefined, undefined, 3);

tests/cases/fourslash/completionForStringLiteralImport2.ts

@@ -21,13 +21,13 @@
 //// export var x = 9;
 
 goTo.marker("0");
-verify.importModuleCompletionListContains("someFile.ts", 0);
+verify.completionListContains("someFile.ts", undefined, undefined, undefined, 0);
 
 goTo.marker("1");
-verify.importModuleCompletionListContains("some-module", 1);
+verify.completionListContains("some-module", undefined, undefined, undefined, 1);
 
 goTo.marker("2");
-verify.importModuleCompletionListContains("someOtherFile.ts", 2);
+verify.completionListContains("someOtherFile.ts", undefined, undefined, undefined, 2);
 
 goTo.marker("3");
-verify.importModuleCompletionListContains("some-module", 3);
+verify.completionListContains("some-module", undefined, undefined, undefined, 3);