Skip to content

Commit d5ed937

Browse files
authored
fix(plugins/x): fix 'no-unstable-rules' false positives (#896)
1 parent 12f2754 commit d5ed937

File tree

4 files changed

+109
-39
lines changed

4 files changed

+109
-39
lines changed

packages/plugins/eslint-plugin-react-x/src/rules/no-unstable-context-value.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as AST from "@eslint-react/ast";
2-
import { useComponentCollector } from "@eslint-react/core";
2+
import { isReactHookCall, useComponentCollector } from "@eslint-react/core";
33
import { F, O } from "@eslint-react/eff";
44
import type { RuleFeature } from "@eslint-react/types";
55
import * as VAR from "@eslint-react/var";
@@ -65,9 +65,10 @@ export default createRule<[], MessageID>({
6565
const initialScope = context.sourceCode.getScope(valueExpression);
6666
return O.some(VAR.inspectConstruction(valueExpression, initialScope));
6767
}),
68-
O.filter(({ construction }) => construction._tag !== "None"),
6968
);
7069
for (const { construction, function: [_, fNode] } of O.toArray(constructionEntry)) {
70+
if (construction._tag === "None") continue;
71+
if (isReactHookCall(construction.node)) continue;
7172
constructions.set(fNode, [
7273
...constructions.get(fNode) ?? [],
7374
construction,

packages/plugins/eslint-plugin-react-x/src/rules/no-unstable-default-props.spec.ts

+75-3
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ ruleTester.run(RULE_NAME, rule, {
133133
g = new Thing(),
134134
h = <Thing />,
135135
i = Symbol('foo'),
136-
j = unknownFunction()
136+
j = unknownFunction(),
137+
k = window.name
137138
}) {
138139
return null
139140
}
@@ -152,7 +153,8 @@ ruleTester.run(RULE_NAME, rule, {
152153
g = new Thing(),
153154
h = <Thing />,
154155
i = Symbol('foo'),
155-
j = unknownFunction()
156+
j = unknownFunction(),
157+
k = window.name
156158
}) => {
157159
return null
158160
}
@@ -212,6 +214,76 @@ ruleTester.run(RULE_NAME, rule, {
212214
return <div>{items}</div>;
213215
}
214216
`,
215-
/* tsx */ `export default function NonComponent({ foo = {} }) {}`,
217+
/* tsx */ `
218+
export default function NonComponent({ foo = {} }) {}
219+
`,
220+
/* tsx */ `
221+
export function DrawerItem(props: Props) {
222+
const { colors, fonts } = useTheme();
223+
224+
const {
225+
href,
226+
icon,
227+
label,
228+
labelStyle,
229+
focused = false,
230+
allowFontScaling,
231+
activeTintColor = colors.primary,
232+
inactiveBackgroundColor = 'transparent',
233+
style,
234+
onPress,
235+
pressColor,
236+
pressOpacity = 1,
237+
testID,
238+
accessibilityLabel,
239+
...rest
240+
} = props;
241+
242+
const { borderRadius = 56 } = StyleSheet.flatten(style || {});
243+
const color = focused ? activeTintColor : inactiveTintColor;
244+
const backgroundColor = focused
245+
? activeBackgroundColor
246+
: inactiveBackgroundColor;
247+
248+
const iconNode = icon ? icon({ size: 24, focused, color }) : null;
249+
250+
return (
251+
<View
252+
collapsable={false}
253+
{...rest}
254+
style={[styles.container, { borderRadius, backgroundColor }, style]}
255+
>
256+
<PlatformPressable
257+
testID={testID}
258+
onPress={onPress}
259+
accessibilityLabel={accessibilityLabel}
260+
accessibilityRole="button"
261+
accessibilityState={{ selected: focused }}
262+
pressColor={pressColor}
263+
pressOpacity={pressOpacity}
264+
hoverEffect={{ color }}
265+
href={href}
266+
>
267+
<View style={[styles.wrapper, { borderRadius }]}>
268+
{iconNode}
269+
<View style={[styles.label, { marginStart: iconNode ? 12 : 0 }]}>
270+
{typeof label === 'string' ? (
271+
<Text
272+
numberOfLines={1}
273+
allowFontScaling={allowFontScaling}
274+
style={[styles.labelText, { color }, fonts.medium, labelStyle]}
275+
>
276+
{label}
277+
</Text>
278+
) : (
279+
label({ color, focused })
280+
)}
281+
</View>
282+
</View>
283+
</PlatformPressable>
284+
</View>
285+
);
286+
}
287+
`,
216288
],
217289
});

packages/plugins/eslint-plugin-react-x/src/rules/no-unstable-default-props.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as AST from "@eslint-react/ast";
2-
import { useComponentCollector } from "@eslint-react/core";
2+
import { isReactHookCall, useComponentCollector } from "@eslint-react/core";
33
import { O } from "@eslint-react/eff";
44
import type { RuleFeature } from "@eslint-react/types";
55
import * as VAR from "@eslint-react/var";
@@ -71,6 +71,7 @@ export default createRule<[], MessageID>({
7171
VAR.ConstructionHint.StrictCallExpression,
7272
);
7373
if (construction._tag === "None") continue;
74+
if (isReactHookCall(construction.node)) continue;
7475
const forbiddenType = AST.toReadableNodeType(right);
7576
context.report({
7677
messageId: "noUnstableDefaultProps",

packages/utilities/var/src/construction.ts

+29-33
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import * as AST from "@eslint-react/ast";
2-
import { Data, isNullable, isObject, isString, O } from "@eslint-react/eff";
2+
import { Data, F, isNullable, isObject, isString, not, O } from "@eslint-react/eff";
33
import type { Scope } from "@typescript-eslint/scope-manager";
4-
import { DefinitionType } from "@typescript-eslint/scope-manager";
54
import type { TSESTree } from "@typescript-eslint/types";
65
import { AST_NODE_TYPES } from "@typescript-eslint/types";
76
import { match } from "ts-pattern";
87

8+
import { getVariableNode } from "./get-variable-node";
9+
910
export type Construction = Data.TaggedEnum<{
1011
Array: {
1112
node: TSESTree.ArrayExpression;
@@ -47,7 +48,10 @@ export type Construction = Data.TaggedEnum<{
4748
node: TSESTree.NewExpression;
4849
usage: O.Option<TSESTree.Node>;
4950
};
50-
None: {};
51+
None: {
52+
node: TSESTree.Node;
53+
usage: O.Option<TSESTree.Node>;
54+
};
5155
ObjectExpression: {
5256
node: TSESTree.ObjectExpression;
5357
usage: O.Option<TSESTree.Node>;
@@ -95,7 +99,7 @@ export function inspectConstruction(
9599
if (hint & ConstructionHint.StrictCallExpression) {
96100
return Construction.CallExpression({ node, usage: O.none() });
97101
}
98-
return Construction.None();
102+
return Construction.None({ node, usage: O.none() });
99103
})
100104
.when(AST.is(AST_NODE_TYPES.NewExpression), (node) => Construction.NewExpression({ node, usage: O.none() }))
101105
.when(
@@ -108,7 +112,7 @@ export function inspectConstruction(
108112
},
109113
)
110114
.when(AST.is(AST_NODE_TYPES.MemberExpression), (node) => {
111-
if (!("object" in node)) return Construction.None();
115+
if (!("object" in node)) return Construction.None({ node, usage: O.none() });
112116
const object = detect(node.object);
113117
if (object._tag === "None") return object;
114118
return {
@@ -117,7 +121,7 @@ export function inspectConstruction(
117121
} as const satisfies Construction;
118122
})
119123
.when(AST.is(AST_NODE_TYPES.AssignmentExpression), (node) => {
120-
if (!("right" in node)) return Construction.None();
124+
if (!("right" in node)) return Construction.None({ node, usage: O.none() });
121125
const right = detect(node.right);
122126
if (right._tag === "None") return right;
123127
return Construction.AssignmentExpression({
@@ -126,7 +130,7 @@ export function inspectConstruction(
126130
});
127131
})
128132
.when(AST.is(AST_NODE_TYPES.AssignmentPattern), (node) => {
129-
if (!("right" in node)) return Construction.None();
133+
if (!("right" in node)) return Construction.None({ node, usage: O.none() });
130134
const right = detect(node.right);
131135
if (right._tag === "None") return right;
132136
return Construction.AssignmentPattern({
@@ -135,54 +139,46 @@ export function inspectConstruction(
135139
});
136140
})
137141
.when(AST.is(AST_NODE_TYPES.LogicalExpression), (node) => {
138-
if (!("left" in node && "right" in node)) return Construction.None();
142+
if (!("left" in node && "right" in node)) return Construction.None({ node, usage: O.none() });
139143
const left = detect(node.left);
140144
if (left._tag !== "None") return left;
141145
return detect(node.right);
142146
})
143147
.when(AST.is(AST_NODE_TYPES.ConditionalExpression), (node) => {
144148
if (!("consequent" in node && "alternate" in node && !isNullable(node.alternate))) {
145-
return Construction.None();
149+
return Construction.None({ node, usage: O.none() });
146150
}
147151
const consequent = detect(node.consequent);
148-
if (consequent._tag !== "None") return Construction.None();
152+
if (consequent._tag !== "None") return Construction.None({ node, usage: O.none() });
149153
return detect(node.alternate);
150154
})
151155
.when(AST.is(AST_NODE_TYPES.Identifier), (node) => {
152-
if (!("name" in node && isString(node.name))) return Construction.None();
153-
const maybeLatestDef = O.fromNullable(initialScope.set.get(node.name)?.defs.at(-1));
154-
if (O.isNone(maybeLatestDef)) return Construction.None();
155-
const latestDef = maybeLatestDef.value;
156-
if (
157-
latestDef.type !== DefinitionType.Variable
158-
&& latestDef.type !== DefinitionType.FunctionName
159-
) {
160-
return Construction.None();
161-
}
162-
if (latestDef.node.type === AST_NODE_TYPES.FunctionDeclaration) {
163-
return Construction.FunctionDeclaration({
164-
node: latestDef.node,
165-
usage: O.some(node),
166-
});
167-
}
168-
if (!("init" in latestDef.node) || latestDef.node.init === null) {
169-
return Construction.None();
170-
}
171-
return detect(latestDef.node.init);
156+
if (!("name" in node && isString(node.name))) return Construction.None({ node, usage: O.none() });
157+
const construction = F.pipe(
158+
O.fromNullable(initialScope.set.get(node.name)),
159+
O.flatMap(getVariableNode(-1)),
160+
O.map(detect),
161+
O.filter(not(Construction.$is("None"))),
162+
);
163+
if (O.isNone(construction)) return Construction.None({ node, usage: O.none() });
164+
return {
165+
...construction.value,
166+
usage: O.some(node),
167+
} as const;
172168
})
173169
.when(AST.is(AST_NODE_TYPES.Literal), (node) => {
174170
if ("regex" in node) {
175171
return Construction.RegExpLiteral({ node, usage: O.none() });
176172
}
177-
return Construction.None();
173+
return Construction.None({ node, usage: O.none() });
178174
})
179175
.when(AST.isTypeExpression, () => {
180176
if (!("expression" in node) || !isObject(node.expression)) {
181-
return Construction.None();
177+
return Construction.None({ node, usage: O.none() });
182178
}
183179
return detect(node.expression);
184180
})
185-
.otherwise(() => Construction.None());
181+
.otherwise(() => Construction.None({ node, usage: O.none() }));
186182
};
187183
return detect(node);
188184
}

0 commit comments

Comments
 (0)