Skip to content

Commit

Permalink
feat(rules): add 'valid-locator-type' rule
Browse files Browse the repository at this point in the history
  • Loading branch information
alecxe committed Nov 13, 2016
1 parent 0069c15 commit 1bb1139
Show file tree
Hide file tree
Showing 7 changed files with 349 additions and 13 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | |
Expand Down Expand Up @@ -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
Expand Down
42 changes: 42 additions & 0 deletions docs/rules/valid-locator-type.md
Original file line number Diff line number Diff line change
@@ -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");
```
5 changes: 4 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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: {
Expand All @@ -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,
Expand Down
28 changes: 17 additions & 11 deletions lib/is-element-array-finder.js
Original file line number Diff line number Diff line change
@@ -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
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/array-callback-return.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
128 changes: 128 additions & 0 deletions lib/rules/valid-locator-type.js
Original file line number Diff line number Diff line change
@@ -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.'
})
}
}
}
}
}
}
Loading

0 comments on commit 1bb1139

Please sign in to comment.