Skip to content

Commit 4d38484

Browse files
authored
re-implement jsx/no-leaked-conditional-rendering to use type infomation (#91)
1 parent bfb0277 commit 4d38484

File tree

22 files changed

+258
-118
lines changed

22 files changed

+258
-118
lines changed

package.json

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eslint-react/monorepo",
3-
"version": "0.7.3",
3+
"version": "0.7.4",
44
"description": "ESLint plugin for React function components with TypeScript, built (mostly) from scratch.",
55
"keywords": [
66
"eslint",
@@ -68,8 +68,8 @@
6868
"@typescript-eslint/parser": ">=6.10.0",
6969
"@typescript-eslint/rule-tester": "6.10.0",
7070
"@vitest/ui": "0.34.6",
71-
"bun": "1.0.10",
72-
"bun-types": "1.0.10",
71+
"bun": "1.0.11",
72+
"bun-types": "1.0.11",
7373
"cspell": "7.3.9",
7474
"dprint": "0.42.5",
7575
"effect": "2.0.0-next.54",
@@ -95,7 +95,7 @@
9595
"tiny-invariant": "1.3.1",
9696
"ts-pattern": "5.0.5",
9797
"turbo": "1.10.16",
98-
"type-fest": "4.6.0",
98+
"type-fest": "4.7.0",
9999
"typedoc": "0.25.3",
100100
"typedoc-plugin-markdown": "3.17.1",
101101
"typedoc-plugin-mermaid": "1.10.0",

packages/ast/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eslint-react/ast",
3-
"version": "0.7.3",
3+
"version": "0.7.4",
44
"description": "AST Utility Module for Static Analysis of TypeScript",
55
"homepage": "https://github.com/eslint-react/eslint-react",
66
"bugs": {

packages/core/docs/README.md

+18-7
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
- [RE\_HOOK\_NAME](README.md#re_hook_name)
4040
- [SuspenseComponent](README.md#suspensecomponent)
4141
- [SuspenseListComponent](README.md#suspenselistcomponent)
42+
- [defaultComponentCollectorHint](README.md#defaultcomponentcollectorhint)
4243

4344
### Functions
4445

@@ -109,8 +110,10 @@
109110
| `SkipClassProperty` | `bigint` |
110111
| `SkipCreateElement` | `bigint` |
111112
| `SkipMapCall` | `bigint` |
112-
| `SkipNull` | `bigint` |
113+
| `SkipNullLiteral` | `bigint` |
114+
| `SkipNumberLiteral` | `bigint` |
113115
| `SkipObjectMethod` | `bigint` |
116+
| `SkipStringLiteral` | `bigint` |
114117
| `StrictArray` | `bigint` |
115118
| `StrictConditional` | `bigint` |
116119
| `StrictLogical` | `bigint` |
@@ -127,7 +130,9 @@
127130
| :------------------ | :------- |
128131
| `None` | `0n` |
129132
| `SkipCreateElement` | `bigint` |
130-
| `SkipNull` | `bigint` |
133+
| `SkipNullLiteral` | `bigint` |
134+
| `SkipNumberLiteral` | `bigint` |
135+
| `SkipStringLiteral` | `bigint` |
131136
| `StrictArray` | `bigint` |
132137
| `StrictConditional` | `bigint` |
133138
| `StrictLogical` | `bigint` |
@@ -270,6 +275,12 @@
270275

271276
`Const` **SuspenseListComponent**: `19`
272277

278+
---
279+
280+
### defaultComponentCollectorHint
281+
282+
`Const` **defaultComponentCollectorHint**: `bigint`
283+
273284
## Functions
274285

275286
### componentCollector
@@ -278,11 +289,11 @@
278289

279290
#### Parameters
280291

281-
| Name | Type | Default value |
282-
| :-------- | :------------------------------------------------------------- | :---------------------------- |
283-
| `context` | `Readonly`\<`RuleContext`\<`string`, readonly `unknown`[]\>\> | `undefined` |
284-
| `hint` | `bigint` | `ComponentCollectorHint.None` |
285-
| `cache` | [`ComponentCollectorCache`](README.md#componentcollectorcache) | `undefined` |
292+
| Name | Type | Default value |
293+
| :-------- | :------------------------------------------------------------- | :------------------------------ |
294+
| `context` | `Readonly`\<`RuleContext`\<`string`, readonly `unknown`[]\>\> | `undefined` |
295+
| `hint` | `bigint` | `defaultComponentCollectorHint` |
296+
| `cache` | [`ComponentCollectorCache`](README.md#componentcollectorcache) | `undefined` |
286297

287298
#### Returns
288299

packages/core/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eslint-react/core",
3-
"version": "0.7.3",
3+
"version": "0.7.4",
44
"description": "AST Utility Module for Static Analysis of React core API and Patterns.",
55
"homepage": "https://github.com/eslint-react/eslint-react",
66
"bugs": {

packages/core/src/component/component-collector.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,13 @@ export const ComponentCollectorHint = {
5959
} as const;
6060
/* eslint-enable perfectionist/sort-objects */
6161

62+
export const defaultComponentCollectorHint = ComponentCollectorHint.SkipStringLiteral
63+
| ComponentCollectorHint.SkipNumberLiteral;
64+
6265
// TODO: support for detecting component types listed in core/component/component-types.ts
6366
export function componentCollector(
6467
context: RuleContext,
65-
hint: bigint = ComponentCollectorHint.None,
68+
hint: bigint = defaultComponentCollectorHint,
6669
cache: ComponentCollectorCache = new WeakMap(),
6770
) {
6871
const components: TSESTreeFunction[] = [];

packages/core/src/render-prop/is-render-prop.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export function unsafeIsRenderFunction(node: TSESTreeFunction, context: RuleCont
3737
return isJSXValue(
3838
body,
3939
context,
40-
JSXValueCheckHint.SkipNull | JSXValueCheckHint.StrictLogical | JSXValueCheckHint.StrictConditional,
40+
JSXValueCheckHint.SkipNullLiteral | JSXValueCheckHint.StrictLogical | JSXValueCheckHint.StrictConditional,
4141
);
4242
}
4343

packages/eslint-plugin-debug/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eslint-react/eslint-plugin-debug",
3-
"version": "0.7.3",
3+
"version": "0.7.4",
44
"description": "Debug specific rules for @eslint-react/eslint-plugin",
55
"homepage": "https://github.com/eslint-react/eslint-react",
66
"bugs": {

packages/eslint-plugin-hooks/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eslint-react/eslint-plugin-hooks",
3-
"version": "0.7.3",
3+
"version": "0.7.4",
44
"description": "Hooks specific rules for @eslint-react/eslint-plugin",
55
"homepage": "https://github.com/eslint-react/eslint-react",
66
"bugs": {

packages/eslint-plugin-jsx/package.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eslint-react/eslint-plugin-jsx",
3-
"version": "0.7.3",
3+
"version": "0.7.4",
44
"description": "JSX specific rules for @eslint-react/eslint-plugin",
55
"homepage": "https://github.com/eslint-react/eslint-react",
66
"bugs": {
@@ -56,6 +56,7 @@
5656
"effect": "2.0.0-next.54",
5757
"rambda": "8.5.0",
5858
"string-ts": "1.3.2",
59+
"ts-api-utils": "1.0.3",
5960
"ts-pattern": "5.0.5"
6061
},
6162
"peerDependencies": {

packages/eslint-plugin-jsx/src/rules/no-leaked-conditional-rendering.spec.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ ruleTester.run(RULE_NAME, rule, {
7373
`,
7474
dedent`
7575
const App = ({ items, count }) => {
76-
return <div>{direction ? (direction === "down" ? "▼" : "▲") : ""}</div>
76+
return <div>{direction ? (direction === "down" ? "▼" : "▲") : "o"}</div>
7777
}
7878
`,
7979
dedent`

packages/eslint-plugin-jsx/src/rules/no-leaked-conditional-rendering.ts

+147-40
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,147 @@
1-
/* eslint-disable unicorn/no-typeof-undefined */
2-
import { isOneOf, NodeType } from "@eslint-react/ast";
1+
import { isJSXValue, JSXValueCheckHint } from "@eslint-react/jsx";
2+
import { getConstrainedTypeAtLocation } from "@typescript-eslint/type-utils";
33
import { type TSESTree } from "@typescript-eslint/types";
4-
import type { ESLintUtils } from "@typescript-eslint/utils";
4+
import { ESLintUtils } from "@typescript-eslint/utils";
55
import type { ConstantCase } from "string-ts";
6+
import * as tsutils from "ts-api-utils";
7+
import * as ts from "typescript";
68

79
import { createRule } from "../utils";
810

911
export const RULE_NAME = "no-leaked-conditional-rendering";
1012

1113
export type MessageID = ConstantCase<typeof RULE_NAME>;
1214

13-
type TernaryAlternateValue = RegExp | bigint | boolean | null | number | string;
15+
const allowTypes = [
16+
"boolean",
17+
"string",
1418

15-
const COERCE_STRATEGY = "coerce";
16-
const TERNARY_STRATEGY = "ternary";
17-
const DEFAULT_VALID_STRATEGIES = [TERNARY_STRATEGY, COERCE_STRATEGY] as const;
18-
const COERCE_VALID_LEFT_SIDE_EXPRESSIONS = [
19-
NodeType.UnaryExpression,
20-
NodeType.BinaryExpression,
21-
NodeType.CallExpression,
19+
"truthy boolean",
20+
"truthy string",
2221
] as const;
2322

24-
const TERNARY_INVALID_ALTERNATE_VALUES = new Set<TernaryAlternateValue>([null, false]);
23+
/** The types we care about */
24+
type VariantType =
25+
| "any"
26+
| "boolean"
27+
| "enum"
28+
| "never"
29+
| "nullish"
30+
| "number"
31+
| "object"
32+
| "string"
33+
| "truthy boolean"
34+
| "truthy number"
35+
| "truthy string";
2536

26-
function getIsCoerceValidNestedLogicalExpression(node: TSESTree.Node): boolean {
27-
if (node.type === NodeType.LogicalExpression) {
28-
return getIsCoerceValidNestedLogicalExpression(node.left)
29-
&& getIsCoerceValidNestedLogicalExpression(node.right);
37+
/**
38+
* Ported from https://github.com/typescript-eslint/typescript-eslint/blob/eb736bbfc22554694400e6a4f97051d845d32e0b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts#L826
39+
* Check union variants for the types we care about
40+
* @param types
41+
*/
42+
function inspectVariantTypes(types: ts.Type[]) {
43+
const variantTypes = new Set<VariantType>();
44+
45+
if (
46+
types.some(type =>
47+
tsutils.isTypeFlagSet(
48+
type,
49+
ts.TypeFlags.Null | ts.TypeFlags.Undefined | ts.TypeFlags.VoidLike,
50+
)
51+
)
52+
) {
53+
variantTypes.add("nullish");
3054
}
55+
const booleans = types.filter(type => tsutils.isTypeFlagSet(type, ts.TypeFlags.BooleanLike));
3156

32-
return isOneOf(COERCE_VALID_LEFT_SIDE_EXPRESSIONS)(node);
33-
}
57+
// If incoming type is either "true" or "false", there will be one type
58+
// object with intrinsicName set accordingly
59+
// If incoming type is boolean, there will be two type objects with
60+
// intrinsicName set "true" and "false" each because of ts-api-utils.unionTypeParts()
61+
// eslint-disable-next-line no-restricted-syntax
62+
if (booleans.length === 1 && booleans[0]) {
63+
tsutils.isTrueLiteralType(booleans[0])
64+
? variantTypes.add("truthy boolean")
65+
: variantTypes.add("boolean");
66+
} else if (booleans.length === 2) {
67+
variantTypes.add("boolean");
68+
}
69+
70+
const strings = types.filter(type => tsutils.isTypeFlagSet(type, ts.TypeFlags.StringLike));
71+
72+
if (strings.length > 0) {
73+
// eslint-disable-next-line no-restricted-syntax
74+
if (
75+
strings.every(type => type.isStringLiteral() && type.value !== "")
76+
) {
77+
variantTypes.add("truthy string");
78+
} else {
79+
variantTypes.add("string");
80+
}
81+
}
82+
83+
const numbers = types.filter(type =>
84+
tsutils.isTypeFlagSet(
85+
type,
86+
ts.TypeFlags.NumberLike | ts.TypeFlags.BigIntLike,
87+
)
88+
);
89+
90+
if (numbers.length > 0) {
91+
// eslint-disable-next-line no-restricted-syntax
92+
if (numbers.every(type => type.isNumberLiteral() && type.value !== 0)) {
93+
variantTypes.add("truthy number");
94+
} else {
95+
variantTypes.add("number");
96+
}
97+
}
98+
99+
if (
100+
types.some(type => tsutils.isTypeFlagSet(type, ts.TypeFlags.EnumLike))
101+
) {
102+
variantTypes.add("enum");
103+
}
104+
105+
if (
106+
types.some(
107+
type =>
108+
!tsutils.isTypeFlagSet(
109+
type,
110+
ts.TypeFlags.Null
111+
| ts.TypeFlags.Undefined
112+
| ts.TypeFlags.VoidLike
113+
| ts.TypeFlags.BooleanLike
114+
| ts.TypeFlags.StringLike
115+
| ts.TypeFlags.NumberLike
116+
| ts.TypeFlags.BigIntLike
117+
| ts.TypeFlags.TypeParameter
118+
| ts.TypeFlags.Any
119+
| ts.TypeFlags.Unknown
120+
| ts.TypeFlags.Never,
121+
),
122+
)
123+
) {
124+
variantTypes.add("object");
125+
}
34126

35-
function isValidTernaryAlternate(node: TSESTree.ConditionalExpression) {
36-
if (!("alternate" in node && "value" in node.alternate)) {
37-
return true;
127+
if (
128+
types.some(type =>
129+
tsutils.isTypeFlagSet(
130+
type,
131+
ts.TypeFlags.TypeParameter
132+
| ts.TypeFlags.Any
133+
| ts.TypeFlags.Unknown,
134+
)
135+
)
136+
) {
137+
variantTypes.add("any");
38138
}
39139

40-
if (typeof node.alternate.value === "undefined") {
41-
return false;
140+
if (types.some(type => tsutils.isTypeFlagSet(type, ts.TypeFlags.Never))) {
141+
variantTypes.add("never");
42142
}
43143

44-
return !TERNARY_INVALID_ALTERNATE_VALUES.has(node.alternate.value);
144+
return [...variantTypes];
45145
}
46146

47147
export default createRule<[], MessageID>({
@@ -61,34 +161,41 @@ export default createRule<[], MessageID>({
61161
},
62162
defaultOptions: [],
63163
create(context) {
164+
const services = ESLintUtils.getParserServices(context);
165+
166+
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition, @typescript-eslint/strict-boolean-expressions
167+
if (!services.program) {
168+
throw new Error("see https://typescript-eslint.io/docs/linting/type-linting");
169+
}
170+
64171
return {
65172
"JSXExpressionContainer > ConditionalExpression"(node: TSESTree.ConditionalExpression) {
66-
if (DEFAULT_VALID_STRATEGIES.includes(TERNARY_STRATEGY)) {
67-
return;
68-
}
69-
const isJSXElementAlternate = node.alternate.type === NodeType.JSXElement;
70-
if (isValidTernaryAlternate(node) || isJSXElementAlternate) {
173+
const hint = JSXValueCheckHint.SkipNumberLiteral
174+
| JSXValueCheckHint.StrictArray
175+
| JSXValueCheckHint.StrictLogical
176+
| JSXValueCheckHint.StrictConditional;
177+
178+
if (!isJSXValue(node, context, hint)) {
71179
context.report({
72180
messageId: "NO_LEAKED_CONDITIONAL_RENDERING",
73-
node: node.alternate,
181+
node,
74182
});
75183
}
76184
},
77185
'JSXExpressionContainer > LogicalExpression[operator="&&"]'(node: TSESTree.LogicalExpression) {
78-
const leftSide = node.left;
79-
const isCoerceValidLeftSide = isOneOf(COERCE_VALID_LEFT_SIDE_EXPRESSIONS)(leftSide);
80-
if (
81-
DEFAULT_VALID_STRATEGIES.includes(COERCE_STRATEGY)
82-
&& (isCoerceValidLeftSide || getIsCoerceValidNestedLogicalExpression(leftSide))
83-
) {
84-
return;
85-
}
86-
if (leftSide.type === NodeType.Literal && leftSide.value === "") {
186+
const { left } = node;
187+
188+
const leftType = getConstrainedTypeAtLocation(services, left);
189+
190+
const types = inspectVariantTypes([leftType]);
191+
192+
if (types.every(type => allowTypes.includes(type as never))) {
87193
return;
88194
}
195+
89196
context.report({
90197
messageId: "NO_LEAKED_CONDITIONAL_RENDERING",
91-
node: leftSide,
198+
node: left,
92199
});
93200
},
94201
};

0 commit comments

Comments
 (0)