Skip to content

Commit cdbc37d

Browse files
juzerzarifbenmonro
andauthored
fix(rules): Only report prefer-to-have-value errors on accepted queries (#133)
* fix(rules): Only report prefer-to-have-value errors on accepted queries * Apply suggestions from code review Co-authored-by: Ben Monro <[email protected]>
1 parent 429e1ba commit cdbc37d

File tree

2 files changed

+33
-162
lines changed

2 files changed

+33
-162
lines changed

src/__tests__/lib/rules/prefer-to-have-value.js

+8-113
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2020 } });
2020
const errors = [{ messageId: "use-to-have-value" }];
2121
ruleTester.run("prefer-to-have-value", rule, {
2222
valid: [
23+
`expect(screen.getByRole("radio").value).toEqual("foo")`,
24+
`expect(screen.queryAllByRole("checkbox")[0].value).toStrictEqual("foo")`,
25+
`async function x() { expect((await screen.findByRole("button")).value).toBe("foo") }`,
26+
2327
`expect(element).toHaveValue('foo')`,
2428
`expect(element.value).toBeGreaterThan(2);`,
2529
`expect(element.value).toBeLessThan(2);`,
@@ -34,6 +38,10 @@ ruleTester.run("prefer-to-have-value", rule, {
3438
`const element = { value: 'foo' };
3539
expect(element.value).toBe('foo');`,
3640

41+
`expect(screen.getByRole("radio").value).not.toEqual("foo")`,
42+
`expect(screen.queryAllByRole("checkbox")[0].value).not.toStrictEqual("foo")`,
43+
`async function x() { expect((await screen.findByRole("button")).value).not.toBe("foo") }`,
44+
3745
`const element = document.getElementById('asdfasf');
3846
expect(element.value).not.toEqual('foo');`,
3947

@@ -106,118 +114,5 @@ ruleTester.run("prefer-to-have-value", rule, {
106114
errors,
107115
output: `const element = screen.getByRole("textbox"); expect(element).not.toHaveValue("foo");`,
108116
},
109-
//==========================================================================
110-
{
111-
code: `expect(screen.getByTestId('bananas').value).toEqual('foo')`,
112-
errors: [
113-
{
114-
...errors[0],
115-
suggestions: [
116-
{
117-
desc: "Replace toEqual with toHaveValue",
118-
output: `expect(screen.getByTestId('bananas')).toHaveValue('foo')`,
119-
},
120-
],
121-
},
122-
],
123-
},
124-
{
125-
code: `expect(screen.queryByTestId('bananas').value).toBe('foo')`,
126-
errors: [
127-
{
128-
...errors[0],
129-
suggestions: [
130-
{
131-
desc: "Replace toBe with toHaveValue",
132-
output: `expect(screen.queryByTestId('bananas')).toHaveValue('foo')`,
133-
},
134-
],
135-
},
136-
],
137-
},
138-
{
139-
code: `async function x() { expect((await screen.findByTestId("bananas")).value).toStrictEqual("foo") }`,
140-
errors: [
141-
{
142-
...errors[0],
143-
suggestions: [
144-
{
145-
desc: "Replace toStrictEqual with toHaveValue",
146-
output: `async function x() { expect((await screen.findByTestId("bananas"))).toHaveValue("foo") }`,
147-
},
148-
],
149-
},
150-
],
151-
},
152-
{
153-
code: `let element; element = screen.getByTestId('bananas'); expect(element.value).toEqual('foo');`,
154-
errors: [
155-
{
156-
...errors[0],
157-
suggestions: [
158-
{
159-
desc: "Replace toEqual with toHaveValue",
160-
output: `let element; element = screen.getByTestId('bananas'); expect(element).toHaveValue('foo');`,
161-
},
162-
],
163-
},
164-
],
165-
},
166-
{
167-
code: `expect(screen.getByTestId('bananas').value).not.toEqual('foo')`,
168-
errors: [
169-
{
170-
...errors[0],
171-
suggestions: [
172-
{
173-
desc: "Replace toEqual with toHaveValue",
174-
output: `expect(screen.getByTestId('bananas')).not.toHaveValue('foo')`,
175-
},
176-
],
177-
},
178-
],
179-
},
180-
{
181-
code: `expect(screen.queryByTestId('bananas').value).not.toBe('foo')`,
182-
errors: [
183-
{
184-
...errors[0],
185-
suggestions: [
186-
{
187-
desc: "Replace toBe with toHaveValue",
188-
output: `expect(screen.queryByTestId('bananas')).not.toHaveValue('foo')`,
189-
},
190-
],
191-
},
192-
],
193-
},
194-
{
195-
code: `async function x() { expect((await screen.findByTestId("bananas")).value).not.toStrictEqual("foo") }`,
196-
errors: [
197-
{
198-
...errors[0],
199-
suggestions: [
200-
{
201-
desc: "Replace toStrictEqual with toHaveValue",
202-
output: `async function x() { expect((await screen.findByTestId("bananas"))).not.toHaveValue("foo") }`,
203-
},
204-
],
205-
},
206-
],
207-
},
208-
{
209-
code: `let element; element = screen.getByTestId('bananas'); expect(element.value).not.toEqual('foo');`,
210-
errors: [
211-
{
212-
...errors[0],
213-
suggestions: [
214-
{
215-
desc: "Replace toEqual with toHaveValue",
216-
output: `let element; element = screen.getByTestId('bananas'); expect(element).not.toHaveValue('foo');`,
217-
},
218-
],
219-
},
220-
],
221-
},
222117
],
223118
});

src/rules/prefer-to-have-value.js

+25-49
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,17 @@ export const meta = {
2424
const messageId = "use-to-have-value";
2525

2626
export const create = (context) => {
27-
function validateQueryNode(nodeWithValueProp) {
27+
function isValidQueryNode(nodeWithValueProp) {
2828
const { query, queryArg, isDTLQuery } = getQueryNodeFrom(
2929
context,
3030
nodeWithValueProp
3131
);
32-
if (!query) {
33-
return {
34-
isDTLQuery: false,
35-
isValidElement: false,
36-
};
37-
}
38-
const isValidElement =
39-
query.match(/^(get|find|query)ByRole$/) &&
40-
["textbox", "dropdown"].includes(queryArg);
41-
return { isDTLQuery, isValidElement };
32+
return (
33+
!!query &&
34+
isDTLQuery &&
35+
!!query.match(/^(get|find|query)(All)?ByRole$/) &&
36+
["textbox", "dropdown"].includes(queryArg)
37+
);
4238
}
4339
return {
4440
// expect(element.value).toBe('foo') / toEqual / toStrictEqual
@@ -50,28 +46,18 @@ export const create = (context) => {
5046
const valueProp = node.callee.object.arguments[0].property;
5147
const matcher = node.callee.property;
5248
const queryNode = node.callee.object.arguments[0].object;
53-
const { isDTLQuery, isValidElement } = validateQueryNode(queryNode);
5449

55-
function fix(fixer) {
56-
return [
57-
fixer.remove(context.getSourceCode().getTokenBefore(valueProp)),
58-
fixer.remove(valueProp),
59-
fixer.replaceText(matcher, "toHaveValue"),
60-
];
61-
}
62-
if (isDTLQuery) {
50+
if (isValidQueryNode(queryNode)) {
6351
context.report({
6452
messageId,
6553
node,
66-
fix: isValidElement ? fix : undefined,
67-
suggest: isValidElement
68-
? undefined
69-
: [
70-
{
71-
desc: `Replace ${matcher.name} with toHaveValue`,
72-
fix,
73-
},
74-
],
54+
fix(fixer) {
55+
return [
56+
fixer.remove(context.getSourceCode().getTokenBefore(valueProp)),
57+
fixer.remove(valueProp),
58+
fixer.replaceText(matcher, "toHaveValue"),
59+
];
60+
},
7561
});
7662
}
7763
},
@@ -86,29 +72,19 @@ export const create = (context) => {
8672
const valueProp = node.callee.object.object.arguments[0].property;
8773
const matcher = node.callee.property;
8874

89-
const { isDTLQuery, isValidElement } = validateQueryNode(queryNode);
90-
function fix(fixer) {
91-
return [
92-
fixer.removeRange([
93-
context.getSourceCode().getTokenBefore(valueProp).range[0],
94-
valueProp.range[1],
95-
]),
96-
fixer.replaceText(matcher, "toHaveValue"),
97-
];
98-
}
99-
if (isDTLQuery) {
75+
if (isValidQueryNode(queryNode)) {
10076
context.report({
10177
messageId,
10278
node,
103-
fix: isValidElement ? fix : undefined,
104-
suggest: isValidElement
105-
? undefined
106-
: [
107-
{
108-
desc: `Replace ${matcher.name} with toHaveValue`,
109-
fix,
110-
},
111-
],
79+
fix(fixer) {
80+
return [
81+
fixer.removeRange([
82+
context.getSourceCode().getTokenBefore(valueProp).range[0],
83+
valueProp.range[1],
84+
]),
85+
fixer.replaceText(matcher, "toHaveValue"),
86+
];
87+
},
11288
});
11389
}
11490
},

0 commit comments

Comments
 (0)