Skip to content

Commit a309bf0

Browse files
Add additional test cases to cover edge cases. Clean up code comments.
1 parent 1123656 commit a309bf0

File tree

2 files changed

+397
-99
lines changed

2 files changed

+397
-99
lines changed

lib/util/labelUtils.ts

Lines changed: 66 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -9,30 +9,22 @@ import { JSXOpeningElement } from "estree-jsx";
99
import { TSESTree } from "@typescript-eslint/utils";
1010

1111
/**
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).
12+
* Utility helpers for determining whether JSX controls are associated with visual labels
13+
* via id/htmlFor/aria-labelledby/aria-describedby attributes.
1414
*
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 ${...})
15+
* Supports these attribute value forms:
16+
* - Literal strings: id="value" or id='value'
17+
* - Expression containers: id={"value"} or id={variable}
18+
* - Binary concatenations: id={"prefix" + suffix}
19+
* - Template literals: id={`prefix-${variable}`}
2120
*
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.
21+
* getAttributeValueInfo normalizes attributes into these canonical forms:
22+
* - { kind: "string", raw: string, tokens: string[], exprText?: string }
23+
* - { kind: "identifier", name: string }
24+
* - { kind: "template", template: string }
25+
* - { kind: "empty" | "none" }
3526
*/
27+
3628
const validIdentifierRe = /^[A-Za-z_$][A-Za-z0-9_$]*$/;
3729

