Skip to content

Commit d2d5d42

Browse files
author
Andy
authored
Merge pull request microsoft#9646 from Microsoft/no_ts_extension
Don't allow `.ts` to appear in an import
2 parents 19cde06 + b452469 commit d2d5d42

28 files changed

Lines changed: 175 additions & 80 deletions

‎src/compiler/checker.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,7 @@ namespace ts {
10241024
}
10251025

10261026
function getDeclarationOfAliasSymbol(symbol: Symbol): Declaration {
1027-
return find(symbol.declarations, d => isAliasSymbolDeclaration(d) ? d : undefined);
1027+
return findMap(symbol.declarations, d => isAliasSymbolDeclaration(d) ? d : undefined);
10281028
}
10291029

10301030
function getTargetOfImportEqualsDeclaration(node: ImportEqualsDeclaration): Symbol {
@@ -1361,7 +1361,14 @@ namespace ts {
13611361

13621362
if (moduleNotFoundError) {
13631363
// report errors only if it was requested
1364-
error(moduleReferenceLiteral, moduleNotFoundError, moduleName);
1364+
const tsExtension = tryExtractTypeScriptExtension(moduleName);
1365+
if (tsExtension) {
1366+
const diag = Diagnostics.An_import_path_cannot_end_with_a_0_extension_Consider_importing_1_instead;
1367+
error(moduleReferenceLiteral, diag, tsExtension, removeExtension(moduleName, tsExtension));
1368+
}
1369+
else {
1370+
error(moduleReferenceLiteral, moduleNotFoundError, moduleName);
1371+
}
13651372
}
13661373
return undefined;
13671374
}

‎src/compiler/core.ts‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,22 @@ namespace ts {
113113
return undefined;
114114
}
115115

116-
/** Like `forEach`, but assumes existence of array and fails if no truthy value is found. */
117-
export function find<T, U>(array: T[], callback: (element: T, index: number) => U | undefined): U {
116+
/** Works like Array.prototype.find, returning `undefined` if no element satisfying the predicate is found. */
117+
export function find<T>(array: T[], predicate: (element: T, index: number) => boolean): T | undefined {
118+
for (let i = 0, len = array.length; i < len; i++) {
119+
const value = array[i];
120+
if (predicate(value, i)) {
121+
return value;
122+
}
123+
}
124+
return undefined;
125+
}
126+
127+
/**
128+
* Returns the first truthy result of `callback`, or else fails.
129+
* This is like `forEach`, but never returns undefined.
130+
*/
131+
export function findMap<T, U>(array: T[], callback: (element: T, index: number) => U | undefined): U {
118132
for (let i = 0, len = array.length; i < len; i++) {
119133
const result = callback(array[i], i);
120134
if (result) {
@@ -1315,6 +1329,8 @@ namespace ts {
13151329
* List of supported extensions in order of file resolution precedence.
13161330
*/
13171331
export const supportedTypeScriptExtensions = [".ts", ".tsx", ".d.ts"];
1332+
/** Must have ".d.ts" first because if ".ts" goes first, that will be detected as the extension instead of ".d.ts". */
1333+
export const supportedTypescriptExtensionsForExtractExtension = [".d.ts", ".ts", ".tsx"];
13181334
export const supportedJavascriptExtensions = [".js", ".jsx"];
13191335
const allSupportedExtensions = supportedTypeScriptExtensions.concat(supportedJavascriptExtensions);
13201336

@@ -1397,8 +1413,12 @@ namespace ts {
13971413
return path;
13981414
}
13991415

1400-
export function tryRemoveExtension(path: string, extension: string): string {
1401-
return fileExtensionIs(path, extension) ? path.substring(0, path.length - extension.length) : undefined;
1416+
export function tryRemoveExtension(path: string, extension: string): string | undefined {
1417+
return fileExtensionIs(path, extension) ? removeExtension(path, extension) : undefined;
1418+
}
1419+
1420+
export function removeExtension(path: string, extension: string): string {
1421+
return path.substring(0, path.length - extension.length);
14021422
}
14031423

14041424
export function isJsxOrTsxExtension(ext: string): boolean {

‎src/compiler/diagnosticMessages.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1951,6 +1951,10 @@
19511951
"category": "Error",
19521952
"code": 2690
19531953
},
1954+
"An import path cannot end with a '{0}' extension. Consider importing '{1}' instead.": {
1955+
"category": "Error",
1956+
"code": 2691
1957+
},
19541958
"Import declaration '{0}' is using private name '{1}'.": {
19551959
"category": "Error",
19561960
"code": 4000

‎src/compiler/program.ts‎

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -661,51 +661,52 @@ namespace ts {
661661
* @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
662662
* 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.
663663
*/
664-
function loadModuleFromFile(candidate: string, extensions: string[], failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string {
665-
// First try to keep/add an extension: importing "./foo.ts" can be matched by a file "./foo.ts", and "./foo" by "./foo.d.ts"
666-
const resolvedByAddingOrKeepingExtension = loadModuleFromFileWorker(candidate, extensions, failedLookupLocation, onlyRecordFailures, state);
667-
if (resolvedByAddingOrKeepingExtension) {
668-
return resolvedByAddingOrKeepingExtension;
664+
function loadModuleFromFile(candidate: string, extensions: string[], failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string | undefined {
665+
// First, try adding an extension. An import of "foo" could be matched by a file "foo.ts", or "foo.js" by "foo.js.ts"
666+
const resolvedByAddingExtension = tryAddingExtensions(candidate, extensions, failedLookupLocation, onlyRecordFailures, state);
667+
if (resolvedByAddingExtension) {
668+
return resolvedByAddingExtension;
669669
}
670-
// 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"
670+
671+
// If that didn't work, try stripping a ".js" or ".jsx" extension and replacing it with a TypeScript one;
672+
// e.g. "./foo.js" can be matched by "./foo.ts" or "./foo.d.ts"
671673
if (hasJavaScriptFileExtension(candidate)) {
672674
const extensionless = removeFileExtension(candidate);
673675
if (state.traceEnabled) {
674676
const extension = candidate.substring(extensionless.length);
675677
trace(state.host, Diagnostics.File_name_0_has_a_1_extension_stripping_it, candidate, extension);
676678
}
677-
return loadModuleFromFileWorker(extensionless, extensions, failedLookupLocation, onlyRecordFailures, state);
679+
return tryAddingExtensions(extensionless, extensions, failedLookupLocation, onlyRecordFailures, state);
678680
}
679681
}
680682

681-
function loadModuleFromFileWorker(candidate: string, extensions: string[], failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string {
683+
/** Try to return an existing file that adds one of the `extensions` to `candidate`. */
684+
function tryAddingExtensions(candidate: string, extensions: string[], failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string | undefined {
682685
if (!onlyRecordFailures) {
683686
// check if containing folder exists - if it doesn't then just record failures for all supported extensions without disk probing
684687
const directory = getDirectoryPath(candidate);
685688
if (directory) {
686689
onlyRecordFailures = !directoryProbablyExists(directory, state.host);
687690
}
688691
}
689-
return forEach(extensions, tryLoad);
692+
return forEach(extensions, ext =>
693+
!(state.skipTsx && isJsxOrTsxExtension(ext)) && tryFile(candidate + ext, failedLookupLocation, onlyRecordFailures, state));
694+
}
690695

691-
function tryLoad(ext: string): string {
692-
if (state.skipTsx && isJsxOrTsxExtension(ext)) {
693-
return undefined;
694-
}
695-
const fileName = fileExtensionIs(candidate, ext) ? candidate : candidate + ext;
696-
if (!onlyRecordFailures && state.host.fileExists(fileName)) {
697-
if (state.traceEnabled) {
698-
trace(state.host, Diagnostics.File_0_exist_use_it_as_a_name_resolution_result, fileName);
699-
}
700-
return fileName;
696+
/** Return the file if it exists. */
697+
function tryFile(fileName: string, failedLookupLocation: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): string | undefined {
698+
if (!onlyRecordFailures && state.host.fileExists(fileName)) {
699+
if (state.traceEnabled) {
700+
trace(state.host, Diagnostics.File_0_exist_use_it_as_a_name_resolution_result, fileName);
701701
}
702-
else {
703-
if (state.traceEnabled) {
704-
trace(state.host, Diagnostics.File_0_does_not_exist, fileName);
705-
}
706-
failedLookupLocation.push(fileName);
707-
return undefined;
702+
return fileName;
703+
}
704+
else {
705+
if (state.traceEnabled) {
706+
trace(state.host, Diagnostics.File_0_does_not_exist, fileName);
708707
}
708+
failedLookupLocation.push(fileName);
709+
return undefined;
709710
}
710711
}
711712

@@ -718,7 +719,9 @@ namespace ts {
718719
}
719720
const typesFile = tryReadTypesSection(packageJsonPath, candidate, state);
720721
if (typesFile) {
721-
const result = loadModuleFromFile(typesFile, extensions, failedLookupLocation, !directoryProbablyExists(getDirectoryPath(typesFile), state.host), state);
722+
const onlyRecordFailures = !directoryProbablyExists(getDirectoryPath(typesFile), state.host);
723+
// The package.json "typings" property must specify the file with extension, so just try that exact filename.
724+
const result = tryFile(typesFile, failedLookupLocation, onlyRecordFailures, state);
722725
if (result) {
723726
return result;
724727
}

‎src/compiler/utilities.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2694,6 +2694,11 @@ namespace ts {
26942694
return forEach(supportedTypeScriptExtensions, extension => fileExtensionIs(fileName, extension));
26952695
}
26962696

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

‎src/harness/unittests/moduleResolution.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -446,8 +446,8 @@ export = C;
446446
"/a/B/c/moduleB.ts": `import a = require("./moduleC")`,
447447
"/a/B/c/moduleC.ts": "export var x",
448448
"/a/B/c/moduleD.ts": `
449-
import a = require("./moduleA.ts");
450-
import b = require("./moduleB.ts");
449+
import a = require("./moduleA");
450+
import b = require("./moduleB");
451451
`
452452
});
453453
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");
458458
"/a/B/c/moduleB.ts": `import a = require("./moduleC")`,
459459
"/a/B/c/moduleC.ts": "export var x",
460460
"/a/B/c/moduleD.ts": `
461-
import a = require("./moduleA.ts");
462-
import b = require("./moduleB.ts");
461+
import a = require("./moduleA");
462+
import b = require("./moduleB");
463463
`
464464
});
465465
test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true }, "/a/B/c", /*useCaseSensitiveFileNames*/ false, ["moduleD.ts"], []);

‎tests/baselines/reference/exportDefaultProperty2.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace C {
1313
export default C.B;
1414

1515
//// [b.ts]
16-
import B from "./a.ts";
16+
import B from "./a";
1717
const x: B = { c: B };
1818

1919

@@ -29,5 +29,5 @@ exports.__esModule = true;
2929
exports["default"] = C.B;
3030
//// [b.js]
3131
"use strict";
32-
var a_ts_1 = require("./a.ts");
33-
var x = { c: a_ts_1["default"] };
32+
var a_1 = require("./a");
33+
var x = { c: a_1["default"] };

‎tests/baselines/reference/exportDefaultProperty2.symbols‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export default C.B;
2121
>B : Symbol(default, Decl(a.ts, 2, 9), Decl(a.ts, 5, 13))
2222

2323
=== tests/cases/compiler/b.ts ===
24-
import B from "./a.ts";
24+
import B from "./a";
2525
>B : Symbol(B, Decl(b.ts, 0, 6))
2626

2727
const x: B = { c: B };

‎tests/baselines/reference/exportDefaultProperty2.types‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export default C.B;
2121
>B : number
2222

2323
=== tests/cases/compiler/b.ts ===
24-
import B from "./a.ts";
24+
import B from "./a";
2525
>B : number
2626

2727
const x: B = { c: B };

‎tests/baselines/reference/exportEqualsProperty2.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace C {
1313
export = C.B;
1414

1515
//// [b.ts]
16-
import B = require("./a.ts");
16+
import B = require("./a");
1717
const x: B = { c: B };
1818

1919

@@ -28,5 +28,5 @@ var C = (function () {
2828
module.exports = C.B;
2929
//// [b.js]
3030
"use strict";
31-
var B = require("./a.ts");
31+
var B = require("./a");
3232
var x = { c: B };

0 commit comments

Comments
 (0)