From 157a9b02fc5e9aa825697bd7779145eded74cdf5 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 5 Dec 2016 08:43:16 -0800 Subject: [PATCH 01/11] Properly determine whether an augmentation is a ValueModule or NamespaceModule --- src/compiler/binder.ts | 19 ++++++++++------ .../reference/augmentExportEquals7.errors.txt | 22 +++++++++++++++++++ tests/cases/compiler/augmentExportEquals7.ts | 10 +++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 tests/baselines/reference/augmentExportEquals7.errors.txt create mode 100644 tests/cases/compiler/augmentExportEquals7.ts diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index a6f5993f6c8..e00e9294529 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -1509,7 +1509,7 @@ namespace ts { errorOnFirstToken(node, Diagnostics.export_modifier_cannot_be_applied_to_ambient_modules_and_module_augmentations_since_they_are_always_visible); } if (isExternalModuleAugmentation(node)) { - declareSymbolAndAddToSymbolTable(node, SymbolFlags.NamespaceModule, SymbolFlags.NamespaceModuleExcludes); + declareModuleSymbol(node); } else { let pattern: Pattern | undefined; @@ -1531,12 +1531,8 @@ namespace ts { } } else { - const state = getModuleInstanceState(node); - if (state === ModuleInstanceState.NonInstantiated) { - declareSymbolAndAddToSymbolTable(node, SymbolFlags.NamespaceModule, SymbolFlags.NamespaceModuleExcludes); - } - else { - declareSymbolAndAddToSymbolTable(node, SymbolFlags.ValueModule, SymbolFlags.ValueModuleExcludes); + const state = declareModuleSymbol(node); + if (state !== ModuleInstanceState.NonInstantiated) { if (node.symbol.flags & (SymbolFlags.Function | SymbolFlags.Class | SymbolFlags.RegularEnum)) { // if module was already merged with some function, class or non-const enum // treat is a non-const-enum-only @@ -1557,6 +1553,15 @@ namespace ts { } } + function declareModuleSymbol(node: ModuleDeclaration): ModuleInstanceState { + const state = getModuleInstanceState(node); + const instantiated = state !== ModuleInstanceState.NonInstantiated; + declareSymbolAndAddToSymbolTable(node, + instantiated ? SymbolFlags.ValueModule : SymbolFlags.NamespaceModule, + instantiated ? SymbolFlags.ValueModuleExcludes : SymbolFlags.NamespaceModuleExcludes); + return state; + } + function bindFunctionOrConstructorType(node: SignatureDeclaration): void { // For a given function symbol "<...>(...) => T" we want to generate a symbol identical // to the one we would get for: { <...>(...): T } diff --git a/tests/baselines/reference/augmentExportEquals7.errors.txt b/tests/baselines/reference/augmentExportEquals7.errors.txt new file mode 100644 index 00000000000..1cb9ddc041c --- /dev/null +++ b/tests/baselines/reference/augmentExportEquals7.errors.txt @@ -0,0 +1,22 @@ +/node_modules/@types/lib-extender/index.d.ts(2,16): error TS2300: Duplicate identifier '"lib"'. +/node_modules/lib/index.d.ts(1,13): error TS2300: Duplicate identifier '"lib"'. +/node_modules/lib/index.d.ts(2,19): error TS2300: Duplicate identifier '"lib"'. + + +==== /node_modules/lib/index.d.ts (2 errors) ==== + declare var lib: () => void; + ~~~ +!!! error TS2300: Duplicate identifier '"lib"'. + declare namespace lib {} + ~~~ +!!! error TS2300: Duplicate identifier '"lib"'. + export = lib; + +==== /node_modules/@types/lib-extender/index.d.ts (1 errors) ==== + import * as lib from "lib"; + declare module "lib" { + ~~~~~ +!!! error TS2300: Duplicate identifier '"lib"'. + export function fn(): void; + } + \ No newline at end of file diff --git a/tests/cases/compiler/augmentExportEquals7.ts b/tests/cases/compiler/augmentExportEquals7.ts new file mode 100644 index 00000000000..c287bcbe8c4 --- /dev/null +++ b/tests/cases/compiler/augmentExportEquals7.ts @@ -0,0 +1,10 @@ +// @Filename: /node_modules/lib/index.d.ts +declare var lib: () => void; +declare namespace lib {} +export = lib; + +// @Filename: /node_modules/@types/lib-extender/index.d.ts +import * as lib from "lib"; +declare module "lib" { + export function fn(): void; +} From 7beeb77201450aa8a95d494f3fde167c1fb9b550 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 5 Dec 2016 10:58:47 -0800 Subject: [PATCH 02/11] Update baseline --- .../reference/globalAugmentationModuleResolution.types | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/baselines/reference/globalAugmentationModuleResolution.types b/tests/baselines/reference/globalAugmentationModuleResolution.types index c057ea2943d..75f23b0cd6b 100644 --- a/tests/baselines/reference/globalAugmentationModuleResolution.types +++ b/tests/baselines/reference/globalAugmentationModuleResolution.types @@ -3,7 +3,7 @@ export { }; declare global { ->global : any +>global : typeof global var x: number; >x : number From 39040f61540c2a2ec3cb9a9cc5c2920a3a0188fe Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 7 Dec 2016 13:13:21 -0800 Subject: [PATCH 03/11] Provide better error message --- src/compiler/checker.ts | 3 +++ src/compiler/diagnosticMessages.json | 4 ++++ .../reference/augmentExportEquals7.errors.txt | 12 +++--------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 903c8cfda1c..d2d4502c79d 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -479,6 +479,9 @@ namespace ts { } recordMergedSymbol(target, source); } + else if (target.flags & SymbolFlags.NamespaceModule) { + error(source.valueDeclaration.name, Diagnostics.Cannot_augment_module_0_with_value_exports_because_it_resolves_to_a_non_module_entity, symbolToString(target)); + } else { const message = target.flags & SymbolFlags.BlockScopedVariable || source.flags & SymbolFlags.BlockScopedVariable ? Diagnostics.Cannot_redeclare_block_scoped_variable_0 : Diagnostics.Duplicate_identifier_0; diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 4d42c77e566..19b297fd4a9 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -1807,6 +1807,10 @@ "category": "Error", "code": 2608 }, + "Cannot augment module '{0}' with value exports because it resolves to a non-module entity.": { + "category": "Error", + "code": 2649 + }, "Cannot emit namespaced JSX elements in React": { "category": "Error", "code": 2650 diff --git a/tests/baselines/reference/augmentExportEquals7.errors.txt b/tests/baselines/reference/augmentExportEquals7.errors.txt index 1cb9ddc041c..e13cf037023 100644 --- a/tests/baselines/reference/augmentExportEquals7.errors.txt +++ b/tests/baselines/reference/augmentExportEquals7.errors.txt @@ -1,22 +1,16 @@ -/node_modules/@types/lib-extender/index.d.ts(2,16): error TS2300: Duplicate identifier '"lib"'. -/node_modules/lib/index.d.ts(1,13): error TS2300: Duplicate identifier '"lib"'. -/node_modules/lib/index.d.ts(2,19): error TS2300: Duplicate identifier '"lib"'. +/node_modules/@types/lib-extender/index.d.ts(2,16): error TS2649: Cannot augment module 'lib' with value exports because it resolves to a non-module entity. -==== /node_modules/lib/index.d.ts (2 errors) ==== +==== /node_modules/lib/index.d.ts (0 errors) ==== declare var lib: () => void; - ~~~ -!!! error TS2300: Duplicate identifier '"lib"'. declare namespace lib {} - ~~~ -!!! error TS2300: Duplicate identifier '"lib"'. export = lib; ==== /node_modules/@types/lib-extender/index.d.ts (1 errors) ==== import * as lib from "lib"; declare module "lib" { ~~~~~ -!!! error TS2300: Duplicate identifier '"lib"'. +!!! error TS2649: Cannot augment module 'lib' with value exports because it resolves to a non-module entity. export function fn(): void; } \ No newline at end of file From a478bfddd2233d3579a2b037fc0df32807cb0ebe Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 24 Jan 2017 13:44:36 -0800 Subject: [PATCH 04/11] Simplify code in 'rename' --- src/services/rename.ts | 172 ++++++++++++++++++++--------------------- 1 file changed, 84 insertions(+), 88 deletions(-) diff --git a/src/services/rename.ts b/src/services/rename.ts index 6f28e505f79..afb16618b03 100644 --- a/src/services/rename.ts +++ b/src/services/rename.ts @@ -1,98 +1,94 @@ /* @internal */ namespace ts.Rename { export function getRenameInfo(typeChecker: TypeChecker, defaultLibFileName: string, getCanonicalFileName: (fileName: string) => string, sourceFile: SourceFile, position: number): RenameInfo { - const canonicalDefaultLibName = getCanonicalFileName(ts.normalizePath(defaultLibFileName)); - const node = getTouchingWord(sourceFile, position, /*includeJsDocComment*/ true); + const renameInfo = node && nodeIsEligibleForRename(node) + ? getRenameInfoForNode(node, typeChecker, sourceFile, getIsDefinedInLibraryFile(defaultLibFileName, getCanonicalFileName)) + : undefined; + return renameInfo || getRenameInfoError(Diagnostics.You_cannot_rename_this_element); + } - if (node) { - if (node.kind === SyntaxKind.Identifier || - node.kind === SyntaxKind.StringLiteral || - isLiteralNameOfPropertyDeclarationOrIndexAccess(node) || - isThis(node)) { - const symbol = typeChecker.getSymbolAtLocation(node); - - // Only allow a symbol to be renamed if it actually has at least one declaration. - if (symbol) { - const declarations = symbol.getDeclarations(); - if (declarations && declarations.length > 0) { - // Disallow rename for elements that are defined in the standard TypeScript library. - if (forEach(declarations, isDefinedInLibraryFile)) { - return getRenameInfoError(getLocaleSpecificMessage(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library)); - } - - const displayName = stripQuotes(getDeclaredName(typeChecker, symbol, node)); - const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, node); - if (kind) { - return { - canRename: true, - kind, - displayName, - localizedErrorMessage: undefined, - fullDisplayName: typeChecker.getFullyQualifiedName(symbol), - kindModifiers: SymbolDisplay.getSymbolModifiers(symbol), - triggerSpan: createTriggerSpanForNode(node, sourceFile) - }; - } - } - } - else if (node.kind === SyntaxKind.StringLiteral) { - const type = getStringLiteralTypeForNode(node, typeChecker); - if (type) { - if (isDefinedInLibraryFile(node)) { - return getRenameInfoError(getLocaleSpecificMessage(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library)); - } - else { - const displayName = stripQuotes(type.text); - return { - canRename: true, - kind: ScriptElementKind.variableElement, - displayName, - localizedErrorMessage: undefined, - fullDisplayName: displayName, - kindModifiers: ScriptElementKindModifier.none, - triggerSpan: createTriggerSpanForNode(node, sourceFile) - }; - } - } - } - } + function getIsDefinedInLibraryFile(defaultLibFileName: string, getCanonicalFileName: (fileName: string) => string): (declaration: Node) => boolean { + if (!defaultLibFileName) { + return () => false; } - return getRenameInfoError(getLocaleSpecificMessage(Diagnostics.You_cannot_rename_this_element)); - - function getRenameInfoError(localizedErrorMessage: string): RenameInfo { - return { - canRename: false, - localizedErrorMessage: localizedErrorMessage, - displayName: undefined, - fullDisplayName: undefined, - kind: undefined, - kindModifiers: undefined, - triggerSpan: undefined - }; - } - - function isDefinedInLibraryFile(declaration: Node) { - if (defaultLibFileName) { - const sourceFile = declaration.getSourceFile(); - const canonicalName = getCanonicalFileName(ts.normalizePath(sourceFile.fileName)); - if (canonicalName === canonicalDefaultLibName) { - return true; - } - } - return false; - } - - function createTriggerSpanForNode(node: Node, sourceFile: SourceFile) { - let start = node.getStart(sourceFile); - let width = node.getWidth(sourceFile); - if (node.kind === SyntaxKind.StringLiteral) { - // Exclude the quotes - start += 1; - width -= 2; - } - return createTextSpan(start, width); + const canonicalDefaultLibName = getCanonicalFileName(ts.normalizePath(defaultLibFileName)); + return declaration => { + const sourceFile = declaration.getSourceFile(); + const canonicalName = getCanonicalFileName(ts.normalizePath(sourceFile.fileName)); + return canonicalName === canonicalDefaultLibName; } } + + function getRenameInfoForNode(node: Node, typeChecker: TypeChecker, sourceFile: SourceFile, isDefinedInLibraryFile: (declaration: Node) => boolean): RenameInfo | undefined { + const symbol = typeChecker.getSymbolAtLocation(node); + + // Only allow a symbol to be renamed if it actually has at least one declaration. + if (symbol) { + const declarations = symbol.getDeclarations(); + if (declarations && declarations.length > 0) { + // Disallow rename for elements that are defined in the standard TypeScript library. + if (some(declarations, isDefinedInLibraryFile)) { + return getRenameInfoError(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library); + } + + const displayName = stripQuotes(getDeclaredName(typeChecker, symbol, node)); + const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, node); + return kind ? getRenameInfoSuccess(displayName, typeChecker.getFullyQualifiedName(symbol), kind, SymbolDisplay.getSymbolModifiers(symbol), node, sourceFile) : undefined; + } + } + else if (node.kind === SyntaxKind.StringLiteral) { + const type = getStringLiteralTypeForNode(node, typeChecker); + if (type) { + if (isDefinedInLibraryFile(node)) { + return getRenameInfoError(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library); + } + + const displayName = stripQuotes(type.text); + return getRenameInfoSuccess(displayName, displayName, ScriptElementKind.variableElement, ScriptElementKindModifier.none, node, sourceFile); + } + } + } + + function getRenameInfoSuccess(displayName: string, fullDisplayName: string, kind: string, kindModifiers: string, node: Node, sourceFile: SourceFile): RenameInfo { + return { + canRename: true, + kind, + displayName, + localizedErrorMessage: undefined, + fullDisplayName, + kindModifiers, + triggerSpan: createTriggerSpanForNode(node, sourceFile) + }; + } + + function getRenameInfoError(diagnostic: DiagnosticMessage): RenameInfo { + return { + canRename: false, + localizedErrorMessage: getLocaleSpecificMessage(diagnostic), + displayName: undefined, + fullDisplayName: undefined, + kind: undefined, + kindModifiers: undefined, + triggerSpan: undefined + }; + } + + function createTriggerSpanForNode(node: Node, sourceFile: SourceFile) { + let start = node.getStart(sourceFile); + let width = node.getWidth(sourceFile); + if (node.kind === SyntaxKind.StringLiteral) { + // Exclude the quotes + start += 1; + width -= 2; + } + return createTextSpan(start, width); + } + + function nodeIsEligibleForRename(node: Node) { + return node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.StringLiteral || + isLiteralNameOfPropertyDeclarationOrIndexAccess(node) || + isThis(node); + } } From 33b8677cb52eb54a3d4d8365fb3e6b7eba2c0dec Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 24 Jan 2017 14:32:39 -0800 Subject: [PATCH 05/11] Change "getIsDefinedInLibraryFile" back to just "isDefinedInLibraryFile" --- src/services/rename.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/services/rename.ts b/src/services/rename.ts index afb16618b03..2e9396888ef 100644 --- a/src/services/rename.ts +++ b/src/services/rename.ts @@ -1,23 +1,21 @@ /* @internal */ namespace ts.Rename { export function getRenameInfo(typeChecker: TypeChecker, defaultLibFileName: string, getCanonicalFileName: (fileName: string) => string, sourceFile: SourceFile, position: number): RenameInfo { + const getCanonicalDefaultLibName = memoize(() => getCanonicalFileName(ts.normalizePath(defaultLibFileName))); const node = getTouchingWord(sourceFile, position, /*includeJsDocComment*/ true); const renameInfo = node && nodeIsEligibleForRename(node) - ? getRenameInfoForNode(node, typeChecker, sourceFile, getIsDefinedInLibraryFile(defaultLibFileName, getCanonicalFileName)) + ? getRenameInfoForNode(node, typeChecker, sourceFile, isDefinedInLibraryFile) : undefined; return renameInfo || getRenameInfoError(Diagnostics.You_cannot_rename_this_element); - } - function getIsDefinedInLibraryFile(defaultLibFileName: string, getCanonicalFileName: (fileName: string) => string): (declaration: Node) => boolean { - if (!defaultLibFileName) { - return () => false; - } + function isDefinedInLibraryFile(declaration: Node) { + if (!defaultLibFileName) { + return false; + } - const canonicalDefaultLibName = getCanonicalFileName(ts.normalizePath(defaultLibFileName)); - return declaration => { const sourceFile = declaration.getSourceFile(); const canonicalName = getCanonicalFileName(ts.normalizePath(sourceFile.fileName)); - return canonicalName === canonicalDefaultLibName; + return canonicalName === getCanonicalDefaultLibName(); } } From 9be46006e60c6ce218086dec27534c8e17651816 Mon Sep 17 00:00:00 2001 From: Anatoly Ressin Date: Wed, 25 Jan 2017 15:41:46 +0200 Subject: [PATCH 06/11] fixes #13674 --- tests/webTestServer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/webTestServer.ts b/tests/webTestServer.ts index 67bbce2a472..abfd71a8fff 100644 --- a/tests/webTestServer.ts +++ b/tests/webTestServer.ts @@ -315,7 +315,6 @@ if (browser === "chrome") { let defaultChromePath = ""; switch (os.platform()) { case "win32": - case "win64": defaultChromePath = "C:/Program Files (x86)/Google/Chrome/Application/chrome.exe"; break; case "darwin": From 128b84d1db7359119fa211c3a222d43f5fb99a8b Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 25 Jan 2017 09:13:00 -0800 Subject: [PATCH 07/11] Add "workers" option in gulpfile. This allows e.g. `gulp runtests-parallel --w 3`. --- Gulpfile.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Gulpfile.ts b/Gulpfile.ts index bf5a074ef06..4f205bfb0ff 100644 --- a/Gulpfile.ts +++ b/Gulpfile.ts @@ -50,7 +50,8 @@ const cmdLineOptions = minimist(process.argv.slice(2), { r: "reporter", color: "colors", f: "files", - file: "files" + file: "files", + w: "workers", }, default: { soft: false, @@ -63,6 +64,7 @@ const cmdLineOptions = minimist(process.argv.slice(2), { reporter: process.env.reporter || process.env.r, lint: process.env.lint || true, files: process.env.f || process.env.file || process.env.files || "", + workers: process.env.workerCount || os.cpus().length, } }); @@ -604,7 +606,7 @@ function runConsoleTests(defaultReporter: string, runInParallel: boolean, done: } while (fs.existsSync(taskConfigsFolder)); fs.mkdirSync(taskConfigsFolder); - workerCount = process.env.workerCount || os.cpus().length; + workerCount = cmdLineOptions["workers"]; } if (tests || light || taskConfigsFolder) { @@ -1017,7 +1019,7 @@ gulp.task("lint", "Runs tslint on the compiler sources. Optional arguments are: cb(); }, (cb) => { files = files.filter(file => fileMatcher.test(file.path)).sort((filea, fileb) => filea.stat.size - fileb.stat.size); - const workerCount = (process.env.workerCount && +process.env.workerCount) || os.cpus().length; + const workerCount = cmdLineOptions["workers"]; for (let i = 0; i < workerCount; i++) { spawnLintWorker(files, finished); } From abc30b26c75247d839d2a2ecf5ee2b80ff5864ac Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Wed, 25 Jan 2017 09:53:34 -0800 Subject: [PATCH 08/11] handle cases when body of for-of statement is expanded after loop conversion (#13677) --- src/compiler/transformers/es2015.ts | 2 +- .../nestedLoopWithOnlyInnerLetCaptured.js | 18 +++++++++++++++++ ...nestedLoopWithOnlyInnerLetCaptured.symbols | 15 ++++++++++++++ .../nestedLoopWithOnlyInnerLetCaptured.types | 20 +++++++++++++++++++ .../nestedLoopWithOnlyInnerLetCaptured.ts | 6 ++++++ 5 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.js create mode 100644 tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.symbols create mode 100644 tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.types create mode 100644 tests/cases/compiler/nestedLoopWithOnlyInnerLetCaptured.ts diff --git a/src/compiler/transformers/es2015.ts b/src/compiler/transformers/es2015.ts index d1dab8eeb7a..3b8b4524aab 100644 --- a/src/compiler/transformers/es2015.ts +++ b/src/compiler/transformers/es2015.ts @@ -2316,7 +2316,7 @@ namespace ts { addRange(statements, convertedLoopBodyStatements); } else { - const statement = visitNode(node.statement, visitor, isStatement); + const statement = visitNode(node.statement, visitor, isStatement, /*optional*/ false, liftToBlock); if (isBlock(statement)) { addRange(statements, statement.statements); bodyLocation = statement; diff --git a/tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.js b/tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.js new file mode 100644 index 00000000000..ee17bb53eec --- /dev/null +++ b/tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.js @@ -0,0 +1,18 @@ +//// [nestedLoopWithOnlyInnerLetCaptured.ts] +declare let doSomething; + +for (let a1 of []) + for (let a2 of a1.someArray) + doSomething(() => a2); + +//// [nestedLoopWithOnlyInnerLetCaptured.js] +for (var _i = 0, _a = []; _i < _a.length; _i++) { + var a1 = _a[_i]; + var _loop_1 = function (a2) { + doSomething(function () { return a2; }); + }; + for (var _b = 0, _c = a1.someArray; _b < _c.length; _b++) { + var a2 = _c[_b]; + _loop_1(a2); + } +} diff --git a/tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.symbols b/tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.symbols new file mode 100644 index 00000000000..f134dcbffa9 --- /dev/null +++ b/tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.symbols @@ -0,0 +1,15 @@ +=== tests/cases/compiler/nestedLoopWithOnlyInnerLetCaptured.ts === +declare let doSomething; +>doSomething : Symbol(doSomething, Decl(nestedLoopWithOnlyInnerLetCaptured.ts, 0, 11)) + +for (let a1 of []) +>a1 : Symbol(a1, Decl(nestedLoopWithOnlyInnerLetCaptured.ts, 2, 8)) + + for (let a2 of a1.someArray) +>a2 : Symbol(a2, Decl(nestedLoopWithOnlyInnerLetCaptured.ts, 3, 12)) +>a1 : Symbol(a1, Decl(nestedLoopWithOnlyInnerLetCaptured.ts, 2, 8)) + + doSomething(() => a2); +>doSomething : Symbol(doSomething, Decl(nestedLoopWithOnlyInnerLetCaptured.ts, 0, 11)) +>a2 : Symbol(a2, Decl(nestedLoopWithOnlyInnerLetCaptured.ts, 3, 12)) + diff --git a/tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.types b/tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.types new file mode 100644 index 00000000000..6be9dbf6abf --- /dev/null +++ b/tests/baselines/reference/nestedLoopWithOnlyInnerLetCaptured.types @@ -0,0 +1,20 @@ +=== tests/cases/compiler/nestedLoopWithOnlyInnerLetCaptured.ts === +declare let doSomething; +>doSomething : any + +for (let a1 of []) +>a1 : any +>[] : undefined[] + + for (let a2 of a1.someArray) +>a2 : any +>a1.someArray : any +>a1 : any +>someArray : any + + doSomething(() => a2); +>doSomething(() => a2) : any +>doSomething : any +>() => a2 : () => any +>a2 : any + diff --git a/tests/cases/compiler/nestedLoopWithOnlyInnerLetCaptured.ts b/tests/cases/compiler/nestedLoopWithOnlyInnerLetCaptured.ts new file mode 100644 index 00000000000..210465b13da --- /dev/null +++ b/tests/cases/compiler/nestedLoopWithOnlyInnerLetCaptured.ts @@ -0,0 +1,6 @@ +// @target: es5 +declare let doSomething; + +for (let a1 of []) + for (let a2 of a1.someArray) + doSomething(() => a2); \ No newline at end of file From 76b1e95c3d89fa2ca762eda75c9bd182612ee85a Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Wed, 25 Jan 2017 10:40:59 -0800 Subject: [PATCH 09/11] Always call `checkExpression` on JSX attribute values Fixes #13676 --- src/compiler/checker.ts | 19 ++--- .../baselines/reference/reactImportDropped.js | 48 ++++++++++++ .../reference/reactImportDropped.symbols | 65 ++++++++++++++++ .../reference/reactImportDropped.types | 74 +++++++++++++++++++ tests/cases/compiler/reactImportDropped.ts | 42 +++++++++++ 5 files changed, 239 insertions(+), 9 deletions(-) create mode 100644 tests/baselines/reference/reactImportDropped.js create mode 100644 tests/baselines/reference/reactImportDropped.symbols create mode 100644 tests/baselines/reference/reactImportDropped.types create mode 100644 tests/cases/compiler/reactImportDropped.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index a09b63d46fd..7c8a0b7337f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11879,6 +11879,16 @@ namespace ts { function checkJsxAttribute(node: JsxAttribute, elementAttributesType: Type, nameTable: Map) { let correspondingPropType: Type = undefined; + // We need to unconditionally get the expression type + let exprType: Type; + if (node.initializer) { + exprType = checkExpression(node.initializer); + } + else { + // is sugar for + exprType = booleanType; + } + // Look up the corresponding property for this attribute if (elementAttributesType === emptyObjectType && isUnhyphenatedJsxName(node.name.text)) { // If there is no 'props' property, you may not have non-"data-" attributes @@ -11902,15 +11912,6 @@ namespace ts { } } - let exprType: Type; - if (node.initializer) { - exprType = checkExpression(node.initializer); - } - else { - // is sugar for - exprType = booleanType; - } - if (correspondingPropType) { checkTypeAssignableTo(exprType, correspondingPropType, node); } diff --git a/tests/baselines/reference/reactImportDropped.js b/tests/baselines/reference/reactImportDropped.js new file mode 100644 index 00000000000..7ada76b42f4 --- /dev/null +++ b/tests/baselines/reference/reactImportDropped.js @@ -0,0 +1,48 @@ +//// [tests/cases/compiler/reactImportDropped.ts] //// + +//// [react.d.ts] + +export = React; +export as namespace React; + +declare namespace React { + + function createClass(spec: any): ClassicComponentClass; + + interface ClassicComponentClass { + new (props?: any): ClassicComponentClass; + } +} + +declare global { + namespace JSX { + interface ElementAttributesProperty { } + } +} + + +//// [TabBar.js] +export default React.createClass({ + render() { + return ( + null + ); + } +}); + +//// [NavigationView.js] +import TabBar from '../../components/TabBar'; +import {layout} from '../../utils/theme'; // <- DO NOT DROP this import +const x = ; + + +//// [TabBar.js] +export default React.createClass({ + render() { + return (null); + } +}); +//// [NavigationView.js] +import TabBar from '../../components/TabBar'; +import { layout } from '../../utils/theme'; // <- DO NOT DROP this import +const x = React.createElement(TabBar, { height: layout.footerHeight }); diff --git a/tests/baselines/reference/reactImportDropped.symbols b/tests/baselines/reference/reactImportDropped.symbols new file mode 100644 index 00000000000..d7374df044e --- /dev/null +++ b/tests/baselines/reference/reactImportDropped.symbols @@ -0,0 +1,65 @@ +=== tests/cases/compiler/react.d.ts === + +export = React; +>React : Symbol(React, Decl(react.d.ts, 2, 26)) + +export as namespace React; +>React : Symbol(React, Decl(react.d.ts, 1, 15)) + +declare namespace React { +>React : Symbol(React, Decl(react.d.ts, 2, 26)) + + function createClass(spec: any): ClassicComponentClass; +>createClass : Symbol(createClass, Decl(react.d.ts, 4, 25)) +>spec : Symbol(spec, Decl(react.d.ts, 6, 25)) +>ClassicComponentClass : Symbol(ClassicComponentClass, Decl(react.d.ts, 6, 59)) + + interface ClassicComponentClass { +>ClassicComponentClass : Symbol(ClassicComponentClass, Decl(react.d.ts, 6, 59)) + + new (props?: any): ClassicComponentClass; +>props : Symbol(props, Decl(react.d.ts, 9, 13)) +>ClassicComponentClass : Symbol(ClassicComponentClass, Decl(react.d.ts, 6, 59)) + } +} + +declare global { +>global : Symbol(global, Decl(react.d.ts, 11, 1)) + + namespace JSX { +>JSX : Symbol(JSX, Decl(react.d.ts, 13, 16)) + + interface ElementAttributesProperty { } +>ElementAttributesProperty : Symbol(ElementAttributesProperty, Decl(react.d.ts, 14, 19)) + } +} + + +=== tests/cases/compiler/src/components/TabBar.js === +export default React.createClass({ +>React.createClass : Symbol(React.createClass, Decl(react.d.ts, 4, 25)) +>React : Symbol(React, Decl(react.d.ts, 1, 15)) +>createClass : Symbol(React.createClass, Decl(react.d.ts, 4, 25)) + + render() { +>render : Symbol(render, Decl(TabBar.js, 0, 34)) + + return ( + null + ); + } +}); + +=== tests/cases/compiler/src/modules/navigation/NavigationView.js === +import TabBar from '../../components/TabBar'; +>TabBar : Symbol(TabBar, Decl(NavigationView.js, 0, 6)) + +import {layout} from '../../utils/theme'; // <- DO NOT DROP this import +>layout : Symbol(layout, Decl(NavigationView.js, 1, 8)) + +const x = ; +>x : Symbol(x, Decl(NavigationView.js, 2, 5)) +>TabBar : Symbol(TabBar, Decl(NavigationView.js, 0, 6)) +>height : Symbol(layout) +>layout : Symbol(layout, Decl(NavigationView.js, 1, 8)) + diff --git a/tests/baselines/reference/reactImportDropped.types b/tests/baselines/reference/reactImportDropped.types new file mode 100644 index 00000000000..36f1a6a4779 --- /dev/null +++ b/tests/baselines/reference/reactImportDropped.types @@ -0,0 +1,74 @@ +=== tests/cases/compiler/react.d.ts === + +export = React; +>React : typeof React + +export as namespace React; +>React : typeof React + +declare namespace React { +>React : typeof React + + function createClass(spec: any): ClassicComponentClass; +>createClass : (spec: any) => ClassicComponentClass +>spec : any +>ClassicComponentClass : ClassicComponentClass + + interface ClassicComponentClass { +>ClassicComponentClass : ClassicComponentClass + + new (props?: any): ClassicComponentClass; +>props : any +>ClassicComponentClass : ClassicComponentClass + } +} + +declare global { +>global : any + + namespace JSX { +>JSX : any + + interface ElementAttributesProperty { } +>ElementAttributesProperty : ElementAttributesProperty + } +} + + +=== tests/cases/compiler/src/components/TabBar.js === +export default React.createClass({ +>React.createClass({ render() { return ( null ); }}) : React.ClassicComponentClass +>React.createClass : (spec: any) => React.ClassicComponentClass +>React : typeof React +>createClass : (spec: any) => React.ClassicComponentClass +>{ render() { return ( null ); }} : { render(): any; } + + render() { +>render : () => any + + return ( +>( null ) : null + + null +>null : null + + ); + } +}); + +=== tests/cases/compiler/src/modules/navigation/NavigationView.js === +import TabBar from '../../components/TabBar'; +>TabBar : React.ClassicComponentClass + +import {layout} from '../../utils/theme'; // <- DO NOT DROP this import +>layout : any + +const x = ; +>x : any +> : any +>TabBar : React.ClassicComponentClass +>height : any +>layout.footerHeight : any +>layout : any +>footerHeight : any + diff --git a/tests/cases/compiler/reactImportDropped.ts b/tests/cases/compiler/reactImportDropped.ts new file mode 100644 index 00000000000..a345e7c74fd --- /dev/null +++ b/tests/cases/compiler/reactImportDropped.ts @@ -0,0 +1,42 @@ +//@module: es6 +//@moduleResolution: node +//@target: es6 +//@noImplicitAny: false +//@allowSyntheticDefaultImports: true +//@allowJs: true +//@jsx: react +//@outDir: "build" + +//@filename: react.d.ts +export = React; +export as namespace React; + +declare namespace React { + + function createClass(spec: any): ClassicComponentClass; + + interface ClassicComponentClass { + new (props?: any): ClassicComponentClass; + } +} + +declare global { + namespace JSX { + interface ElementAttributesProperty { } + } +} + + +//@filename: src/components/TabBar.js +export default React.createClass({ + render() { + return ( + null + ); + } +}); + +//@filename: src/modules/navigation/NavigationView.js +import TabBar from '../../components/TabBar'; +import {layout} from '../../utils/theme'; // <- DO NOT DROP this import +const x = ; From 16bdaaa0a6db70a3594d3e847545e6ce370ce620 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 25 Jan 2017 11:04:23 -0800 Subject: [PATCH 10/11] Fix positionToLineOffset conversion for getImplementation --- src/server/session.ts | 16 +++++++++------- .../goToImplementation_inDifferentFiles.ts | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) create mode 100644 tests/cases/fourslash/server/goToImplementation_inDifferentFiles.ts diff --git a/src/server/session.ts b/src/server/session.ts index 1fa3986f66f..825ad8506d7 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -484,18 +484,20 @@ namespace ts.server { private getImplementation(args: protocol.FileLocationRequestArgs, simplifiedResult: boolean): protocol.FileSpan[] | ImplementationLocation[] { const { file, project } = this.getFileAndProject(args); - const scriptInfo = project.getScriptInfoForNormalizedPath(file); - const position = this.getPosition(args, scriptInfo); + const position = this.getPosition(args, project.getScriptInfoForNormalizedPath(file)); const implementations = project.getLanguageService().getImplementationAtPosition(file, position); if (!implementations) { return []; } if (simplifiedResult) { - return implementations.map(impl => ({ - file: impl.fileName, - start: scriptInfo.positionToLineOffset(impl.textSpan.start), - end: scriptInfo.positionToLineOffset(ts.textSpanEnd(impl.textSpan)) - })); + return implementations.map(({ fileName, textSpan }) => { + const scriptInfo = project.getScriptInfo(fileName); + return { + file: fileName, + start: scriptInfo.positionToLineOffset(textSpan.start), + end: scriptInfo.positionToLineOffset(ts.textSpanEnd(textSpan)) + }; + }); } else { return implementations; diff --git a/tests/cases/fourslash/server/goToImplementation_inDifferentFiles.ts b/tests/cases/fourslash/server/goToImplementation_inDifferentFiles.ts new file mode 100644 index 00000000000..e4289b83102 --- /dev/null +++ b/tests/cases/fourslash/server/goToImplementation_inDifferentFiles.ts @@ -0,0 +1,19 @@ +/// + +// @Filename: /bar.ts +////import {Foo} from './foo' +//// +////[|class A implements Foo { +//// func() {} +////}|] +//// +////[|class B implements Foo { +//// func() {} +////}|] + +// @Filename: /foo.ts +////export interface /**/Foo { +//// func(); +////} + +verify.allRangesAppearInImplementationList(""); From 916e67a92c0c9d348ac74f8f3e78374d55964d78 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 25 Jan 2017 08:32:08 -0800 Subject: [PATCH 11/11] For goToDefinition, verify that tryGetSignatureDeclaration returns a signature declaration and not a FunctionType. --- src/services/goToDefinition.ts | 17 +++++++++++++++-- .../fourslash/goToDefinitionFunctionType.ts | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/goToDefinitionFunctionType.ts diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index ab35a17fcef..73cf06d31ef 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -187,7 +187,15 @@ namespace ts.GoToDefinition { } function isSignatureDeclaration(node: Node): boolean { - return node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.MethodSignature + switch (node.kind) { + case ts.SyntaxKind.Constructor: + case ts.SyntaxKind.FunctionDeclaration: + case ts.SyntaxKind.MethodDeclaration: + case ts.SyntaxKind.MethodSignature: + return true; + default: + return false; + } } /** Creates a DefinitionInfo from a Declaration, using the declaration's name if possible. */ @@ -254,6 +262,11 @@ namespace ts.GoToDefinition { function tryGetSignatureDeclaration(typeChecker: TypeChecker, node: Node): SignatureDeclaration | undefined { const callLike = getAncestorCallLikeExpression(node); - return callLike && typeChecker.getResolvedSignature(callLike).declaration; + const decl = callLike && typeChecker.getResolvedSignature(callLike).declaration; + if (decl && isSignatureDeclaration(decl)) { + return decl; + } + // Don't go to a function type, go to the value having that type. + return undefined; } } diff --git a/tests/cases/fourslash/goToDefinitionFunctionType.ts b/tests/cases/fourslash/goToDefinitionFunctionType.ts new file mode 100644 index 00000000000..1b50de3af6d --- /dev/null +++ b/tests/cases/fourslash/goToDefinitionFunctionType.ts @@ -0,0 +1,17 @@ +/// + +// Tests that goToDefinition does not go to a function type; it goes to the value. + +////const /*constDefinition*/c: () => void; +/////*constReference*/c(); +////function test(/*cbDefinition*/cb: () => void) { +//// /*cbReference*/cb(); +////} +////class C { +//// /*propDefinition*/prop: () => void; +//// m() { +//// this./*propReference*/prop(); +//// } +////} + +verify.goToDefinitionForMarkers("const", "cb", "prop");