Skip to content
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

Fix prefer-to-have-count rule for expect arguments that need dereferencing #292

Merged
merged 5 commits into from
May 17, 2024
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
77 changes: 77 additions & 0 deletions src/rules/prefer-to-have-count.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,83 @@ runRuleTester('prefer-to-have-count', rule, {
},
},
},
{
code: `
const filesCount = await files.count();
expect(filesCount).toBe(1)
`,
errors: [
{ column: 28, endColumn: 32, line: 3, messageId: 'useToHaveCount' },
],
output: `
const filesCount = files;
await expect(filesCount).toHaveCount(1)
`,
},
{
code: `
const filesCount = await files.count();
const unrelatedConst = unrelated;
expect(filesCount).toBe(1)
`,
errors: [
{ column: 28, endColumn: 32, line: 4, messageId: 'useToHaveCount' },
],
output: `
const filesCount = files;
const unrelatedConst = unrelated;
await expect(filesCount).toHaveCount(1)
`,
},
{
code: `
let filesCount = 3;
filesCount = await files.count();
expect(filesCount).toBe(1);
`,
errors: [
{ column: 28, endColumn: 32, line: 4, messageId: 'useToHaveCount' },
],
output: `
let filesCount = 3;
filesCount = files;
await expect(filesCount).toHaveCount(1);
`,
},
{
code: `
let filesCount = 3;
filesCount = await files.count();
let unrelatedVar = unrelated;
expect(filesCount).toBe(1);
`,
errors: [
{ column: 28, endColumn: 32, line: 5, messageId: 'useToHaveCount' },
],
output: `
let filesCount = 3;
filesCount = files;
let unrelatedVar = unrelated;
await expect(filesCount).toHaveCount(1);
`,
},
{
code: `
let filesCount = 3;
filesCount = await files.count();
expect(filesCount).toBe(1);
filesCount = 0;
`,
errors: [
{ column: 28, endColumn: 32, line: 4, messageId: 'useToHaveCount' },
],
output: `
let filesCount = 3;
filesCount = files;
await expect(filesCount).toHaveCount(1);
filesCount = 0;
`,
},
],
valid: [
{ code: 'await expect(files).toHaveCount(1)' },
Expand Down
4 changes: 2 additions & 2 deletions src/rules/prefer-to-have-count.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { equalityMatchers, isPropertyAccessor } from '../utils/ast'
import { dereference, equalityMatchers, isPropertyAccessor } from '../utils/ast'
import { createRule } from '../utils/createRule'
import { replaceAccessorFixer } from '../utils/fixer'
import { parseFnCall } from '../utils/parseFnCall'
Expand All @@ -15,7 +15,7 @@ export default createRule({
return
}

const [argument] = call.args
const argument = dereference(context, call.args[0])
if (
argument?.type !== 'AwaitExpression' ||
argument.argument.type !== 'CallExpression' ||
Expand Down
75 changes: 1 addition & 74 deletions src/rules/prefer-web-first-assertions.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import { Rule } from 'eslint'
import ESTree, { AssignmentExpression } from 'estree'
import {
dereference,
findParent,
getRawValue,
getStringValue,
isBooleanLiteral,
} from '../utils/ast'
import { createRule } from '../utils/createRule'
import { parseFnCall } from '../utils/parseFnCall'
import { TypedNodeWithParent } from '../utils/types'

type MethodConfig = {
inverse?: string
Expand Down Expand Up @@ -60,77 +58,6 @@ const supportedMatchers = new Set([
'toBeFalsy',
])

const isVariableDeclarator = (
node: ESTree.Node,
): node is TypedNodeWithParent<'VariableDeclarator'> =>
node.type === 'VariableDeclarator'

const isAssignmentExpression = (
node: ESTree.Node,
): node is TypedNodeWithParent<'AssignmentExpression'> =>
node.type === 'AssignmentExpression'

/**
* Given a Node and an assignment expression, finds out if the assignment
* expression happens before the node identifier (based on their range
* properties) and if the assignment expression left side is of the same name as
* the name of the given node.
*
* @param node The node we are comparing the assignment expression to.
* @param assignment The assignment that will be verified to see if its left
* operand is the same as the node.name and if it happens before it.
* @returns True if the assignment left hand operator belongs to the node and
* occurs before it, false otherwise. If either the node or the assignment
* expression doesn't contain a range array, this will also return false
* because their relative positions cannot be calculated.
*/
function isNodeLastAssignment(
node: ESTree.Identifier,
assignment: AssignmentExpression,
) {
if (node.range && assignment.range && node.range[0] < assignment.range[1]) {
return false
}

return (
assignment.left.type === 'Identifier' && assignment.left.name === node.name
)
}

/**
* If the expect call argument is a variable reference, finds the variable
* initializer or last variable assignment.
*
* If a variable is assigned after initialization we have to look for the last
* time it was assigned because it could have been changed multiple times. We
* then use its right hand assignment operator as the dereferenced node.
*/
function dereference(context: Rule.RuleContext, node: ESTree.Node | undefined) {
if (node?.type !== 'Identifier') {
return node
}

const scope = context.sourceCode.getScope(node)
const parents = scope.references
.map((ref) => ref.identifier as Rule.Node)
.map((ident) => ident.parent)

// Look for any variable declarators in the scope references that match the
// dereferenced node variable name
const decl = parents
.filter(isVariableDeclarator)
.find((p) => p.id.type === 'Identifier' && p.id.name === node.name)

// Look for any variable assignments in the scope references and pick the last
// one that matches the dereferenced node variable name
const expr = parents
.filter(isAssignmentExpression)
.reverse()
.find((assignment) => isNodeLastAssignment(node, assignment))

return expr?.right ?? decl?.init
}

export default createRule({
create(context) {
return {
Expand Down
87 changes: 86 additions & 1 deletion src/utils/ast.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Rule } from 'eslint'
import ESTree from 'estree'
import ESTree, { AssignmentExpression } from 'estree'
import { isSupportedAccessor } from './parseFnCall'
import { NodeWithParent, TypedNodeWithParent } from './types'

Expand Down Expand Up @@ -158,3 +158,88 @@ export function getNodeName(node: ESTree.Node): string | null {

return null
}

const isVariableDeclarator = (
node: ESTree.Node,
): node is TypedNodeWithParent<'VariableDeclarator'> =>
node.type === 'VariableDeclarator'

const isAssignmentExpression = (
node: ESTree.Node,
): node is TypedNodeWithParent<'AssignmentExpression'> =>
node.type === 'AssignmentExpression'

/**
* Given a Node and an assignment expression, finds out if the assignment
* expression happens before the node identifier (based on their range
* properties) and if the assignment expression left side is of the same name as
* the name of the given node.
*
* @param node The node we are comparing the assignment expression to.
* @param assignment The assignment that will be verified to see if its left
* operand is the same as the node.name and if it happens before it.
* @returns True if the assignment left hand operator belongs to the node and
* occurs before it, false otherwise. If either the node or the assignment
* expression doesn't contain a range array, this will also return false
* because their relative positions cannot be calculated.
*/
function isNodeLastAssignment(
node: ESTree.Identifier,
assignment: AssignmentExpression,
) {
if (node.range && assignment.range && node.range[0] < assignment.range[1]) {
return false
}

return (
assignment.left.type === 'Identifier' && assignment.left.name === node.name
)
}

/**
* If the node argument is a variable reference, finds the variable initializer
* or last variable assignment and returns the assigned value.
*
* If a variable is assigned after initialization we have to look for the last
* time it was assigned because it could have been changed multiple times. We
* then use its right hand assignment operator as the dereferenced node.
*
* @example <caption>Dereference a `const` initialized node:</caption>
* // returns 1
* const variable = 1
* console.log(variable) // dereferenced value of the 'variable' node is 1
*
* @example <caption>Dereference a `let` re-assigned node:</caption>
* // returns 1
* let variable = 0
* variable = 1
* console.log(variable) // dereferenced value of the 'variable' node is 1
*/
export function dereference(
context: Rule.RuleContext,
node: ESTree.Node | undefined,
) {
if (node?.type !== 'Identifier') {
return node
}

const scope = context.sourceCode.getScope(node)
const parents = scope.references
.map((ref) => ref.identifier as Rule.Node)
.map((ident) => ident.parent)

// Look for any variable declarators in the scope references that match the
// dereferenced node variable name
const decl = parents
.filter(isVariableDeclarator)
.find((p) => p.id.type === 'Identifier' && p.id.name === node.name)

// Look for any variable assignments in the scope references and pick the last
// one that matches the dereferenced node variable name
const expr = parents
.filter(isAssignmentExpression)
.reverse()
.find((assignment) => isNodeLastAssignment(node, assignment))

return expr?.right ?? decl?.init
}
Loading