Skip to content

Commit a6cfa34

Browse files
committed
Refactor getJsxAttribute utility function
The commit consolidates attribute lookup functionality by removing the redundant hasJsxAttribute function and improving the getJsxAttribute API. The main changes: - Change getJsxAttribute to accept JSXElement node instead of attributes array - Add JSDoc comments explaining the function purpose and params - Remove the separate hasJsxAttribute function since it can be replaced with getJsxAttribute(ctx, node)(name) != null - Update all usage sites to use the new API - Use consistent "prop" terminology instead of mixing "attribute" and "prop"
1 parent 53ba364 commit a6cfa34

17 files changed

+98
-188
lines changed

packages/core/docs/README.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@
122122
- [getJsxConfigFromContext](functions/getJsxConfigFromContext.md)
123123
- [getJsxElementType](functions/getJsxElementType.md)
124124
- [getPhaseKindOfFunction](functions/getPhaseKindOfFunction.md)
125-
- [hasJsxAttribute](functions/hasJsxAttribute.md)
126125
- [hasNoneOrLooseComponentName](functions/hasNoneOrLooseComponentName.md)
127126
- [isAssignmentToThisState](functions/isAssignmentToThisState.md)
128127
- [isChildrenOfCreateElement](functions/isChildrenOfCreateElement.md)

packages/core/docs/functions/getJsxAttribute.md

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,35 @@
66

77
# Function: getJsxAttribute()
88

9-
> **getJsxAttribute**(`context`, `attributes`, `initialScope?`): (`name`) => `undefined` \| `TSESTreeJSXAttributeLike`
9+
> **getJsxAttribute**(`context`, `node`, `initialScope?`): (`name`) => `undefined` \| `JSXAttribute` \| `JSXSpreadAttribute`
10+
11+
Get a function to find JSX attributes by name, considering direct attributes and spread attributes.
1012

1113
## Parameters
1214

1315
### context
1416

1517
`RuleContext`
1618

17-
### attributes
19+
The ESLint rule context
20+
21+
### node
22+
23+
`JSXElement`
1824

19-
`TSESTreeJSXAttributeLike`[]
25+
The JSX element node
2026

2127
### initialScope?
2228

2329
`Scope`
2430

31+
Optional initial scope for variable resolution
32+
2533
## Returns
2634

27-
> (`name`): `undefined` \| `TSESTreeJSXAttributeLike`
35+
A function that takes an attribute name and returns the corresponding JSX attribute node or undefined
36+
37+
> (`name`): `undefined` \| `JSXAttribute` \| `JSXSpreadAttribute`
2838
2939
### Parameters
3040

@@ -34,4 +44,4 @@
3444

3545
### Returns
3646

37-
`undefined` \| `TSESTreeJSXAttributeLike`
47+
`undefined` \| `JSXAttribute` \| `JSXSpreadAttribute`

packages/core/docs/functions/hasJsxAttribute.md

Lines changed: 0 additions & 43 deletions
This file was deleted.
Lines changed: 19 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,47 @@
1-
import type * as AST from "@eslint-react/ast";
21
import type { RuleContext } from "@eslint-react/kit";
32
import { findProperty, findVariable, getVariableDefinitionNode } from "@eslint-react/var";
4-
import type { Scope } from "@typescript-eslint/scope-manager";
53
import type { TSESTree } from "@typescript-eslint/types";
64
import { AST_NODE_TYPES as T } from "@typescript-eslint/types";
75

6+
import type { Scope } from "@typescript-eslint/scope-manager";
87
import { getJsxAttributeName } from "./jsx-attribute-name";
98

