From 21f4a16ff0092e998eb80428fb30fca675b5757c Mon Sep 17 00:00:00 2001
From: Andy Hanson <anhans@microsoft.com>
Date: Wed, 28 Mar 2018 09:02:50 -0700
Subject: [PATCH 1/2] Don't offer import completions in non-module files unless
 "--module" is set

---
 src/services/completions.ts                   | 16 ++++++----
 .../fourslash/completionsImportBaseUrl.ts     |  5 ++--
 ...completionsImport_compilerOptionsModule.ts | 30 +++++++++++++++++++
 .../completionsImport_default_anonymous.ts    |  1 +
 ...letionsImport_default_didNotExistBefore.ts |  2 +-
 ...sImport_default_exportDefaultIdentifier.ts |  2 ++
 .../completionsImport_fromAmbientModule.ts    |  2 ++
 .../fourslash/completionsImport_matching.ts   |  2 ++
 .../completionsImport_multipleWithSameName.ts |  2 ++
 ...mpletionsImport_named_didNotExistBefore.ts |  1 +
 ...tionsImport_named_exportEqualsNamespace.ts |  2 ++
 ...port_named_exportEqualsNamespace_merged.ts |  2 ++
 ...tionsImport_notFromUnrelatedNodeModules.ts |  2 ++
 .../fourslash/completionsImport_ofAlias.ts    |  2 ++
 .../fourslash/completionsImport_quoteStyle.ts |  2 ++
 .../completionsImport_reExportDefault.ts      |  3 ++
 .../fourslash/completionsImport_require.ts    | 30 ++++++++++++-------
 17 files changed, 86 insertions(+), 20 deletions(-)
 create mode 100644 tests/cases/fourslash/completionsImport_compilerOptionsModule.ts

diff --git a/src/services/completions.ts b/src/services/completions.ts
index e7bbb47513f74..79d5639f10bd0 100644
--- a/src/services/completions.ts
+++ b/src/services/completions.ts
@@ -55,7 +55,7 @@ namespace ts.Completions {
             return getLabelCompletionAtPosition(contextToken.parent);
         }
 
-        const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, preferences, compilerOptions.target);
+        const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, preferences, compilerOptions);
         if (!completionData) {
             return undefined;
         }
@@ -490,7 +490,7 @@ namespace ts.Completions {
         { name, source }: CompletionEntryIdentifier,
         allSourceFiles: ReadonlyArray<SourceFile>,
     ): SymbolCompletion | { type: "request", request: Request } | { type: "none" } {
-        const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeCompletionsForModuleExports: true, includeCompletionsWithInsertText: true }, compilerOptions.target);
+        const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeCompletionsForModuleExports: true, includeCompletionsWithInsertText: true }, compilerOptions);
         if (!completionData) {
             return { type: "none" };
         }
@@ -767,7 +767,7 @@ namespace ts.Completions {
         position: number,
         allSourceFiles: ReadonlyArray<SourceFile>,
         preferences: Pick<UserPreferences, "includeCompletionsForModuleExports" | "includeCompletionsWithInsertText">,
-        target: ScriptTarget,
+        compilerOptions: CompilerOptions,
     ): CompletionData | Request | undefined {
         let start = timestamp();
         let currentToken = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); // TODO: GH#15853
@@ -1169,13 +1169,17 @@ 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 : "", compilerOptions.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)
+            return preferences.includeCompletionsForModuleExports && (!!sourceFile.externalModuleIndicator || !sourceFile.commonJsModuleIndicator && !!compilerOptions.module);
+        }
+
         function isSnippetScope(scopeNode: Node): boolean {
             switch (scopeNode.kind) {
                 case SyntaxKind.SourceFile:
diff --git a/tests/cases/fourslash/completionsImportBaseUrl.ts b/tests/cases/fourslash/completionsImportBaseUrl.ts
index e5f5b07b9f4f7..372dd86d4e969 100644
--- a/tests/cases/fourslash/completionsImportBaseUrl.ts
+++ b/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",
 });
