feat(rules): add 'use-first-last' rule

Author: alecxe

README.md

@@ -43,6 +43,7 @@ Rule                         | Default       | Options
 [use-angular-locators][]     | 1             |
 [use-simple-repeaters][]     | 1             |
 [no-shadowing][]             | 1             |
+[use-first-last][]           | 1             |
 [by-css-shortcut][]          | 0             |
 
 For example, the `missing-perform` rule is enabled by default and will cause
@@ -70,6 +71,7 @@ See [configuring rules][] for more information.
 [use-angular-locators]: docs/rules/use-angular-locators.md
 [use-simple-repeaters]: docs/rules/use-simple-repeaters.md
 [no-shadowing]: docs/rules/no-shadowing.md
+[use-first-last]: docs/rules/use-first-last.md
 [by-css-shortcut]: docs/rules/by-css-shortcut.md
 [configuring rules]: http://eslint.org/docs/user-guide/configuring#configuring-rules
 

docs/rules/use-first-last.md

@@ -0,0 +1,31 @@
+# Recommend using `first()` instead of `get(0)` and `last()` instead of `get(-1)`
+
+The `first()` and `last()` shortcuts is, generally speaking, more readable than `get(0)` and `get(-1)`.
+
+## Rule details
+
+Any use of the following patterns are considered warnings:
+
+```js
+element.all(by.css(".class")).get(0);
+element(by.id("id")).all(by.css(".class")).get(-1);
+$$(".class").get(0);
+element(by.id("id")).$$(".class").get(-1);
+element.all(by.css(".class")).get(0).getText();
+element.all(by.css(".class")).get(-1).getText();
+```
+
+The following patterns are not warnings:
+
+```js
+element.all(by.css(".class")).first();
+element(by.id("id")).all(by.css(".class")).last();
+$$(".class").first();
+element(by.id("id")).$$(".class").last();
+element.all(by.css(".class")).get(1);
+element(by.id("id")).all(by.css(".class")).get(-10).getText();
+$$(".class").get(10);
+element(by.id("id")).$$(".class").get(-10);
+var myMap = new Map(); myMap.get(0);
+var myMap = new Map(); myMap.get(-1);
+```

index.js

@@ -12,7 +12,8 @@ module.exports = {
     'no-angular-classes': require('./lib/rules/no-angular-classes'),
     'use-angular-locators': require('./lib/rules/use-angular-locators'),
     'use-simple-repeaters': require('./lib/rules/use-simple-repeaters'),
-    'no-shadowing': require('./lib/rules/no-shadowing')
+    'no-shadowing': require('./lib/rules/no-shadowing'),
+    'use-first-last': require('./lib/rules/use-first-last')
   },
   configs: {
     recommended: {
@@ -27,6 +28,7 @@ module.exports = {
         'protractor/use-angular-locators': 1,
         'protractor/use-simple-repeaters': 1,
         'protractor/no-shadowing': 1,
+        'protractor/use-first-last': 1,
         'protractor/by-css-shortcut': 0
       }
     }

lib/rules/use-first-last.js

@@ -0,0 +1,40 @@
+'use strict'
+
+/**
+ * @fileoverview Recommend using `first()` instead of `get(0)` and `last()` instead of `get(-1)`
+ * @author Alexander Afanasyev
+ */
+
+module.exports = function (context) {
+  var valueFunction = {
+    '0': 'first()',
+    '-1': 'last()'
+  }
+
+  return {
+    CallExpression: function (node) {
+      var property = node.callee.property
+
+      if (property && property.name === 'get' && node.arguments) {
+        var argument = node.arguments[0]
+        var argumentValue
+
+        if (argument.value === 0) {
+          argumentValue = 0
+        } else if (argument.operator === '-' && argument.argument && argument.argument.value === 1) {
+          argumentValue = -1
+        }
+
+        if (argumentValue === 0 || argumentValue === -1) {
+          var object = node.callee.object
+          var callee = object.callee
+
+          if (callee && ((callee.property && (callee.property.name === 'all' || callee.property.name === '$$')) ||
+                         (callee.name === '$$'))) {
+            context.report(node, 'Unexpected "get(' + argumentValue + ')" call, use "' + valueFunction[argumentValue] + '" instead')
+          }
+        }
+      }
+    }
+  }
+}

test/rules/use-first-last.js

@@ -0,0 +1,72 @@
+'use strict'
+
+var rule = require('../../lib/rules/use-first-last')
+var RuleTester = require('eslint').RuleTester
+
+var eslintTester = new RuleTester()
+
+eslintTester.run('use-first-last', rule, {
+  valid: [
+    'element.all(by.css(".class")).first();',
+    'element(by.id("id")).all(by.css(".class")).last();',
+    '$$(".class").first();',
+    'element(by.id("id")).$$(".class").last();',
+    'element.all(by.css(".class")).get(1);',
+    'element(by.id("id")).all(by.css(".class")).get(-10).getText();',
+    '$$(".class").get(10);',
+    'element(by.id("id")).$$(".class").get(-10);',
+    'var myMap = new Map(); myMap.get(0);',
+    'var myMap = new Map(); myMap.get(-1);'
+  ],
+
+  invalid: [
+    {
+      code: 'element.all(by.css(".class")).get(0);',
+      errors: [
+        {
+          message: 'Unexpected "get(0)" call, use "first()" instead'
+        }
+      ]
+    },
+    {
+      code: 'element(by.id("id")).all(by.css(".class")).get(-1);',
+      errors: [
+        {
+          message: 'Unexpected "get(-1)" call, use "last()" instead'
+        }
+      ]
+    },
+    {
+      code: '$$(".class").get(0);',
+      errors: [
+        {
+          message: 'Unexpected "get(0)" call, use "first()" instead'
+        }
+      ]
+    },
+    {
+      code: 'element(by.id("id")).$$(".class").get(-1);',
+      errors: [
+        {
+          message: 'Unexpected "get(-1)" call, use "last()" instead'
+        }
+      ]
+    },
+    {
+      code: 'element.all(by.css(".class")).get(0).getText();',
+      errors: [
+        {
+          message: 'Unexpected "get(0)" call, use "first()" instead'
+        }
+      ]
+    },
+    {
+      code: 'element.all(by.css(".class")).get(-1).getText();',
+      errors: [
+        {
+          message: 'Unexpected "get(-1)" call, use "last()" instead'
+        }
+      ]
+    }
+  ]
+})