diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index 6e3b838..9100ef2 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -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 diff --git a/src/__tests__/lib/rules/prefer-in-document.js b/src/__tests__/lib/rules/prefer-in-document.js index b13948a..30891b1 100644 --- a/src/__tests__/lib/rules/prefer-in-document.js +++ b/src/__tests__/lib/rules/prefer-in-document.js @@ -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)`, diff --git a/src/__tests__/lib/rules/prefer-to-have-value.js b/src/__tests__/lib/rules/prefer-to-have-value.js index 91dc4f8..9f8d208 100644 --- a/src/__tests__/lib/rules/prefer-to-have-value.js +++ b/src/__tests__/lib/rules/prefer-to-have-value.js @@ -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, { +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'); + 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: [ { @@ -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');`, + }, + ], + }, + ], }, ], }); diff --git a/src/assignment-ast.js b/src/assignment-ast.js new file mode 100644 index 0000000..ae87e88 --- /dev/null +++ b/src/assignment-ast.js @@ -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; +} diff --git a/src/rules/prefer-in-document.js b/src/rules/prefer-in-document.js index 8864c36..b43d78e 100644 --- a/src/rules/prefer-in-document.js +++ b/src/rules/prefer-in-document.js @@ -4,6 +4,7 @@ */ import { queries } from "../queries"; +import { getAssignmentForIdentifier } from "../assignment-ast"; export const meta = { type: "suggestion", @@ -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; } @@ -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().not. [`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,6 +151,7 @@ export const create = (context) => { node ) { const queryNode = getAssignmentForIdentifier( + context, node.object.object.arguments[0].name ); const matcherNode = node.property; @@ -187,10 +159,9 @@ export const create = (context) => { const matcherArguments = node.parent.arguments; const expect = node.object.object; - check({ negatedMatcher: true, - queryNode, + queryNode: (queryNode && queryNode.callee) || queryNode, matcherNode, matcherArguments, expect, @@ -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, }); diff --git a/src/rules/prefer-to-have-class.js b/src/rules/prefer-to-have-class.js index 493348c..1e5c4e6 100644 --- a/src/rules/prefer-to-have-class.js +++ b/src/rules/prefer-to-have-class.js @@ -48,8 +48,8 @@ export const create = (context) => ({ matcherArg, context.getSourceCode().getText(classValue) ) - : fixer.insertTextBeforeRange( - [node.range[1] - 1, node.range[1] - 1], + : fixer.insertTextBefore( + context.getSourceCode().getTokenAfter(matcher, { skip: 1 }), context.getSourceCode().getText(classValue) ), ]; diff --git a/src/rules/prefer-to-have-value.js b/src/rules/prefer-to-have-value.js index 5632a9a..6f5bd60 100644 --- a/src/rules/prefer-to-have-value.js +++ b/src/rules/prefer-to-have-value.js @@ -3,6 +3,12 @@ * @author Ben Monro */ +import { queries } from "../queries"; +import { + getInnerNodeFrom, + getAssignmentForIdentifier, +} from "../assignment-ast"; + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -21,62 +27,119 @@ export const meta = { }; const messageId = "use-to-have-value"; -export const create = (context) => ({ - // expect(element.value).toBe('foo') / toEqual / toStrictEqual - [`CallExpression[callee.property.name=/to(Be|(Strict)?Equal)$/][callee.object.arguments.0.property.name=value][callee.object.callee.name=expect]`]( - node - ) { - context.report({ - messageId, - node, - fix(fixer) { - return [ - fixer.removeRange([ - node.callee.object.arguments[0].object.range[1], - node.callee.object.arguments[0].property.range[1], - ]), - fixer.replaceText(node.callee.property, "toHaveValue"), - ]; - }, - }); - }, +export const create = (context) => { + function validateQueryNode(nodeWithValueProp) { + const queryNode = + nodeWithValueProp.type === "Identifier" + ? getAssignmentForIdentifier(context, nodeWithValueProp.name) + : getInnerNodeFrom(nodeWithValueProp); + + if (!queryNode || !queryNode.callee) { + return { + isValidQuery: false, + isValidElement: false, + }; + } + + const query = queryNode.callee.name || queryNode.callee.property.name; + const queryArg = queryNode.arguments[0] && queryNode.arguments[0].value; + const isValidQuery = queries.includes(query); + const isValidElement = + query.match(/^(get|find|query)ByRole$/) && + ["textbox", "dropdown"].includes(queryArg); + return { isValidQuery, isValidElement }; + } + return { + // expect(element.value).toBe('foo') / toEqual / toStrictEqual + // expect(.value).toBe('foo') / toEqual / toStrictEqual + // expect((await ).value).toBe('foo') / toEqual / toStrictEqual + [`CallExpression[callee.property.name=/to(Be|(Strict)?Equal)$/][callee.object.arguments.0.property.name=value][callee.object.callee.name=expect]`]( + node + ) { + const valueProp = node.callee.object.arguments[0].property; + const matcher = node.callee.property; + const queryNode = node.callee.object.arguments[0].object; + const { isValidQuery, isValidElement } = validateQueryNode(queryNode); - // expect(element.value).not.toBe('foo') / toEqual / toStrictEqual - [`CallExpression[callee.property.name=/to(Be|(Strict)?Equal)$/][callee.object.object.callee.name=expect][callee.object.property.name=not][callee.object.object.arguments.0.property.name=value]`]( - node - ) { - context.report({ - messageId, - node, - fix(fixer) { + function fix(fixer) { return [ - fixer.removeRange([ - node.callee.object.object.arguments[0].object.range[1], - node.callee.object.object.arguments[0].property.range[1], - ]), - fixer.replaceText(node.callee.property, "toHaveValue"), + fixer.remove(context.getSourceCode().getTokenBefore(valueProp)), + fixer.remove(valueProp), + fixer.replaceText(matcher, "toHaveValue"), ]; - }, - }); - }, + } + if (isValidQuery) { + context.report({ + messageId, + node, + fix: isValidElement ? fix : undefined, + suggest: isValidElement + ? undefined + : [ + { + desc: `Replace ${matcher.name} with toHaveValue`, + fix, + }, + ], + }); + } + }, - //expect(element).toHaveAttribute('value', 'foo') / Property - [`CallExpression[callee.property.name=/toHave(Attribute|Property)/][arguments.0.value=value][arguments.1][callee.object.callee.name=expect], CallExpression[callee.property.name=/toHave(Attribute|Property)/][arguments.0.value=value][arguments.1][callee.object.object.callee.name=expect][callee.object.property.name=not]`]( - node - ) { - context.report({ - messageId, - node, + // expect(element.value).not.toBe('foo') / toEqual / toStrictEqual + // expect(.value).not.toBe('foo') / toEqual / toStrictEqual + // expect((await ).value).not.toBe('foo') / toEqual / toStrictEqual + [`CallExpression[callee.property.name=/to(Be|(Strict)?Equal)$/][callee.object.object.callee.name=expect][callee.object.property.name=not][callee.object.object.arguments.0.property.name=value]`]( + node + ) { + const queryNode = node.callee.object.object.arguments[0].object; + const valueProp = node.callee.object.object.arguments[0].property; + const matcher = node.callee.property; - fix(fixer) { + const { isValidQuery, isValidElement } = validateQueryNode(queryNode); + function fix(fixer) { return [ - fixer.replaceText(node.callee.property, "toHaveValue"), fixer.removeRange([ - node.arguments[0].range[0], - node.arguments[1].range[0], + context.getSourceCode().getTokenBefore(valueProp).range[0], + valueProp.range[1], ]), + fixer.replaceText(matcher, "toHaveValue"), ]; - }, - }); - }, -}); + } + if (isValidQuery) { + context.report({ + messageId, + node, + fix: isValidElement ? fix : undefined, + suggest: isValidElement + ? undefined + : [ + { + desc: `Replace ${matcher.name} with toHaveValue`, + fix, + }, + ], + }); + } + }, + + //expect(element).toHaveAttribute('value', 'foo') / Property + [`CallExpression[callee.property.name=/toHave(Attribute|Property)/][arguments.0.value=value][arguments.1][callee.object.callee.name=expect], CallExpression[callee.property.name=/toHave(Attribute|Property)/][arguments.0.value=value][arguments.1][callee.object.object.callee.name=expect][callee.object.property.name=not]`]( + node + ) { + const matcher = node.callee.property; + const [prop, value] = node.arguments; + + context.report({ + messageId, + node, + + fix(fixer) { + return [ + fixer.replaceText(matcher, "toHaveValue"), + fixer.removeRange([prop.range[0], value.range[0]]), + ]; + }, + }); + }, + }; +};