Skip to content
Open
80 changes: 77 additions & 3 deletions packages/jsts/src/rules/S6767/decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
*/
// https://sonarsource.github.io/rspec/#/rspec/S6767/javascript

import type { Rule, SourceCode } from 'eslint';
import type { Rule, Scope, SourceCode } from 'eslint';
import type estree from 'estree';
import type { JSXSpreadAttribute } from 'estree-jsx';
import { childrenOf } from '../helpers/ancestor.js';
import { isIdentifier } from '../helpers/ast.js';
import { childrenOf, getNodeParent } from '../helpers/ancestor.js';
import { isFunctionNode, isIdentifier } from '../helpers/ast.js';
import { interceptReportForReact } from '../helpers/decorators/interceptor.js';
import { generateMeta } from '../helpers/generate-meta.js';
import { findComponentNode } from '../helpers/react.js';
Expand All @@ -35,6 +35,15 @@ const propsArgPatterns: Array<(arg: estree.Node) => boolean> = [
isIdentifier(arg.property, 'props'),
];

/** Composable callee checkers for forwardRef call detection. */
const forwardRefCalleePatterns: Array<(callee: estree.Expression | estree.Super) => boolean> = [
callee => isIdentifier(callee, 'forwardRef'),
callee =>
callee.type === 'MemberExpression' &&
isIdentifier(callee.object, 'React') &&
isIdentifier(callee.property, 'forwardRef'),
];

export function decorate(rule: Rule.RuleModule): Rule.RuleModule {
return interceptReportForReact(
{ ...rule, meta: generateMeta(meta, rule.meta) },
Expand All @@ -44,11 +53,76 @@ export function decorate(rule: Rule.RuleModule): Rule.RuleModule {
if (componentNode && hasPropsCall(componentNode, context.sourceCode.visitorKeys)) {
return;
}
// Suppress FP only when the specific reported prop is referenced inside a forwardRef callback.
const { data } = descriptor as { data?: Record<string, string> };
const propName = data?.name;
if (
propName &&
componentNode &&
hasPropMemberReference(context.sourceCode, componentNode, propName)
) {
return;
}
context.report(descriptor);
},
);
}

/**
* Returns true only when `propName` is referenced through the component's actual
* first parameter binding and the matching member access sits inside a forwardRef callback.
*/
function hasPropMemberReference(
sourceCode: SourceCode,
componentNode: estree.Node,
propName: string,
): boolean {
if (!isFunctionNode(componentNode)) {
return false;
}

const propsParam = componentNode.params[0];
if (!isIdentifier(propsParam)) {
return false;
}

const variable = sourceCode.getScope(componentNode).set.get(propsParam.name);
if (!variable) {
return false;
}

return variable.references.some(reference =>
isPropReferenceInForwardRefCallback(reference, propName),
);
}

function isPropReferenceInForwardRefCallback(
reference: Scope.Reference,
propName: string,
): boolean {
const memberExpression = getNodeParent(reference.identifier);
if (
memberExpression?.type !== 'MemberExpression' ||
memberExpression.object !== reference.identifier ||
memberExpression.computed ||
!isIdentifier(memberExpression.property, propName)
) {
return false;
}

let current: estree.Node | undefined = memberExpression;
while (current) {
if (current.type === 'CallExpression') {
const call = current;
if (forwardRefCalleePatterns.some(pattern => pattern(call.callee))) {
return true;
}
}
current = getNodeParent(current);
}
return false;
}

function hasPropsCall(root: estree.Node, keys: SourceCode.VisitorKeys): boolean {
if (!root) {
return false;
Expand Down
157 changes: 157 additions & 0 deletions packages/jsts/src/rules/S6767/unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,96 @@ VictoryAxis.propTypes = {
});
});

