Skip to content

fix(rules): only report errors for prefer-to-have-value on valid DTL queries #122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions CODE_OF_CONDUCT.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,22 @@ appearance, race, religion, or sexual identity and orientation.
Examples of behavior that contributes to creating a positive environment
include:

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

Examples of unacceptable behavior by participants include:

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

## Our Responsibilities

Expand Down
3 changes: 3 additions & 0 deletions src/__tests__/lib/rules/prefer-in-document.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const valid = [
foo = "bar";
expect(foo).toHaveLength(0);`,
`let foo;
foo = bar;
expect(foo).not.toHaveLength(0)`,
`let foo;
expect(foo).toHaveLength(1);`,
`expect(screen.notAQuery('foo-bar')).toHaveLength(1)`,
`expect(screen.getAllByText('foo-bar')).toHaveLength(2)`,
Expand Down
172 changes: 158 additions & 14 deletions src/__tests__/lib/rules/prefer-to-have-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,34 @@ import * as rule from "../../../rules/prefer-to-have-value";
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2015 } });
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2020 } });

const errors = [{ messageId: "use-to-have-value" }];
ruleTester.run("prefer-empty", rule, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow thanks, that's what I get for copy pasta. :)

ruleTester.run("prefer-to-have-value", rule, {
valid: [
`expect(element).toHaveValue('foo')`,
`expect(element.value).toBeGreaterThan(2);`,
`expect(element.value).toBeLessThan(2);`,

`const element = document.getElementById('asdfasf');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now this is fine, but it would be nice to at some point support document queries as well as RTL. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, my initial idea was to create a list of all the DOM api query functions but then it occurred to me that you can also get a reference to a dom element via a lot of other keys like first/lastElementChild, children etc. so I figured probably best to leave that to be implemented as its own piece

expect(element.value).toEqual('foo');`,

`let element;
element = someOtherFunction();
expect(element.value).toStrictEqual('foo');`,

`const element = { value: 'foo' };
expect(element.value).toBe('foo');`,

`const element = document.getElementById('asdfasf');
expect(element.value).not.toEqual('foo');`,

`let element;
element = someOtherFunction();
expect(element.value).not.toStrictEqual('foo');`,

`const element = { value: 'foo' };
expect(element.value).not.toBe('foo');`,
],
invalid: [
{
Expand All @@ -45,35 +65,159 @@ ruleTester.run("prefer-empty", rule, {
errors,
output: `expect(element).not.toHaveValue("foo")`,
},
//==========================================================================
{
code: `expect(element.value).toBe('foo')`,
code: `expect(screen.getByRole("textbox").value).toEqual("foo")`,
errors,
output: `expect(element).toHaveValue('foo')`,
output: `expect(screen.getByRole("textbox")).toHaveValue("foo")`,
},
{
code: `expect(element.value).toEqual('foo')`,
code: `expect(screen.queryByRole("dropdown").value).toEqual("foo")`,
errors,
output: `expect(element).toHaveValue('foo')`,
output: `expect(screen.queryByRole("dropdown")).toHaveValue("foo")`,
},
{
code: `expect(element.value).toStrictEqual('foo')`,
code: `async function x() { expect((await screen.findByRole("textbox")).value).toEqual("foo") }`,
errors,
output: `expect(element).toHaveValue('foo')`,
output: `async function x() { expect((await screen.findByRole("textbox"))).toHaveValue("foo") }`,
},
{
code: `expect(element.value).not.toBe('foo')`,
code: `const element = screen.getByRole("textbox"); expect(element.value).toBe("foo");`,
errors,
output: `expect(element).not.toHaveValue('foo')`,
output: `const element = screen.getByRole("textbox"); expect(element).toHaveValue("foo");`,
},
{
code: `expect(element.value).not.toEqual('foo')`,
code: `expect(screen.getByRole("textbox").value).not.toEqual("foo")`,
errors,
output: `expect(element).not.toHaveValue('foo')`,
output: `expect(screen.getByRole("textbox")).not.toHaveValue("foo")`,
},
{
code: `expect(element.value).not.toStrictEqual('foo')`,
code: `expect(screen.queryByRole("dropdown").value).not.toEqual("foo")`,
errors,
output: `expect(element).not.toHaveValue('foo')`,
output: `expect(screen.queryByRole("dropdown")).not.toHaveValue("foo")`,
},
{
code: `async function x() { expect((await screen.getByRole("textbox")).value).not.toEqual("foo") }`,
errors,
output: `async function x() { expect((await screen.getByRole("textbox"))).not.toHaveValue("foo") }`,
},
{
code: `const element = screen.getByRole("textbox"); expect(element.value).not.toBe("foo");`,
errors,
output: `const element = screen.getByRole("textbox"); expect(element).not.toHaveValue("foo");`,
},
//==========================================================================
{
code: `expect(screen.getByTestId('bananas').value).toEqual('foo')`,
errors: [
{
...errors[0],
suggestions: [
{
desc: "Replace toEqual with toHaveValue",
output: `expect(screen.getByTestId('bananas')).toHaveValue('foo')`,
},
],
},
],
},
{
code: `expect(screen.queryByTestId('bananas').value).toBe('foo')`,
errors: [
{
...errors[0],
suggestions: [
{
desc: "Replace toBe with toHaveValue",
output: `expect(screen.queryByTestId('bananas')).toHaveValue('foo')`,
},
],
},
],
},
{
code: `async function x() { expect((await screen.findByTestId("bananas")).value).toStrictEqual("foo") }`,
errors: [
{
...errors[0],
suggestions: [
{
desc: "Replace toStrictEqual with toHaveValue",
output: `async function x() { expect((await screen.findByTestId("bananas"))).toHaveValue("foo") }`,
},
],
},
],
},
{
code: `let element; element = screen.getByTestId('bananas'); expect(element.value).toEqual('foo');`,
errors: [
{
...errors[0],
suggestions: [
{
desc: "Replace toEqual with toHaveValue",
output: `let element; element = screen.getByTestId('bananas'); expect(element).toHaveValue('foo');`,
},
],
},
],
},
{
code: `expect(screen.getByTestId('bananas').value).not.toEqual('foo')`,
errors: [
{
...errors[0],
suggestions: [
{
desc: "Replace toEqual with toHaveValue",
output: `expect(screen.getByTestId('bananas')).not.toHaveValue('foo')`,
},
],
},
],
},
{
code: `expect(screen.queryByTestId('bananas').value).not.toBe('foo')`,
errors: [
{
...errors[0],
suggestions: [
{
desc: "Replace toBe with toHaveValue",
output: `expect(screen.queryByTestId('bananas')).not.toHaveValue('foo')`,
},
],
},
],
},
{
code: `async function x() { expect((await screen.findByTestId("bananas")).value).not.toStrictEqual("foo") }`,
errors: [
{
...errors[0],
suggestions: [
{
desc: "Replace toStrictEqual with toHaveValue",
output: `async function x() { expect((await screen.findByTestId("bananas"))).not.toHaveValue("foo") }`,
},
],
},
],
},
{
code: `let element; element = screen.getByTestId('bananas'); expect(element.value).not.toEqual('foo');`,
errors: [
{
...errors[0],
suggestions: [
{
desc: "Replace toEqual with toHaveValue",
output: `let element; element = screen.getByTestId('bananas'); expect(element).not.toHaveValue('foo');`,
},
],
},
],
},
],
});
46 changes: 46 additions & 0 deletions src/assignment-ast.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* Gets the inner relevant node (CallExpression, Identity, et al.) given a generic expression node
* await someAsyncFunc() => someAsyncFunc()
* someElement as HTMLDivElement => someElement
*
* @param {Object} expression - An expression node
* @returns {Object} - A node
*/
export function getInnerNodeFrom(expression) {
return expression.type === "TSAsExpression"
? getInnerNodeFrom(expression.expression)
: expression.type === "AwaitExpression"
? getInnerNodeFrom(expression.argument)
: expression;
}

/**
* Get the node corresponding to the latest assignment to a variable named `identifierName`
*
* @param {Object} context - Context for a rule
* @param {String} identifierName - Name of an identifier
* @returns {Object} - A node, possibly undefined
*/
export function getAssignmentForIdentifier(context, identifierName) {
const variable = context.getScope().set.get(identifierName);

if (!variable) return;
const init = variable.defs[0].node.init;

let assignmentNode;
if (init) {
// let foo = bar;
assignmentNode = getInnerNodeFrom(init);
} else {
// let foo;
// foo = bar;
const assignmentRef = variable.references
.reverse()
.find((ref) => !!ref.writeExpr);
if (!assignmentRef) {
return;
}
assignmentNode = getInnerNodeFrom(assignmentRef.writeExpr);
}
return assignmentNode;
}
47 changes: 9 additions & 38 deletions src/rules/prefer-in-document.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import { queries } from "../queries";
import { getAssignmentForIdentifier } from "../assignment-ast";

export const meta = {
type: "suggestion",
Expand Down Expand Up @@ -37,7 +38,10 @@ export const create = (context) => {
let lengthValue;

if (matcherArguments[0].type === "Identifier") {
const assignment = getAssignmentForIdentifier(matcherArguments[0].name);
const assignment = getAssignmentForIdentifier(
context,
matcherArguments[0].name
);
if (!assignment) {
return;
}
Expand Down Expand Up @@ -122,39 +126,6 @@ export const create = (context) => {
}
}

function getAssignmentFrom(expression) {
return expression.type === "TSAsExpression"
? getAssignmentFrom(expression.expression)
: expression.type === "AwaitExpression"
? getAssignmentFrom(expression.argument)
: expression.callee
? expression.callee
: expression;
}

function getAssignmentForIdentifier(identifierName) {
const variable = context.getScope().set.get(identifierName);

if (!variable) return;
const init = variable.defs[0].node.init;

let assignmentNode;
if (init) {
// let foo = bar;
assignmentNode = getAssignmentFrom(init);
} else {
// let foo;
// foo = bar;
const assignmentRef = variable.references
.reverse()
.find((ref) => !!ref.writeExpr);
if (!assignmentRef) {
return;
}
assignmentNode = getAssignmentFrom(assignmentRef.writeExpr);
}
return assignmentNode;
}
return {
// expect(<query>).not.<matcher>
[`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}]`](
Expand All @@ -180,17 +151,17 @@ export const create = (context) => {
node
) {
const queryNode = getAssignmentForIdentifier(
context,
node.object.object.arguments[0].name
);
const matcherNode = node.property;

const matcherArguments = node.parent.arguments;

const expect = node.object.object;

check({
negatedMatcher: true,
queryNode,
queryNode: (queryNode && queryNode.callee) || queryNode,
matcherNode,
matcherArguments,
expect,
Expand All @@ -201,15 +172,15 @@ export const create = (context) => {
node
) {
const queryNode = getAssignmentForIdentifier(
context,
node.object.arguments[0].name
);
const matcherNode = node.property;

const matcherArguments = node.parent.arguments;

check({
negatedMatcher: false,
queryNode,
queryNode: (queryNode && queryNode.callee) || queryNode,
matcherNode,
matcherArguments,
});
Expand Down
Loading