Don't offer import completions in non-module files unless "--module" is set (#22951)

* Don't offer import completions in non-module files unless "--module" is set

* Even smarter shouldOfferImportCompletions

src/services/completions.ts

@@ -27,16 +27,9 @@ namespace ts.Completions {
 
     const enum GlobalsSearch { Continue, Success, Fail }
 
-    export function getCompletionsAtPosition(
-        host: LanguageServiceHost,
-        typeChecker: TypeChecker,
-        log: Log,
-        compilerOptions: CompilerOptions,
-        sourceFile: SourceFile,
-        position: number,
-        allSourceFiles: ReadonlyArray<SourceFile>,
-        preferences: UserPreferences,
-    ): CompletionInfo | undefined {
+    export function getCompletionsAtPosition(host: LanguageServiceHost, program: Program, log: Log, sourceFile: SourceFile, position: number, preferences: UserPreferences): CompletionInfo | undefined {
+        const typeChecker = program.getTypeChecker();
+        const compilerOptions = program.getCompilerOptions();
         if (isInReferenceComment(sourceFile, position)) {
             const entries = PathCompletions.getTripleSlashReferenceCompletion(sourceFile, position, compilerOptions, host);
             return entries && convertPathCompletions(entries);
@@ -55,7 +48,7 @@ namespace ts.Completions {
             return getLabelCompletionAtPosition(contextToken.parent);
         }
 
-        const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, preferences, compilerOptions.target);
+        const completionData = getCompletionData(program, log, sourceFile, position, preferences);
         if (!completionData) {
             return undefined;
         }
@@ -480,16 +473,9 @@ namespace ts.Completions {
         previousToken: Node;
         readonly isJsxInitializer: IsJsxInitializer;
     }
-    function getSymbolCompletionFromEntryId(
-        typeChecker: TypeChecker,
-        log: (message: string) => void,
-        compilerOptions: CompilerOptions,
-        sourceFile: SourceFile,
-        position: number,
-        { name, source }: CompletionEntryIdentifier,
-        allSourceFiles: ReadonlyArray<SourceFile>,
+    function getSymbolCompletionFromEntryId(program: Program, log: Log, sourceFile: SourceFile, position: number, { name, source }: CompletionEntryIdentifier,
     ): SymbolCompletion | { type: "request", request: Request } | { type: "none" } {
-        const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeCompletionsForModuleExports: true, includeCompletionsWithInsertText: true }, compilerOptions.target);
+        const completionData = getCompletionData(program, log, sourceFile, position, { includeCompletionsForModuleExports: true, includeCompletionsWithInsertText: true });
         if (!completionData) {
             return { type: "none" };
         }
@@ -505,7 +491,7 @@ namespace ts.Completions {
         // completion entry.
         return firstDefined<Symbol, SymbolCompletion>(symbols, (symbol): SymbolCompletion => { // TODO: Shouldn't need return type annotation (GH#12632)
             const origin = symbolToOriginInfoMap[getSymbolId(symbol)];
-            const info = getCompletionEntryDisplayNameForSymbol(symbol, compilerOptions.target, origin, completionKind);
+            const info = getCompletionEntryDisplayNameForSymbol(symbol, program.getCompilerOptions().target, origin, completionKind);
             return info && info.name === name && getSourceFromOrigin(origin) === source ? { type: "symbol" as "symbol", symbol, location, symbolToOriginInfoMap, previousToken, isJsxInitializer } : undefined;
         }) || { type: "none" };
     }
@@ -525,18 +511,17 @@ namespace ts.Completions {
 
     export function getCompletionEntryDetails(
         program: Program,
-        log: (message: string) => void,
-        compilerOptions: CompilerOptions,
+        log: Log,
         sourceFile: SourceFile,
         position: number,
         entryId: CompletionEntryIdentifier,
-        allSourceFiles: ReadonlyArray<SourceFile>,
         host: LanguageServiceHost,
         formatContext: formatting.FormatContext,
         getCanonicalFileName: GetCanonicalFileName,
         preferences: UserPreferences,
     ): CompletionEntryDetails {
         const typeChecker = program.getTypeChecker();
+        const compilerOptions = program.getCompilerOptions();
         const { name } = entryId;
 
         const contextToken = findPrecedingToken(position, sourceFile);
@@ -548,7 +533,7 @@ namespace ts.Completions {
         }
 
         // Compute all the completion symbols again.
-        const symbolCompletion = getSymbolCompletionFromEntryId(typeChecker, log, compilerOptions, sourceFile, position, entryId, allSourceFiles);
+        const symbolCompletion = getSymbolCompletionFromEntryId(program, log, sourceFile, position, entryId);
         switch (symbolCompletion.type) {
             case "request": {
                 const { request } = symbolCompletion;
@@ -565,7 +550,7 @@ namespace ts.Completions {
             }
             case "symbol": {
                 const { symbol, location, symbolToOriginInfoMap, previousToken } = symbolCompletion;
-                const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, previousToken, formatContext, getCanonicalFileName, allSourceFiles, preferences);
+                const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, previousToken, formatContext, getCanonicalFileName, program.getSourceFiles(), preferences);
                 return createCompletionDetailsForSymbol(symbol, typeChecker, sourceFile, location, codeActions, sourceDisplay);
             }
             case "none":
@@ -642,16 +627,8 @@ namespace ts.Completions {
         return { sourceDisplay: [textPart(moduleSpecifier)], codeActions: [codeAction] };
     }
 
-    export function getCompletionEntrySymbol(
-        typeChecker: TypeChecker,
-        log: (message: string) => void,
-        compilerOptions: CompilerOptions,
-        sourceFile: SourceFile,
-        position: number,
-        entryId: CompletionEntryIdentifier,
-        allSourceFiles: ReadonlyArray<SourceFile>,
-    ): Symbol | undefined {
-        const completion = getSymbolCompletionFromEntryId(typeChecker, log, compilerOptions, sourceFile, position, entryId, allSourceFiles);
+    export function getCompletionEntrySymbol(program: Program, log: Log, sourceFile: SourceFile, position: number, entryId: CompletionEntryIdentifier): Symbol | undefined {
+        const completion = getSymbolCompletionFromEntryId(program, log, sourceFile, position, entryId);
         return completion.type === "symbol" ? completion.symbol : undefined;
     }
 
@@ -760,14 +737,14 @@ namespace ts.Completions {
     }
 
     function getCompletionData(
-        typeChecker: TypeChecker,
+        program: Program,
         log: (message: string) => void,
         sourceFile: SourceFile,
         position: number,
-        allSourceFiles: ReadonlyArray<SourceFile>,
         preferences: Pick<UserPreferences, "includeCompletionsForModuleExports" | "includeCompletionsWithInsertText">,
-        target: ScriptTarget,
     ): CompletionData | Request | undefined {
+        const typeChecker = program.getTypeChecker();
+
         let start = timestamp();
         let currentToken = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); // TODO: GH#15853
         // We will check for jsdoc comments with insideComment and getJsDocTagAtPosition. (TODO: that seems rather inefficient to check the same thing so many times.)
@@ -1168,13 +1145,30 @@ namespace ts.Completions {
                 }
             }
 
-            // Don't suggest import completions for a commonjs-only module
-            if (preferences.includeCompletionsForModuleExports && !(sourceFile.commonJsModuleIndicator && !sourceFile.externalModuleIndicator)) {
-                getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "", target);
+            if (shouldOfferImportCompletions()) {
+                getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "", program.getCompilerOptions().target);
             }
             filterGlobalCompletion(symbols);
         }
 
+        function shouldOfferImportCompletions(): boolean {
+            // If not already a module, must have modules enabled and not currently be in a commonjs module. (TODO: import completions for commonjs)
+            if (!preferences.includeCompletionsForModuleExports) return false;
+            // If already using ES6 modules, OK to continue using them.
+            if (sourceFile.externalModuleIndicator) return true;
+            // If already using commonjs, don't introduce ES6.
+            if (sourceFile.commonJsModuleIndicator) return false;
+            // If some file is using ES6 modules, assume that it's OK to add more.
+            if (program.getSourceFiles().some(s => !s.isDeclarationFile && !program.isSourceFileFromExternalLibrary(s) && !!s.externalModuleIndicator)) {
+                return true;
+            }
+            // For JS, stay on the safe side.
+            if (isSourceFileJavaScript(sourceFile)) return false;
+            // If module transpilation is enabled or we're targeting es6 or above, or not emitting, OK.
+            const compilerOptions = program.getCompilerOptions();
+            return !!compilerOptions.module || compilerOptions.target >= ScriptTarget.ES2015 || !!compilerOptions.noEmit;
+        }
+
         function isSnippetScope(scopeNode: Node): boolean {
             switch (scopeNode.kind) {
                 case SyntaxKind.SourceFile:
@@ -1268,7 +1262,7 @@ namespace ts.Completions {
         function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string, target: ScriptTarget): void {
             const tokenTextLowerCase = tokenText.toLowerCase();
 
-            codefix.forEachExternalModuleToImportFrom(typeChecker, sourceFile, allSourceFiles, moduleSymbol => {
+            codefix.forEachExternalModuleToImportFrom(typeChecker, sourceFile, program.getSourceFiles(), moduleSymbol => {
                 for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) {
                     // Don't add a completion for a re-export, only for the original.
                     // The actual import fix might end up coming from a re-export -- we don't compute that until getting completion details.

src/services/services.ts

@@ -1404,12 +1404,10 @@ namespace ts {
             synchronizeHostData();
             return Completions.getCompletionsAtPosition(
                 host,
-                program.getTypeChecker(),
+                program,
                 log,
-                program.getCompilerOptions(),
                 getValidSourceFile(fileName),
                 position,
-                program.getSourceFiles(),
                 fullPreferences);
         }
 
@@ -1418,11 +1416,9 @@ namespace ts {
             return Completions.getCompletionEntryDetails(
                 program,
                 log,
-                program.getCompilerOptions(),
                 getValidSourceFile(fileName),
                 position,
                 { name, source },
-                program.getSourceFiles(),
                 host,
                 formattingOptions && formatting.getFormatContext(formattingOptions),
                 getCanonicalFileName,
@@ -1431,14 +1427,7 @@ namespace ts {
 
         function getCompletionEntrySymbol(fileName: string, position: number, name: string, source?: string): Symbol {
             synchronizeHostData();
-            return Completions.getCompletionEntrySymbol(
-                program.getTypeChecker(),
-                log,
-                program.getCompilerOptions(),
-                getValidSourceFile(fileName),
-                position,
-                { name, source },
-                program.getSourceFiles());
+            return Completions.getCompletionEntrySymbol(program, log, getValidSourceFile(fileName), position, { name, source });
         }
 
         function getQuickInfoAtPosition(fileName: string, position: number): QuickInfo {

tests/cases/fourslash/completionsImportBaseUrl.ts

@@ -3,7 +3,8 @@
 // @Filename: /tsconfig.json
 ////{
 ////    "compilerOptions": {
-////        "baseUrl": "."
+////        "baseUrl": ".",
+////        "module": "esnext"
 ////    }
 ////}
 
@@ -16,6 +17,6 @@
 // Test that it prefers a relative import (see sourceDisplay).
 goTo.marker("");
 verify.completionListContains({ name: "foo", source: "/src/a" }, "const foo: 0", "", "const", undefined, /*hasAction*/ true, {
-    includeExternalModuleExports: true,
+    includeCompletionsForModuleExports: true,
     sourceDisplay: "./a",
 });

tests/cases/fourslash/completionsImport_compilerOptionsModule.ts

@@ -0,0 +1,40 @@
+/// <reference path="fourslash.ts" />
+
+// @allowJs: true
+// @module: commonjs
+
+// @Filename: /node_modules/a/index.d.ts
+////export const foo = 0;
+
+// @Filename: /b.js
+////const a = require("./a");
+////fo/*b*/
+
+// @Filename: /c.js
+////const x = 0;/*c*/ // Off for JS files (unless a non-declaration external module exists in the project)
+
+// @Filename: /c2.ts
+////const x = 0;/*c2*/
+
+// @Filename: /d.js
+////const a = import("./a"); // Does not make this an external module
+////fo/*d*/
+
+// @Filename: /d2.ts
+////const a = import("./a"); // Does not make this an external module
+////fo/*d2*/
+
+for (const marker of ["b", "c", "d"]) {
+    goTo.marker(marker);
+    verify.not.completionListContains({ name: "foo", source: "/node_modules/a/index" }, undefined, undefined, undefined, undefined, undefined, {
+        includeCompletionsForModuleExports: true
+    });
+}
+
+for (const marker of ["c2", "d2"]) {
+    goTo.marker(marker);
+    verify.completionListContains({ name: "foo", source: "/node_modules/a/index" }, "const foo: 0", "", "const", /*spanIndex*/ undefined, /*hasAction*/ true, {
+        includeCompletionsForModuleExports: true,
+        sourceDisplay: "a",
+    });
+}

tests/cases/fourslash/completionsImport_default_anonymous.ts

@@ -1,6 +1,7 @@
 /// <reference path="fourslash.ts" />
 
 // Use `/src` to test that directory names are not included in conversion from module path to identifier.
+// @module: esnext
 
 // @Filename: /src/foo-bar.ts
 ////export default 0;

tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts

@@ -1,6 +1,6 @@
 /// <reference path="fourslash.ts" />
 
-// @noLib: true
+// @module: esnext
 
 // @Filename: /a.ts
 ////export default function foo() {}

tests/cases/fourslash/completionsImport_default_exportDefaultIdentifier.ts

@@ -2,6 +2,8 @@
 
 // Tests that we use the name "foo".
 
+// @module: esnext
+
 // @Filename: /a.ts
 ////const foo = 0;
 ////export default foo;

tests/cases/fourslash/completionsImport_fromAmbientModule.ts

@@ -1,5 +1,7 @@
 /// <reference path="fourslash.ts" />
 
+// @module: esnext
+
 // @Filename: /a.ts
 ////declare module "m" {
 ////    export const x: number;

tests/cases/fourslash/completionsImport_matching.ts

@@ -1,5 +1,7 @@
 /// <reference path="fourslash.ts" />
 
+// @module: esnext
+
 // @Filename: /a.ts
 // Not included:
 ////export function abcde() {}

tests/cases/fourslash/completionsImport_multipleWithSameName.ts

@@ -1,5 +1,7 @@
 /// <reference path="fourslash.ts" />
 
+// @module: esnext
+
 // @Filename: /global.d.ts
 // A local variable would prevent import completions (see `completionsImport_shadowedByLocal.ts`), but a global doesn't.
 ////declare var foo: number;

tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts

@@ -1,5 +1,6 @@
 /// <reference path="fourslash.ts" />
 
+
 // @Filename: /a.ts
 ////export function Test1() {}
 ////export function Test2() {}

tests/cases/fourslash/completionsImport_named_exportEqualsNamespace.ts

@@ -1,5 +1,7 @@
 /// <reference path="fourslash.ts" />
 
+// @module: esnext
+
 // @Filename: /a.d.ts
 ////declare namespace N {
 ////    export const foo = 0;

tests/cases/fourslash/completionsImport_named_exportEqualsNamespace_merged.ts

@@ -1,5 +1,7 @@
 /// <reference path="fourslash.ts" />
 
+// @module: esnext
+
 // @Filename: /b.d.ts
 ////declare namespace N {
 ////    export const foo: number;

tests/cases/fourslash/completionsImport_notFromUnrelatedNodeModules.ts

@@ -1,5 +1,7 @@
 /// <reference path="fourslash.ts" />
 
+// @module: esnext
+
 // @Filename: /unrelated/node_modules/@types/foo/index.d.ts
 ////export function foo() {}
 

tests/cases/fourslash/completionsImport_ofAlias.ts

@@ -3,6 +3,8 @@
 // Tests that we don't filter out a completion for an alias,
 // so long as it's not an alias to a different module.
 
+// @module: esnext
+
 // @Filename: /a.ts
 ////const foo = 0;
 ////export { foo };

tests/cases/fourslash/completionsImport_quoteStyle.ts

@@ -1,5 +1,7 @@
 /// <reference path="fourslash.ts" />
 
+// @module: esnext
+
 // @Filename: /a.ts
 ////export const foo = 0;
 

tests/cases/fourslash/completionsImport_reExportDefault.ts

@@ -1,5 +1,8 @@
 /// <reference path="fourslash.ts" />
 
+// @module: esnext
+// @moduleResolution: node
+
 // @Filename: /a/b/impl.ts
 ////export default function foo() {}
 

tests/cases/fourslash/completionsImport_require.ts

@@ -6,29 +6,37 @@
 ////export const foo = 0;
 
 // @Filename: /b.js
-////const a = require("./a");
+////import * as s from "something";
 ////fo/*b*/
 
 // @Filename: /c.js
-////const a = import("./a");
+////const a = require("./a");
 ////fo/*c*/
 
-goTo.marker("b");
-// Doesn't activate for commonjs-only module
-verify.not.completionListContains({ name: "foo", source: "/a" });
+// @Filename: /d.js
+////const x = 0;/*d*/
 
-goTo.marker("c");
+// @Filename: /e.js
+////const a = import("./a"); // Does not make this an external module
+////fo/*e*/
+
+for (const marker of ["c", "d", "e"]) {
+    // Doesn't activate for commonjs-only module, or non-module file unless 'module' is set see also completionsImport_compilerOptionsModule)
+    verify.not.completionListContains({ name: "foo", source: "/a" });
+}
+
+goTo.marker("b");
 verify.completionListContains({ name: "foo", source: "/a" }, "const foo: 0", "", "const", /*spanIndex*/ undefined, /*hasAction*/ true, {
-    includeExternalModuleExports: true,
+    includeCompletionsForModuleExports: true,
     sourceDisplay: "./a",
 });
 
-verify.applyCodeActionFromCompletion("c", {
+verify.applyCodeActionFromCompletion("b", {
     name: "foo",
     source: "/a",
     description: `Import 'foo' from module "./a"`,
-    newFileContent: `import { foo } from "./a";
-
-const a = import("./a");
+    newFileContent:
+`import * as s from "something";
+import { foo } from "./a";
 fo`,
 });