Skip to content

Commit c6fcc09

Browse files
hjylewishjylewis
committed
fix(label-has-for): Make error messages for specific
The general error message 'Form label must have associated control' was used for every type of violation which was particularly confusing when the 'every' option was used. Instead the error messages should specify if the JSX requires every, some, or a specific type of associated control. Closes #419
1 parent 76fcbbf commit c6fcc09

File tree

2 files changed

+71
-49
lines changed

2 files changed

+71
-49
lines changed

‎__tests__/src/rules/label-has-for-test.js

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,22 @@ import rule from '../../../src/rules/label-has-for';
1919

2020
const ruleTester = new RuleTester();
2121

22-
const expectedError = {
23-
message: 'Form label must have associated control',
22+
const expectedNestingError = {
23+
message: 'Form label must have the following type of associated control: nesting',
2424
type: 'JSXOpeningElement',
2525
};
2626

27+
const expectedSomeError = {
28+
message: 'Form label must have ANY of the following types of associated control: nesting, id',
29+
type: 'JSXOpeningElement',
30+
};
31+
32+
const expectedEveryError = {
33+
message: 'Form label must have ALL of the following types of associated control: nesting, id',
34+
type: 'JSXOpeningElement',
35+
};
36+
37+
2738
const array = [{
2839
components: ['Label', 'Descriptor'],
2940
}];
@@ -81,78 +92,78 @@ ruleTester.run('label-has-for', rule, {
8192
].map(parserOptionsMapper),
8293
invalid: [
8394
// DEFAULT ELEMENT 'label' TESTS
84-
{ code: '<label id="foo" />', errors: [expectedError] },
85-
{ code: '<label htmlFor={undefined} />', errors: [expectedError] },
86-
{ code: '<label htmlFor={`${undefined}`} />', errors: [expectedError] },
87-
{ code: '<label>First Name</label>', errors: [expectedError] },
88-
{ code: '<label {...props}>Foo</label>', errors: [expectedError] },
89-
{ code: '<label><input /></label>', errors: [expectedError] },
90-
{ code: '<label>{children}</label>', errors: [expectedError] },
91-
{ code: '<label htmlFor="foo" />', errors: [expectedError] },
92-
{ code: '<label htmlFor={"foo"} />', errors: [expectedError] },
93-
{ code: '<label htmlFor={foo} />', errors: [expectedError] },
94-
{ code: '<label htmlFor={`${id}`} />', errors: [expectedError] },
95-
{ code: '<label htmlFor="foo">Test!</label>', errors: [expectedError] },
95+
{ code: '<label id="foo" />', errors: [expectedEveryError] },
96+
{ code: '<label htmlFor={undefined} />', errors: [expectedEveryError] },
97+
{ code: '<label htmlFor={`${undefined}`} />', errors: [expectedEveryError] },
98+
{ code: '<label>First Name</label>', errors: [expectedEveryError] },
99+
{ code: '<label {...props}>Foo</label>', errors: [expectedEveryError] },
100+
{ code: '<label><input /></label>', errors: [expectedEveryError] },
101+
{ code: '<label>{children}</label>', errors: [expectedEveryError] },
102+
{ code: '<label htmlFor="foo" />', errors: [expectedEveryError] },
103+
{ code: '<label htmlFor={"foo"} />', errors: [expectedEveryError] },
104+
{ code: '<label htmlFor={foo} />', errors: [expectedEveryError] },
105+
{ code: '<label htmlFor={`${id}`} />', errors: [expectedEveryError] },
106+
{ code: '<label htmlFor="foo">Test!</label>', errors: [expectedEveryError] },
96107
//
97108
// // CUSTOM ELEMENT ARRAY OPTION TESTS
98-
{ code: '<Label></Label>', errors: [expectedError], options: array },
99-
{ code: '<Label htmlFor="foo" />', errors: [expectedError], options: array },
100-
{ code: '<Label htmlFor={"foo"} />', errors: [expectedError], options: array },
101-
{ code: '<Label htmlFor={foo} />', errors: [expectedError], options: array },
102-
{ code: '<Label htmlFor={`${id}`} />', errors: [expectedError], options: array },
103-
{ code: '<Label htmlFor="foo">Test!</Label>', errors: [expectedError], options: array },
104-
{ code: '<Descriptor htmlFor="foo" />', errors: [expectedError], options: array },
105-
{ code: '<Descriptor htmlFor={"foo"} />', errors: [expectedError], options: array },
106-
{ code: '<Descriptor htmlFor={foo} />', errors: [expectedError], options: array },
107-
{ code: '<Descriptor htmlFor={`${id}`} />', errors: [expectedError], options: array },
109+
{ code: '<Label></Label>', errors: [expectedEveryError], options: array },
110+
{ code: '<Label htmlFor="foo" />', errors: [expectedEveryError], options: array },
111+
{ code: '<Label htmlFor={"foo"} />', errors: [expectedEveryError], options: array },
112+
{ code: '<Label htmlFor={foo} />', errors: [expectedEveryError], options: array },
113+
{ code: '<Label htmlFor={`${id}`} />', errors: [expectedEveryError], options: array },
114+
{ code: '<Label htmlFor="foo">Test!</Label>', errors: [expectedEveryError], options: array },
115+
{ code: '<Descriptor htmlFor="foo" />', errors: [expectedEveryError], options: array },
116+
{ code: '<Descriptor htmlFor={"foo"} />', errors: [expectedEveryError], options: array },
117+
{ code: '<Descriptor htmlFor={foo} />', errors: [expectedEveryError], options: array },
118+
{ code: '<Descriptor htmlFor={`${id}`} />', errors: [expectedEveryError], options: array },
108119
{
109120
code: '<Descriptor htmlFor="foo">Test!</Descriptor>',
110-
errors: [expectedError],
121+
errors: [expectedEveryError],
111122
options: array,
112123
},
113-
{ code: '<Label id="foo" />', errors: [expectedError], options: array },
124+
{ code: '<Label id="foo" />', errors: [expectedEveryError], options: array },
114125
{
115126
code: '<Label htmlFor={undefined} />',
116-
errors: [expectedError],
127+
errors: [expectedEveryError],
117128
options: array,
118129
},
119130
{
120131
code: '<Label htmlFor={`${undefined}`} />',
121-
errors: [expectedError],
132+
errors: [expectedEveryError],
122133
options: array,
123134
},
124-
{ code: '<Label>First Name</Label>', errors: [expectedError], options: array },
135+
{ code: '<Label>First Name</Label>', errors: [expectedEveryError], options: array },
125136
{
126137
code: '<Label {...props}>Foo</Label>',
127-
errors: [expectedError],
138+
errors: [expectedEveryError],
128139
options: array,
129140
},
130-
{ code: '<Descriptor id="foo" />', errors: [expectedError], options: array },
141+
{ code: '<Descriptor id="foo" />', errors: [expectedEveryError], options: array },
131142
{
132143
code: '<Descriptor htmlFor={undefined} />',
133-
errors: [expectedError],
144+
errors: [expectedEveryError],
134145
options: array,
135146
},
136147
{
137148
code: '<Descriptor htmlFor={`${undefined}`} />',
138-
errors: [expectedError],
149+
errors: [expectedEveryError],
139150
options: array,
140151
},
141152
{
142153
code: '<Descriptor>First Name</Descriptor>',
143-
errors: [expectedError],
154+
errors: [expectedEveryError],
144155
options: array,
145156
},
146157
{
147158
code: '<Descriptor {...props}>Foo</Descriptor>',
148-
errors: [expectedError],
159+
errors: [expectedEveryError],
149160
options: array,
150161
},
151-
{ code: '<label>{children}</label>', errors: [expectedError], options: array },
152-
{ code: '<label htmlFor="foo" />', errors: [expectedError], options: optionsRequiredNesting },
153-
{ code: '<label>First Name</label>', errors: [expectedError], options: optionsRequiredNesting },
154-
{ code: '<label>First Name</label>', errors: [expectedError], options: optionsRequiredSome },
155-
{ code: '<label>{children}</label>', errors: [expectedError], options: optionsRequiredSome },
156-
{ code: '<label>{children}</label>', errors: [expectedError], options: optionsRequiredNesting },
162+
{ code: '<label>{children}</label>', errors: [expectedEveryError], options: array },
163+
{ code: '<label htmlFor="foo" />', errors: [expectedNestingError], options: optionsRequiredNesting },
164+
{ code: '<label>First Name</label>', errors: [expectedNestingError], options: optionsRequiredNesting },
165+
{ code: '<label>First Name</label>', errors: [expectedSomeError], options: optionsRequiredSome },
166+
{ code: '<label>{children}</label>', errors: [expectedSomeError], options: optionsRequiredSome },
167+
{ code: '<label>{children}</label>', errors: [expectedNestingError], options: optionsRequiredNesting },
157168
].map(parserOptionsMapper),
158169
});

