Skip to content

Commit 3173ef3

Browse files
alecxealecxe
committed
feat(rules): add 'no-repetitive-selectors' rule
1 parent 0ad518e commit 3173ef3

File tree

5 files changed

+174
-0
lines changed

5 files changed

+174
-0
lines changed

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ Rule | Default Error Level | Auto-fixable | Options
5656
[no-promise-in-if][] | 1 | |
5757
[no-execute-script][] | 1 | | requires plugin "settings"
5858
[no-repetitive-locators][] | 1 | |
59+
[no-repetitive-selectors][] | 1 | |
5960
[no-get-inner-outer-html][] | 1 | |
6061
[by-css-shortcut][] | 0 | |
6162

@@ -99,6 +100,7 @@ See [configuring rules][] for more information.
99100
[correct-chaining]: docs/rules/correct-chaining.md
100101
[no-repetitive-locators]: docs/rules/no-repetitive-locators.md
101102
[no-get-inner-outer-html]: docs/rules/no-get-inner-outer-html.md
103+
[no-repetitive-selectors]: docs/rules/no-repetitive-selectors.md
102104
[configuring rules]: http://eslint.org/docs/user-guide/configuring#configuring-rules
103105

104106
## Recommended configuration

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Discourage repeating parts of CSS selectors
2+
3+
In support of the [DRY principle](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself) and to improve readability of page object fields definitions, this rule would try its best finding parts of CSS selectors that are unnecessarily repeated.
4+
5+
Consider this Page Object:
6+
7+
```js
8+
var MyPage = function () {
9+
this.parent = $(".container #parent");
10+
this.child1 = $(".container #parent div:first-of-type");
11+
this.child2 = $(".container #parent #subcontainer > .add-client");
12+
}
13+
```
14+
15+
The `.container #parent` part in this case is repeated and should be reused instead:
16+
17+
```js
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+
```

‎index.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ 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')
2323
var noRepetitiveLocators = require('./lib/rules/no-repetitive-locators')
24+
var noRepetitiveSelectors = require('./lib/rules/no-repetitive-selectors')
2425
var noGetInnerOuterHtml = require('./lib/rules/no-get-inner-outer-html')
2526

2627
module.exports = {
@@ -45,6 +46,7 @@ module.exports = {
4546
'no-promise-in-if': noPromiseInIf,
4647
'no-execute-script': noExecuteScript,
4748
'no-repetitive-locators': noRepetitiveLocators,
49+
'no-repetitive-selectors': noRepetitiveSelectors,
4850
'correct-chaining': correctChaining,
4951
'no-get-inner-outer-html': noGetInnerOuterHtml
5052
},
@@ -71,6 +73,7 @@ module.exports = {
7173
'protractor/no-promise-in-if': 1,
7274
'protractor/no-execute-script': 1,
7375
'protractor/no-repetitive-locators': 1,
76+
'protractor/no-repetitive-selectors': 1,
7477
'protractor/no-get-inner-outer-html': 1,
7578
'protractor/by-css-shortcut': 0
7679
},

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
'use strict'
2+
3+
/**
4+
* @fileoverview Discourage repeating parts of CSS selectors
5+
* @author Alexander Afanasyev
6+
*/
7+
8+
var isCSSLocator = require('../find-css-locator')
9+
10+
module.exports = {
11+
meta: {
12+
schema: []
13+
},
14+
15+
create: function (context) {
16+
// keeping the processed CSS selectors to check the next one against
17+
var selectors = []
18+
19+
return {
20+
'CallExpression': function (node) {
21+
if (node.arguments && node.arguments.length && node.arguments[0].hasOwnProperty('value')) {
22+
if (isCSSLocator(node)) {
23+
// we don't want to report the same repeated part multiple times
24+
var alreadyReported = []
25+
26+
var currentSelector = node.arguments[0].value
27+
var currentSelectorParts = currentSelector.split(' ')
28+
29+
selectors.forEach(function (selectorParts) {
30+
// match the parts until a different part is found
31+
var sameParts = []
32+
for (var i = 0; i < selectorParts.length; i++) {
33+
if (selectorParts[i] === currentSelectorParts[i]) {
34+
sameParts.push(selectorParts[i])
35+
} else {
36+
break
37+
}
38+
}
39+
40+
var samePart = sameParts.join(' ').trim()
41+
if (samePart && alreadyReported.indexOf(samePart) < 0) {
42+
context.report({
43+
node: node,
44+
message: 'Repetitive part of a selector detected: "' + samePart + '"'
45+
})
46+
alreadyReported.push(samePart)
47+
}
48+
})
49+
50+
selectors.push(currentSelectorParts)
51+
}
52+
}
53+
}
54+
}
55+
}
56+
}

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
'use strict'
2+
3+
var rule = require('../../lib/rules/no-repetitive-selectors')
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-selectors', rule, {
10+
valid: [
11+
toCode([
12+
'var MyPage = function () {',
13+
' this.parent = $(".container #parent");',
14+
' this.child1 = this.parent.$("div:first-of-type");',
15+
' this.child2 = this.parent.$("#subcontainer > .add-client");',
16+
'}'
17+
]),
18+
toCode([
19+
'var MyPage = function () {',
20+
' this.elm1 = $(".container #parent");',
21+
' this.elm2 = element(by.css("#parent"));',
22+
'}'
23+
]),
24+
toCode([
25+
'var MyPage = function () {',
26+
' this.elm1 = $(".container > #parent");',
27+
' this.elm2 = element(by.css(".container>#parent"));',
28+
'}'
29+
])
30+
],
31+
32+
invalid: [
33+
{
34+
code: toCode([
35+
'var MyPage = function () {',
36+
' this.parent = $(".container");',
37+
' this.child1 = $(".container div:first-of-type");',
38+
'}'
39+
]),
40+
errors: [
41+
{
42+
message: 'Repetitive part of a selector detected: ".container"'
43+
}
44+
]
45+
},
46+
{
47+
code: toCode([
48+
'var MyPage = function () {',
49+
' this.parent = $(".container #parent");',
50+
' this.child1 = $(".container #parent div:first-of-type");',
51+
' this.child2 = $(".container #parent #subcontainer > .add-client");',
52+
'}'
53+
]),
54+
errors: [
55+
{
56+
message: 'Repetitive part of a selector detected: ".container #parent"'
57+
},
58+
{
59+
message: 'Repetitive part of a selector detected: ".container #parent"'
60+
}
61+
]
62+
},
63+
{
64+
code: toCode([
65+
'var MyPage = function () {',
66+
' this.field1 = $(".container #parent div:first-of-type");',
67+
' this.field2 = $(".container div:first-of-type");',
68+
'}'
69+
]),
70+
errors: [
71+
{
72+
message: 'Repetitive part of a selector detected: ".container"'
73+
}
74+
]
75+
},
76+
{
77+
code: toCode([
78+
'var MyPage = function () {',
79+
' this.elm1 = $(".container");',
80+
' this.elm2 = element(by.css(".container"));',
81+
'}'
82+
]),
83+
errors: [
84+
{
85+
message: 'Repetitive part of a selector detected: ".container"'
86+
}
87+
]
88+
}
89+
]
90+
})

0 commit comments

Comments
 (0)