From 684f7d38615b0c04d98a6a56eab9feb48af4d604 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Sat, 5 Jan 2019 20:20:02 -0800 Subject: [PATCH] feat: add new rule 'no-unnecessary-route-path-option'. --- README.md | 1 + .../rules/no-unnecessary-route-path-option.md | 32 +++++++++ lib/index.js | 1 + lib/recommended-rules.js | 1 + lib/rules/no-unnecessary-route-path-option.js | 61 +++++++++++++++++ .../rules/no-unnecessary-route-path-option.js | 67 +++++++++++++++++++ 6 files changed, 163 insertions(+) create mode 100644 docs/rules/no-unnecessary-route-path-option.md create mode 100644 lib/rules/no-unnecessary-route-path-option.js create mode 100644 tests/lib/rules/no-unnecessary-route-path-option.js diff --git a/README.md b/README.md index e5884b6757..260b0ac102 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,7 @@ The `--fix` option on the command line automatically fixes problems reported by | :white_check_mark::wrench: | [no-old-shims](./docs/rules/no-old-shims.md) | Prevents usage of old shims for modules | | :white_check_mark: | [no-on-calls-in-components](./docs/rules/no-on-calls-in-components.md) | Prevents usage of `on` to call lifecycle hooks in components | | :white_check_mark: | [no-restricted-resolver-tests](./docs/rules/no-restricted-resolver-tests.md) | Prevents the use of patterns that use the restricted resolver in tests. | +| | [no-unnecessary-route-path-option](./docs/rules/no-unnecessary-route-path-option.md) | Disallow unnecessary route `path` option | | :wrench: | [use-ember-get-and-set](./docs/rules/use-ember-get-and-set.md) | Enforces usage of Ember.get and Ember.set | diff --git a/docs/rules/no-unnecessary-route-path-option.md b/docs/rules/no-unnecessary-route-path-option.md new file mode 100644 index 0000000000..a543a0a26c --- /dev/null +++ b/docs/rules/no-unnecessary-route-path-option.md @@ -0,0 +1,32 @@ +## Disallow unnecessary route `path` option + +### Rule name: `no-unnecessary-route-path-option` + +When defining a route, it's not necessary to specify the `path` option if it matches the route name. + +### Rule Details + +Examples of **incorrect** code for this rule: + +```js +this.route('blog-posts', { path: '/blog-posts' }); +``` + +Examples of **correct** code for this rule: + +```js +this.route('blog-posts'); +``` + +```js +this.route('blog-posts', { path: '/blog' }); +``` + +### References + +* [Ember Routing Guide](https://guides.emberjs.com/release/routing/) + +### Related Rules + +* [no-capital-letters-in-routes](no-capital-letters-in-routes.md) +* [routes-segments-snake-case](routes-segments-snake-case.md) diff --git a/lib/index.js b/lib/index.js index 7fd09f2657..91536da784 100644 --- a/lib/index.js +++ b/lib/index.js @@ -32,6 +32,7 @@ module.exports = { 'no-side-effects': require('./rules/no-side-effects'), 'no-test-and-then': require('./rules/no-test-and-then'), 'no-test-import-export': require('./rules/no-test-import-export'), + 'no-unnecessary-route-path-option': require('./rules/no-unnecessary-route-path-option'), 'order-in-components': require('./rules/order-in-components'), 'order-in-controllers': require('./rules/order-in-controllers'), 'order-in-models': require('./rules/order-in-models'), diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js index ec69ec3fd4..8b93e7eda7 100644 --- a/lib/recommended-rules.js +++ b/lib/recommended-rules.js @@ -34,6 +34,7 @@ module.exports = { "ember/no-side-effects": "error", "ember/no-test-and-then": "off", "ember/no-test-import-export": "off", + "ember/no-unnecessary-route-path-option": "off", "ember/order-in-components": "off", "ember/order-in-controllers": "off", "ember/order-in-models": "off", diff --git a/lib/rules/no-unnecessary-route-path-option.js b/lib/rules/no-unnecessary-route-path-option.js new file mode 100644 index 0000000000..8c50c0b3fe --- /dev/null +++ b/lib/rules/no-unnecessary-route-path-option.js @@ -0,0 +1,61 @@ +'use strict'; + +const utils = require('../utils/utils'); +const ember = require('../utils/ember'); + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +const ERROR_MESSAGE = 'Do not provide unnecessary `path` option which matches the route name.'; + +module.exports = { + meta: { + docs: { + description: 'Disallow unnecessary route `path` option', + category: 'Best Practices', + recommended: false + }, + fixable: null, + ERROR_MESSAGE + }, + + create(context) { + return { + CallExpression(node) { + if (!ember.isRoute(node)) { + return; + } + + const hasExplicitPathOption = utils.isObjectExpression(node.arguments[1]) && hasPropertyWithKeyName(node.arguments[1], 'path'); + if (!hasExplicitPathOption) { + return; + } + + const pathOptionNode = getPropertyByKeyName(node.arguments[1], 'path'); + const pathOptionValue = pathOptionNode.value.value; + const routeName = node.arguments[0].value; + + if (pathMatchesRouteName(pathOptionValue, routeName)) { + context.report({ + node: pathOptionNode, + message: ERROR_MESSAGE + }); + } + } + }; + } +}; + +function hasPropertyWithKeyName(objectExpression, keyName) { + return getPropertyByKeyName(objectExpression, keyName) !== undefined; +} + +function getPropertyByKeyName(objectExpression, keyName) { + return objectExpression.properties.find(property => property.key.name === keyName); +} + +function pathMatchesRouteName(path, routeName) { + const pathWithoutOptionalLeadingSlash = path.substring(0, 1) === '/' ? path.substring(1) : path; + return pathWithoutOptionalLeadingSlash === routeName; +} diff --git a/tests/lib/rules/no-unnecessary-route-path-option.js b/tests/lib/rules/no-unnecessary-route-path-option.js new file mode 100644 index 0000000000..19beb7a4ab --- /dev/null +++ b/tests/lib/rules/no-unnecessary-route-path-option.js @@ -0,0 +1,67 @@ +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-unnecessary-route-path-option'); +const RuleTester = require('eslint').RuleTester; + +const { ERROR_MESSAGE } = rule; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester(); +ruleTester.run('no-unnecessary-route-path-option', rule, { + valid: [ + 'this.route("blog");', + 'this.route("blog", function() {});', + 'this.route("blog", { path: "/" });', + 'this.route("blog", { path: "blog-posts" });', + 'this.route("blog", { path: "blog-posts" }, function() {});', + 'this.route("blog", { path: "/blog-posts" });', + 'this.route("blog", { path: "blog-posts", otherOption: true });', + + // With dynamic segment: + 'this.route("blog", { path: ":blog" });', + 'this.route("blog", { path: "/:blog" });', + 'this.route("blog", { path: "blog/:blog_id" });', + + // With wildcard segment: + 'this.route("blog", { path: "*blog" });', + 'this.route("blog", { path: "/*blog" });', + 'this.route("blog", { path: "blog/*blog" });', + + // Not Ember's route function: + 'test();', + "test('blog');", + "test('blog', { path: 'blog' });", + "test('blog', { path: '/blog' });", + "this.test('blog');", + "this.test('blog', { path: 'blog' });", + "this.test('blog', { path: '/blog' });", + "MyClass.route('blog');", + "MyClass.route('blog', { path: 'blog' });", + "MyClass.route('blog', { path: '/blog' });", + "route.unrelatedFunction('blog', { path: 'blog' });", + "this.route.unrelatedFunction('blog', { path: 'blog' });" + ], + invalid: [ + { + code: 'this.route("blog", { path: "blog" });', + errors: [{ message: ERROR_MESSAGE, type: 'Property' }] + }, + { + code: 'this.route("blog", { path: "blog" }, function() {});', + errors: [{ message: ERROR_MESSAGE, type: 'Property' }] + }, + { + code: 'this.route("blog", { path: "/blog" });', + errors: [{ message: ERROR_MESSAGE, type: 'Property' }] + }, + { + code: 'this.route("blog", { path: "/blog", otherOption: true });', + errors: [{ message: ERROR_MESSAGE, type: 'Property' }] + } + ] +});