Skip to content

Commit 1bb1139

Browse files
committed
feat(rules): add 'valid-locator-type' rule
1 parent 0069c15 commit 1bb1139

File tree

7 files changed

+349
-13
lines changed

7 files changed

+349
-13
lines changed

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ Rule | Default Error Level | Auto-fixable | Options
4646
[correct-chaining][] | 2 | Yes |
4747
[no-invalid-selectors][] | 2 | |
4848
[no-array-finder-methods][] | 2 | |
49+
[valid-locator-type][] | 2 | |
4950
[missing-wait-message][] | 1 (Warning) | |
5051
[no-browser-sleep][] | 1 | |
5152
[no-by-xpath][] | 1 | |
@@ -113,6 +114,7 @@ See [configuring rules][] for more information.
113114
[no-invalid-selectors]: docs/rules/no-invalid-selectors.md
114115
[use-promise-all]: docs/rules/use-promise-all.md
115116
[no-array-finder-methods]: docs/rules/no-array-finder-methods.md
117+
[valid-locator-type]: docs/rules/valid-locator-type.md
116118
[configuring rules]: http://eslint.org/docs/user-guide/configuring#configuring-rules
117119

118120
## Recommended configuration

docs/rules/valid-locator-type.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Ensure correct locator argument type for `element()`, `element.all()`, `$()` and `$$()`
2+
3+
This rule would warn if there is a incorrect "by" locator type passed to `element()` and `element.all()`.
4+
Additionally, this rule would warn if there is a "by"-type locator passed to `$()` and `$$()` shortcuts.
5+
6+
## Rule details
7+
8+
Any use of the following patterns are considered errors:
9+
10+
```js
11+
element(".class");
12+
element.all(".class");
13+
$(by.css(".class"));
14+
$$(by.css(".class"));
15+
element(by.css(".class1")).element(".class2");
16+
element(by.css(".class1")).all(".class2");
17+
element.all(by.css(".class1")).all(".class2");
18+
$(".class1").all(".class2");
19+
$(".class1").element(".class2");
20+
$$(".class1").all(".class2");
21+
$(".class1").$(by.css(".class2"));
22+
$(".class1").$$(by.css(".class2"));
23+
$$(".class1").$$(by.css(".class2"));
24+
```
25+
26+
The following patterns are not errors:
27+
28+
```js
29+
element(by.css(".class"));
30+
element.all(by.css(".class"));
31+
$(".class");
32+
$$(".class");
33+
element(by.css(".class1")).element(by.css(".class2"));
34+
element(by.css(".class1")).all(by.css(".class2"));
35+
element.all(by.css(".class1")).all(by.css(".class2"));
36+
$(".class1").all(by.css(".class2"));
37+
$(".class1").element(by.css(".class2"));
38+
$$(".class1").all(by.css(".class2"));
39+
$(".class1").$(".class2");
40+
$(".class1").$$(".class2");
41+
$$(".class1").$$(".class2");
42+
```

index.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ var noAngularAttributes = require('./lib/rules/no-angular-attributes')
2727
var noInvalidSelectors = require('./lib/rules/no-invalid-selectors')
2828
var usePromiseAll = require('./lib/rules/use-promise-all')
2929
var noArrayFinderMethods = require('./lib/rules/no-array-finder-methods')
30+
var validLocatorType = require('./lib/rules/valid-locator-type')
3031

3132
module.exports = {
3233
rules: {
@@ -56,7 +57,8 @@ module.exports = {
5657
'no-angular-attributes': noAngularAttributes,
5758
'no-invalid-selectors': noInvalidSelectors,
5859
'use-promise-all': usePromiseAll,
59-
'no-array-finder-methods': noArrayFinderMethods
60+
'no-array-finder-methods': noArrayFinderMethods,
61+
'valid-locator-type': validLocatorType
6062
},
6163
configs: {
6264
recommended: {
@@ -66,6 +68,7 @@ module.exports = {
6668
'protractor/correct-chaining': 2,
6769
'protractor/no-invalid-selectors': 2,
6870
'protractor/no-array-finder-methods': 2,
71+
'protractor/valid-locator-type': 2,
6972
'protractor/missing-wait-message': 1,
7073
'protractor/no-browser-sleep': 1,
7174
'protractor/no-by-xpath': 1,

lib/is-element-array-finder.js

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,35 @@
11
'use strict'
22

33
/**
4-
* Checks a given MemberExpression node is an ElementArrayFinder instance.
4+
* Checks a given CallExpression node is an ElementArrayFinder instance.
55
*
66
* @fileoverview Utility function to determine if a node is an ElementArrayFinder
77
* @author Alexander Afanasyev
88
*/
99
module.exports = function (node) {
10-
// handling $$ shortcut
11-
var object = node.object
12-
if (object && object.callee && object.callee.name === '$$') {
13-
return true
14-
}
10+
var callee = node.callee
11+
12+
if (callee) {
13+
// handling $$ shortcut
14+
if (callee.name === '$$') {
15+
return true
16+
}
17+
18+
// handling element.all()
19+
if (callee.object && callee.object.name === 'element' && callee.property && callee.property.name === 'all') {
20+
return true
21+
}
1522

16-
if (object) {
17-
var callee = object.callee
18-
if (callee && callee.object && callee.property) {
23+
// handling nested expressions
24+
if (callee.object && callee.property) {
1925
// handling chained $$
2026
if (callee.property.name === '$$') {
2127
return true
2228
}
2329

2430
// handling element.all() and chained element() and all()
25-
if (callee.property.name === 'all' &&
26-
(callee.object.name === 'element' || (callee.object.callee && callee.object.callee.name === 'element'))) {
31+
if (callee.property.name === 'all') {
32+
// TODO: inspect all the callees and search for element or $?
2733
return true
2834
}
2935
}

lib/rules/array-callback-return.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ function getLocation (node, sourceCode) {
4747
* the specified name's property
4848
*/
4949
function isTargetMethod (node) {
50-
return (isElementArrayFinder(node) &&
50+
return (node.object && isElementArrayFinder(node.object) &&
5151
node.type === 'MemberExpression' &&
5252
node.property &&
5353
TARGET_METHODS.test(node.property.name)

lib/rules/valid-locator-type.js

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
'use strict'
2+
3+
/**
4+
* @fileoverview Ensure correct locator argument type for `element()`, `element.all()`, `$()` and `$$()`
5+
* @author Alexander Afanasyev
6+
*/
7+
8+
var isElementFinder = require('../is-element-finder')
9+
var isElementArrayFinder = require('../is-element-array-finder')
10+
11+
// The first four "is" functions below are simple helper functions that help to distinguish between `$` vs `element`, and `$$` vs `element.all`
12+
13+
/**
14+
* Checks if a given node is an ElementFinder represented by element() callee - both nested and not
15+
*
16+
* @param {ASTNode} node - A node to check.
17+
* @returns {boolean}
18+
*/
19+
function isElement (node) {
20+
return node.callee.name === 'element' || (node.callee.property && node.callee.property.name === 'element')
21+
}
22+
23+
/**
24+
* Checks if a given node is an ElementArrayFinder represented by all() callee - both nested and not
25+
*
26+
* @param {ASTNode} node - A node to check.
27+
* @returns {boolean}
28+
*/
29+
function isElementAll (node) {
30+
return node.callee.property && node.callee.property.name === 'all'
31+
}
32+
33+
/**
34+
* Checks if a given node is an ElementFinder represented by $() callee - both nested and not
35+
*
36+
* @param {ASTNode} node - A node to check.
37+
* @returns {boolean}
38+
*/
39+
function is$ (node) {
40+
return node.callee.name === '$' || (node.callee.property && node.callee.property.name === '$')
41+
}
42+
43+
/**
44+
* Checks if a given node is an ElementArrayFinder represented by $$() callee - both nested and not
45+
*
46+
* @param {ASTNode} node - A node to check.
47+
* @returns {boolean}
48+
*/
49+
function is$$ (node) {
50+
return node.callee.name === '$$' || (node.callee.property && node.callee.property.name === '$$')
51+
}
52+
53+
/**
54+
* Checks if a given CallExpression node has the first literal argument
55+
*
56+
* @param {ASTNode} node - A node to check.
57+
* @returns {boolean}
58+
*/
59+
function isArgumentLiteral (node) {
60+
return node.arguments && node.arguments[0].type === 'Literal'
61+
}
62+
63+
/**
64+
* Checks if a given CallExpression node has the argument the "by" expression
65+
*
66+
* @param {ASTNode} node - A node to check.
67+
* @returns {boolean}
68+
*/
69+
function isArgumentByLocator (node) {
70+
if (node.arguments && node.arguments[0].type === 'CallExpression') {
71+
var argument = node.arguments[0]
72+
if (argument.callee && argument.callee.object && argument.callee.object.name === 'by') {
73+
return true
74+
}
75+
}
76+
return false
77+
}
78+
79+
module.exports = {
80+
meta: {
81+
schema: []
82+
},
83+
84+
// TODO: need to make isElementFinder and isElementArrayFinder universal - handling both Member and Call expressions
85+
create: function (context) {
86+
return {
87+
CallExpression: function (node) {
88+
// element finders
89+
if (isElementFinder(node)) {
90+
// handle element
91+
if (isElement(node) && isArgumentLiteral(node)) {
92+
context.report({
93+
node: node.arguments[0],
94+
message: 'Invalid locator type.'
95+
})
96+
}
97+
98+
// handle $
99+
if (is$(node) && isArgumentByLocator(node)) {
100+
context.report({
101+
node: node.arguments[0],
102+
message: 'Invalid locator type.'
103+
})
104+
}
105+
}
106+
107+
// element array finders
108+
if (isElementArrayFinder(node)) {
109+
// handle element.all
110+
if (isElementAll(node) && isArgumentLiteral(node)) {
111+
context.report({
112+
node: node.arguments[0],
113+
message: 'Invalid locator type.'
114+
})
115+
}
116+
117+
// handle $$
118+
if (is$$(node) && isArgumentByLocator(node)) {
119+
context.report({
120+
node: node.arguments[0],
121+
message: 'Invalid locator type.'
122+
})
123+
}
124+
}
125+
}
126+
}
127+
}
128+
}

0 commit comments

Comments
 (0)