Skip to content

Commit

Permalink
Navto covers all projects (#38027)
Browse files Browse the repository at this point in the history 
* Remove needless structure/destructuring

Just pass multiple arguments! Sheesh!

* Basic working prototype

* Cleaned up version

1. Add test
2. Change protocol. navto-all only happens when filename is undefined or
missing.
3. Change location to earlier code path. This change was largely
type-guided and resulted in some duplicated code, but I think it's less
fault-prone.

* remove temp notes

* Single-project hits if projectFileName is provided

and file is not

* use original code as fallback
  • Loading branch information
@sandersn
sandersn authored Apr 21, 2020
1 parent 892427a commit d571a09
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 35 deletions.
8 changes: 6 additions & 2 deletions src/server/protocol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2784,7 +2784,7 @@ namespace ts.server.protocol {
/**
* Arguments for navto request message.
*/
export interface NavtoRequestArgs extends FileRequestArgs {
export interface NavtoRequestArgs {
/**
* Search term to navigate to from current location; term can
* be '.*' or an identifier prefix.
Expand All @@ -2794,6 +2794,10 @@ namespace ts.server.protocol {
* Optional limit on the number of items to return.
*/
maxResultCount?: number;
/**
* The file for the request (absolute pathname required).
*/
file?: string;
/**
* Optional flag to indicate we want results for just the current file
* or the entire project.
Expand All @@ -2809,7 +2813,7 @@ namespace ts.server.protocol {
* match the search term given in argument 'searchTerm'. The
* context for the search is given by the named file.
*/
export interface NavtoRequest extends FileRequest {
export interface NavtoRequest extends Request {
command: CommandTypes.Navto;
arguments: NavtoRequestArgs;
}
Expand Down
67 changes: 40 additions & 27 deletions src/server/session.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ namespace ts.server {
projects,
defaultProject,
/*initialLocation*/ undefined,
({ project }, tryAddToTodo) => {
(project, _, tryAddToTodo) => {
for (const output of action(project)) {
if (!contains(outputs, output, resultsEqual) && !tryAddToTodo(project, getLocation(output))) {
outputs.push(output);
Expand All @@ -329,7 +329,7 @@ namespace ts.server {
projects,
defaultProject,
initialLocation,
({ project, location }, tryAddToTodo) => {
(project, location, tryAddToTodo) => {
for (const output of project.getLanguageService().findRenameLocations(location.fileName, location.pos, findInStrings, findInComments, hostPreferences.providePrefixAndSuffixTextForRename) || emptyArray) {
if (!contains(outputs, output, documentSpansEqual) && !tryAddToTodo(project, documentSpanLocation(output))) {
outputs.push(output);
Expand Down Expand Up @@ -358,7 +358,7 @@ namespace ts.server {
projects,
defaultProject,
initialLocation,
({ project, location }, getMappedLocation) => {
(project, location, getMappedLocation) => {
for (const outputReferencedSymbol of project.getLanguageService().findReferences(location.fileName, location.pos) || emptyArray) {
const mappedDefinitionFile = getMappedLocation(project, documentSpanLocation(outputReferencedSymbol.definition));
const definition: ReferencedSymbolDefinitionInfo = mappedDefinitionFile === undefined ?
Expand Down Expand Up @@ -408,7 +408,8 @@ namespace ts.server {
}

type CombineProjectOutputCallback<TLocation extends DocumentPosition | undefined> = (
where: ProjectAndLocation<TLocation>,
project: Project,
location: TLocation,
getMappedLocation: (project: Project, location: DocumentPosition) => DocumentPosition | undefined,
) => void;

Expand All @@ -422,9 +423,9 @@ namespace ts.server {
let toDo: ProjectAndLocation<TLocation>[] | undefined;
const seenProjects = createMap<true>();
forEachProjectInProjects(projects, initialLocation && initialLocation.fileName, (project, path) => {
// TLocation shoud be either `DocumentPosition` or `undefined`. Since `initialLocation` is `TLocation` this cast should be valid.
// TLocation should be either `DocumentPosition` or `undefined`. Since `initialLocation` is `TLocation` this cast should be valid.
const location = (initialLocation ? { fileName: path, pos: initialLocation.pos } : undefined) as TLocation;
toDo = callbackProjectAndLocation({ project, location }, projectService, toDo, seenProjects, cb);
toDo = callbackProjectAndLocation(project, location, projectService, toDo, seenProjects, cb);
});

// After initial references are collected, go over every other project and see if it has a reference for the symbol definition.
Expand All @@ -442,14 +443,16 @@ namespace ts.server {
if (!addToSeen(seenProjects, project)) return;
const definition = mapDefinitionInProject(defaultDefinition, project, getGeneratedDefinition, getSourceDefinition);
if (definition) {
toDo = callbackProjectAndLocation<TLocation>({ project, location: definition as TLocation }, projectService, toDo, seenProjects, cb);
toDo = callbackProjectAndLocation<TLocation>(project, definition as TLocation, projectService, toDo, seenProjects, cb);
}
});
}
}

while (toDo && toDo.length) {
toDo = callbackProjectAndLocation(Debug.checkDefined(toDo.pop()), projectService, toDo, seenProjects, cb);
const next = toDo.pop();
Debug.assertIsDefined(next);
toDo = callbackProjectAndLocation(next.project, next.location, projectService, toDo, seenProjects, cb);
}
}

Expand Down Expand Up @@ -487,40 +490,42 @@ namespace ts.server {
}

function callbackProjectAndLocation<TLocation extends DocumentPosition | undefined>(
projectAndLocation: ProjectAndLocation<TLocation>,
project: Project,
location: TLocation,
projectService: ProjectService,
toDo: ProjectAndLocation<TLocation>[] | undefined,
seenProjects: Map<true>,
cb: CombineProjectOutputCallback<TLocation>,
): ProjectAndLocation<TLocation>[] | undefined {
const { project, location } = projectAndLocation;
if (project.getCancellationToken().isCancellationRequested()) return undefined; // Skip rest of toDo if cancelled
// If this is not the file we were actually looking, return rest of the toDo
if (isLocationProjectReferenceRedirect(project, location)) return toDo;
cb(projectAndLocation, (project, location) => {
addToSeen(seenProjects, projectAndLocation.project);
const originalLocation = projectService.getOriginalLocationEnsuringConfiguredProject(project, location);
cb(project, location, (innerProject, location) => {
addToSeen(seenProjects, project);
const originalLocation = projectService.getOriginalLocationEnsuringConfiguredProject(innerProject, location);
if (!originalLocation) return undefined;

const originalScriptInfo = projectService.getScriptInfo(originalLocation.fileName)!;
toDo = toDo || [];

for (const project of originalScriptInfo.containingProjects) {
addToTodo({ project, location: originalLocation as TLocation }, toDo, seenProjects);
addToTodo(project, originalLocation as TLocation, toDo, seenProjects);
}
const symlinkedProjectsMap = projectService.getSymlinkedProjects(originalScriptInfo);
if (symlinkedProjectsMap) {
symlinkedProjectsMap.forEach((symlinkedProjects, symlinkedPath) => {
for (const symlinkedProject of symlinkedProjects) addToTodo({ project: symlinkedProject, location: { fileName: symlinkedPath, pos: originalLocation.pos } as TLocation }, toDo!, seenProjects);
for (const symlinkedProject of symlinkedProjects) {
addToTodo(symlinkedProject, { fileName: symlinkedPath, pos: originalLocation.pos } as TLocation, toDo!, seenProjects);
}
});
}
return originalLocation === location ? undefined : originalLocation;
});
return toDo;
}

function addToTodo<TLocation extends DocumentPosition | undefined>(projectAndLocation: ProjectAndLocation<TLocation>, toDo: Push<ProjectAndLocation<TLocation>>, seenProjects: Map<true>): void {
if (addToSeen(seenProjects, projectAndLocation.project)) toDo.push(projectAndLocation);
function addToTodo<TLocation extends DocumentPosition | undefined>(project: Project, location: TLocation, toDo: Push<ProjectAndLocation<TLocation>>, seenProjects: Map<true>): void {
if (addToSeen(seenProjects, project)) toDo.push({ project, location });
}

function addToSeen(seenProjects: Map<true>, project: Project) {
Expand Down Expand Up @@ -1323,7 +1328,7 @@ namespace ts.server {
// filter handles case when 'projects' is undefined
projects = filter(projects, p => p.languageServiceEnabled && !p.isOrphan());
if (!ignoreNoProjectError && (!projects || !projects.length) && !symLinkedProjects) {
this.projectService.logErrorForScriptInfoNotFound(args.file);
this.projectService.logErrorForScriptInfoNotFound(args.file ?? args.projectFileName);
return Errors.ThrowNoProject();
}
return symLinkedProjects ? { projects: projects!, symLinkedProjects } : projects!; // TODO: GH#18217
Expand All @@ -1335,6 +1340,9 @@ namespace ts.server {
if (project) {
return project;
}
if (!args.file) {
return Errors.ThrowNoProject();
}
}
const info = this.projectService.getScriptInfo(args.file)!;
return info.getDefaultProject();
Expand Down Expand Up @@ -1894,20 +1902,25 @@ namespace ts.server {
}

private getFullNavigateToItems(args: protocol.NavtoRequestArgs): readonly NavigateToItem[] {
const { currentFileOnly, searchValue, maxResultCount } = args;
const { currentFileOnly, searchValue, maxResultCount, projectFileName } = args;
if (currentFileOnly) {
const { file, project } = this.getFileAndProject(args);
Debug.assertDefined(args.file);
const { file, project } = this.getFileAndProject(args as protocol.FileRequestArgs);
return project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, file);
}
else {
return combineProjectOutputWhileOpeningReferencedProjects<NavigateToItem>(
this.getProjects(args),
this.getDefaultProject(args),
project =>
project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, /*fileName*/ undefined, /*excludeDts*/ project.isNonTsProject()),
documentSpanLocation,
else if (!args.file && !projectFileName) {
return combineProjectOutputFromEveryProject(
this.projectService,
project => project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, /*filename*/ undefined, /*excludeDts*/ project.isNonTsProject()),
navigateToItemIsEqualTo);
}
const fileArgs = args as protocol.FileRequestArgs;
return combineProjectOutputWhileOpeningReferencedProjects<NavigateToItem>(
this.getProjects(fileArgs),
this.getDefaultProject(fileArgs),
project => project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, /*fileName*/ undefined, /*excludeDts*/ project.isNonTsProject()),
documentSpanLocation,
navigateToItemIsEqualTo);

function navigateToItemIsEqualTo(a: NavigateToItem, b: NavigateToItem): boolean {
if (a === b) {
Expand Down
68 changes: 64 additions & 4 deletions src/testRunner/unittests/tsserver/declarationFileMaps.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ namespace ts.projectSystem {
};
const aDts: File = {
path: "/a/bin/a.d.ts",
// Need to mangle the sourceMappingURL part or it breaks the build
// ${""} is needed to mangle the sourceMappingURL part or it breaks the build
content: `export declare function fnA(): void;\nexport interface IfaceA {\n}\nexport declare const instanceA: IfaceA;\n//# source${""}MappingURL=a.d.ts.map`,
};

Expand All @@ -86,7 +86,7 @@ namespace ts.projectSystem {
content: JSON.stringify(bDtsMapContent),
};
const bDts: File = {
// Need to mangle the sourceMappingURL part or it breaks the build
// ${""} is need to mangle the sourceMappingURL part so it doesn't break the build
path: "/b/bin/b.d.ts",
content: `export declare function fnB(): void;\n//# source${""}MappingURL=b.d.ts.map`,
};
Expand Down Expand Up @@ -114,15 +114,17 @@ namespace ts.projectSystem {
})
};

function makeSampleProjects(addUserTsConfig?: boolean) {
function makeSampleProjects(addUserTsConfig?: boolean, keepAllFiles?: boolean) {
const host = createServerHost([aTs, aTsconfig, aDtsMap, aDts, bTsconfig, bTs, bDtsMap, bDts, ...(addUserTsConfig ? [userTsForConfigProject, userTsconfig] : [userTs]), dummyFile]);
const session = createSession(host);

checkDeclarationFiles(aTs, session, [aDtsMap, aDts]);
checkDeclarationFiles(bTs, session, [bDtsMap, bDts]);

// Testing what happens if we delete the original sources.
host.deleteFile(bTs.path);
if (!keepAllFiles) {
host.deleteFile(bTs.path);
}

openFilesForSession([userTs], session);
const service = session.getProjectService();
Expand Down Expand Up @@ -322,6 +324,64 @@ namespace ts.projectSystem {
verifyATsConfigOriginalProject(session);
});

it("navigateToAll -- when neither file nor project is specified", () => {
const session = makeSampleProjects(/*addUserTsConfig*/ true, /*keepAllFiles*/ true);
const response = executeSessionRequest<protocol.NavtoRequest, protocol.NavtoResponse>(session, CommandNames.Navto, { file: undefined, searchValue: "fn" });
assert.deepEqual<readonly protocol.NavtoItem[] | undefined>(response, [
{
...protocolFileSpanFromSubstring({
file: bTs,
text: "export function fnB() {}"
}),
name: "fnB",
matchKind: "prefix",
isCaseSensitive: true,
kind: ScriptElementKind.functionElement,
kindModifiers: "export",
},
{
...protocolFileSpanFromSubstring({
file: aTs,
text: "export function fnA() {}"
}),
name: "fnA",
matchKind: "prefix",
isCaseSensitive: true,
kind: ScriptElementKind.functionElement,
kindModifiers: "export",
},
{
...protocolFileSpanFromSubstring({
file: userTs,
text: "export function fnUser() { a.fnA(); b.fnB(); a.instanceA; }"
}),
name: "fnUser",
matchKind: "prefix",
isCaseSensitive: true,
kind: ScriptElementKind.functionElement,
kindModifiers: "export",
}
]);
});

it("navigateToAll -- when file is not specified but project is", () => {
const session = makeSampleProjects(/*addUserTsConfig*/ true, /*keepAllFiles*/ true);
const response = executeSessionRequest<protocol.NavtoRequest, protocol.NavtoResponse>(session, CommandNames.Navto, { projectFileName: bTsconfig.path, file: undefined, searchValue: "fn" });
assert.deepEqual<readonly protocol.NavtoItem[] | undefined>(response, [
{
...protocolFileSpanFromSubstring({
file: bTs,
text: "export function fnB() {}"
}),
name: "fnB",
matchKind: "prefix",
isCaseSensitive: true,
kind: ScriptElementKind.functionElement,
kindModifiers: "export",
}
]);
});

const referenceATs = (aTs: File): protocol.ReferencesResponseItem => makeReferenceItem({
file: aTs,
isDefinition: true,
Expand Down
8 changes: 6 additions & 2 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8319,7 +8319,7 @@ declare namespace ts.server.protocol {
/**
* Arguments for navto request message.
*/
interface NavtoRequestArgs extends FileRequestArgs {
interface NavtoRequestArgs {
/**
* Search term to navigate to from current location; term can
* be '.*' or an identifier prefix.
Expand All @@ -8329,6 +8329,10 @@ declare namespace ts.server.protocol {
* Optional limit on the number of items to return.
*/
maxResultCount?: number;
/**
* The file for the request (absolute pathname required).
*/
file?: string;
/**
* Optional flag to indicate we want results for just the current file
* or the entire project.
Expand All @@ -8342,7 +8346,7 @@ declare namespace ts.server.protocol {
* match the search term given in argument 'searchTerm'. The
* context for the search is given by the named file.
*/
interface NavtoRequest extends FileRequest {
interface NavtoRequest extends Request {
command: CommandTypes.Navto;
arguments: NavtoRequestArgs;
}
Expand Down

0 comments on commit d571a09

Please sign in to comment.