Skip to content

Commit

Permalink
Baseline arity checks for jsx sfc tags
Browse files Browse the repository at this point in the history
  • Loading branch information
weswigham committed Feb 5, 2020
1 parent 68cbe9e commit 388801e
Show file tree
Hide file tree
Showing 7 changed files with 258 additions and 4 deletions.
88 changes: 84 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,7 @@ namespace ts {
if (jsxPragma) {
const chosenpragma = isArray(jsxPragma) ? jsxPragma[0] : jsxPragma;
file.localJsxFactory = parseIsolatedEntityName(chosenpragma.arguments.factory, languageVersion);
visitNode(file.localJsxFactory, markAsSynthetic);
if (file.localJsxFactory) {
return file.localJsxNamespace = getFirstIdentifier(file.localJsxFactory).escapedText;
}
Expand All @@ -949,6 +950,7 @@ namespace ts {
_jsxNamespace = "React" as __String;
if (compilerOptions.jsxFactory) {
_jsxFactoryEntity = parseIsolatedEntityName(compilerOptions.jsxFactory, languageVersion);
visitNode(_jsxFactoryEntity, markAsSynthetic);
if (_jsxFactoryEntity) {
_jsxNamespace = getFirstIdentifier(_jsxFactoryEntity).escapedText;
}
Expand All @@ -957,7 +959,16 @@ namespace ts {
_jsxNamespace = escapeLeadingUnderscores(compilerOptions.reactNamespace);
}
}
if (!_jsxFactoryEntity) {
_jsxFactoryEntity = createQualifiedName(createIdentifier(unescapeLeadingUnderscores(_jsxNamespace)), "createElement");
}
return _jsxNamespace;

function markAsSynthetic(node: Node): VisitResult<Node> {
node.pos = -1;
node.end = -1;
return visitEachChild(node, markAsSynthetic, nullTransformationContext);
}
}

function getEmitResolver(sourceFile: SourceFile, cancellationToken: CancellationToken) {
Expand Down Expand Up @@ -2801,8 +2812,8 @@ namespace ts {
const namespaceMeaning = SymbolFlags.Namespace | (isInJSFile(name) ? meaning & SymbolFlags.Value : 0);
let symbol: Symbol | undefined;
if (name.kind === SyntaxKind.Identifier) {
const message = meaning === namespaceMeaning ? Diagnostics.Cannot_find_namespace_0 : getCannotFindNameDiagnosticForName(getFirstIdentifier(name));
const symbolFromJSPrototype = isInJSFile(name) ? resolveEntityNameFromAssignmentDeclaration(name, meaning) : undefined;
const message = meaning === namespaceMeaning || nodeIsSynthesized(name) ? Diagnostics.Cannot_find_namespace_0 : getCannotFindNameDiagnosticForName(getFirstIdentifier(name));
const symbolFromJSPrototype = isInJSFile(name) && !nodeIsSynthesized(name) ? resolveEntityNameFromAssignmentDeclaration(name, meaning) : undefined;
symbol = resolveName(location || name, name.escapedText, meaning, ignoreErrors || symbolFromJSPrototype ? undefined : message, name, /*isUse*/ true);
if (!symbol) {
return symbolFromJSPrototype;
Expand Down Expand Up @@ -2845,7 +2856,7 @@ namespace ts {
throw Debug.assertNever(name, "Unknown entity name kind.");
}
Debug.assert((getCheckFlags(symbol) & CheckFlags.Instantiated) === 0, "Should never get an instantiated symbol here.");
if (isEntityName(name) && (symbol.flags & SymbolFlags.Alias || name.parent.kind === SyntaxKind.ExportAssignment)) {
if (!nodeIsSynthesized(name) && isEntityName(name) && (symbol.flags & SymbolFlags.Alias || name.parent.kind === SyntaxKind.ExportAssignment)) {
markSymbolOfAliasDeclarationIfTypeOnly(getAliasDeclarationFromName(name), symbol, /*finalTarget*/ undefined, /*overwriteEmpty*/ true);
}
return (symbol.flags & meaning) || dontResolveAlias ? symbol : resolveAlias(symbol);
Expand Down Expand Up @@ -24298,6 +24309,9 @@ namespace ts {
// can be specified by users through attributes property.
const paramType = getEffectiveFirstArgumentForJsxSignature(signature, node);
const attributesType = checkExpressionWithContextualType(node.attributes, paramType, /*inferenceContext*/ undefined, checkMode);
if (!checkTagNameDoesNotExpectTooManyArguments()) {
return false;
}
return checkTypeRelatedToAndOptionallyElaborate(
attributesType,
paramType,
Expand All @@ -24307,6 +24321,68 @@ namespace ts {
/*headMessage*/ undefined,
containingMessageChain,
errorOutputContainer);

function checkTagNameDoesNotExpectTooManyArguments(): boolean {
const tagType = isJsxOpeningElement(node) || isJsxSelfClosingElement(node) && !isJsxIntrinsicIdentifier(node.tagName) ? checkExpression(node.tagName) : undefined;
if (!tagType) {
return true;
}
const tagCallSignatures = getSignaturesOfType(tagType, SignatureKind.Call);
if (!length(tagCallSignatures)) {
return true;
}
const factory = getJsxFactoryEntity(node);
if (!factory) {
return true;
}
const factorySymbol = resolveEntityName(factory, SymbolFlags.Value, /*ignoreErrors*/ true, /*dontResolveAlias*/ false, node);
if (!factorySymbol) {
return true;
}

const factoryType = getTypeOfSymbol(factorySymbol);
const callSignatures = getSignaturesOfType(factoryType, SignatureKind.Call);
if (!length(callSignatures)) {
return true;
}

let hasFirstparamSignatures = false;
// Check that _some_
for (const sig of callSignatures) {
const firstparam = getTypeAtPosition(sig, 0);
const signaturesOfParam = getSignaturesOfType(firstparam, SignatureKind.Call);
if (!length(signaturesOfParam)) continue;
for (const paramSig of signaturesOfParam) {
hasFirstparamSignatures = true;
if (hasEffectiveRestParameter(paramSig)) {
return true; // some signature has a rest param, so function components can have an aritrary number of arguments
}
const paramCount = getParameterCount(paramSig);
for (const tagSig of tagCallSignatures) {
const tagParamCount = getParameterCount(tagSig);
if (tagParamCount <= paramCount) {
return true; // some signature accepts the number of arguments the function component provides
}
}
}
}
if (!hasFirstparamSignatures) {
// Not a single signature had a first parameter which expected a signature - for back compat, and
// to guard against generic factories which won't have signatures directly, return true
return true;
}

if (reportErrors) {
const diag = createDiagnosticForNode(node.tagName, Diagnostics.Function_like_tag_expects_more_arguments_than_the_JSX_factory_can_provide);
if (errorOutputContainer && errorOutputContainer.skipLogging) {
(errorOutputContainer.errors || (errorOutputContainer.errors = [])).push(diag);
}
if (!errorOutputContainer.skipLogging) {
diagnostics.add(diag);
}
}
return false;
}
}

function getSignatureApplicabilityError(
Expand Down Expand Up @@ -35175,6 +35251,10 @@ namespace ts {
return literalTypeToNode(<FreshableType>type, node, tracker);
}

function getJsxFactoryEntity(location: Node) {
return location ? (getJsxNamespace(location), (getSourceFileOfNode(location).localJsxFactory || _jsxFactoryEntity)) : _jsxFactoryEntity;
}

function createResolver(): EmitResolver {
// this variable and functions that use it are deliberately moved here from the outer scope
// to avoid scope pollution
Expand Down Expand Up @@ -35246,7 +35326,7 @@ namespace ts {
const symbol = node && getSymbolOfNode(node);
return !!(symbol && getCheckFlags(symbol) & CheckFlags.Late);
},
getJsxFactoryEntity: location => location ? (getJsxNamespace(location), (getSourceFileOfNode(location).localJsxFactory || _jsxFactoryEntity)) : _jsxFactoryEntity,
getJsxFactoryEntity,
getAllAccessorDeclarations(accessor: AccessorDeclaration): AllAccessorDeclarations {
accessor = getParseTreeNode(accessor, isGetOrSetAccessorDeclaration)!; // TODO: GH#18217
const otherKind = accessor.kind === SyntaxKind.SetAccessor ? SyntaxKind.GetAccessor : SyntaxKind.SetAccessor;
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4280,6 +4280,10 @@
"category": "Message",
"code": 6228
},
"Function-like tag expects more arguments than the JSX factory can provide.": {
"category": "Error",
"code": 6229
},

"Projects to reference": {
"category": "Message",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx(13,12): error TS6229: Function-like tag expects more arguments than the JSX factory can provide.


==== tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx (1 errors) ====
/// <reference path="/.lib/react16.d.ts" />

import * as React from "react";

interface MyProps {
x: number;
}

function MyComp(props: MyProps, context: any, bad: any, verybad: any) {
return <div></div>;
}

const a = <MyComp x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
~~~~~~
!!! error TS6229: Function-like tag expects more arguments than the JSX factory can provide.

function MyComp2(props: MyProps, context: any) {
return <div></div>
}
const b = <MyComp2 x={2}/>; // Should be OK, `context` is allowed, per react rules
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//// [jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx]
/// <reference path="/.lib/react16.d.ts" />

import * as React from "react";

interface MyProps {
x: number;
}

function MyComp(props: MyProps, context: any, bad: any, verybad: any) {
return <div></div>;
}

const a = <MyComp x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides

function MyComp2(props: MyProps, context: any) {
return <div></div>
}
const b = <MyComp2 x={2}/>; // Should be OK, `context` is allowed, per react rules
//// [jsxIssuesErrorWhenTagExpectsTooManyArguments.js]
"use strict";
/// <reference path="react16.d.ts" />
exports.__esModule = true;
var React = require("react");
function MyComp(props, context, bad, verybad) {
return React.createElement("div", null);
}
var a = React.createElement(MyComp, { x: 2 }); // using `MyComp` as a component should error - it expects more arguments than react provides
function MyComp2(props, context) {
return React.createElement("div", null);
}
var b = React.createElement(MyComp2, { x: 2 }); // Should be OK, `context` is allowed, per react rules
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
=== tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx ===
/// <reference path="react16.d.ts" />

import * as React from "react";
>React : Symbol(React, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 2, 6))

interface MyProps {
>MyProps : Symbol(MyProps, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 2, 31))

x: number;
>x : Symbol(MyProps.x, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 4, 19))
}

function MyComp(props: MyProps, context: any, bad: any, verybad: any) {
>MyComp : Symbol(MyComp, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 6, 1))
>props : Symbol(props, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 8, 16))
>MyProps : Symbol(MyProps, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 2, 31))
>context : Symbol(context, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 8, 31))
>bad : Symbol(bad, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 8, 45))
>verybad : Symbol(verybad, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 8, 55))

return <div></div>;
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
}

const a = <MyComp x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
>a : Symbol(a, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 12, 5))
>MyComp : Symbol(MyComp, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 6, 1))
>x : Symbol(x, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 12, 17))

