Skip to content

Commit 299d487

Browse files
committed
Baseline arity checks for jsx sfc tags
Finish comment PR feedback
1 parent 68cbe9e commit 299d487

8 files changed

+363
-7
lines changed

src/compiler/checker.ts

+94-5
Original file line numberDiff line numberDiff line change
@@ -939,6 +939,7 @@ namespace ts {
939939
if (jsxPragma) {
940940
const chosenpragma = isArray(jsxPragma) ? jsxPragma[0] : jsxPragma;
941941
file.localJsxFactory = parseIsolatedEntityName(chosenpragma.arguments.factory, languageVersion);
942+
visitNode(file.localJsxFactory, markAsSynthetic);
942943
if (file.localJsxFactory) {
943944
return file.localJsxNamespace = getFirstIdentifier(file.localJsxFactory).escapedText;
944945
}
@@ -949,6 +950,7 @@ namespace ts {
949950
_jsxNamespace = "React" as __String;
950951
if (compilerOptions.jsxFactory) {
951952
_jsxFactoryEntity = parseIsolatedEntityName(compilerOptions.jsxFactory, languageVersion);
953+
visitNode(_jsxFactoryEntity, markAsSynthetic);
952954
if (_jsxFactoryEntity) {
953955
_jsxNamespace = getFirstIdentifier(_jsxFactoryEntity).escapedText;
954956
}
@@ -957,7 +959,16 @@ namespace ts {
957959
_jsxNamespace = escapeLeadingUnderscores(compilerOptions.reactNamespace);
958960
}
959961
}
962+
if (!_jsxFactoryEntity) {
963+
_jsxFactoryEntity = createQualifiedName(createIdentifier(unescapeLeadingUnderscores(_jsxNamespace)), "createElement");
964+
}
960965
return _jsxNamespace;
966+
967+
function markAsSynthetic(node: Node): VisitResult<Node> {
968+
node.pos = -1;
969+
node.end = -1;
970+
return visitEachChild(node, markAsSynthetic, nullTransformationContext);
971+
}
961972
}
962973

963974
function getEmitResolver(sourceFile: SourceFile, cancellationToken: CancellationToken) {
@@ -2801,8 +2812,8 @@ namespace ts {
28012812
const namespaceMeaning = SymbolFlags.Namespace | (isInJSFile(name) ? meaning & SymbolFlags.Value : 0);
28022813
let symbol: Symbol | undefined;
28032814
if (name.kind === SyntaxKind.Identifier) {
2804-
const message = meaning === namespaceMeaning ? Diagnostics.Cannot_find_namespace_0 : getCannotFindNameDiagnosticForName(getFirstIdentifier(name));
2805-
const symbolFromJSPrototype = isInJSFile(name) ? resolveEntityNameFromAssignmentDeclaration(name, meaning) : undefined;
2815+
const message = meaning === namespaceMeaning || nodeIsSynthesized(name) ? Diagnostics.Cannot_find_namespace_0 : getCannotFindNameDiagnosticForName(getFirstIdentifier(name));
2816+
const symbolFromJSPrototype = isInJSFile(name) && !nodeIsSynthesized(name) ? resolveEntityNameFromAssignmentDeclaration(name, meaning) : undefined;
28062817
symbol = resolveName(location || name, name.escapedText, meaning, ignoreErrors || symbolFromJSPrototype ? undefined : message, name, /*isUse*/ true);
28072818
if (!symbol) {
28082819
return symbolFromJSPrototype;
@@ -2845,7 +2856,7 @@ namespace ts {
28452856
throw Debug.assertNever(name, "Unknown entity name kind.");
28462857
}
28472858
Debug.assert((getCheckFlags(symbol) & CheckFlags.Instantiated) === 0, "Should never get an instantiated symbol here.");
2848-
if (isEntityName(name) && (symbol.flags & SymbolFlags.Alias || name.parent.kind === SyntaxKind.ExportAssignment)) {
2859+
if (!nodeIsSynthesized(name) && isEntityName(name) && (symbol.flags & SymbolFlags.Alias || name.parent.kind === SyntaxKind.ExportAssignment)) {
28492860
markSymbolOfAliasDeclarationIfTypeOnly(getAliasDeclarationFromName(name), symbol, /*finalTarget*/ undefined, /*overwriteEmpty*/ true);
28502861
}
28512862
return (symbol.flags & meaning) || dontResolveAlias ? symbol : resolveAlias(symbol);
@@ -24298,7 +24309,7 @@ namespace ts {
2429824309
// can be specified by users through attributes property.
2429924310
const paramType = getEffectiveFirstArgumentForJsxSignature(signature, node);
2430024311
const attributesType = checkExpressionWithContextualType(node.attributes, paramType, /*inferenceContext*/ undefined, checkMode);
24301-
return checkTypeRelatedToAndOptionallyElaborate(
24312+
return checkTagNameDoesNotExpectTooManyArguments() && checkTypeRelatedToAndOptionallyElaborate(
2430224313
attributesType,
2430324314
paramType,
2430424315
relation,
@@ -24307,6 +24318,80 @@ namespace ts {
2430724318
/*headMessage*/ undefined,
2430824319
containingMessageChain,
2430924320
errorOutputContainer);
24321+
24322+
function checkTagNameDoesNotExpectTooManyArguments(): boolean {
24323+
const tagType = isJsxOpeningElement(node) || isJsxSelfClosingElement(node) && !isJsxIntrinsicIdentifier(node.tagName) ? checkExpression(node.tagName) : undefined;
24324+
if (!tagType) {
24325+
return true;
24326+
}
24327+
const tagCallSignatures = getSignaturesOfType(tagType, SignatureKind.Call);
24328+
if (!length(tagCallSignatures)) {
24329+
return true;
24330+
}
24331+
const factory = getJsxFactoryEntity(node);
24332+
if (!factory) {
24333+
return true;
24334+
}
24335+
const factorySymbol = resolveEntityName(factory, SymbolFlags.Value, /*ignoreErrors*/ true, /*dontResolveAlias*/ false, node);
24336+
if (!factorySymbol) {
24337+
return true;
24338+
}
24339+
24340+
const factoryType = getTypeOfSymbol(factorySymbol);
24341+
const callSignatures = getSignaturesOfType(factoryType, SignatureKind.Call);
24342+
if (!length(callSignatures)) {
24343+
return true;
24344+
}
24345+
24346+
let hasFirstParamSignatures = false;
24347+
let maxParamCount = 0;
24348+
// Check that _some_ first parameter expects a FC-like thing, and that some overload of the SFC expects an acceptable number of arguments
24349+
for (const sig of callSignatures) {
24350+
const firstparam = getTypeAtPosition(sig, 0);
24351+
const signaturesOfParam = getSignaturesOfType(firstparam, SignatureKind.Call);
24352+
if (!length(signaturesOfParam)) continue;
24353+
for (const paramSig of signaturesOfParam) {
24354+
hasFirstParamSignatures = true;
24355+
if (hasEffectiveRestParameter(paramSig)) {
24356+
return true; // some signature has a rest param, so function components can have an arbitrary number of arguments
24357+
}
24358+
const paramCount = getParameterCount(paramSig);
24359+
if (paramCount > maxParamCount) {
24360+
maxParamCount = paramCount;
24361+
}
24362+
}
24363+
}
24364+
if (!hasFirstParamSignatures) {
24365+
// Not a single signature had a first parameter which expected a signature - for back compat, and
24366+
// to guard against generic factories which won't have signatures directly, do not error
24367+
return true;
24368+
}
24369+
let absoluteMinArgCount = Infinity;
24370+
for (const tagSig of tagCallSignatures) {
24371+
const tagRequiredArgCount = getMinArgumentCount(tagSig);
24372+
if (tagRequiredArgCount < absoluteMinArgCount) {
24373+
absoluteMinArgCount = tagRequiredArgCount;
24374+
}
24375+
}
24376+
if (absoluteMinArgCount <= maxParamCount) {
24377+
return true; // some signature accepts the number of arguments the function component provides
24378+
}
24379+
24380+
if (reportErrors) {
24381+
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);
24382+
const tagNameDeclaration = getSymbolAtLocation(node.tagName)?.valueDeclaration;
24383+
if (tagNameDeclaration) {
24384+
addRelatedInfo(diag, createDiagnosticForNode(tagNameDeclaration, Diagnostics._0_is_declared_here, entityNameToString(node.tagName)));
24385+
}
24386+
if (errorOutputContainer && errorOutputContainer.skipLogging) {
24387+
(errorOutputContainer.errors || (errorOutputContainer.errors = [])).push(diag);
24388+
}
24389+
if (!errorOutputContainer.skipLogging) {
24390+
diagnostics.add(diag);
24391+
}
24392+
}
24393+
return false;
24394+
}
2431024395
}
2431124396

2431224397
function getSignatureApplicabilityError(
@@ -35175,6 +35260,10 @@ namespace ts {
3517535260
return literalTypeToNode(<FreshableType>type, node, tracker);
3517635261
}
3517735262

35263+
function getJsxFactoryEntity(location: Node) {
35264+
return location ? (getJsxNamespace(location), (getSourceFileOfNode(location).localJsxFactory || _jsxFactoryEntity)) : _jsxFactoryEntity;
35265+
}
35266+
3517835267
function createResolver(): EmitResolver {
3517935268
// this variable and functions that use it are deliberately moved here from the outer scope
3518035269
// to avoid scope pollution
@@ -35246,7 +35335,7 @@ namespace ts {
3524635335
const symbol = node && getSymbolOfNode(node);
3524735336
return !!(symbol && getCheckFlags(symbol) & CheckFlags.Late);
3524835337
},
35249-
getJsxFactoryEntity: location => location ? (getJsxNamespace(location), (getSourceFileOfNode(location).localJsxFactory || _jsxFactoryEntity)) : _jsxFactoryEntity,
35338+
getJsxFactoryEntity,
3525035339
getAllAccessorDeclarations(accessor: AccessorDeclaration): AllAccessorDeclarations {
3525135340
accessor = getParseTreeNode(accessor, isGetOrSetAccessorDeclaration)!; // TODO: GH#18217
3525235341
const otherKind = accessor.kind === SyntaxKind.SetAccessor ? SyntaxKind.GetAccessor : SyntaxKind.SetAccessor;

src/compiler/diagnosticMessages.json

+4
Original file line numberDiff line numberDiff line change
@@ -4280,6 +4280,10 @@
42804280
"category": "Message",
42814281
"code": 6228
42824282
},
4283+
"Tag '{0}' expects at least '{1}' arguments, but the JSX factory '{2}' provides at most '{3}'.": {
4284+
"category": "Error",
4285+
"code": 6229
4286+
},
42834287

42844288
"Projects to reference": {
42854289
"category": "Message",

src/compiler/utilities.ts

+5-2
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 {
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
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)