Skip to content

Commit bd2637f

Browse files
michaelfaithmichaelfaith
committed
feat: improve error messages for label-has-associated-control
This change updates the error messages of the label-has-associate-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 feat: improve error messages for label-has-associated-control This change updates the error messages of the label-has-associate-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 bd2637f

File tree

2 files changed

+103
-81
lines changed

2 files changed

+103
-81
lines changed

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

+70-59
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,58 +128,58 @@ 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] },
131+
const htmlForInvalid = (assertType) => [
132+
{ code: '<label htmlFor="js_id"><span><span><span>A label</span></span></span></label>', options: [{ depth: 4 }], errors: [expectedErrors[assertType]] },
133+
{ code: '<label htmlFor="js_id" aria-label="A label" />', errors: [expectedErrors[assertType]] },
134+
{ code: '<label htmlFor="js_id" aria-labelledby="A label" />', errors: [expectedErrors[assertType]] },
130135
// 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] },
136+
{ code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedErrors[assertType]] },
137+
{ code: '<CustomLabel htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedErrors[assertType]] },
138+
{ code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', settings: componentsSettings, errors: [expectedErrors[assertType]] },
134139
// Custom label attributes.
135-
{ code: '<label htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
140+
{ code: '<label htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedErrors[assertType]] },
136141
];
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] },
142+
const nestingInvalid = (assertType) => [
143+
{ code: '<label>A label<input /></label>', errors: [expectedErrors[assertType]] },
144+
{ code: '<label>A label<textarea /></label>', errors: [expectedErrors[assertType]] },
145+
{ code: '<label><img alt="A label" /><input /></label>', errors: [expectedErrors[assertType]] },
146+
{ code: '<label><img aria-label="A label" /><input /></label>', errors: [expectedErrors[assertType]] },
147+
{ code: '<label><span>A label<input /></span></label>', errors: [expectedErrors[assertType]] },
148+
{ code: '<label><span><span>A label<input /></span></span></label>', options: [{ depth: 3 }], errors: [expectedErrors[assertType]] },
149+
{ code: '<label><span><span><span>A label<input /></span></span></span></label>', options: [{ depth: 4 }], errors: [expectedErrors[assertType]] },
150+
{ code: '<label><span><span><span><span>A label</span><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedErrors[assertType]] },
151+
{ code: '<label><span><span><span><span aria-label="A label" /><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedErrors[assertType]] },
152+
{ code: '<label><span><span><span><input aria-label="A label" /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedErrors[assertType]] },
148153
// 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] },
154+
{ code: '<label>A label<OtherCustomInput /></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedErrors[assertType]] },
155+
{ code: '<label><span>A label<CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedErrors[assertType]] },
156+
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedErrors[assertType]] },
157+
{ code: '<CustomLabel><span label="A label"><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedErrors[assertType]] },
158+
{ code: '<label><span>A label<CustomInput /></span></label>', settings: componentsSettings, errors: [expectedErrors[assertType]] },
159+
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedErrors[assertType]] },
155160
];
156161

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] },
162+
const neverValid = (assertType) => [
163+
{ code: '<label htmlFor="js_id" />', errors: [expectedErrors.accessibleLabel] },
164+
{ code: '<label htmlFor="js_id"><input /></label>', errors: [expectedErrors.accessibleLabel] },
165+
{ code: '<label htmlFor="js_id"><textarea /></label>', errors: [expectedErrors.accessibleLabel] },
166+
{ code: '<label></label>', errors: [expectedErrors.accessibleLabel] },
167+
{ code: '<label>A label</label>', errors: [expectedErrors[assertType]] },
168+
{ code: '<div><label /><input /></div>', errors: [expectedErrors.accessibleLabel] },
169+
{ code: '<div><label>A label</label><input /></div>', errors: [expectedErrors[assertType]] },
165170
// 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] },
171+
{ code: '<CustomLabel aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedErrors[assertType]] },
172+
{ code: '<MUILabel aria-label="A label" />', options: [{ labelComponents: ['???Label'] }], errors: [expectedErrors[assertType]] },
173+
{ code: '<CustomLabel label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedErrors[assertType]] },
174+
{ code: '<CustomLabel aria-label="A label" />', settings: componentsSettings, errors: [expectedErrors[assertType]] },
170175
// Custom label attributes.
171-
{ code: '<label label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
176+
{ code: '<label label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedErrors[assertType]] },
172177
// 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+
{ code: '<label><span><CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedErrors.accessibleLabel] },
179+
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedErrors.accessibleLabel] },
180+
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedErrors.accessibleLabel] },
181+
{ code: '<label><span><CustomInput /></span></label>', settings: componentsSettings, errors: [expectedErrors.accessibleLabel] },
182+
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedErrors.accessibleLabel] },
178183
];
179184
// htmlFor valid
180185
ruleTester.run(ruleName, rule, {
@@ -187,8 +192,8 @@ ruleTester.run(ruleName, rule, {
187192
}))
188193
.map(parserOptionsMapper),
189194
invalid: parsers.all([].concat(
190-
...neverValid,
191-
...nestingInvalid,
195+
...neverValid('htmlFor'),
196+
...nestingInvalid('htmlFor'),
192197
))
193198
.map(ruleOptionsMapperFactory({
194199
assert: 'htmlFor',
@@ -207,8 +212,8 @@ ruleTester.run(ruleName, rule, {
207212
}))
208213
.map(parserOptionsMapper),
209214
invalid: parsers.all([].concat(
210-
...neverValid,
211-
...htmlForInvalid,
215+
...neverValid('nesting'),
216+
...htmlForInvalid('nesting'),
212217
))
213218
.map(ruleOptionsMapperFactory({
214219
assert: 'nesting',
@@ -228,8 +233,10 @@ ruleTester.run(ruleName, rule, {
228233
}))
229234
.map(parserOptionsMapper),
230235
invalid: parsers.all([].concat(
231-
...neverValid,
232-
)).map(parserOptionsMapper),
236+
...neverValid('either'),
237+
)).map(ruleOptionsMapperFactory({
238+
assert: 'either',
239+
})).map(parserOptionsMapper),
233240
});
234241

235242
// both valid
@@ -243,6 +250,10 @@ ruleTester.run(ruleName, rule, {
243250
}))
244251
.map(parserOptionsMapper),
245252
invalid: parsers.all([].concat(
246-
...neverValid,
247-
)).map(parserOptionsMapper),
253+
...neverValid('both'),
254+
...htmlForInvalid('both'),
255+
...nestingInvalid('both'),
256+
)).map(ruleOptionsMapperFactory({
257+
assert: 'both',
258+
})).map(parserOptionsMapper),
248259
});

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

