Skip to content

Commit f999951

Browse files
iisaduanandrewbranch
iisaduan
and
andrewbranch
authored
fix55816: exclude files with re-exports if excluded by preferences.autoImportFileExcludePatterns (microsoft#58537)
Co-authored-by: Andrew Branch <[email protected]>
1 parent fb88f02 commit f999951

12 files changed

+439
-19
lines changed

‎src/services/codefixes/importFixes.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,14 @@ import {
4747
FutureSymbolExportInfo,
4848
getAllowSyntheticDefaultImports,
4949
getBaseFileName,
50+
getDeclarationOfKind,
5051
getDefaultLikeExportInfo,
5152
getDirectoryPath,
5253
getEmitModuleKind,
5354
getEmitModuleResolutionKind,
5455
getEmitScriptTarget,
5556
getExportInfoMap,
57+
getIsFileExcluded,
5658
getMeaningFromLocation,
5759
getNameForExportedSymbol,
5860
getOutputExtension,
@@ -277,8 +279,14 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog
277279
const checker = program.getTypeChecker();
278280
const symbol = checker.getMergedSymbol(skipAlias(exportedSymbol, checker));
279281
const exportInfo = getAllExportInfoForSymbol(sourceFile, symbol, symbolName, moduleSymbol, /*preferCapitalized*/ false, program, host, preferences, cancellationToken);
282+
if (!exportInfo) {
283+
// If no exportInfo is found, this means export could not be resolved when we have filtered for autoImportFileExcludePatterns,
284+
// so we should not generate an import.
285+
Debug.assert(preferences.autoImportFileExcludePatterns?.length);
286+
return;
287+
}
280288
const useRequire = shouldUseRequire(sourceFile, program);
281-
let fix = getImportFixForSymbol(sourceFile, Debug.checkDefined(exportInfo), program, /*position*/ undefined, !!isValidTypeOnlyUseSite, useRequire, host, preferences);
289+
let fix = getImportFixForSymbol(sourceFile, exportInfo, program, /*position*/ undefined, !!isValidTypeOnlyUseSite, useRequire, host, preferences);
282290
if (fix) {
283291
const localName = tryCast(referenceImport?.name, isIdentifier)?.text ?? symbolName;
284292
if (
@@ -859,9 +867,16 @@ function codeFixActionToCodeAction({ description, changes, commands }: CodeFixAc
859867

860868
function getAllExportInfoForSymbol(importingFile: SourceFile | FutureSourceFile, symbol: Symbol, symbolName: string, moduleSymbol: Symbol, preferCapitalized: boolean, program: Program, host: LanguageServiceHost, preferences: UserPreferences, cancellationToken: CancellationToken | undefined): readonly SymbolExportInfo[] | undefined {
861869
const getChecker = createGetChecker(program, host);
870+
const isFileExcluded = preferences.autoImportFileExcludePatterns && getIsFileExcluded(host, preferences);
871+
const mergedModuleSymbol = program.getTypeChecker().getMergedSymbol(moduleSymbol);
872+
const moduleSourceFile = isFileExcluded && mergedModuleSymbol.declarations && getDeclarationOfKind(mergedModuleSymbol, SyntaxKind.SourceFile);
873+
const moduleSymbolExcluded = moduleSourceFile && isFileExcluded(moduleSourceFile as SourceFile);
862874
return getExportInfoMap(importingFile, host, program, preferences, cancellationToken)
863875
.search(importingFile.path, preferCapitalized, name => name === symbolName, info => {
864-
if (skipAlias(info[0].symbol, getChecker(info[0].isFromPackageJson)) === symbol && info.some(i => i.moduleSymbol === moduleSymbol || i.symbol.parent === moduleSymbol)) {
876+
if (
877+
getChecker(info[0].isFromPackageJson).getMergedSymbol(skipAlias(info[0].symbol, getChecker(info[0].isFromPackageJson))) === symbol
878+
&& (moduleSymbolExcluded || info.some(i => i.moduleSymbol === moduleSymbol || i.symbol.parent === moduleSymbol))
879+
) {
865880
return info;
866881
}
867882
});

‎src/services/exportInfoMap.ts

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -426,12 +426,7 @@ export function forEachExternalModuleToImportFrom(
426426
cb: (module: Symbol, moduleFile: SourceFile | undefined, program: Program, isFromPackageJson: boolean) => void,
427427
) {
428428
const useCaseSensitiveFileNames = hostUsesCaseSensitiveFileNames(host);
429-
const excludePatterns = preferences.autoImportFileExcludePatterns && mapDefined(preferences.autoImportFileExcludePatterns, spec => {
430-
// The client is expected to send rooted path specs since we don't know
431-
// what directory a relative path is relative to.
432-
const pattern = getSubPatternFromSpec(spec, "", "exclude");
433-
return pattern ? getRegexFromPattern(pattern, useCaseSensitiveFileNames) : undefined;
434-
});
429+
const excludePatterns = preferences.autoImportFileExcludePatterns && getIsExcludedPatterns(preferences, useCaseSensitiveFileNames);
435430

436431
forEachExternalModule(program.getTypeChecker(), program.getSourceFiles(), excludePatterns, host, (module, file) => cb(module, file, program, /*isFromPackageJson*/ false));
437432
const autoImportProvider = useAutoImportProvider && host.getPackageJsonAutoImportProvider?.();
@@ -451,9 +446,33 @@ export function forEachExternalModuleToImportFrom(
451446
}
452447
}
453448

449+
function getIsExcludedPatterns(preferences: UserPreferences, useCaseSensitiveFileNames: boolean) {
450+
return mapDefined(preferences.autoImportFileExcludePatterns, spec => {
451+
// The client is expected to send rooted path specs since we don't know
452+
// what directory a relative path is relative to.
453+
const pattern = getSubPatternFromSpec(spec, "", "exclude");
454+
return pattern ? getRegexFromPattern(pattern, useCaseSensitiveFileNames) : undefined;
455+
});
456+
}
457+
454458
function forEachExternalModule(checker: TypeChecker, allSourceFiles: readonly SourceFile[], excludePatterns: readonly RegExp[] | undefined, host: LanguageServiceHost, cb: (module: Symbol, sourceFile: SourceFile | undefined) => void) {
459+
const isExcluded = excludePatterns && getIsExcluded(excludePatterns, host);
460+
461+
for (const ambient of checker.getAmbientModules()) {
462+
if (!ambient.name.includes("*") && !(excludePatterns && ambient.declarations?.every(d => isExcluded!(d.getSourceFile())))) {
463+
cb(ambient, /*sourceFile*/ undefined);
464+
}
465+
}
466+
for (const sourceFile of allSourceFiles) {
467+
if (isExternalOrCommonJsModule(sourceFile) && !isExcluded?.(sourceFile)) {
468+
cb(checker.getMergedSymbol(sourceFile.symbol), sourceFile);
469+
}
470+
}
471+
}
472+
473+
function getIsExcluded(excludePatterns: readonly RegExp[], host: LanguageServiceHost) {
455474
const realpathsWithSymlinks = host.getSymlinkCache?.().getSymlinkedDirectoriesByRealpath();
456-
const isExcluded = excludePatterns && (({ fileName, path }: SourceFile) => {
475+
return (({ fileName, path }: SourceFile) => {
457476
if (excludePatterns.some(p => p.test(fileName))) return true;
458477
if (realpathsWithSymlinks?.size && pathContainsNodeModules(fileName)) {
459478
let dir = getDirectoryPath(fileName);
@@ -467,17 +486,12 @@ function forEachExternalModule(checker: TypeChecker, allSourceFiles: readonly So
467486
}
468487
return false;
469488
});
489+
}
470490

471-
for (const ambient of checker.getAmbientModules()) {
472-
if (!ambient.name.includes("*") && !(excludePatterns && ambient.declarations?.every(d => isExcluded!(d.getSourceFile())))) {
473-
cb(ambient, /*sourceFile*/ undefined);
474-
}
475-
}
476-
for (const sourceFile of allSourceFiles) {
477-
if (isExternalOrCommonJsModule(sourceFile) && !isExcluded?.(sourceFile)) {
478-
cb(checker.getMergedSymbol(sourceFile.symbol), sourceFile);
479-
}
480-
}
491+
/** @internal */
492+
export function getIsFileExcluded(host: LanguageServiceHost, preferences: UserPreferences) {
493+
if (!preferences.autoImportFileExcludePatterns) return () => false;
494+
return getIsExcluded(getIsExcludedPatterns(preferences, hostUsesCaseSensitiveFileNames(host)), host);
481495
}
482496

483497
/** @internal */

‎tests/cases/fourslash/autoImportFileExcludePatterns10.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /src/vs/test.ts
4+
//// import { Parts } from './parts';
5+
//// export class /**/Extended implements Parts {
6+
//// }
7+
8+
// @Filename: /src/vs/parts.ts
9+
//// import { Event } from '../event/event';
10+
////
11+
//// export interface Parts {
12+
//// readonly options: Event;
13+
//// }
14+
15+
// @Filename: /src/event/event.ts
16+
//// export interface Event {
17+
//// (): string;
18+
//// }
19+
20+
// @Filename: /src/thing.ts
21+
//// import { Event } from './event/event';
22+
//// export { Event };
23+
24+
// @Filename: /src/a.ts
25+
//// import './thing'
26+
//// declare module './thing' {
27+
//// interface Event {
28+
//// c: string;
29+
//// }
30+
//// }
31+
32+
33+
verify.codeFix({
34+
description: "Implement interface 'Parts'",
35+
newFileContent:
36+
`import { Event } from '../event/event';
37+
import { Parts } from './parts';
38+
export class Extended implements Parts {
39+
options: Event;
40+
}`,
41+
preferences: {
42+
autoImportFileExcludePatterns: ["src/thing.ts"],
43+
}
44+
});

‎tests/cases/fourslash/autoImportFileExcludePatterns11.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /src/vs/test.ts
4+
//// import { Parts } from './parts';
5+
//// export class /**/Extended implements Parts {
6+
//// }
7+
8+
// @Filename: /src/vs/parts.ts
9+
//// import { Event } from '../thing';
10+
//// export interface Parts {
11+
//// readonly options: Event;
12+
//// }
13+
14+
// @Filename: /src/event/event.ts
15+
//// export interface Event {
16+
//// (): string;
17+
//// }
18+
19+
// @Filename: /src/thing.ts
20+
//// import { Event } from './event/event';
21+
//// export { Event };
22+
23+
// @Filename: /src/a.ts
24+
//// import './thing'
25+
//// declare module './thing' {
26+
//// interface Event {
27+
//// c: string;
28+
//// }
29+
//// }
30+
31+
32+
verify.codeFix({
33+
description: "Implement interface 'Parts'",
34+
newFileContent:
35+
`import { Event } from '../event/event';
36+
import { Parts } from './parts';
37+
export class Extended implements Parts {
38+
options: Event;
39+
}`,
40+
preferences: {
41+
autoImportFileExcludePatterns: ["src/thing.ts"],
42+
}
43+
});

‎tests/cases/fourslash/autoImportFileExcludePatterns12.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /src/vs/test.ts
4+
//// import { Parts } from './parts';
5+
//// export class /**/Extended implements Parts {
6+
//// }
7+
8+
// @Filename: /src/vs/parts.ts
9+
//// import { Event } from '../thing';
10+
//// export interface Parts {
11+
//// readonly options: Event;
12+
//// }
13+
14+
// @Filename: /src/event/event.ts
15+
//// export interface Event {
16+
//// (): string;
17+
//// }
18+
19+
// @Filename: /src/thing.ts
20+
//// import { Event } from '../event/event';
21+
//// export { Event };
22+
23+
// @Filename: /src/a.ts
24+
//// import './thing'
25+
//// declare module './thing' {
26+
//// interface Event {
27+
//// c: string;
28+
//// }
29+
//// }
30+
31+
// In this test, `Event` is incorrectly imported in `thing.ts`
32+
verify.codeFix({
33+
description: "Implement interface 'Parts'",
34+
newFileContent:
35+
`import { Parts } from './parts';
36+
export class Extended implements Parts {
37+
options: Event;
38+
}`,
39+
preferences: {
40+
autoImportFileExcludePatterns: ["src/thing.ts"],
41+
}
42+
});

‎tests/cases/fourslash/autoImportFileExcludePatterns13.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /src/vs/test.ts
4+
//// import { Parts } from './parts';
5+
//// export class /**/Extended implements Parts {
6+
//// }
7+
8+
// @Filename: /src/vs/parts.ts
9+
//// import { Event } from '../event/event';
10+
////
11+
//// export interface Parts {
12+
//// readonly options: Event;
13+
//// }
14+
15+
// @Filename: /src/event/event.ts
16+
//// export interface Event {
17+
//// (): string;
18+
//// }
19+
20+
// @Filename: /src/thing.ts
21+
//// import { Event } from '../event/event';
22+
//// export { Event };
23+
24+
// @Filename: /src/a.ts
25+
//// import './thing'
26+
//// declare module './thing' {
27+
//// interface Event {
28+
//// c: string;
29+
//// }
30+
//// }
31+
32+
// In this test, `Event` is incorrectly imported in `thing.ts`
33+
verify.codeFix({
34+
description: "Implement interface 'Parts'",
35+
newFileContent:
36+
`import { Event } from '../event/event';
37+
import { Parts } from './parts';
38+
export class Extended implements Parts {
39+
options: Event;
40+
}`,
41+
preferences: {
42+
autoImportFileExcludePatterns: ["src/thing.ts"],
43+
}
44+
});

‎tests/cases/fourslash/autoImportFileExcludePatterns4.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /src/vs/workbench/test.ts
4+
//// import { Parts } from './parts';
5+
//// export class /**/EditorParts implements Parts { }
6+
7+
// @Filename: /src/vs/event/event.ts
8+
//// export interface Event {
9+
//// (): string;
10+
//// }
11+
12+
// @Filename: /src/vs/workbench/parts.ts
13+
//// import { Event } from '../event/event';
14+
//// export interface Parts {
15+
//// readonly options: Event;
16+
//// }
17+
18+
// @Filename: /src/vs/workbench/workbench.ts
19+
//// import { Event } from '../event/event';
20+
//// export { Event };
21+
22+
verify.codeFix({
23+
description: "Implement interface 'Parts'",
24+
newFileContent:
25+
`import { Event } from '../event/event';
26+
import { Parts } from './parts';
27+
export class EditorParts implements Parts {
28+
options: Event;
29+
}`,
30+
preferences: {
31+
autoImportFileExcludePatterns: ["src/vs/workbench/workbench.ts"],
32+
}
33+
});

‎tests/cases/fourslash/autoImportFileExcludePatterns5.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /src/vs/workbench/test.ts
4+
//// import { Parts } from './parts';
5+
//// export class /**/EditorParts implements Parts { }
6+
7+
// @Filename: /src/vs/event/event.ts
8+
//// export interface Event {
9+
//// (): string;
10+
//// }
11+
12+
// @Filename: /src/vs/workbench/parts.ts
13+
//// import { Event } from '../event/event';
14+
//// export interface Parts {
15+
//// readonly options: Event;
16+
//// }
17+
18+
// @Filename: /src/vs/workbench/workbench.ts
19+
//// import { Event } from '../event/event';
20+
//// export { Event };
21+
22+
// @Filename: /src/vs/workbench/canImport.ts
23+
//// import { Event } from '../event/event';
24+
//// export { Event };
25+
26+
verify.codeFix({
27+
description: "Implement interface 'Parts'",
28+
newFileContent:
29+
`import { Event } from './canImport';
30+
import { Parts } from './parts';
31+
export class EditorParts implements Parts {
32+
options: Event;
33+
}`,
34+
preferences: {
35+
autoImportFileExcludePatterns: ["src/vs/workbench/workbench.ts"],
36+
}
37+
});

0 commit comments

Comments
 (0)