diff --git a/README.md b/README.md index 519e0429cf..0417286ffd 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,7 @@ Each rule has emojis denoting what configuration it belongs to and/or a :wrench: | :white_check_mark: | [closure-actions](./docs/rules/closure-actions.md) | enforce usage of closure actions | | | [named-functions-in-promises](./docs/rules/named-functions-in-promises.md) | enforce usage of named functions in promises | | :white_check_mark: | [new-module-imports](./docs/rules/new-module-imports.md) | enforce using "New Module Imports" from Ember RFC #176 | +| | [no-controllers](./docs/rules/no-controllers.md) | disallow non-essential controllers | | :white_check_mark: | [no-function-prototype-extensions](./docs/rules/no-function-prototype-extensions.md) | disallow usage of Ember's `function` prototype extensions | | :car: | [no-get-with-default](./docs/rules/no-get-with-default.md) | disallow usage of the Ember's `getWithDefault` function | | :car::wrench: | [no-get](./docs/rules/no-get.md) | require using ES5 getters instead of Ember's `get` / `getProperties` functions | diff --git a/docs/rules/no-controllers.md b/docs/rules/no-controllers.md new file mode 100644 index 0000000000..8960b4bf41 --- /dev/null +++ b/docs/rules/no-controllers.md @@ -0,0 +1,35 @@ +# no-controllers + +Some people may prefer to avoid the use of controllers in their applications, typically in favor of components which can be more portable and easier to test. + +This rule disallows controller usage, except when the controller is used to define `queryParams`. + +## Examples + +Examples of **incorrect** code for this rule: + +```js +import Controller from '@ember/controller'; +export default class ArticlesController extends Controller { + // Controller disallowed since it doesn't use `queryParams`. + ... +} +``` + +Examples of **correct** code for this rule: + +```js +import Controller from '@ember/controller'; +export default class ArticlesController extends Controller { + queryParams = ['category']; // Controller allowed for defining `queryParams`. + @tracked category = null; + ... +} +``` + +Note that this rule supports both classic and native class syntax. + +## Further Reading + +- [Guide on controllers](https://guides.emberjs.com/release/routing/controllers/) +- [Guide on `queryParameters`](https://guides.emberjs.com/release/routing/query-params/) diff --git a/lib/index.js b/lib/index.js index 1f2163b2b4..dd348d6d7c 100644 --- a/lib/index.js +++ b/lib/index.js @@ -20,6 +20,7 @@ module.exports = { 'no-classic-classes': require('./rules/no-classic-classes'), 'no-classic-components': require('./rules/no-classic-components'), 'no-computed-properties-in-native-classes': require('./rules/no-computed-properties-in-native-classes'), + 'no-controllers': require('./rules/no-controllers'), 'no-deeply-nested-dependent-keys-with-each': require('./rules/no-deeply-nested-dependent-keys-with-each'), 'no-duplicate-dependent-keys': require('./rules/no-duplicate-dependent-keys'), 'no-ember-super-in-es-classes': require('./rules/no-ember-super-in-es-classes'), diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js index 5640f97752..1b71784541 100644 --- a/lib/recommended-rules.js +++ b/lib/recommended-rules.js @@ -23,6 +23,7 @@ module.exports = { "ember/no-classic-classes": "off", "ember/no-classic-components": "off", "ember/no-computed-properties-in-native-classes": "off", + "ember/no-controllers": "off", "ember/no-deeply-nested-dependent-keys-with-each": "error", "ember/no-duplicate-dependent-keys": "error", "ember/no-ember-super-in-es-classes": "error", diff --git a/lib/rules/no-controllers.js b/lib/rules/no-controllers.js new file mode 100644 index 0000000000..638ea8abd9 --- /dev/null +++ b/lib/rules/no-controllers.js @@ -0,0 +1,67 @@ +const ember = require('../utils/ember'); +const types = require('../utils/types'); +const assert = require('assert'); + +const ERROR_MESSAGE = 'Avoid using controllers except for specifying `queryParams`'; + +module.exports = { + ERROR_MESSAGE, + meta: { + type: 'suggestion', + docs: { + description: 'disallow non-essential controllers', + category: 'Best Practices', + recommended: false, + url: + 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-controllers.md', + }, + fixable: null, + schema: [], + }, + + create: context => { + return { + ClassDeclaration(node) { + if ( + ember.isEmberController(context, node) && + (node.body.body.length === 0 || !classDeclarationHasProperty(node, 'queryParams')) + ) { + context.report(node, ERROR_MESSAGE); + } + }, + + CallExpression(node) { + if ( + ember.isEmberController(context, node) && + (node.arguments.length === 0 || !callExpressionClassHasProperty(node, 'queryParams')) + ) { + context.report(node, ERROR_MESSAGE); + } + }, + }; + }, +}; + +function classDeclarationHasProperty(classDeclaration, propertyName) { + assert(types.isClassDeclaration(classDeclaration)); + return ( + classDeclaration.body.body.length >= 1 && + classDeclaration.body.body.some( + item => + types.isClassProperty(item) && + types.isIdentifier(item.key) && + item.key.name === propertyName + ) + ); +} + +function callExpressionClassHasProperty(callExpression, propertyName) { + assert(types.isCallExpression(callExpression)); + return ( + callExpression.arguments.length >= 1 && + types.isObjectExpression(callExpression.arguments[callExpression.arguments.length - 1]) && + callExpression.arguments[callExpression.arguments.length - 1].properties.some( + prop => types.isIdentifier(prop.key) && prop.key.name === propertyName + ) + ); +} diff --git a/tests/lib/rules/no-controllers.js b/tests/lib/rules/no-controllers.js new file mode 100644 index 0000000000..3eeeb49c00 --- /dev/null +++ b/tests/lib/rules/no-controllers.js @@ -0,0 +1,117 @@ +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-controllers'); +const RuleTester = require('eslint').RuleTester; + +const { ERROR_MESSAGE } = rule; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ + parser: require.resolve('babel-eslint'), + parserOptions: { ecmaVersion: 6, sourceType: 'module' }, +}); +ruleTester.run('no-controllers', rule, { + valid: [ + ` + import Controller from '@ember/controller'; + export default Controller.extend({ + someFunction() {}, + query: null, + sortType: null, + sortOrder: null, + queryParams: ['query', 'sortType', 'sortOrder'] + }); + `, + ` + import Controller from '@ember/controller'; + export default class ArticlesController extends Controller { + get filteredArticles() {} + @tracked category = null; + queryParams = ['category']; + } + `, + ], + + invalid: [ + // *************** + // Legacy classes: + // *************** + + { + code: ` + import Controller from '@ember/controller'; + export default Controller.extend(); + `, + output: null, + errors: [{ type: 'CallExpression', message: ERROR_MESSAGE }], + }, + { + code: ` + import Controller from '@ember/controller'; + export default Controller.extend(SomeMixin); + `, + output: null, + errors: [{ type: 'CallExpression', message: ERROR_MESSAGE }], + }, + { + code: ` + import Controller from '@ember/controller'; + export default Controller.extend({}); + `, + output: null, + errors: [{ type: 'CallExpression', message: ERROR_MESSAGE }], + }, + { + code: ` + import Controller from '@ember/controller'; + export default Controller.extend({ + randomProperty: true, + randomFunction() {} + }); + `, + output: null, + errors: [{ type: 'CallExpression', message: ERROR_MESSAGE }], + }, + { + // With mixin. + code: ` + import Controller from '@ember/controller'; + export default Controller.extend(SomeMixin, { + randomProperty: true, + randomFunction() {} + }); + `, + output: null, + errors: [{ type: 'CallExpression', message: ERROR_MESSAGE }], + }, + + // *************** + // Native classes: + // *************** + + { + code: ` + import Controller from '@ember/controller'; + export default class ArticlesController extends Controller {} + `, + output: null, + errors: [{ type: 'ClassDeclaration', message: ERROR_MESSAGE }], + }, + { + code: ` + import Controller from '@ember/controller'; + export default class ArticlesController extends Controller { + randomProperty = true; + get filteredArticles() {} + } + `, + output: null, + errors: [{ type: 'ClassDeclaration', message: ERROR_MESSAGE }], + }, + ], +});