Skip to content

Commit

Permalink
Merge pull request #370 from bmish/no-unnecessary-route-path-option
Browse files Browse the repository at this point in the history
feat: add new rule 'no-unnecessary-route-path-option'.
  • Loading branch information
Turbo87 committed Jan 10, 2019
2 parents d842ef6 + 684f7d3 commit 16e0a6f
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -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 |


Expand Down
32 changes: 32 additions & 0 deletions 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)
1 change: 1 addition & 0 deletions lib/index.js
Expand Up @@ -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'),
Expand Down
1 change: 1 addition & 0 deletions lib/recommended-rules.js
Expand Up @@ -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",
Expand Down
61 changes: 61 additions & 0 deletions 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;
}
67 changes: 67 additions & 0 deletions 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' }]
}
]
});

0 comments on commit 16e0a6f

Please sign in to comment.