10-
export function getJsxAttribute(
11-
context: RuleContext,
12-
attributes: AST.TSESTreeJSXAttributeLike[],
13-
initialScope?: Scope,
14-
) {
9+
/**
10+
* Get a function to find JSX attributes by name, considering direct attributes and spread attributes.
11+
* @param context The ESLint rule context
12+
* @param node The JSX element node
13+
* @param initialScope Optional initial scope for variable resolution
14+
* @returns A function that takes an attribute name and returns the corresponding JSX attribute node or undefined
15+
*/
16+
export function getJsxAttribute(context: RuleContext, node: TSESTree.JSXElement, initialScope?: Scope) {
17+
const scope = initialScope ?? context.sourceCode.getScope(node);
18+
const attributes = node.openingElement.attributes;
19+
/**
20+
* Find a JSX attribute by name, considering both direct attributes and spread attributes.
21+
* @param name The name of the attribute to find
22+
* @returns The JSX attribute node if found, otherwise undefined
23+
*/
1524
return (name: string) => {
1625
return attributes.findLast((attr) => {
1726
// Case 1: Direct JSX attribute (e.g., className="value")
1827
if (attr.type === T.JSXAttribute) {
1928
return getJsxAttributeName(context, attr) === name;
2029
}
21-
// For spread attributes, we need a scope to resolve variables
22-
if (initialScope == null) return false;
2330
switch (attr.argument.type) {
2431
// Case 2: Spread from variable (e.g., {...props})
2532
case T.Identifier: {
26-
const variable = findVariable(attr.argument.name, initialScope);
33+
const variable = findVariable(attr.argument.name, scope);
2734
const variableNode = getVariableDefinitionNode(variable, 0);
2835
if (variableNode?.type === T.ObjectExpression) {
29-
return findProperty(name, variableNode.properties, initialScope) != null;
36+
return findProperty(name, variableNode.properties, scope) != null;
3037
}
3138
return false;
3239
}
3340
// Case 3: Spread from object literal (e.g., {{...{prop: value}}})
3441
case T.ObjectExpression:
35-
return findProperty(name, attr.argument.properties, initialScope) != null;
42+
return findProperty(name, attr.argument.properties, scope) != null;
3643
}
3744
return false;
3845
});
3946
};
4047
}
41-
42-
/**
43-
* Checks if a JSX element has a specific attribute
44-
*
45-
* @param context - ESLint rule context
46-
* @param name - Name of the attribute to check for
47-
* @param attributes - List of JSX attributes from opening element
48-
* @param initialScope - Optional scope for resolving variables in spread attributes
49-
* @returns boolean indicating whether the attribute exists
50-
*/
51-
export function hasJsxAttribute(
52-
context: RuleContext,
53-
name: string,
54-
attributes: TSESTree.JSXOpeningElement["attributes"],
55-
initialScope?: Scope,
56-
) {
57-
return getJsxAttribute(context, attributes, initialScope)(name) != null;
58-
}

packages/plugins/eslint-plugin-react-dom/src/rules/no-dangerously-set-innerhtml-with-children.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,13 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
5555

5656
return {
5757
JSXElement(node) {
58-
const findJsxAttribute = getJsxAttribute(
59-
context,
60-
node.openingElement.attributes,
61-
context.sourceCode.getScope(node),
62-
);
58+
const findJsxAttribute = getJsxAttribute(context, node);
6359
if (findJsxAttribute(DSIH) == null) return;
64-
const children = findJsxAttribute("children") ?? node.children.find(isSignificantChildren);
65-
if (children == null) return;
60+
const childrenPropOrNode = findJsxAttribute("children") ?? node.children.find(isSignificantChildren);
61+
if (childrenPropOrNode == null) return;
6662
context.report({
6763
messageId: "noDangerouslySetInnerhtmlWithChildren",
68-
node: children,
64+
node: childrenPropOrNode,
6965
});
7066
},
7167
};

packages/plugins/eslint-plugin-react-dom/src/rules/no-dangerously-set-innerhtml.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,11 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
3535
if (!context.sourceCode.text.includes(DSIH)) return {};
3636
return {
3737
JSXElement(node) {
38-
const findJsxAttribute = getJsxAttribute(
39-
context,
40-
node.openingElement.attributes,
41-
context.sourceCode.getScope(node),
42-
);
43-
const attr = findJsxAttribute(DSIH);
44-
if (attr == null) return;
38+
const dsihProp = getJsxAttribute(context, node)(DSIH);
39+
if (dsihProp == null) return;
4540
context.report({
4641
messageId: "noDangerouslySetInnerhtml",
47-
node: attr,
42+
node: dsihProp,
4843
});
4944
},
5045
};

packages/plugins/eslint-plugin-react-dom/src/rules/no-missing-button-type.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,7 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
4545
return;
4646
}
4747

