Skip to content

Commit 1123656

Browse files
Move comments to above of the function. Keep some inline comments for better readability.
1 parent 661bbb0 commit 1123656

File tree

1 file changed

+58
-29
lines changed

1 file changed

+58
-29
lines changed

lib/util/labelUtils.ts

Lines changed: 58 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,31 @@ import { TSESLint } from "@typescript-eslint/utils";
88
import { JSXOpeningElement } from "estree-jsx";
99
import { TSESTree } from "@typescript-eslint/utils";
1010

11+
/**
12+
* Utility helpers used by several rules to determine whether a JSX control is
13+
* associated with a visual label (via id/htmlFor/aria-labelledby/aria-describedby).
14+
*
15+
* Supported attribute RHS shapes (matched by `idOrExprRegex` and in AST inspection):
16+
* - "double-quoted" and 'single-quoted' attribute values
17+
* - expression containers with quoted strings: htmlFor={"id"} or id={'id'}
18+
* - expression containers with an Identifier: htmlFor={someId} or id={someId}
19+
* - simple constant BinaryExpression concatenations made only of Literals: {"my-" + 1}
20+
* - TemplateLiterals: `my-${value}` (we canonicalize literal parts and allow any expression inside ${...})
21+
*
22+
* getAttributeValueInfo normalizes the attribute into one of:
23+
* - { kind: "string", raw: string, tokens: string[], exprText?: string }
24+
* - { kind: "identifier", name: string }
25+
* - { kind: "template", template: string }
26+
* - { kind: "empty" }
27+
* - { kind: "none" }
28+
*
29+
* Design notes:
30+
* - Prefer AST inspection (JSXExpressionContainer) where available for precision.
31+
* - Evaluate BinaryExpression concatenations only when composed entirely of Literals (conservative).
32+
* - For TemplateLiteral matching we compare literal parts but allow any expression inside ${...}.
33+
* - For some binary-constant cases we reconstruct a small source form (exprText) and attempt to
34+
* find an exact-matching expression occurrence in the raw source when necessary.
35+
*/
1136
const validIdentifierRe = /^[A-Za-z_$][A-Za-z0-9_$]*$/;
1237

1338
/**
@@ -20,20 +45,15 @@ const isInsideLabelTag = (context: TSESLint.RuleContext<string, unknown[]>): boo
2045
return tagName.toLowerCase() === "label";
2146
});
2247

23-
/**
24-
* idOrExprRegex supports:
25-
* - "double-quoted" and 'single-quoted' attribute values
26-
* - expression containers with quoted strings: htmlFor={"id"} or id={'id'}
27-
* - expression containers with an Identifier: htmlFor={someId} or id={someId}
28-
*
29-
* Capture groups (when the alternation matches) are in positions 2..6
30-
* (group 1 is the element/tag capture used in some surrounding regexes).
31-
*/
48+
/* id / expression alternatives used when matching id/htmlFor attributes in source text.
49+
Capture groups (when this alternation is embedded in a surrounding regex) occupy
50+
consecutive groups so extractCapturedId reads groups 2..6 (group 1 is typically the tag).
51+
*/
3252
const idLiteralDouble = '"([^"]*)"';
3353
const idLiteralSingle = "'([^']*)'";
3454
const exprStringDouble = '\\{\\s*"([^"]*)"\\s*\\}';
3555
const exprStringSingle = "\\{\\s*'([^']*)'\\s*\\}";
36-
const exprIdentifier = "\\{\\s*([A-Za-z_$][A-Za-z0-9_$]*)\\s*\\}";
56+
const exprIdentifier = "\\{\\s*([A-Za-z_$][A-Za-l0-9_$]*)\\s*\\}";
3757

