From 1bb11394d018baa3e23c54ce07bd2b225322f95d Mon Sep 17 00:00:00 2001 From: Alexander Afanasyev Date: Sat, 12 Nov 2016 21:17:48 -0500 Subject: [PATCH] feat(rules): add 'valid-locator-type' rule --- README.md | 2 + docs/rules/valid-locator-type.md | 42 ++++++++ index.js | 5 +- lib/is-element-array-finder.js | 28 ++++-- lib/rules/array-callback-return.js | 2 +- lib/rules/valid-locator-type.js | 128 ++++++++++++++++++++++++ test/rules/valid-locator-type.js | 155 +++++++++++++++++++++++++++++ 7 files changed, 349 insertions(+), 13 deletions(-) create mode 100644 docs/rules/valid-locator-type.md create mode 100644 lib/rules/valid-locator-type.js create mode 100644 test/rules/valid-locator-type.js diff --git a/README.md b/README.md index 3f683f6..bf79e72 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,7 @@ Rule | Default Error Level | Auto-fixable | Options [correct-chaining][] | 2 | Yes | [no-invalid-selectors][] | 2 | | [no-array-finder-methods][] | 2 | | +[valid-locator-type][] | 2 | | [missing-wait-message][] | 1 (Warning) | | [no-browser-sleep][] | 1 | | [no-by-xpath][] | 1 | | @@ -113,6 +114,7 @@ See [configuring rules][] for more information. [no-invalid-selectors]: docs/rules/no-invalid-selectors.md [use-promise-all]: docs/rules/use-promise-all.md [no-array-finder-methods]: docs/rules/no-array-finder-methods.md +[valid-locator-type]: docs/rules/valid-locator-type.md [configuring rules]: http://eslint.org/docs/user-guide/configuring#configuring-rules ## Recommended configuration diff --git a/docs/rules/valid-locator-type.md b/docs/rules/valid-locator-type.md new file mode 100644 index 0000000..dbf5b6f --- /dev/null +++ b/docs/rules/valid-locator-type.md @@ -0,0 +1,42 @@ +# Ensure correct locator argument type for `element()`, `element.all()`, `$()` and `$$()` + +This rule would warn if there is a incorrect "by" locator type passed to `element()` and `element.all()`. +Additionally, this rule would warn if there is a "by"-type locator passed to `$()` and `$$()` shortcuts. + +## Rule details + +Any use of the following patterns are considered errors: + +```js +element(".class"); +element.all(".class"); +$(by.css(".class")); +$$(by.css(".class")); +element(by.css(".class1")).element(".class2"); +element(by.css(".class1")).all(".class2"); +element.all(by.css(".class1")).all(".class2"); +$(".class1").all(".class2"); +$(".class1").element(".class2"); +$$(".class1").all(".class2"); +$(".class1").$(by.css(".class2")); +$(".class1").$$(by.css(".class2")); +$$(".class1").$$(by.css(".class2")); +``` + +The following patterns are not errors: + +```js +element(by.css(".class")); +element.all(by.css(".class")); +$(".class"); +$$(".class"); +element(by.css(".class1")).element(by.css(".class2")); +element(by.css(".class1")).all(by.css(".class2")); +element.all(by.css(".class1")).all(by.css(".class2")); +$(".class1").all(by.css(".class2")); +$(".class1").element(by.css(".class2")); +$$(".class1").all(by.css(".class2")); +$(".class1").$(".class2"); +$(".class1").$$(".class2"); +$$(".class1").$$(".class2"); +``` diff --git a/index.js b/index.js index f5d67ce..fd65027 100644 --- a/index.js +++ b/index.js @@ -27,6 +27,7 @@ var noAngularAttributes = require('./lib/rules/no-angular-attributes') var noInvalidSelectors = require('./lib/rules/no-invalid-selectors') var usePromiseAll = require('./lib/rules/use-promise-all') var noArrayFinderMethods = require('./lib/rules/no-array-finder-methods') +var validLocatorType = require('./lib/rules/valid-locator-type') module.exports = { rules: { @@ -56,7 +57,8 @@ module.exports = { 'no-angular-attributes': noAngularAttributes, 'no-invalid-selectors': noInvalidSelectors, 'use-promise-all': usePromiseAll, - 'no-array-finder-methods': noArrayFinderMethods + 'no-array-finder-methods': noArrayFinderMethods, + 'valid-locator-type': validLocatorType }, configs: { recommended: { @@ -66,6 +68,7 @@ module.exports = { 'protractor/correct-chaining': 2, 'protractor/no-invalid-selectors': 2, 'protractor/no-array-finder-methods': 2, + 'protractor/valid-locator-type': 2, 'protractor/missing-wait-message': 1, 'protractor/no-browser-sleep': 1, 'protractor/no-by-xpath': 1, diff --git a/lib/is-element-array-finder.js b/lib/is-element-array-finder.js index 39b75c4..0851298 100644 --- a/lib/is-element-array-finder.js +++ b/lib/is-element-array-finder.js @@ -1,29 +1,35 @@ 'use strict' /** - * Checks a given MemberExpression node is an ElementArrayFinder instance. + * Checks a given CallExpression node is an ElementArrayFinder instance. * * @fileoverview Utility function to determine if a node is an ElementArrayFinder * @author Alexander Afanasyev */ module.exports = function (node) { - // handling $$ shortcut - var object = node.object - if (object && object.callee && object.callee.name === '$$') { - return true - } + var callee = node.callee + + if (callee) { + // handling $$ shortcut + if (callee.name === '$$') { + return true + } + + // handling element.all() + if (callee.object && callee.object.name === 'element' && callee.property && callee.property.name === 'all') { + return true + } - if (object) { - var callee = object.callee - if (callee && callee.object && callee.property) { + // handling nested expressions + if (callee.object && callee.property) { // handling chained $$ if (callee.property.name === '$$') { return true } // handling element.all() and chained element() and all() - if (callee.property.name === 'all' && - (callee.object.name === 'element' || (callee.object.callee && callee.object.callee.name === 'element'))) { + if (callee.property.name === 'all') { + // TODO: inspect all the callees and search for element or $? return true } } diff --git a/lib/rules/array-callback-return.js b/lib/rules/array-callback-return.js index 47a9bb4..169ed26 100644 --- a/lib/rules/array-callback-return.js +++ b/lib/rules/array-callback-return.js @@ -47,7 +47,7 @@ function getLocation (node, sourceCode) { * the specified name's property */ function isTargetMethod (node) { - return (isElementArrayFinder(node) && + return (node.object && isElementArrayFinder(node.object) && node.type === 'MemberExpression' && node.property && TARGET_METHODS.test(node.property.name) diff --git a/lib/rules/valid-locator-type.js b/lib/rules/valid-locator-type.js new file mode 100644 index 0000000..223547b --- /dev/null +++ b/lib/rules/valid-locator-type.js @@ -0,0 +1,128 @@ +'use strict' + +/** + * @fileoverview Ensure correct locator argument type for `element()`, `element.all()`, `$()` and `$$()` + * @author Alexander Afanasyev + */ + +var isElementFinder = require('../is-element-finder') +var isElementArrayFinder = require('../is-element-array-finder') + +// The first four "is" functions below are simple helper functions that help to distinguish between `$` vs `element`, and `$$` vs `element.all` + +/** + * Checks if a given node is an ElementFinder represented by element() callee - both nested and not + * + * @param {ASTNode} node - A node to check. + * @returns {boolean} + */ +function isElement (node) { + return node.callee.name === 'element' || (node.callee.property && node.callee.property.name === 'element') +} + +/** + * Checks if a given node is an ElementArrayFinder represented by all() callee - both nested and not + * + * @param {ASTNode} node - A node to check. + * @returns {boolean} + */ +function isElementAll (node) { + return node.callee.property && node.callee.property.name === 'all' +} + +/** + * Checks if a given node is an ElementFinder represented by $() callee - both nested and not + * + * @param {ASTNode} node - A node to check. + * @returns {boolean} + */ +function is$ (node) { + return node.callee.name === '$' || (node.callee.property && node.callee.property.name === '$') +} + +/** + * Checks if a given node is an ElementArrayFinder represented by $$() callee - both nested and not + * + * @param {ASTNode} node - A node to check. + * @returns {boolean} + */ +function is$$ (node) { + return node.callee.name === '$$' || (node.callee.property && node.callee.property.name === '$$') +} + +/** + * Checks if a given CallExpression node has the first literal argument + * + * @param {ASTNode} node - A node to check. + * @returns {boolean} + */ +function isArgumentLiteral (node) { + return node.arguments && node.arguments[0].type === 'Literal' +} + +/** + * Checks if a given CallExpression node has the argument the "by" expression + * + * @param {ASTNode} node - A node to check. + * @returns {boolean} + */ +function isArgumentByLocator (node) { + if (node.arguments && node.arguments[0].type === 'CallExpression') { + var argument = node.arguments[0] + if (argument.callee && argument.callee.object && argument.callee.object.name === 'by') { + return true + } + } + return false +} + +module.exports = { + meta: { + schema: [] + }, + + // TODO: need to make isElementFinder and isElementArrayFinder universal - handling both Member and Call expressions + create: function (context) { + return { + CallExpression: function (node) { + // element finders + if (isElementFinder(node)) { + // handle element + if (isElement(node) && isArgumentLiteral(node)) { + context.report({ + node: node.arguments[0], + message: 'Invalid locator type.' + }) + } + + // handle $ + if (is$(node) && isArgumentByLocator(node)) { + context.report({ + node: node.arguments[0], + message: 'Invalid locator type.' + }) + } + } + + // element array finders + if (isElementArrayFinder(node)) { + // handle element.all + if (isElementAll(node) && isArgumentLiteral(node)) { + context.report({ + node: node.arguments[0], + message: 'Invalid locator type.' + }) + } + + // handle $$ + if (is$$(node) && isArgumentByLocator(node)) { + context.report({ + node: node.arguments[0], + message: 'Invalid locator type.' + }) + } + } + } + } + } +} diff --git a/test/rules/valid-locator-type.js b/test/rules/valid-locator-type.js new file mode 100644 index 0000000..74dcb53 --- /dev/null +++ b/test/rules/valid-locator-type.js @@ -0,0 +1,155 @@ +'use strict' + +var rule = require('../../lib/rules/valid-locator-type') +var RuleTester = require('eslint').RuleTester + +var eslintTester = new RuleTester() + +eslintTester.run('valid-locator-type', rule, { + valid: [ + 'element(by.css(".class"));', + 'element.all(by.css(".class"));', + '$(".class");', + '$$(".class");', + 'element(by.css(".class1")).element(by.css(".class2"));', + 'element(by.css(".class1")).all(by.css(".class2"));', + 'element.all(by.css(".class1")).all(by.css(".class2"));', + '$(".class1").all(by.css(".class2"));', + '$(".class1").element(by.css(".class2"));', + '$$(".class1").all(by.css(".class2"));', + '$(".class1").$(".class2");', + '$(".class1").$$(".class2");', + '$$(".class1").$$(".class2");' + ], + + invalid: [ + { + code: 'element(".class");', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: 'element.all(".class");', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: '$(by.css(".class"));', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: '$$(by.css(".class"));', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: 'element(by.css(".class1")).element(".class2");', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: 'element(by.css(".class1")).all(".class2");', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: '$(".class1").all(".class2");', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: '$(".class1").element(".class2");', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: '$$(".class1").all(".class2");', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: '$(".class1").$(by.css(".class2"));', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: '$(".class1").$$(by.css(".class2"));', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: '$$(".class1").$$(by.css(".class2"));', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: '$$(".class1").all(by.css(".class2")).all(".class3");', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: 'element.all(by.css(".class1")).all(".class2");', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: '$$(".class1").all(".class2");', + errors: [ + { + message: 'Invalid locator type.' + } + ] + }, + { + code: 'element.all(by.css(".class1")).$$(by.css(".class2"));', + errors: [ + { + message: 'Invalid locator type.' + } + ] + } + ] +})