diff --git a/tests/cases/fourslash/completionsImport_compilerOptionsModule.ts b/tests/cases/fourslash/completionsImport_compilerOptionsModule.ts
new file mode 100644
index 0000000000000..5d69a6bb208b2
--- /dev/null
+++ b/tests/cases/fourslash/completionsImport_compilerOptionsModule.ts
@@ -0,0 +1,30 @@
+/// <reference path="fourslash.ts" />
+
+// @allowJs: true
+// @module: commonjs
+
+// @Filename: /a.ts
+////export const foo = 0;
+
+// @Filename: /b.js
+////const a = require("./a");
+////fo/*b*/
+
+// @Filename: /c.js
+////const x = 0;/*c*/
+
+// @Filename: /d.js
+////const a = import("./a"); // Does not make this an external module
+////fo/*d*/
+
+goTo.marker("b");
+verify.completionListContains("x");
+verify.not.completionListContains({ name: "foo", source: "/a" });
+
+for (const marker of ["c", "d"]) {
+    goTo.marker(marker);
+    verify.completionListContains({ name: "foo", source: "/a" }, "const foo: 0", "", "const", /*spanIndex*/ undefined, /*hasAction*/ true, {
+        includeCompletionsForModuleExports: true,
+        sourceDisplay: "./a",
+    });
+}
diff --git a/tests/cases/fourslash/completionsImport_default_anonymous.ts b/tests/cases/fourslash/completionsImport_default_anonymous.ts
index ca9d95799010f..bf8f209f86651 100644
--- a/tests/cases/fourslash/completionsImport_default_anonymous.ts
+++ b/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;
diff --git a/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts b/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts
index 0e9edb4d1b253..814366e0e6c47 100644
--- a/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts
+++ b/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() {}
diff --git a/tests/cases/fourslash/completionsImport_default_exportDefaultIdentifier.ts b/tests/cases/fourslash/completionsImport_default_exportDefaultIdentifier.ts
index 45cdf71156eb4..a120c8435f856 100644
--- a/tests/cases/fourslash/completionsImport_default_exportDefaultIdentifier.ts
+++ b/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;
diff --git a/tests/cases/fourslash/completionsImport_fromAmbientModule.ts b/tests/cases/fourslash/completionsImport_fromAmbientModule.ts
index 43789dea0950d..afbf3a254c4bd 100644
--- a/tests/cases/fourslash/completionsImport_fromAmbientModule.ts
+++ b/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;
diff --git a/tests/cases/fourslash/completionsImport_matching.ts b/tests/cases/fourslash/completionsImport_matching.ts
index d8979e18bd9e6..444831bd9c788 100644
--- a/tests/cases/fourslash/completionsImport_matching.ts
+++ b/tests/cases/fourslash/completionsImport_matching.ts
@@ -1,5 +1,7 @@
 /// <reference path="fourslash.ts" />
 
+// @module: esnext
+
 // @Filename: /a.ts
 // Not included:
 ////export function abcde() {}
diff --git a/tests/cases/fourslash/completionsImport_multipleWithSameName.ts b/tests/cases/fourslash/completionsImport_multipleWithSameName.ts
index da118826235e9..e6f2565d5c197 100644
--- a/tests/cases/fourslash/completionsImport_multipleWithSameName.ts
+++ b/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;
diff --git a/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts b/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts
index a69c0ab19ca86..5eaecd7b5571d 100644
--- a/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts
+++ b/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts
@@ -1,5 +1,6 @@
 /// <reference path="fourslash.ts" />
 
+
 // @Filename: /a.ts
 ////export function Test1() {}
 ////export function Test2() {}
diff --git a/tests/cases/fourslash/completionsImport_named_exportEqualsNamespace.ts b/tests/cases/fourslash/completionsImport_named_exportEqualsNamespace.ts
index 788c9695e4588..b9b9b2b3ced0c 100644
--- a/tests/cases/fourslash/completionsImport_named_exportEqualsNamespace.ts
+++ b/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;
diff --git a/tests/cases/fourslash/completionsImport_named_exportEqualsNamespace_merged.ts b/tests/cases/fourslash/completionsImport_named_exportEqualsNamespace_merged.ts
index 55b33c22114cb..ba9f5675a1d7e 100644
--- a/tests/cases/fourslash/completionsImport_named_exportEqualsNamespace_merged.ts
+++ b/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;
diff --git a/tests/cases/fourslash/completionsImport_notFromUnrelatedNodeModules.ts b/tests/cases/fourslash/completionsImport_notFromUnrelatedNodeModules.ts
index 44392f8b87d60..72fdd16dc191f 100644
--- a/tests/cases/fourslash/completionsImport_notFromUnrelatedNodeModules.ts
+++ b/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() {}
 
