Skip to content

Commit bfb0e9e

Browse files
michaelfaithljharb
michaelfaith
authored and
ljharb
committed
[patch] label-has-associated-control: improve error messages
This change updates the error messages of the label-has-associated-control rule so that each assert type gets an error message with verbiage specific to the assertion. I wanted to land this before adding support for matching a label's htmlFor attribute with the associated control's id
1 parent 7566e13 commit bfb0e9e

File tree

2 files changed

+123
-91
lines changed

2 files changed

+123
-91
lines changed

‎__tests__/src/rules/label-has-associated-control-test.js

+88-68
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,20 @@ const ruleTester = new RuleTester();
2121

2222
const ruleName = 'label-has-associated-control';
2323

24-
const expectedError = {
25-
message: 'A form label must be associated with a control.',
26-
type: 'JSXOpeningElement',
27-
};
28-
29-
const expectedErrorNoLabel = {
30-
message: 'A form label must have accessible text.',
31-
type: 'JSXOpeningElement',
24+
const errorMessages = {
25+
accessibleLabel: 'A form label must have accessible text.',
26+
htmlFor: 'A form label must have a valid htmlFor attribute.',
27+
nesting: 'A form label must have an associated control as a descendant.',
28+
either: 'A form label must either have a valid htmlFor attribute or a control as a descendant.',
29+
both: 'A form label must have a valid htmlFor attribute and a control as a descendant.',
3230
};
31+
const expectedErrors = {};
32+
Object.keys(errorMessages).forEach((key) => {
33+
expectedErrors[key] = {
34+
message: errorMessages[key],
35+
type: 'JSXOpeningElement',
36+
};
37+
});
3338

3439
const componentsSettings = {
3540
'jsx-a11y': {
@@ -123,59 +128,68 @@ const alwaysValid = [
123128
{ code: '<input type="hidden" />' },
124129
];
125130

126-
const htmlForInvalid = [
127-
{ code: '<label htmlFor="js_id"><span><span><span>A label</span></span></span></label>', options: [{ depth: 4 }], errors: [expectedError] },
128-
{ code: '<label htmlFor="js_id" aria-label="A label" />', errors: [expectedError] },
129-
{ code: '<label htmlFor="js_id" aria-labelledby="A label" />', errors: [expectedError] },
130-
// Custom label component.
131-
{ code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedError] },
132-
{ code: '<CustomLabel htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
133-
{ code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', settings: componentsSettings, errors: [expectedError] },
134-
// Custom label attributes.
135-
{ code: '<label htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
136-
];
137-
const nestingInvalid = [
138-
{ code: '<label>A label<input /></label>', errors: [expectedError] },
139-
{ code: '<label>A label<textarea /></label>', errors: [expectedError] },
140-
{ code: '<label><img alt="A label" /><input /></label>', errors: [expectedError] },
141-
{ code: '<label><img aria-label="A label" /><input /></label>', errors: [expectedError] },
142-
{ code: '<label><span>A label<input /></span></label>', errors: [expectedError] },
143-
{ code: '<label><span><span>A label<input /></span></span></label>', options: [{ depth: 3 }], errors: [expectedError] },
144-
{ code: '<label><span><span><span>A label<input /></span></span></span></label>', options: [{ depth: 4 }], errors: [expectedError] },
145-
{ code: '<label><span><span><span><span>A label</span><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
146-
{ code: '<label><span><span><span><span aria-label="A label" /><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
147-
{ code: '<label><span><span><span><input aria-label="A label" /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
148-
// Custom controlComponents.
149-
{ code: '<label>A label<OtherCustomInput /></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedError] },
150-
{ code: '<label><span>A label<CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedError] },
151-
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
152-
{ code: '<CustomLabel><span label="A label"><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedError] },
153-
{ code: '<label><span>A label<CustomInput /></span></label>', settings: componentsSettings, errors: [expectedError] },
154-
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedError] },
155-
];
131+
const htmlForInvalid = (assertType) => {
132+
const expectedError = expectedErrors[assertType];
133+
return [
134+
{ code: '<label htmlFor="js_id"><span><span><span>A label</span></span></span></label>', options: [{ depth: 4 }], errors: [expectedError] },
135+
{ code: '<label htmlFor="js_id" aria-label="A label" />', errors: [expectedError] },
136+
{ code: '<label htmlFor="js_id" aria-labelledby="A label" />', errors: [expectedError] },
137+
// Custom label component.
138+
{ code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedError] },
139+
{ code: '<CustomLabel htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
140+
{ code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', settings: componentsSettings, errors: [expectedError] },
141+
// Custom label attributes.
142+
{ code: '<label htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
143+
];
144+
};
145+
const nestingInvalid = (assertType) => {
146+
const expectedError = expectedErrors[assertType];
147+
return [
148+
{ code: '<label>A label<input /></label>', errors: [expectedError] },
149+
{ code: '<label>A label<textarea /></label>', errors: [expectedError] },
150+
{ code: '<label><img alt="A label" /><input /></label>', errors: [expectedError] },
151+
{ code: '<label><img aria-label="A label" /><input /></label>', errors: [expectedError] },
152+
{ code: '<label><span>A label<input /></span></label>', errors: [expectedError] },
153+
{ code: '<label><span><span>A label<input /></span></span></label>', options: [{ depth: 3 }], errors: [expectedError] },
154+
{ code: '<label><span><span><span>A label<input /></span></span></span></label>', options: [{ depth: 4 }], errors: [expectedError] },
155+
{ code: '<label><span><span><span><span>A label</span><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
156+
{ code: '<label><span><span><span><span aria-label="A label" /><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
157+
{ code: '<label><span><span><span><input aria-label="A label" /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
158+
// Custom controlComponents.
159+
{ code: '<label>A label<OtherCustomInput /></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedError] },
160+
{ code: '<label><span>A label<CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedError] },
161+
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
162+
{ code: '<CustomLabel><span label="A label"><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedError] },
163+
{ code: '<label><span>A label<CustomInput /></span></label>', settings: componentsSettings, errors: [expectedError] },
164+
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedError] },
165+
];
166+
};
156167

157-
const neverValid = [
158-
{ code: '<label htmlFor="js_id" />', errors: [expectedErrorNoLabel] },
159-
{ code: '<label htmlFor="js_id"><input /></label>', errors: [expectedErrorNoLabel] },
160-
{ code: '<label htmlFor="js_id"><textarea /></label>', errors: [expectedErrorNoLabel] },
161-
{ code: '<label></label>', errors: [expectedErrorNoLabel] },
162-
{ code: '<label>A label</label>', errors: [expectedError] },
163-
{ code: '<div><label /><input /></div>', errors: [expectedErrorNoLabel] },
164-
{ code: '<div><label>A label</label><input /></div>', errors: [expectedError] },
165-
// Custom label component.
166-
{ code: '<CustomLabel aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedError] },
167-
{ code: '<MUILabel aria-label="A label" />', options: [{ labelComponents: ['???Label'] }], errors: [expectedError] },
168-
{ code: '<CustomLabel label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
169-
{ code: '<CustomLabel aria-label="A label" />', settings: componentsSettings, errors: [expectedError] },
170-
// Custom label attributes.
171-
{ code: '<label label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
172-
// Custom controlComponents.
173-
{ code: '<label><span><CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedErrorNoLabel] },
174-
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedErrorNoLabel] },
175-
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedErrorNoLabel] },
176-
{ code: '<label><span><CustomInput /></span></label>', settings: componentsSettings, errors: [expectedErrorNoLabel] },
177-
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedErrorNoLabel] },
178-
];
168+
const neverValid = (assertType) => {
169+
const expectedError = expectedErrors[assertType];
170+
return [
171+
{ code: '<label htmlFor="js_id" />', errors: [expectedErrors.accessibleLabel] },
172+
{ code: '<label htmlFor="js_id"><input /></label>', errors: [expectedErrors.accessibleLabel] },
173+
{ code: '<label htmlFor="js_id"><textarea /></label>', errors: [expectedErrors.accessibleLabel] },
174+
{ code: '<label></label>', errors: [expectedErrors.accessibleLabel] },
175+
{ code: '<label>A label</label>', errors: [expectedError] },
176+
{ code: '<div><label /><input /></div>', errors: [expectedErrors.accessibleLabel] },
177+
{ code: '<div><label>A label</label><input /></div>', errors: [expectedError] },
178+
// Custom label component.
179+
{ code: '<CustomLabel aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedError] },
180+
{ code: '<MUILabel aria-label="A label" />', options: [{ labelComponents: ['???Label'] }], errors: [expectedError] },
181+
{ code: '<CustomLabel label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
182+
{ code: '<CustomLabel aria-label="A label" />', settings: componentsSettings, errors: [expectedError] },
183+
// Custom label attributes.
184+
{ code: '<label label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
185+
// Custom controlComponents.
186+
{ code: '<label><span><CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedErrors.accessibleLabel] },
187+
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedErrors.accessibleLabel] },
188+
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedErrors.accessibleLabel] },
189+
{ code: '<label><span><CustomInput /></span></label>', settings: componentsSettings, errors: [expectedErrors.accessibleLabel] },
190+
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedErrors.accessibleLabel] },
191+
];
192+
};
179193
// htmlFor valid
180194
ruleTester.run(ruleName, rule, {
181195
valid: parsers.all([].concat(
@@ -187,8 +201,8 @@ ruleTester.run(ruleName, rule, {
187201
}))
188202
.map(parserOptionsMapper),
189203
invalid: parsers.all([].concat(
190-
...neverValid,
191-
...nestingInvalid,
204+
...neverValid('htmlFor'),
205+
...nestingInvalid('htmlFor'),
192206
))
193207
.map(ruleOptionsMapperFactory({
194208
assert: 'htmlFor',
@@ -207,8 +221,8 @@ ruleTester.run(ruleName, rule, {
207221
}))
208222
.map(parserOptionsMapper),
209223
invalid: parsers.all([].concat(
210-
...neverValid,
211-
...htmlForInvalid,
224+
...neverValid('nesting'),
225+
...htmlForInvalid('nesting'),
212226
))
213227
.map(ruleOptionsMapperFactory({
214228
assert: 'nesting',
@@ -228,8 +242,10 @@ ruleTester.run(ruleName, rule, {
228242
}))
229243
.map(parserOptionsMapper),
230244
invalid: parsers.all([].concat(
231-
...neverValid,
232-
)).map(parserOptionsMapper),
245+
...neverValid('either'),
246+
)).map(ruleOptionsMapperFactory({
247+
assert: 'either',
248+
})).map(parserOptionsMapper),
233249
});
234250

235251
// both valid
@@ -243,6 +259,10 @@ ruleTester.run(ruleName, rule, {
243259
}))
244260
.map(parserOptionsMapper),
245261
invalid: parsers.all([].concat(
246-
...neverValid,
247-
)).map(parserOptionsMapper),
262+
...neverValid('both'),
263+
...htmlForInvalid('both'),
264+
...nestingInvalid('both'),
265+
)).map(ruleOptionsMapperFactory({
266+
assert: 'both',
267+
})).map(parserOptionsMapper),
248268
});

‎src/rules/label-has-associated-control.js

+35-23
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,13 @@ import getElementType from '../util/getElementType';
1818
import mayContainChildComponent from '../util/mayContainChildComponent';
1919
import mayHaveAccessibleLabel from '../util/mayHaveAccessibleLabel';
2020

21-
const errorMessage = 'A form label must be associated with a control.';
22-
const errorMessageNoLabel = 'A form label must have accessible text.';
21+
const errorMessages = {
22+
accessibleLabel: 'A form label must have accessible text.',
23+
htmlFor: 'A form label must have a valid htmlFor attribute.',
24+
nesting: 'A form label must have an associated control as a descendant.',
25+
either: 'A form label must either have a valid htmlFor attribute or a control as a descendant.',
26+
both: 'A form label must have a valid htmlFor attribute and a control as a descendant.',
27+
};
2328

2429
const schema = generateObjSchema({
2530
labelComponents: arraySchema,
@@ -37,7 +42,7 @@ const schema = generateObjSchema({
3742
},
3843
});
3944

40-
function validateID(node, context) {
45+
const validateHtmlFor = (node, context) => {
4146
const { settings } = context;
4247
const htmlForAttributes = settings['jsx-a11y']?.attributes?.for ?? ['htmlFor'];
4348

@@ -52,7 +57,7 @@ function validateID(node, context) {
5257
}
5358

5459
return false;
55-
}
60+
};
5661

5762
export default ({
5863
meta: {
@@ -76,20 +81,21 @@ export default ({
7681
return;
7782
}
7883

79-
const controlComponents = [
84+
const controlComponents = [].concat(
8085
'input',
8186
'meter',
8287
'output',
8388
'progress',
8489
'select',
8590
'textarea',
86-
].concat((options.controlComponents || []));
91+
options.controlComponents || [],
92+
);
8793
// Prevent crazy recursion.
8894
const recursionDepth = Math.min(
8995
options.depth === undefined ? 2 : options.depth,
9096
25,
9197
);
92-
const hasLabelId = validateID(node.openingElement, context);
98+
const hasHtmlFor = validateHtmlFor(node.openingElement, context);
9399
// Check for multiple control components.
94100
const hasNestedControl = controlComponents.some((name) => mayContainChildComponent(
95101
node,
@@ -105,44 +111,50 @@ export default ({
105111
controlComponents,
106112
);
107113

114+
// Bail out immediately if we don't have an accessible label.
108115
if (!hasAccessibleLabel) {
109116
context.report({
110117
node: node.openingElement,
111-
message: errorMessageNoLabel,
118+
message: errorMessages.accessibleLabel,
112119
});
113120
return;
114121
}
115-
116122
switch (assertType) {
117123
case 'htmlFor':
118-
if (hasLabelId) {
119-
return;
124+
if (!hasHtmlFor) {
125+
context.report({
126+
node: node.openingElement,
127+
message: errorMessages.htmlFor,
128+
});
120129
}
121130
break;
122131
case 'nesting':
123-
if (hasNestedControl) {
124-
return;
132+
if (!hasNestedControl) {
133+
context.report({
134+
node: node.openingElement,
135+
message: errorMessages.nesting,
136+
});
125137
}
126138
break;
127139
case 'both':
128-
if (hasLabelId && hasNestedControl) {
129-
return;
140+
if (!hasHtmlFor || !hasNestedControl) {
141+
context.report({
142+
node: node.openingElement,
143+
message: errorMessages.both,
144+
});
130145
}
131146
break;
132147
case 'either':
133-
if (hasLabelId || hasNestedControl) {
134-
return;
148+
if (!hasHtmlFor && !hasNestedControl) {
149+
context.report({
150+
node: node.openingElement,
151+
message: errorMessages.either,
152+
});
135153
}
136154
break;
137155
default:
138156
break;
139157
}
140-
141-
// htmlFor case
142-
context.report({
143-
node: node.openingElement,
144-
message: errorMessage,
145-
});
146158
};
147159

148160
// Create visitor selectors.

0 commit comments

Comments
 (0)