Skip to content

Commit ac4b1be

Browse files
sonar-nigel[bot]Vibe Botclaude
authored
JS-1387 Fix FP in S6767: props spread into JSX elements reported as unused (#6565)
Co-authored-by: Vibe Bot <vibe-bot@sonarsource.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 41f32bb commit ac4b1be

3 files changed

Lines changed: 70 additions & 4 deletions

File tree

its/ruling/src/test/expected/eigen/typescript-S6767.json

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,6 @@
6767
"eigen:src/app/Scenes/Artwork/Components/CommercialButtons/BidButton.tsx": [
6868
20
6969
],
70-
"eigen:src/app/Scenes/Artwork/Components/CommercialInformation.tsx": [
71-
34,
72-
37
73-
],
7470
"eigen:src/app/Scenes/Artwork/Components/FollowArtistLink.tsx": [
7571
11
7672
],

packages/jsts/src/rules/S6767/decorator.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import type { Rule, SourceCode } from 'eslint';
2020
import type estree from 'estree';
21+
import type { JSXSpreadAttribute } from 'estree-jsx';
2122
import { childrenOf } from '../helpers/ancestor.js';
2223
import { isIdentifier } from '../helpers/ast.js';
2324
import { interceptReportForReact } from '../helpers/decorators/interceptor.js';
@@ -70,6 +71,14 @@ function hasPropsCall(root: estree.Node, keys: SourceCode.VisitorKeys): boolean
7071
return true;
7172
}
7273

74+
// Check if this is a JSXSpreadAttribute with props (for {...props} or {...this.props} in JSX elements)
75+
if (
76+
root.type === 'JSXSpreadAttribute' &&
77+
propsArgPatterns.some(p => p((root as unknown as JSXSpreadAttribute).argument))
78+
) {
79+
return true;
80+
}
81+
7382
// Recursively check all children
7483
return childrenOf(root, keys).some(child => hasPropsCall(child, keys));
7584
}

packages/jsts/src/rules/S6767/unit.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,41 @@ class Button extends React.Component {
129129
});
130130
});
131131

132+
it('should not report props when spread into a JSX element via JSXSpreadAttribute', () => {
133+
const ruleTester = new NoTypeCheckingRuleTester();
134+
135+
ruleTester.run('no-unused-prop-types', rule, {
136+
valid: [
137+
{
138+
// FP: props forwarded via JSX spread; second object-literal spread resets upstream flag
139+
code: `
140+
function Wrapper(props) {
141+
return <div {...props} {...{extra: 'extra-value'}} />;
142+
}
143+
Wrapper.propTypes = {
144+
onClick: PropTypes.func,
145+
};
146+
`,
147+
},
148+
{
149+
// FP: class component this.props forwarded via JSX spread; second object-literal spread resets upstream flag
150+
code: `
151+
class MyComponent extends React.Component {
152+
render() {
153+
return <Child {...this.props} {...{extra: 'extra-value'}} />;
154+
}
155+
}
156+
MyComponent.propTypes = {
157+
onClick: PropTypes.func,
158+
label: PropTypes.string,
159+
};
160+
`,
161+
},
162+
],
163+
invalid: [],
164+
});
165+
});
166+
132167
it('should exercise TypeScript type-checking paths (Strategy C in react helpers)', () => {
133168
const ruleTester = new RuleTester({
134169
parserOptions: {
@@ -289,6 +324,32 @@ Button.propTypes = { color: PropTypes.string };
289324
});
290325
});
291326

327+
it('upstream rule should report JSX spread FP pattern (sentinel: remove decorator if this fails)', () => {
328+
// Confirms that the upstream eslint-plugin-react no-unused-prop-types rule DOES raise
329+
// issues when {...props} or {...this.props} in JSX is followed by an object-literal
330+
// spread that resets the upstream ignoreUnusedPropTypesValidation flag.
331+
// If this test starts failing, the upstream rule has been fixed and the JSXSpreadAttribute
332+
// handling in the S6767 decorator can be removed.
333+
const upstreamRule = rules['no-unused-prop-types'];
334+
const ruleTester = new NoTypeCheckingRuleTester();
335+
336+
ruleTester.run('no-unused-prop-types (upstream, JSX spread)', upstreamRule, {
337+
valid: [],
338+
invalid: [
339+
{
340+
// upstream resets ignoreUnusedPropTypesValidation when an object-literal spread follows the props spread
341+
code: `
342+
function Wrapper(props) {
343+
return <div {...props} {...{extra: 'extra-value'}} />;
344+
}
345+
Wrapper.propTypes = { onClick: PropTypes.func };
346+
`,
347+
errors: 1,
348+
},
349+
],
350+
});
351+
});
352+
292353
it('upstream rule should NOT report state interface properties when TypeScript type info is available', () => {
293354
// Confirms that the upstream rule uses TypeScript type information to distinguish
294355
// state types from props types. AnchorState (used as the second type parameter of

0 commit comments

Comments
 (0)