Fix 8549: Using variable as Jsx tagname (#9337)

* Parse JSXElement's name as property access instead of just entity name. So when one accesses property of the class through this, checker will check correctly * wip - just resolve to any type for now * Resolve string type to anytype and look up property in intrinsicElementsType of Jsx * Add tests and update baselines * Remove unneccessary comment * wip-address PR * Address PR * Add tets and update baselines * Fix linting error
microsoft · Jun 24, 2016 · db0d8e0 · db0d8e0
1 parent 2aa1d71
commit db0d8e0
Show file tree

Hide file tree

Showing 40 changed files with 836 additions and 24 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -9657,8 +9657,9 @@ namespace ts {
         /**
          * Returns true iff React would emit this tag name as a string rather than an identifier or qualified name
          */
-        function isJsxIntrinsicIdentifier(tagName: Identifier | QualifiedName) {
-            if (tagName.kind === SyntaxKind.QualifiedName) {
+        function isJsxIntrinsicIdentifier(tagName: JsxTagNameExpression) {
+            // TODO (yuisu): comment
+            if (tagName.kind === SyntaxKind.PropertyAccessExpression || tagName.kind === SyntaxKind.ThisKeyword) {
                 return false;
             }
             else {
@@ -9854,6 +9855,29 @@ namespace ts {
                 }));
             }
 
+            // If the elemType is a string type, we have to return anyType to prevent an error downstream as we will try to find construct or call signature of the type
+            if (elemType.flags & TypeFlags.String) {
+                return anyType;
+            }
+            else if (elemType.flags & TypeFlags.StringLiteral) {
+                // If the elemType is a stringLiteral type, we can then provide a check to make sure that the string literal type is one of the Jsx intrinsic element type
+                const intrinsicElementsType = getJsxType(JsxNames.IntrinsicElements);
+                if (intrinsicElementsType !== unknownType) {
+                    const stringLiteralTypeName = (<StringLiteralType>elemType).text;
+                    const intrinsicProp = getPropertyOfType(intrinsicElementsType, stringLiteralTypeName);
+                    if (intrinsicProp) {
+                        return getTypeOfSymbol(intrinsicProp);
+                    }
+                    const indexSignatureType = getIndexTypeOfType(intrinsicElementsType, IndexKind.String);
+                    if (indexSignatureType) {
+                        return indexSignatureType;
+                    }
+                    error(node, Diagnostics.Property_0_does_not_exist_on_type_1, stringLiteralTypeName, "JSX." + JsxNames.IntrinsicElements);
+                }
+                // If we need to report an error, we already done so here. So just return any to prevent any more error downstream
+                return anyType;
+            }
+
             // Get the element instance type (the result of newing or invoking this tag)
             const elemInstanceType = getJsxElementInstanceType(node, elemType);
 

diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -1219,7 +1219,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
             function jsxEmitReact(node: JsxElement | JsxSelfClosingElement) {
                 /// Emit a tag name, which is either '"div"' for lower-cased names, or
                 /// 'Div' for upper-cased or dotted names
-                function emitTagName(name: Identifier | QualifiedName) {
+                function emitTagName(name: LeftHandSideExpression) {
                     if (name.kind === SyntaxKind.Identifier && isIntrinsicJsxName((<Identifier>name).text)) {
                         write('"');
                         emit(name);

diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts
@@ -3576,7 +3576,7 @@ namespace ts {
             return finishNode(node);
         }
 
-        function tagNamesAreEquivalent(lhs: EntityName, rhs: EntityName): boolean {
+        function tagNamesAreEquivalent(lhs: JsxTagNameExpression, rhs: JsxTagNameExpression): boolean {
             if (lhs.kind !== rhs.kind) {
                 return false;
             }
@@ -3585,8 +3585,15 @@ namespace ts {
                 return (<Identifier>lhs).text === (<Identifier>rhs).text;
             }
 
-            return (<QualifiedName>lhs).right.text === (<QualifiedName>rhs).right.text &&
-                tagNamesAreEquivalent((<QualifiedName>lhs).left, (<QualifiedName>rhs).left);
+            if (lhs.kind === SyntaxKind.ThisKeyword) {
+                return true;
+            }
+
+            // If we are at this statement then we must have PropertyAccessExpression and because tag name in Jsx element can only
+            // take forms of JsxTagNameExpression which includes an identifier, "this" expression, or another propertyAccessExpression
+            // it is safe to case the expression property as such. See parseJsxElementName for how we parse tag name in Jsx element
+            return (<PropertyAccessExpression>lhs).name.text === (<PropertyAccessExpression>rhs).name.text &&
+                tagNamesAreEquivalent((<PropertyAccessExpression>lhs).expression as JsxTagNameExpression, (<PropertyAccessExpression>rhs).expression as JsxTagNameExpression);
         }
 
 
@@ -3654,7 +3661,7 @@ namespace ts {
             Debug.fail("Unknown JSX child kind " + token);
         }
 
-        function parseJsxChildren(openingTagName: EntityName): NodeArray<JsxChild> {
+        function parseJsxChildren(openingTagName: LeftHandSideExpression): NodeArray<JsxChild> {
             const result = <NodeArray<JsxChild>>[];
             result.pos = scanner.getStartPos();
             const saveParsingContext = parsingContext;
@@ -3717,17 +3724,22 @@ namespace ts {
             return finishNode(node);
         }
 
-        function parseJsxElementName(): EntityName {
+        function parseJsxElementName(): JsxTagNameExpression {
             scanJsxIdentifier();
-            let elementName: EntityName = parseIdentifierName();
+            // JsxElement can have name in the form of
+            //      propertyAccessExpression
+            //      primaryExpression in the form of an identifier and "this" keyword
+            // We can't just simply use parseLeftHandSideExpressionOrHigher because then we will start consider class,function etc as a keyword
+            // We only want to consider "this" as a primaryExpression
+            let expression: JsxTagNameExpression = token === SyntaxKind.ThisKeyword ?
+                parseTokenNode<PrimaryExpression>() : parseIdentifierName();
             while (parseOptional(SyntaxKind.DotToken)) {
-                scanJsxIdentifier();
-                const node: QualifiedName = <QualifiedName>createNode(SyntaxKind.QualifiedName, elementName.pos);  // !!!
-                node.left = elementName;
-                node.right = parseIdentifierName();
-                elementName = finishNode(node);
+                const propertyAccess: PropertyAccessExpression = <PropertyAccessExpression>createNode(SyntaxKind.PropertyAccessExpression, expression.pos);
+                propertyAccess.expression = expression;
+                propertyAccess.name = parseRightSideOfDot(/*allowIdentifierNames*/ true);
+                expression = finishNode(propertyAccess);
             }
-            return elementName;
+            return expression;
         }
 
         function parseJsxExpression(inExpressionContext: boolean): JsxExpression {

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -1042,11 +1042,13 @@ namespace ts {
         closingElement: JsxClosingElement;
     }
 
+    export type JsxTagNameExpression = PrimaryExpression | PropertyAccessExpression;
+
     /// The opening element of a <Tag>...</Tag> JsxElement
     // @kind(SyntaxKind.JsxOpeningElement)
     export interface JsxOpeningElement extends Expression {
         _openingElementBrand?: any;
-        tagName: EntityName;
+        tagName: JsxTagNameExpression;
         attributes: NodeArray<JsxAttribute | JsxSpreadAttribute>;
     }
 
@@ -1073,7 +1075,7 @@ namespace ts {
 
     // @kind(SyntaxKind.JsxClosingElement)
     export interface JsxClosingElement extends Node {
-        tagName: EntityName;
+        tagName: JsxTagNameExpression;
     }
 
     // @kind(SyntaxKind.JsxExpression)

diff --git a/tests/baselines/reference/tsxDynamicTagName1.js b/tests/baselines/reference/tsxDynamicTagName1.js
@@ -0,0 +1,8 @@
+//// [tsxDynamicTagName1.tsx]
+
+var CustomTag = "h1";
+<CustomTag> Hello World </CustomTag>  // No error
+
+//// [tsxDynamicTagName1.jsx]
+var CustomTag = "h1";
+<CustomTag> Hello World </CustomTag>; // No error
diff --git a/tests/baselines/reference/tsxDynamicTagName1.symbols b/tests/baselines/reference/tsxDynamicTagName1.symbols
@@ -0,0 +1,9 @@
+=== tests/cases/conformance/jsx/tsxDynamicTagName1.tsx ===
+
+var CustomTag = "h1";
+>CustomTag : Symbol(CustomTag, Decl(tsxDynamicTagName1.tsx, 1, 3))
+
+<CustomTag> Hello World </CustomTag>  // No error
+>CustomTag : Symbol(CustomTag, Decl(tsxDynamicTagName1.tsx, 1, 3))
+>CustomTag : Symbol(CustomTag, Decl(tsxDynamicTagName1.tsx, 1, 3))
+
diff --git a/tests/baselines/reference/tsxDynamicTagName1.types b/tests/baselines/reference/tsxDynamicTagName1.types
@@ -0,0 +1,11 @@
+=== tests/cases/conformance/jsx/tsxDynamicTagName1.tsx ===
+
+var CustomTag = "h1";
+>CustomTag : string
+>"h1" : string
+
+<CustomTag> Hello World </CustomTag>  // No error
+><CustomTag> Hello World </CustomTag> : any
+>CustomTag : string
+>CustomTag : string
+
diff --git a/tests/baselines/reference/tsxDynamicTagName2.errors.txt b/tests/baselines/reference/tsxDynamicTagName2.errors.txt
@@ -0,0 +1,19 @@
+tests/cases/conformance/jsx/tsxDynamicTagName2.tsx(10,1): error TS2339: Property 'customTag' does not exist on type 'JSX.IntrinsicElements'.
+tests/cases/conformance/jsx/tsxDynamicTagName2.tsx(10,25): error TS2339: Property 'customTag' does not exist on type 'JSX.IntrinsicElements'.
+
+
+==== tests/cases/conformance/jsx/tsxDynamicTagName2.tsx (2 errors) ====
+
+    declare module JSX {
+    	interface Element { }
+    	interface IntrinsicElements {
+            div: any
+    	}
+    }
+
+    var customTag = "h1";
+    <customTag> Hello World </customTag>  // This should be an error. The lower-case is look up as an intrinsic element name
+    ~~~~~~~~~~~
+!!! error TS2339: Property 'customTag' does not exist on type 'JSX.IntrinsicElements'.
+                            ~~~~~~~~~~~~
+!!! error TS2339: Property 'customTag' does not exist on type 'JSX.IntrinsicElements'.
diff --git a/tests/baselines/reference/tsxDynamicTagName2.js b/tests/baselines/reference/tsxDynamicTagName2.js
@@ -0,0 +1,15 @@
+//// [tsxDynamicTagName2.tsx]
+
+declare module JSX {
+	interface Element { }
+	interface IntrinsicElements {
+        div: any
+	}
+}
+
+var customTag = "h1";
+<customTag> Hello World </customTag>  // This should be an error. The lower-case is look up as an intrinsic element name
+
+//// [tsxDynamicTagName2.jsx]
+var customTag = "h1";
+<customTag> Hello World </customTag>; // This should be an error. The lower-case is look up as an intrinsic element name
diff --git a/tests/baselines/reference/tsxDynamicTagName3.errors.txt b/tests/baselines/reference/tsxDynamicTagName3.errors.txt
@@ -0,0 +1,16 @@
+tests/cases/conformance/jsx/tsxDynamicTagName3.tsx(10,1): error TS2339: Property 'h1' does not exist on type 'JSX.IntrinsicElements'.
+
+
+==== tests/cases/conformance/jsx/tsxDynamicTagName3.tsx (1 errors) ====
+
+    declare module JSX {
+    	interface Element { }
+    	interface IntrinsicElements {
+    		div: any
+    	}
+    }
+
+    var CustomTag: "h1" = "h1";
+    <CustomTag> Hello World </CustomTag>  // This should be an error. we will try look up string literal type in JSX.IntrinsicElements
+    ~~~~~~~~~~~
+!!! error TS2339: Property 'h1' does not exist on type 'JSX.IntrinsicElements'.
diff --git a/tests/baselines/reference/tsxDynamicTagName3.js b/tests/baselines/reference/tsxDynamicTagName3.js
@@ -0,0 +1,15 @@
+//// [tsxDynamicTagName3.tsx]
+
+declare module JSX {
+	interface Element { }
+	interface IntrinsicElements {
+		div: any
+	}
+}
+
+var CustomTag: "h1" = "h1";
+<CustomTag> Hello World </CustomTag>  // This should be an error. we will try look up string literal type in JSX.IntrinsicElements
+
+//// [tsxDynamicTagName3.jsx]
+var CustomTag = "h1";
+<CustomTag> Hello World </CustomTag>; // This should be an error. we will try look up string literal type in JSX.IntrinsicElements
diff --git a/tests/baselines/reference/tsxDynamicTagName4.js b/tests/baselines/reference/tsxDynamicTagName4.js
@@ -0,0 +1,16 @@
+//// [tsxDynamicTagName4.tsx]
+
+declare module JSX {
+	interface Element { }
+	interface IntrinsicElements {
+		div: any
+		h1: any
+	}
+}
+
+var CustomTag: "h1" = "h1";
+<CustomTag> Hello World </CustomTag>
+
+//// [tsxDynamicTagName4.jsx]
+var CustomTag = "h1";
+<CustomTag> Hello World </CustomTag>;
diff --git a/tests/baselines/reference/tsxDynamicTagName4.symbols b/tests/baselines/reference/tsxDynamicTagName4.symbols
@@ -0,0 +1,26 @@
+=== tests/cases/conformance/jsx/tsxDynamicTagName4.tsx ===
+
+declare module JSX {
+>JSX : Symbol(JSX, Decl(tsxDynamicTagName4.tsx, 0, 0))
+
+	interface Element { }
+>Element : Symbol(Element, Decl(tsxDynamicTagName4.tsx, 1, 20))
+
+	interface IntrinsicElements {
+>IntrinsicElements : Symbol(IntrinsicElements, Decl(tsxDynamicTagName4.tsx, 2, 22))
+
+		div: any
+>div : Symbol(IntrinsicElements.div, Decl(tsxDynamicTagName4.tsx, 3, 30))
+
+		h1: any
+>h1 : Symbol(IntrinsicElements.h1, Decl(tsxDynamicTagName4.tsx, 4, 10))
+	}
+}
+
+var CustomTag: "h1" = "h1";
+>CustomTag : Symbol(CustomTag, Decl(tsxDynamicTagName4.tsx, 9, 3))
+
+<CustomTag> Hello World </CustomTag>
+>CustomTag : Symbol(CustomTag, Decl(tsxDynamicTagName4.tsx, 9, 3))
+>CustomTag : Symbol(CustomTag, Decl(tsxDynamicTagName4.tsx, 9, 3))
+
diff --git a/tests/baselines/reference/tsxDynamicTagName4.types b/tests/baselines/reference/tsxDynamicTagName4.types
@@ -0,0 +1,28 @@
+=== tests/cases/conformance/jsx/tsxDynamicTagName4.tsx ===
+
+declare module JSX {
+>JSX : any
+
+	interface Element { }
+>Element : Element
+
+	interface IntrinsicElements {
+>IntrinsicElements : IntrinsicElements
+
+		div: any
+>div : any
+
+		h1: any
+>h1 : any
+	}
+}
+
+var CustomTag: "h1" = "h1";
+>CustomTag : "h1"
+>"h1" : "h1"
+
+<CustomTag> Hello World </CustomTag>
+><CustomTag> Hello World </CustomTag> : JSX.Element
+>CustomTag : "h1"
+>CustomTag : "h1"
+
diff --git a/tests/baselines/reference/tsxDynamicTagName5.js b/tests/baselines/reference/tsxDynamicTagName5.js
@@ -0,0 +1,41 @@
+//// [tests/cases/conformance/jsx/tsxDynamicTagName5.tsx] ////
+
+//// [react.d.ts]
+
+declare module 'react' {
+	class Component<T, U> { }
+}
+
+//// [app.tsx]
+import * as React from 'react';
+
+export class Text extends React.Component<{}, {}> {
+  _tagName: string = 'div';
+
+  render() {
+    return (
+      <this._tagName />
+    );
+  }
+}
+
+//// [app.jsx]
+"use strict";
+var __extends = (this && this.__extends) || function (d, b) {
+    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
+    function __() { this.constructor = d; }
+    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
+};
+var React = require('react');
+var Text = (function (_super) {
+    __extends(Text, _super);
+    function Text() {
+        _super.apply(this, arguments);
+        this._tagName = 'div';
+    }
+    Text.prototype.render = function () {
+        return (<this._tagName />);
+    };
+    return Text;
+}(React.Component));
+exports.Text = Text;
diff --git a/tests/baselines/reference/tsxDynamicTagName5.symbols b/tests/baselines/reference/tsxDynamicTagName5.symbols
@@ -0,0 +1,34 @@
+=== tests/cases/conformance/jsx/react.d.ts ===
+
+declare module 'react' {
+	class Component<T, U> { }
+>Component : Symbol(Component, Decl(react.d.ts, 1, 24))
+>T : Symbol(T, Decl(react.d.ts, 2, 17))
+>U : Symbol(U, Decl(react.d.ts, 2, 19))
+}
+
+=== tests/cases/conformance/jsx/app.tsx ===
+import * as React from 'react';
+>React : Symbol(React, Decl(app.tsx, 0, 6))
+
+export class Text extends React.Component<{}, {}> {
+>Text : Symbol(Text, Decl(app.tsx, 0, 31))
+>React.Component : Symbol(React.Component, Decl(react.d.ts, 1, 24))
+>React : Symbol(React, Decl(app.tsx, 0, 6))
+>Component : Symbol(React.Component, Decl(react.d.ts, 1, 24))
+
+  _tagName: string = 'div';
+>_tagName : Symbol(Text._tagName, Decl(app.tsx, 2, 51))
+
+  render() {
+>render : Symbol(Text.render, Decl(app.tsx, 3, 27))
+
+    return (
+      <this._tagName />
+>this._tagName : Symbol(Text._tagName, Decl(app.tsx, 2, 51))
+>this : Symbol(Text, Decl(app.tsx, 0, 31))
+>_tagName : Symbol(Text._tagName, Decl(app.tsx, 2, 51))
+
+    );
+  }
+}