feat(rules): add 'no-absolute-url' rule

alecxe · Jul 17, 2016 · 325301b · 325301b
1 parent 9cfd024
commit 325301b
Show file tree

Hide file tree

Showing 7 changed files with 171 additions and 20 deletions.
diff --git a/README.md b/README.md
@@ -49,6 +49,7 @@ Rule                         | Default       | Options
 [use-first-last][]           | 1             |
 [no-get-in-it][]             | 1             |
 [array-callback-return][]    | 1             |
+[no-absolute-url][]          | 1             |
 [by-css-shortcut][]          | 0             |
 
 For example, the `missing-perform` rule is enabled by default and will cause
@@ -79,6 +80,7 @@ See [configuring rules][] for more information.
 [use-first-last]: docs/rules/use-first-last.md
 [no-get-in-it]: docs/rules/no-get-in-it.md
 [array-callback-return]: docs/rules/array-callback-return.md
+[no-absolute-url]: docs/rules/no-absolute-url.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/no-absolute-url.md b/docs/rules/no-absolute-url.md
@@ -0,0 +1,22 @@
+# Recommend against navigating to absolute URLs inside `browser.get()` or `browser.driver.get()`
+
+The rule would warn if `browser.get()` or `browser.driver.get()` are called with an absolute URL as an argument. 
+
+Instead, it is recommended to have [`baseUrl` configuration option](https://github.com/angular/protractor/blob/master/docs/referenceConf.js) set and navigating to relative URLs in tests.
+This helps to easily switch to a different target application URL by simply changing the `baseUrl` setting (e.g. switch from dev to test or staging).
+
+## Rule details
+
+Any use of the following patterns are considered warnings:
+
+```js
+browser.get("http://google.com");
+browser.driver.get("https://google.com");
+```
+
+The following patterns are not warnings:
+
+```js
+browser.get("login");
+browser.driver.get("account/signup");
+```
diff --git a/index.js b/index.js
@@ -14,6 +14,7 @@ var noShadowing = require('./lib/rules/no-shadowing')
 var useFirstLast = require('./lib/rules/use-first-last')
 var noGetInIt = require('./lib/rules/no-get-in-it')
 var arrayCallbackReturn = require('./lib/rules/array-callback-return')
+var noAbsoluteURL = require('./lib/rules/no-absolute-url')
 
 module.exports = {
   rules: {
@@ -30,7 +31,8 @@ module.exports = {
     'no-shadowing': noShadowing,
     'use-first-last': useFirstLast,
     'no-get-in-it': noGetInIt,
-    'array-callback-return': arrayCallbackReturn
+    'array-callback-return': arrayCallbackReturn,
+    'no-absolute-url': noAbsoluteURL
   },
   configs: {
     recommended: {
@@ -48,6 +50,7 @@ module.exports = {
         'protractor/use-first-last': 1,
         'protractor/no-get-in-it': 1,
         'protractor/array-callback-return': 1,
+        'protractor/no-absolute-url': 1,
         'protractor/by-css-shortcut': 0
       },
       globals: {

diff --git a/lib/is-browser-get.js b/lib/is-browser-get.js
@@ -0,0 +1,24 @@
+'use strict'
+
+/**
+ * @fileoverview Utility function to determine if a node is a browser.get() or browser.driver.get() call
+ * @author Alexander Afanasyev
+ */
+module.exports = function (node) {
+  var object = node.callee.object
+  var property = node.callee.property
+
+  if (object && property && property.name === 'get') {
+    var isBrowser = object.name === 'browser'
+    var isBrowserDriver = object.object && object.object.name === 'browser' &&
+                                           object.property && object.property.name === 'driver'
+    if (isBrowser || isBrowserDriver) {
+      return {
+        'browserGet': isBrowser,
+        'browserDriverGet': isBrowserDriver
+      }
+    }
+  }
+
+  return false
+}
diff --git a/lib/rules/no-absolute-url.js b/lib/rules/no-absolute-url.js
@@ -0,0 +1,36 @@
+'use strict'
+
+/**
+ * @fileoverview Recommend against navigating to absolute URLs inside `browser.get()` or `browser.driver.get()`
+ * @author Alexander Afanasyev
+ */
+
+var isBrowserGet = require('../is-browser-get')
+var isAbsoluteURL = new RegExp('^(?:[a-z]+:)?//', 'i')
+
+module.exports = {
+  meta: {
+    schema: []
+  },
+
+  create: function (context) {
+    return {
+      'CallExpression': function (node) {
+        if (node.arguments && node.arguments[0]) {
+          var result = isBrowserGet(node)
+
+          if (result) {
+            var url = node.arguments[0].value
+
+            if (isAbsoluteURL.test(url)) {
+              context.report({
+                node: node,
+                message: 'Unexpected absolute URL'
+              })
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/lib/rules/no-get-in-it.js b/lib/rules/no-get-in-it.js
@@ -5,6 +5,8 @@
  * @author Alexander Afanasyev
  */
 
+var isBrowserGet = require('../is-browser-get')
+
 var testFunctionNames = [
   'it'
 ]
@@ -17,26 +19,19 @@ module.exports = {
   create: function (context) {
     return {
       'CallExpression': function (node) {
-        var object = node.callee.object
-        var property = node.callee.property
-
-        if (object && property && property.name === 'get') {
-          var isBrowserGet = object.name === 'browser'
-          var isBrowserDriverGet = object.object && object.object.name === 'browser' &&
-                                   object.property && object.property.name === 'driver'
+        var result = isBrowserGet(node)
 
-          if (isBrowserGet || isBrowserDriverGet) {
-            // Use ancestors to determine if we are inside the it() block currently
-            for (var i = 0; i < context.getAncestors().length; i++) {
-              var parent = context.getAncestors()[i]
-              if (parent.type === 'CallExpression' && testFunctionNames.indexOf(parent.callee.name) >= 0) {
-                var methodName = isBrowserGet ? 'browser.get()' : 'browser.driver.get()'
-                context.report({
-                  node: node,
-                  message: 'Unexpected "' + methodName + '" inside it'
-                })
-                break
-              }
+        if (result) {
+          // Use ancestors to determine if we are inside the it() block currently
+          for (var i = 0; i < context.getAncestors().length; i++) {
+            var parent = context.getAncestors()[i]
+            if (parent.type === 'CallExpression' && testFunctionNames.indexOf(parent.callee.name) >= 0) {
+              var methodName = result.browserGet ? 'browser.get()' : 'browser.driver.get()'
+              context.report({
+                node: node,
+                message: 'Unexpected "' + methodName + '" inside it'
+              })
+              break
             }
           }
         }

diff --git a/test/rules/no-absolute-url.js b/test/rules/no-absolute-url.js
@@ -0,0 +1,69 @@
+'use strict'
+
+var rule = require('../../lib/rules/no-absolute-url')
+var RuleTester = require('eslint').RuleTester
+var eslintTester = new RuleTester()
+
+eslintTester.run('no-absolute-url', rule, {
+  valid: [
+    'browser.get();',
+    'browser.driver.get();',
+    'browser.get("mypage");',
+    'browser.driver.get("mypage");',
+    'browser.get("MYPAGE");',
+    'browser.driver.get("MYPAGE");',
+    'browser.get("/mypage/mypage");',
+    'browser.driver.get("/mypage/mypage");'
+  ],
+
+  invalid: [
+    {
+      code: 'browser.get("http://google.com");',
+      errors: [{
+        message: 'Unexpected absolute URL'
+      }]
+    },
+    {
+      code: 'browser.driver.get("https://google.com");',
+      errors: [{
+        message: 'Unexpected absolute URL'
+      }]
+    },
+    {
+      code: 'browser.get("HTTP://google.com");',
+      errors: [{
+        message: 'Unexpected absolute URL'
+      }]
+    },
+    {
+      code: 'browser.driver.get("HTTPS://google.com");',
+      errors: [{
+        message: 'Unexpected absolute URL'
+      }]
+    },
+    {
+      code: 'browser.get("ftp://google.com");',
+      errors: [{
+        message: 'Unexpected absolute URL'
+      }]
+    },
+    {
+      code: 'browser.driver.get("ftp://google.com");',
+      errors: [{
+        message: 'Unexpected absolute URL'
+      }]
+    },
+    {
+      code: 'browser.get("//google.com");',
+      errors: [{
+        message: 'Unexpected absolute URL'
+      }]
+    },
+    {
+      code: 'browser.driver.get("//google.com");',
+      errors: [{
+        message: 'Unexpected absolute URL'
+      }]
+    }
+  ]
+})