From d654ea13f0efe20fc94a1f6fe159918a0572bc76 Mon Sep 17 00:00:00 2001 From: Senja Jarva Date: Thu, 1 Sep 2022 16:44:32 +0300 Subject: [PATCH 1/2] fix: prefer-query-by-disappearance error location Refactor reporting an error deeper, where the get/find query variants are found. Refactor function names to convey that they also check/report the error. Update unit tests. --- lib/rules/prefer-query-by-disappearance.ts | 44 ++++--- .../prefer-query-by-disappearance.test.ts | 110 +++++++++--------- 2 files changed, 81 insertions(+), 73 deletions(-) diff --git a/lib/rules/prefer-query-by-disappearance.ts b/lib/rules/prefer-query-by-disappearance.ts index 1819656c..ad4a5503 100644 --- a/lib/rules/prefer-query-by-disappearance.ts +++ b/lib/rules/prefer-query-by-disappearance.ts @@ -50,7 +50,15 @@ export default createTestingLibraryRule({ return helpers.isAsyncUtil(identifierNode, ['waitForElementToBeRemoved']); } - function isReportableExpression(node: TSESTree.LeftHandSideExpression) { + /** + * Checks if a node is a query node and starts with "get" or "find". + * + * @param {TSESTree.LeftHandSideExpression} node - Node to be tested + * @returns {Boolean} True if node should be reported and reports it with `context.report()`. False if node should not be reported. + */ + function isReportableExpression( + node: TSESTree.LeftHandSideExpression + ): boolean { const argumentProperty = isMemberExpression(node) ? getPropertyIdentifierNode(node.property) : getPropertyIdentifierNode(node); @@ -59,13 +67,20 @@ export default createTestingLibraryRule({ return false; } - return ( + if ( helpers.isGetQueryVariant(argumentProperty) || helpers.isFindQueryVariant(argumentProperty) - ); + ) { + context.report({ + node: argumentProperty, + messageId: 'preferQueryByDisappearance', + }); + return true; + } + return false; } - function isNonCallbackViolation(node: TSESTree.CallExpressionArgument) { + function checkNonCallbackViolation(node: TSESTree.CallExpressionArgument) { if (!isCallExpression(node)) { return false; } @@ -107,7 +122,7 @@ export default createTestingLibraryRule({ return isReturnViolation(statement) || isNonReturnViolation(statement); } - function isFunctionExpressionViolation( + function checkFunctionExpressionViolation( node: TSESTree.CallExpressionArgument ) { if (!isFunctionExpression(node)) { @@ -148,7 +163,9 @@ export default createTestingLibraryRule({ return isReportableExpression(node.body.callee); } - function isArrowFunctionViolation(node: TSESTree.CallExpressionArgument) { + function checkArrowFunctionViolation( + node: TSESTree.CallExpressionArgument + ) { return ( isArrowFunctionBodyViolation(node) || isArrowFunctionImplicitReturnViolation(node) @@ -162,18 +179,9 @@ export default createTestingLibraryRule({ const argumentNode = node.arguments[0]; - if ( - !isNonCallbackViolation(argumentNode) && - !isArrowFunctionViolation(argumentNode) && - !isFunctionExpressionViolation(argumentNode) - ) { - return; - } - - context.report({ - node: argumentNode, - messageId: 'preferQueryByDisappearance', - }); + checkNonCallbackViolation(argumentNode); + checkArrowFunctionViolation(argumentNode); + checkFunctionExpressionViolation(argumentNode); } return { diff --git a/tests/lib/rules/prefer-query-by-disappearance.test.ts b/tests/lib/rules/prefer-query-by-disappearance.test.ts index 010de651..f6623652 100644 --- a/tests/lib/rules/prefer-query-by-disappearance.test.ts +++ b/tests/lib/rules/prefer-query-by-disappearance.test.ts @@ -225,7 +225,7 @@ ruleTester.run(RULE_NAME, rule, { { messageId: 'preferQueryByDisappearance', line: 4, - column: 41, + column: 54, }, ], }, @@ -239,7 +239,7 @@ ruleTester.run(RULE_NAME, rule, { { messageId: 'preferQueryByDisappearance', line: 4, - column: 41, + column: 54, }, ], }, @@ -253,7 +253,7 @@ ruleTester.run(RULE_NAME, rule, { { messageId: 'preferQueryByDisappearance', line: 4, - column: 41, + column: 54, }, ], }, @@ -268,8 +268,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 4, - column: 41, + line: 5, + column: 18, }, ], }, @@ -284,8 +284,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 4, - column: 41, + line: 5, + column: 18, }, ], }, @@ -300,8 +300,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 4, - column: 41, + line: 5, + column: 25, }, ], }, @@ -316,8 +316,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 4, - column: 41, + line: 5, + column: 25, }, ], }, @@ -331,7 +331,7 @@ ruleTester.run(RULE_NAME, rule, { { messageId: 'preferQueryByDisappearance', line: 4, - column: 41, + column: 48, }, ], }, @@ -345,7 +345,7 @@ ruleTester.run(RULE_NAME, rule, { { messageId: 'preferQueryByDisappearance', line: 4, - column: 41, + column: 48, }, ], }, @@ -360,8 +360,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 4, - column: 41, + line: 5, + column: 25, }, ], }, @@ -376,8 +376,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 4, - column: 41, + line: 5, + column: 25, }, ], }, @@ -392,8 +392,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 4, - column: 41, + line: 5, + column: 18, }, ], }, @@ -408,8 +408,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 4, - column: 41, + line: 5, + column: 18, }, ], }, @@ -425,8 +425,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 11, }, ], }, @@ -442,8 +442,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 11, }, ], }, @@ -459,8 +459,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 11, }, ], }, @@ -476,8 +476,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 11, }, ], }, @@ -493,8 +493,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 18, }, ], }, @@ -510,8 +510,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 18, }, ], }, @@ -527,8 +527,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 18, }, ], }, @@ -544,8 +544,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 18, }, ], }, @@ -561,8 +561,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 18, }, ], }, @@ -578,8 +578,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 18, }, ], }, @@ -595,8 +595,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 18, }, ], }, @@ -612,8 +612,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 18, }, ], }, @@ -629,8 +629,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 11, }, ], }, @@ -646,8 +646,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 11, }, ], }, @@ -663,8 +663,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 11, }, ], }, @@ -680,8 +680,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferQueryByDisappearance', - line: 5, - column: 41, + line: 6, + column: 11, }, ], }, @@ -696,7 +696,7 @@ ruleTester.run(RULE_NAME, rule, { { messageId: 'preferQueryByDisappearance', line: 5, - column: 41, + column: 47, }, ], }, @@ -711,7 +711,7 @@ ruleTester.run(RULE_NAME, rule, { { messageId: 'preferQueryByDisappearance', line: 5, - column: 41, + column: 47, }, ], }, From 6fcbd195a01ed8f3388bc16b99842423c13adeeb Mon Sep 17 00:00:00 2001 From: Senja Jarva Date: Sat, 3 Sep 2022 21:12:15 +0300 Subject: [PATCH 2/2] fix: review feedback Make function name and JSDoc comment more clear --- lib/rules/prefer-query-by-disappearance.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/rules/prefer-query-by-disappearance.ts b/lib/rules/prefer-query-by-disappearance.ts index ad4a5503..f19b3796 100644 --- a/lib/rules/prefer-query-by-disappearance.ts +++ b/lib/rules/prefer-query-by-disappearance.ts @@ -51,14 +51,12 @@ export default createTestingLibraryRule({ } /** - * Checks if a node is a query node and starts with "get" or "find". + * Checks if node is reportable (starts with "get" or "find") and if it is, reports it with `context.report()`. * * @param {TSESTree.LeftHandSideExpression} node - Node to be tested - * @returns {Boolean} True if node should be reported and reports it with `context.report()`. False if node should not be reported. + * @returns {Boolean} Boolean indicating if expression was reported */ - function isReportableExpression( - node: TSESTree.LeftHandSideExpression - ): boolean { + function reportExpression(node: TSESTree.LeftHandSideExpression): boolean { const argumentProperty = isMemberExpression(node) ? getPropertyIdentifierNode(node.property) : getPropertyIdentifierNode(node); @@ -92,7 +90,7 @@ export default createTestingLibraryRule({ return false; } - return isReportableExpression(node.callee); + return reportExpression(node.callee); } function isReturnViolation(node: TSESTree.Statement) { @@ -100,7 +98,7 @@ export default createTestingLibraryRule({ return false; } - return isReportableExpression(node.argument.callee); + return reportExpression(node.argument.callee); } function isNonReturnViolation(node: TSESTree.Statement) { @@ -115,7 +113,7 @@ export default createTestingLibraryRule({ return false; } - return isReportableExpression(node.expression.callee); + return reportExpression(node.expression.callee); } function isStatementViolation(statement: TSESTree.Statement) { @@ -160,7 +158,7 @@ export default createTestingLibraryRule({ return false; } - return isReportableExpression(node.body.callee); + return reportExpression(node.body.callee); } function checkArrowFunctionViolation(