Skip to content

Commit

Permalink
feat(rules): add 'limit-selector-depth' rule
Browse files Browse the repository at this point in the history
  • Loading branch information
alecxe committed Jun 26, 2017
1 parent d33f8c8 commit b174bff
Show file tree
Hide file tree
Showing 6 changed files with 202 additions and 1 deletion.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ There are various types of rules implemented in the plugin. Here is a rough cate
* [no-repetitive-selectors][]: Discourage repeating parts of CSS selectors
* [valid-by-id][]: Prohibit use of invalid ID value when using `by.id()` locator
* [valid-by-tagname][]: Prohibit use of invalid Html Tag Name value when using `by.tagName()` locator
* [limit-selector-depth][]: Warn about potentially "deep" CSS selectors with too many nodes in the path
#### Style Guide Recommendations and Best Practices
Expand Down Expand Up @@ -132,6 +133,7 @@ Rule | Default Error Level | Auto-fixable | Options
[use-count-method][] | 1 | |
[valid-by-id][] | 1 | |
[valid-by-tagname][] | 1 | |
[limit-selector-depth][] | 1 | | number of nodes (default 5)
[use-promise-all][] | 0 (Turned off) | |
[by-css-shortcut][] | 0 | |
[no-browser-driver][] | 0 | |
Expand Down Expand Up @@ -182,6 +184,7 @@ See [configuring rules][] for more information.
[no-array-finder-methods]: docs/rules/no-array-finder-methods.md
[valid-locator-type]: docs/rules/valid-locator-type.md
[no-compound-classes]: docs/rules/no-compound-classes.md
[limit-selector-depth]: docs/rules/limit-selector-depth.md
[use-count-method]: docs/rules/use-count-method.md
[no-browser-driver]: docs/rules/no-browser-driver.md
[valid-by-id]: docs/rules/valid-by-id.md
Expand Down
41 changes: 41 additions & 0 deletions docs/rules/limit-selector-depth.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Warn about potentially complex selectors with too many nodes

This rule would warn if a CSS selector is too "deep" - has a significant number of nodes in the path.

For instance, this CSS selector would be considered fragile and complex, because of 6 nodes in the path:

```css
.content > table > tbody > tr:nth-child(2) > td.cell > input#email
```

A simpler version in this case could be (2 nodes):

```css
.content input#email
```

The idea behind this rule is that the more nodes you have in your CSS selector path - the more fragile, the more dependant on the HTML structure of the page it is.

Number of nodes is configurable, default value is `5`.

## Rule details

This rule is based on [`css-selector-parser`](https://github.com/mdevils/node-css-selector-parser) parsing abilities.

Any use of the following patterns are considered warnings (with the default depth value of `5`):

```js
element(by.css(".content > table > tbody > tr:nth-child(2) > td.cell > input#email"));
element.all(by.css(".content > table > tbody > tr:nth-child(2) > td.cell > input#email"));
$(".content > table > tbody > tr:nth-child(2) > td.cell > input#email");
$$(".content > table > tbody > tr:nth-child(2) > td.cell > input#email");
```

The following patterns are not warnings:

```js
element(by.css(".myclass"));
element.all(by.css(".content input#email"));
$("#email");
$$("tr:nth-child(2)");
```
5 changes: 4 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ var validById = require('./lib/rules/valid-by-id')
var validByTagName = require('./lib/rules/valid-by-tagname')
var noGetRawId = require('./lib/rules/no-get-raw-id')
var noGetLocationAbsUrl = require('./lib/rules/no-get-location-abs-url')
var limitSelectorDepth = require('./lib/rules/limit-selector-depth')

module.exports = {
rules: {
Expand Down Expand Up @@ -72,7 +73,8 @@ module.exports = {
'valid-by-id': validById,
'valid-by-tagname': validByTagName,
'no-get-raw-id': noGetRawId,
'no-get-location-abs-url': noGetLocationAbsUrl
'no-get-location-abs-url': noGetLocationAbsUrl,
'limit-selector-depth': limitSelectorDepth
},
configs: {
recommended: {
Expand Down Expand Up @@ -108,6 +110,7 @@ module.exports = {
'protractor/use-count-method': 1,
'protractor/valid-by-id': 1,
'protractor/valid-by-tagname': 1,
'protractor/limit-selector-depth': 1,
'protractor/no-get-location-abs-url': 1,
'protractor/use-promise-all': 0,
'protractor/by-css-shortcut': 0,
Expand Down
24 changes: 24 additions & 0 deletions lib/get-selector-depth.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict'

/**
* @fileoverview Utility function to calculate the number of nodes in a given CSS selector
* @author Alexander Afanasyev
*/

var parser = require('./get-css-parser')

module.exports = function (cssSelector) {
var parsedSelector = parser.parse(cssSelector)

if (!parsedSelector) {
return 0
}

var rule = parsedSelector.rule
var depth = 0
while (rule) {
rule = rule.rule
depth += 1
}
return depth
}
41 changes: 41 additions & 0 deletions lib/rules/limit-selector-depth.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
'use strict'

/**
* @fileoverview Warn about potentially "deep" CSS selectors with too many nodes in the path
* @author Alexander Afanasyev
*/
var isCSSLocator = require('../find-css-locator')
var getSelectorDepth = require('../get-selector-depth')

module.exports = {
meta: {
schema: [
{
type: 'integer',
additionalProperties: false
}
]
},

create: function (context) {
var maxDepth = context.options.length ? context.options[0] : 5

return {
'CallExpression': function (node) {
if (node.arguments && node.arguments.length && node.arguments[0].hasOwnProperty('value')) {
if (isCSSLocator(node)) {
var cssSelector = node.arguments[0].value
var depth = getSelectorDepth(cssSelector)

if (depth > maxDepth) {
context.report({
node: node.arguments[0],
message: 'CSS selector has too many nodes.'
})
}
}
}
}
}
}
}
89 changes: 89 additions & 0 deletions test/rules/limit-selector-depth.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
'use strict'

var rule = require('../../lib/rules/limit-selector-depth')
var RuleTester = require('eslint').RuleTester

var eslintTester = new RuleTester()

eslintTester.run('limit-selector-depth', rule, {
valid: [
'element(by.css(".myclass"));',
'element.all(by.css(".myclass"));',
'someOtherFunction("input[name=email]");',
{
options: [
5
],
code: 'element(by.css(".content > table > tbody td.cell > input#email"));'
},
'element(by.css("input.email.email-input.custom-input.other.classValue"));',
{
options: [
1
],
code: '$(".content");'
},
'$$("");',
'$();'
],

invalid: [
{
code: 'element(by.css(".content > table > tbody > tr:nth-child(2) > td.cell > input#email"));',
errors: [
{
message: 'CSS selector has too many nodes.'
}
]
},
{
code: 'element(by.css("div div div div div div"));',
errors: [
{
message: 'CSS selector has too many nodes.'
}
]
},
{
code: 'element(by.css("div > div > div > div > div > div"));',
errors: [
{
message: 'CSS selector has too many nodes.'
}
]
},
{
options: [
1
],
code: '$(".content > table");',
errors: [
{
message: 'CSS selector has too many nodes.'
}
]
},
{
options: [
0
],
code: '$(".content");',
errors: [
{
message: 'CSS selector has too many nodes.'
}
]
},
{
options: [
4
],
code: '$$(".content > table > tbody td.cell > input#email");',
errors: [
{
message: 'CSS selector has too many nodes.'
}
]
}
]
})

0 comments on commit b174bff

Please sign in to comment.