3858
const idOrExprRegex = new RegExp(
3959
`(?:${idLiteralDouble}|${idLiteralSingle}|${exprStringDouble}|${exprStringSingle}|${exprIdentifier})`,
@@ -72,9 +92,13 @@ const evalConstantString = (node: any): string | undefined => {
7292
};
7393

7494
/**
75-
* Small renderer to reconstruct simple expression source text for BinaryExpressions and Literals.
76-
* This provides a normalized textual form we can use to search the raw source for an exact expression match.
77-
* For strings, we preserve quotes by using JSON.stringify; numbers use String(value).
95+
* Reconstruct a small, predictable source-like form for Literal and BinaryExpression nodes.
96+
* Used for conservative source-text matching of constant binary concatenations.
97+
* - Strings are JSON.stringify'd to preserve quoting.
98+
* - Numbers are converted with String().
99+
* - BinaryExpressions are rendered as "<left> + <right>" with spaces (consistent formatting).
100+
*
101+
* Note: this deliberately avoids attempting to stringify arbitrary expressions.
78102
*/
79103
const renderSimpleExprSource = (node: any): string | undefined => {
80104
if (!node || typeof node !== "object") return undefined;
@@ -95,14 +119,8 @@ const renderSimpleExprSource = (node: any): string | undefined => {
95119
};
96120

97121
/**
98-
* New small helper: normalize attribute value (string list vs identifier vs empty/none vs template)
99-
*
100-
* Return shapes:
101-
* { kind: "string", raw: string, tokens: string[] }
102-
* { kind: "identifier", name: string }
103-
* { kind: "template", template: string } // template uses backticks and ${exprName} placeholders
104-
* { kind: "empty" }
105-
* { kind: "none" }
122+
* Normalize attribute values into canonical shapes so callers can reason about
123+
* id/htmlFor/aria attributes in a small set of cases.
106124
*/
107125
const getAttributeValueInfo = (
108126
openingElement: TSESTree.JSXOpeningElement,
@@ -115,28 +133,28 @@ const getAttributeValueInfo = (
115133
if (prop && prop.value && (prop.value as any).type === "JSXExpressionContainer") {
116134
const expr = (prop.value as any).expression;
117135

118-
// Identifier: only accept valid JS identifiers
136+
// Identifier: only accept valid JS identifiers (no hyphens, etc.)
119137
if (expr && expr.type === "Identifier") {
120138
if (typeof expr.name === "string" && validIdentifierRe.test(expr.name)) {
121139
return { kind: "identifier", name: expr.name as string };
122140
}
123141
return { kind: "none" };
124142
}
125143

126-
// Literal inside expression container: {"x"} or {'x'}
144+
// Expression container with a literal string: {"x"} or {'x'}
127145
if (expr && expr.type === "Literal" && typeof (expr as any).value === "string") {
128146
const trimmed = ((expr as any).value as string).trim();
129147
if (trimmed === "") return { kind: "empty" };
130148
return { kind: "string", raw: trimmed, tokens: trimmed.split(/\s+/), exprText: JSON.stringify((expr as any).value) };
131149
}
132150

133-
// BinaryExpression evaluation for constant concatenations: {"my-" + "label"} or {"my-" + 1}
151+
// BinaryExpression evaluation for constant concatenations only when composed of Literals.
134152
if (expr && expr.type === "BinaryExpression") {
135153
const v = evalConstantString(expr);
136154
if (typeof v === "string") {
137155
const trimmed = v.trim();
138156
if (trimmed === "") return { kind: "empty" };
139-
// Reconstruct simple source for the binary expression so we can search for an exact occurrence in raw source
157+
// Reconstruct a small source-like form for matching in raw source.
140158
const exprText = renderSimpleExprSource(expr);
141159
if (exprText) {
142160
return { kind: "string", raw: trimmed, tokens: trimmed.split(/\s+/), exprText };
@@ -145,7 +163,8 @@ const getAttributeValueInfo = (
145163
}
146164
}
147165

148-
// TemplateLiteral: reconstruct a canonical template string (preserve placeholders as ${name})
166+
// TemplateLiteral: reconstruct a canonical template string (preserve literal parts and insert
167+
// ${name} placeholders for identifiers, ${} for unknown expressions).
149168
if (expr && expr.type === "TemplateLiteral") {
150169
try {
151170
const quasis = (expr as any).quasis || [];
@@ -170,12 +189,12 @@ const getAttributeValueInfo = (
170189
templateRaw += "`";
171190
return { kind: "template", template: templateRaw };
172191
} catch {
173-
// if anything goes wrong, fall through
192+
// If anything goes wrong, fall through to fallback.
174193
}
175194
}
176195
}
177196

178-
// Fallback: try to resolve via getPropValue (covers literal attrs and expression-literals and other resolvable forms)
197+
// Fallback: try to resolve via getPropValue (covers literal attrs and some resolvable cases)
179198
const resolved = prop ? getPropValue(prop) : undefined;
180199
if (typeof resolved === "string") {
181200
const trimmed = resolved.trim();
@@ -186,6 +205,10 @@ const getAttributeValueInfo = (
186205
return { kind: "none" };
187206
};
188207

208+
/**
209+
* Look for an element with the given attribute written as a braced id: e.g. <Label id={foo}></Label>
210+
* Used as a narrow fallback for identifier-based matches.
211+
*/
189212
const hasBracedAttrId = (
190213
tagPattern: string,
191214
attrName: string,
@@ -251,6 +274,12 @@ const hasOtherElementWithHtmlId = (idValue: string, context: TSESLint.RuleContex
251274

252275
/**
253276
* Generic helper for aria-* attributes.
277+
*
278+
* - For string-kind attributes we check label/other elements by raw token id and also
279+
* attempt to match binary-constant expressions via exprText (when available).
280+
* - For identifier-kind attributes we look for braced identifier matches in the source.
281+
* - For template-kind attributes we build a flexible pattern that matches literal parts of the
282+
* template while permitting any expression text inside ${...} placeholders.
254283
*/
255284
const hasAssociatedAriaText = (
256285
openingElement: TSESTree.JSXOpeningElement,
@@ -264,7 +293,7 @@ const hasAssociatedAriaText = (
264293
if (hasLabelWithHtmlId(id, context) || hasOtherElementWithHtmlId(id, context)) {
265294
return true;
266295
}
267-
// Fallback: if this string was produced by evaluating a BinaryExpression in the source,
296+
// If this string was produced by evaluating a BinaryExpression in the source,
268297
// attempt to match the exact binary-expression source in other element id attributes.
269298
if (info.exprText) {
270299
const labelRe = new RegExp(`<(?:Label|label)[^>]*\\bid\\s*=\\s*\\{\\s*${escapeForRegExp(info.exprText)}\\s*\\}`, "i");

0 commit comments

Comments
 (0)