From 8448e741c924ebe56a029f53cd5bd3e9f5b6a226 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 27 Oct 2016 08:08:02 -0700 Subject: [PATCH 1/6] Fix bug: Return a resolution diagnostic for a `.jsx` import if `--allowJs` is turned off --- src/compiler/program.ts | 14 ++++++++++---- ...utionWithExtensions_notSupported3.errors.txt | 12 ++++++++++++ ...uleResolutionWithExtensions_notSupported3.js | 12 ++++++++++++ ...utionWithExtensions_notSupported3.trace.json | 17 +++++++++++++++++ ...uleResolutionWithExtensions_notSupported3.ts | 9 +++++++++ 5 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.errors.txt create mode 100644 tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.js create mode 100644 tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.trace.json create mode 100644 tests/cases/compiler/moduleResolutionWithExtensions_notSupported3.ts diff --git a/src/compiler/program.ts b/src/compiler/program.ts index fab35141ba4af..431479752d1c3 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1580,13 +1580,19 @@ namespace ts { case Extension.Dts: // These are always allowed. return undefined; - case Extension.Tsx: + return needJsx(); case Extension.Jsx: - return options.jsx ? undefined : Diagnostics.Module_0_was_resolved_to_1_but_jsx_is_not_set; - + return needJsx() || needAllowJs(); case Extension.Js: - return options.allowJs ? undefined : Diagnostics.Module_0_was_resolved_to_1_but_allowJs_is_not_set; + return needAllowJs(); + } + + function needJsx() { + return options.jsx ? undefined : Diagnostics.Module_0_was_resolved_to_1_but_jsx_is_not_set; + } + function needAllowJs() { + return options.allowJs ? undefined : Diagnostics.Module_0_was_resolved_to_1_but_allowJs_is_not_set; } } } diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.errors.txt b/tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.errors.txt new file mode 100644 index 0000000000000..45e058bae54ed --- /dev/null +++ b/tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.errors.txt @@ -0,0 +1,12 @@ +/a.ts(1,17): error TS6143: Module './jsx' was resolved to '/jsx.jsx', but '--allowJs' is not set. + + +==== /a.ts (1 errors) ==== + import jsx from "./jsx"; + ~~~~~~~ +!!! error TS6143: Module './jsx' was resolved to '/jsx.jsx', but '--allowJs' is not set. + +==== /jsx.jsx (0 errors) ==== + // Test the error message if we have `--jsx` but not `--allowJw`. + + \ No newline at end of file diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.js b/tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.js new file mode 100644 index 0000000000000..f362701bbda71 --- /dev/null +++ b/tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.js @@ -0,0 +1,12 @@ +//// [tests/cases/compiler/moduleResolutionWithExtensions_notSupported3.ts] //// + +//// [jsx.jsx] +// Test the error message if we have `--jsx` but not `--allowJw`. + + +//// [a.ts] +import jsx from "./jsx"; + + +//// [a.js] +"use strict"; diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.trace.json b/tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.trace.json new file mode 100644 index 0000000000000..c366843ce86e6 --- /dev/null +++ b/tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.trace.json @@ -0,0 +1,17 @@ +[ + "======== Resolving module './jsx' from '/a.ts'. ========", + "Module resolution kind is not specified, using 'NodeJs'.", + "Loading module as file / folder, candidate module location '/jsx'.", + "File '/jsx.ts' does not exist.", + "File '/jsx.tsx' does not exist.", + "File '/jsx.d.ts' does not exist.", + "File '/jsx/package.json' does not exist.", + "File '/jsx/index.ts' does not exist.", + "File '/jsx/index.tsx' does not exist.", + "File '/jsx/index.d.ts' does not exist.", + "Loading module as file / folder, candidate module location '/jsx'.", + "File '/jsx.js' does not exist.", + "File '/jsx.jsx' exist - use it as a name resolution result.", + "Resolving real path for '/jsx.jsx', result '/jsx.jsx'", + "======== Module name './jsx' was successfully resolved to '/jsx.jsx'. ========" +] \ No newline at end of file diff --git a/tests/cases/compiler/moduleResolutionWithExtensions_notSupported3.ts b/tests/cases/compiler/moduleResolutionWithExtensions_notSupported3.ts new file mode 100644 index 0000000000000..e06f603377c19 --- /dev/null +++ b/tests/cases/compiler/moduleResolutionWithExtensions_notSupported3.ts @@ -0,0 +1,9 @@ +// @noImplicitReferences: true +// @jsx: preserve +// @traceResolution: true +// Test the error message if we have `--jsx` but not `--allowJw`. + +// @Filename: /jsx.jsx + +// @Filename: /a.ts +import jsx from "./jsx"; From 4937d9c8b45e950cdd1095be0afeb07fe032aa6f Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 27 Oct 2016 07:19:37 -0700 Subject: [PATCH 2/6] Allow untyped imports --- src/compiler/checker.ts | 35 ++++++++++++++--- src/compiler/diagnosticMessages.json | 4 ++ src/compiler/program.ts | 4 +- src/compiler/utilities.ts | 5 ++- src/harness/harness.ts | 38 +++++++++++-------- .../reference/untypedModuleImport.js | 37 ++++++++++++++++++ .../reference/untypedModuleImport.symbols | 25 ++++++++++++ .../reference/untypedModuleImport.types | 31 +++++++++++++++ .../reference/untypedModuleImport_allowJs.js | 16 ++++++++ .../untypedModuleImport_allowJs.symbols | 17 +++++++++ .../untypedModuleImport_allowJs.types | 22 +++++++++++ ...typedModuleImport_noImplicitAny.errors.txt | 13 +++++++ .../untypedModuleImport_noImplicitAny.js | 13 +++++++ ...ypedModuleImport_noLocalImports.errors.txt | 13 +++++++ .../untypedModuleImport_noLocalImports.js | 13 +++++++ .../untypedModuleImport_vsAmbient.js | 23 +++++++++++ .../untypedModuleImport_vsAmbient.symbols | 14 +++++++ .../untypedModuleImport_vsAmbient.types | 14 +++++++ .../moduleResolution/untypedModuleImport.ts | 21 ++++++++++ .../untypedModuleImport_allowJs.ts | 12 ++++++ .../untypedModuleImport_noImplicitAny.ts | 10 +++++ .../untypedModuleImport_noLocalImports.ts | 9 +++++ .../untypedModuleImport_vsAmbient.ts | 16 ++++++++ tests/cases/fourslash/untypedModuleImport.ts | 22 +++++++++++ 24 files changed, 402 insertions(+), 25 deletions(-) create mode 100644 tests/baselines/reference/untypedModuleImport.js create mode 100644 tests/baselines/reference/untypedModuleImport.symbols create mode 100644 tests/baselines/reference/untypedModuleImport.types create mode 100644 tests/baselines/reference/untypedModuleImport_allowJs.js create mode 100644 tests/baselines/reference/untypedModuleImport_allowJs.symbols create mode 100644 tests/baselines/reference/untypedModuleImport_allowJs.types create mode 100644 tests/baselines/reference/untypedModuleImport_noImplicitAny.errors.txt create mode 100644 tests/baselines/reference/untypedModuleImport_noImplicitAny.js create mode 100644 tests/baselines/reference/untypedModuleImport_noLocalImports.errors.txt create mode 100644 tests/baselines/reference/untypedModuleImport_noLocalImports.js create mode 100644 tests/baselines/reference/untypedModuleImport_vsAmbient.js create mode 100644 tests/baselines/reference/untypedModuleImport_vsAmbient.symbols create mode 100644 tests/baselines/reference/untypedModuleImport_vsAmbient.types create mode 100644 tests/cases/conformance/moduleResolution/untypedModuleImport.ts create mode 100644 tests/cases/conformance/moduleResolution/untypedModuleImport_allowJs.ts create mode 100644 tests/cases/conformance/moduleResolution/untypedModuleImport_noImplicitAny.ts create mode 100644 tests/cases/conformance/moduleResolution/untypedModuleImport_noLocalImports.ts create mode 100644 tests/cases/conformance/moduleResolution/untypedModuleImport_vsAmbient.ts create mode 100644 tests/cases/fourslash/untypedModuleImport.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4aee170f195d4..f9c00b5c1587f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1069,7 +1069,7 @@ namespace ts { const moduleSymbol = resolveExternalModuleName(node, (node.parent).moduleSpecifier); if (moduleSymbol) { - const exportDefaultSymbol = isShorthandAmbientModuleSymbol(moduleSymbol) ? + const exportDefaultSymbol = isUntypedModuleSymbol(moduleSymbol) ? moduleSymbol : moduleSymbol.exports["export="] ? getPropertyOfType(getTypeOfSymbol(moduleSymbol.exports["export="]), "default") : @@ -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; } @@ -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); @@ -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(); + // 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) { @@ -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 { @@ -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; } @@ -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; } } diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 0580f7db541e3..1c14d005cb222 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -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.": { + "category": "Error", + "code": 6144 + }, "Variable '{0}' implicitly has an '{1}' type.": { "category": "Error", "code": 7005 diff --git a/src/compiler/program.ts b/src/compiler/program.ts index fab35141ba4af..9e89644f4f18e 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -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) { @@ -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) { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 3da6cf7aa3713..bbffdd470d790 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -406,8 +406,9 @@ namespace ts { ((node).name.kind === SyntaxKind.StringLiteral || isGlobalScopeAugmentation(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 { diff --git a/src/harness/harness.ts b/src/harness/harness.ts index 47b76793af6af..17e67b053a3ee 100644 --- a/src/harness/harness.ts +++ b/src/harness/harness.ts @@ -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(option, value, errors); - break; - default: - options[option.name] = ts.parseCustomTypeOption(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}'.`); } @@ -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(option, value, errors); + default: + return ts.parseCustomTypeOption(option, value, errors); + } + } + export interface TestFile { unitName: string; content: string; diff --git a/tests/baselines/reference/untypedModuleImport.js b/tests/baselines/reference/untypedModuleImport.js new file mode 100644 index 0000000000000..e3548856411ee --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport.js @@ -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()); diff --git a/tests/baselines/reference/untypedModuleImport.symbols b/tests/baselines/reference/untypedModuleImport.symbols new file mode 100644 index 0000000000000..2cc04ed6efab2 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport.symbols @@ -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)) + diff --git a/tests/baselines/reference/untypedModuleImport.types b/tests/baselines/reference/untypedModuleImport.types new file mode 100644 index 0000000000000..8a0c7e97ede11 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport.types @@ -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 + diff --git a/tests/baselines/reference/untypedModuleImport_allowJs.js b/tests/baselines/reference/untypedModuleImport_allowJs.js new file mode 100644 index 0000000000000..8ca5115a386e2 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_allowJs.js @@ -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(); diff --git a/tests/baselines/reference/untypedModuleImport_allowJs.symbols b/tests/baselines/reference/untypedModuleImport_allowJs.symbols new file mode 100644 index 0000000000000..d660e8116309d --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_allowJs.symbols @@ -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)) + diff --git a/tests/baselines/reference/untypedModuleImport_allowJs.types b/tests/baselines/reference/untypedModuleImport_allowJs.types new file mode 100644 index 0000000000000..5a34fdcb646ff --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_allowJs.types @@ -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 + diff --git a/tests/baselines/reference/untypedModuleImport_noImplicitAny.errors.txt b/tests/baselines/reference/untypedModuleImport_noImplicitAny.errors.txt new file mode 100644 index 0000000000000..7f846c1cd2e7f --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_noImplicitAny.errors.txt @@ -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. + \ No newline at end of file diff --git a/tests/baselines/reference/untypedModuleImport_noImplicitAny.js b/tests/baselines/reference/untypedModuleImport_noImplicitAny.js new file mode 100644 index 0000000000000..ab1e1940e3625 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_noImplicitAny.js @@ -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"; diff --git a/tests/baselines/reference/untypedModuleImport_noLocalImports.errors.txt b/tests/baselines/reference/untypedModuleImport_noLocalImports.errors.txt new file mode 100644 index 0000000000000..0f376ea61ec20 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_noLocalImports.errors.txt @@ -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. + \ No newline at end of file diff --git a/tests/baselines/reference/untypedModuleImport_noLocalImports.js b/tests/baselines/reference/untypedModuleImport_noLocalImports.js new file mode 100644 index 0000000000000..8fa67a0857f21 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_noLocalImports.js @@ -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"; diff --git a/tests/baselines/reference/untypedModuleImport_vsAmbient.js b/tests/baselines/reference/untypedModuleImport_vsAmbient.js new file mode 100644 index 0000000000000..080cd4433754b --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_vsAmbient.js @@ -0,0 +1,23 @@ +//// [tests/cases/conformance/moduleResolution/untypedModuleImport_vsAmbient.ts] //// + +//// [index.js] +// This tests that an ambient module declaration overrides an untyped import. + +This file is not processed. + +//// [declarations.d.ts] +declare module "foo" { + export const x: number; +} + +//// [a.ts] +/// +import { x } from "foo"; +x; + + +//// [a.js] +"use strict"; +/// +var foo_1 = require("foo"); +foo_1.x; diff --git a/tests/baselines/reference/untypedModuleImport_vsAmbient.symbols b/tests/baselines/reference/untypedModuleImport_vsAmbient.symbols new file mode 100644 index 0000000000000..8def3c6a09b6c --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_vsAmbient.symbols @@ -0,0 +1,14 @@ +=== /a.ts === +/// +import { x } from "foo"; +>x : Symbol(x, Decl(a.ts, 1, 8)) + +x; +>x : Symbol(x, Decl(a.ts, 1, 8)) + +=== /declarations.d.ts === +declare module "foo" { + export const x: number; +>x : Symbol(x, Decl(declarations.d.ts, 1, 16)) +} + diff --git a/tests/baselines/reference/untypedModuleImport_vsAmbient.types b/tests/baselines/reference/untypedModuleImport_vsAmbient.types new file mode 100644 index 0000000000000..58b0eabefdfe3 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_vsAmbient.types @@ -0,0 +1,14 @@ +=== /a.ts === +/// +import { x } from "foo"; +>x : number + +x; +>x : number + +=== /declarations.d.ts === +declare module "foo" { + export const x: number; +>x : number +} + diff --git a/tests/cases/conformance/moduleResolution/untypedModuleImport.ts b/tests/cases/conformance/moduleResolution/untypedModuleImport.ts new file mode 100644 index 0000000000000..2ea07db3ee496 --- /dev/null +++ b/tests/cases/conformance/moduleResolution/untypedModuleImport.ts @@ -0,0 +1,21 @@ +// @noImplicitReferences: true +// @currentDirectory: / +// This tests that importing from a JS file globally works in an untyped way. +// (Assuming we don't have `--noImplicitAny` or `--allowJs`.) + +// @filename: /node_modules/foo/index.js +This file is not processed. + +// @filename: /a.ts +import * as foo from "foo"; +foo.bar(); + +// @filename: /b.ts +import foo = require("foo"); +foo(); + +// @filename: /c.ts +import foo, { bar } from "foo"; +import "./a"; +import "./b"; +foo(bar()); diff --git a/tests/cases/conformance/moduleResolution/untypedModuleImport_allowJs.ts b/tests/cases/conformance/moduleResolution/untypedModuleImport_allowJs.ts new file mode 100644 index 0000000000000..fe509189d9667 --- /dev/null +++ b/tests/cases/conformance/moduleResolution/untypedModuleImport_allowJs.ts @@ -0,0 +1,12 @@ +// @noImplicitReferences: true +// @currentDirectory: / +// @allowJs: true +// @maxNodeModuleJsDepth: 1 +// Same as untypedModuleImport.ts but with --allowJs, so the package will actually be typed. + +// @filename: /node_modules/foo/index.js +exports.default = { bar() { return 0; } } + +// @filename: /a.ts +import foo from "foo"; +foo.bar(); diff --git a/tests/cases/conformance/moduleResolution/untypedModuleImport_noImplicitAny.ts b/tests/cases/conformance/moduleResolution/untypedModuleImport_noImplicitAny.ts new file mode 100644 index 0000000000000..1aee7c069dedf --- /dev/null +++ b/tests/cases/conformance/moduleResolution/untypedModuleImport_noImplicitAny.ts @@ -0,0 +1,10 @@ +// @noImplicitReferences: true +// @currentDirectory: / +// @noImplicitAny: true +// This tests that `--noImplicitAny` disables untyped modules. + +// @filename: /node_modules/foo/index.js +This file is not processed. + +// @filename: /a.ts +import * as foo from "foo"; diff --git a/tests/cases/conformance/moduleResolution/untypedModuleImport_noLocalImports.ts b/tests/cases/conformance/moduleResolution/untypedModuleImport_noLocalImports.ts new file mode 100644 index 0000000000000..8313628e6d7ee --- /dev/null +++ b/tests/cases/conformance/moduleResolution/untypedModuleImport_noLocalImports.ts @@ -0,0 +1,9 @@ +// @noImplicitReferences: true +// @currentDirectory: / +// This tests that untyped module imports don't happen with local imports. + +// @filename: /foo.js +This file is not processed. + +// @filename: /a.ts +import * as foo from "./foo"; diff --git a/tests/cases/conformance/moduleResolution/untypedModuleImport_vsAmbient.ts b/tests/cases/conformance/moduleResolution/untypedModuleImport_vsAmbient.ts new file mode 100644 index 0000000000000..e157772a5edad --- /dev/null +++ b/tests/cases/conformance/moduleResolution/untypedModuleImport_vsAmbient.ts @@ -0,0 +1,16 @@ +// @noImplicitReferences: true +// @currentDirectory: / +// This tests that an ambient module declaration overrides an untyped import. + +// @filename: /node_modules/foo/index.js +This file is not processed. + +// @filename: /declarations.d.ts +declare module "foo" { + export const x: number; +} + +// @filename: /a.ts +/// +import { x } from "foo"; +x; diff --git a/tests/cases/fourslash/untypedModuleImport.ts b/tests/cases/fourslash/untypedModuleImport.ts new file mode 100644 index 0000000000000..3fd209d480d31 --- /dev/null +++ b/tests/cases/fourslash/untypedModuleImport.ts @@ -0,0 +1,22 @@ +/// + +// @Filename: node_modules/foo/index.js +////{} + +// @Filename: a.ts +////import /*foo*/[|foo|] from /*fooModule*/"foo"; +////[|foo|](); + +goTo.file("a.ts"); +debug.printErrorList(); +verify.numberOfErrorsInCurrentFile(0); + +goTo.marker("fooModule"); +verify.goToDefinitionIs([]); +verify.quickInfoIs('module "foo"'); +verify.referencesAre([]) + +goTo.marker("foo"); +verify.goToDefinitionIs([]); +verify.quickInfoIs("import foo"); +verify.rangesReferenceEachOther(); From ca09ef4499d845c0f9d44df898ccd3b842391d24 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 27 Oct 2016 06:36:39 -0700 Subject: [PATCH 3/6] Simplify for loops in fourslash.ts --- src/compiler/core.ts | 7 ++ src/harness/fourslash.ts | 141 +++++++++++++-------------------------- 2 files changed, 55 insertions(+), 93 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index c143caa974bfb..7d3949e981fb4 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -124,6 +124,13 @@ namespace ts { return undefined; } + export function zip(arrayA: T[], arrayB: U[], callback: (a: T, b: U, index: number) => void): void { + Debug.assert(arrayA.length === arrayB.length); + for (let i = 0; i < arrayA.length; i++) { + callback(arrayA[i], arrayB[i], i); + } + } + /** * Iterates through `array` by index and performs the callback on each element of array until the callback * returns a falsey value, then returns false. diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index e9fa5d6be2e80..b50c12b816f54 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -438,9 +438,8 @@ namespace FourSlash { private getAllDiagnostics(): ts.Diagnostic[] { const diagnostics: ts.Diagnostic[] = []; - const fileNames = this.languageServiceAdapterHost.getFilenames(); - for (let i = 0, n = fileNames.length; i < n; i++) { - diagnostics.push.apply(this.getDiagnostics(fileNames[i])); + for (const fileName of this.languageServiceAdapterHost.getFilenames()) { + diagnostics.push.apply(this.getDiagnostics(fileName)); } return diagnostics; @@ -580,12 +579,12 @@ namespace FourSlash { this.raiseError(`goToDefinitions failed - expected to find ${endMarkers.length} definitions but got ${definitions.length}`); } - for (let i = 0; i < endMarkers.length; i++) { - const marker = this.getMarkerByName(endMarkers[i]), definition = definitions[i]; + ts.zip(endMarkers, definitions, (endMarker, definition, i) => { + const marker = this.getMarkerByName(endMarker); if (marker.fileName !== definition.fileName || marker.position !== definition.textSpan.start) { this.raiseError(`goToDefinition failed for definition ${i}: expected ${marker.fileName} at ${marker.position}, got ${definition.fileName} at ${definition.textSpan.start}`); } - } + }); } public verifyGetEmitOutputForCurrentFile(expected: string): void { @@ -602,10 +601,10 @@ namespace FourSlash { public verifyGetEmitOutputContentsForCurrentFile(expected: ts.OutputFile[]): void { const emit = this.languageService.getEmitOutput(this.activeFile.fileName); assert.equal(emit.outputFiles.length, expected.length, "Number of emit output files"); - for (let i = 0; i < emit.outputFiles.length; i++) { - assert.equal(emit.outputFiles[i].name, expected[i].name, "FileName"); - assert.equal(emit.outputFiles[i].text, expected[i].text, "Content"); - } + ts.zip(emit.outputFiles, expected, (outputFile, expected) => { + assert.equal(outputFile.name, expected.name, "FileName"); + assert.equal(outputFile.text, expected.text, "Content"); + }); } public verifyMemberListContains(symbol: string, text?: string, documentation?: string, kind?: string) { @@ -668,9 +667,9 @@ namespace FourSlash { const entries = this.getCompletionListAtCaret().entries; assert.isTrue(items.length <= entries.length, `Amount of expected items in completion list [ ${items.length} ] is greater than actual number of items in list [ ${entries.length} ]`); - for (let i = 0; i < items.length; i++) { - assert.equal(entries[i].name, items[i], `Unexpected item in completion list`); - } + ts.zip(entries, items, (entry, item) => { + assert.equal(entry.name, item, `Unexpected item in completion list`); + }); } public noItemsWithSameNameButDifferentKind(): void { @@ -692,15 +691,7 @@ namespace FourSlash { this.raiseError("Member list is empty at Caret"); } else if ((members && members.entries.length !== 0) && !negative) { - - let errorMsg = "\n" + "Member List contains: [" + members.entries[0].name; - for (let i = 1; i < members.entries.length; i++) { - errorMsg += ", " + members.entries[i].name; - } - errorMsg += "]\n"; - - this.raiseError("Member list is not empty at Caret: " + errorMsg); - + this.raiseError(`Member list is not empty at Caret:\nMember List contains: ${stringify(members.entries.map(e => e.name))}`); } } @@ -710,13 +701,8 @@ namespace FourSlash { this.raiseError("Completion list is empty at caret at position " + this.activeFile.fileName + " " + this.currentCaretPosition); } else if (completions && completions.entries.length !== 0 && !negative) { - let errorMsg = "\n" + "Completion List contains: [" + completions.entries[0].name; - for (let i = 1; i < completions.entries.length; i++) { - errorMsg += ", " + completions.entries[i].name; - } - errorMsg += "]\n"; - - this.raiseError("Completion list is not empty at caret at position " + this.activeFile.fileName + " " + this.currentCaretPosition + errorMsg); + this.raiseError(`Completion list is not empty at caret at position ${this.activeFile.fileName} ${this.currentCaretPosition}\n` + + `Completion List contains: ${stringify(completions.entries.map(e => e.name))}`); } } @@ -890,8 +876,7 @@ namespace FourSlash { } private verifyReferencesWorker(references: ts.ReferenceEntry[], fileName: string, start: number, end: number, isWriteAccess?: boolean, isDefinition?: boolean) { - for (let i = 0; i < references.length; i++) { - const reference = references[i]; + for (const reference of references) { if (reference && reference.fileName === fileName && reference.textSpan.start === start && ts.textSpanEnd(reference.textSpan) === end) { if (typeof isWriteAccess !== "undefined" && reference.isWriteAccess !== isWriteAccess) { this.raiseError(`verifyReferencesAtPositionListContains failed - item isWriteAccess value does not match, actual: ${reference.isWriteAccess}, expected: ${isWriteAccess}.`); @@ -1008,16 +993,11 @@ namespace FourSlash { ranges = ranges.sort((r1, r2) => r1.start - r2.start); references = references.sort((r1, r2) => r1.textSpan.start - r2.textSpan.start); - for (let i = 0, n = ranges.length; i < n; i++) { - const reference = references[i]; - const range = ranges[i]; - - if (reference.textSpan.start !== range.start || - ts.textSpanEnd(reference.textSpan) !== range.end) { - + ts.zip(references, ranges, (reference, range) => { + if (reference.textSpan.start !== range.start || ts.textSpanEnd(reference.textSpan) !== range.end) { this.raiseError("Rename location results do not match.\n\nExpected: " + stringify(ranges) + "\n\nActual:" + JSON.stringify(references)); } - } + }); } else { this.raiseError("Expected rename to succeed, but it actually failed."); @@ -1247,8 +1227,7 @@ namespace FourSlash { const emitFiles: FourSlashFile[] = []; // List of FourSlashFile that has emitThisFile flag on const allFourSlashFiles = this.testData.files; - for (let idx = 0; idx < allFourSlashFiles.length; idx++) { - const file = allFourSlashFiles[idx]; + for (const file of allFourSlashFiles) { if (file.fileOptions[metadataOptionNames.emitThisFile] === "true") { // Find a file with the flag emitThisFile turned on emitFiles.push(file); @@ -1273,8 +1252,8 @@ namespace FourSlash { if (emitOutput.emitSkipped) { resultString += "Diagnostics:" + Harness.IO.newLine(); const diagnostics = ts.getPreEmitDiagnostics(this.languageService.getProgram()); - for (let i = 0, n = diagnostics.length; i < n; i++) { - resultString += " " + diagnostics[0].messageText + Harness.IO.newLine(); + for (const diagnostic of diagnostics) { + resultString += " " + diagnostic.messageText + Harness.IO.newLine(); } } @@ -1340,8 +1319,7 @@ namespace FourSlash { } public printCurrentFileState(makeWhitespaceVisible = false, makeCaretVisible = true) { - for (let i = 0; i < this.testData.files.length; i++) { - const file = this.testData.files[i]; + for (const file of this.testData.files) { const active = (this.activeFile === file); Harness.IO.log(`=== Script (${file.fileName}) ${(active ? "(active, cursor at |)" : "")} ===`); let content = this.getFileContent(file.fileName); @@ -1576,10 +1554,10 @@ namespace FourSlash { edits = edits.sort((a, b) => a.span.start - b.span.start); // Get a snapshot of the content of the file so we can make sure any formatting edits didn't destroy non-whitespace characters const oldContent = this.getFileContent(this.activeFile.fileName); - for (let j = 0; j < edits.length; j++) { - this.languageServiceAdapterHost.editScript(fileName, edits[j].span.start + runningOffset, ts.textSpanEnd(edits[j].span) + runningOffset, edits[j].newText); - this.updateMarkersForEdit(fileName, edits[j].span.start + runningOffset, ts.textSpanEnd(edits[j].span) + runningOffset, edits[j].newText); - const change = (edits[j].span.start - ts.textSpanEnd(edits[j].span)) + edits[j].newText.length; + for (const edit of edits) { + this.languageServiceAdapterHost.editScript(fileName, edit.span.start + runningOffset, ts.textSpanEnd(edit.span) + runningOffset, edit.newText); + this.updateMarkersForEdit(fileName, edit.span.start + runningOffset, ts.textSpanEnd(edit.span) + runningOffset, edit.newText); + const change = (edit.span.start - ts.textSpanEnd(edit.span)) + edit.newText.length; runningOffset += change; // TODO: Consider doing this at least some of the time for higher fidelity. Currently causes a failure (bug 707150) // this.languageService.getScriptLexicalStructure(fileName); @@ -1913,10 +1891,7 @@ namespace FourSlash { jsonMismatchString()); } - for (let i = 0; i < expected.length; i++) { - const expectedClassification = expected[i]; - const actualClassification = actual[i]; - + ts.zip(expected, actual, (expectedClassification, actualClassification) => { const expectedType: string = (ts.ClassificationTypeNames)[expectedClassification.classificationType]; if (expectedType !== actualClassification.classificationType) { this.raiseError("verifyClassifications failed - expected classifications type to be " + @@ -1946,7 +1921,7 @@ namespace FourSlash { actualText + jsonMismatchString()); } - } + }); function jsonMismatchString() { return Harness.IO.newLine() + @@ -1991,13 +1966,11 @@ namespace FourSlash { this.raiseError(`verifyOutliningSpans failed - expected total spans to be ${spans.length}, but was ${actual.length}`); } - for (let i = 0; i < spans.length; i++) { - const expectedSpan = spans[i]; - const actualSpan = actual[i]; + ts.zip(spans, actual, (expectedSpan, actualSpan, i) => { if (expectedSpan.start !== actualSpan.textSpan.start || expectedSpan.end !== ts.textSpanEnd(actualSpan.textSpan)) { this.raiseError(`verifyOutliningSpans failed - span ${(i + 1)} expected: (${expectedSpan.start},${expectedSpan.end}), actual: (${actualSpan.textSpan.start},${ts.textSpanEnd(actualSpan.textSpan)})`); } - } + }); } public verifyTodoComments(descriptors: string[], spans: TextSpan[]) { @@ -2008,15 +1981,13 @@ namespace FourSlash { this.raiseError(`verifyTodoComments failed - expected total spans to be ${spans.length}, but was ${actual.length}`); } - for (let i = 0; i < spans.length; i++) { - const expectedSpan = spans[i]; - const actualComment = actual[i]; + ts.zip(spans, actual, (expectedSpan, actualComment, i) => { const actualCommentSpan = ts.createTextSpan(actualComment.position, actualComment.message.length); if (expectedSpan.start !== actualCommentSpan.start || expectedSpan.end !== ts.textSpanEnd(actualCommentSpan)) { this.raiseError(`verifyOutliningSpans failed - span ${(i + 1)} expected: (${expectedSpan.start},${expectedSpan.end}), actual: (${actualCommentSpan.start},${ts.textSpanEnd(actualCommentSpan)})`); } - } + }); } private getCodeFixes(errorCode?: number) { @@ -2163,11 +2134,9 @@ namespace FourSlash { public verifyNavigationItemsCount(expected: number, searchValue: string, matchKind?: string, fileName?: string) { const items = this.languageService.getNavigateToItems(searchValue, /*maxResultCount*/ undefined, fileName); let actual = 0; - let item: ts.NavigateToItem; // Count only the match that match the same MatchKind - for (let i = 0; i < items.length; i++) { - item = items[i]; + for (const item of items) { if (!matchKind || item.matchKind === matchKind) { actual++; } @@ -2195,8 +2164,7 @@ namespace FourSlash { this.raiseError("verifyNavigationItemsListContains failed - found 0 navigation items, expected at least one."); } - for (let i = 0; i < items.length; i++) { - const item = items[i]; + for (const item of items) { if (item && item.name === name && item.kind === kind && (matchKind === undefined || item.matchKind === matchKind) && (fileName === undefined || item.fileName === fileName) && @@ -2247,24 +2215,16 @@ namespace FourSlash { public printNavigationItems(searchValue: string) { const items = this.languageService.getNavigateToItems(searchValue); - const length = items && items.length; - - Harness.IO.log(`NavigationItems list (${length} items)`); - - for (let i = 0; i < length; i++) { - const item = items[i]; + Harness.IO.log(`NavigationItems list (${items.length} items)`); + for (const item of items) { Harness.IO.log(`name: ${item.name}, kind: ${item.kind}, parentName: ${item.containerName}, fileName: ${item.fileName}`); } } public printNavigationBar() { const items = this.languageService.getNavigationBarItems(this.activeFile.fileName); - const length = items && items.length; - - Harness.IO.log(`Navigation bar (${length} items)`); - - for (let i = 0; i < length; i++) { - const item = items[i]; + Harness.IO.log(`Navigation bar (${items.length} items)`); + for (const item of items) { Harness.IO.log(`${repeatString(item.indent, " ")}name: ${item.text}, kind: ${item.kind}, childItems: ${item.childItems.map(child => child.text)}`); } } @@ -2385,8 +2345,7 @@ namespace FourSlash { } private assertItemInCompletionList(items: ts.CompletionEntry[], name: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) { - for (let i = 0; i < items.length; i++) { - const item = items[i]; + for (const item of items) { if (item.name === name) { if (documentation != undefined || text !== undefined) { const details = this.getCompletionEntryDetails(item.name); @@ -2435,20 +2394,17 @@ namespace FourSlash { name = name.indexOf("/") === -1 ? (this.basePath + "/" + name) : name; const availableNames: string[] = []; - let foundIt = false; - for (let i = 0; i < this.testData.files.length; i++) { - const fn = this.testData.files[i].fileName; + result = ts.forEach(this.testData.files, file => { + const fn = file.fileName; if (fn) { if (fn === name) { - result = this.testData.files[i]; - foundIt = true; - break; + return file; } availableNames.push(fn); } - } + }); - if (!foundIt) { + if (!result) { throw new Error(`No test file named "${name}" exists. Available file names are: ${availableNames.join(", ")}`); } } @@ -2549,8 +2505,8 @@ ${code} function chompLeadingSpace(content: string) { const lines = content.split("\n"); - for (let i = 0; i < lines.length; i++) { - if ((lines[i].length !== 0) && (lines[i].charAt(0) !== " ")) { + for (const line of lines) { + if ((line.length !== 0) && (line.charAt(0) !== " ")) { return content; } } @@ -2588,8 +2544,7 @@ ${code} currentFileName = fileName; } - for (let i = 0; i < lines.length; i++) { - let line = lines[i]; + for (let line of lines) { const lineLength = line.length; if (lineLength > 0 && line.charAt(lineLength - 1) === "\r") { From eb45962e20486984b3193a7cd0f2fc4a4bf6b7c2 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 27 Oct 2016 10:26:46 -0700 Subject: [PATCH 4/6] Rename to zipWith --- src/compiler/core.ts | 2 +- src/harness/fourslash.ts | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 7d3949e981fb4..f207cf010e2a5 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -124,7 +124,7 @@ namespace ts { return undefined; } - export function zip(arrayA: T[], arrayB: U[], callback: (a: T, b: U, index: number) => void): void { + export function zipWith(arrayA: T[], arrayB: U[], callback: (a: T, b: U, index: number) => void): void { Debug.assert(arrayA.length === arrayB.length); for (let i = 0; i < arrayA.length; i++) { callback(arrayA[i], arrayB[i], i); diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index b50c12b816f54..b008ca653702d 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -579,7 +579,7 @@ namespace FourSlash { this.raiseError(`goToDefinitions failed - expected to find ${endMarkers.length} definitions but got ${definitions.length}`); } - ts.zip(endMarkers, definitions, (endMarker, definition, i) => { + ts.zipWith(endMarkers, definitions, (endMarker, definition, i) => { const marker = this.getMarkerByName(endMarker); if (marker.fileName !== definition.fileName || marker.position !== definition.textSpan.start) { this.raiseError(`goToDefinition failed for definition ${i}: expected ${marker.fileName} at ${marker.position}, got ${definition.fileName} at ${definition.textSpan.start}`); @@ -601,7 +601,7 @@ namespace FourSlash { public verifyGetEmitOutputContentsForCurrentFile(expected: ts.OutputFile[]): void { const emit = this.languageService.getEmitOutput(this.activeFile.fileName); assert.equal(emit.outputFiles.length, expected.length, "Number of emit output files"); - ts.zip(emit.outputFiles, expected, (outputFile, expected) => { + ts.zipWith(emit.outputFiles, expected, (outputFile, expected) => { assert.equal(outputFile.name, expected.name, "FileName"); assert.equal(outputFile.text, expected.text, "Content"); }); @@ -667,7 +667,7 @@ namespace FourSlash { const entries = this.getCompletionListAtCaret().entries; assert.isTrue(items.length <= entries.length, `Amount of expected items in completion list [ ${items.length} ] is greater than actual number of items in list [ ${entries.length} ]`); - ts.zip(entries, items, (entry, item) => { + ts.zipWith(entries, items, (entry, item) => { assert.equal(entry.name, item, `Unexpected item in completion list`); }); } @@ -993,7 +993,7 @@ namespace FourSlash { ranges = ranges.sort((r1, r2) => r1.start - r2.start); references = references.sort((r1, r2) => r1.textSpan.start - r2.textSpan.start); - ts.zip(references, ranges, (reference, range) => { + ts.zipWith(references, ranges, (reference, range) => { if (reference.textSpan.start !== range.start || ts.textSpanEnd(reference.textSpan) !== range.end) { this.raiseError("Rename location results do not match.\n\nExpected: " + stringify(ranges) + "\n\nActual:" + JSON.stringify(references)); } @@ -1891,7 +1891,7 @@ namespace FourSlash { jsonMismatchString()); } - ts.zip(expected, actual, (expectedClassification, actualClassification) => { + ts.zipWith(expected, actual, (expectedClassification, actualClassification) => { const expectedType: string = (ts.ClassificationTypeNames)[expectedClassification.classificationType]; if (expectedType !== actualClassification.classificationType) { this.raiseError("verifyClassifications failed - expected classifications type to be " + @@ -1966,7 +1966,7 @@ namespace FourSlash { this.raiseError(`verifyOutliningSpans failed - expected total spans to be ${spans.length}, but was ${actual.length}`); } - ts.zip(spans, actual, (expectedSpan, actualSpan, i) => { + ts.zipWith(spans, actual, (expectedSpan, actualSpan, i) => { if (expectedSpan.start !== actualSpan.textSpan.start || expectedSpan.end !== ts.textSpanEnd(actualSpan.textSpan)) { this.raiseError(`verifyOutliningSpans failed - span ${(i + 1)} expected: (${expectedSpan.start},${expectedSpan.end}), actual: (${actualSpan.textSpan.start},${ts.textSpanEnd(actualSpan.textSpan)})`); } @@ -1981,7 +1981,7 @@ namespace FourSlash { this.raiseError(`verifyTodoComments failed - expected total spans to be ${spans.length}, but was ${actual.length}`); } - ts.zip(spans, actual, (expectedSpan, actualComment, i) => { + ts.zipWith(spans, actual, (expectedSpan, actualComment, i) => { const actualCommentSpan = ts.createTextSpan(actualComment.position, actualComment.message.length); if (expectedSpan.start !== actualCommentSpan.start || expectedSpan.end !== ts.textSpanEnd(actualCommentSpan)) { From ce099e5374cd0a1b598728fd4aa2dfe6caceeb66 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 27 Oct 2016 11:10:18 -0700 Subject: [PATCH 5/6] Change diagnostic message --- src/compiler/checker.ts | 2 +- src/compiler/diagnosticMessages.json | 8 ++++---- .../untypedModuleImport_noImplicitAny.errors.txt | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f9c00b5c1587f..4f8f9b3f68c7c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1401,7 +1401,7 @@ namespace ts { 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, + Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type, moduleReference, resolvedModule.resolvedFileName); } diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 1c14d005cb222..e2a2a4b9a0cf4 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -2869,10 +2869,6 @@ "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.": { - "category": "Error", - "code": 6144 - }, "Variable '{0}' implicitly has an '{1}' type.": { "category": "Error", "code": 7005 @@ -2905,6 +2901,10 @@ "category": "Error", "code": 7015 }, + "Could not find a declaration file for module '{0}'. '{1}' implicitly has an 'any' type.": { + "category": "Error", + "code": 7016 + }, "Index signature of object type implicitly has an 'any' type.": { "category": "Error", "code": 7017 diff --git a/tests/baselines/reference/untypedModuleImport_noImplicitAny.errors.txt b/tests/baselines/reference/untypedModuleImport_noImplicitAny.errors.txt index 7f846c1cd2e7f..349a944deebb2 100644 --- a/tests/baselines/reference/untypedModuleImport_noImplicitAny.errors.txt +++ b/tests/baselines/reference/untypedModuleImport_noImplicitAny.errors.txt @@ -1,10 +1,10 @@ -/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,22): error TS7016: Could not find a declaration file for module 'foo'. '/node_modules/foo/index.js' implicitly has an 'any' type. ==== /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. +!!! error TS7016: Could not find a declaration file for module 'foo'. '/node_modules/foo/index.js' implicitly has an 'any' type. ==== /node_modules/foo/index.js (0 errors) ==== // This tests that `--noImplicitAny` disables untyped modules. From 0f8003fb39392b93b94d493ea5925d4e9f320fb5 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 27 Oct 2016 11:33:01 -0700 Subject: [PATCH 6/6] Make `extension` property of `ResolvedModule` optional; introduce `ResolvedModuleFull` interface for when the extension is provided. --- src/compiler/moduleNameResolver.ts | 2 +- src/compiler/program.ts | 16 ++++++++-------- src/compiler/types.ts | 23 ++++++++++++++++------- src/compiler/utilities.ts | 12 ++++-------- src/harness/unittests/moduleResolution.ts | 6 +++--- src/server/lsHost.ts | 2 +- src/services/services.ts | 2 +- src/services/shims.ts | 2 +- 8 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 1b0a579180270..9e159a356c7c7 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -45,7 +45,7 @@ namespace ts { } /** Adds `isExernalLibraryImport` to a Resolved to get a ResolvedModule. */ - function resolvedModuleFromResolved({ path, extension }: Resolved, isExternalLibraryImport: boolean): ResolvedModule { + function resolvedModuleFromResolved({ path, extension }: Resolved, isExternalLibraryImport: boolean): ResolvedModuleFull { return { resolvedFileName: path, extension, isExternalLibraryImport }; } diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 6b5a09bfbc3e8..fc481510e8568 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -329,16 +329,16 @@ namespace ts { // Map storing if there is emit blocking diagnostics for given input const hasEmitBlockingDiagnostics = createFileMap(getCanonicalFileName); - let resolveModuleNamesWorker: (moduleNames: string[], containingFile: string) => ResolvedModule[]; + let resolveModuleNamesWorker: (moduleNames: string[], containingFile: string) => ResolvedModuleFull[]; if (host.resolveModuleNames) { resolveModuleNamesWorker = (moduleNames, containingFile) => host.resolveModuleNames(moduleNames, containingFile).map(resolved => { // An older host may have omitted extension, in which case we should infer it from the file extension of resolvedFileName. - if (!resolved || resolved.extension !== undefined) { - return resolved; + if (!resolved || (resolved as ResolvedModuleFull).extension !== undefined) { + return resolved as ResolvedModuleFull; } - resolved = clone(resolved); - resolved.extension = extensionFromPath(resolved.resolvedFileName); - return resolved; + const withExtension = clone(resolved) as ResolvedModuleFull; + withExtension.extension = extensionFromPath(resolved.resolvedFileName); + return withExtension; }); } else { @@ -1294,7 +1294,7 @@ namespace ts { function processImportedModules(file: SourceFile) { collectExternalModuleReferences(file); if (file.imports.length || file.moduleAugmentations.length) { - file.resolvedModules = createMap(); + file.resolvedModules = createMap(); const moduleNames = map(concatenate(file.imports, file.moduleAugmentations), getTextOfLiteral); const resolutions = resolveModuleNamesWorker(moduleNames, getNormalizedAbsolutePath(file.fileName, currentDirectory)); Debug.assert(resolutions.length === moduleNames.length); @@ -1571,7 +1571,7 @@ namespace ts { * Returns a DiagnosticMessage if we can't use a resolved module due to its extension. * The DiagnosticMessage's parameters are the imported module name, and the filename it resolved to. */ - export function getResolutionDiagnostic(options: CompilerOptions, { extension }: ResolvedModule): DiagnosticMessage | undefined { + export function getResolutionDiagnostic(options: CompilerOptions, { extension }: ResolvedModuleFull): DiagnosticMessage | undefined { switch (extension) { case Extension.Ts: case Extension.Dts: diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 9034ca32e4e91..419d062bb7f9e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -423,7 +423,7 @@ namespace ts { ThisNodeHasError = 1 << 19, // If the parser encountered an error when parsing the code that created this node JavaScriptFile = 1 << 20, // If node was parsed in a JavaScript ThisNodeOrAnySubNodesHasError = 1 << 21, // If this node or any of its children had an error - HasAggregatedChildData = 1 << 22, // If we've computed data from children and cached it in this node + HasAggregatedChildData = 1 << 22, // If we've computed data from children and cached it in this node BlockScoped = Let | Const, @@ -2083,7 +2083,7 @@ namespace ts { // Stores a mapping 'external module reference text' -> 'resolved file name' | undefined // It is used to resolve module names in the checker. // Content of this field should never be used directly - use getResolvedModuleFileName/setResolvedModuleFileName functions instead - /* @internal */ resolvedModules: Map; + /* @internal */ resolvedModules: Map; /* @internal */ resolvedTypeReferenceDirectiveNames: Map; /* @internal */ imports: LiteralExpression[]; /* @internal */ moduleAugmentations: LiteralExpression[]; @@ -3352,14 +3352,11 @@ namespace ts { * Module resolution will pick up tsx/jsx/js files even if '--jsx' and '--allowJs' are turned off. * The Program will then filter results based on these flags. * - * At least one of `resolvedTsFileName` or `resolvedJsFileName` must be defined, - * else resolution should just return `undefined` instead of a ResolvedModule. + * Prefer to return a `ResolvedModuleFull` so that the file type does not have to be inferred. */ export interface ResolvedModule { /** Path of the file the module was resolved to. */ resolvedFileName: string; - /** Extension of resolvedFileName. This must match what's at the end of resolvedFileName. */ - extension: Extension; /** * Denotes if 'resolvedFileName' is isExternalLibraryImport and thus should be a proper external module: * - be a .d.ts file @@ -3369,6 +3366,18 @@ namespace ts { isExternalLibraryImport?: boolean; } + /** + * ResolvedModule with an explicitly provided `extension` property. + * Prefer this over `ResolvedModule`. + */ + export interface ResolvedModuleFull extends ResolvedModule { + /** + * Extension of resolvedFileName. This must match what's at the end of resolvedFileName. + * This is optional for backwards-compatibility, but will be added if not provided. + */ + extension: Extension; + } + export enum Extension { Ts, Tsx, @@ -3379,7 +3388,7 @@ namespace ts { } export interface ResolvedModuleWithFailedLookupLocations { - resolvedModule: ResolvedModule | undefined; + resolvedModule: ResolvedModuleFull | undefined; failedLookupLocations: string[]; } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 3da6cf7aa3713..6b635e37bc389 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -87,13 +87,13 @@ namespace ts { return !!(sourceFile && sourceFile.resolvedModules && sourceFile.resolvedModules[moduleNameText]); } - export function getResolvedModule(sourceFile: SourceFile, moduleNameText: string): ResolvedModule { + export function getResolvedModule(sourceFile: SourceFile, moduleNameText: string): ResolvedModuleFull { return hasResolvedModule(sourceFile, moduleNameText) ? sourceFile.resolvedModules[moduleNameText] : undefined; } - export function setResolvedModule(sourceFile: SourceFile, moduleNameText: string, resolvedModule: ResolvedModule): void { + export function setResolvedModule(sourceFile: SourceFile, moduleNameText: string, resolvedModule: ResolvedModuleFull): void { if (!sourceFile.resolvedModules) { - sourceFile.resolvedModules = createMap(); + sourceFile.resolvedModules = createMap(); } sourceFile.resolvedModules[moduleNameText] = resolvedModule; @@ -108,11 +108,7 @@ namespace ts { } /* @internal */ - /** - * Considers two ResolvedModules equal if they have the same `resolvedFileName`. - * Thus `{ ts: foo, js: bar }` is equal to `{ ts: foo, js: baz }` because `ts` is preferred. - */ - export function moduleResolutionIsEqualTo(oldResolution: ResolvedModule, newResolution: ResolvedModule): boolean { + export function moduleResolutionIsEqualTo(oldResolution: ResolvedModuleFull, newResolution: ResolvedModuleFull): boolean { return oldResolution.isExternalLibraryImport === newResolution.isExternalLibraryImport && oldResolution.extension === newResolution.extension && oldResolution.resolvedFileName === newResolution.resolvedFileName; diff --git a/src/harness/unittests/moduleResolution.ts b/src/harness/unittests/moduleResolution.ts index 0c1934bb01c24..104d9f9e394eb 100644 --- a/src/harness/unittests/moduleResolution.ts +++ b/src/harness/unittests/moduleResolution.ts @@ -1,7 +1,7 @@ /// namespace ts { - export function checkResolvedModule(expected: ResolvedModule, actual: ResolvedModule): boolean { + export function checkResolvedModule(expected: ResolvedModuleFull, actual: ResolvedModuleFull): boolean { if (!expected === !actual) { if (expected) { assert.isTrue(expected.resolvedFileName === actual.resolvedFileName, `'resolvedFileName': expected '${expected.resolvedFileName}' to be equal to '${actual.resolvedFileName}'`); @@ -13,13 +13,13 @@ namespace ts { return false; } - export function checkResolvedModuleWithFailedLookupLocations(actual: ResolvedModuleWithFailedLookupLocations, expectedResolvedModule: ResolvedModule, expectedFailedLookupLocations: string[]): void { + export function checkResolvedModuleWithFailedLookupLocations(actual: ResolvedModuleWithFailedLookupLocations, expectedResolvedModule: ResolvedModuleFull, expectedFailedLookupLocations: string[]): void { assert.isTrue(actual.resolvedModule !== undefined, "module should be resolved"); checkResolvedModule(actual.resolvedModule, expectedResolvedModule); assert.deepEqual(actual.failedLookupLocations, expectedFailedLookupLocations); } - export function createResolvedModule(resolvedFileName: string, isExternalLibraryImport = false): ResolvedModule { + export function createResolvedModule(resolvedFileName: string, isExternalLibraryImport = false): ResolvedModuleFull { return { resolvedFileName, extension: extensionFromPath(resolvedFileName), isExternalLibraryImport }; } diff --git a/src/server/lsHost.ts b/src/server/lsHost.ts index 641f8dbabe742..628de71bb7a91 100644 --- a/src/server/lsHost.ts +++ b/src/server/lsHost.ts @@ -151,7 +151,7 @@ namespace ts.server { m => m.resolvedTypeReferenceDirective, r => r.resolvedFileName, /*logChanges*/ false); } - resolveModuleNames(moduleNames: string[], containingFile: string): ResolvedModule[] { + resolveModuleNames(moduleNames: string[], containingFile: string): ResolvedModuleFull[] { return this.resolveNamesWithLocalCache(moduleNames, containingFile, this.resolvedModuleNames, this.resolveModuleName, m => m.resolvedModule, r => r.resolvedFileName, /*logChanges*/ true); } diff --git a/src/services/services.ts b/src/services/services.ts index 5669134d37e75..8c11c707e5d60 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -467,7 +467,7 @@ namespace ts { public languageVariant: LanguageVariant; public identifiers: Map; public nameTable: Map; - public resolvedModules: Map; + public resolvedModules: Map; public resolvedTypeReferenceDirectiveNames: Map; public imports: LiteralExpression[]; public moduleAugmentations: LiteralExpression[]; diff --git a/src/services/shims.ts b/src/services/shims.ts index a3b99f81158c9..b1fac14674c32 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -316,7 +316,7 @@ namespace ts { private loggingEnabled = false; private tracingEnabled = false; - public resolveModuleNames: (moduleName: string[], containingFile: string) => ResolvedModule[]; + public resolveModuleNames: (moduleName: string[], containingFile: string) => ResolvedModuleFull[]; public resolveTypeReferenceDirectives: (typeDirectiveNames: string[], containingFile: string) => ResolvedTypeReferenceDirective[]; public directoryExists: (directoryName: string) => boolean;