Skip to content

Commit 07ca867

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 a08fbcc commit 07ca867

File tree

2 files changed

+121
-87
lines changed

2 files changed

+121
-87
lines changed

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

Lines changed: 68 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +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',
24+
const errorMessages = {
25+
accessibleLabel: 'A form label must be associated with a control.',
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.',
2730
};
31+
const expectedErrors = {};
32+
Object.keys(errorMessages).forEach((key) => {
33+
expectedErrors[key] = {
34+
message: errorMessages[key],
35+
type: 'JSXOpeningElement',
36+
};
37+
});
2838

2939
const componentsSettings = {
3040
'jsx-a11y': {
@@ -101,56 +111,56 @@ const alwaysValid = [
101111
{ code: '<input type="hidden" />' },
102112
];
103113

104-
const htmlForInvalid = [
105-
{ code: '<label htmlFor="js_id"><span><span><span>A label</span></span></span></label>', options: [{ depth: 4 }], errors: [expectedError] },
106-
{ code: '<label htmlFor="js_id" aria-label="A label" />', errors: [expectedError] },
107-
{ code: '<label htmlFor="js_id" aria-labelledby="A label" />', errors: [expectedError] },
114+
const htmlForInvalid = (assertType) => [
115+
{ code: '<label htmlFor="js_id"><span><span><span>A label</span></span></span></label>', options: [{ depth: 4 }], errors: [expectedErrors[assertType]] },
116+
{ code: '<label htmlFor="js_id" aria-label="A label" />', errors: [expectedErrors[assertType]] },
117+
{ code: '<label htmlFor="js_id" aria-labelledby="A label" />', errors: [expectedErrors[assertType]] },
108118
// Custom label component.
109-
{ code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedError] },
110-
{ code: '<CustomLabel htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
111-
{ code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', settings: componentsSettings, errors: [expectedError] },
119+
{ code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedErrors[assertType]] },
120+
{ code: '<CustomLabel htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedErrors[assertType]] },
121+
{ code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', settings: componentsSettings, errors: [expectedErrors[assertType]] },
112122
// Custom label attributes.
113-
{ code: '<label htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
123+
{ code: '<label htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedErrors[assertType]] },
114124
];
115-
const nestingInvalid = [
116-
{ code: '<label>A label<input /></label>', errors: [expectedError] },
117-
{ code: '<label>A label<textarea /></label>', errors: [expectedError] },
118-
{ code: '<label><img alt="A label" /><input /></label>', errors: [expectedError] },
119-
{ code: '<label><img aria-label="A label" /><input /></label>', errors: [expectedError] },
120-
{ code: '<label><span>A label<input /></span></label>', errors: [expectedError] },
121-
{ code: '<label><span><span>A label<input /></span></span></label>', options: [{ depth: 3 }], errors: [expectedError] },
122-
{ code: '<label><span><span><span>A label<input /></span></span></span></label>', options: [{ depth: 4 }], errors: [expectedError] },
123-
{ code: '<label><span><span><span><span>A label</span><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
124-
{ code: '<label><span><span><span><span aria-label="A label" /><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
125-
{ code: '<label><span><span><span><input aria-label="A label" /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
125+
const nestingInvalid = (assertType) => [
126+
{ code: '<label>A label<input /></label>', errors: [expectedErrors[assertType]] },
127+
{ code: '<label>A label<textarea /></label>', errors: [expectedErrors[assertType]] },
128+
{ code: '<label><img alt="A label" /><input /></label>', errors: [expectedErrors[assertType]] },
129+
{ code: '<label><img aria-label="A label" /><input /></label>', errors: [expectedErrors[assertType]] },
130+
{ code: '<label><span>A label<input /></span></label>', errors: [expectedErrors[assertType]] },
131+
{ code: '<label><span><span>A label<input /></span></span></label>', options: [{ depth: 3 }], errors: [expectedErrors[assertType]] },
132+
{ code: '<label><span><span><span>A label<input /></span></span></span></label>', options: [{ depth: 4 }], errors: [expectedErrors[assertType]] },
133+
{ code: '<label><span><span><span><span>A label</span><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedErrors[assertType]] },
134+
{ code: '<label><span><span><span><span aria-label="A label" /><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedErrors[assertType]] },
135+
{ code: '<label><span><span><span><input aria-label="A label" /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedErrors[assertType]] },
126136
// Custom controlComponents.
127-
{ code: '<label><span>A label<CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedError] },
128-
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
129-
{ code: '<CustomLabel><span label="A label"><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedError] },
130-
{ code: '<label><span>A label<CustomInput /></span></label>', settings: componentsSettings, errors: [expectedError] },
131-
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedError] },
137+
{ code: '<label><span>A label<CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedErrors[assertType]] },
138+
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedErrors[assertType]] },
139+
{ code: '<CustomLabel><span label="A label"><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedErrors[assertType]] },
140+
{ code: '<label><span>A label<CustomInput /></span></label>', settings: componentsSettings, errors: [expectedErrors[assertType]] },
141+
{ code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedErrors[assertType]] },
132142
];
133143

134-
const neverValid = [
135-
{ code: '<label htmlFor="js_id" />', errors: [expectedError] },
136-
{ code: '<label htmlFor="js_id"><input /></label>', errors: [expectedError] },
137-
{ code: '<label htmlFor="js_id"><textarea /></label>', errors: [expectedError] },
138-
{ code: '<label></label>', errors: [expectedError] },
139-
{ code: '<label>A label</label>', errors: [expectedError] },
140-
{ code: '<div><label /><input /></div>', errors: [expectedError] },
141-
{ code: '<div><label>A label</label><input /></div>', errors: [expectedError] },
144+
const neverValid = (assertType) => [
145+
{ code: '<label htmlFor="js_id" />', errors: [expectedErrors.accessibleLabel] },
146+
{ code: '<label htmlFor="js_id"><input /></label>', errors: [expectedErrors.accessibleLabel] },
147+
{ code: '<label htmlFor="js_id"><textarea /></label>', errors: [expectedErrors.accessibleLabel] },
148+
{ code: '<label></label>', errors: [expectedErrors.accessibleLabel] },
149+
{ code: '<label>A label</label>', errors: [expectedErrors[assertType]] },
150+
{ code: '<div><label /><input /></div>', errors: [expectedErrors.accessibleLabel] },
151+
{ code: '<div><label>A label</label><input /></div>', errors: [expectedErrors[assertType]] },
142152
// Custom label component.
143-
{ code: '<CustomLabel aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedError] },
144-
{ code: '<CustomLabel label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
145-
{ code: '<CustomLabel aria-label="A label" />', settings: componentsSettings, errors: [expectedError] },
153+
{ code: '<CustomLabel aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedErrors[assertType]] },
154+
{ code: '<CustomLabel label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedErrors[assertType]] },
155+
{ code: '<CustomLabel aria-label="A label" />', settings: componentsSettings, errors: [expectedErrors[assertType]] },
146156
// Custom label attributes.
147-
{ code: '<label label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
157+
{ code: '<label label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedErrors[assertType]] },
148158
// Custom controlComponents.
149-
{ code: '<label><span><CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedError] },
150-
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
151-
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedError] },
152-
{ code: '<label><span><CustomInput /></span></label>', settings: componentsSettings, errors: [expectedError] },
153-
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedError] },
159+
{ code: '<label><span><CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedErrors.accessibleLabel] },
160+
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedErrors.accessibleLabel] },
161+
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedErrors.accessibleLabel] },
162+
{ code: '<label><span><CustomInput /></span></label>', settings: componentsSettings, errors: [expectedErrors.accessibleLabel] },
163+
{ code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedErrors.accessibleLabel] },
154164
];
155165
// htmlFor valid
156166
ruleTester.run(ruleName, rule, {
@@ -163,8 +173,8 @@ ruleTester.run(ruleName, rule, {
163173
}))
164174
.map(parserOptionsMapper),
165175
invalid: parsers.all([].concat(
166-
...neverValid,
167-
...nestingInvalid,
176+
...neverValid('htmlFor'),
177+
...nestingInvalid('htmlFor'),
168178
))
169179
.map(ruleOptionsMapperFactory({
170180
assert: 'htmlFor',
@@ -183,8 +193,8 @@ ruleTester.run(ruleName, rule, {
183193
}))
184194
.map(parserOptionsMapper),
185195
invalid: parsers.all([].concat(
186-
...neverValid,
187-
...htmlForInvalid,
196+
...neverValid('nesting'),
197+
...htmlForInvalid('nesting'),
188198
))
189199
.map(ruleOptionsMapperFactory({
190200
assert: 'nesting',
@@ -204,8 +214,10 @@ ruleTester.run(ruleName, rule, {
204214
}))
205215
.map(parserOptionsMapper),
206216
invalid: parsers.all([].concat(
207-
...neverValid,
208-
)).map(parserOptionsMapper),
217+
...neverValid('either'),
218+
)).map(ruleOptionsMapperFactory({
219+
assert: 'either',
220+
})).map(parserOptionsMapper),
209221
});
210222

211223
// both valid
@@ -219,6 +231,10 @@ ruleTester.run(ruleName, rule, {
219231
}))
220232
.map(parserOptionsMapper),
221233
invalid: parsers.all([].concat(
222-
...neverValid,
223-
)).map(parserOptionsMapper),
234+
...neverValid('both'),
235+
...htmlForInvalid('both'),
236+
...nestingInvalid('both'),
237+
)).map(ruleOptionsMapperFactory({
238+
assert: 'both',
239+
})).map(parserOptionsMapper),
224240
});

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

Lines changed: 53 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,13 @@ import getElementType from '../util/getElementType';
1717
import mayContainChildComponent from '../util/mayContainChildComponent';
1818
import mayHaveAccessibleLabel from '../util/mayHaveAccessibleLabel';
1919

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

2228
const schema = generateObjSchema({
2329
labelComponents: arraySchema,
@@ -35,7 +41,7 @@ const schema = generateObjSchema({
3541
},
3642
});
3743

38-
const validateId = (node) => {
44+
const validateHtmlFor = (node) => {
3945
const htmlForAttr = getProp(node.attributes, 'htmlFor');
4046
const htmlForValue = getPropValue(htmlForAttr);
4147

@@ -69,13 +75,13 @@ export default ({
6975
'progress',
7076
'select',
7177
'textarea',
72-
].concat((options.controlComponents || []));
78+
].concat(options.controlComponents || []);
7379
// Prevent crazy recursion.
7480
const recursionDepth = Math.min(
7581
options.depth === undefined ? 2 : options.depth,
7682
25,
7783
);
78-
const hasLabelId = validateId(node.openingElement);
84+
const hasHtmlFor = validateHtmlFor(node.openingElement);
7985
// Check for multiple control components.
8086
const hasNestedControl = controlComponents.some((name) => mayContainChildComponent(
8187
node,
@@ -91,38 +97,50 @@ export default ({
9197
controlComponents,
9298
);
9399

94-
if (hasAccessibleLabel) {
95-
switch (assertType) {
96-
case 'htmlFor':
97-
if (hasLabelId) {
98-
return;
99-
}
100-
break;
101-
case 'nesting':
102-
if (hasNestedControl) {
103-
return;
104-
}
105-
break;
106-
case 'both':
107-
if (hasLabelId && hasNestedControl) {
108-
return;
109-
}
110-
break;
111-
case 'either':
112-
if (hasLabelId || hasNestedControl) {
113-
return;
114-
}
115-
break;
116-
default:
117-
break;
118-
}
100+
// Bail out immediately if we don't have an accessible label.
101+
if (!hasAccessibleLabel) {
102+
context.report({
103+
node: node.openingElement,
104+
message: errorMessages.accessibleLabel,
105+
});
106+
return;
107+
}
108+
switch (assertType) {
109+
case 'htmlFor':
110+
if (!hasHtmlFor) {
111+
context.report({
112+
node: node.openingElement,
113+
message: errorMessages.htmlFor,
114+
});
115+
}
116+
break;
117+
case 'nesting':
118+
if (!hasNestedControl) {
119+
context.report({
120+
node: node.openingElement,
121+
message: errorMessages.nesting,
122+
});
123+
}
124+
break;
125+
case 'both':
126+
if (!hasHtmlFor || !hasNestedControl) {
127+
context.report({
128+
node: node.openingElement,
129+
message: errorMessages.both,
130+
});
131+
}
132+
break;
133+
case 'either':
134+
if (!hasHtmlFor && !hasNestedControl) {
135+
context.report({
136+
node: node.openingElement,
137+
message: errorMessages.either,
138+
});
139+
}
140+
break;
141+
default:
142+
break;
119143
}
120-
121-
// htmlFor case
122-
context.report({
123-
node: node.openingElement,
124-
message: errorMessage,
125-
});
126144
};
127145

128146
// Create visitor selectors.

0 commit comments

Comments
 (0)