Skip to content

Don't allow .ts to appear in an import #9646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
10 commits merged into from
Aug 19, 2016
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1342,7 +1342,12 @@ namespace ts {

if (moduleNotFoundError) {
// report errors only if it was requested
error(moduleReferenceLiteral, moduleNotFoundError, moduleName);
if (hasTypeScriptFileExtensionNonDts(moduleName)) {
error(moduleReferenceLiteral, Diagnostics.Module_name_should_not_include_a_ts_extension_Colon_0, moduleName);
}
else {
error(moduleReferenceLiteral, moduleNotFoundError, moduleName);
}
}
return undefined;
}
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1144,7 +1144,8 @@ namespace ts {
/**
* List of supported extensions in order of file resolution precedence.
*/
export const supportedTypeScriptExtensions = [".ts", ".tsx", ".d.ts"];
export const supportedTypeScriptExtensionsNonDts = [".ts", ".tsx"];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you'll need this if you correctly support errors for importing .d.ts files

export const supportedTypeScriptExtensions = supportedTypeScriptExtensionsNonDts.concat([".d.ts"]);
export const supportedJavascriptExtensions = [".js", ".jsx"];
const allSupportedExtensions = supportedTypeScriptExtensions.concat(supportedJavascriptExtensions);

Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1943,6 +1943,10 @@
"category": "Error",
"code": 2689
},
"Module name should not include a '.ts' extension: '{0}'.": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try making this

An import path should not include a '.ts' extension. Consider importing '{0}' instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An import path should not end with a '{0}' extension. Consider importing '{1}' instead.

Since this error is being used for .tsx as well.

"category": "Error",
"code": 2690
},
"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
"code": 4000
Expand Down
61 changes: 36 additions & 25 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -668,51 +668,62 @@ 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 {
// If the candidate already has an extension load that or quit.
if (hasTypeScriptFileExtension(candidate)) {
// Don't allow `.ts` to appear at the end
if (!fileExtensionIs(candidate, ".d.ts")) {
return undefined;
}
return tryFile(candidate, failedLookupLocation, onlyRecordFailures, state);
}
// 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"

// Next, try adding an extension.
// We don't allow an import of "foo.ts" to be matched by "foo.ts.ts", but we do allow "foo.js" to be matched by "foo.js.ts".
const resolvedByAddingExtension = tryAddingExtensions(candidate, extensions, failedLookupLocation, onlyRecordFailures, state);
if (resolvedByAddingExtension) {
return resolvedByAddingExtension;
}

// 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;
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2726,6 +2726,10 @@ namespace ts {
return forEach(supportedTypeScriptExtensions, extension => fileExtensionIs(fileName, extension));
}

export function hasTypeScriptFileExtensionNonDts(fileName: string) {
return forEach(supportedTypeScriptExtensionsNonDts, 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.
Expand Down
8 changes: 4 additions & 4 deletions src/harness/unittests/moduleResolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,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]);
Expand All @@ -477,8 +477,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"], []);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

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

Expand Down
14 changes: 14 additions & 0 deletions tests/baselines/reference/moduleResolutionNoTs.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
tests/cases/compiler/user.ts(4,15): error TS2690: Module name should not include a '.ts' extension: './m.ts'.


==== tests/cases/compiler/user.ts (1 errors) ====
// '.ts' extension is OK in a reference
///<reference path="./m.ts"/>

import x from "./m.ts";
~~~~~~~~
!!! error TS2690: Module name should not include a '.ts' extension: './m.ts'.

==== tests/cases/compiler/m.ts (0 errors) ====
export default 0;

20 changes: 20 additions & 0 deletions tests/baselines/reference/moduleResolutionNoTs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//// [tests/cases/compiler/moduleResolutionNoTs.ts] ////

//// [m.ts]
export default 0;

//// [user.ts]
// '.ts' extension is OK in a reference
///<reference path="./m.ts"/>

import x from "./m.ts";


//// [m.js]
"use strict";
exports.__esModule = true;
exports["default"] = 0;
//// [user.js]
// '.ts' extension is OK in a reference
///<reference path="./m.ts"/>
"use strict";
7 changes: 0 additions & 7 deletions tests/baselines/reference/moduleResolutionWithExtensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@
"File '/src/a.ts' exist - use it as a name resolution result.",
"Resolving real path for '/src/a.ts', result '/src/a.ts'",
"======== Module name './a' was successfully resolved to '/src/a.ts'. ========",
"======== Resolving module './a.ts' from '/src/c.ts'. ========",
"Module resolution kind is not specified, using 'NodeJs'.",
"Loading module as file / folder, candidate module location '/src/a.ts'.",
"File '/src/a.ts' exist - use it as a name resolution result.",
"Resolving real path for '/src/a.ts', result '/src/a.ts'",
"======== Module name './a.ts' was successfully resolved to '/src/a.ts'. ========",
"======== Resolving module './a.js' from '/src/d.ts'. ========",
"Module resolution kind is not specified, using 'NodeJs'.",
"Loading module as file / folder, candidate module location '/src/a.js'.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@ No type information for this code.=== /src/b.ts ===
import a from './a';
>a : number

// Matching extension
=== /src/c.ts ===
import a from './a.ts';
>a : number

// '.js' extension: stripped and replaced with '.ts'
=== /src/d.ts ===
import a from './a.js';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ tests/cases/compiler/staticInstanceResolution5_1.ts(6,16): error TS2304: Cannot


==== tests/cases/compiler/staticInstanceResolution5_1.ts (3 errors) ====
import WinJS = require('staticInstanceResolution5_0.ts');
import WinJS = require('staticInstanceResolution5_0');

// these 3 should be errors
var x = (w1: WinJS) => { };
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/staticInstanceResolution5.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export class Promise {
}

//// [staticInstanceResolution5_1.ts]
import WinJS = require('staticInstanceResolution5_0.ts');
import WinJS = require('staticInstanceResolution5_0');

// these 3 should be errors
var x = (w1: WinJS) => { };
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/compiler/missingFunctionImplementation2.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @Filename: missingFunctionImplementation2_a.ts
export {};
declare module "./missingFunctionImplementation2_b.ts" {
declare module "./missingFunctionImplementation2_b" {
export function f(a, b): void;
}

Expand Down
8 changes: 8 additions & 0 deletions tests/cases/compiler/moduleResolutionNoTs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// @filename: m.ts
export default 0;

// @filename: user.ts
// '.ts' extension is OK in a reference
///<reference path="./m.ts"/>

import x from "./m.ts";
2 changes: 1 addition & 1 deletion tests/cases/compiler/staticInstanceResolution5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export class Promise {
}

// @Filename: staticInstanceResolution5_1.ts
import WinJS = require('staticInstanceResolution5_0.ts');
import WinJS = require('staticInstanceResolution5_0');

// these 3 should be errors
var x = (w1: WinJS) => { };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ export default 0;
// @Filename: /src/b.ts
import a from './a';

// Matching extension
// @Filename: /src/c.ts
import a from './a.ts';

// '.js' extension: stripped and replaced with '.ts'
// @Filename: /src/d.ts
import a from './a.js';
Expand Down