When testing references, also test documentHighlights respects filesToSearch (#23306)

Andy · web-flow · commit f6b206a75a4b · 2018-04-11T14:07:22.000-07:00
* When testing references, also test documentHighlights respects filesToSearch

* Fix handling for redirects and move assertion inside getDocumentHighlights

* Add another assert
diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts
@@ -1081,8 +1081,20 @@ namespace FourSlash {
             }
         }
 
+        private verifyDocumentHighlightsRespectFilesList(files: ReadonlyArray<string>): void {
+            const startFile = this.activeFile.fileName;
+            for (const fileName of files) {
+                const searchFileNames = startFile === fileName ? [startFile] : [startFile, fileName];
+                const highlights = this.getDocumentHighlightsAtCurrentPosition(searchFileNames);
+                if (!highlights.every(dh => ts.contains(searchFileNames, dh.fileName))) {
+                    this.raiseError(`When asking for document highlights only in files ${searchFileNames}, got document highlights in ${unique(highlights, dh => dh.fileName)}`);
+                }
+            }
+        }
+
         public verifyReferencesOf(range: Range, references: Range[]) {
             this.goToRangeStart(range);
+            this.verifyDocumentHighlightsRespectFilesList(unique(references, e => e.fileName));
             this.verifyReferencesAre(references);
         }
 
@@ -1094,7 +1106,7 @@ namespace FourSlash {
             }
         }
 
-        public verifyReferenceGroups(starts: string | string[] | Range | Range[], parts: FourSlashInterface.ReferenceGroup[]): void {
+        public verifyReferenceGroups(starts: string | string[] | Range | Range[], parts: FourSlashInterface.ReferenceGroup[] | undefined): void {
             interface ReferenceGroupJson {
                 definition: string | { text: string, range: ts.TextSpan };
                 references: ts.ReferenceEntry[];
@@ -1128,6 +1140,10 @@ namespace FourSlash {
                     };
                 });
                 this.assertObjectsEqual(fullActual, fullExpected);
+
+                if (parts) {
+                    this.verifyDocumentHighlightsRespectFilesList(unique(ts.flatMap(parts, p => p.ranges), r => r.fileName));
+                }
             }
         }
 
