Skip to content

Commit 6b84aff

Browse files
committed
Baseline arity checks for jsx sfc tags
Finish comment PR feedback
1 parent 7a3b6b4 commit 6b84aff

8 files changed

+363
-7
lines changed

src/compiler/checker.ts

Lines changed: 94 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -940,6 +940,7 @@ namespace ts {
940940
if (jsxPragma) {
941941
const chosenpragma = isArray(jsxPragma) ? jsxPragma[0] : jsxPragma;
942942
file.localJsxFactory = parseIsolatedEntityName(chosenpragma.arguments.factory, languageVersion);
943+
visitNode(file.localJsxFactory, markAsSynthetic);
943944
if (file.localJsxFactory) {
944945
return file.localJsxNamespace = getFirstIdentifier(file.localJsxFactory).escapedText;
945946
}
@@ -950,6 +951,7 @@ namespace ts {
950951
_jsxNamespace = "React" as __String;
951952
if (compilerOptions.jsxFactory) {
952953
_jsxFactoryEntity = parseIsolatedEntityName(compilerOptions.jsxFactory, languageVersion);
954+
visitNode(_jsxFactoryEntity, markAsSynthetic);
953955
if (_jsxFactoryEntity) {
954956
_jsxNamespace = getFirstIdentifier(_jsxFactoryEntity).escapedText;
955957
}
@@ -958,7 +960,16 @@ namespace ts {
958960
_jsxNamespace = escapeLeadingUnderscores(compilerOptions.reactNamespace);
959961
}
960962
}
963+
if (!_jsxFactoryEntity) {
964+
_jsxFactoryEntity = createQualifiedName(createIdentifier(unescapeLeadingUnderscores(_jsxNamespace)), "createElement");
965+
}
961966
return _jsxNamespace;
967+
968+
function markAsSynthetic(node: Node): VisitResult<Node> {
969+
node.pos = -1;
970+
node.end = -1;
971+
return visitEachChild(node, markAsSynthetic, nullTransformationContext);
972+
}
962973
}
963974

964975
function getEmitResolver(sourceFile: SourceFile, cancellationToken: CancellationToken) {
@@ -2802,8 +2813,8 @@ namespace ts {
28022813
const namespaceMeaning = SymbolFlags.Namespace | (isInJSFile(name) ? meaning & SymbolFlags.Value : 0);
28032814
let symbol: Symbol | undefined;
28042815
if (name.kind === SyntaxKind.Identifier) {
2805-
const message = meaning === namespaceMeaning ? Diagnostics.Cannot_find_namespace_0 : getCannotFindNameDiagnosticForName(getFirstIdentifier(name));
2806-
const symbolFromJSPrototype = isInJSFile(name) ? resolveEntityNameFromAssignmentDeclaration(name, meaning) : undefined;
2816+
const message = meaning === namespaceMeaning || nodeIsSynthesized(name) ? Diagnostics.Cannot_find_namespace_0 : getCannotFindNameDiagnosticForName(getFirstIdentifier(name));
2817+
const symbolFromJSPrototype = isInJSFile(name) && !nodeIsSynthesized(name) ? resolveEntityNameFromAssignmentDeclaration(name, meaning) : undefined;
28072818
symbol = resolveName(location || name, name.escapedText, meaning, ignoreErrors || symbolFromJSPrototype ? undefined : message, name, /*isUse*/ true);
28082819
if (!symbol) {
28092820
return symbolFromJSPrototype;
@@ -2846,7 +2857,7 @@ namespace ts {
28462857
throw Debug.assertNever(name, "Unknown entity name kind.");
28472858
}
28482859
Debug.assert((getCheckFlags(symbol) & CheckFlags.Instantiated) === 0, "Should never get an instantiated symbol here.");
2849-
if (isEntityName(name) && (symbol.flags & SymbolFlags.Alias || name.parent.kind === SyntaxKind.ExportAssignment)) {
2860+
if (!nodeIsSynthesized(name) && isEntityName(name) && (symbol.flags & SymbolFlags.Alias || name.parent.kind === SyntaxKind.ExportAssignment)) {
28502861
markSymbolOfAliasDeclarationIfTypeOnly(getAliasDeclarationFromName(name), symbol, /*finalTarget*/ undefined, /*overwriteEmpty*/ true);
28512862
}
28522863
return (symbol.flags & meaning) || dontResolveAlias ? symbol : resolveAlias(symbol);
@@ -24389,7 +24400,7 @@ namespace ts {
2438924400
// can be specified by users through attributes property.
2439024401
const paramType = getEffectiveFirstArgumentForJsxSignature(signature, node);
2439124402
const attributesType = checkExpressionWithContextualType(node.attributes, paramType, /*inferenceContext*/ undefined, checkMode);
24392-
return checkTypeRelatedToAndOptionallyElaborate(
24403+
return checkTagNameDoesNotExpectTooManyArguments() && checkTypeRelatedToAndOptionallyElaborate(
2439324404
attributesType,
2439424405
paramType,
2439524406
relation,
@@ -24398,6 +24409,80 @@ namespace ts {
2439824409
/*headMessage*/ undefined,
2439924410
containingMessageChain,
2440024411
errorOutputContainer);
24412+
24413+
function checkTagNameDoesNotExpectTooManyArguments(): boolean {
24414+
const tagType = isJsxOpeningElement(node) || isJsxSelfClosingElement(node) && !isJsxIntrinsicIdentifier(node.tagName) ? checkExpression(node.tagName) : undefined;
24415+
if (!tagType) {
24416+
return true;
24417+
}
24418+
const tagCallSignatures = getSignaturesOfType(tagType, SignatureKind.Call);
24419+
if (!length(tagCallSignatures)) {
24420+
return true;
24421+
}
24422+
const factory = getJsxFactoryEntity(node);
24423+
if (!factory) {
24424+
return true;
24425+
}
24426+
const factorySymbol = resolveEntityName(factory, SymbolFlags.Value, /*ignoreErrors*/ true, /*dontResolveAlias*/ false, node);
24427+
if (!factorySymbol) {
24428+
return true;
24429+
}
24430+
24431+
const factoryType = getTypeOfSymbol(factorySymbol);
24432+
const callSignatures = getSignaturesOfType(factoryType, SignatureKind.Call);
24433+
if (!length(callSignatures)) {
24434+
return true;
24435+
}
24436+
24437+
let hasFirstParamSignatures = false;
24438+
let maxParamCount = 0;
24439+
// Check that _some_ first parameter expects a FC-like thing, and that some overload of the SFC expects an acceptable number of arguments
24440+
for (const sig of callSignatures) {
24441+
const firstparam = getTypeAtPosition(sig, 0);
24442+
const signaturesOfParam = getSignaturesOfType(firstparam, SignatureKind.Call);
24443+
if (!length(signaturesOfParam)) continue;
24444+
for (const paramSig of signaturesOfParam) {
24445+
hasFirstParamSignatures = true;
24446+
if (hasEffectiveRestParameter(paramSig)) {
24447+
return true; // some signature has a rest param, so function components can have an arbitrary number of arguments
24448+
}
24449+
const paramCount = getParameterCount(paramSig);
24450+
if (paramCount > maxParamCount) {
24451+
maxParamCount = paramCount;
24452+
}
24453+
}
24454+
}
24455+
if (!hasFirstParamSignatures) {
24456+
// Not a single signature had a first parameter which expected a signature - for back compat, and
24457+
// to guard against generic factories which won't have signatures directly, do not error
24458+
return true;
24459+
}
24460+
let absoluteMinArgCount = Infinity;
24461+
for (const tagSig of tagCallSignatures) {
24462+
const tagRequiredArgCount = getMinArgumentCount(tagSig);
24463+
if (tagRequiredArgCount < absoluteMinArgCount) {
24464+
absoluteMinArgCount = tagRequiredArgCount;
24465+
}
24466+
}
24467+
if (absoluteMinArgCount <= maxParamCount) {
24468+
return true; // some signature accepts the number of arguments the function component provides
24469+
}
24470+
24471+
if (reportErrors) {
24472+
const diag = createDiagnosticForNode(node.tagName, Diagnostics.Tag_0_expects_at_least_1_arguments_but_the_JSX_factory_2_provides_at_most_3, entityNameToString(node.tagName), absoluteMinArgCount, entityNameToString(factory), maxParamCount);
24473+
const tagNameDeclaration = getSymbolAtLocation(node.tagName)?.valueDeclaration;
24474+
if (tagNameDeclaration) {
24475+
addRelatedInfo(diag, createDiagnosticForNode(tagNameDeclaration, Diagnostics._0_is_declared_here, entityNameToString(node.tagName)));
24476+
}
24477+
if (errorOutputContainer && errorOutputContainer.skipLogging) {
24478+
(errorOutputContainer.errors || (errorOutputContainer.errors = [])).push(diag);
24479+
}
24480+
if (!errorOutputContainer.skipLogging) {
24481+
diagnostics.add(diag);
24482+
}
24483+
}
24484+
return false;
24485+
}
2440124486
}
2440224487

2440324488
function getSignatureApplicabilityError(
@@ -35276,6 +35361,10 @@ namespace ts {
3527635361
return literalTypeToNode(<FreshableType>type, node, tracker);
3527735362
}
3527835363

35364+
function getJsxFactoryEntity(location: Node) {
35365+
return location ? (getJsxNamespace(location), (getSourceFileOfNode(location).localJsxFactory || _jsxFactoryEntity)) : _jsxFactoryEntity;
35366+
}
35367+
3527935368
function createResolver(): EmitResolver {
3528035369
// this variable and functions that use it are deliberately moved here from the outer scope
3528135370
// to avoid scope pollution
@@ -35347,7 +35436,7 @@ namespace ts {
3534735436
const symbol = node && getSymbolOfNode(node);
3534835437
return !!(symbol && getCheckFlags(symbol) & CheckFlags.Late);
3534935438
},
35350-
getJsxFactoryEntity: location => location ? (getJsxNamespace(location), (getSourceFileOfNode(location).localJsxFactory || _jsxFactoryEntity)) : _jsxFactoryEntity,
35439+
getJsxFactoryEntity,
3535135440
getAllAccessorDeclarations(accessor: AccessorDeclaration): AllAccessorDeclarations {
3535235441
accessor = getParseTreeNode(accessor, isGetOrSetAccessorDeclaration)!; // TODO: GH#18217
3535335442
const otherKind = accessor.kind === SyntaxKind.SetAccessor ? SyntaxKind.GetAccessor : SyntaxKind.SetAccessor;

src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4292,6 +4292,10 @@
42924292
"category": "Message",
42934293
"code": 6228
42944294
},
4295+
"Tag '{0}' expects at least '{1}' arguments, but the JSX factory '{2}' provides at most '{3}'.": {
4296+
"category": "Error",
4297+
"code": 6229
4298+
},
42954299

42964300
"Projects to reference": {
42974301
"category": "Message",

src/compiler/utilities.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -864,14 +864,17 @@ namespace ts {
864864
}
865865
}
866866

867-
export function entityNameToString(name: EntityNameOrEntityNameExpression): string {
867+
export function entityNameToString(name: EntityNameOrEntityNameExpression | JsxTagNameExpression | PrivateIdentifier): string {
868868
switch (name.kind) {
869+
case SyntaxKind.ThisKeyword:
870+
return "this";
871+
case SyntaxKind.PrivateIdentifier:
869872
case SyntaxKind.Identifier:
870873
return getFullWidth(name) === 0 ? idText(name) : getTextOfNode(name);
871874
case SyntaxKind.QualifiedName:
872875
return entityNameToString(name.left) + "." + entityNameToString(name.right);
873876
case SyntaxKind.PropertyAccessExpression:
874-
if (isIdentifier(name.name)) {
877+
if (isIdentifier(name.name) || isPrivateIdentifier(name.name)) {
875878
return entityNameToString(name.expression) + "." + entityNameToString(name.name);
876879
}
877880
else {
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx(19,12): error TS6229: Tag 'MyComp4' expects at least '4' arguments, but the JSX factory 'React.createElement' provides at most '2'.
2+
tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx(20,12): error TS6229: Tag 'MyComp3' expects at least '3' arguments, but the JSX factory 'React.createElement' provides at most '2'.
3+
4+
5+
==== tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx (2 errors) ====
6+
/// <reference path="/.lib/react16.d.ts" />
7+
8+
import * as React from "react";
9+
10+
interface MyProps {
11+
x: number;
12+
}
13+
14+
function MyComp4(props: MyProps, context: any, bad: any, verybad: any) {
15+
return <div></div>;
16+
}
17+
function MyComp3(props: MyProps, context: any, bad: any) {
18+
return <div></div>;
19+
}
20+
function MyComp2(props: MyProps, context: any) {
21+
return <div></div>
22+
}
23+
24+
const a = <MyComp4 x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
25+
~~~~~~~
26+
!!! error TS6229: Tag 'MyComp4' expects at least '4' arguments, but the JSX factory 'React.createElement' provides at most '2'.
27+
!!! related TS2728 tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx:9:10: 'MyComp4' is declared here.
28+
const b = <MyComp3 x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
29+
~~~~~~~
30+
!!! error TS6229: Tag 'MyComp3' expects at least '3' arguments, but the JSX factory 'React.createElement' provides at most '2'.
31+
!!! related TS2728 tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx:12:10: 'MyComp3' is declared here.
32+
const c = <MyComp2 x={2}/>; // Should be OK, `context` is allowed, per react rules
33+
34+
declare function MyTagWithOptionalNonJSXBits(props: MyProps, context: any, nonReactArg?: string): JSX.Element;
35+
const d = <MyTagWithOptionalNonJSXBits x={2} />; // Technically OK, but probably questionable
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
//// [jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx]
2+
/// <reference path="/.lib/react16.d.ts" />
3+
4+
import * as React from "react";
5+
6+
interface MyProps {
7+
x: number;
8+
}
9+
10+
function MyComp4(props: MyProps, context: any, bad: any, verybad: any) {
11+
return <div></div>;
12+
}
13+
function MyComp3(props: MyProps, context: any, bad: any) {
14+
return <div></div>;
15+
}
16+
function MyComp2(props: MyProps, context: any) {
17+
return <div></div>
18+
}
19+
20+
const a = <MyComp4 x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
21+
const b = <MyComp3 x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
22+
const c = <MyComp2 x={2}/>; // Should be OK, `context` is allowed, per react rules
23+
24+
declare function MyTagWithOptionalNonJSXBits(props: MyProps, context: any, nonReactArg?: string): JSX.Element;
25+
const d = <MyTagWithOptionalNonJSXBits x={2} />; // Technically OK, but probably questionable
26+
27+
//// [jsxIssuesErrorWhenTagExpectsTooManyArguments.js]
28+
"use strict";
29+
/// <reference path="react16.d.ts" />
30+
exports.__esModule = true;
31+
var React = require("react");
32+
function MyComp4(props, context, bad, verybad) {
33+
return React.createElement("div", null);
34+
}
35+
function MyComp3(props, context, bad) {
36+
return React.createElement("div", null);
37+
}
38+
function MyComp2(props, context) {
39+
return React.createElement("div", null);
40+
}
41+
var a = React.createElement(MyComp4, { x: 2 }); // using `MyComp` as a component should error - it expects more arguments than react provides
42+
var b = React.createElement(MyComp3, { x: 2 }); // using `MyComp` as a component should error - it expects more arguments than react provides
43+
var c = React.createElement(MyComp2, { x: 2 }); // Should be OK, `context` is allowed, per react rules
44+
var d = React.createElement(MyTagWithOptionalNonJSXBits, { x: 2 }); // Technically OK, but probably questionable

0 commit comments

Comments
 (0)