diff --git a/README.md b/README.md index 03844ce13a..b291b05aff 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,7 @@ module.exports = { | [new-module-imports](docs/rules/new-module-imports.md) | enforce using "New Module Imports" from Ember RFC #176 | ✅ | | | | [no-array-prototype-extensions](docs/rules/no-array-prototype-extensions.md) | disallow usage of Ember's `Array` prototype extensions | | 🔧 | | | [no-function-prototype-extensions](docs/rules/no-function-prototype-extensions.md) | disallow usage of Ember's `function` prototype extensions | ✅ | | | +| [no-implicit-injections](docs/rules/no-implicit-injections.md) | enforce usage of implicit service injections | | 🔧 | | | [no-mixins](docs/rules/no-mixins.md) | disallow the usage of mixins | ✅ | | | | [no-new-mixins](docs/rules/no-new-mixins.md) | disallow the creation of new mixins | ✅ | | | | [no-observers](docs/rules/no-observers.md) | disallow usage of observers | ✅ | | | diff --git a/docs/rules/no-implicit-injections.md b/docs/rules/no-implicit-injections.md new file mode 100644 index 0000000000..c9337ee2aa --- /dev/null +++ b/docs/rules/no-implicit-injections.md @@ -0,0 +1,116 @@ +# ember/no-implicit-injections + +🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + +Ember 3.26 introduced a deprecation for relying on implicit service injections or allowing addons to implicitly inject services into all classes of certain types. Support for this is dropped in Ember 4.0. + +In many applications, `this.store` from Ember Data is often used without injecting the `store` service in Controllers or Routes. Other addons may also have included implicit service injections via initializers and the `application.inject` API. + +To resolve this deprecation, a service should be explicitly declared and injected using the [service injection decorator](https://api.emberjs.com/ember/3.28/functions/@ember%2Fservice/inject). + +## Rule Details + +This rule checks for a configured list of previously auto injected services and warns if they are used in classes without explicit injected service properties. + +## Examples + +Examples of **incorrect** code for this rule: + +```js +// routes/index.js +import Route from '@ember/routing/route'; + +export default class IndexRoute extends Route { + model() { + return this.store.findAll('rental'); + } +} + +``` + +```js +// controllers/index.js +import Controller from '@ember/controller'; +import { action } from '@ember/object'; + +export default class IndexController extends Controller { + @action + loadUsers() { + return this.store.findAll('user'); + } +} +``` + +Examples of **correct** code for this rule: + +```js +// routes/index.js +import Route from '@ember/routing/route'; +import { inject as service } from '@ember/service'; + +export default class IndexRoute extends Route { + @service('store') store; + + model() { + return this.store.findAll('rental'); + } +} +``` + +```js +// controller/index.js +import Route from '@ember/routing/route'; +import { action } from '@ember/object'; +import { inject as service } from '@ember/service'; + +export default class IndexController extends Controller { + @service('store') store; + + @action + loadUsers() { + return this.store.findAll('user'); + } +} +``` + +## Migration + +The autofixer for this rule will update classes and add injections for the configured services. + +## Configuration + +This lint rule will search for instances of `store` used in routes or controllers by default. If you have other services that you would like to check for uses of, the configuration can be overridden. + +- object -- containing the following properties: + - array -- `denyList` -- Array of configuration objects configuring the lint rule to check for use of implicit injected services + - string -- `service` -- The (kebab-case) service name that should be checked for implicit injections and error if found + - array -- `propertyName` -- The property name where the service would be injected in classes. This defaults to the camel case version of the `service` config above. + - array -- `moduleNames` -- Array of string listing the types of classes (`Controller`, `Route`, `Component`, etc) to check for implicit injections. If an array is declared, only those class types will be checked for implicit injection. (Defaults to checking all Ember Module class files/types) + +Example config: + +```js +module.exports = { + rules: { + 'ember/no-implicit-injections': ['error', { + denyList: [ + // Ember Responsive Used to Auto Inject the media service in Components/Controllers + { service: 'media', moduleNames: ['Component', 'Controller'] }, + // Ember CLI Flash Used to Auto Inject the flashMessages service in all modules + { service: 'flash-messages' }, + // Check for uses of the store in Routes or Controllers + { service: 'store', moduleNames: ['Route', 'Controller'] }, + // Check for the feature service injected as "featureChecker" + { service: 'feature', propertyName: 'featureChecker' }, + ] + }] + } +} +``` + +## References + +- [Deprecation](https://deprecations.emberjs.com/v3.x/#toc_implicit-injections) +- [Ember Data Store Service](https://api.emberjs.com/ember-data/release/classes/Store) diff --git a/lib/rules/no-implicit-injections.js b/lib/rules/no-implicit-injections.js new file mode 100644 index 0000000000..b9df3d138e --- /dev/null +++ b/lib/rules/no-implicit-injections.js @@ -0,0 +1,242 @@ +'use strict'; + +const assert = require('assert'); +const emberUtils = require('../utils/ember'); +const { getImportIdentifier } = require('../utils/import'); +const Stack = require('../utils/stack'); +const types = require('../utils/types'); +const camelCase = require('lodash.camelcase'); + +const defaultServiceConfig = { service: 'store', moduleNames: ['Route', 'Controller'] }; +const MODULE_TYPES = [ + 'Component', + 'GlimmerComponent', + 'Controller', + 'Mixin', + 'Route', + 'Service', + 'ArrayProxy', + 'ObjectProxy', + 'EmberObject', + 'Helper', +]; + +// ----- ------------------------------------------------------------------------- +// Rule Definition +//------------------------------------------------------------------------------ + +function fixService(fixer, currentClass, serviceInjectImportName, failedConfig) { + const serviceInjectPath = failedConfig.service; + + return currentClass.node.type === 'CallExpression' + ? fixer.insertTextBefore( + currentClass.node.arguments[0].properties[0], + `${failedConfig.propertyName}: ${serviceInjectImportName}('${serviceInjectPath}'),\n` + ) + : fixer.insertTextBefore( + currentClass.node.body.body[0], + `@${serviceInjectImportName}('${serviceInjectPath}') ${failedConfig.propertyName};\n` + ); +} + +function normalizeConfiguration(denyList) { + return denyList.map((config) => ({ + service: config.service, + propertyName: config.propertyName ?? camelCase(config.service), + moduleNames: config.moduleNames ?? MODULE_TYPES, + })); +} + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: 'enforce usage of implicit service injections', + category: 'Deprecations', + recommended: false, + url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-implicit-injections.md', + }, + fixable: 'code', + schema: [ + { + type: 'object', + required: ['denyList'], + properties: { + denyList: { + minItems: 1, + type: 'array', + items: { + type: 'object', + default: [defaultServiceConfig], + required: ['service'], + properties: { + service: { + type: 'string', + minLength: 1, + }, + propertyName: { + type: 'string', + minLength: 1, + }, + moduleNames: { + type: 'array', + items: { + enum: MODULE_TYPES, + }, + }, + }, + additionalProperties: false, + }, + }, + }, + additionalProperties: false, + }, + ], + messages: { + main: 'Do not rely on implicit service injections for the "{{serviceName}}" service. Implicit service injections were deprecated in Ember 3.26 and will not work in 4.0.', + }, + }, + + create(context) { + const options = context.options[0] || { + denyList: [defaultServiceConfig], + }; + const denyList = options.denyList || [defaultServiceConfig]; + + for (const config of denyList) { + assert( + emberUtils.convertServiceNameToKebabCase(config.service) === config.service, + 'Service name should be passed in kebab-case (all lower case)' + ); + + assert( + !config.service.includes('/') || config.propertyName, + 'Nested services must declare a property name' + ); + } + + // State being tracked for this file. + let serviceInjectImportName = undefined; + const normalizedConfiguration = normalizeConfiguration(denyList); + + // State being tracked for the current class we're inside. + const classStack = new Stack(); + + // Array of posible types that could declare existing properties on native or legacy modules + const propertyDefintionTypes = new Set([ + 'Property', + 'ClassProperty', + 'PropertyDefinition', + 'MethodDefinition', + ]); + + function onModuleFound(node) { + // Get the name of services for the current module type + let configToCheckFor = normalizedConfiguration.filter((serviceConfig) => { + return ( + serviceConfig.moduleNames === undefined || + serviceConfig.moduleNames.some((moduleName) => + emberUtils.isEmberCoreModule(context, node, moduleName) + ) + ); + }); + + const modulePropertyDeclarations = + node.type === 'CallExpression' ? node.arguments[0].properties : node.body.body; + + // Get Services that don't have properties/service injections declared + configToCheckFor = modulePropertyDeclarations.reduce((accum, n) => { + if (propertyDefintionTypes.has(n.type)) { + return accum.filter((config) => !(config.propertyName === n.key.name)); + } + return accum; + }, configToCheckFor); + + classStack.push({ + node, + isEmberModule: true, + configToCheckFor, + }); + } + + function onClassEnter(node) { + if (emberUtils.isAnyEmberCoreModule(context, node)) { + onModuleFound(node); + } else { + classStack.push({ + node, + isEmberModule: false, + }); + } + } + + function onClassExit(node) { + // Leaving current (native) class. + if (classStack.size() > 0 && classStack.peek().node === node) { + classStack.pop(); + } + } + + return { + ImportDeclaration(node) { + if (node.source.value === '@ember/service') { + serviceInjectImportName = + serviceInjectImportName || getImportIdentifier(node, '@ember/service', 'inject'); + } + }, + + ClassDeclaration: onClassEnter, + ClassExpression: onClassEnter, + CallExpression(node) { + if (emberUtils.isAnyEmberCoreModule(context, node)) { + onModuleFound(node); + } + }, + + 'ClassDeclaration:exit': onClassExit, + 'ClassExpression:exit': onClassExit, + 'CallExpression:exit': onClassExit, + + MemberExpression(node) { + const currentClass = classStack.peek(); + + if (!currentClass || !currentClass.isEmberModule) { + return; + } + + if (types.isThisExpression(node.object) && types.isIdentifier(node.property)) { + const failedConfig = currentClass.configToCheckFor.find( + (s) => s.propertyName === node.property.name + ); + + if (failedConfig) { + context.report({ + node, + messageId: 'main', + data: { + serviceName: failedConfig.service, + }, + fix(fixer) { + const sourceCode = context.getSourceCode(); + + // service inject is already declared + if (serviceInjectImportName) { + return fixService(fixer, currentClass, serviceInjectImportName, failedConfig); + } + + return [ + fixer.insertTextBefore( + sourceCode.ast, + "import { inject as service } from '@ember/service';\n" + ), + fixService(fixer, currentClass, 'service', failedConfig), + ]; + }, + }); + } + } + }, + }; + }, +}; diff --git a/lib/utils/ember.js b/lib/utils/ember.js index e2edd762ca..cbc1249a17 100644 --- a/lib/utils/ember.js +++ b/lib/utils/ember.js @@ -222,7 +222,7 @@ function isEmberCoreModule(context, node, moduleName) { if (types.isCallExpression(node)) { // "classic" class pattern return isClassicEmberCoreModule(node, moduleName, context.getFilename()); - } else if (types.isClassDeclaration(node)) { + } else if (types.isClassDeclaration(node) || node.type === 'ClassExpression') { // native classes if ( // class Foo extends Component @@ -251,7 +251,7 @@ function isEmberCoreModule(context, node, moduleName) { } else { assert( false, - 'Function should only be called on a `CallExpression` (classic class) or `ClassDeclaration` (native class)' + 'Function should only be called on a `CallExpression` (classic class) or `ClassDeclaration`/`ClassExpression` (native class)' ); } return false; diff --git a/package.json b/package.json index 6c5d3a42e4..27e347e579 100644 --- a/package.json +++ b/package.json @@ -71,6 +71,7 @@ "ember-template-imports": "^3.1.1", "eslint-utils": "^3.0.0", "estraverse": "^5.2.0", + "lodash.camelcase": "^4.1.1", "lodash.kebabcase": "^4.1.1", "magic-string": "^0.27.0", "requireindex": "^1.2.0", diff --git a/tests/lib/rules/no-implicit-injections.js b/tests/lib/rules/no-implicit-injections.js new file mode 100644 index 0000000000..e0e180b94a --- /dev/null +++ b/tests/lib/rules/no-implicit-injections.js @@ -0,0 +1,812 @@ +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-implicit-injections'); +const RuleTester = require('eslint').RuleTester; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ + parser: require.resolve('@babel/eslint-parser'), + parserOptions: { + ecmaVersion: 2020, + sourceType: 'module', + babelOptions: { + configFile: require.resolve('../../../.babelrc'), + }, + }, +}); + +const FLASH_MESSAGES_CONFIG = { + denyList: [{ service: 'flash-messages' }], +}; +const MEDIA_CONFIG = { + denyList: [{ service: 'media', moduleNames: ['Component', 'Controller'] }], +}; +const FEATURE_CHECKER_CONFIG = { + denyList: [{ service: 'feature', propertyName: 'featureChecker' }], +}; +const NESTED_SERVICE_CONFIG = { + denyList: [{ service: 'cart/checkout', propertyName: 'checkout' }], +}; + +function createClassUsage(serviceDefinition) { + return { + filename: 'pods/index.js', + code: ` + import { inject as service } from '@ember/service'; + import Component from '@ember/component'; + + export default class FoobarTestError extends Component { + ${serviceDefinition} + + @action + save() { + return this.flashMessages.warn('some message'); + } + }`, + options: [FLASH_MESSAGES_CONFIG], + }; +} + +function createExtendUsage(serviceDefinition) { + return { + filename: 'pods/index.js', + code: ` + import { inject as service } from '@ember/service'; + import Component from '@ember/component'; + + export default Component.extend({ + ${serviceDefinition} + + actions: { + + save() { + return this.flashMessages.warn('some message'); + } + } + });`, + options: [FLASH_MESSAGES_CONFIG], + }; +} + +ruleTester.run('no-implicit-injections', rule, { + valid: [ + // Basic use in Route and Controller in single file + { + filename: 'pods/index.js', + code: ` + import Route from '@ember/routing/route'; + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export class IndexController extends Controller { + @service('store') store; + async loadData() { + return this.store.findAll('rental'); + } + } + + export class IndexRoute extends Route { + @service('store') store; + async model() { + return this.store.findAll('rental'); + } + }`, + }, + // Basic use in Controller + { + filename: 'controller/index.js', + code: ` + import Controller from '@ember/controller'; + import { action } from '@ember/object'; + import { inject as service } from '@ember/service'; + + export default class IndexController extends Controller { + @service('store') store; + @action + async loadUsers() { + return this.store.findAll('user'); + } + }`, + }, + // Ignores if some other property getter is defined + { + filename: 'controller/index.js', + code: ` + import Controller from '@ember/controller'; + import { action } from '@ember/object'; + import { inject as service } from '@ember/service'; + import storeMock from './my-mock'; + + export default class IndexController extends Controller { + store = storeMock; + + @action + async loadUsers() { + return this.store.findAll('user'); + } + }`, + }, + // Ignores if some other property getter is defined + { + filename: 'controller/index.js', + code: ` + import Controller from '@ember/controller'; + import { action } from '@ember/object'; + import { inject as service } from '@ember/service'; + + export default class IndexController extends Controller { + get store() { + return {} + } + @action + async loadUsers() { + return this.store.findAll('user'); + } + }`, + }, + // Only checks for ThisExpression + { + filename: 'controller/index.js', + code: ` + import Controller from '@ember/controller'; + import { action } from '@ember/object'; + + export default class IndexController extends Controller { + @action + async loadUsers(arg) { + return arg.store.findAll('user'); + } + }`, + }, + // Does not check for Store use in Components + { + filename: 'components/foobar.js', + code: ` + import Component from '@ember/component'; + + export default class FoobarTest extends Component { + async model() { + return this.store.isXs; + } + }`, + }, + // Does not check for Store use in GlimmerComponents + { + filename: 'components/foobar.js', + code: ` + import Component from '@glimmer/component'; + + export default class FoobarTest extends Component { + async model() { + return this.store.isXs; + } + }`, + }, + // Checks custom config + { + filename: 'controllers/index.js', + code: ` + import Controller from '@ember/controller'; + + export default class IndexController extends Controller { + @service('media') media; + + get isSmallScreen() { + return this.media.isXs; + } + }`, + options: [MEDIA_CONFIG], + }, + // Ignores use when property name is modified from 1:1 mapping + { + filename: 'controllers/index.js', + code: ` + import Controller from '@ember/controller'; + + export default class IndexController extends Controller { + get isSmallScreen() { + return this.feature.isXs; + } + }`, + options: [FEATURE_CHECKER_CONFIG], + }, + // Can detect changed property mapping + { + filename: 'routes/index.js', + code: ` + import { inject as service } from '@ember/service'; + import Route from '@ember/routing/route'; + + export default class IndexRoute extends Route { + @service('feature') featureChecker; + + get canVisitCheckout() { + return this.featureChecker.isEnabled('checkout'); + } + }`, + options: [FEATURE_CHECKER_CONFIG], + }, + // Can work with services with nested module paths + { + filename: 'routes/index.js', + code: ` + import Route from '@ember/routing/route'; + import { inject as service } from '@ember/service'; + + export default class IndexRoute extends Route { + @service('cart/checkout') checkout; + + model() { + return this.checkout.viewCart(); + } + }`, + options: [NESTED_SERVICE_CONFIG], + }, + // Ignores use outside of classes + { + filename: 'utils/support.js', + code: ` + export function checkMedia() { + return this.media.isXs; + }`, + options: [ + { + denyList: [{ service: 'media' }], + }, + ], + }, + // Custom Configs work with multiple class/module types both matching and not + { + filename: 'pods/user.js', + code: ` + import Route from '@ember/routing/route'; + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export class User { + get mediaAccount() { + return this.media.account; + } + } + + export class UserController extends Controller { + @service('media') media; + + get isSmallScreen() { + return this.media.isXs; + } + } + + export class UserRoute extends Route { + get isSmallScreen() { + return this.media.isXs; + } + }`, + options: [MEDIA_CONFIG], + }, + + // Reassesses Module Type for Nested Classes + { + filename: 'controllers/register.js', + code: ` + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export class RegisterController extends Controller { + @service('store') store; + async loadData() { + return this.store.findAll('rental'); + } + + getMediaUser() { + return class Register { + get storeInfo() { + return this.store.address; + } + } + } + }`, + }, + + // Nested Ember Module Definition (used in some meta programming instances or decorators) + { + filename: 'utils/loads-user-controller.js', + code: ` + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export function mediaAwareRoute() { + return class UserController extends Controller { + @inject('store') store; + async loadData() { + return this.store.findAll('rental'); + } + } + }`, + }, + + // Exhaustive check for property definition type + createClassUsage('@service flashMessages'), + createClassUsage('@service() flashMessages'), + createClassUsage("@service('flashMessages') flashMessages"), + createClassUsage("@service('flash-messages') flashMessages"), + + createExtendUsage('flashMessages: service(),'), + createExtendUsage("flashMessages: service('flashMessages'),"), + createExtendUsage("flashMessages: service('flash-messages'),"), + ], + invalid: [ + // Basic store lint error in routes/controllers + { + filename: 'routes/index.js', + code: ` + import Route from '@ember/routing/route'; + + export default class IndexRoute extends Route { + message = 'hello'; + async model() { + return this.store.findAll('rental'); + } + }`, + output: ` + import { inject as service } from '@ember/service'; +import Route from '@ember/routing/route'; + + export default class IndexRoute extends Route { + @service('store') store; +message = 'hello'; + async model() { + return this.store.findAll('rental'); + } + }`, + errors: [{ messageId: 'main', data: { serviceName: 'store' }, type: 'MemberExpression' }], + }, + { + filename: 'controller/index.js', + code: ` + import Controller from '@ember/controller'; + import { action } from '@ember/object'; + + export default class IndexController extends Controller { + @action + async loadUsers() { + return this.store.findAll('user'); + } + }`, + output: ` + import { inject as service } from '@ember/service'; +import Controller from '@ember/controller'; + import { action } from '@ember/object'; + + export default class IndexController extends Controller { + @service('store') store; +@action + async loadUsers() { + return this.store.findAll('user'); + } + }`, + errors: [{ messageId: 'main', data: { serviceName: 'store' }, type: 'MemberExpression' }], + }, + // Existing import for service injection + { + filename: 'routes/index.js', + code: ` + import { inject as service } from '@ember/service'; + import Route from '@ember/routing/route'; + import Component from '@glimmer/component'; + + export default class IndexRoute extends Route { + @service('router') router; + message = 'hello'; + async model() { + return this.store.findAll('rental'); + } + } + + export class IndexRow extends Component { + @service('store') storeService; + + navigate() { + this.store.find(this.args.id); + } + } + + + export class IndexTable extends Component { + loadData() { + this.store.find(this.args.id); + } + }`, + output: ` + import { inject as service } from '@ember/service'; + import Route from '@ember/routing/route'; + import Component from '@glimmer/component'; + + export default class IndexRoute extends Route { + @service('store') store; +@service('router') router; + message = 'hello'; + async model() { + return this.store.findAll('rental'); + } + } + + export class IndexRow extends Component { + @service('store') storeService; + + navigate() { + this.store.find(this.args.id); + } + } + + + export class IndexTable extends Component { + loadData() { + this.store.find(this.args.id); + } + }`, + errors: [{ messageId: 'main', data: { serviceName: 'store' }, type: 'MemberExpression' }], + }, + + { + filename: 'routes/index.js', + code: ` + import { inject } from '@ember/service'; + import Route from '@ember/routing/route'; + + export default class IndexRoute extends Route { + @inject('router') router; + message = 'hello'; + async model() { + return this.store.findAll('rental'); + } + }`, + output: ` + import { inject } from '@ember/service'; + import Route from '@ember/routing/route'; + + export default class IndexRoute extends Route { + @inject('store') store; +@inject('router') router; + message = 'hello'; + async model() { + return this.store.findAll('rental'); + } + }`, + errors: [{ messageId: 'main', data: { serviceName: 'store' }, type: 'MemberExpression' }], + }, + + // Custom options + { + filename: 'components/foobar.js', + code: ` + import Component from '@ember/component'; + + export default class FoobarTestError extends Component { + get isSmallScreen() { + return this.media.isXs; + } + }`, + output: ` + import { inject as service } from '@ember/service'; +import Component from '@ember/component'; + + export default class FoobarTestError extends Component { + @service('media') media; +get isSmallScreen() { + return this.media.isXs; + } + }`, + options: [MEDIA_CONFIG], + errors: [{ messageId: 'main', data: { serviceName: 'media' }, type: 'MemberExpression' }], + }, + // Custom options with dasherized service name + { + filename: 'components/foobar.js', + code: ` + import Component from '@ember/component'; + + export default class FoobarTestError extends Component { + @action + save() { + return this.flashMessages.warn('some message'); + } + }`, + output: ` + import { inject as service } from '@ember/service'; +import Component from '@ember/component'; + + export default class FoobarTestError extends Component { + @service('flash-messages') flashMessages; +@action + save() { + return this.flashMessages.warn('some message'); + } + }`, + options: [FLASH_MESSAGES_CONFIG], + errors: [ + { messageId: 'main', data: { serviceName: 'flash-messages' }, type: 'MemberExpression' }, + ], + }, + { + filename: 'routes/index.js', + code: ` + import Route from '@ember/routing/route'; + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export class IndexController extends Controller { + async loadData() { + return this.store.findAll('rental'); + } + } + + export default class IndexRoute extends Route { + async model() { + return this.store.findAll('rental'); + } + }`, + output: ` + import Route from '@ember/routing/route'; + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export class IndexController extends Controller { + @service('store') store; +async loadData() { + return this.store.findAll('rental'); + } + } + + export default class IndexRoute extends Route { + @service('store') store; +async model() { + return this.store.findAll('rental'); + } + }`, + errors: [ + { messageId: 'main', data: { serviceName: 'store' }, type: 'MemberExpression' }, + { messageId: 'main', data: { serviceName: 'store' }, type: 'MemberExpression' }, + ], + }, + // Works for modules with multiple module types + { + filename: 'pods/user.js', + code: ` + import Route from '@ember/routing/route'; + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export class User { + get mediaAccount() { + return this.media.account; + } + } + + export class UserController extends Controller { + get isSmallScreen() { + return this.media.isXs; + } + } + + export class UserRoute extends Route { + get isSmallScreen() { + return this.media.isXs; + } + }`, + output: ` + import Route from '@ember/routing/route'; + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export class User { + get mediaAccount() { + return this.media.account; + } + } + + export class UserController extends Controller { + @service('media') media; +get isSmallScreen() { + return this.media.isXs; + } + } + + export class UserRoute extends Route { + get isSmallScreen() { + return this.media.isXs; + } + }`, + options: [MEDIA_CONFIG], + errors: [{ messageId: 'main', data: { serviceName: 'media' }, type: 'MemberExpression' }], + }, + + // Reassesses Module Type for Nested Classes + { + filename: 'controllers/register.js', + code: ` + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export class RegisterController extends Controller { + async loadData() { + return this.store.findAll('rental'); + } + + getMediaUser() { + return class Register { + get storeInfo() { + return this.store.address; + } + } + } + }`, + output: ` + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export class RegisterController extends Controller { + @service('store') store; +async loadData() { + return this.store.findAll('rental'); + } + + getMediaUser() { + return class Register { + get storeInfo() { + return this.store.address; + } + } + } + }`, + errors: [{ messageId: 'main', data: { serviceName: 'store' }, type: 'MemberExpression' }], + }, + + // Nested Ember Module Definition (used in some meta programming instances or decorators) + { + filename: 'utils/loads-user-controller.js', + code: ` + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export function mediaAwareRoute() { + return class UserController extends Controller { + async loadData() { + return this.store.findAll('rental'); + } + } + }`, + output: ` + import Controller from '@ember/controller'; + import { inject as service } from '@ember/service'; + + export function mediaAwareRoute() { + return class UserController extends Controller { + @service('store') store; +async loadData() { + return this.store.findAll('rental'); + } + } + }`, + errors: [{ messageId: 'main', data: { serviceName: 'store' }, type: 'MemberExpression' }], + }, + + { + code: ` + import { inject as service } from '@ember/service'; + import Component from '@ember/component'; + + export default class FoobarTestError extends Component { + @action + save() { + return this.flashMessages.warn('some message'); + } + }`, + output: ` + import { inject as service } from '@ember/service'; + import Component from '@ember/component'; + + export default class FoobarTestError extends Component { + @service('flash-messages') flashMessages; +@action + save() { + return this.flashMessages.warn('some message'); + } + }`, + options: [FLASH_MESSAGES_CONFIG], + errors: [ + { messageId: 'main', data: { serviceName: 'flash-messages' }, type: 'MemberExpression' }, + ], + }, + // Can detect changed property mapping + { + filename: 'routes/checkout.js', + code: ` + import { inject as service } from '@ember/service'; + import Route from '@ember/routing/route'; + + export default class CheckoutRoute extends Route { + get canVisitCheckout() { + return this.featureChecker.isEnabled('checkout'); + } + }`, + output: ` + import { inject as service } from '@ember/service'; + import Route from '@ember/routing/route'; + + export default class CheckoutRoute extends Route { + @service('feature') featureChecker; +get canVisitCheckout() { + return this.featureChecker.isEnabled('checkout'); + } + }`, + options: [FEATURE_CHECKER_CONFIG], + errors: [{ messageId: 'main', data: { serviceName: 'feature' }, type: 'MemberExpression' }], + }, + + // Can work with services with nested module paths + { + filename: 'routes/index.js', + code: ` + import Route from '@ember/routing/route'; + import { inject as service } from '@ember/service'; + + export default class IndexRoute extends Route { + model() { + return this.checkout.viewCart(); + } + }`, + output: ` + import Route from '@ember/routing/route'; + import { inject as service } from '@ember/service'; + + export default class IndexRoute extends Route { + @service('cart/checkout') checkout; +model() { + return this.checkout.viewCart(); + } + }`, + options: [NESTED_SERVICE_CONFIG], + errors: [ + { messageId: 'main', data: { serviceName: 'cart/checkout' }, type: 'MemberExpression' }, + ], + }, + + // Check use and fix in legacy ember components + { + filename: 'pods/index.js', + code: ` + import { inject as service } from '@ember/service'; + import Component from '@ember/component'; + + export default Component.extend({ + actions: { + save() { + return this.flashMessages.warn('some message'); + } + } + });`, + output: ` + import { inject as service } from '@ember/service'; + import Component from '@ember/component'; + + export default Component.extend({ + flashMessages: service('flash-messages'), +actions: { + save() { + return this.flashMessages.warn('some message'); + } + } + });`, + options: [FLASH_MESSAGES_CONFIG], + errors: [ + { messageId: 'main', data: { serviceName: 'flash-messages' }, type: 'MemberExpression' }, + ], + }, + ], +}); diff --git a/tests/lib/utils/ember-test.js b/tests/lib/utils/ember-test.js index e53dd8cbc3..a9639215e7 100644 --- a/tests/lib/utils/ember-test.js +++ b/tests/lib/utils/ember-test.js @@ -222,6 +222,17 @@ describe('isEmberCoreModule', () => { expect(emberUtils.isEmberCoreModule(context, node, 'Route')).toBeTruthy(); }); + it('should check core modules from ClassExpressions', () => { + const context = new FauxContext( + `import Route from '@ember/routing/route'; + + (class MyRoute extends Route {})`, + 'example-app/some-twisted-path/some-route.js' + ); + const node = context.ast.body[1].expression; + expect(emberUtils.isEmberCoreModule(context, node, 'Route')).toBeTruthy(); + }); + it('ignores a native class with a non-identifier super class', () => { const context = new FauxContext( 'class MyRoute extends this.ContainerObject {}', @@ -235,7 +246,7 @@ describe('isEmberCoreModule', () => { const context = new FauxContext('const x = 123;'); const node = context.ast.body[0]; expect(() => emberUtils.isEmberCoreModule(context, node, 'Route')).toThrow( - 'Function should only be called on a `CallExpression` (classic class) or `ClassDeclaration` (native class)' + 'Function should only be called on a `CallExpression` (classic class) or `ClassDeclaration`/`ClassExpression` (native class)' ); }); }); diff --git a/yarn.lock b/yarn.lock index 8ed986aa06..dd644cef6d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4619,7 +4619,7 @@ lodash._reinterpolate@^3.0.0: resolved "https://registry.yarnpkg.com/lodash._reinterpolate/-/lodash._reinterpolate-3.0.0.tgz#0ccf2d89166af03b3663c796538b75ac6e114d9d" integrity sha512-xYHt68QRoYGjeeM/XOE1uJtvXQAgvszfBhjV4yvsQH0u2i9I6cI6c6/eG4Hh3UAOVn0y/xAXwmTzEay49Q//HA== -lodash.camelcase@4.3.0: +lodash.camelcase@4.3.0, lodash.camelcase@^4.1.1: version "4.3.0" resolved "https://registry.yarnpkg.com/lodash.camelcase/-/lodash.camelcase-4.3.0.tgz#b28aa6288a2b9fc651035c7711f65ab6190331a6" integrity sha512-TwuEnCnxbc3rAvhf/LbG7tJUDzhqXyFnv3dtzLOPgCG/hODL7WFnsbwktkD7yUV0RrreP/l1PALq/YSg6VvjlA==