Skip to content

Commit 7d8dc73

Browse files
authored
Baseline arity checks for jsx sfc tags (#36643)
Finish comment PR feedback
1 parent e536c89 commit 7d8dc73

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);
@@ -24391,7 +24402,7 @@ namespace ts {
2439124402
// can be specified by users through attributes property.
2439224403
const paramType = getEffectiveFirstArgumentForJsxSignature(signature, node);
2439324404
const attributesType = checkExpressionWithContextualType(node.attributes, paramType, /*inferenceContext*/ undefined, checkMode);
24394-
return checkTypeRelatedToAndOptionallyElaborate(
24405+
return checkTagNameDoesNotExpectTooManyArguments() && checkTypeRelatedToAndOptionallyElaborate(
2439524406
attributesType,
2439624407
paramType,
2439724408
relation,
@@ -24400,6 +24411,80 @@ namespace ts {
2440024411
/*headMessage*/ undefined,
2440124412
containingMessageChain,
2440224413
errorOutputContainer);
24414+
24415+
function checkTagNameDoesNotExpectTooManyArguments(): boolean {
24416+
const tagType = isJsxOpeningElement(node) || isJsxSelfClosingElement(node) && !isJsxIntrinsicIdentifier(node.tagName) ? checkExpression(node.tagName) : undefined;
24417+
if (!tagType) {
24418+
return true;
24419+
}
24420+
const tagCallSignatures = getSignaturesOfType(tagType, SignatureKind.Call);
24421+
if (!length(tagCallSignatures)) {
24422+
return true;
24423+
}
24424+
const factory = getJsxFactoryEntity(node);
24425+
if (!factory) {
24426+
return true;
24427+
}
24428+
const factorySymbol = resolveEntityName(factory, SymbolFlags.Value, /*ignoreErrors*/ true, /*dontResolveAlias*/ false, node);
24429+
if (!factorySymbol) {
24430+
return true;
24431+
}
24432+
24433+
const factoryType = getTypeOfSymbol(factorySymbol);
24434+
const callSignatures = getSignaturesOfType(factoryType, SignatureKind.Call);
24435+
if (!length(callSignatures)) {
24436+
return true;
24437+
}
24438+
24439+
let hasFirstParamSignatures = false;
24440+
let maxParamCount = 0;
24441+
// Check that _some_ first parameter expects a FC-like thing, and that some overload of the SFC expects an acceptable number of arguments
24442+
for (const sig of callSignatures) {
24443+
const firstparam = getTypeAtPosition(sig, 0);
24444+
const signaturesOfParam = getSignaturesOfType(firstparam, SignatureKind.Call);
24445+
if (!length(signaturesOfParam)) continue;
24446+
for (const paramSig of signaturesOfParam) {
24447+
hasFirstParamSignatures = true;
24448+
if (hasEffectiveRestParameter(paramSig)) {
24449+
return true; // some signature has a rest param, so function components can have an arbitrary number of arguments
24450+
}
24451+
const paramCount = getParameterCount(paramSig);
24452+
if (paramCount > maxParamCount) {
24453+
maxParamCount = paramCount;
24454+
}
24455+
}
24456+
}
24457+
if (!hasFirstParamSignatures) {
24458+
// Not a single signature had a first parameter which expected a signature - for back compat, and
24459+
// to guard against generic factories which won't have signatures directly, do not error
24460+
return true;
24461+
}
24462+
let absoluteMinArgCount = Infinity;
24463+
for (const tagSig of tagCallSignatures) {
24464+
const tagRequiredArgCount = getMinArgumentCount(tagSig);
24465+
if (tagRequiredArgCount < absoluteMinArgCount) {
24466+
absoluteMinArgCount = tagRequiredArgCount;
24467+
}
24468+
}
24469+
if (absoluteMinArgCount <= maxParamCount) {
24470+
return true; // some signature accepts the number of arguments the function component provides
24471+
}
24472+
24473+
if (reportErrors) {
24474+
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);
24475+
const tagNameDeclaration = getSymbolAtLocation(node.tagName)?.valueDeclaration;
24476+
if (tagNameDeclaration) {
24477+
addRelatedInfo(diag, createDiagnosticForNode(tagNameDeclaration, Diagnostics._0_is_declared_here, entityNameToString(node.tagName)));
24478+
}
24479+
if (errorOutputContainer && errorOutputContainer.skipLogging) {
24480+
(errorOutputContainer.errors || (errorOutputContainer.errors = [])).push(diag);
24481+
}
24482+
if (!errorOutputContainer.skipLogging) {
24483+
diagnostics.add(diag);
24484+
}
24485+
}
24486+
return false;
24487+
}
2440324488
}
2440424489

2440524490
function getSignatureApplicabilityError(
@@ -35282,6 +35367,10 @@ namespace ts {
3528235367
return literalTypeToNode(<FreshableType>type, node, tracker);
3528335368
}
3528435369

35370+
function getJsxFactoryEntity(location: Node) {
35371+
return location ? (getJsxNamespace(location), (getSourceFileOfNode(location).localJsxFactory || _jsxFactoryEntity)) : _jsxFactoryEntity;
35372+
}
35373+
3528535374
function createResolver(): EmitResolver {
3528635375
// this variable and functions that use it are deliberately moved here from the outer scope
3528735376
// to avoid scope pollution
@@ -35353,7 +35442,7 @@ namespace ts {
3535335442
const symbol = node && getSymbolOfNode(node);
3535435443
return !!(symbol && getCheckFlags(symbol) & CheckFlags.Late);
3535535444
},
35356-
getJsxFactoryEntity: location => location ? (getJsxNamespace(location), (getSourceFileOfNode(location).localJsxFactory || _jsxFactoryEntity)) : _jsxFactoryEntity,
35445+
getJsxFactoryEntity,
3535735446
getAllAccessorDeclarations(accessor: AccessorDeclaration): AllAccessorDeclarations {
3535835447
accessor = getParseTreeNode(accessor, isGetOrSetAccessorDeclaration)!; // TODO: GH#18217
3535935448
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)