it('should not report props accessed via closure inside a forwardRef callback', () => {
const ruleTester = new NoTypeCheckingRuleTester();

ruleTester.run('no-unused-prop-types', rule, {
valid: [
{
// FP: React.forwardRef closure — props used inside the callback via closure
code: `
function Wrapper(props) {
const ForwardedInput = React.forwardRef((_, ref) => (
<label>{props.label}<input ref={ref} /></label>
));
return <ForwardedInput />;
}
Wrapper.propTypes = {
label: PropTypes.string,
};
`,
},
{
// FP: bare forwardRef closure — props used inside the callback via closure
code: `
function Wrapper(props) {
const ForwardedButton = forwardRef((_, ref) => (
<button ref={ref}>{props.label}</button>
));
return <ForwardedButton />;
}
Wrapper.propTypes = {
label: PropTypes.string,
};
`,
},
{
// FP: renamed props parameter is still recognized inside the forwardRef closure
code: `
function Wrapper(ownProps) {
const ForwardedButton = forwardRef((_, ref) => (
<button ref={ref}>{ownProps.label}</button>
));
return <ForwardedButton />;
}
Wrapper.propTypes = {
label: PropTypes.string,
};
`,
},
],
invalid: [
{
// TP: suppression is component-scoped; sibling component's unused prop is still reported
code: `
function WrapperWithForwardRef(props) {
const FwdComp = React.forwardRef((_, ref) => <div ref={ref}>{props.title}</div>);
return <FwdComp />;
}
WrapperWithForwardRef.propTypes = {
title: PropTypes.string,
};
function Button(props) {
return <button>{props.label}</button>;
}
Button.propTypes = {
label: PropTypes.string,
color: PropTypes.string,
};
`,
errors: 1,
},
{
// TP: forwardRef present, 'label' is referenced inside the callback (suppressed),
// but 'color' is not — it must still be reported
code: `
function Wrapper(props) {
const ForwardedInput = React.forwardRef((_, ref) => (
<label>{props.label}<input ref={ref} /></label>
));
return <ForwardedInput />;
}
Wrapper.propTypes = {
label: PropTypes.string,
color: PropTypes.string,
};
`,
errors: 1,
},
],
});
});

it('should exercise TypeScript type-checking paths (Strategy C in react helpers)', () => {
const ruleTester = new RuleTester({
parserOptions: {
Expand Down Expand Up @@ -281,6 +371,43 @@ class VictoryAxis extends React.Component<VictoryAxisProps> {
return <div>{this.animate()}</div>;
}
}
`,
filename: fixtureFile,
},
{
// FP: TypeScript function component with React.forwardRef closure — containsForwardRefCall
// suppresses the report when the component subtree contains a React.forwardRef call.
code: `
declare const React: any;
interface InputProps {
label: string;
disabled?: boolean;
}
function InputWrapper(props: InputProps) {
const ForwardedInput = React.forwardRef((_: any, ref: any) => (
<label>
{props.label}
<input ref={ref} disabled={props.disabled} />
</label>
));
return <ForwardedInput />;
}
`,
filename: fixtureFile,
},
{
// FP: TypeScript function component with renamed props parameter in React.forwardRef closure.
code: `
declare const React: any;
interface InputProps {
label: string;
}
function InputWrapper(ownProps: InputProps) {
const ForwardedInput = React.forwardRef((_: any, ref: any) => (
<label ref={ref}>{ownProps.label}</label>
));
return <ForwardedInput />;
}
`,
filename: fixtureFile,
},
Expand Down Expand Up @@ -439,6 +566,36 @@ AnimationComponent.propTypes = {
});
});

it('upstream rule should report forwardRef closure FP pattern (sentinel: remove decorator check if this fails)', () => {
// Confirms that the upstream eslint-plugin-react no-unused-prop-types rule DOES raise
// issues when props are accessed via closure inside a forwardRef callback.
// If this test starts failing, the upstream rule has been fixed and the forwardRef
// handling in the S6767 decorator can be removed.
const upstreamRule = rules['no-unused-prop-types'];
const ruleTester = new NoTypeCheckingRuleTester();

ruleTester.run('no-unused-prop-types (upstream, forwardRef)', upstreamRule, {
valid: [],
invalid: [
{
// upstream does not credit outer component when props are accessed via closure in forwardRef
code: `
function Wrapper(props) {
const ForwardedInput = React.forwardRef((_, ref) => (
<label>{props.label}</label>
));
return <ForwardedInput />;
}
Wrapper.propTypes = {
label: PropTypes.string,
};
`,
errors: 1,
},
],
});
});

it('upstream rule should NOT report state interface properties when TypeScript type info is available', () => {
// Confirms that the upstream rule uses TypeScript type information to distinguish
// state types from props types. AnchorState (used as the second type parameter of
Expand Down
Loading