48-
const findJsxAttribute = getJsxAttribute(
49-
context,
50-
node.openingElement.attributes,
51-
context.sourceCode.getScope(node),
52-
);
53-
54-
if (findJsxAttribute("type") != null) {
48+
if (getJsxAttribute(context, node)("type") != null) {
5549
return;
5650
}
5751

packages/plugins/eslint-plugin-react-dom/src/rules/no-missing-iframe-sandbox.ts

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@ export default createRule<[], MessageID>({
1919
meta: {
2020
type: "problem",
2121
docs: {
22-
description: "Enforces explicit `sandbox` attribute for `iframe` elements.",
22+
description: "Enforces explicit `sandbox` prop for `iframe` elements.",
2323
[Symbol.for("rule_features")]: RULE_FEATURES,
2424
},
2525
fixable: "code",
2626
hasSuggestions: true,
2727
messages: {
28-
addIframeSandbox: "Add 'sandbox' attribute with value '{{value}}'.",
29-
noMissingIframeSandbox: "Add missing 'sandbox' attribute on 'iframe' component.",
28+
addIframeSandbox: "Add 'sandbox' prop with value '{{value}}'.",
29+
noMissingIframeSandbox: "Add missing 'sandbox' prop on 'iframe' component.",
3030
},
3131
schema: [],
3232
},
@@ -44,17 +44,11 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
4444
// If the element is not an iframe, we don't need to do anything.
4545
if (domElementType !== "iframe") return;
4646

47-
const findJsxAttribute = getJsxAttribute(
48-
context,
49-
node.openingElement.attributes,
50-
context.sourceCode.getScope(node),
51-
);
47+
// Find the 'sandbox' prop on the iframe element.
48+
const sandboxProp = getJsxAttribute(context, node)("sandbox");
5249

53-
// Find the 'sandbox' attribute on the iframe element.
54-
const sandboxAttr = findJsxAttribute("sandbox");
55-
56-
// If the 'sandbox' attribute is missing, report an error.
57-
if (sandboxAttr == null) {
50+
// If the 'sandbox' prop is missing, report an error.
51+
if (sandboxProp == null) {
5852
context.report({
5953
messageId: "noMissingIframeSandbox",
6054
node: node.openingElement,
@@ -71,23 +65,23 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
7165
}
7266

7367
// Resolve the value of the 'sandbox' attribute.
74-
const sandboxValue = resolveJsxAttributeValue(context, sandboxAttr);
75-
// If the value is a static string, the attribute is correctly used.
68+
const sandboxValue = resolveJsxAttributeValue(context, sandboxProp);
69+
// If the value is a static string, the prop is correctly used.
7670
if (typeof sandboxValue.toStatic("sandbox") === "string") return;
7771

7872
// If the value is not a static string (e.g., a variable), report an error.
7973
context.report({
8074
messageId: "noMissingIframeSandbox",
81-
node: sandboxValue.node ?? sandboxAttr,
75+
node: sandboxValue.node ?? sandboxProp,
8276
suggest: [
8377
{
8478
messageId: "addIframeSandbox",
8579
data: { value: "" },
8680
fix(fixer) {
8781
// Do not try to fix spread attributes.
8882
if (sandboxValue.kind.startsWith("spread")) return null;
89-
// Suggest replacing the attribute with a valid one.
90-
return fixer.replaceText(sandboxAttr, `sandbox=""`);
83+
// Suggest replacing the prop with a valid one.
84+
return fixer.replaceText(sandboxProp, `sandbox=""`);
9185
},
9286
},
9387
],

packages/plugins/eslint-plugin-react-dom/src/rules/no-string-style-prop.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,28 +36,22 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
3636
return;
3737
}
3838

39-
const findJsxAttribute = getJsxAttribute(
40-
context,
41-
node.openingElement.attributes,
42-
context.sourceCode.getScope(node),
43-
);
44-
45-
// Find the 'style' attribute on the element.
46-
const styleAttr = findJsxAttribute("style");
47-
if (styleAttr == null) {
39+
// Find the 'style' prop on the element.
40+
const styleProp = getJsxAttribute(context, node)("style");
41+
if (styleProp == null) {
4842
return;
4943
}
5044

51-
// Resolve the static value of the 'style' attribute.
52-
const styleValue = resolveJsxAttributeValue(context, styleAttr);
45+
// Resolve the static value of the 'style' prop.
46+
const styleValue = resolveJsxAttributeValue(context, styleProp);
5347
const staticValue = styleValue.toStatic();
5448

5549
// If the resolved value is a string, report an error.
5650
// e.g., <div style="color: red;" />
5751
if (typeof staticValue === "string") {
5852
context.report({
5953
messageId: "noStringStyleProp",
60-
node: styleValue.node ?? styleAttr,
54+
node: styleValue.node ?? styleProp,
6155
});
6256
}
6357
},

packages/plugins/eslint-plugin-react-dom/src/rules/no-unsafe-iframe-sandbox.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,23 +50,18 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
5050
if (resolver.resolve(node).domElementType !== "iframe") {
5151
return;
5252
}
53-
const findJsxAttribute = getJsxAttribute(
54-
context,
55-
node.openingElement.attributes,
56-
context.sourceCode.getScope(node),
57-
);
58-
const sandboxAttr = findJsxAttribute("sandbox");
59-
if (sandboxAttr == null) {
53+
const sandboxProp = getJsxAttribute(context, node)("sandbox");
54+
if (sandboxProp == null) {
6055
return;
6156
}
6257

63-
const sandboxValue = resolveJsxAttributeValue(context, sandboxAttr);
58+
const sandboxValue = resolveJsxAttributeValue(context, sandboxProp);
6459
const sandboxValueStatic = sandboxValue.toStatic("sandbox");
6560

6661
if (isUnsafeSandboxCombination(sandboxValueStatic)) {
6762
context.report({
6863
messageId: "noUnsafeIframeSandbox",
69-
node: sandboxValue.node ?? sandboxAttr,
64+
node: sandboxValue.node ?? sandboxProp,
7065
});
7166
}
7267
},

0 commit comments

Comments
 (0)