From 79607c16adb8b9609a8f3e27d6302d86dd141cbc Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 6 Aug 2024 14:10:15 -0700 Subject: [PATCH 1/5] Simple implementation of autoImportSpecifierExcludeRegexes --- src/compiler/moduleSpecifiers.ts | 51 +++++++++++++++---- src/compiler/types.ts | 1 + src/services/completions.ts | 11 ++-- .../autoImportSpecifierExcludeRegexes1.ts | 21 ++++++++ .../autoImportSpecifierExcludeRegexes2.ts | 25 +++++++++ .../autoImportSpecifierExcludeRegexes3.ts | 25 +++++++++ tests/cases/fourslash/fourslash.ts | 1 + 7 files changed, 121 insertions(+), 14 deletions(-) create mode 100644 tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts create mode 100644 tests/cases/fourslash/autoImportSpecifierExcludeRegexes2.ts create mode 100644 tests/cases/fourslash/autoImportSpecifierExcludeRegexes3.ts diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index f380ff56a02f6..2117a24d0213b 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -83,6 +83,7 @@ import { mapDefined, MapLike, matchPatternOrExact, + memoizeOne, min, ModuleDeclaration, ModuleKind, @@ -127,6 +128,13 @@ import { UserPreferences, } from "./_namespaces/ts.js"; +const stringToRegex = memoizeOne((pattern: string) => { + try { + return new RegExp(pattern); + } + catch {} +}); + // Used by importFixes, getEditsForFileRename, and declaration emit to synthesize import module specifiers. /** @internal */ @@ -144,11 +152,12 @@ export interface ModuleSpecifierPreferences { * @param syntaxImpliedNodeFormat Used when the import syntax implies ESM or CJS irrespective of the mode of the file. */ getAllowedEndingsInPreferredOrder(syntaxImpliedNodeFormat?: ResolutionMode): ModuleSpecifierEnding[]; + readonly excludeRegexes?: readonly string[]; } /** @internal */ export function getModuleSpecifierPreferences( - { importModuleSpecifierPreference, importModuleSpecifierEnding }: UserPreferences, + { importModuleSpecifierPreference, importModuleSpecifierEnding, autoImportSpecifierExcludeRegexes }: UserPreferences, host: Pick, compilerOptions: CompilerOptions, importingSourceFile: Pick, @@ -156,6 +165,7 @@ export function getModuleSpecifierPreferences( ): ModuleSpecifierPreferences { const filePreferredEnding = getPreferredEnding(); return { + excludeRegexes: autoImportSpecifierExcludeRegexes, relativePreference: oldImportSpecifier !== undefined ? (isExternalModuleNameRelative(oldImportSpecifier) ? RelativePreference.Relative : RelativePreference.NonRelative) : @@ -362,7 +372,13 @@ export function getModuleSpecifiersWithCacheInfo( ): ModuleSpecifierResult { let computedWithoutCache = false; const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol, checker); - if (ambient) return { kind: "ambient", moduleSpecifiers: [ambient], computedWithoutCache }; + if (ambient) { + return { + kind: "ambient", + moduleSpecifiers: !(forAutoImport && isExcludedByRegex(ambient, userPreferences.autoImportSpecifierExcludeRegexes)) ? [ambient] : emptyArray, + computedWithoutCache, + }; + } // eslint-disable-next-line prefer-const let [kind, specifiers, moduleSourceFile, modulePaths, cache] = tryGetModuleSpecifiersFromCacheWorker( @@ -459,11 +475,13 @@ function computeModuleSpecifiers( const specifier = modulePath.isInNodeModules ? tryGetModuleNameAsNodeModule(modulePath, info, importingSourceFile, host, compilerOptions, userPreferences, /*packageNameOnly*/ undefined, options.overrideImportMode) : undefined; - nodeModulesSpecifiers = append(nodeModulesSpecifiers, specifier); - if (specifier && modulePath.isRedirect) { - // If we got a specifier for a redirect, it was a bare package specifier (e.g. "@foo/bar", - // not "@foo/bar/path/to/file"). No other specifier will be this good, so stop looking. - return { kind: "node_modules", moduleSpecifiers: nodeModulesSpecifiers!, computedWithoutCache: true }; + if (specifier && !(forAutoImport && isExcludedByRegex(specifier, preferences.excludeRegexes))) { + nodeModulesSpecifiers = append(nodeModulesSpecifiers, specifier); + if (modulePath.isRedirect) { + // If we got a specifier for a redirect, it was a bare package specifier (e.g. "@foo/bar", + // not "@foo/bar/path/to/file"). No other specifier will be this good, so stop looking. + return { kind: "node_modules", moduleSpecifiers: nodeModulesSpecifiers, computedWithoutCache: true }; + } } if (!specifier) { @@ -476,7 +494,7 @@ function computeModuleSpecifiers( preferences, /*pathsOnly*/ modulePath.isRedirect, ); - if (!local) { + if (!local || forAutoImport && isExcludedByRegex(local, preferences.excludeRegexes)) { continue; } if (modulePath.isRedirect) { @@ -512,7 +530,11 @@ function computeModuleSpecifiers( return pathsSpecifiers?.length ? { kind: "paths", moduleSpecifiers: pathsSpecifiers, computedWithoutCache: true } : redirectPathsSpecifiers?.length ? { kind: "redirect", moduleSpecifiers: redirectPathsSpecifiers, computedWithoutCache: true } : nodeModulesSpecifiers?.length ? { kind: "node_modules", moduleSpecifiers: nodeModulesSpecifiers, computedWithoutCache: true } : - { kind: "relative", moduleSpecifiers: Debug.checkDefined(relativeSpecifiers), computedWithoutCache: true }; + { kind: "relative", moduleSpecifiers: relativeSpecifiers ?? emptyArray, computedWithoutCache: true }; +} + +function isExcludedByRegex(moduleSpecifier: string, excludeRegexes: readonly string[] | undefined): boolean { + return some(excludeRegexes, pattern => !!stringToRegex(pattern)?.test(moduleSpecifier)); } interface Info { @@ -536,7 +558,7 @@ function getInfo(importingSourceFileName: string, host: ModuleSpecifierResolutio function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOptions: CompilerOptions, host: ModuleSpecifierResolutionHost, importMode: ResolutionMode, preferences: ModuleSpecifierPreferences): string; function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOptions: CompilerOptions, host: ModuleSpecifierResolutionHost, importMode: ResolutionMode, preferences: ModuleSpecifierPreferences, pathsOnly?: boolean): string | undefined; -function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOptions: CompilerOptions, host: ModuleSpecifierResolutionHost, importMode: ResolutionMode, { getAllowedEndingsInPreferredOrder: getAllowedEndingsInPrefererredOrder, relativePreference }: ModuleSpecifierPreferences, pathsOnly?: boolean): string | undefined { +function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOptions: CompilerOptions, host: ModuleSpecifierResolutionHost, importMode: ResolutionMode, { getAllowedEndingsInPreferredOrder: getAllowedEndingsInPrefererredOrder, relativePreference, excludeRegexes }: ModuleSpecifierPreferences, pathsOnly?: boolean): string | undefined { const { baseUrl, paths, rootDirs } = compilerOptions; if (pathsOnly && !paths) { return undefined; @@ -568,6 +590,15 @@ function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOpt return relativePath; } + const relativeIsExcluded = isExcludedByRegex(relativePath, excludeRegexes); + const nonRelativeIsExcluded = isExcludedByRegex(maybeNonRelative, excludeRegexes); + if (!relativeIsExcluded && nonRelativeIsExcluded) { + return relativePath; + } + if (relativeIsExcluded && !nonRelativeIsExcluded) { + return maybeNonRelative; + } + if (relativePreference === RelativePreference.NonRelative && !pathIsRelative(maybeNonRelative)) { return maybeNonRelative; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 564fa64e1294c..c1370af817801 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -10251,6 +10251,7 @@ export interface UserPreferences { readonly interactiveInlayHints?: boolean; readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: string[]; + readonly autoImportSpecifierExcludeRegexes?: string[]; readonly preferTypeOnlyAutoImports?: boolean; /** * Indicates whether imports should be organized in a case-insensitive manner. diff --git a/src/services/completions.ts b/src/services/completions.ts index 1300005c8b570..fa2ef144bfaba 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -84,7 +84,6 @@ import { getEffectiveBaseTypeNode, getEffectiveModifierFlags, getEffectiveTypeAnnotationNode, - getEmitModuleResolutionKind, getEmitScriptTarget, getEscapedTextOfIdentifierOrLiteral, getEscapedTextOfJsxAttributeName, @@ -106,6 +105,7 @@ import { getPropertyNameForPropertyNameNode, getQuotePreference, getReplacementSpanForContextToken, + getResolvePackageJsonExports, getRootDeclaration, getSourceFileOfModule, getSwitchedType, @@ -301,7 +301,6 @@ import { ModuleDeclaration, moduleExportNameTextEscaped, ModuleReference, - moduleResolutionSupportsPackageJsonExportsAndImports, NamedImportBindings, newCaseClauseTracker, Node, @@ -629,12 +628,16 @@ function resolvingModuleSpecifiers( cb: (context: ModuleSpecifierResolutionContext) => TReturn, ): TReturn { const start = timestamp(); - // Under `--moduleResolution nodenext`, we have to resolve module specifiers up front, because + // Under `--moduleResolution nodenext` or `bundler`, we have to resolve module specifiers up front, because // package.json exports can mean we *can't* resolve a module specifier (that doesn't include a // relative path into node_modules), and we want to filter those completions out entirely. // Import statement completions always need specifier resolution because the module specifier is // part of their `insertText`, not the `codeActions` creating edits away from the cursor. - const needsFullResolution = isForImportStatementCompletion || moduleResolutionSupportsPackageJsonExportsAndImports(getEmitModuleResolutionKind(program.getCompilerOptions())); + // Finally, `autoImportSpecifierExcludeRegexes` necessitates eagerly resolving module specifiers + // because completion items are being explcitly filtered out by module specifier. + const needsFullResolution = isForImportStatementCompletion + || getResolvePackageJsonExports(program.getCompilerOptions()) + || preferences.autoImportSpecifierExcludeRegexes?.length; let skippedAny = false; let ambientCount = 0; let resolvedCount = 0; diff --git a/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts b/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts new file mode 100644 index 0000000000000..b161f7bb301dc --- /dev/null +++ b/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts @@ -0,0 +1,21 @@ +/// + +// @module: preserve + +// @Filename: /node_modules/lib/index.d.ts +//// declare module "ambient" { +//// export const x: number; +//// } +//// declare module "ambient/utils" { +//// export const x: number; +//// } + +// @Filename: /index.ts +//// x/**/ + +verify.importFixModuleSpecifiers("", ["ambient", "ambient/utils"]); +verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRegexes: ["utils"] }); +verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRegexes: ["/.*?$"]}); +verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRegexes: ["^ambient/"] }); +verify.importFixModuleSpecifiers("", ["ambient/utils"], { autoImportSpecifierExcludeRegexes: ["ambient$"] }); +verify.importFixModuleSpecifiers("", ["ambient", "ambient/utils"], { autoImportSpecifierExcludeRegexes: ["oops("] }); \ No newline at end of file diff --git a/tests/cases/fourslash/autoImportSpecifierExcludeRegexes2.ts b/tests/cases/fourslash/autoImportSpecifierExcludeRegexes2.ts new file mode 100644 index 0000000000000..37ab15c5c36b6 --- /dev/null +++ b/tests/cases/fourslash/autoImportSpecifierExcludeRegexes2.ts @@ -0,0 +1,25 @@ +/// + +// @Filename: /tsconfig.json +//// { +//// "compilerOptions": { +//// "module": "preserve", +//// "paths": { +//// "@app/*": ["./src/*"] +//// } +//// } +//// } + +// @Filename: /src/utils.ts +//// export function add(a: number, b: number) {} + +// @Filename: /src/index.ts +//// add/**/ + +verify.importFixModuleSpecifiers("", ["./utils"]); +verify.importFixModuleSpecifiers("", ["@app/utils"], { autoImportSpecifierExcludeRegexes: ["^\\./"] }); + +verify.importFixModuleSpecifiers("", ["@app/utils"], { importModuleSpecifierPreference: "non-relative" }); +verify.importFixModuleSpecifiers("", ["./utils"], { importModuleSpecifierPreference: "non-relative", autoImportSpecifierExcludeRegexes: ["^@app/"] }); + +verify.importFixModuleSpecifiers("", [], { autoImportSpecifierExcludeRegexes: ["utils"] }); diff --git a/tests/cases/fourslash/autoImportSpecifierExcludeRegexes3.ts b/tests/cases/fourslash/autoImportSpecifierExcludeRegexes3.ts new file mode 100644 index 0000000000000..c4cc27c871b34 --- /dev/null +++ b/tests/cases/fourslash/autoImportSpecifierExcludeRegexes3.ts @@ -0,0 +1,25 @@ +/// + +// @module: preserve + +// @Filename: /node_modules/pkg/package.json +//// { +//// "name": "pkg", +//// "version": "1.0.0", +//// "exports": { +//// ".": "./index.js", +//// "./utils": "./utils.js" +//// } +//// } + +// @Filename: /node_modules/pkg/utils.d.ts +//// export function add(a: number, b: number) {} + +// @Filename: /node_modules/pkg/index.d.ts +//// export * from "./utils"; + +// @Filename: /src/index.ts +//// add/**/ + +verify.importFixModuleSpecifiers("", ["pkg", "pkg/utils"]); +verify.importFixModuleSpecifiers("", ["pkg/utils"], { autoImportSpecifierExcludeRegexes: ["^pkg$"] }); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index c13f27a807ac4..6fa6907a9c0a6 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -686,6 +686,7 @@ declare namespace FourSlashInterface { readonly providePrefixAndSuffixTextForRename?: boolean; readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: readonly string[]; + readonly autoImportSpecifierExcludeRegexes?: readonly string[]; readonly preferTypeOnlyAutoImports?: boolean; readonly organizeImportsIgnoreCase?: "auto" | boolean; readonly organizeImportsCollation?: "unicode" | "ordinal"; From ef8341a090d8b29845ce6e2cb1060a0a28a9d43f Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 6 Aug 2024 17:01:10 -0700 Subject: [PATCH 2/5] Add completions test --- .../autoImportSpecifierExcludeRegexes1.ts | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts b/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts index b161f7bb301dc..f82a2ff1f2bdf 100644 --- a/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts +++ b/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts @@ -18,4 +18,35 @@ verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRe verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRegexes: ["/.*?$"]}); verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRegexes: ["^ambient/"] }); verify.importFixModuleSpecifiers("", ["ambient/utils"], { autoImportSpecifierExcludeRegexes: ["ambient$"] }); -verify.importFixModuleSpecifiers("", ["ambient", "ambient/utils"], { autoImportSpecifierExcludeRegexes: ["oops("] }); \ No newline at end of file +verify.importFixModuleSpecifiers("", ["ambient", "ambient/utils"], { autoImportSpecifierExcludeRegexes: ["oops("] }); + +verify.completions({ + marker: "", + includes: [{ + name: "x", + source: "ambient", + sourceDisplay: "ambient", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions + }, { + name: "x", + source: "ambient/utils", + sourceDisplay: "ambient/utils", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions + }], + preferences: { + includeCompletionsForModuleExports: true, + allowIncompleteCompletions: true + } +}); + +verify.completions({ + marker: "", + excludes: ["ambient/utils"], + preferences: { + includeCompletionsForModuleExports: true, + allowIncompleteCompletions: true, + autoImportSpecifierExcludeRegexes: ["utils"] + }, +}) \ No newline at end of file From a86fef6fb6e1439915b9cb406ebbf845cf9b4471 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 6 Aug 2024 17:11:53 -0700 Subject: [PATCH 3/5] Update API baseline --- tests/baselines/reference/api/typescript.d.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 8275e4222820c..8f5b0ad80d48c 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -8240,6 +8240,7 @@ declare namespace ts { readonly interactiveInlayHints?: boolean; readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: string[]; + readonly autoImportSpecifierExcludeRegexes?: string[]; readonly preferTypeOnlyAutoImports?: boolean; /** * Indicates whether imports should be organized in a case-insensitive manner. From 941c1cf0182a0bdd371c432f3ecd9fda187470da Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 6 Aug 2024 17:12:54 -0700 Subject: [PATCH 4/5] Fix lint --- src/compiler/moduleSpecifiers.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 2117a24d0213b..4b40a494881ae 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -132,7 +132,9 @@ const stringToRegex = memoizeOne((pattern: string) => { try { return new RegExp(pattern); } - catch {} + catch { + return undefined; + } }); // Used by importFixes, getEditsForFileRename, and declaration emit to synthesize import module specifiers. From e38941b53c85731d09d994a49123cbef1b889c4c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 8 Aug 2024 16:51:12 -0700 Subject: [PATCH 5/5] Allow patterns to be given as regex literals with slashes and flags --- src/compiler/moduleSpecifiers.ts | 21 ++++++++++++++++++- .../autoImportSpecifierExcludeRegexes1.ts | 9 ++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 4b40a494881ae..f193aeb6784f1 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -130,7 +130,26 @@ import { const stringToRegex = memoizeOne((pattern: string) => { try { - return new RegExp(pattern); + let slash = pattern.indexOf("/"); + if (slash !== 0) { + // No leading slash, treat as a pattern + return new RegExp(pattern); + } + const lastSlash = pattern.lastIndexOf("/"); + if (slash === lastSlash) { + // Only one slash, treat as a pattern + return new RegExp(pattern); + } + while ((slash = pattern.indexOf("/", slash + 1)) !== lastSlash) { + if (pattern[slash - 1] !== "\\") { + // Unescaped middle slash, treat as a pattern + return new RegExp(pattern); + } + } + // Only case-insensitive and unicode flags make sense + const flags = pattern.substring(lastSlash + 1).replace(/[^iu]/g, ""); + pattern = pattern.substring(1, lastSlash); + return new RegExp(pattern, flags); } catch { return undefined; diff --git a/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts b/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts index f82a2ff1f2bdf..f3eff409abf54 100644 --- a/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts +++ b/tests/cases/fourslash/autoImportSpecifierExcludeRegexes1.ts @@ -15,7 +15,16 @@ verify.importFixModuleSpecifiers("", ["ambient", "ambient/utils"]); verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRegexes: ["utils"] }); +// case sensitive, no match +verify.importFixModuleSpecifiers("", ["ambient", "ambient/utils"], { autoImportSpecifierExcludeRegexes: ["/UTILS/"] }); +// case insensitive flag given +verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRegexes: ["/UTILS/i"] }); +// invalid due to unescaped slash, treated as pattern +verify.importFixModuleSpecifiers("", ["ambient", "ambient/utils"], { autoImportSpecifierExcludeRegexes: ["/ambient/utils/"] }); +verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRegexes: ["/ambient\\/utils/"] }); +// no trailing slash, treated as pattern, slash doesn't need to be escaped verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRegexes: ["/.*?$"]}); +// no leading slash, treated as pattern, slash doesn't need to be escaped verify.importFixModuleSpecifiers("", ["ambient"], { autoImportSpecifierExcludeRegexes: ["^ambient/"] }); verify.importFixModuleSpecifiers("", ["ambient/utils"], { autoImportSpecifierExcludeRegexes: ["ambient$"] }); verify.importFixModuleSpecifiers("", ["ambient", "ambient/utils"], { autoImportSpecifierExcludeRegexes: ["oops("] });