Merge pull request #9646 from Microsoft/no_ts_extension

Don't allow `.ts` to appear in an import

Author: Andy

src/compiler/checker.ts

@@ -1024,7 +1024,7 @@ namespace ts {
         }
 
         function getDeclarationOfAliasSymbol(symbol: Symbol): Declaration {
-            return find(symbol.declarations, d => isAliasSymbolDeclaration(d) ? d : undefined);
+            return findMap(symbol.declarations, d => isAliasSymbolDeclaration(d) ? d : undefined);
         }
 
         function getTargetOfImportEqualsDeclaration(node: ImportEqualsDeclaration): Symbol {
@@ -1361,7 +1361,14 @@ namespace ts {
 
             if (moduleNotFoundError) {
                 // report errors only if it was requested
-                error(moduleReferenceLiteral, moduleNotFoundError, moduleName);
+                const tsExtension = tryExtractTypeScriptExtension(moduleName);
+                if (tsExtension) {
+                    const diag = Diagnostics.An_import_path_cannot_end_with_a_0_extension_Consider_importing_1_instead;
+                    error(moduleReferenceLiteral, diag, tsExtension, removeExtension(moduleName, tsExtension));
+                }
+                else {
+                    error(moduleReferenceLiteral, moduleNotFoundError, moduleName);
+                }
             }
             return undefined;
         }

src/compiler/core.ts

@@ -113,8 +113,22 @@ namespace ts {
         return undefined;
     }
 
-    /** Like `forEach`, but assumes existence of array and fails if no truthy value is found. */
-    export function find<T, U>(array: T[], callback: (element: T, index: number) => U | undefined): U {
+    /** Works like Array.prototype.find, returning `undefined` if no element satisfying the predicate is found. */
+    export function find<T>(array: T[], predicate: (element: T, index: number) => boolean): T | undefined {
+        for (let i = 0, len = array.length; i < len; i++) {
+            const value = array[i];
+            if (predicate(value, i)) {
+                return value;
+            }
+        }
+        return undefined;
+    }
+
+    /**
+     * Returns the first truthy result of `callback`, or else fails.
+     * This is like `forEach`, but never returns undefined.
+     */
+    export function findMap<T, U>(array: T[], callback: (element: T, index: number) => U | undefined): U {
         for (let i = 0, len = array.length; i < len; i++) {
             const result = callback(array[i], i);
             if (result) {
@@ -1315,6 +1329,8 @@ namespace ts {
      *  List of supported extensions in order of file resolution precedence.
      */
     export const supportedTypeScriptExtensions = [".ts", ".tsx", ".d.ts"];
+    /** Must have ".d.ts" first because if ".ts" goes first, that will be detected as the extension instead of ".d.ts". */
+    export const supportedTypescriptExtensionsForExtractExtension = [".d.ts", ".ts", ".tsx"];
     export const supportedJavascriptExtensions = [".js", ".jsx"];
     const allSupportedExtensions  = supportedTypeScriptExtensions.concat(supportedJavascriptExtensions);
 
@@ -1397,8 +1413,12 @@ namespace ts {
         return path;
     }
 
-    export function tryRemoveExtension(path: string, extension: string): string {
-        return fileExtensionIs(path, extension) ? path.substring(0, path.length - extension.length) : undefined;
+    export function tryRemoveExtension(path: string, extension: string): string | undefined {
+        return fileExtensionIs(path, extension) ? removeExtension(path, extension) : undefined;
+    }
+
+    export function removeExtension(path: string, extension: string): string {
+        return path.substring(0, path.length - extension.length);
     }
 
     export function isJsxOrTsxExtension(ext: string): boolean {

src/compiler/diagnosticMessages.json

@@ -1951,6 +1951,10 @@
         "category": "Error",
         "code": 2690
     },
+    "An import path cannot end with a '{0}' extension. Consider importing '{1}' instead.": {
+        "category": "Error",
+        "code": 2691
+    },
     "Import declaration '{0}' is using private name '{1}'.": {
         "category": "Error",
         "code": 4000

src/compiler/program.ts

@@ -661,51 +661,52 @@ namespace ts {
      * @param {boolean} onlyRecordFailures - if true then function won't try to actually load files but instead record all attempts as failures. This flag is necessary
      * in cases when we know upfront that all load attempts will fail (because containing folder does not exists) however we still need to record all failed lookup locations.
      */
-    function loadModuleFromFile(candidate: string, extensions: string[], failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string {
-        // First try to keep/add an extension: importing "./foo.ts" can be matched by a file "./foo.ts", and "./foo" by "./foo.d.ts"
-        const resolvedByAddingOrKeepingExtension = loadModuleFromFileWorker(candidate, extensions, failedLookupLocation, onlyRecordFailures, state);
-        if (resolvedByAddingOrKeepingExtension) {
-            return resolvedByAddingOrKeepingExtension;
+    function loadModuleFromFile(candidate: string, extensions: string[], failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string | undefined {
+        // First, try adding an extension. An import of "foo" could be matched by a file "foo.ts", or "foo.js" by "foo.js.ts"
+        const resolvedByAddingExtension = tryAddingExtensions(candidate, extensions, failedLookupLocation, onlyRecordFailures, state);
+        if (resolvedByAddingExtension) {
+            return resolvedByAddingExtension;
         }
-        // Then try stripping a ".js" or ".jsx" extension and replacing it with a TypeScript one, e.g. "./foo.js" can be matched by "./foo.ts" or "./foo.d.ts"
+
+        // If that didn't work, try stripping a ".js" or ".jsx" extension and replacing it with a TypeScript one;
+        // e.g. "./foo.js" can be matched by "./foo.ts" or "./foo.d.ts"
         if (hasJavaScriptFileExtension(candidate)) {
             const extensionless = removeFileExtension(candidate);
             if (state.traceEnabled) {
                 const extension = candidate.substring(extensionless.length);
                 trace(state.host, Diagnostics.File_name_0_has_a_1_extension_stripping_it, candidate, extension);
             }
-            return loadModuleFromFileWorker(extensionless, extensions, failedLookupLocation, onlyRecordFailures, state);
+            return tryAddingExtensions(extensionless, extensions, failedLookupLocation, onlyRecordFailures, state);
         }
     }
 
-    function loadModuleFromFileWorker(candidate: string, extensions: string[], failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string {
+    /** Try to return an existing file that adds one of the `extensions` to `candidate`. */
+    function tryAddingExtensions(candidate: string, extensions: string[], failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string | undefined {
         if (!onlyRecordFailures) {
             // check if containing folder exists - if it doesn't then just record failures for all supported extensions without disk probing
             const directory = getDirectoryPath(candidate);
             if (directory) {
                 onlyRecordFailures = !directoryProbablyExists(directory, state.host);
             }
         }
-        return forEach(extensions, tryLoad);
+        return forEach(extensions, ext =>
+            !(state.skipTsx && isJsxOrTsxExtension(ext)) && tryFile(candidate + ext, failedLookupLocation, onlyRecordFailures, state));
+    }
 
-        function tryLoad(ext: string): string {
-            if (state.skipTsx && isJsxOrTsxExtension(ext)) {
-                return undefined;
-            }
-            const fileName = fileExtensionIs(candidate, ext) ? candidate : candidate + ext;
-            if (!onlyRecordFailures && state.host.fileExists(fileName)) {
-                if (state.traceEnabled) {
-                    trace(state.host, Diagnostics.File_0_exist_use_it_as_a_name_resolution_result, fileName);
-                }
-                return fileName;
+    /** Return the file if it exists. */
+    function tryFile(fileName: string, failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string | undefined {
+        if (!onlyRecordFailures && state.host.fileExists(fileName)) {
+            if (state.traceEnabled) {
+                trace(state.host, Diagnostics.File_0_exist_use_it_as_a_name_resolution_result, fileName);
             }
-            else {
-                if (state.traceEnabled) {
-                    trace(state.host, Diagnostics.File_0_does_not_exist, fileName);
-                }
-                failedLookupLocation.push(fileName);
-                return undefined;
+            return fileName;
+        }
+        else {
+            if (state.traceEnabled) {
+                trace(state.host, Diagnostics.File_0_does_not_exist, fileName);
             }
+            failedLookupLocation.push(fileName);
+            return undefined;
         }
     }
 
@@ -718,7 +719,9 @@ namespace ts {
             }
             const typesFile = tryReadTypesSection(packageJsonPath, candidate, state);
             if (typesFile) {
-                const result = loadModuleFromFile(typesFile, extensions, failedLookupLocation, !directoryProbablyExists(getDirectoryPath(typesFile), state.host), state);
+                const onlyRecordFailures = !directoryProbablyExists(getDirectoryPath(typesFile), state.host);
+                // The package.json "typings" property must specify the file with extension, so just try that exact filename.
+                const result = tryFile(typesFile, failedLookupLocation, onlyRecordFailures, state);
                 if (result) {
                     return result;
                 }

src/compiler/utilities.ts

@@ -2694,6 +2694,11 @@ namespace ts {
         return forEach(supportedTypeScriptExtensions, extension => fileExtensionIs(fileName, extension));
     }
 
+    /** Return ".ts", ".d.ts", or ".tsx", if that is the extension. */
+    export function tryExtractTypeScriptExtension(fileName: string): string | undefined {
+        return find(supportedTypescriptExtensionsForExtractExtension, extension => fileExtensionIs(fileName, extension));
+    }
+
     /**
      * Replace each instance of non-ascii characters by one, two, three, or four escape sequences
      * representing the UTF-8 encoding of the character, and return the expanded char code list.

src/harness/unittests/moduleResolution.ts

@@ -446,8 +446,8 @@ export = C;
                 "/a/B/c/moduleB.ts": `import a = require("./moduleC")`,
                 "/a/B/c/moduleC.ts": "export var x",
                 "/a/B/c/moduleD.ts": `
-import a = require("./moduleA.ts");
-import b = require("./moduleB.ts");
+import a = require("./moduleA");
+import b = require("./moduleB");
                 `
             });
             test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true },  "/a/B/c", /*useCaseSensitiveFileNames*/ false, ["moduleD.ts"], [1149]);
@@ -458,8 +458,8 @@ import b = require("./moduleB.ts");
                 "/a/B/c/moduleB.ts": `import a = require("./moduleC")`,
                 "/a/B/c/moduleC.ts": "export var x",
                 "/a/B/c/moduleD.ts": `
-import a = require("./moduleA.ts");
-import b = require("./moduleB.ts");
+import a = require("./moduleA");
+import b = require("./moduleB");
                 `
             });
             test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true },  "/a/B/c", /*useCaseSensitiveFileNames*/ false, ["moduleD.ts"], []);

tests/baselines/reference/exportDefaultProperty2.js

@@ -13,7 +13,7 @@ namespace C {
 export default C.B;
 
 //// [b.ts]
-import B from "./a.ts";
+import B from "./a";
 const x: B = { c: B };
 
 
@@ -29,5 +29,5 @@ exports.__esModule = true;
 exports["default"] = C.B;
 //// [b.js]
 "use strict";
-var a_ts_1 = require("./a.ts");
-var x = { c: a_ts_1["default"] };
+var a_1 = require("./a");
+var x = { c: a_1["default"] };

tests/baselines/reference/exportDefaultProperty2.symbols

@@ -21,7 +21,7 @@ export default C.B;
 >B : Symbol(default, Decl(a.ts, 2, 9), Decl(a.ts, 5, 13))
 
 === tests/cases/compiler/b.ts ===
-import B from "./a.ts";
+import B from "./a";
 >B : Symbol(B, Decl(b.ts, 0, 6))
 
 const x: B = { c: B };

tests/baselines/reference/exportDefaultProperty2.types

@@ -21,7 +21,7 @@ export default C.B;
 >B : number
 
 === tests/cases/compiler/b.ts ===
-import B from "./a.ts";
+import B from "./a";
 >B : number
 
 const x: B = { c: B };

tests/baselines/reference/exportEqualsProperty2.js

@@ -13,7 +13,7 @@ namespace C {
 export = C.B;
 
 //// [b.ts]
-import B = require("./a.ts");
+import B = require("./a");
 const x: B = { c: B };
 
 
@@ -28,5 +28,5 @@ var C = (function () {
 module.exports = C.B;
 //// [b.js]
 "use strict";
-var B = require("./a.ts");
+var B = require("./a");
 var x = { c: B };

tests/baselines/reference/exportEqualsProperty2.symbols

@@ -1,5 +1,5 @@
 === tests/cases/compiler/b.ts ===
-import B = require("./a.ts");
+import B = require("./a");
 >B : Symbol(B, Decl(b.ts, 0, 0))
 
 const x: B = { c: B };

tests/baselines/reference/exportEqualsProperty2.types

@@ -1,5 +1,5 @@
 === tests/cases/compiler/b.ts ===
-import B = require("./a.ts");
+import B = require("./a");
 >B : number
 
 const x: B = { c: B };

tests/baselines/reference/missingFunctionImplementation2.errors.txt

@@ -4,7 +4,7 @@ tests/cases/compiler/missingFunctionImplementation2_b.ts(1,17): error TS2391: Fu
 
 ==== tests/cases/compiler/missingFunctionImplementation2_a.ts (1 errors) ====
     export {};
-    declare module "./missingFunctionImplementation2_b.ts" {
+    declare module "./missingFunctionImplementation2_b" {
       export function f(a, b): void;
                       ~
 !!! error TS2384: Overload signatures must all be ambient or non-ambient.

tests/baselines/reference/missingFunctionImplementation2.js

@@ -2,7 +2,7 @@
 
 //// [missingFunctionImplementation2_a.ts]
 export {};
-declare module "./missingFunctionImplementation2_b.ts" {
+declare module "./missingFunctionImplementation2_b" {
   export function f(a, b): void;
 }
 

tests/baselines/reference/moduleResolutionNoTs.errors.txt

@@ -0,0 +1,31 @@
+tests/cases/compiler/user.ts(1,15): error TS2691: An import path cannot end with a '.ts' extension. Consider importing './x' instead.
+tests/cases/compiler/user.ts(2,15): error TS2691: An import path cannot end with a '.tsx' extension. Consider importing './y' instead.
+tests/cases/compiler/user.ts(3,15): error TS2691: An import path cannot end with a '.d.ts' extension. Consider importing './z' instead.
+
+
+==== tests/cases/compiler/x.ts (0 errors) ====
+    export default 0;
+    
+==== tests/cases/compiler/y.tsx (0 errors) ====
+    export default 0;
+    
+==== tests/cases/compiler/z.d.ts (0 errors) ====
+    declare const x: number;
+    export default x;
+    
+==== tests/cases/compiler/user.ts (3 errors) ====
+    import x from "./x.ts";
+                  ~~~~~~~~
+!!! error TS2691: An import path cannot end with a '.ts' extension. Consider importing './x' instead.
+    import y from "./y.tsx";
+                  ~~~~~~~~~
+!!! error TS2691: An import path cannot end with a '.tsx' extension. Consider importing './y' instead.
+    import z from "./z.d.ts";
+                  ~~~~~~~~~~
+!!! error TS2691: An import path cannot end with a '.d.ts' extension. Consider importing './z' instead.
+    
+    // Making sure the suggested fixes are valid:
+    import x2 from "./x";
+    import y2 from "./y";
+    import z2 from "./z";
+    

tests/baselines/reference/moduleResolutionNoTs.js

@@ -0,0 +1,33 @@
+//// [tests/cases/compiler/moduleResolutionNoTs.ts] ////
+
+//// [x.ts]
+export default 0;
+
+//// [y.tsx]
+export default 0;
+
+//// [z.d.ts]
+declare const x: number;
+export default x;
+
+//// [user.ts]
+import x from "./x.ts";
+import y from "./y.tsx";
+import z from "./z.d.ts";
+
+// Making sure the suggested fixes are valid:
+import x2 from "./x";
+import y2 from "./y";
+import z2 from "./z";
+
+
+//// [x.js]
+"use strict";
+exports.__esModule = true;
+exports["default"] = 0;
+//// [y.js]
+"use strict";
+exports.__esModule = true;
+exports["default"] = 0;
+//// [user.js]
+"use strict";

tests/baselines/reference/moduleResolutionWithExtensions.js

@@ -8,10 +8,6 @@ export default 0;
 //// [b.ts]
 import a from './a';
 
-// Matching extension
-//// [c.ts]
-import a from './a.ts';
-
 // '.js' extension: stripped and replaced with '.ts'
 //// [d.ts]
 import a from './a.js';
@@ -36,9 +32,6 @@ exports["default"] = 0;
 // No extension: '.ts' added 
 //// [b.js]
 "use strict";
-// Matching extension 
-//// [c.js]
-"use strict";
 // '.js' extension: stripped and replaced with '.ts' 
 //// [d.js]
 "use strict";

tests/baselines/reference/moduleResolutionWithExtensions.symbols

@@ -7,11 +7,6 @@ No type information for this code.=== /src/b.ts ===
 import a from './a';
 >a : Symbol(a, Decl(b.ts, 0, 6))
 
-// Matching extension
-=== /src/c.ts ===
-import a from './a.ts';
->a : Symbol(a, Decl(c.ts, 0, 6))
-
 // '.js' extension: stripped and replaced with '.ts'
 === /src/d.ts ===
 import a from './a.js';