Skip to content

Add option to exclude library symbols from navTo results #55605

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9936,6 +9936,7 @@ export interface UserPreferences {
readonly organizeImportsNumericCollation?: boolean;
readonly organizeImportsAccentCollation?: boolean;
readonly organizeImportsCaseFirst?: "upper" | "lower" | false;
readonly excludeLibrarySymbolsInNavTo?: boolean;
}

/** Represents a bigint literal value without requiring bigint support */
Expand Down
25 changes: 18 additions & 7 deletions src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,15 +314,25 @@ export class SessionClient implements LanguageService {
return notImplemented();
}

getNavigateToItems(searchValue: string): NavigateToItem[] {
getNavigateToItems(searchValue: string, maxResultCount: number, file: string | undefined, _excludeDtsFiles: boolean | undefined, excludeLibFiles: boolean | undefined): NavigateToItem[] {
const args: protocol.NavtoRequestArgs = {
searchValue,
file: this.host.getScriptFileNames()[0],
file,
currentFileOnly: !!file,
maxResultCount,
};
const oldPreferences = this.preferences;
if (excludeLibFiles) {
this.configure({ excludeLibrarySymbolsInNavTo: true });
}

const request = this.processRequest<protocol.NavtoRequest>(protocol.CommandTypes.Navto, args);
const response = this.processResponse<protocol.NavtoResponse>(request);

if (excludeLibFiles) {
this.configure(oldPreferences || {});
}

return response.body!.map(entry => ({ // TODO: GH#18217
name: entry.name,
containerName: entry.containerName || "",
Expand Down Expand Up @@ -579,14 +589,13 @@ export class SessionClient implements LanguageService {
const providePrefixAndSuffixTextForRename = typeof preferences === "boolean" ? preferences : preferences?.providePrefixAndSuffixTextForRename;
const quotePreference = typeof preferences === "boolean" ? undefined : preferences?.quotePreference;
if (providePrefixAndSuffixTextForRename !== undefined || quotePreference !== undefined) {
const oldPreferences = this.preferences;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other diffs in the file were just a drive-by fix to how we restore preferences for the harness client. I think we don't have tests that observe this though.

// User preferences have to be set through the `Configure` command
this.configure({ providePrefixAndSuffixTextForRename, quotePreference });
// Options argument is not used, so don't pass in options
this.getRenameInfo(fileName, position, /*preferences*/ {}, findInStrings, findInComments);
// Restore previous user preferences
if (this.preferences) {
this.configure(this.preferences);
}
this.configure(oldPreferences || {});
}
else {
this.getRenameInfo(fileName, position, /*preferences*/ {}, findInStrings, findInComments);
Expand Down Expand Up @@ -812,6 +821,7 @@ export class SessionClient implements LanguageService {
kind?: string,
includeInteractiveActions?: boolean,
): ApplicableRefactorInfo[] {
const oldPreferences = this.preferences;
if (preferences) { // Temporarily set preferences
this.configure(preferences);
}
Expand All @@ -822,7 +832,7 @@ export class SessionClient implements LanguageService {
const request = this.processRequest<protocol.GetApplicableRefactorsRequest>(protocol.CommandTypes.GetApplicableRefactors, args);
const response = this.processResponse<protocol.GetApplicableRefactorsResponse>(request);
if (preferences) { // Restore preferences
this.configure(this.preferences || {});
this.configure(oldPreferences || {});
}
return response.body!; // TODO: GH#18217
}
Expand All @@ -844,6 +854,7 @@ export class SessionClient implements LanguageService {
preferences: UserPreferences | undefined,
interactiveRefactorArguments?: InteractiveRefactorArguments,
): RefactorEditInfo {
const oldPreferences = this.preferences;
if (preferences) { // Temporarily set preferences
this.configure(preferences);
}
Expand All @@ -868,7 +879,7 @@ export class SessionClient implements LanguageService {
}

if (preferences) { // Restore preferences
this.configure(this.preferences || {});
this.configure(oldPreferences || {});
}

return {
Expand Down
4 changes: 2 additions & 2 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3765,9 +3765,9 @@ export class TestState {
}

public verifyNavigateTo(options: readonly FourSlashInterface.VerifyNavigateToOptions[]): void {
for (const { pattern, expected, fileName } of options) {
for (const { pattern, expected, fileName, excludeLibFiles } of options) {
const file = fileName && this.findFile(fileName).fileName;
const items = this.languageService.getNavigateToItems(pattern, /*maxResultCount*/ undefined, file);
const items = this.languageService.getNavigateToItems(pattern, /*maxResultCount*/ undefined, file, /*excludeDtsFiles*/ undefined, excludeLibFiles);
this.assertObjectsEqual(
items,
expected.map((e): ts.NavigateToItem => ({
Expand Down
1 change: 1 addition & 0 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1809,6 +1809,7 @@ export interface VerifyNavigateToOptions {
readonly pattern: string;
readonly fileName?: string;
readonly expected: readonly ExpectedNavigateToItem[];
readonly excludeLibFiles?: boolean;
}

export interface ExpectedNavigateToItem {
Expand Down
5 changes: 5 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3599,6 +3599,11 @@ export interface UserPreferences {
* Indicates whether {@link ReferencesResponseItem.lineText} is supported.
*/
readonly disableLineTextInReferences?: boolean;

/**
* Indicates whether to exclude standard library and node_modules file symbols from navTo results.
*/
readonly excludeLibrarySymbolsInNavTo?: boolean;
}

export interface CompilerOptions {
Expand Down
9 changes: 8 additions & 1 deletion src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2613,6 +2613,7 @@ export class Session<TMessage = string> implements EventSender {
const { file, project } = this.getFileAndProject(args as protocol.FileRequestArgs);
return [{ project, navigateToItems: project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, file) }];
}
const preferences = this.getHostPreferences();

const outputs: ProjectNavigateToItems[] = [];

Expand Down Expand Up @@ -2646,7 +2647,13 @@ export class Session<TMessage = string> implements EventSender {

// Mutates `outputs`
function addItemsForProject(project: Project) {
const projectItems = project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, /*fileName*/ undefined, /*excludeDts*/ project.isNonTsProject());
const projectItems = project.getLanguageService().getNavigateToItems(
searchValue,
maxResultCount,
/*fileName*/ undefined,
/*excludeDts*/ project.isNonTsProject(),
/*excludeLibFiles*/ preferences.excludeLibrarySymbolsInNavTo,
);
const unseenItems = filter(projectItems, item => tryAddSeenItem(item) && !getMappedLocationForProject(documentSpanLocation(item), project));
if (unseenItems.length) {
outputs.push({ project, navigateToItems: unseenItems });
Expand Down
42 changes: 35 additions & 7 deletions src/services/navigateTo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
ImportClause,
ImportEqualsDeclaration,
ImportSpecifier,
isInsideNodeModules,
isPropertyAccessExpression,
isPropertyNameLiteral,
NavigateToItem,
Expand All @@ -37,11 +38,11 @@ interface RawNavigateToItem {
}

/** @internal */
export function getNavigateToItems(sourceFiles: readonly SourceFile[], checker: TypeChecker, cancellationToken: CancellationToken, searchValue: string, maxResultCount: number | undefined, excludeDtsFiles: boolean): NavigateToItem[] {
export function getNavigateToItems(sourceFiles: readonly SourceFile[], checker: TypeChecker, cancellationToken: CancellationToken, searchValue: string, maxResultCount: number | undefined, excludeDtsFiles: boolean, excludeLibFiles?: boolean): NavigateToItem[] {
const patternMatcher = createPatternMatcher(searchValue);
if (!patternMatcher) return emptyArray;
const rawItems: RawNavigateToItem[] = [];

const singleCurrentFile = sourceFiles.length === 1 ? sourceFiles[0] : undefined;
// Search the declarations in all files and output matched NavigateToItem into array of NavigateToItem[]
for (const sourceFile of sourceFiles) {
cancellationToken.throwIfCancellationRequested();
Expand All @@ -50,16 +51,37 @@ export function getNavigateToItems(sourceFiles: readonly SourceFile[], checker:
continue;
}

if (shouldExcludeFile(sourceFile, !!excludeLibFiles, singleCurrentFile)) {
continue;
}

sourceFile.getNamedDeclarations().forEach((declarations, name) => {
getItemsFromNamedDeclaration(patternMatcher, name, declarations, checker, sourceFile.fileName, rawItems);
getItemsFromNamedDeclaration(patternMatcher, name, declarations, checker, sourceFile.fileName, !!excludeLibFiles, singleCurrentFile, rawItems);
});
}

rawItems.sort(compareNavigateToItems);
return (maxResultCount === undefined ? rawItems : rawItems.slice(0, maxResultCount)).map(createNavigateToItem);
}

function getItemsFromNamedDeclaration(patternMatcher: PatternMatcher, name: string, declarations: readonly Declaration[], checker: TypeChecker, fileName: string, rawItems: RawNavigateToItem[]): void {
/**
* Exclude 'node_modules/' files and standard library files if 'excludeLibFiles' is true.
* If we're in current file only mode, we don't exclude the current file, even if it is a library file.
*/
function shouldExcludeFile(file: SourceFile, excludeLibFiles: boolean, singleCurrentFile: SourceFile | undefined): boolean {
return file !== singleCurrentFile && excludeLibFiles && (isInsideNodeModules(file.path) || file.hasNoDefaultLib);
}

function getItemsFromNamedDeclaration(
patternMatcher: PatternMatcher,
name: string,
declarations: readonly Declaration[],
checker: TypeChecker,
fileName: string,
excludeLibFiles: boolean,
singleCurrentFile: SourceFile | undefined,
rawItems: RawNavigateToItem[],
): void {
// First do a quick check to see if the name of the declaration matches the
// last portion of the (possibly) dotted name they're searching for.
const match = patternMatcher.getMatchForLastSegmentOfPattern(name);
Expand All @@ -68,7 +90,7 @@ function getItemsFromNamedDeclaration(patternMatcher: PatternMatcher, name: stri
}

for (const declaration of declarations) {
if (!shouldKeepItem(declaration, checker)) continue;
if (!shouldKeepItem(declaration, checker, excludeLibFiles, singleCurrentFile)) continue;

if (patternMatcher.patternContainsDots) {
// If the pattern has dots in it, then also see if the declaration container matches as well.
Expand All @@ -83,14 +105,20 @@ function getItemsFromNamedDeclaration(patternMatcher: PatternMatcher, name: stri
}
}

function shouldKeepItem(declaration: Declaration, checker: TypeChecker): boolean {
function shouldKeepItem(
declaration: Declaration,
checker: TypeChecker,
excludeLibFiles: boolean,
singleCurrentFile: SourceFile | undefined,
): boolean {
switch (declaration.kind) {
case SyntaxKind.ImportClause:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.ImportEqualsDeclaration:
const importer = checker.getSymbolAtLocation((declaration as ImportClause | ImportSpecifier | ImportEqualsDeclaration).name!)!; // TODO: GH#18217
const imported = checker.getAliasedSymbol(importer);
return importer.escapedName !== imported.escapedName;
return importer.escapedName !== imported.escapedName
&& !imported.declarations?.every(d => shouldExcludeFile(d.getSourceFile(), excludeLibFiles, singleCurrentFile));
default:
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2192,10 +2192,10 @@ export function createLanguageService(
return FindAllReferences.Core.getReferencesForFileName(fileName, program, program.getSourceFiles()).map(FindAllReferences.toReferenceEntry);
}

function getNavigateToItems(searchValue: string, maxResultCount?: number, fileName?: string, excludeDtsFiles = false): NavigateToItem[] {
function getNavigateToItems(searchValue: string, maxResultCount?: number, fileName?: string, excludeDtsFiles = false, excludeLibFiles = false): NavigateToItem[] {
synchronizeHostData();
const sourceFiles = fileName ? [getValidSourceFile(fileName)] : program.getSourceFiles();
return NavigateTo.getNavigateToItems(sourceFiles, program.getTypeChecker(), cancellationToken, searchValue, maxResultCount, excludeDtsFiles);
return NavigateTo.getNavigateToItems(sourceFiles, program.getTypeChecker(), cancellationToken, searchValue, maxResultCount, excludeDtsFiles, excludeLibFiles);
}

function getEmitOutput(fileName: string, emitOnlyDtsFiles?: boolean, forceDtsEmit?: boolean) {
Expand Down
2 changes: 1 addition & 1 deletion src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ export interface LanguageService {
getDocumentHighlights(fileName: string, position: number, filesToSearch: string[]): DocumentHighlights[] | undefined;
getFileReferences(fileName: string): ReferenceEntry[];

getNavigateToItems(searchValue: string, maxResultCount?: number, fileName?: string, excludeDtsFiles?: boolean): NavigateToItem[];
getNavigateToItems(searchValue: string, maxResultCount?: number, fileName?: string, excludeDtsFiles?: boolean, excludeLibFiles?: boolean): NavigateToItem[];
getNavigationBarItems(fileName: string): NavigationBarItem[];
getNavigationTree(fileName: string): NavigationTree;

Expand Down
7 changes: 6 additions & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2897,6 +2897,10 @@ declare namespace ts {
* Indicates whether {@link ReferencesResponseItem.lineText} is supported.
*/
readonly disableLineTextInReferences?: boolean;
/**
* Indicates whether to exclude standard library and node_modules file symbols from navTo results.
*/
readonly excludeLibrarySymbolsInNavTo?: boolean;
}
interface CompilerOptions {
allowJs?: boolean;
Expand Down Expand Up @@ -8640,6 +8644,7 @@ declare namespace ts {
readonly organizeImportsNumericCollation?: boolean;
readonly organizeImportsAccentCollation?: boolean;
readonly organizeImportsCaseFirst?: "upper" | "lower" | false;
readonly excludeLibrarySymbolsInNavTo?: boolean;
}
/** Represents a bigint literal value without requiring bigint support */
interface PseudoBigInt {
Expand Down Expand Up @@ -10396,7 +10401,7 @@ declare namespace ts {
findReferences(fileName: string, position: number): ReferencedSymbol[] | undefined;
getDocumentHighlights(fileName: string, position: number, filesToSearch: string[]): DocumentHighlights[] | undefined;
getFileReferences(fileName: string): ReferenceEntry[];
getNavigateToItems(searchValue: string, maxResultCount?: number, fileName?: string, excludeDtsFiles?: boolean): NavigateToItem[];
getNavigateToItems(searchValue: string, maxResultCount?: number, fileName?: string, excludeDtsFiles?: boolean, excludeLibFiles?: boolean): NavigateToItem[];
getNavigationBarItems(fileName: string): NavigationBarItem[];
getNavigationTree(fileName: string): NavigationTree;
prepareCallHierarchy(fileName: string, position: number): CallHierarchyItem | CallHierarchyItem[] | undefined;
Expand Down
1 change: 1 addition & 0 deletions tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@ declare namespace FourSlashInterface {
readonly pattern: string;
readonly fileName?: string;
readonly expected: ReadonlyArray<ExpectedNavigateToItem>;
readonly excludeLibFiles?: boolean;
}
interface ExpectedNavigateToItem {
readonly name: string;
Expand Down
46 changes: 46 additions & 0 deletions tests/cases/fourslash/navto_excludeLib1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/// <reference path="fourslash.ts"/>

// @filename: /index.ts
//// import { weirdName as otherName } from "bar";
//// const [|weirdName: number = 1|];

// @filename: /tsconfig.json
//// {}

// @filename: /node_modules/bar/index.d.ts
//// export const [|weirdName: number|];

// @filename: /node_modules/bar/package.json
//// {}


const [weird, otherWeird] = test.ranges();

verify.navigateTo({
pattern: "weirdName",
expected: [{
name: "weirdName",
kind: "const",
kindModifiers: "export,declare",
range: otherWeird,
matchKind: "exact",
},
{
name: "weirdName",
kind: "const",
range: weird,
matchKind: "exact",
}],
});

verify.navigateTo({
pattern: "weirdName",
excludeLibFiles: true,
expected: [{
name: "weirdName",
kind: "const",
range: weird,
matchKind: "exact",
}],

});
32 changes: 32 additions & 0 deletions tests/cases/fourslash/navto_excludeLib2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/// <reference path="fourslash.ts"/>

// @filename: /index.ts
//// import { [|someName as weirdName|] } from "bar";

// @filename: /tsconfig.json
//// {}

// @filename: /node_modules/bar/index.d.ts
//// export const someName: number;

// @filename: /node_modules/bar/package.json
//// {}


const [weird] = test.ranges();

verify.navigateTo({
pattern: "weirdName",
expected: [{
name: "weirdName",
kind: "alias",
range: weird,
matchKind: "exact",
}],
});

verify.navigateTo({
pattern: "weirdName",
excludeLibFiles: true,
expected: [],
});
19 changes: 19 additions & 0 deletions tests/cases/fourslash/navto_excludeLib3.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/// <reference path="fourslash.ts"/>

// @filename: /index.ts
//// [|function parseInt(s: string): number {}|]

const [local] = test.ranges();

verify.navigateTo({
pattern: "parseInt",
excludeLibFiles: true,
expected: [
{
name: "parseInt",
kind: "function",
range: local,
matchKind: "exact",
},
],
});
Loading