From 325301b61d30ede9c31340e1ed992f17071e652c Mon Sep 17 00:00:00 2001 From: Alexander Afanasyev Date: Sat, 16 Jul 2016 22:12:57 -0400 Subject: [PATCH] feat(rules): add 'no-absolute-url' rule --- README.md | 2 + docs/rules/no-absolute-url.md | 22 +++++++++++ index.js | 5 ++- lib/is-browser-get.js | 24 ++++++++++++ lib/rules/no-absolute-url.js | 36 ++++++++++++++++++ lib/rules/no-get-in-it.js | 33 +++++++---------- test/rules/no-absolute-url.js | 69 +++++++++++++++++++++++++++++++++++ 7 files changed, 171 insertions(+), 20 deletions(-) create mode 100644 docs/rules/no-absolute-url.md create mode 100644 lib/is-browser-get.js create mode 100644 lib/rules/no-absolute-url.js create mode 100644 test/rules/no-absolute-url.js diff --git a/README.md b/README.md index 09be962..406c318 100644 --- 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 new file mode 100644 index 0000000..541da7e --- /dev/null +++ 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 index afd3f1b..f151434 100644 --- 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 new file mode 100644 index 0000000..972df31 --- /dev/null +++ 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 new file mode 100644 index 0000000..576cc69 --- /dev/null +++ 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 index cd25ca3..b0205e3 100644 --- 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 new file mode 100644 index 0000000..408f55d --- /dev/null +++ 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' + }] + } + ] +})