Skip to content

Commit 56afe4f

Browse files
juzerzarifbenmonro
juzerzarif
and
benmonro
authored
fix(rules): only report errors for prefer-to-have-value on valid DTL queries (#122)
Co-authored-by: Ben Monro <[email protected]>
1 parent a912e68 commit 56afe4f

7 files changed

+344
-117
lines changed

‎CODE_OF_CONDUCT.md

+13-13
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,22 @@ appearance, race, religion, or sexual identity and orientation.
1414
Examples of behavior that contributes to creating a positive environment
1515
include:
1616

17-
* Using welcoming and inclusive language
18-
* Being respectful of differing viewpoints and experiences
19-
* Gracefully accepting constructive criticism
20-
* Focusing on what is best for the community
21-
* Showing empathy towards other community members
17+
- Using welcoming and inclusive language
18+
- Being respectful of differing viewpoints and experiences
19+
- Gracefully accepting constructive criticism
20+
- Focusing on what is best for the community
21+
- Showing empathy towards other community members
2222

2323
Examples of unacceptable behavior by participants include:
2424

25-
* The use of sexualized language or imagery and unwelcome sexual attention or
26-
advances
27-
* Trolling, insulting/derogatory comments, and personal or political attacks
28-
* Public or private harassment
29-
* Publishing others' private information, such as a physical or electronic
30-
address, without explicit permission
31-
* Other conduct which could reasonably be considered inappropriate in a
32-
professional setting
25+
- The use of sexualized language or imagery and unwelcome sexual attention or
26+
advances
27+
- Trolling, insulting/derogatory comments, and personal or political attacks
28+
- Public or private harassment
29+
- Publishing others' private information, such as a physical or electronic
30+
address, without explicit permission
31+
- Other conduct which could reasonably be considered inappropriate in a
32+
professional setting
3333

3434
## Our Responsibilities
3535

‎src/__tests__/lib/rules/prefer-in-document.js

+3
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ const valid = [
4343
foo = "bar";
4444
expect(foo).toHaveLength(0);`,
4545
`let foo;
46+
foo = bar;
47+
expect(foo).not.toHaveLength(0)`,
48+
`let foo;
4649
expect(foo).toHaveLength(1);`,
4750
`expect(screen.notAQuery('foo-bar')).toHaveLength(1)`,
4851
`expect(screen.getAllByText('foo-bar')).toHaveLength(2)`,

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

+158-14
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,34 @@ import * as rule from "../../../rules/prefer-to-have-value";
1515
// Tests
1616
//------------------------------------------------------------------------------
1717

18-
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2015 } });
18+
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2020 } });
1919

2020
const errors = [{ messageId: "use-to-have-value" }];
21-
ruleTester.run("prefer-empty", rule, {
21+
ruleTester.run("prefer-to-have-value", rule, {
2222
valid: [
2323
`expect(element).toHaveValue('foo')`,
2424
`expect(element.value).toBeGreaterThan(2);`,
2525
`expect(element.value).toBeLessThan(2);`,
26+
27+
`const element = document.getElementById('asdfasf');
28+
expect(element.value).toEqual('foo');`,
29+
30+
`let element;
31+
element = someOtherFunction();
32+
expect(element.value).toStrictEqual('foo');`,
33+
34+
`const element = { value: 'foo' };
35+
expect(element.value).toBe('foo');`,
36+
37+
`const element = document.getElementById('asdfasf');
38+
expect(element.value).not.toEqual('foo');`,
39+
40+
`let element;
41+
element = someOtherFunction();
42+
expect(element.value).not.toStrictEqual('foo');`,
43+
44+
`const element = { value: 'foo' };
45+
expect(element.value).not.toBe('foo');`,
2646
],
2747
invalid: [
2848
{
@@ -45,35 +65,159 @@ ruleTester.run("prefer-empty", rule, {
4565
errors,
4666
output: `expect(element).not.toHaveValue("foo")`,
4767
},
68+
//==========================================================================
4869
{
49-
code: `expect(element.value).toBe('foo')`,
70+
code: `expect(screen.getByRole("textbox").value).toEqual("foo")`,
5071
errors,
51-
output: `expect(element).toHaveValue('foo')`,
72+
output: `expect(screen.getByRole("textbox")).toHaveValue("foo")`,
5273
},
5374
{
54-
code: `expect(element.value).toEqual('foo')`,
75+
code: `expect(screen.queryByRole("dropdown").value).toEqual("foo")`,
5576
errors,
56-
output: `expect(element).toHaveValue('foo')`,
77+
output: `expect(screen.queryByRole("dropdown")).toHaveValue("foo")`,
5778
},
5879
{
59-
code: `expect(element.value).toStrictEqual('foo')`,
80+
code: `async function x() { expect((await screen.findByRole("textbox")).value).toEqual("foo") }`,
6081
errors,
61-
output: `expect(element).toHaveValue('foo')`,
82+
output: `async function x() { expect((await screen.findByRole("textbox"))).toHaveValue("foo") }`,
6283
},
6384
{
64-
code: `expect(element.value).not.toBe('foo')`,
85+
code: `const element = screen.getByRole("textbox"); expect(element.value).toBe("foo");`,
6586
errors,
66-
output: `expect(element).not.toHaveValue('foo')`,
87+
output: `const element = screen.getByRole("textbox"); expect(element).toHaveValue("foo");`,
6788
},
6889
{
69-
code: `expect(element.value).not.toEqual('foo')`,
90+
code: `expect(screen.getByRole("textbox").value).not.toEqual("foo")`,
7091
errors,
71-
output: `expect(element).not.toHaveValue('foo')`,
92+
output: `expect(screen.getByRole("textbox")).not.toHaveValue("foo")`,
7293
},
7394
{
74-
code: `expect(element.value).not.toStrictEqual('foo')`,
95+
code: `expect(screen.queryByRole("dropdown").value).not.toEqual("foo")`,
7596
errors,
76-
output: `expect(element).not.toHaveValue('foo')`,
97+
output: `expect(screen.queryByRole("dropdown")).not.toHaveValue("foo")`,
98+
},
99+
{
100+
code: `async function x() { expect((await screen.getByRole("textbox")).value).not.toEqual("foo") }`,
101+
errors,
102+
output: `async function x() { expect((await screen.getByRole("textbox"))).not.toHaveValue("foo") }`,
103+
},
104+
{
105+
code: `const element = screen.getByRole("textbox"); expect(element.value).not.toBe("foo");`,
106+
errors,
107+
output: `const element = screen.getByRole("textbox"); expect(element).not.toHaveValue("foo");`,
108+
},
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+
],
77221
},
78222
],
79223
});

‎src/assignment-ast.js

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/**
2+
* Gets the inner relevant node (CallExpression, Identity, et al.) given a generic expression node
3+
* await someAsyncFunc() => someAsyncFunc()
4+
* someElement as HTMLDivElement => someElement
5+
*
6+
* @param {Object} expression - An expression node
7+
* @returns {Object} - A node
8+
*/
9+
export function getInnerNodeFrom(expression) {
10+
return expression.type === "TSAsExpression"
11+
? getInnerNodeFrom(expression.expression)
12+
: expression.type === "AwaitExpression"
13+
? getInnerNodeFrom(expression.argument)
14+
: expression;
15+
}
16+
17+
/**
18+
* Get the node corresponding to the latest assignment to a variable named `identifierName`
19+
*
20+
* @param {Object} context - Context for a rule
21+
* @param {String} identifierName - Name of an identifier
22+
* @returns {Object} - A node, possibly undefined
23+
*/
24+
export function getAssignmentForIdentifier(context, identifierName) {
25+
const variable = context.getScope().set.get(identifierName);
26+
27+
if (!variable) return;
28+
const init = variable.defs[0].node.init;
29+
30+
let assignmentNode;
31+
if (init) {
32+
// let foo = bar;
33+
assignmentNode = getInnerNodeFrom(init);
34+
} else {
35+
// let foo;
36+
// foo = bar;
37+
const assignmentRef = variable.references
38+
.reverse()
39+
.find((ref) => !!ref.writeExpr);
40+
if (!assignmentRef) {
41+
return;
42+
}
43+
assignmentNode = getInnerNodeFrom(assignmentRef.writeExpr);
44+
}
45+
return assignmentNode;
46+
}

‎src/rules/prefer-in-document.js

+9-38
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
import { queries } from "../queries";
7+
import { getAssignmentForIdentifier } from "../assignment-ast";
78

89
export const meta = {
910
type: "suggestion",
@@ -37,7 +38,10 @@ export const create = (context) => {
3738
let lengthValue;
3839

3940
if (matcherArguments[0].type === "Identifier") {
40-
const assignment = getAssignmentForIdentifier(matcherArguments[0].name);
41+
const assignment = getAssignmentForIdentifier(
42+
context,
43+
matcherArguments[0].name
44+
);
4145
if (!assignment) {
4246
return;
4347
}
@@ -122,39 +126,6 @@ export const create = (context) => {
122126
}
123127
}
124128

125-
function getAssignmentFrom(expression) {
126-
return expression.type === "TSAsExpression"
127-
? getAssignmentFrom(expression.expression)
128-
: expression.type === "AwaitExpression"
129-
? getAssignmentFrom(expression.argument)
130-
: expression.callee
131-
? expression.callee
132-
: expression;
133-
}
134-
135-
function getAssignmentForIdentifier(identifierName) {
136-
const variable = context.getScope().set.get(identifierName);
137-
138-
if (!variable) return;
139-
const init = variable.defs[0].node.init;
140-
141-
let assignmentNode;
142-
if (init) {
143-
// let foo = bar;
144-
assignmentNode = getAssignmentFrom(init);
145-
} else {
146-
// let foo;
147-
// foo = bar;
148-
const assignmentRef = variable.references
149-
.reverse()
150-
.find((ref) => !!ref.writeExpr);
151-
if (!assignmentRef) {
152-
return;
153-
}
154-
assignmentNode = getAssignmentFrom(assignmentRef.writeExpr);
155-
}
156-
return assignmentNode;
157-
}
158129
return {
159130
// expect(<query>).not.<matcher>
160131
[`CallExpression[callee.object.object.callee.name='expect'][callee.object.property.name='not'][callee.property.name=${alternativeMatchers}], CallExpression[callee.object.callee.name='expect'][callee.object.property.name='not'][callee.object.arguments.0.argument.callee.name=${alternativeMatchers}]`](
@@ -180,17 +151,17 @@ export const create = (context) => {
180151
node
181152
) {
182153
const queryNode = getAssignmentForIdentifier(
154+
context,
183155
node.object.object.arguments[0].name
184156
);
185157
const matcherNode = node.property;
186158

187159
const matcherArguments = node.parent.arguments;
188160

189161
const expect = node.object.object;
190-
191162
check({
192163
negatedMatcher: true,
193-
queryNode,
164+
queryNode: (queryNode && queryNode.callee) || queryNode,
194165
matcherNode,
195166
matcherArguments,
196167
expect,
@@ -201,15 +172,15 @@ export const create = (context) => {
201172
node
202173
) {
203174
const queryNode = getAssignmentForIdentifier(
175+
context,
204176
node.object.arguments[0].name
205177
);
206178
const matcherNode = node.property;
207179

208180
const matcherArguments = node.parent.arguments;
209-
210181
check({
211182
negatedMatcher: false,
212-
queryNode,
183+
queryNode: (queryNode && queryNode.callee) || queryNode,
213184
matcherNode,
214185
matcherArguments,
215186
});

0 commit comments

Comments
 (0)