Skip to content

Commit

Permalink
feat(rules): add 'use-simple-repeaters' rule
Browse files Browse the repository at this point in the history
  • Loading branch information
alecxe committed Jun 10, 2016
1 parent b376e85 commit 8a54521
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 1 deletion.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Rule | Default | Options
[no-describe-selectors][] | 1 |
[no-angular-classes][] | 1 |
[use-angular-locators][] | 1 |
[use-simple-repeaters][] | 1 |
[by-css-shortcut][] | 0 |

For example, the `missing-perform` rule is enabled by default and will cause
Expand All @@ -65,6 +66,7 @@ See [configuring rules][] for more information.
[no-describe-selectors]: docs/rules/no-describe-selectors.md
[no-angular-classes]: docs/rules/no-angular-classes.md
[use-angular-locators]: docs/rules/use-angular-locators.md
[use-simple-repeaters]: docs/rules/use-simple-repeaters.md
[by-css-shortcut]: docs/rules/by-css-shortcut.md
[configuring rules]: http://eslint.org/docs/user-guide/configuring#configuring-rules

Expand Down
38 changes: 38 additions & 0 deletions docs/rules/use-simple-repeaters.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Discourage using extended ng-repeat syntax in by.repeater() locators

Warn about using [filters, ordering or tracking](https://docs.angularjs.org/api/ng/directive/ngRepeat) inside `by.repeater()` locators.
Typically filters, ordering or tracking don't contribute to the usefulness and reliability of a locator.

Imagine you have the following elements you need to locate:

<div ng-repeat="customer in customers | orderBy: 'id' track by $index">...</div>

Now the `| orderBy: 'id' track by $index"` part is not data-oriented and does not give any more useful or unique piece of information about the elements.

This rule would recommend against having this extra part inside `by.repeater()` values. This would be bad:

element.all(by.repeater("customer in customers | orderBy: 'id' track by $index"));

And, this what would be better:

element.all(by.repeater("customer in customers"));
element.all(by.exactRepeater("customer in customers"));

## Rule details

The rule would warn if it detects a pipe - `|` inside a repeater or a `track by` substring.

Any use of the following patterns are considered warnings:

```js
element.all(by.repeater("item in items | filter : x | orderBy : order | limitTo : limit as results"));
element.all(by.repeater("item in items | filter:searchTerm"));
element.all(by.repeater("item in items track by $index"));
```

The following patterns are not warnings:

```js
element.all(by.repeater("item in items"));
element.all(by.exactRepeater("item in items"));
```
4 changes: 3 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ module.exports = {
'no-describe-selectors': require('./lib/rules/no-describe-selectors'),
'by-css-shortcut': require('./lib/rules/by-css-shortcut'),
'no-angular-classes': require('./lib/rules/no-angular-classes'),
'use-angular-locators': require('./lib/rules/use-angular-locators')
'use-angular-locators': require('./lib/rules/use-angular-locators'),
'use-simple-repeaters': require('./lib/rules/use-simple-repeaters')
},
configs: {
recommended: {
Expand All @@ -23,6 +24,7 @@ module.exports = {
'protractor/no-describe-selectors': 1,
'protractor/no-angular-classes': 1,
'protractor/use-angular-locators': 1,
'protractor/use-simple-repeaters': 1,
'protractor/by-css-shortcut': 0
}
}
Expand Down
29 changes: 29 additions & 0 deletions lib/rules/use-simple-repeaters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use strict'

/**
* @fileoverview Discourage using extended ng-repeat syntax in by.repeater() locators
* @author Alexander Afanasyev
*/

module.exports = function (context) {
return {
'CallExpression': function (node) {
if (node.arguments) {
var object = node.callee.object
var property = node.callee.property

if (object && property && object.name === 'by' && property.name === 'repeater') {
var repeaterValue = node.arguments[0].value

if (repeaterValue.indexOf('|') > 0) {
context.report(node, 'Unexpected filter inside a by.repeater() locator.')
}

if (repeaterValue.indexOf('track by') > 0) {
context.report(node, 'Unexpected "track by" inside a by.repeater() locator.')
}
}
}
}
}
}
40 changes: 40 additions & 0 deletions test/rules/use-simple-repeaters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict'

var rule = require('../../lib/rules/use-simple-repeaters')
var RuleTester = require('eslint').RuleTester

var eslintTester = new RuleTester()

eslintTester.run('use-simple-repeaters', rule, {
valid: [
'element.all(by.repeater("item in items"));',
'element.all(by.exactRepeater("item in items"));'
],

invalid: [
{
code: 'element.all(by.repeater("item in items | filter : x | orderBy : order | limitTo : limit as results"));',
errors: [
{
message: 'Unexpected filter inside a by.repeater() locator.'
}
]
},
{
code: 'element.all(by.repeater("item in items | filter:searchTerm"));',
errors: [
{
message: 'Unexpected filter inside a by.repeater() locator.'
}
]
},
{
code: 'element.all(by.repeater("item in items track by $index"));',
errors: [
{
message: 'Unexpected "track by" inside a by.repeater() locator.'
}
]
}
]
})

0 comments on commit 8a54521

Please sign in to comment.