From 311d20fa99a48eff5cff7f5d4ea311b95f7bd280 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 7 Jul 2015 12:46:58 -0700 Subject: [PATCH 1/6] Fix bug #3737 (exported JSX classes props not validated) --- src/compiler/checker.ts | 18 +++---- .../tsxAttributeResolution9.errors.txt | 31 ++++++++++++ .../reference/tsxAttributeResolution9.js | 42 +++++++++++++++++ .../reference/tsxAttributeResolution9.symbols | 45 ++++++++++++++++++ .../reference/tsxAttributeResolution9.types | 47 +++++++++++++++++++ .../jsx/tsxAttributeResolution9.tsx | 27 +++++++++++ 6 files changed, 199 insertions(+), 11 deletions(-) create mode 100644 tests/baselines/reference/tsxAttributeResolution9.errors.txt create mode 100644 tests/baselines/reference/tsxAttributeResolution9.js create mode 100644 tests/baselines/reference/tsxAttributeResolution9.symbols create mode 100644 tests/baselines/reference/tsxAttributeResolution9.types create mode 100644 tests/cases/conformance/jsx/tsxAttributeResolution9.tsx diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3ef2c6ada2a..b910dc3306b 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7095,13 +7095,18 @@ namespace ts { // Look up the value in the current scope if (node.tagName.kind === SyntaxKind.Identifier) { - valueSymbol = getResolvedSymbol(node.tagName); + let tag = node.tagName; + valueSymbol = resolveName(tag, tag.text, SymbolFlags.Value, Diagnostics.Cannot_find_name_0, tag.text); } else { valueSymbol = checkQualifiedName(node.tagName).symbol; } - if (valueSymbol !== unknownSymbol) { + if (valueSymbol && valueSymbol !== unknownSymbol) { + let symbolLinks = getSymbolLinks(valueSymbol); + if (symbolLinks) { + symbolLinks.referenced = true; + } links.jsxFlags |= JsxFlags.ClassElement; } @@ -7301,15 +7306,6 @@ namespace ts { let targetAttributesType = getJsxElementAttributesType(node); - if (getNodeLinks(node).jsxFlags & JsxFlags.ClassElement) { - if (node.tagName.kind === SyntaxKind.Identifier) { - checkIdentifier(node.tagName); - } - else { - checkQualifiedName(node.tagName); - } - } - let nameTable: Map = {}; // Process this array in right-to-left order so we know which // attributes (mostly from spreads) are being overwritten and diff --git a/tests/baselines/reference/tsxAttributeResolution9.errors.txt b/tests/baselines/reference/tsxAttributeResolution9.errors.txt new file mode 100644 index 00000000000..c25532eaa0a --- /dev/null +++ b/tests/baselines/reference/tsxAttributeResolution9.errors.txt @@ -0,0 +1,31 @@ +tests/cases/conformance/jsx/file.tsx(9,14): error TS2322: Type 'number' is not assignable to type 'string'. + + +==== tests/cases/conformance/jsx/react.d.ts (0 errors) ==== + + declare module JSX { + interface Element { } + interface IntrinsicElements { + } + interface ElementAttributesProperty { + props; + } + } + + interface Props { + foo: string; + } + +==== tests/cases/conformance/jsx/file.tsx (1 errors) ==== + export class MyComponent { + render() { + } + + props: { foo: string; } + } + + ; // ok + ; // should be an error + ~~~~~~~ +!!! error TS2322: Type 'number' is not assignable to type 'string'. + \ No newline at end of file diff --git a/tests/baselines/reference/tsxAttributeResolution9.js b/tests/baselines/reference/tsxAttributeResolution9.js new file mode 100644 index 00000000000..bfc19a7ba4f --- /dev/null +++ b/tests/baselines/reference/tsxAttributeResolution9.js @@ -0,0 +1,42 @@ +//// [tests/cases/conformance/jsx/tsxAttributeResolution9.tsx] //// + +//// [react.d.ts] + +declare module JSX { + interface Element { } + interface IntrinsicElements { + } + interface ElementAttributesProperty { + props; + } +} + +interface Props { + foo: string; +} + +//// [file.tsx] +export class MyComponent { + render() { + } + + props: { foo: string; } +} + +; // ok +; // should be an error + + +//// [file.jsx] +define(["require", "exports"], function (require, exports) { + var MyComponent = (function () { + function MyComponent() { + } + MyComponent.prototype.render = function () { + }; + return MyComponent; + })(); + exports.MyComponent = MyComponent; + ; // ok + ; // should be an error +}); diff --git a/tests/baselines/reference/tsxAttributeResolution9.symbols b/tests/baselines/reference/tsxAttributeResolution9.symbols new file mode 100644 index 00000000000..cbb30864c81 --- /dev/null +++ b/tests/baselines/reference/tsxAttributeResolution9.symbols @@ -0,0 +1,45 @@ +=== tests/cases/conformance/jsx/tsxAttributeResolution9.tsx === +declare module JSX { +>JSX : Symbol(JSX, Decl(tsxAttributeResolution9.tsx, 0, 0)) + + interface Element { } +>Element : Symbol(Element, Decl(tsxAttributeResolution9.tsx, 0, 20)) + + interface IntrinsicElements { +>IntrinsicElements : Symbol(IntrinsicElements, Decl(tsxAttributeResolution9.tsx, 1, 22)) + } + interface ElementAttributesProperty { +>ElementAttributesProperty : Symbol(ElementAttributesProperty, Decl(tsxAttributeResolution9.tsx, 3, 2)) + + props; +>props : Symbol(props, Decl(tsxAttributeResolution9.tsx, 4, 38)) + } +} + +interface Props { +>Props : Symbol(Props, Decl(tsxAttributeResolution9.tsx, 7, 1)) + + foo: string; +>foo : Symbol(foo, Decl(tsxAttributeResolution9.tsx, 9, 17)) +} + +export class MyComponent { +>MyComponent : Symbol(MyComponent, Decl(tsxAttributeResolution9.tsx, 11, 1)) + + render() { +>render : Symbol(render, Decl(tsxAttributeResolution9.tsx, 13, 26)) + } + + props: { foo: string; } +>props : Symbol(props, Decl(tsxAttributeResolution9.tsx, 15, 3)) +>foo : Symbol(foo, Decl(tsxAttributeResolution9.tsx, 17, 10)) +} + +; // ok +>MyComponent : Symbol(MyComponent, Decl(tsxAttributeResolution9.tsx, 11, 1)) +>foo : Symbol(unknown) + +; // should be an error +>MyComponent : Symbol(MyComponent, Decl(tsxAttributeResolution9.tsx, 11, 1)) +>foo : Symbol(unknown) + diff --git a/tests/baselines/reference/tsxAttributeResolution9.types b/tests/baselines/reference/tsxAttributeResolution9.types new file mode 100644 index 00000000000..aab3806a5f0 --- /dev/null +++ b/tests/baselines/reference/tsxAttributeResolution9.types @@ -0,0 +1,47 @@ +=== tests/cases/conformance/jsx/tsxAttributeResolution9.tsx === +declare module JSX { +>JSX : any + + interface Element { } +>Element : Element + + interface IntrinsicElements { +>IntrinsicElements : IntrinsicElements + } + interface ElementAttributesProperty { +>ElementAttributesProperty : ElementAttributesProperty + + props; +>props : any + } +} + +interface Props { +>Props : Props + + foo: string; +>foo : string +} + +export class MyComponent { +>MyComponent : MyComponent + + render() { +>render : () => void + } + + props: { foo: string; } +>props : { foo: string; } +>foo : string +} + +; // ok +> : any +>MyComponent : typeof MyComponent +>foo : any + +; // should be an error +> : any +>MyComponent : typeof MyComponent +>foo : any + diff --git a/tests/cases/conformance/jsx/tsxAttributeResolution9.tsx b/tests/cases/conformance/jsx/tsxAttributeResolution9.tsx new file mode 100644 index 00000000000..9768d65d000 --- /dev/null +++ b/tests/cases/conformance/jsx/tsxAttributeResolution9.tsx @@ -0,0 +1,27 @@ +//@jsx: preserve +//@module: amd + +//@filename: react.d.ts +declare module JSX { + interface Element { } + interface IntrinsicElements { + } + interface ElementAttributesProperty { + props; + } +} + +interface Props { + foo: string; +} + +//@filename: file.tsx +export class MyComponent { + render() { + } + + props: { foo: string; } +} + +; // ok +; // should be an error From 99fc99f3bcbc123b7c37be05a4be7ac25e929fd7 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 7 Jul 2015 14:27:57 -0700 Subject: [PATCH 2/6] Improved fix from @JsonFreeman --- src/compiler/checker.ts | 10 +++---- .../reference/tsxAttributeResolution9.symbols | 30 ++++++++++--------- .../reference/tsxAttributeResolution9.types | 8 +++-- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b910dc3306b..842b6dcd550 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7096,18 +7096,18 @@ namespace ts { // Look up the value in the current scope if (node.tagName.kind === SyntaxKind.Identifier) { let tag = node.tagName; - valueSymbol = resolveName(tag, tag.text, SymbolFlags.Value, Diagnostics.Cannot_find_name_0, tag.text); + let maybeExportSymbol = getResolvedSymbol(node.tagName); + let valueDecl = maybeExportSymbol.valueDeclaration; + + valueSymbol = (valueDecl && valueDecl.localSymbol) || maybeExportSymbol; } else { valueSymbol = checkQualifiedName(node.tagName).symbol; } if (valueSymbol && valueSymbol !== unknownSymbol) { - let symbolLinks = getSymbolLinks(valueSymbol); - if (symbolLinks) { - symbolLinks.referenced = true; - } links.jsxFlags |= JsxFlags.ClassElement; + getSymbolLinks(valueSymbol).referenced = true; } return valueSymbol || unknownSymbol; diff --git a/tests/baselines/reference/tsxAttributeResolution9.symbols b/tests/baselines/reference/tsxAttributeResolution9.symbols index cbb30864c81..081482d5d47 100644 --- a/tests/baselines/reference/tsxAttributeResolution9.symbols +++ b/tests/baselines/reference/tsxAttributeResolution9.symbols @@ -1,45 +1,47 @@ -=== tests/cases/conformance/jsx/tsxAttributeResolution9.tsx === +=== tests/cases/conformance/jsx/react.d.ts === + declare module JSX { ->JSX : Symbol(JSX, Decl(tsxAttributeResolution9.tsx, 0, 0)) +>JSX : Symbol(JSX, Decl(react.d.ts, 0, 0)) interface Element { } ->Element : Symbol(Element, Decl(tsxAttributeResolution9.tsx, 0, 20)) +>Element : Symbol(Element, Decl(react.d.ts, 1, 20)) interface IntrinsicElements { ->IntrinsicElements : Symbol(IntrinsicElements, Decl(tsxAttributeResolution9.tsx, 1, 22)) +>IntrinsicElements : Symbol(IntrinsicElements, Decl(react.d.ts, 2, 22)) } interface ElementAttributesProperty { ->ElementAttributesProperty : Symbol(ElementAttributesProperty, Decl(tsxAttributeResolution9.tsx, 3, 2)) +>ElementAttributesProperty : Symbol(ElementAttributesProperty, Decl(react.d.ts, 4, 2)) props; ->props : Symbol(props, Decl(tsxAttributeResolution9.tsx, 4, 38)) +>props : Symbol(props, Decl(react.d.ts, 5, 38)) } } interface Props { ->Props : Symbol(Props, Decl(tsxAttributeResolution9.tsx, 7, 1)) +>Props : Symbol(Props, Decl(react.d.ts, 8, 1)) foo: string; ->foo : Symbol(foo, Decl(tsxAttributeResolution9.tsx, 9, 17)) +>foo : Symbol(foo, Decl(react.d.ts, 10, 17)) } +=== tests/cases/conformance/jsx/file.tsx === export class MyComponent { ->MyComponent : Symbol(MyComponent, Decl(tsxAttributeResolution9.tsx, 11, 1)) +>MyComponent : Symbol(MyComponent, Decl(file.tsx, 0, 0)) render() { ->render : Symbol(render, Decl(tsxAttributeResolution9.tsx, 13, 26)) +>render : Symbol(render, Decl(file.tsx, 0, 26)) } props: { foo: string; } ->props : Symbol(props, Decl(tsxAttributeResolution9.tsx, 15, 3)) ->foo : Symbol(foo, Decl(tsxAttributeResolution9.tsx, 17, 10)) +>props : Symbol(props, Decl(file.tsx, 2, 3)) +>foo : Symbol(foo, Decl(file.tsx, 4, 10)) } ; // ok ->MyComponent : Symbol(MyComponent, Decl(tsxAttributeResolution9.tsx, 11, 1)) +>MyComponent : Symbol(MyComponent, Decl(file.tsx, 0, 0)) >foo : Symbol(unknown) ; // should be an error ->MyComponent : Symbol(MyComponent, Decl(tsxAttributeResolution9.tsx, 11, 1)) +>MyComponent : Symbol(MyComponent, Decl(file.tsx, 0, 0)) >foo : Symbol(unknown) diff --git a/tests/baselines/reference/tsxAttributeResolution9.types b/tests/baselines/reference/tsxAttributeResolution9.types index aab3806a5f0..1b2c6d42389 100644 --- a/tests/baselines/reference/tsxAttributeResolution9.types +++ b/tests/baselines/reference/tsxAttributeResolution9.types @@ -1,4 +1,5 @@ -=== tests/cases/conformance/jsx/tsxAttributeResolution9.tsx === +=== tests/cases/conformance/jsx/react.d.ts === + declare module JSX { >JSX : any @@ -23,6 +24,7 @@ interface Props { >foo : string } +=== tests/cases/conformance/jsx/file.tsx === export class MyComponent { >MyComponent : MyComponent @@ -36,12 +38,12 @@ export class MyComponent { } ; // ok -> : any +> : JSX.Element >MyComponent : typeof MyComponent >foo : any ; // should be an error -> : any +> : JSX.Element >MyComponent : typeof MyComponent >foo : any From 3ccaa0ee261c70cd32e31af3e458c44a23fe6b10 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 7 Jul 2015 14:33:59 -0700 Subject: [PATCH 3/6] Cleanup --- src/compiler/checker.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 842b6dcd550..da7ac5844b8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7096,9 +7096,8 @@ namespace ts { // Look up the value in the current scope if (node.tagName.kind === SyntaxKind.Identifier) { let tag = node.tagName; - let maybeExportSymbol = getResolvedSymbol(node.tagName); + let maybeExportSymbol = getResolvedSymbol(tag); let valueDecl = maybeExportSymbol.valueDeclaration; - valueSymbol = (valueDecl && valueDecl.localSymbol) || maybeExportSymbol; } else { From 6e332cf26697f4dbb557ce2199ab88ac4b612b78 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 7 Jul 2015 15:45:35 -0700 Subject: [PATCH 4/6] Remove file that shouldn't have been in this branch --- .../tsxAttributeResolution9.errors.txt | 31 ------------------- 1 file changed, 31 deletions(-) delete mode 100644 tests/baselines/reference/tsxAttributeResolution9.errors.txt diff --git a/tests/baselines/reference/tsxAttributeResolution9.errors.txt b/tests/baselines/reference/tsxAttributeResolution9.errors.txt deleted file mode 100644 index c25532eaa0a..00000000000 --- a/tests/baselines/reference/tsxAttributeResolution9.errors.txt +++ /dev/null @@ -1,31 +0,0 @@ -tests/cases/conformance/jsx/file.tsx(9,14): error TS2322: Type 'number' is not assignable to type 'string'. - - -==== tests/cases/conformance/jsx/react.d.ts (0 errors) ==== - - declare module JSX { - interface Element { } - interface IntrinsicElements { - } - interface ElementAttributesProperty { - props; - } - } - - interface Props { - foo: string; - } - -==== tests/cases/conformance/jsx/file.tsx (1 errors) ==== - export class MyComponent { - render() { - } - - props: { foo: string; } - } - - ; // ok - ; // should be an error - ~~~~~~~ -!!! error TS2322: Type 'number' is not assignable to type 'string'. - \ No newline at end of file From 8f3bce121d6f429ea90b121346fe2dc39542a5d3 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 7 Jul 2015 16:22:04 -0700 Subject: [PATCH 5/6] Actually fix the bug this time. --- src/compiler/checker.ts | 5 ++- .../tsxAttributeResolution9.errors.txt | 31 ------------------- 2 files changed, 2 insertions(+), 34 deletions(-) delete mode 100644 tests/baselines/reference/tsxAttributeResolution9.errors.txt diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7e631ff5bce..1edd2a7a920 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7096,9 +7096,8 @@ namespace ts { // Look up the value in the current scope if (node.tagName.kind === SyntaxKind.Identifier) { let tag = node.tagName; - let maybeExportSymbol = getResolvedSymbol(tag); - let valueDecl = maybeExportSymbol.valueDeclaration; - valueSymbol = (valueDecl && valueDecl.localSymbol) || maybeExportSymbol; + let sym = getResolvedSymbol(tag); + valueSymbol = sym.exportSymbol || sym; } else { valueSymbol = checkQualifiedName(node.tagName).symbol; diff --git a/tests/baselines/reference/tsxAttributeResolution9.errors.txt b/tests/baselines/reference/tsxAttributeResolution9.errors.txt deleted file mode 100644 index c25532eaa0a..00000000000 --- a/tests/baselines/reference/tsxAttributeResolution9.errors.txt +++ /dev/null @@ -1,31 +0,0 @@ -tests/cases/conformance/jsx/file.tsx(9,14): error TS2322: Type 'number' is not assignable to type 'string'. - - -==== tests/cases/conformance/jsx/react.d.ts (0 errors) ==== - - declare module JSX { - interface Element { } - interface IntrinsicElements { - } - interface ElementAttributesProperty { - props; - } - } - - interface Props { - foo: string; - } - -==== tests/cases/conformance/jsx/file.tsx (1 errors) ==== - export class MyComponent { - render() { - } - - props: { foo: string; } - } - - ; // ok - ; // should be an error - ~~~~~~~ -!!! error TS2322: Type 'number' is not assignable to type 'string'. - \ No newline at end of file From 3e57af13b93e082b05b3c1cbf21ef2e7516e5591 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 7 Jul 2015 17:04:03 -0700 Subject: [PATCH 6/6] Add missed file --- .../tsxAttributeResolution9.errors.txt | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/baselines/reference/tsxAttributeResolution9.errors.txt diff --git a/tests/baselines/reference/tsxAttributeResolution9.errors.txt b/tests/baselines/reference/tsxAttributeResolution9.errors.txt new file mode 100644 index 00000000000..c25532eaa0a --- /dev/null +++ b/tests/baselines/reference/tsxAttributeResolution9.errors.txt @@ -0,0 +1,31 @@ +tests/cases/conformance/jsx/file.tsx(9,14): error TS2322: Type 'number' is not assignable to type 'string'. + + +==== tests/cases/conformance/jsx/react.d.ts (0 errors) ==== + + declare module JSX { + interface Element { } + interface IntrinsicElements { + } + interface ElementAttributesProperty { + props; + } + } + + interface Props { + foo: string; + } + +==== tests/cases/conformance/jsx/file.tsx (1 errors) ==== + export class MyComponent { + render() { + } + + props: { foo: string; } + } + + ; // ok + ; // should be an error + ~~~~~~~ +!!! error TS2322: Type 'number' is not assignable to type 'string'. + \ No newline at end of file