diff --git a/tests/cases/fourslash/completionsImport_ofAlias.ts b/tests/cases/fourslash/completionsImport_ofAlias.ts
index 3951a93a57fc7..903f724722e79 100644
--- a/tests/cases/fourslash/completionsImport_ofAlias.ts
+++ b/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 };
diff --git a/tests/cases/fourslash/completionsImport_quoteStyle.ts b/tests/cases/fourslash/completionsImport_quoteStyle.ts
index cd3cc805daaa6..282a3fe94eb2c 100644
--- a/tests/cases/fourslash/completionsImport_quoteStyle.ts
+++ b/tests/cases/fourslash/completionsImport_quoteStyle.ts
@@ -1,5 +1,7 @@
 /// <reference path="fourslash.ts" />
 
+// @module: esnext
+
 // @Filename: /a.ts
 ////export const foo = 0;
 
diff --git a/tests/cases/fourslash/completionsImport_reExportDefault.ts b/tests/cases/fourslash/completionsImport_reExportDefault.ts
index 1ca24233bd360..b5c93e9dc0a55 100644
--- a/tests/cases/fourslash/completionsImport_reExportDefault.ts
+++ b/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() {}
 
diff --git a/tests/cases/fourslash/completionsImport_require.ts b/tests/cases/fourslash/completionsImport_require.ts
index 5ad80d35e3c9d..e61cdafbd944e 100644
--- a/tests/cases/fourslash/completionsImport_require.ts
+++ b/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*/
+
+// @Filename: /e.js
+////const a = import("./a"); // Does not make this an external module
+////fo/*e*/
 
-goTo.marker("c");
+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`,
 });

From d88b884273ca93bd326289b84390bc2423df30f6 Mon Sep 17 00:00:00 2001
From: Andy Hanson <anhans@microsoft.com>
Date: Fri, 30 Mar 2018 15:16:37 -0700
Subject: [PATCH 2/2] Even smarter shouldOfferImportCompletions

---
 src/services/completions.ts                   | 74 ++++++++-----------
 src/services/services.ts                      | 15 +---
 ...completionsImport_compilerOptionsModule.ts | 26 +++++--
 3 files changed, 52 insertions(+), 63 deletions(-)

diff --git a/src/services/completions.ts b/src/services/completions.ts
index 02d2a0ac50d63..1fcd947e5bb79 100644
--- a/src/services/completions.ts
+++ b/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);
+        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);
+        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">,
-        compilerOptions: CompilerOptions,
     ): 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.)
@@ -1169,14 +1146,27 @@ namespace ts.Completions {
             }
 
             if (shouldOfferImportCompletions()) {
-                getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "", compilerOptions.target);
+                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)
-            return preferences.includeCompletionsForModuleExports && (!!sourceFile.externalModuleIndicator || !sourceFile.commonJsModuleIndicator && !!compilerOptions.module);
+            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 {
@@ -1272,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.
diff --git a/src/services/services.ts b/src/services/services.ts
index dbab3436ec306..59f8fd3067849 100644
--- a/src/services/services.ts
+++ b/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 {
diff --git a/tests/cases/fourslash/completionsImport_compilerOptionsModule.ts b/tests/cases/fourslash/completionsImport_compilerOptionsModule.ts
index 5d69a6bb208b2..be568bb4212cf 100644
--- a/tests/cases/fourslash/completionsImport_compilerOptionsModule.ts
+++ b/tests/cases/fourslash/completionsImport_compilerOptionsModule.ts
@@ -3,7 +3,7 @@
 // @allowJs: true
 // @module: commonjs
 
-// @Filename: /a.ts
+// @Filename: /node_modules/a/index.d.ts
 ////export const foo = 0;
 
 // @Filename: /b.js
@@ -11,20 +11,30 @@
 ////fo/*b*/
 
 // @Filename: /c.js
-////const x = 0;/*c*/
+////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*/
 
-goTo.marker("b");
-verify.completionListContains("x");
-verify.not.completionListContains({ name: "foo", source: "/a" });
+// @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 ["c", "d"]) {
+for (const marker of ["c2", "d2"]) {
     goTo.marker(marker);
-    verify.completionListContains({ name: "foo", source: "/a" }, "const foo: 0", "", "const", /*spanIndex*/ undefined, /*hasAction*/ true, {
+    verify.completionListContains({ name: "foo", source: "/node_modules/a/index" }, "const foo: 0", "", "const", /*spanIndex*/ undefined, /*hasAction*/ true, {
         includeCompletionsForModuleExports: true,
-        sourceDisplay: "./a",
+        sourceDisplay: "a",
     });
 }