‎src/rules/label-has-for.js

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ import { getProp, getPropValue, elementType } from 'jsx-ast-utils';
1111
import { generateObjSchema, arraySchema, enumArraySchema } from '../util/schemas';
1212
import hasAccessibleChild from '../util/hasAccessibleChild';
1313

14-
const errorMessage = 'Form label must have associated control';
15-
1614
const enumValues = ['nesting', 'id'];
1715
const schema = {
1816
type: 'object',
@@ -48,14 +46,26 @@ const validate = (node, required, allowChildren) => {
4846
return validateId(node);
4947
};
5048

51-
const isValid = (node, required, allowChildren) => {
49+
const getValidityStatus = (node, required, allowChildren) => {
5250
if (Array.isArray(required.some)) {
53-
return required.some.some(rule => validate(node, rule, allowChildren));
51+
const isValid = required.some.some(rule => validate(node, rule, allowChildren));
52+
const message = !isValid
53+
? `Form label must have ANY of the following types of associated control: ${required.some.join(', ')}`
54+
: null;
55+
return { isValid, message };
5456
} else if (Array.isArray(required.every)) {
55-
return required.every.every(rule => validate(node, rule, allowChildren));
57+
const isValid = required.every.every(rule => validate(node, rule, allowChildren));
58+
const message = !isValid
59+
? `Form label must have ALL of the following types of associated control: ${required.every.join(', ')}`
60+
: null;
61+
return { isValid, message };
5662
}
5763

58-
return validate(node, required, allowChildren);
64+
const isValid = validate(node, required, allowChildren);
65+
const message = !isValid
66+
? `Form label must have the following type of associated control: ${required}`
67+
: null;
68+
return { isValid, message };
5969
};
6070

6171
module.exports = {
@@ -81,10 +91,11 @@ module.exports = {
8191
const required = options.required || { every: ['nesting', 'id'] };
8292
const allowChildren = options.allowChildren || false;
8393

84-
if (!isValid(node, required, allowChildren)) {
94+
const { isValid, message } = getValidityStatus(node, required, allowChildren);
95+
if (!isValid) {
8596
context.report({
8697
node,
87-
message: errorMessage,
98+
message,
8899
});
89100
}
90101
},

0 commit comments

Comments
 (0)