function MyComp2(props: MyProps, context: any) {
>MyComp2 : Symbol(MyComp2, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 12, 26))
>props : Symbol(props, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 14, 17))
>MyProps : Symbol(MyProps, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 2, 31))
>context : Symbol(context, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 14, 32))

return <div></div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
}
const b = <MyComp2 x={2}/>; // Should be OK, `context` is allowed, per react rules
>b : Symbol(b, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 17, 5))
>MyComp2 : Symbol(MyComp2, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 12, 26))
>x : Symbol(x, Decl(jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx, 17, 19))

Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
=== tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx ===
/// <reference path="react16.d.ts" />

import * as React from "react";
>React : typeof React

interface MyProps {
x: number;
>x : number
}

function MyComp(props: MyProps, context: any, bad: any, verybad: any) {
>MyComp : (props: MyProps, context: any, bad: any, verybad: any) => JSX.Element
>props : MyProps
>context : any
>bad : any
>verybad : any

return <div></div>;
><div></div> : JSX.Element
>div : any
>div : any
}

const a = <MyComp x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
>a : JSX.Element
><MyComp x={2}/> : JSX.Element
>MyComp : (props: MyProps, context: any, bad: any, verybad: any) => JSX.Element
>x : number
>2 : 2

function MyComp2(props: MyProps, context: any) {
>MyComp2 : (props: MyProps, context: any) => JSX.Element
>props : MyProps
>context : any

return <div></div>
><div></div> : JSX.Element
>div : any
>div : any
}
const b = <MyComp2 x={2}/>; // Should be OK, `context` is allowed, per react rules
>b : JSX.Element
><MyComp2 x={2}/> : JSX.Element
>MyComp2 : (props: MyProps, context: any) => JSX.Element
>x : number
>2 : 2

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// @jsx: react
/// <reference path="/.lib/react16.d.ts" />

import * as React from "react";

interface MyProps {
x: number;
}

function MyComp(props: MyProps, context: any, bad: any, verybad: any) {
return <div></div>;
}

const a = <MyComp x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides

function MyComp2(props: MyProps, context: any) {
return <div></div>
}
const b = <MyComp2 x={2}/>; // Should be OK, `context` is allowed, per react rules

0 comments on commit 388801e

Please sign in to comment.