From 306de4d6816d6b365a1f7d4f2040d5f10ce30f2f Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Fri, 26 Jun 2015 14:18:51 -0700 Subject: [PATCH] CR feedback --- src/compiler/checker.ts | 33 ++++---- src/compiler/emitter.ts | 60 +++++++------- src/compiler/parser.ts | 6 +- src/compiler/types.ts | 2 - src/compiler/utilities.ts | 5 ++ src/services/services.ts | 2 +- .../jsxEsprimaFbTestSuite.errors.txt | 81 +++++++++++++++++++ .../reference/jsxEsprimaFbTestSuite.js | 5 +- .../jsxInvalidEsprimaTestSuite.errors.txt | 9 ++- .../reference/jsxInvalidEsprimaTestSuite.js | 4 +- 10 files changed, 145 insertions(+), 62 deletions(-) create mode 100644 tests/baselines/reference/jsxEsprimaFbTestSuite.errors.txt diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 101b8ceb23c..f3f87f05c0e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -165,7 +165,7 @@ namespace ts { } }; - let JsxNames = { + const JsxNames = { JSX: "JSX", IntrinsicElements: "IntrinsicElements", ElementClass: "ElementClass", @@ -5578,6 +5578,7 @@ namespace ts { case SyntaxKind.JsxAttribute: case SyntaxKind.JsxSpreadAttribute: case SyntaxKind.JsxOpeningElement: + case SyntaxKind.JsxExpression: return forEachChild(node, isAssignedIn); } return false; @@ -6774,8 +6775,7 @@ namespace ts { return false; } else { - let firstChar = (tagName).text.charAt(0); - return firstChar.toLowerCase() === firstChar; + return isIntrinsicJsxName((tagName).text); } } @@ -6836,10 +6836,7 @@ namespace ts { /// Returns the type JSX.IntrinsicElements. May return `unknownType` if that type is not present. function getJsxIntrinsicElementsType() { if (!jsxIntrinsicElementsType) { - let jsxNamespace = getGlobalSymbol(JsxNames.JSX, SymbolFlags.Namespace, undefined); - let intrinsicsSymbol = jsxNamespace && getSymbol(jsxNamespace.exports, JsxNames.IntrinsicElements, SymbolFlags.Type); - let intrinsicsType = intrinsicsSymbol && getDeclaredTypeOfSymbol(intrinsicsSymbol); - jsxIntrinsicElementsType = intrinsicsType || unknownType; + jsxIntrinsicElementsType = getExportedTypeFromNamespace(JsxNames.JSX, JsxNames.IntrinsicElements) || unknownType; } return jsxIntrinsicElementsType; } @@ -6977,18 +6974,23 @@ namespace ts { let attribProperties = attribPropType && getPropertiesOfType(attribPropType); if (attribProperties) { + // Element Attributes has zero properties, so the element attributes type will be the class instance type if (attribProperties.length === 0) { return ''; } + // Element Attributes has one property, so the element attributes type will be the type of the corresponding + // property of the class instance type else if (attribProperties.length === 1) { return attribProperties[0].name; } + // More than one property on ElementAttributesProperty is an error else { error(attribsPropTypeSym.declarations[0], Diagnostics.The_global_type_JSX_0_may_not_have_more_than_one_property, JsxNames.ElementAttributesPropertyNameContainer); return undefined; } } else { + // No interface exists, so the element attributes type will be an implicit any return undefined; } } @@ -7044,7 +7046,7 @@ namespace ts { return links.resolvedJsxType = getIndexTypeOfSymbol(sym, IndexKind.String); } else { - // Resolution failed + // Resolution failed, so we don't know return links.resolvedJsxType = anyType; } } @@ -7063,16 +7065,12 @@ namespace ts { return prop || unknownSymbol; } + let jsxElementClassType: Type = undefined; function getJsxGlobalElementClassType(): Type { - let jsxNS = getGlobalSymbol(JsxNames.JSX, SymbolFlags.Namespace, /*diagnosticMessage*/ undefined); - if (jsxNS) { - let sym = getSymbol(jsxNS.exports, JsxNames.ElementClass, SymbolFlags.Type); - let elemClassType = sym && getDeclaredTypeOfSymbol(sym); - return elemClassType; - } - else { - return undefined; + if(!jsxElementClassType) { + jsxElementClassType = getExportedTypeFromNamespace(JsxNames.JSX, JsxNames.ElementClass); } + return jsxElementClassType; } /// Returns all the properties of the Jsx.IntrinsicElements interface @@ -7137,8 +7135,7 @@ namespace ts { return checkExpression(node.expression); } else { - /// is shorthand for - return booleanType; + return unknownType; } } diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 2764ae8e86a..3b1a1609c39 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -1113,20 +1113,12 @@ var __param = (this && this.__param) || function (paramIndex, decorator) { /// 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) { - if (name.kind === SyntaxKind.Identifier) { - var ch = (name).text.charAt(0); - if (ch.toUpperCase() === ch) { - emit(name); - } - else { - write('"'); - emit(name); - write('"'); - } - return ch.toUpperCase() !== ch; + if (name.kind === SyntaxKind.Identifier && isIntrinsicJsxName((name).text)) { + write('"'); + emit(name); + write('"'); } else { - Debug.assert(name.kind === SyntaxKind.QualifiedName); emit(name); } } @@ -1234,12 +1226,19 @@ var __param = (this && this.__param) || function (paramIndex, decorator) { } // Don't emit empty strings - if (children[i].kind === SyntaxKind.JsxText && !shouldEmitJsxText(children[i])) { - continue; + if (children[i].kind === SyntaxKind.JsxText) { + let text = getTextToEmit(children[i]); + if(text !== undefined) { + write(', "'); + write(text); + write('"'); + } + } + else { + write(', '); + emit(children[i]); } - write(', '); - emit(children[i]); } } @@ -5895,12 +5894,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) { } function trimReactWhitespace(node: JsxText): string { - // Could be empty string, do not use !node.formattedReactText - if (node.formattedReactText !== undefined) { - return node.formattedReactText; - } - - let lines: string[] = []; + let result: string = undefined; let text = getTextOfNode(node); let firstNonWhitespace = 0; let lastNonWhitespace = -1; @@ -5910,9 +5904,10 @@ var __param = (this && this.__param) || function (paramIndex, decorator) { // on the same line as the closing tag. See examples in tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx for (let i = 0; i < text.length; i++) { let c = text.charCodeAt(i); - if (c === CharacterCodes.lineFeed || c === CharacterCodes.carriageReturn) { + if (isLineBreak(c)) { if (firstNonWhitespace !== -1 && (lastNonWhitespace - firstNonWhitespace + 1 > 0)) { - lines.push(text.substr(firstNonWhitespace, lastNonWhitespace - firstNonWhitespace + 1)); + let part = text.substr(firstNonWhitespace, lastNonWhitespace - firstNonWhitespace + 1); + result = (result ? result + '" + \' \' + "' : '') + part; } firstNonWhitespace = -1; } @@ -5924,19 +5919,26 @@ var __param = (this && this.__param) || function (paramIndex, decorator) { } } if (firstNonWhitespace !== -1) { - lines.push(text.substr(firstNonWhitespace)); + let part = text.substr(firstNonWhitespace); + result = (result ? result + '" + \' \' + "' : '') + part; } - return node.formattedReactText = lines.join('" + \' \' + "'); + return result; } - function shouldEmitJsxText(node: JsxText) { + function getTextToEmit(node: JsxText) { switch (compilerOptions.jsx) { case JsxEmit.React: - return trimReactWhitespace(node).length > 0; + let text = trimReactWhitespace(node); + if (text.length === 0) { + return undefined; + } + else { + return text; + } case JsxEmit.Preserve: default: - return true; + return getTextOfNode(node, true); } } diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index af2e6b98547..84f05719151 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -657,7 +657,7 @@ namespace ts { sourceFile.languageVersion = languageVersion; sourceFile.fileName = normalizePath(fileName); sourceFile.flags = fileExtensionIs(sourceFile.fileName, ".d.ts") ? NodeFlags.DeclarationFile : 0; - sourceFile.languageVariant = fileExtensionIs(sourceFile.fileName, ".tsx") ? LanguageVariant.JSX : LanguageVariant.Standard; + sourceFile.languageVariant = isTsx(sourceFile.fileName) ? LanguageVariant.JSX : LanguageVariant.Standard; return sourceFile; } @@ -1298,7 +1298,6 @@ namespace ts { case ParsingContext.HeritageClauses: return token === SyntaxKind.OpenBraceToken || token === SyntaxKind.CloseBraceToken; case ParsingContext.JsxAttributes: - // REMOVE -> // For error recovery, include } here (otherwise an over-braced {expr}} will close the surrounding statement block and mess up the entire file). return token === SyntaxKind.GreaterThanToken || token === SyntaxKind.SlashToken; case ParsingContext.JsxChildren: return token === SyntaxKind.LessThanToken && lookAhead(nextTokenIsSlash); @@ -3377,9 +3376,6 @@ namespace ts { node.name = parseIdentifierName(); if (parseOptional(SyntaxKind.EqualsToken)) { switch (token) { - case SyntaxKind.LessThanToken: - node.initializer = parseJsxElementOrSelfClosingElement(); - break; case SyntaxKind.StringLiteral: node.initializer = parseLiteralNode(); break; diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 7f45138bf7d..0b7605ef77f 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -877,8 +877,6 @@ namespace ts { export interface JsxText extends Node { _jsxTextExpressionBrand: any; - /// Used by the emitter to avoid recomputation - formattedReactText?: string; } export type JsxChild = JsxText | JsxExpression | JsxElement | JsxSelfClosingElement; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 29f91e8c502..b326d5256f0 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1535,6 +1535,11 @@ namespace ts { } } + export function isIntrinsicJsxName(name: string) { + let ch = name.substr(0, 1); + return ch.toLowerCase() === ch; + } + function get16BitUnicodeEscapeSequence(charCode: number): string { let hexCharCode = charCode.toString(16).toUpperCase(); let paddedHexCode = ("0000" + hexCharCode).slice(-4); diff --git a/src/services/services.ts b/src/services/services.ts index 7b9de36e33c..d2229bdbf91 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2934,7 +2934,7 @@ namespace ts { getTypeScriptMemberSymbols(); } else if (isRightOfOpenTag) { - let tagSymbols = typeChecker.getJsxIntrinsicTagNames();; + let tagSymbols = typeChecker.getJsxIntrinsicTagNames(); if (tryGetGlobalSymbols()) { symbols = tagSymbols.concat(symbols.filter(s => !!(s.flags & SymbolFlags.Value))); } diff --git a/tests/baselines/reference/jsxEsprimaFbTestSuite.errors.txt b/tests/baselines/reference/jsxEsprimaFbTestSuite.errors.txt new file mode 100644 index 00000000000..5758db36960 --- /dev/null +++ b/tests/baselines/reference/jsxEsprimaFbTestSuite.errors.txt @@ -0,0 +1,81 @@ +tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,17): error TS1005: '{' expected. +tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,23): error TS1005: '}' expected. +tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,29): error TS1005: '{' expected. +tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,57): error TS1109: Expression expected. +tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,58): error TS1109: Expression expected. +tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,1): error TS1003: Identifier expected. +tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,6): error TS1109: Expression expected. +tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,12): error TS1109: Expression expected. + + +==== tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx (8 errors) ==== + declare var React: any; + declare var 日本語; + declare var AbC_def; + declare var LeftRight; + declare var x; + declare var a; + declare var props; + + ; + + //; Namespace unsuported + + // {value} ; Namespace unsuported + + ; + + ; + ; + + <日本語>; + + + bar + baz + ; + + : } />; + + {}; + + {/* this is a comment */}; + +
@test content
; + +

7x invalid-js-identifier
; + + right=monkeys /> gorillas />; + ~ +!!! error TS1005: '{' expected. + ~~~~~ +!!! error TS1005: '}' expected. + ~ +!!! error TS1005: '{' expected. + ~ +!!! error TS1109: Expression expected. + ~ +!!! error TS1109: Expression expected. + + ; + ~ +!!! error TS1003: Identifier expected. + ~~ +!!! error TS1109: Expression expected. + ~ +!!! error TS1109: Expression expected. + + ; + + (
) < x; + +
; + +
; + +
; + + ; + \ No newline at end of file diff --git a/tests/baselines/reference/jsxEsprimaFbTestSuite.js b/tests/baselines/reference/jsxEsprimaFbTestSuite.js index 2050c39972c..1e22826f440 100644 --- a/tests/baselines/reference/jsxEsprimaFbTestSuite.js +++ b/tests/baselines/reference/jsxEsprimaFbTestSuite.js @@ -71,8 +71,9 @@ baz ;
@test content
;

7x invalid-js-identifier
; - right=monkeys /> gorillas/>; -; +} right={monkeys /> gorillas / > }/> + < a.b > ; +a.b > ; ; (
) < x;
; diff --git a/tests/baselines/reference/jsxInvalidEsprimaTestSuite.errors.txt b/tests/baselines/reference/jsxInvalidEsprimaTestSuite.errors.txt index e63809913f2..3e2a10e9931 100644 --- a/tests/baselines/reference/jsxInvalidEsprimaTestSuite.errors.txt +++ b/tests/baselines/reference/jsxInvalidEsprimaTestSuite.errors.txt @@ -65,12 +65,13 @@ tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(28,10): error TS2304: tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(28,28): error TS1005: '>' expected. tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(28,29): error TS1109: Expression expected. tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(32,6): error TS1005: '{' expected. -tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(33,7): error TS1003: Identifier expected. +tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(33,6): error TS1005: '{' expected. +tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(33,7): error TS1109: Expression expected. tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(35,4): error TS1003: Identifier expected. tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(35,21): error TS17002: Expected corresponding JSX closing tag for 'a'. -==== tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx (70 errors) ==== +==== tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx (71 errors) ==== declare var React: any; ; @@ -238,8 +239,10 @@ tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(35,21): error TS17002 ~ !!! error TS1005: '{' expected. ; + ~ +!!! error TS1005: '{' expected. ~ -!!! error TS1003: Identifier expected. +!!! error TS1109: Expression expected. }; ; ~~~ diff --git a/tests/baselines/reference/jsxInvalidEsprimaTestSuite.js b/tests/baselines/reference/jsxInvalidEsprimaTestSuite.js index a53cf33410d..13808cc29e0 100644 --- a/tests/baselines/reference/jsxInvalidEsprimaTestSuite.js +++ b/tests/baselines/reference/jsxInvalidEsprimaTestSuite.js @@ -76,6 +76,6 @@ var x =
one
/* intervening comment */ /* intervening comment */