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

* When testing references, also test documentHighlights respects filesToSearch

* Fix handling for redirects and move assertion inside getDocumentHighlights

* Add another assert
This commit is contained in:
Andy 2018-04-11 14:07:22 -07:00 committed by GitHub
parent fef28665f7
commit f6b206a75a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 59 additions and 37 deletions

View file

@ -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));
}
}
}

View file

@ -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[] {

View file

@ -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,10 +471,9 @@ 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,
@ -480,17 +481,16 @@ namespace ts.FindAllReferences.Core {
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);
}

View file

@ -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);
}
}

View file

@ -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);

View file

@ -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]);