Skip to content
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

Allow untyped imports #11889

Merged
2 commits merged into from
Oct 27, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
35 changes: 29 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@ namespace ts {
const moduleSymbol = resolveExternalModuleName(node, (<ImportDeclaration>node.parent).moduleSpecifier);

if (moduleSymbol) {
const exportDefaultSymbol = isShorthandAmbientModuleSymbol(moduleSymbol) ?
const exportDefaultSymbol = isUntypedModuleSymbol(moduleSymbol) ?
moduleSymbol :
moduleSymbol.exports["export="] ?
getPropertyOfType(getTypeOfSymbol(moduleSymbol.exports["export="]), "default") :
Expand Down Expand Up @@ -1145,7 +1145,7 @@ namespace ts {
if (targetSymbol) {
const name = specifier.propertyName || specifier.name;
if (name.text) {
if (isShorthandAmbientModuleSymbol(moduleSymbol)) {
if (isUntypedModuleSymbol(moduleSymbol)) {
return moduleSymbol;
}

Expand Down Expand Up @@ -1365,8 +1365,9 @@ namespace ts {
}

const isRelative = isExternalModuleNameRelative(moduleName);
const quotedName = '"' + moduleName + '"';
if (!isRelative) {
const symbol = getSymbol(globals, '"' + moduleName + '"', SymbolFlags.ValueModule);
const symbol = getSymbol(globals, quotedName, SymbolFlags.ValueModule);
if (symbol) {
// merged symbol is module declaration symbol combined with all augmentations
return getMergedSymbol(symbol);
Expand Down Expand Up @@ -1395,6 +1396,28 @@ namespace ts {
}
}

// May be an untyped module. If so, ignore resolutionDiagnostic.
if (!isRelative && resolvedModule && !extensionIsTypeScript(resolvedModule.extension)) {
if (compilerOptions.noImplicitAny) {
if (moduleNotFoundError) {
error(errorNode,
Diagnostics.A_package_for_0_was_found_at_1_but_is_untyped_Because_noImplicitAny_is_enabled_this_package_must_have_a_declaration,
moduleReference,
resolvedModule.resolvedFileName);
}
return undefined;
}

// Create a new symbol to represent the untyped module and store it in globals.
// This provides a name to the module. See the test tests/cases/fourslash/untypedModuleImport.ts
const newSymbol = createSymbol(SymbolFlags.ValueModule, quotedName);
// Module symbols are expected to have 'exports', although since this is an untyped module it can be empty.
newSymbol.exports = createMap<Symbol>();
// Cache it so subsequent accesses will return the same module.
globals[quotedName] = newSymbol;
return newSymbol;
}

if (moduleNotFoundError) {
// report errors only if it was requested
if (resolutionDiagnostic) {
Expand Down Expand Up @@ -3462,7 +3485,7 @@ namespace ts {
function getTypeOfFuncClassEnumModule(symbol: Symbol): Type {
const links = getSymbolLinks(symbol);
if (!links.type) {
if (symbol.valueDeclaration.kind === SyntaxKind.ModuleDeclaration && isShorthandAmbientModuleSymbol(symbol)) {
if (symbol.flags & SymbolFlags.Module && isUntypedModuleSymbol(symbol)) {
links.type = anyType;
}
else {
Expand Down Expand Up @@ -19011,7 +19034,7 @@ namespace ts {

function moduleExportsSomeValue(moduleReferenceExpression: Expression): boolean {
let moduleSymbol = resolveExternalModuleName(moduleReferenceExpression.parent, moduleReferenceExpression);
if (!moduleSymbol || isShorthandAmbientModuleSymbol(moduleSymbol)) {
if (!moduleSymbol || isUntypedModuleSymbol(moduleSymbol)) {
// If the module is not found or is shorthand, assume that it may export a value.
return true;
}
Expand Down Expand Up @@ -19512,7 +19535,7 @@ namespace ts {
(typeReferenceDirectives || (typeReferenceDirectives = [])).push(typeReferenceDirective);
}
else {
// found at least one entry that does not originate from type reference directive
// found at least one entry that does not originate from type reference directive
return undefined;
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2869,6 +2869,10 @@
"category": "Error",
"code": 6143
},
"A package for '{0}' was found at '{1}', but is untyped. Because '--noImplicitAny' is enabled, this package must have a declaration.": {
Copy link
Contributor

Choose a reason for hiding this comment

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

how about something more consistent with the rest of the the --noImplicitAny error message, e.g.: Could not find a declaration file for module '{0}'. All imports implicitly have type 'any'.

Copy link
Author

Choose a reason for hiding this comment

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

How about: "Could not find a declaration file for module '{0}'. '{1}' implicitly has an 'any' type."

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

"category": "Error",
"code": 6144
Copy link
Contributor

Choose a reason for hiding this comment

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

make this error code in the 7000 along with the other --noImplicitAnyErrors

},
"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
"code": 7005
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1324,6 +1324,7 @@ namespace ts {
// - it's not a top level JavaScript module that exceeded the search max
const elideImport = isJsFileFromNodeModules && currentNodeModulesDepth > maxNodeModuleJsDepth;
// Don't add the file if it has a bad extension (e.g. 'tsx' if we don't have '--allowJs')
// This may still end up being an untyped module -- the file won't be included but imports will be allowed.
const shouldAddFile = resolvedFileName && !getResolutionDiagnostic(options, resolution) && !options.noResolve && i < file.imports.length && !elideImport;

if (elideImport) {
Expand Down Expand Up @@ -1571,8 +1572,9 @@ namespace ts {

/* @internal */
/**
* Returns a DiagnosticMessage if we can't use a resolved module due to its extension.
* Returns a DiagnosticMessage if we won't include a resolved module due to its extension.
* The DiagnosticMessage's parameters are the imported module name, and the filename it resolved to.
* This returns a diagnostic even if the module will be an untyped module.
*/
export function getResolutionDiagnostic(options: CompilerOptions, { extension }: ResolvedModule): DiagnosticMessage | undefined {
switch (extension) {
Expand Down
5 changes: 3 additions & 2 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,9 @@ namespace ts {
((<ModuleDeclaration>node).name.kind === SyntaxKind.StringLiteral || isGlobalScopeAugmentation(<ModuleDeclaration>node));
}

export function isShorthandAmbientModuleSymbol(moduleSymbol: Symbol): boolean {
return isShorthandAmbientModule(moduleSymbol.valueDeclaration);
/** Given a symbol for a module, checks that it is either an untyped import or a shorthand ambient module. */
export function isUntypedModuleSymbol(moduleSymbol: Symbol): boolean {
return !moduleSymbol.valueDeclaration || isShorthandAmbientModule(moduleSymbol.valueDeclaration);
}

function isShorthandAmbientModule(node: Node): boolean {
Expand Down
38 changes: 22 additions & 16 deletions src/harness/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1108,22 +1108,7 @@ namespace Harness {
const option = getCommandLineOption(name);
if (option) {
const errors: ts.Diagnostic[] = [];
switch (option.type) {
case "boolean":
options[option.name] = value.toLowerCase() === "true";
break;
case "string":
options[option.name] = value;
break;
// If not a primitive, the possible types are specified in what is effectively a map of options.
case "list":
options[option.name] = ts.parseListTypeOption(<ts.CommandLineOptionOfListType>option, value, errors);
break;
default:
options[option.name] = ts.parseCustomTypeOption(<ts.CommandLineOptionOfCustomType>option, value, errors);
break;
}

options[option.name] = optionValue(option, value, errors);
if (errors.length > 0) {
throw new Error(`Unknown value '${value}' for compiler option '${name}'.`);
}
Expand All @@ -1135,6 +1120,27 @@ namespace Harness {
}
}

function optionValue(option: ts.CommandLineOption, value: string, errors: ts.Diagnostic[]): any {
switch (option.type) {
case "boolean":
return value.toLowerCase() === "true";
case "string":
return value;
case "number": {
const number = parseInt(value, 10);
if (isNaN(number)) {
throw new Error(`Value must be a number, got: ${JSON.stringify(value)}`);
}
return number;
}
// If not a primitive, the possible types are specified in what is effectively a map of options.
case "list":
return ts.parseListTypeOption(<ts.CommandLineOptionOfListType>option, value, errors);
default:
return ts.parseCustomTypeOption(<ts.CommandLineOptionOfCustomType>option, value, errors);
}
}

export interface TestFile {
unitName: string;
content: string;
Expand Down
37 changes: 37 additions & 0 deletions tests/baselines/reference/untypedModuleImport.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//// [tests/cases/conformance/moduleResolution/untypedModuleImport.ts] ////

//// [index.js]
// This tests that importing from a JS file globally works in an untyped way.
// (Assuming we don't have `--noImplicitAny` or `--allowJs`.)

This file is not processed.

//// [a.ts]
import * as foo from "foo";
foo.bar();

//// [b.ts]
import foo = require("foo");
foo();

//// [c.ts]
import foo, { bar } from "foo";
import "./a";
import "./b";
foo(bar());


//// [a.js]
"use strict";
var foo = require("foo");
foo.bar();
//// [b.js]
"use strict";
var foo = require("foo");
foo();
//// [c.js]
"use strict";
var foo_1 = require("foo");
require("./a");
require("./b");
foo_1["default"](foo_1.bar());
25 changes: 25 additions & 0 deletions tests/baselines/reference/untypedModuleImport.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
=== /c.ts ===
import foo, { bar } from "foo";
>foo : Symbol(foo, Decl(c.ts, 0, 6))
>bar : Symbol(bar, Decl(c.ts, 0, 13))

import "./a";
import "./b";
foo(bar());
>foo : Symbol(foo, Decl(c.ts, 0, 6))
>bar : Symbol(bar, Decl(c.ts, 0, 13))

=== /a.ts ===
import * as foo from "foo";
>foo : Symbol(foo, Decl(a.ts, 0, 6))

foo.bar();
>foo : Symbol(foo, Decl(a.ts, 0, 6))

=== /b.ts ===
import foo = require("foo");
>foo : Symbol(foo, Decl(b.ts, 0, 0))

foo();
>foo : Symbol(foo, Decl(b.ts, 0, 0))

31 changes: 31 additions & 0 deletions tests/baselines/reference/untypedModuleImport.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
=== /c.ts ===
import foo, { bar } from "foo";
>foo : any
>bar : any

import "./a";
import "./b";
foo(bar());
>foo(bar()) : any
>foo : any
>bar() : any
>bar : any

=== /a.ts ===
import * as foo from "foo";
>foo : any

foo.bar();
>foo.bar() : any
>foo.bar : any
>foo : any
>bar : any

=== /b.ts ===
import foo = require("foo");
>foo : any

foo();
>foo() : any
>foo : any

16 changes: 16 additions & 0 deletions tests/baselines/reference/untypedModuleImport_allowJs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//// [tests/cases/conformance/moduleResolution/untypedModuleImport_allowJs.ts] ////

//// [index.js]
// Same as untypedModuleImport.ts but with --allowJs, so the package will actually be typed.

exports.default = { bar() { return 0; } }

//// [a.ts]
import foo from "foo";
foo.bar();


//// [a.js]
"use strict";
var foo_1 = require("foo");
foo_1["default"].bar();
17 changes: 17 additions & 0 deletions tests/baselines/reference/untypedModuleImport_allowJs.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
=== /a.ts ===
import foo from "foo";
>foo : Symbol(foo, Decl(a.ts, 0, 6))

foo.bar();
>foo.bar : Symbol(bar, Decl(index.js, 2, 19))
>foo : Symbol(foo, Decl(a.ts, 0, 6))
>bar : Symbol(bar, Decl(index.js, 2, 19))

=== /node_modules/foo/index.js ===
// Same as untypedModuleImport.ts but with --allowJs, so the package will actually be typed.

exports.default = { bar() { return 0; } }
>exports : Symbol(default, Decl(index.js, 0, 0))
>default : Symbol(default, Decl(index.js, 0, 0))
>bar : Symbol(bar, Decl(index.js, 2, 19))

22 changes: 22 additions & 0 deletions tests/baselines/reference/untypedModuleImport_allowJs.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
=== /a.ts ===
import foo from "foo";
>foo : { bar(): number; }

foo.bar();
>foo.bar() : number
>foo.bar : () => number
>foo : { bar(): number; }
>bar : () => number

=== /node_modules/foo/index.js ===
// Same as untypedModuleImport.ts but with --allowJs, so the package will actually be typed.

exports.default = { bar() { return 0; } }
>exports.default = { bar() { return 0; } } : { bar(): number; }
>exports.default : any
>exports : any
>default : any
>{ bar() { return 0; } } : { bar(): number; }
>bar : () => number
>0 : 0

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/a.ts(1,22): error TS6144: A package for 'foo' was found at '/node_modules/foo/index.js', but is untyped. Because '--noImplicitAny' is enabled, this package must have a declaration.


==== /a.ts (1 errors) ====
import * as foo from "foo";
~~~~~
!!! error TS6144: A package for 'foo' was found at '/node_modules/foo/index.js', but is untyped. Because '--noImplicitAny' is enabled, this package must have a declaration.

==== /node_modules/foo/index.js (0 errors) ====
// This tests that `--noImplicitAny` disables untyped modules.

This file is not processed.

13 changes: 13 additions & 0 deletions tests/baselines/reference/untypedModuleImport_noImplicitAny.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//// [tests/cases/conformance/moduleResolution/untypedModuleImport_noImplicitAny.ts] ////

//// [index.js]
// This tests that `--noImplicitAny` disables untyped modules.

This file is not processed.

//// [a.ts]
import * as foo from "foo";


//// [a.js]
"use strict";
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/a.ts(1,22): error TS6143: Module './foo' was resolved to '/foo.js', but '--allowJs' is not set.


==== /a.ts (1 errors) ====
import * as foo from "./foo";
~~~~~~~
!!! error TS6143: Module './foo' was resolved to '/foo.js', but '--allowJs' is not set.

==== /foo.js (0 errors) ====
// This tests that untyped module imports don't happen with local imports.

This file is not processed.

13 changes: 13 additions & 0 deletions tests/baselines/reference/untypedModuleImport_noLocalImports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//// [tests/cases/conformance/moduleResolution/untypedModuleImport_noLocalImports.ts] ////

//// [foo.js]
// This tests that untyped module imports don't happen with local imports.

This file is not processed.

//// [a.ts]
import * as foo from "./foo";


//// [a.js]
"use strict";
Loading