From 8a545216260c122afb30fbaa6258af060a53293c Mon Sep 17 00:00:00 2001 From: Alexander Afanasyev Date: Fri, 10 Jun 2016 00:44:03 -0400 Subject: [PATCH] feat(rules): add 'use-simple-repeaters' rule --- README.md | 2 ++ docs/rules/use-simple-repeaters.md | 38 ++++++++++++++++++++++++++++ index.js | 4 ++- lib/rules/use-simple-repeaters.js | 29 ++++++++++++++++++++++ test/rules/use-simple-repeaters.js | 40 ++++++++++++++++++++++++++++++ 5 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 docs/rules/use-simple-repeaters.md create mode 100644 lib/rules/use-simple-repeaters.js create mode 100644 test/rules/use-simple-repeaters.js diff --git a/README.md b/README.md index f9702f3..7ef1c8d 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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 diff --git a/docs/rules/use-simple-repeaters.md b/docs/rules/use-simple-repeaters.md new file mode 100644 index 0000000..ffeb0f1 --- /dev/null +++ b/docs/rules/use-simple-repeaters.md @@ -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: + +
...
+ +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")); +``` diff --git a/index.js b/index.js index 9aa27a8..8da7605 100644 --- a/index.js +++ b/index.js @@ -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: { @@ -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 } } diff --git a/lib/rules/use-simple-repeaters.js b/lib/rules/use-simple-repeaters.js new file mode 100644 index 0000000..c193c2e --- /dev/null +++ b/lib/rules/use-simple-repeaters.js @@ -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.') + } + } + } + } + } +} diff --git a/test/rules/use-simple-repeaters.js b/test/rules/use-simple-repeaters.js new file mode 100644 index 0000000..d0e6408 --- /dev/null +++ b/test/rules/use-simple-repeaters.js @@ -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.' + } + ] + } + ] +})