Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Baseline arity checks for jsx sfc tags #36643

Merged
merged 1 commit into from
Feb 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 94 additions & 5 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,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 @@ -950,6 +951,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 @@ -958,7 +960,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 @@ -2802,8 +2813,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));
weswigham marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -2846,7 +2857,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 @@ -24389,7 +24400,7 @@ namespace ts {
// can be specified by users through attributes property.
const paramType = getEffectiveFirstArgumentForJsxSignature(signature, node);
const attributesType = checkExpressionWithContextualType(node.attributes, paramType, /*inferenceContext*/ undefined, checkMode);
return checkTypeRelatedToAndOptionallyElaborate(
return checkTagNameDoesNotExpectTooManyArguments() && checkTypeRelatedToAndOptionallyElaborate(
attributesType,
paramType,
relation,
Expand All @@ -24398,6 +24409,80 @@ 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;
let maxParamCount = 0;
// Check that _some_ first parameter expects a FC-like thing, and that some overload of the SFC expects an acceptable number of arguments
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 arbitrary number of arguments
}
const paramCount = getParameterCount(paramSig);
if (paramCount > maxParamCount) {
maxParamCount = paramCount;
}
}
}
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, do not error
return true;
}
let absoluteMinArgCount = Infinity;
for (const tagSig of tagCallSignatures) {
const tagRequiredArgCount = getMinArgumentCount(tagSig);
if (tagRequiredArgCount < absoluteMinArgCount) {
absoluteMinArgCount = tagRequiredArgCount;
}
}
if (absoluteMinArgCount <= maxParamCount) {
return true; // some signature accepts the number of arguments the function component provides
}

if (reportErrors) {
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);
const tagNameDeclaration = getSymbolAtLocation(node.tagName)?.valueDeclaration;
if (tagNameDeclaration) {
addRelatedInfo(diag, createDiagnosticForNode(tagNameDeclaration, Diagnostics._0_is_declared_here, entityNameToString(node.tagName)));
}
if (errorOutputContainer && errorOutputContainer.skipLogging) {
(errorOutputContainer.errors || (errorOutputContainer.errors = [])).push(diag);
}
if (!errorOutputContainer.skipLogging) {
diagnostics.add(diag);
}
}
return false;
}
}

function getSignatureApplicabilityError(
Expand Down Expand Up @@ -35276,6 +35361,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 @@ -35347,7 +35436,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 @@ -4292,6 +4292,10 @@
"category": "Message",
"code": 6228
},
"Tag '{0}' expects at least '{1}' arguments, but the JSX factory '{2}' provides at most '{3}'.": {
"category": "Error",
"code": 6229
},

"Projects to reference": {
"category": "Message",
Expand Down
7 changes: 5 additions & 2 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -864,14 +864,17 @@ namespace ts {
}
}

export function entityNameToString(name: EntityNameOrEntityNameExpression): string {
export function entityNameToString(name: EntityNameOrEntityNameExpression | JsxTagNameExpression | PrivateIdentifier): string {
switch (name.kind) {
case SyntaxKind.ThisKeyword:
return "this";
case SyntaxKind.PrivateIdentifier:
case SyntaxKind.Identifier:
return getFullWidth(name) === 0 ? idText(name) : getTextOfNode(name);
case SyntaxKind.QualifiedName:
return entityNameToString(name.left) + "." + entityNameToString(name.right);
case SyntaxKind.PropertyAccessExpression:
if (isIdentifier(name.name)) {
if (isIdentifier(name.name) || isPrivateIdentifier(name.name)) {
return entityNameToString(name.expression) + "." + entityNameToString(name.name);
}
else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
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'.
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'.


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

import * as React from "react";

interface MyProps {
x: number;
}

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

const a = <MyComp4 x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
~~~~~~~
!!! error TS6229: Tag 'MyComp4' expects at least '4' arguments, but the JSX factory 'React.createElement' provides at most '2'.
!!! related TS2728 tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx:9:10: 'MyComp4' is declared here.
const b = <MyComp3 x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
~~~~~~~
!!! error TS6229: Tag 'MyComp3' expects at least '3' arguments, but the JSX factory 'React.createElement' provides at most '2'.
!!! related TS2728 tests/cases/compiler/jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx:12:10: 'MyComp3' is declared here.
const c = <MyComp2 x={2}/>; // Should be OK, `context` is allowed, per react rules

declare function MyTagWithOptionalNonJSXBits(props: MyProps, context: any, nonReactArg?: string): JSX.Element;
const d = <MyTagWithOptionalNonJSXBits x={2} />; // Technically OK, but probably questionable
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//// [jsxIssuesErrorWhenTagExpectsTooManyArguments.tsx]
/// <reference path="/.lib/react16.d.ts" />

import * as React from "react";

interface MyProps {
x: number;
}

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

const a = <MyComp4 x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
const b = <MyComp3 x={2}/>; // using `MyComp` as a component should error - it expects more arguments than react provides
const c = <MyComp2 x={2}/>; // Should be OK, `context` is allowed, per react rules

declare function MyTagWithOptionalNonJSXBits(props: MyProps, context: any, nonReactArg?: string): JSX.Element;
const d = <MyTagWithOptionalNonJSXBits x={2} />; // Technically OK, but probably questionable

//// [jsxIssuesErrorWhenTagExpectsTooManyArguments.js]
"use strict";
/// <reference path="react16.d.ts" />
exports.__esModule = true;
var React = require("react");
function MyComp4(props, context, bad, verybad) {
return React.createElement("div", null);
}
function MyComp3(props, context, bad) {
return React.createElement("div", null);
}
function MyComp2(props, context) {
return React.createElement("div", null);
}
var a = React.createElement(MyComp4, { x: 2 }); // using `MyComp` as a component should error - it expects more arguments than react provides
var b = React.createElement(MyComp3, { x: 2 }); // using `MyComp` as a component should error - it expects more arguments than react provides
var c = React.createElement(MyComp2, { x: 2 }); // Should be OK, `context` is allowed, per react rules
var d = React.createElement(MyTagWithOptionalNonJSXBits, { x: 2 }); // Technically OK, but probably questionable
Loading