3830
/**
@@ -45,10 +37,7 @@ const isInsideLabelTag = (context: TSESLint.RuleContext<string, unknown[]>): boo
4537
return tagName.toLowerCase() === "label";
4638
});
4739

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-
*/
40+
/** Regex patterns for matching id/htmlFor attributes in source text. */
5241
const idLiteralDouble = '"([^"]*)"';
5342
const idLiteralSingle = "'([^']*)'";
5443
const exprStringDouble = '\\{\\s*"([^"]*)"\\s*\\}';
@@ -65,16 +54,16 @@ const escapeForRegExp = (s: string): string => s.replace(/[.*+?^${}()|[\]\\]/g,
6554
const getSourceText = (context: TSESLint.RuleContext<string, unknown[]>) => (context.getSourceCode() as any).text as string;
6655

6756
/**
68-
* Return captured id value from regex match where idOrExprRegex was used as the RHS.
69-
* match[2]..match[6] are the possible capture positions.
57+
* Extracts captured id value from regex match using idOrExprRegex.
58+
* Returns the first non-empty capture group from positions 2-6.
7059
*/
7160
const extractCapturedId = (match: RegExpExecArray): string | undefined => {
7261
return match[2] || match[3] || match[4] || match[5] || match[6] || undefined;
7362
};
7463

7564
/**
76-
* Evaluate simple constant BinaryExpression concatenations (left/right are Literals or nested BinaryExpressions).
77-
* Returns string when evaluation succeeds, otherwise undefined.
65+
* Evaluates constant BinaryExpression concatenations composed entirely of Literals.
66+
* Returns the computed string value or undefined if evaluation fails.
7867
*/
7968
const evalConstantString = (node: any): string | undefined => {
8069
if (!node || typeof node !== "object") return undefined;
@@ -92,19 +81,15 @@ const evalConstantString = (node: any): string | undefined => {
9281
};
9382

9483
/**
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.
84+
* Reconstructs source-like representation for Literal and BinaryExpression nodes.
85+
* Used for source-text matching of constant binary concatenations.
86+
* Strings are JSON.stringify'd, numbers use String(), binary expressions use "left + right" format.
10287
*/
10388
const renderSimpleExprSource = (node: any): string | undefined => {
10489
if (!node || typeof node !== "object") return undefined;
10590
if (node.type === "Literal") {
10691
const val = (node as any).value;
107-
if (typeof val === "string") return JSON.stringify(val); // keep the quotes "..."
92+
if (typeof val === "string") return JSON.stringify(val);
10893
return String(val);
10994
}
11095
if (node.type === "BinaryExpression" && node.operator === "+") {
@@ -114,13 +99,40 @@ const renderSimpleExprSource = (node: any): string | undefined => {
11499
if (right === undefined) return undefined;
115100
return `${left} + ${right}`;
116101
}
117-
// Not attempting to render arbitrary expressions (Identifiers, MemberExpression, etc.)
118102
return undefined;
119103
};
120104

121105
/**
122-
* Normalize attribute values into canonical shapes so callers can reason about
123-
* id/htmlFor/aria attributes in a small set of cases.
106+
* Builds regex pattern from template literal that matches literal parts
107+
* but allows any expression inside ${...} placeholders.
108+
*/
109+
const buildTemplatePattern = (template: string): string => {
110+
const placeholderRe = /\$\{[^}]*\}/g;
111+
let pattern = "";
112+
let idx = 0;
113+
let m: RegExpExecArray | null;
114+
while ((m = placeholderRe.exec(template)) !== null) {
115+
pattern += escapeForRegExp(template.slice(idx, m.index));
116+
pattern += "\\$\\{[^}]*\\}";
117+
idx = m.index + m[0].length;
118+
}
119+
pattern += escapeForRegExp(template.slice(idx));
120+
return pattern;
121+
};
122+
123+
/**
124+
* Tests if template matches any Label/label or other element with the specified attribute.
125+
*/
126+
const hasTemplateMatch = (template: string, attributeName: string, context: TSESLint.RuleContext<string, unknown[]>): boolean => {
127+
const src = getSourceText(context);
128+
const pattern = buildTemplatePattern(template);
129+
const labelRe = new RegExp(`<(?:Label|label)[^>]*\\b${attributeName}\\s*=\\s*\\{\\s*${pattern}\\s*\\}`, "i");
130+
const otherRe = new RegExp(`<(?:div|span|p|h[1-6])[^>]*\\b${attributeName}\\s*=\\s*\\{\\s*${pattern}\\s*\\}`, "i");
131+
return labelRe.test(src) || otherRe.test(src);
132+
};
133+
134+
/**
135+
* Normalizes attribute values into canonical shapes for consistent processing.
124136
*/
125137
const getAttributeValueInfo = (
126138
openingElement: TSESTree.JSXOpeningElement,
@@ -129,7 +141,6 @@ const getAttributeValueInfo = (
129141
): any => {
130142
const prop = getProp(openingElement.attributes as unknown as JSXOpeningElement["attributes"], attrName);
131143

132-
// Prefer inspecting the AST expression container directly when present
133144
if (prop && prop.value && (prop.value as any).type === "JSXExpressionContainer") {
134145
const expr = (prop.value as any).expression;
135146

@@ -141,20 +152,17 @@ const getAttributeValueInfo = (
141152
return { kind: "none" };
142153
}
143154

144-
// Expression container with a literal string: {"x"} or {'x'}
145155
if (expr && expr.type === "Literal" && typeof (expr as any).value === "string") {
146156
const trimmed = ((expr as any).value as string).trim();
147157
if (trimmed === "") return { kind: "empty" };
148158
return { kind: "string", raw: trimmed, tokens: trimmed.split(/\s+/), exprText: JSON.stringify((expr as any).value) };
149159
}
150160

151-
// BinaryExpression evaluation for constant concatenations only when composed of Literals.
152161
if (expr && expr.type === "BinaryExpression") {
153162
const v = evalConstantString(expr);
154163
if (typeof v === "string") {
155164
const trimmed = v.trim();
156165
if (trimmed === "") return { kind: "empty" };
157-
// Reconstruct a small source-like form for matching in raw source.
158166
const exprText = renderSimpleExprSource(expr);
159167
if (exprText) {
160168
return { kind: "string", raw: trimmed, tokens: trimmed.split(/\s+/), exprText };
@@ -163,8 +171,6 @@ const getAttributeValueInfo = (
163171
}
164172
}
165173

166-
// TemplateLiteral: reconstruct a canonical template string (preserve literal parts and insert
167-
// ${name} placeholders for identifiers, ${} for unknown expressions).
168174
if (expr && expr.type === "TemplateLiteral") {
169175
try {
170176
const quasis = (expr as any).quasis || [];
@@ -181,20 +187,19 @@ const getAttributeValueInfo = (
181187
} else if (e && e.type === "Literal") {
182188
templateRaw += "${" + String((e as any).value) + "}";
183189
} else {
184-
// unknown expression placeholder — include empty placeholder
185190
templateRaw += "${}";
186191
}
187192
}
188193
}
189194
templateRaw += "`";
190195
return { kind: "template", template: templateRaw };
191196
} catch {
192-
// If anything goes wrong, fall through to fallback.
197+
// Fall through to getPropValue fallback
193198
}
194199
}
195200
}
196201

197-
// Fallback: try to resolve via getPropValue (covers literal attrs and some resolvable cases)
202+
// Fallback to jsx-ast-utils for literal attributes and other resolvable cases
198203
const resolved = prop ? getPropValue(prop) : undefined;
199204
if (typeof resolved === "string") {
200205
const trimmed = resolved.trim();
@@ -206,8 +211,7 @@ const getAttributeValueInfo = (
206211
};
207212

208213
/**
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.
214+
* Searches for elements with braced attribute values (e.g., <Label id={variable}>).
211215
*/
212216
const hasBracedAttrId = (
213217
tagPattern: string,
@@ -222,7 +226,7 @@ const hasBracedAttrId = (
222226
};
223227

224228
/**
225-
* Checks if a Label exists with htmlFor that matches idValue.
229+
* Checks if any Label exists with htmlFor matching the given value.
226230
*/
227231
const hasLabelWithHtmlForId = (idValue: string, context: TSESLint.RuleContext<string, unknown[]>): boolean => {
228232
if (!idValue) return false;
@@ -239,7 +243,7 @@ const hasLabelWithHtmlForId = (idValue: string, context: TSESLint.RuleContext<st
239243
};
240244

241245
/**
242-
* Checks if a Label exists with id that matches idValue.
246+
* Checks if any Label exists with id matching the given value.
243247
*/
244248
const hasLabelWithHtmlId = (idValue: string, context: TSESLint.RuleContext<string, unknown[]>): boolean => {
245249
if (!idValue) return false;
@@ -256,7 +260,7 @@ const hasLabelWithHtmlId = (idValue: string, context: TSESLint.RuleContext<strin
256260
};
257261

258262
/**
259-
* Checks other simple elements (div/span/p/h1..h6) for id matching idValue.
263+
* Checks if any other element (div/span/p/h1-h6) has id matching the given value.
260264
*/
261265
const hasOtherElementWithHtmlId = (idValue: string, context: TSESLint.RuleContext<string, unknown[]>): boolean => {
262266
if (!idValue) return false;
@@ -273,13 +277,8 @@ const hasOtherElementWithHtmlId = (idValue: string, context: TSESLint.RuleContex
273277
};
274278

275279
/**
276-
* 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.
280+
* Checks if aria-* attribute references existing label or other elements.
281+
* Handles string tokens, identifier variables, and template literals.
283282
*/
284283
const hasAssociatedAriaText = (
285284
openingElement: TSESTree.JSXOpeningElement,
@@ -293,8 +292,6 @@ const hasAssociatedAriaText = (
293292
if (hasLabelWithHtmlId(id, context) || hasOtherElementWithHtmlId(id, context)) {
294293
return true;
295294
}
296-
// If this string was produced by evaluating a BinaryExpression in the source,
297-
// attempt to match the exact binary-expression source in other element id attributes.
298295
if (info.exprText) {
299296
const labelRe = new RegExp(`<(?:Label|label)[^>]*\\bid\\s*=\\s*\\{\\s*${escapeForRegExp(info.exprText)}\\s*\\}`, "i");
300297
const otherRe = new RegExp(`<(?:div|span|p|h[1-6])[^>]*\\bid\\s*=\\s*\\{\\s*${escapeForRegExp(info.exprText)}\\s*\\}`, "i");
@@ -311,24 +308,7 @@ const hasAssociatedAriaText = (
311308
}
312309

313310
if (info.kind === "template") {
314-
const templ = info.template as string;
315-
const src = getSourceText(context);
316-
// Build a pattern which matches the template's literal parts but allows any expression
317-
// inside `${...}` placeholders. This lets templates with non-Identifier expressions
318-
// (e.g. `${a.b}`) match the canonicalized template produced from the AST.
319-
const placeholderRe = /\$\{[^}]*\}/g;
320-
let pattern = "";
321-
let idx = 0;
322-
let m: RegExpExecArray | null;
323-
while ((m = placeholderRe.exec(templ)) !== null) {
324-
pattern += escapeForRegExp(templ.slice(idx, m.index));
325-
pattern += "\\$\\{[^}]*\\}";
326-
idx = m.index + m[0].length;
327-
}
328-
pattern += escapeForRegExp(templ.slice(idx));
329-
const labelRe = new RegExp(`<(?:Label|label)[^>]*\\bid\\s*=\\s*\\{\\s*${pattern}\\s*\\}`, "i");
330-
const otherRe = new RegExp(`<(?:div|span|p|h[1-6])[^>]*\\bid\\s*=\\s*\\{\\s*${pattern}\\s*\\}`, "i");
331-
return labelRe.test(src) || otherRe.test(src);
311+
return hasTemplateMatch(info.template as string, "id", context);
332312
}
333313

334314
return false;
@@ -344,13 +324,15 @@ const hasAssociatedLabelViaAriaDescribedby = (
344324
context: TSESLint.RuleContext<string, unknown[]>
345325
) => hasAssociatedAriaText(openingElement, context, "aria-describedby");
346326

327+
/**
328+
* Checks if element's id attribute has an associated Label with matching htmlFor.
329+
* Handles string literals, identifier variables, and template literals.
330+
*/
347331
const hasAssociatedLabelViaHtmlFor = (openingElement: TSESTree.JSXOpeningElement, context: TSESLint.RuleContext<string, unknown[]>) => {
348332
const info = getAttributeValueInfo(openingElement, context, "id");
349333

350334
if (info.kind === "string") {
351-
// primary: match literal/htmlFor forms
352335
if (hasLabelWithHtmlForId(info.raw, context)) return true;
353-
// fallback: match htmlFor written as a BinaryExpression / other expression that matches the same source text
354336
if (info.exprText) {
355337
const src = getSourceText(context);
356338
const htmlForRe = new RegExp(`<(?:Label|label)[^>]*\\bhtmlFor\\s*=\\s*\\{\\s*${escapeForRegExp(info.exprText)}\\s*\\}`, "i");
@@ -365,22 +347,7 @@ const hasAssociatedLabelViaHtmlFor = (openingElement: TSESTree.JSXOpeningElement
365347
}
366348

367349
if (info.kind === "template") {
368-
const templ = info.template as string;
369-
const src = getSourceText(context);
370-
// Build a pattern which matches the template's literal parts but allows any expression
371-
// inside `${...}` placeholders. This lets templates with non-Identifier expressions
372-
// (e.g. `${a.b}`) match the canonicalized template produced from the AST.
373-
const placeholderRe = /\$\{[^}]*\}/g;
374-
let pattern = "";
375-
let idx = 0;
376-
let m: RegExpExecArray | null;
377-
while ((m = placeholderRe.exec(templ)) !== null) {
378-
pattern += escapeForRegExp(templ.slice(idx, m.index));
379-
pattern += "\\$\\{[^}]*\\}";
380-
idx = m.index + m[0].length;
381-
}
382-
pattern += escapeForRegExp(templ.slice(idx));
383-
return new RegExp(`<(?:Label|label)[^>]*\\bhtmlFor\\s*=\\s*\\{\\s*${pattern}\\s*\\}`, "i").test(src);
350+
return hasTemplateMatch(info.template as string, "htmlFor", context);
384351
}
385352

386353
return false;

0 commit comments

Comments
 (0)