+33-22
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: {
@@ -83,13 +88,13 @@ export default ({
8388
'progress',
8489
'select',
8590
'textarea',
86-
].concat((options.controlComponents || []));
91+
].concat(options.controlComponents || []);
8792
// Prevent crazy recursion.
8893
const recursionDepth = Math.min(
8994
options.depth === undefined ? 2 : options.depth,
9095
25,
9196
);
92-
const hasLabelId = validateID(node.openingElement, context);
97+
const hasHtmlFor = validateHtmlFor(node.openingElement, context);
9398
// Check for multiple control components.
9499
const hasNestedControl = controlComponents.some((name) => mayContainChildComponent(
95100
node,
@@ -105,44 +110,50 @@ export default ({
105110
controlComponents,
106111
);
107112

113+
// Bail out immediately if we don't have an accessible label.
108114
if (!hasAccessibleLabel) {
109115
context.report({
110116
node: node.openingElement,
111-
message: errorMessageNoLabel,
117+
message: errorMessages.accessibleLabel,
112118
});
113119
return;
114120
}
115-
116121
switch (assertType) {
117122
case 'htmlFor':
118-
if (hasLabelId) {
119-
return;
123+
if (!hasHtmlFor) {
124+
context.report({
125+
node: node.openingElement,
126+
message: errorMessages.htmlFor,
127+
});
120128
}
121129
break;
122130
case 'nesting':
123-
if (hasNestedControl) {
124-
return;
131+
if (!hasNestedControl) {
132+
context.report({
133+
node: node.openingElement,
134+
message: errorMessages.nesting,
135+
});
125136
}
126137
break;
127138
case 'both':
128-
if (hasLabelId && hasNestedControl) {
129-
return;
139+
if (!hasHtmlFor || !hasNestedControl) {
140+
context.report({
141+
node: node.openingElement,
142+
message: errorMessages.both,
143+
});
130144
}
131145
break;
132146
case 'either':
133-
if (hasLabelId || hasNestedControl) {
134-
return;
147+
if (!hasHtmlFor && !hasNestedControl) {
148+
context.report({
149+
node: node.openingElement,
150+
message: errorMessages.either,
151+
});
135152
}
136153
break;
137154
default:
138155
break;
139156
}
140-
141-
// htmlFor case
142-
context.report({
143-
node: node.openingElement,
144-
message: errorMessage,
145-
});
146157
};
147158

148159
// Create visitor selectors.

0 commit comments

Comments
 (0)