Skip to content

Commit

Permalink
feat(rules): add 'user-count-method' rule
Browse files Browse the repository at this point in the history
  • Loading branch information
alecxe committed Dec 12, 2016
1 parent 640eeef commit 3927313
Show file tree
Hide file tree
Showing 7 changed files with 347 additions and 1 deletion.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Rule | Default Error Level | Auto-fixable | Options
[no-repetitive-locators][] | 1 | |
[no-repetitive-selectors][] | 1 | |
[no-get-inner-outer-html][] | 1 | |
[use-count-method][] | 1 | |
[use-promise-all][] | 0 (Turned off) | |
[by-css-shortcut][] | 0 | |

Expand Down Expand Up @@ -117,6 +118,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
[use-count-method]: docs/rules/use-count-method.md
[configuring rules]: http://eslint.org/docs/user-guide/configuring#configuring-rules

## Recommended configuration
Expand Down
42 changes: 42 additions & 0 deletions docs/rules/use-count-method.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Recommend using `count()` instead of `then()` and `length`

When you need to assert how many elements were found, it is more readable and easier when you do it using [`count()`](http://www.protractortest.org/#/api?view=ElementArrayFinder.prototype.count):

```js
expect(element.all(by.repeater('product in products')).count()).toBeGreaterThan(1);
```

instead of resolving the `ElementArrayFinder` value and getting the `.length` property:

```js
element.all(by.repeater('product in products')).then(function (products) {
expect(products.length >= 1).toBeTruthy();
});
```

## Rule details

Any use of the following patterns are considered warnings:

```js
element.all(by.repeater('product in products')).then(function (products) {
expect(products.length >= 1).toBeTruthy();
});

element.all(by.repeater('product in products')).then(function (products) {
expect(10).toEqual(products.length);
});
```

The following patterns are not warnings:

```js
expect(element.all(by.repeater('product in products')).count()).toBeGreaterThan(1);

var products = [];
console.log(products.length);

element.all(by.repeater('product in products')).then(function (products) {
console.log('test');
});
```
5 changes: 4 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ var usePromiseAll = require('./lib/rules/use-promise-all')
var noArrayFinderMethods = require('./lib/rules/no-array-finder-methods')
var validLocatorType = require('./lib/rules/valid-locator-type')
var noCompoundClasses = require('./lib/rules/no-compound-classes')
var useCountMethod = require('./lib/rules/use-count-method')

module.exports = {
rules: {
Expand Down Expand Up @@ -60,7 +61,8 @@ module.exports = {
'use-promise-all': usePromiseAll,
'no-array-finder-methods': noArrayFinderMethods,
'valid-locator-type': validLocatorType,
'no-compound-classes': noCompoundClasses
'no-compound-classes': noCompoundClasses,
'use-count-method': useCountMethod
},
configs: {
recommended: {
Expand Down Expand Up @@ -92,6 +94,7 @@ module.exports = {
'protractor/no-repetitive-selectors': 1,
'protractor/no-get-inner-outer-html': 1,
'protractor/no-angular-attributes': 1,
'protractor/use-count-method': 1,
'protractor/use-promise-all': 0,
'protractor/by-css-shortcut': 0
},
Expand Down
24 changes: 24 additions & 0 deletions lib/is-expect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict'

/**
* Checks if a given node is an "expect" statement - works for both left and right parts
*
* @param {ASTNode} node - A node to check.
* @returns {boolean}
*/
module.exports = function (node) {
var callee = node.callee

// left part of "expect()"
if (callee && callee.name === 'expect') {
return true
}

// right part of "expect()"
if (callee.object && callee.object.type === 'CallExpression' && callee.object.callee) {
callee = callee.object.callee
if (callee.name && callee.name === 'expect') {
return true
}
}
}
24 changes: 24 additions & 0 deletions lib/is-then-callback.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict'

/**
* Checks if a given node is a "then" callback function
*
* @param {ASTNode} node - A node to check.
* @returns {ASTNode} node - The first function argument node.
*/
module.exports = function (node) {
var property = node.callee.property
var isThen = property && property.name === 'then' && node.arguments
if (isThen) {
var argument = node.arguments[0]
// only function type allowed
var isFunction = argument && (argument.type === 'FunctionExpression' || argument.type === 'ArrowFunctionExpression')
if (isFunction) {
// it has to have at least one argument
if (argument.params && argument.params.length > 0) {
return argument.params[0]
}
}
}
return false
}
60 changes: 60 additions & 0 deletions lib/rules/use-count-method.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
'use strict'

/**
* @fileoverview Recommend using `count()` instead of `then()` and `length`
* @author Alexander Afanasyev
*/

var isElementArrayFinder = require('../is-element-array-finder')
var isThenCallBack = require('../is-then-callback')
var isExpect = require('../is-expect')

module.exports = {
meta: {
schema: []
},

create: function (context) {
return {
MemberExpression: function (node) {
if (node.property && node.object && node.property.name === 'length') {
// remember the variable name the ".length" was used on
var variableName = node.object.name

// find out if we are in an expect()
var expectAncestor
var thenAncestor
var ancestors = context.getAncestors(node)

for (var i = 0; i < ancestors.length; i++) {
expectAncestor = ancestors[i]
if (expectAncestor && expectAncestor.type === 'CallExpression' && isExpect(expectAncestor)) {
// find out if we are inside a then callback
ancestors = context.getAncestors(expectAncestor)
for (var j = 0; j < ancestors.length; j++) {
thenAncestor = ancestors[j]
if (thenAncestor && thenAncestor.type === 'CallExpression') {
var thenCallbackArgument = isThenCallBack(thenAncestor)

// the same variable is a "then" callback function argument
if (thenCallbackArgument && thenCallbackArgument.name === variableName) {
// check that it was an ElementArrayFinder resolution
if (thenAncestor.callee && thenAncestor.callee.object) {
if (isElementArrayFinder(thenAncestor.callee.object)) {
context.report({
node: node,
message: 'Array.length inside promise resolution function detected. Use count() instead.'
})
return
}
}
}
}
}
}
}
}
}
}
}
}
191 changes: 191 additions & 0 deletions test/rules/use-count-method.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
'use strict'

var rule = require('../../lib/rules/use-count-method')
var RuleTester = require('eslint').RuleTester
var toCode = require('../lib/join-multiline-code')

var eslintTester = new RuleTester()

eslintTester.run('use-count-method', rule, {
valid: [
toCode([
'expect(element.all(by.repeater("product in products")).count()).toBeGreaterThan(1);'
]),
toCode([
'var products = [];',
'console.log(products.length);'
]),
toCode([
'element.all(by.repeater("product in products")).then(function (products) {',
' console.log("test");',
'});'
]),
toCode([
'element.all(by.repeater("product in products")).then(function (products) {',
' var testArray = [1, 2, 3];',
' expect(testArray.length).toEqual(3)',
'});'
]),
toCode([
'element.all(by.repeater("product in products")).then(function (products) {',
' var testArray = [1, 2, 3];',
' expect(3).toEqual(testArray.length)',
'});'
]),
toCode([
'element.all(by.repeater("product in products")).then(function () {',
'});'
]),
toCode([
'element.all(by.repeater("product in products")).then(function () {',
' var testArray = [1, 2, 3];',
' expect(3).toEqual(testArray.length)',
'});'
]),
toCode([
'element.all(by.repeater("product in products")).filter(function (elm) {',
' return true;',
'});'
]),
toCode([
'element.all(by.repeater("product in products")).then(console.log);'
]),
toCode([
'somePromise.then(function (products) {',
' expect(products.length).toEqual(1);',
'});'
]),
toCode([
'then(function (products) {',
' expect(products.length).toEqual(1);',
'});'
]),
toCode([
'var products = [1, 2, 3];',
'element.all(by.repeater("product in products")).then(function () {',
' expect(products.length).toEqual(1);',
'});'
]),
toCode([
'promise.method().then(function (products) {',
' expect(products.length).toEqual(1);',
'});'
])
// TODO: promise.all().then() - recognized as an ElementArrayFinder!
// TODO: run against ap-ui-html
],

invalid: [
{
code: toCode([
'element.all(by.repeater("product in products")).then(function (products) {',
' expect(products.length >= 1).toBeTruthy();',
'});'
]),
errors: [
{
message: 'Array.length inside promise resolution function detected. Use count() instead.'
}
]
},
{
code: toCode([
'element.all(by.repeater("product in products")).then((products) => {',
' expect(products.length >= 1).toBeTruthy();',
'});'
]),
parserOptions: {
ecmaVersion: 6
},
errors: [
{
message: 'Array.length inside promise resolution function detected. Use count() instead.'
}
]
},
{
code: toCode([
'element.all(by.repeater("product in products")).then(function (products) {',
' expect(1).toEqual(products.length);',
'});'
]),
errors: [
{
message: 'Array.length inside promise resolution function detected. Use count() instead.'
}
]
},
{
code: toCode([
'element.all(by.repeater("product in products")).then((products) => {',
' expect(1).toEqual(products.length);',
'});'
]),
parserOptions: {
ecmaVersion: 6
},
errors: [
{
message: 'Array.length inside promise resolution function detected. Use count() instead.'
}
]
},
{
code: toCode([
'element.all(by.repeater("product in products")).then(function (products) {',
' expect(products.length).toEqual(products.length);',
'});'
]),
errors: [
{
message: 'Array.length inside promise resolution function detected. Use count() instead.'
},
{
message: 'Array.length inside promise resolution function detected. Use count() instead.'
}
]
},
{
code: toCode([
'element.all(by.repeater("product in products")).then((products) => {',
' expect(products.length).toEqual(products.length);',
'});'
]),
parserOptions: {
ecmaVersion: 6
},
errors: [
{
message: 'Array.length inside promise resolution function detected. Use count() instead.'
},
{
message: 'Array.length inside promise resolution function detected. Use count() instead.'
}
]
},
{
code: toCode([
'$$(".product").then(function (products) {',
' expect(products.length >= 1).toBeTruthy();',
'});'
]),
errors: [
{
message: 'Array.length inside promise resolution function detected. Use count() instead.'
}
]
},
{
code: toCode([
'$$(".product").then(function (products) {',
' expect(1).toEqual(products.length);',
'});'
]),
errors: [
{
message: 'Array.length inside promise resolution function detected. Use count() instead.'
}
]
}
]
})

0 comments on commit 3927313

Please sign in to comment.