diff --git a/src/services/documentHighlights.ts b/src/services/documentHighlights.ts
@@ -22,10 +22,20 @@ namespace ts.DocumentHighlights {
     }
 
     function getSemanticDocumentHighlights(position: number, node: Node, program: Program, cancellationToken: CancellationToken, sourceFilesToSearch: ReadonlyArray<SourceFile>): DocumentHighlights[] | undefined {
-        const referenceEntries = FindAllReferences.getReferenceEntriesForNode(position, node, program, sourceFilesToSearch, cancellationToken);
+        const sourceFilesSet = arrayToSet(sourceFilesToSearch, f => f.fileName);
+        const referenceEntries = FindAllReferences.getReferenceEntriesForNode(position, node, program, sourceFilesToSearch, cancellationToken, /*options*/ undefined, sourceFilesSet);
         if (!referenceEntries) return undefined;
         const map = arrayToMultiMap(referenceEntries.map(FindAllReferences.toHighlightSpan), e => e.fileName, e => e.span);
-        return arrayFrom(map.entries(), ([fileName, highlightSpans]) => ({ fileName, highlightSpans }));
+        return arrayFrom(map.entries(), ([fileName, highlightSpans]) => {
+            if (!sourceFilesSet.has(fileName)) {
+                Debug.assert(program.redirectTargetsSet.has(fileName));
+                const redirectTarget = program.getSourceFile(fileName);
+                const redirect = find(sourceFilesToSearch, f => f.redirectInfo && f.redirectInfo.redirectTarget === redirectTarget)!;
+                fileName = redirect.fileName;
+                Debug.assert(sourceFilesSet.has(fileName));
+            }
+            return { fileName, highlightSpans };
+        });
     }
 
     function getSyntacticDocumentHighlights(node: Node, sourceFile: SourceFile): DocumentHighlights[] {
diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts
@@ -43,7 +43,7 @@ namespace ts.FindAllReferences {
 
     export function findReferencedSymbols(program: Program, cancellationToken: CancellationToken, sourceFiles: ReadonlyArray<SourceFile>, sourceFile: SourceFile, position: number): ReferencedSymbol[] | undefined {
         const node = getTouchingPropertyName(sourceFile, position, /*includeJsDocComment*/ true);
-        const referencedSymbols = Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken, /*options*/ {});
+        const referencedSymbols = Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken);
         const checker = program.getTypeChecker();
         return !referencedSymbols || !referencedSymbols.length ? undefined : mapDefined<SymbolAndEntries, ReferencedSymbol>(referencedSymbols, ({ definition, references }) =>
             // Only include referenced symbols that have a valid definition.
@@ -88,8 +88,8 @@ namespace ts.FindAllReferences {
         return map(flattenEntries(Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken, options)), toReferenceEntry);
     }
 
-    export function getReferenceEntriesForNode(position: number, node: Node, program: Program, sourceFiles: ReadonlyArray<SourceFile>, cancellationToken: CancellationToken, options: Options = {}): Entry[] | undefined {
-        return flattenEntries(Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken, options));
+    export function getReferenceEntriesForNode(position: number, node: Node, program: Program, sourceFiles: ReadonlyArray<SourceFile>, cancellationToken: CancellationToken, options: Options = {}, sourceFilesSet: ReadonlyMap<true> = arrayToSet(sourceFiles, f => f.fileName)): Entry[] | undefined {
+        return flattenEntries(Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken, options, sourceFilesSet));
     }
 
     function flattenEntries(referenceSymbols: SymbolAndEntries[]): Entry[] {
@@ -231,10 +231,10 @@ namespace ts.FindAllReferences {
 /* @internal */
 namespace ts.FindAllReferences.Core {
     /** Core find-all-references algorithm. Handles special cases before delegating to `getReferencedSymbolsForSymbol`. */
-    export function getReferencedSymbolsForNode(position: number, node: Node, program: Program, sourceFiles: ReadonlyArray<SourceFile>, cancellationToken: CancellationToken, options: Options = {}): SymbolAndEntries[] | undefined {
+    export function getReferencedSymbolsForNode(position: number, node: Node, program: Program, sourceFiles: ReadonlyArray<SourceFile>, cancellationToken: CancellationToken, options: Options = {}, sourceFilesSet: ReadonlyMap<true> = arrayToSet(sourceFiles, f => f.fileName)): SymbolAndEntries[] | undefined {
         if (isSourceFile(node)) {
             const reference = GoToDefinition.getReferenceAtPosition(node, position, program);
-            return reference && getReferencedSymbolsForModule(program, program.getTypeChecker().getMergedSymbol(reference.file.symbol), sourceFiles);
+            return reference && getReferencedSymbolsForModule(program, program.getTypeChecker().getMergedSymbol(reference.file.symbol), sourceFiles, sourceFilesSet);
         }
 
         if (!options.implementations) {
@@ -254,10 +254,10 @@ namespace ts.FindAllReferences.Core {
         }
 
         if (symbol.flags & SymbolFlags.Module && isModuleReferenceLocation(node)) {
-            return getReferencedSymbolsForModule(program, symbol, sourceFiles);
+            return getReferencedSymbolsForModule(program, symbol, sourceFiles, sourceFilesSet);
         }
 
-        return getReferencedSymbolsForSymbol(symbol, node, sourceFiles, checker, cancellationToken, options);
+        return getReferencedSymbolsForSymbol(symbol, node, sourceFiles, sourceFilesSet, checker, cancellationToken, options);
     }
 
     function isModuleReferenceLocation(node: Node): boolean {
@@ -277,7 +277,7 @@ namespace ts.FindAllReferences.Core {
         }
     }
 
-    function getReferencedSymbolsForModule(program: Program, symbol: Symbol, sourceFiles: ReadonlyArray<SourceFile>): SymbolAndEntries[] {
+    function getReferencedSymbolsForModule(program: Program, symbol: Symbol, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>): SymbolAndEntries[] {
         Debug.assert(!!symbol.valueDeclaration);
 
         const references = findModuleReferences(program, sourceFiles, symbol).map<Entry>(reference => {
@@ -299,7 +299,9 @@ namespace ts.FindAllReferences.Core {
                     // Don't include the source file itself. (This may not be ideal behavior, but awkward to include an entire file as a reference.)
                     break;
                 case SyntaxKind.ModuleDeclaration:
-                    references.push({ type: "node", node: (decl as ModuleDeclaration).name });
+                    if (sourceFilesSet.has(decl.getSourceFile().fileName)) {
+                        references.push({ type: "node", node: (decl as ModuleDeclaration).name });
+                    }
                     break;
                 default:
                     Debug.fail("Expected a module symbol to be declared by a SourceFile or ModuleDeclaration.");
@@ -339,14 +341,14 @@ namespace ts.FindAllReferences.Core {
     }
 
     /** Core find-all-references algorithm for a normal symbol. */
-    function getReferencedSymbolsForSymbol(symbol: Symbol, node: Node, sourceFiles: ReadonlyArray<SourceFile>, checker: TypeChecker, cancellationToken: CancellationToken, options: Options): SymbolAndEntries[] {
+    function getReferencedSymbolsForSymbol(symbol: Symbol, node: Node, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>, checker: TypeChecker, cancellationToken: CancellationToken, options: Options): SymbolAndEntries[] {
         symbol = skipPastExportOrImportSpecifierOrUnion(symbol, node, checker) || symbol;
 
         // Compute the meaning from the location and the symbol it references
         const searchMeaning = getIntersectingMeaningFromDeclarations(node, symbol);
 
         const result: SymbolAndEntries[] = [];
-        const state = new State(sourceFiles, getSpecialSearchKind(node), checker, cancellationToken, searchMeaning, options, result);
+        const state = new State(sourceFiles, sourceFilesSet, getSpecialSearchKind(node), checker, cancellationToken, searchMeaning, options, result);
 
         if (node.kind === SyntaxKind.DefaultKeyword) {
             addReference(node, symbol, state);
@@ -469,28 +471,26 @@ namespace ts.FindAllReferences.Core {
          */
         readonly markSeenReExportRHS = nodeSeenTracker();
 
-        private readonly includedSourceFiles: Map<true>;
-
         constructor(
             readonly sourceFiles: ReadonlyArray<SourceFile>,
+            readonly sourceFilesSet: ReadonlyMap<true>,
             /** True if we're searching for constructor references. */
             readonly specialSearchKind: SpecialSearchKind,
             readonly checker: TypeChecker,
             readonly cancellationToken: CancellationToken,
             readonly searchMeaning: SemanticMeaning,
             readonly options: Options,
             private readonly result: Push<SymbolAndEntries>) {
-            this.includedSourceFiles = arrayToSet(sourceFiles, s => s.fileName);
         }
 
         includesSourceFile(sourceFile: SourceFile): boolean {
-            return this.includedSourceFiles.has(sourceFile.fileName);
+            return this.sourceFilesSet.has(sourceFile.fileName);
         }
 
         private importTracker: ImportTracker | undefined;
         /** Gets every place to look for references of an exported symbols. See `ImportsResult` in `importTracker.ts` for more documentation. */
         getImportSearches(exportSymbol: Symbol, exportInfo: ExportInfo): ImportsResult {
-            if (!this.importTracker) this.importTracker = createImportTracker(this.sourceFiles, this.checker, this.cancellationToken);
+            if (!this.importTracker) this.importTracker = createImportTracker(this.sourceFiles, this.sourceFilesSet, this.checker, this.cancellationToken);
             return this.importTracker(exportSymbol, exportInfo, this.options.isForRename);
         }
 
diff --git a/src/services/importTracker.ts b/src/services/importTracker.ts
@@ -12,10 +12,10 @@ namespace ts.FindAllReferences {
     export type ImportTracker = (exportSymbol: Symbol, exportInfo: ExportInfo, isForRename: boolean) => ImportsResult;
 
     /** Creates the imports map and returns an ImportTracker that uses it. Call this lazily to avoid calling `getDirectImportsMap` unnecessarily.  */
-    export function createImportTracker(sourceFiles: ReadonlyArray<SourceFile>, checker: TypeChecker, cancellationToken: CancellationToken): ImportTracker {
+    export function createImportTracker(sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>, checker: TypeChecker, cancellationToken: CancellationToken): ImportTracker {
         const allDirectImports = getDirectImportsMap(sourceFiles, checker, cancellationToken);
         return (exportSymbol, exportInfo, isForRename) => {
-            const { directImports, indirectUsers } = getImportersForExport(sourceFiles, allDirectImports, exportInfo, checker, cancellationToken);
+            const { directImports, indirectUsers } = getImportersForExport(sourceFiles, sourceFilesSet, allDirectImports, exportInfo, checker, cancellationToken);
             return { indirectUsers, ...getSearchesFromDirectImports(directImports, exportSymbol, exportInfo.exportKind, checker, isForRename) };
         };
     }
@@ -39,6 +39,7 @@ namespace ts.FindAllReferences {
     /** Returns import statements that directly reference the exporting module, and a list of files that may access the module through a namespace. */
     function getImportersForExport(
         sourceFiles: ReadonlyArray<SourceFile>,
+        sourceFilesSet: ReadonlyMap<true>,
         allDirectImports: Map<ImporterOrCallExpression[]>,
         { exportingModuleSymbol, exportKind }: ExportInfo,
         checker: TypeChecker,
@@ -62,7 +63,7 @@ namespace ts.FindAllReferences {
 
             // Module augmentations may use this module's exports without importing it.
             for (const decl of exportingModuleSymbol.declarations) {
-                if (isExternalModuleAugmentation(decl)) {
+                if (isExternalModuleAugmentation(decl) && sourceFilesSet.has(decl.getSourceFile().fileName)) {
                     addIndirectUser(decl);
                 }
             }
diff --git a/src/services/services.ts b/src/services/services.ts
@@ -1697,20 +1697,17 @@ namespace ts {
 
         /// References and Occurrences
         function getOccurrencesAtPosition(fileName: string, position: number): ReferenceEntry[] {
-            const canonicalFileName = getCanonicalFileName(normalizeSlashes(fileName));
-            return flatMap(getDocumentHighlights(fileName, position, [fileName]), entry => entry.highlightSpans.map<ReferenceEntry>(highlightSpan => {
-                Debug.assert(getCanonicalFileName(normalizeSlashes(entry.fileName)) === canonicalFileName); // Get occurrences only supports reporting occurrences for the file queried.
-                return {
-                    fileName: entry.fileName,
-                    textSpan: highlightSpan.textSpan,
-                    isWriteAccess: highlightSpan.kind === HighlightSpanKind.writtenReference,
-                    isDefinition: false,
-                    isInString: highlightSpan.isInString,
-                };
-            }));
+            return flatMap(getDocumentHighlights(fileName, position, [fileName]), entry => entry.highlightSpans.map<ReferenceEntry>(highlightSpan => ({
+                fileName: entry.fileName,
+                textSpan: highlightSpan.textSpan,
+                isWriteAccess: highlightSpan.kind === HighlightSpanKind.writtenReference,
+                isDefinition: false,
+                isInString: highlightSpan.isInString,
+            })));
         }
 
         function getDocumentHighlights(fileName: string, position: number, filesToSearch: ReadonlyArray<string>): DocumentHighlights[] {
+            Debug.assert(contains(filesToSearch, fileName));
             synchronizeHostData();
             const sourceFilesToSearch = map(filesToSearch, f => Debug.assertDefined(program.getSourceFile(f)));
             const sourceFile = getValidSourceFile(fileName);
diff --git a/tests/cases/fourslash/duplicatePackageServices.ts b/tests/cases/fourslash/duplicatePackageServices.ts
@@ -23,7 +23,7 @@
 ////}
 
 // @Filename: /node_modules/b/node_modules/x/package.json
-////{ "name": "x", "version": "1.2./*bVersionPatch*/3" }
+////{ "name": "x", "version": "1.2.3" }
 
 // @Filename: /src/a.ts
 ////import { a } from "a";
@@ -40,7 +40,5 @@ const aImport = { definition: "(alias) class X\nimport X", ranges: [r0, r1] };
 const def = { definition: "class X", ranges: [r2] };
 const bImport = { definition: "(alias) class X\nimport X", ranges: [r3, r4] };
 verify.referenceGroups([r0, r1], [aImport, def, bImport]);
-verify.referenceGroups([r2], [def, aImport, bImport]);
+verify.referenceGroups([r2, r5], [def, aImport, bImport]);
 verify.referenceGroups([r3, r4], [bImport, def, aImport]);
-
-verify.referenceGroups(r5, [def, aImport, bImport]);

Original file line number	Diff line number	Diff line change
`@@ -1081,8 +1081,20 @@ namespace FourSlash {`
`1081`	`1081`	`}`
`1082`	`1082`	`}`
`1083`	`1083`
	`1084`	`+ private verifyDocumentHighlightsRespectFilesList(files: ReadonlyArray<string>): void {`
	`1085`	`+ const startFile = this.activeFile.fileName;`
	`1086`	`+ for (const fileName of files) {`
	`1087`	`+ const searchFileNames = startFile === fileName ? [startFile] : [startFile, fileName];`
	`1088`	`+ const highlights = this.getDocumentHighlightsAtCurrentPosition(searchFileNames);`
	`1089`	`+ if (!highlights.every(dh => ts.contains(searchFileNames, dh.fileName))) {`
	`1090`	+ this.raiseError(`When asking for document highlights only in files ${searchFileNames}, got document highlights in ${unique(highlights, dh => dh.fileName)}`);
	`1091`	`+ }`
	`1092`	`+ }`
	`1093`	`+ }`
	`1094`	`+`
`1084`	`1095`	`public verifyReferencesOf(range: Range, references: Range[]) {`
`1085`	`1096`	`this.goToRangeStart(range);`
	`1097`	`+ this.verifyDocumentHighlightsRespectFilesList(unique(references, e => e.fileName));`
`1086`	`1098`	`this.verifyReferencesAre(references);`
`1087`	`1099`	`}`
`1088`	`1100`
`@@ -1094,7 +1106,7 @@ namespace FourSlash {`
`1094`	`1106`	`}`
`1095`	`1107`	`}`
`1096`	`1108`
`1097`		`- public verifyReferenceGroups(starts: string \| string[] \| Range \| Range[], parts: FourSlashInterface.ReferenceGroup[]): void {`
	`1109`	`+ public verifyReferenceGroups(starts: string \| string[] \| Range \| Range[], parts: FourSlashInterface.ReferenceGroup[] \| undefined): void {`
`1098`	`1110`	`interface ReferenceGroupJson {`
`1099`	`1111`	`definition: string \| { text: string, range: ts.TextSpan };`
`1100`	`1112`	`references: ts.ReferenceEntry[];`
`@@ -1128,6 +1140,10 @@ namespace FourSlash {`
`1128`	`1140`	`};`
`1129`	`1141`	`});`
`1130`	`1142`	`this.assertObjectsEqual(fullActual, fullExpected);`
	`1143`	`+`
	`1144`	`+ if (parts) {`
	`1145`	`+ this.verifyDocumentHighlightsRespectFilesList(unique(ts.flatMap(parts, p => p.ranges), r => r.fileName));`
	`1146`	`+ }`
`1131`	`1147`	`}`
`1132`	`1148`	`}`
`1133`	`1149`