Improve errors on module: node12 and extensionless relative imports (microsoft#46486)

* add new error + suggestions

* push down assert defined

* remove comment

* fix esm module import detection, update baselines

* add and update new tests

* fix review comments

* remove renamed baseline references

* update node modules test baselines

* fix error message, add new tests

* update old tests with new error message

Author: gabritto

src/compiler/checker.ts

@@ -1016,6 +1016,22 @@ namespace ts {
         const builtinGlobals = createSymbolTable();
         builtinGlobals.set(undefinedSymbol.escapedName, undefinedSymbol);
 
+        // Extensions suggested for path imports when module resolution is node12 or higher.
+        // The first element of each tuple is the extension a file has.
+        // The second element of each tuple is the extension that should be used in a path import.
+        // e.g. if we want to import file `foo.mts`, we should write `import {} from "./foo.mjs".
+        const suggestedExtensions: [string, string][] = [
+            [".mts", ".mjs"],
+            [".ts", ".js"],
+            [".cts", ".cjs"],
+            [".mjs", ".mjs"],
+            [".js", ".js"],
+            [".cjs", ".cjs"],
+            [".tsx", compilerOptions.jsx === JsxEmit.Preserve ? ".jsx" : ".js"],
+            [".jsx", ".jsx"],
+            [".json", ".json"],
+        ];
+
         initializeTypeChecker();
 
         return checker;
@@ -3443,7 +3459,7 @@ namespace ts {
                     (isModuleDeclaration(location) ? location : location.parent && isModuleDeclaration(location.parent) && location.parent.name === location ? location.parent : undefined)?.name ||
                     (isLiteralImportTypeNode(location) ? location : undefined)?.argument.literal;
             const mode = contextSpecifier && isStringLiteralLike(contextSpecifier) ? getModeForUsageLocation(currentSourceFile, contextSpecifier) : currentSourceFile.impliedNodeFormat;
-            const resolvedModule = getResolvedModule(currentSourceFile, moduleReference, mode)!; // TODO: GH#18217
+            const resolvedModule = getResolvedModule(currentSourceFile, moduleReference, mode);
             const resolutionDiagnostic = resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule);
             const sourceFile = resolvedModule && !resolutionDiagnostic && host.getSourceFile(resolvedModule.resolvedFileName);
             if (sourceFile) {
@@ -3489,10 +3505,10 @@ namespace ts {
             if (resolvedModule && !resolutionExtensionIsTSOrJson(resolvedModule.extension) && resolutionDiagnostic === undefined || resolutionDiagnostic === Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type) {
                 if (isForAugmentation) {
                     const diag = Diagnostics.Invalid_module_name_in_augmentation_Module_0_resolves_to_an_untyped_module_at_1_which_cannot_be_augmented;
-                    error(errorNode, diag, moduleReference, resolvedModule.resolvedFileName);
+                    error(errorNode, diag, moduleReference, resolvedModule!.resolvedFileName);
                 }
                 else {
-                    errorOnImplicitAnyModule(/*isError*/ noImplicitAny && !!moduleNotFoundError, errorNode, resolvedModule, moduleReference);
+                    errorOnImplicitAnyModule(/*isError*/ noImplicitAny && !!moduleNotFoundError, errorNode, resolvedModule!, moduleReference);
                 }
                 // Failed imports and untyped modules are both treated in an untyped manner; only difference is whether we give a diagnostic first.
                 return undefined;
@@ -3513,6 +3529,10 @@ namespace ts {
                 }
                 else {
                     const tsExtension = tryExtractTSExtension(moduleReference);
+                    const isExtensionlessRelativePathImport = pathIsRelative(moduleReference) && !hasExtension(moduleReference);
+                    const moduleResolutionKind = getEmitModuleResolutionKind(compilerOptions);
+                    const resolutionIsNode12OrNext = moduleResolutionKind === ModuleResolutionKind.Node12 ||
+                        moduleResolutionKind === ModuleResolutionKind.NodeNext;
                     if (tsExtension) {
                         const diag = Diagnostics.An_import_path_cannot_end_with_a_0_extension_Consider_importing_1_instead;
                         const importSourceWithoutExtension = removeExtension(moduleReference, tsExtension);
@@ -3532,6 +3552,18 @@ namespace ts {
                         hasJsonModuleEmitEnabled(compilerOptions)) {
                         error(errorNode, Diagnostics.Cannot_find_module_0_Consider_using_resolveJsonModule_to_import_module_with_json_extension, moduleReference);
                     }
+                    else if (mode === ModuleKind.ESNext && resolutionIsNode12OrNext && isExtensionlessRelativePathImport) {
+                        const absoluteRef = getNormalizedAbsolutePath(moduleReference, getDirectoryPath(currentSourceFile.path));
+                        const suggestedExt = suggestedExtensions.find(([actualExt, _importExt]) => host.fileExists(absoluteRef + actualExt))?.[1];
+                        if (suggestedExt) {
+                            error(errorNode,
+                                Diagnostics.Relative_import_paths_need_explicit_file_extensions_in_EcmaScript_imports_when_moduleResolution_is_node12_or_nodenext_Did_you_mean_0,
+                                moduleReference + suggestedExt);
+                        }
+                        else {
+                            error(errorNode, Diagnostics.Relative_import_paths_need_explicit_file_extensions_in_EcmaScript_imports_when_moduleResolution_is_node12_or_nodenext_Consider_adding_an_extension_to_the_import_path);
+                        }
+                    }
                     else {
                         error(errorNode, moduleNotFoundError, moduleReference);
                     }

src/compiler/diagnosticMessages.json

@@ -3345,6 +3345,14 @@
         "category": "Error",
         "code": 2833
     },
+    "Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Consider adding an extension to the import path.": {
+        "category": "Error",
+        "code": 2834
+    },
+    "Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean '{0}'?": {
+        "category": "Error",
+        "code": 2835
+    },
 
     "Import declaration '{0}' is using private name '{1}'.": {
         "category": "Error",

src/compiler/types.ts

@@ -5992,7 +5992,7 @@ namespace ts {
     export enum ModuleResolutionKind {
         Classic  = 1,
         NodeJs   = 2,
-        // Starting with node12, node's module resolver has significant departures from tranditional cjs resolution
+        // Starting with node12, node's module resolver has significant departures from traditional cjs resolution
         // to better support ecmascript modules and their use within node - more features are still being added, so
         // we can expect it to change over time, and as such, offer both a `NodeNext` moving resolution target, and a `Node12`
         // version-anchored resolution target

tests/baselines/reference/moduleResolutionWithoutExtension1.errors.txt

@@ -0,0 +1,18 @@
+/src/bar.mts(2,21): error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.mjs'?
+/src/bar.mts(3,21): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Consider adding an extension to the import path.
+
+
+==== /src/foo.mts (0 errors) ====
+    export function foo() {
+        return "";
+    }
+    
+==== /src/bar.mts (2 errors) ====
+    // Extensionless relative path ES import in an ES module
+    import { foo } from "./foo"; // should error, suggest adding ".mjs"
+                        ~~~~~~~
+!!! error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.mjs'?
+    import { baz } from "./baz"; // should error, ask for extension, no extension suggestion
+                        ~~~~~~~
+!!! error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Consider adding an extension to the import path.
+    

tests/baselines/reference/moduleResolutionWithoutExtension1.js

@@ -0,0 +1,19 @@
+//// [tests/cases/conformance/externalModules/moduleResolutionWithoutExtension1.ts] ////
+
+//// [foo.mts]
+export function foo() {
+    return "";
+}
+
+//// [bar.mts]
+// Extensionless relative path ES import in an ES module
+import { foo } from "./foo"; // should error, suggest adding ".mjs"
+import { baz } from "./baz"; // should error, ask for extension, no extension suggestion
+
+
+//// [foo.mjs]
+export function foo() {
+    return "";
+}
+//// [bar.mjs]
+export {};

tests/baselines/reference/moduleResolutionWithoutExtension1.symbols

@@ -0,0 +1,15 @@
+=== /src/foo.mts ===
+export function foo() {
+>foo : Symbol(foo, Decl(foo.mts, 0, 0))
+
+    return "";
+}
+
+=== /src/bar.mts ===
+// Extensionless relative path ES import in an ES module
+import { foo } from "./foo"; // should error, suggest adding ".mjs"
+>foo : Symbol(foo, Decl(bar.mts, 1, 8))
+
+import { baz } from "./baz"; // should error, ask for extension, no extension suggestion
+>baz : Symbol(baz, Decl(bar.mts, 2, 8))
+

tests/baselines/reference/moduleResolutionWithoutExtension1.types

@@ -0,0 +1,16 @@
+=== /src/foo.mts ===
+export function foo() {
+>foo : () => string
+
+    return "";
+>"" : ""
+}
+
+=== /src/bar.mts ===
+// Extensionless relative path ES import in an ES module
+import { foo } from "./foo"; // should error, suggest adding ".mjs"
+>foo : any
+
+import { baz } from "./baz"; // should error, ask for extension, no extension suggestion
+>baz : any
+

tests/baselines/reference/moduleResolutionWithoutExtension2.errors.txt

@@ -0,0 +1,8 @@
+/src/buzz.mts(2,22): error TS2307: Cannot find module './foo' or its corresponding type declarations.
+
+
+==== /src/buzz.mts (1 errors) ====
+    // Extensionless relative path cjs import in an ES module
+    import foo = require("./foo"); // should error, should not ask for extension
+                         ~~~~~~~
+!!! error TS2307: Cannot find module './foo' or its corresponding type declarations.

tests/baselines/reference/moduleResolutionWithoutExtension2.js

@@ -0,0 +1,6 @@
+//// [buzz.mts]
+// Extensionless relative path cjs import in an ES module
+import foo = require("./foo"); // should error, should not ask for extension
+
+//// [buzz.mjs]
+export {};

tests/baselines/reference/moduleResolutionWithoutExtension2.symbols

@@ -0,0 +1,5 @@
+=== /src/buzz.mts ===
+// Extensionless relative path cjs import in an ES module
+import foo = require("./foo"); // should error, should not ask for extension
+>foo : Symbol(foo, Decl(buzz.mts, 0, 0))
+

tests/baselines/reference/moduleResolutionWithoutExtension2.types

@@ -0,0 +1,5 @@
+=== /src/buzz.mts ===
+// Extensionless relative path cjs import in an ES module
+import foo = require("./foo"); // should error, should not ask for extension
+>foo : any
+

tests/baselines/reference/moduleResolutionWithoutExtension3.errors.txt

@@ -0,0 +1,14 @@
+/src/bar.mts(2,21): error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.jsx'?
+
+
+==== /src/foo.tsx (0 errors) ====
+    export function foo() {
+        return "";
+    }
+    
+==== /src/bar.mts (1 errors) ====
+    // Extensionless relative path ES import in an ES module
+    import { foo } from "./foo"; // should error, suggest adding ".jsx"
+                        ~~~~~~~
+!!! error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.jsx'?
+    

tests/baselines/reference/moduleResolutionWithoutExtension3.js

@@ -0,0 +1,22 @@
+//// [tests/cases/conformance/externalModules/moduleResolutionWithoutExtension3.ts] ////
+
+//// [foo.tsx]
+export function foo() {
+    return "";
+}
+
+//// [bar.mts]
+// Extensionless relative path ES import in an ES module
+import { foo } from "./foo"; // should error, suggest adding ".jsx"
+
+
+//// [foo.jsx]
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.foo = void 0;
+function foo() {
+    return "";
+}
+exports.foo = foo;
+//// [bar.mjs]
+export {};

tests/baselines/reference/moduleResolutionWithoutExtension3.symbols

@@ -0,0 +1,12 @@
+=== /src/foo.tsx ===
+export function foo() {
+>foo : Symbol(foo, Decl(foo.tsx, 0, 0))
+
+    return "";
+}
+
+=== /src/bar.mts ===
+// Extensionless relative path ES import in an ES module
+import { foo } from "./foo"; // should error, suggest adding ".jsx"
+>foo : Symbol(foo, Decl(bar.mts, 1, 8))
+

tests/baselines/reference/moduleResolutionWithoutExtension3.types

@@ -0,0 +1,13 @@
+=== /src/foo.tsx ===
+export function foo() {
+>foo : () => string
+
+    return "";
+>"" : ""
+}
+
+=== /src/bar.mts ===
+// Extensionless relative path ES import in an ES module
+import { foo } from "./foo"; // should error, suggest adding ".jsx"
+>foo : any
+

tests/baselines/reference/moduleResolutionWithoutExtension4.errors.txt

@@ -0,0 +1,14 @@
+/src/bar.mts(2,21): error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.js'?
+
+
+==== /src/foo.tsx (0 errors) ====
+    export function foo() {
+        return "";
+    }
+    
+==== /src/bar.mts (1 errors) ====
+    // Extensionless relative path ES import in an ES module
+    import { foo } from "./foo"; // should error, suggest adding ".js"
+                        ~~~~~~~
+!!! error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.js'?
+    

tests/baselines/reference/moduleResolutionWithoutExtension4.js

@@ -0,0 +1,22 @@
+//// [tests/cases/conformance/externalModules/moduleResolutionWithoutExtension4.ts] ////
+
+//// [foo.tsx]
+export function foo() {
+    return "";
+}
+
+//// [bar.mts]
+// Extensionless relative path ES import in an ES module
+import { foo } from "./foo"; // should error, suggest adding ".js"
+
+
+//// [foo.js]
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.foo = void 0;
+function foo() {
+    return "";
+}
+exports.foo = foo;
+//// [bar.mjs]
+export {};

tests/baselines/reference/moduleResolutionWithoutExtension4.symbols

@@ -0,0 +1,12 @@
+=== /src/foo.tsx ===
+export function foo() {
+>foo : Symbol(foo, Decl(foo.tsx, 0, 0))
+
+    return "";
+}
+
+=== /src/bar.mts ===
+// Extensionless relative path ES import in an ES module
+import { foo } from "./foo"; // should error, suggest adding ".js"
+>foo : Symbol(foo, Decl(bar.mts, 1, 8))
+

tests/baselines/reference/moduleResolutionWithoutExtension4.types

@@ -0,0 +1,13 @@
+=== /src/foo.tsx ===
+export function foo() {
+>foo : () => string
+
+    return "";
+>"" : ""
+}
+
+=== /src/bar.mts ===
+// Extensionless relative path ES import in an ES module
+import { foo } from "./foo"; // should error, suggest adding ".js"
+>foo : any
+

tests/baselines/reference/moduleResolutionWithoutExtension5.errors.txt

@@ -0,0 +1,8 @@
+/src/buzz.mts(2,8): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Consider adding an extension to the import path.
+
+
+==== /src/buzz.mts (1 errors) ====
+    // Extensionless relative path dynamic import in an ES module
+    import("./foo").then(x => x); // should error, ask for extension
+           ~~~~~~~
+!!! error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Consider adding an extension to the import path.

tests/baselines/reference/moduleResolutionWithoutExtension5.js

@@ -0,0 +1,7 @@
+//// [buzz.mts]
+// Extensionless relative path dynamic import in an ES module
+import("./foo").then(x => x); // should error, ask for extension
+
+//// [buzz.mjs]
+// Extensionless relative path dynamic import in an ES module
+import("./foo").then(x => x); // should error, ask for extension

tests/baselines/reference/moduleResolutionWithoutExtension5.symbols

@@ -0,0 +1,8 @@
+=== /src/buzz.mts ===
+// Extensionless relative path dynamic import in an ES module
+import("./foo").then(x => x); // should error, ask for extension
+>import("./foo").then : Symbol(Promise.then, Decl(lib.es5.d.ts, --, --))
+>then : Symbol(Promise.then, Decl(lib.es5.d.ts, --, --))
+>x : Symbol(x, Decl(buzz.mts, 1, 21))
+>x : Symbol(x, Decl(buzz.mts, 1, 21))
+

tests/baselines/reference/moduleResolutionWithoutExtension5.types

@@ -0,0 +1,12 @@
+=== /src/buzz.mts ===
+// Extensionless relative path dynamic import in an ES module
+import("./foo").then(x => x); // should error, ask for extension
+>import("./foo").then(x => x) : Promise<any>
+>import("./foo").then : <TResult1 = any, TResult2 = never>(onfulfilled?: (value: any) => TResult1 | PromiseLike<TResult1>, onrejected?: (reason: any) => TResult2 | PromiseLike<TResult2>) => Promise<TResult1 | TResult2>
+>import("./foo") : Promise<any>
+>"./foo" : "./foo"
+>then : <TResult1 = any, TResult2 = never>(onfulfilled?: (value: any) => TResult1 | PromiseLike<TResult1>, onrejected?: (reason: any) => TResult2 | PromiseLike<TResult2>) => Promise<TResult1 | TResult2>
+>x => x : (x: any) => any
+>x : any
+>x : any
+

tests/baselines/reference/moduleResolutionWithoutExtension6.errors.txt

@@ -0,0 +1,10 @@
+/src/bar.cts(4,21): error TS2307: Cannot find module './foo' or its corresponding type declarations.
+
+
+==== /src/bar.cts (1 errors) ====
+    // Extensionless relative path import statement in a cjs module
+    // Import statements are not allowed in cjs files,
+    // but other errors should not assume that they are allowed
+    import { foo } from "./foo"; // should error, should not ask for extension
+                        ~~~~~~~
+!!! error TS2307: Cannot find module './foo' or its corresponding type declarations.

tests/baselines/reference/moduleResolutionWithoutExtension6.js

@@ -0,0 +1,9 @@
+//// [bar.cts]
+// Extensionless relative path import statement in a cjs module
+// Import statements are not allowed in cjs files,
+// but other errors should not assume that they are allowed
+import { foo } from "./foo"; // should error, should not ask for extension
+
+//// [bar.cjs]
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });