Skip to content

Commit 04da60c

Browse files
alecxealecxe
committed
feat(rules): add 'no-repetitive-locators' rule
1 parent bb1c9a8 commit 04da60c

File tree

7 files changed

+219
-0
lines changed

7 files changed

+219
-0
lines changed

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ Rule | Default | Options
5555
[no-expect-in-po][] | 1 | requires plugin "settings"
5656
[no-promise-in-if][] | 1 |
5757
[no-execute-script][] | 1 | requires plugin "settings"
58+
[no-repetitive-locators][] | 1 |
5859
[by-css-shortcut][] | 0 |
5960

6061
For example, the `missing-perform` rule is enabled by default and will cause
@@ -95,6 +96,7 @@ See [configuring rules][] for more information.
9596
[no-promise-in-if]: docs/rules/no-promise-in-if.md
9697
[no-execute-script]: docs/rules/no-execute-script.md
9798
[correct-chaining]: docs/rules/correct-chaining.md
99+
[no-repetitive-locators]: docs/rules/no-repetitive-locators.md
98100
[configuring rules]: http://eslint.org/docs/user-guide/configuring#configuring-rules
99101

100102
## Recommended configuration

‎docs/rules/no-repetitive-locators.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Discourage repeating locators
2+
3+
This rule would warn if repetitive locators would be detected *in a single file* in support of the [DRY principle](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself). For example:
4+
5+
```js
6+
var MyPage = function () {
7+
this.grids = element.all(by.css(".mygrid"));
8+
this.firstGrid = element.all(by.css(".mygrid")).first();
9+
}
10+
```
11+
12+
Here, `.mygrid` locator is used two times. A better version:
13+
14+
```js
15+
var MyPage = function () {
16+
this.grids = element.all(by.css(".mygrid"));
17+
this.firstGrid = this.grids.first();
18+
}
19+
```
20+
21+
Note that the rule is looking for exact duplicates of the locators - where both the `by` locator type and the value are repeated.
22+
23+
# When not to have this rule enabled
24+
25+
If this rule produces too much false positives and warns about locators being repetitive when repetition is something you have intended to have, disable the rule.
26+
Though, I suggest leaving it enabled globally and having it disabled for specific files or problematic parts of code instead.

‎index.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ var noExpectInPO = require('./lib/rules/no-expect-in-po')
2020
var noPromiseInIf = require('./lib/rules/no-promise-in-if')
2121
var noExecuteScript = require('./lib/rules/no-execute-script')
2222
var correctChaining = require('./lib/rules/correct-chaining')
23+
var noRepetitiveLocators = require('./lib/rules/no-repetitive-locators')
2324

2425
module.exports = {
2526
rules: {
@@ -42,6 +43,7 @@ module.exports = {
4243
'no-expect-in-po': noExpectInPO,
4344
'no-promise-in-if': noPromiseInIf,
4445
'no-execute-script': noExecuteScript,
46+
'no-repetitive-locators': noRepetitiveLocators,
4547
'correct-chaining': correctChaining
4648
},
4749
configs: {
@@ -66,6 +68,7 @@ module.exports = {
6668
'protractor/no-expect-in-po': 1,
6769
'protractor/no-promise-in-if': 1,
6870
'protractor/no-execute-script': 1,
71+
'protractor/no-repetitive-locators': 1,
6972
'protractor/by-css-shortcut': 0
7073
},
7174
globals: {

‎lib/get-locator.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
'use strict'
2+
3+
/**
4+
* @fileoverview Utility function to extract the "by" locator values. Also handles the "$" and "$$" shortcuts.
5+
* @author Alexander Afanasyev
6+
*/
7+
module.exports = function (node) {
8+
var object = node.callee.object
9+
var property = node.callee.property
10+
11+
var insideBy = object && property && object.name === 'by'
12+
var dollarShortcuts = node.callee.name === '$' || node.callee.name === '$$'
13+
var chainedDollarShortcuts = property && (property.name === '$' || property.name === '$$')
14+
15+
// handling by.smth calls
16+
if (insideBy) {
17+
var hasArgument = node.arguments && node.arguments.length && node.arguments[0].hasOwnProperty('value')
18+
if (hasArgument) {
19+
return {
20+
by: property.name,
21+
value: node.arguments[0].value
22+
}
23+
}
24+
}
25+
26+
// handling $ and $$ calls
27+
if (dollarShortcuts || chainedDollarShortcuts) {
28+
return {
29+
by: 'css',
30+
value: node.arguments[0].value
31+
}
32+
}
33+
}

‎lib/rules/no-repetitive-locators.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
'use strict'
2+
3+
/**
4+
* @fileoverview Discourage repeating locators
5+
* @author Alexander Afanasyev
6+
*/
7+
8+
var getLocator = require('../get-locator')
9+
10+
module.exports = {
11+
meta: {
12+
schema: []
13+
},
14+
15+
create: function (context) {
16+
// locators collects locators grouped by type, e.g.: {css: [".test", "div:first-of-type"], id: ["myid1", "myid2"]}
17+
var locators = {}
18+
19+
return {
20+
CallExpression: function (node) {
21+
var locator = getLocator(node)
22+
if (locator) {
23+
// find exact locator duplicates (both by and value were met before)
24+
if (locator.by in locators && locators[locator.by].indexOf(locator.value) >= 0) {
25+
context.report({
26+
node: node,
27+
message: 'Repetitive locator detected'
28+
})
29+
}
30+
31+
// maintain "locators" object
32+
if (!(locator.by in locators)) {
33+
locators[locator.by] = [locator.value]
34+
} else {
35+
locators[locator.by].push(locator.value)
36+
}
37+
}
38+
}
39+
}
40+
}
41+
}

‎test/lib/join-multiline-code.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
'use strict'
2+
3+
/**
4+
* @fileoverview Utility function to generate multi-line code from an array of code lines
5+
*/
6+
module.exports = function toCode (lines, description) {
7+
return (description ? '// ' + description : '') + '\n' + lines.join('\n')
8+
}

‎test/rules/no-repetitive-locators.js‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
'use strict'
2+
3+
var rule = require('../../lib/rules/no-repetitive-locators')
4+
var RuleTester = require('eslint').RuleTester
5+
var toCode = require('../lib/join-multiline-code')
6+
7+
var eslintTester = new RuleTester()
8+
9+
eslintTester.run('no-repetitive-locators', rule, {
10+
valid: [
11+
toCode([
12+
'var MyPage = function () {',
13+
' this.grids = element.all(by.css(".mygrid"));',
14+
' this.firstGrid = this.grids.first();',
15+
'}'
16+
]),
17+
toCode([
18+
'var MyPage = function () {',
19+
' this.parent = $(".container #parent");',
20+
' this.child1 = this.parent.$("div:first-of-type");',
21+
' this.child2 = this.parent.$("#subcontainer > .add-client");',
22+
'}'
23+
])
24+
],
25+
26+
invalid: [
27+
{
28+
code: toCode([
29+
'var MyPage = function () {',
30+
' this.grids = element.all(by.css(".mygrid"));',
31+
' this.firstGrid = element.all(by.css(".mygrid")).first();',
32+
'}'
33+
]),
34+
errors: [
35+
{
36+
message: 'Repetitive locator detected'
37+
}
38+
]
39+
},
40+
{
41+
code: toCode([
42+
'var MyPage = function () {',
43+
' this.grids = $$(".mygrid");',
44+
' this.firstGrid = $$(".mygrid").first();',
45+
'}'
46+
]),
47+
errors: [
48+
{
49+
message: 'Repetitive locator detected'
50+
}
51+
]
52+
},
53+
{
54+
code: toCode([
55+
'var MyPage = function () {',
56+
' this.grids = $$(".mygrid");',
57+
' this.firstGrid = $(".mygrid")',
58+
'}'
59+
]),
60+
errors: [
61+
{
62+
message: 'Repetitive locator detected'
63+
}
64+
]
65+
},
66+
{
67+
code: toCode([
68+
'var MyPage = function () {',
69+
' this.grids = element.all(by.css(".mygrid"));',
70+
' this.firstGrid = $(".mygrid")',
71+
'}'
72+
]),
73+
errors: [
74+
{
75+
message: 'Repetitive locator detected'
76+
}
77+
]
78+
},
79+
{
80+
code: toCode([
81+
'var MyPage = function () {',
82+
' this.grids = $$(".mygrid");',
83+
' this.firstGrid = element(by.css(".mygrid"))',
84+
'}'
85+
]),
86+
errors: [
87+
{
88+
message: 'Repetitive locator detected'
89+
}
90+
]
91+
},
92+
{
93+
code: toCode([
94+
'var MyPage = function () {',
95+
' this.grids = element.all(by.className("test"));',
96+
' this.firstGrid = element(by.css(".container")).all(by.className("test")).first()',
97+
'}'
98+
]),
99+
errors: [
100+
{
101+
message: 'Repetitive locator detected'
102+
}
103+
]
104+
}
105+
]
106+
})

0 commit comments

Comments
 (0)