Skip to content

Commit

Permalink
feat: allow custom mapping for property name
Browse files Browse the repository at this point in the history
  • Loading branch information
rtablada committed Dec 30, 2022
1 parent 919ce52 commit 61ebd4f
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 46 deletions.
5 changes: 4 additions & 1 deletion docs/rules/no-implicit-injections.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ This lint rule will search for instances of `store` used in routes or controller
- 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 -- `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 class files/types)
- 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:

Expand All @@ -101,6 +102,8 @@ module.exports = {
{ 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' },
]
}]
}
Expand Down
90 changes: 48 additions & 42 deletions lib/rules/no-implicit-injections.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,45 @@ const kebabCase = require('lodash.kebabcase');
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, servicePropertyName) {
const serviceInjectPath = kebabCase(servicePropertyName);
function fixService(fixer, currentClass, serviceInjectImportName, failedConfig) {
const serviceInjectPath = failedConfig.service;

return currentClass.node.type === 'CallExpression'
? fixer.insertTextBefore(
currentClass.node.arguments[0].properties[0],
`${servicePropertyName}: ${serviceInjectImportName}('${serviceInjectPath}'),\n`
`${failedConfig.propertyName}: ${serviceInjectImportName}('${serviceInjectPath}'),\n`
)
: fixer.insertTextBefore(
currentClass.node.body.body[0],
`@${serviceInjectImportName}('${serviceInjectPath}') ${servicePropertyName};\n`
`@${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: {
Expand Down Expand Up @@ -56,21 +76,14 @@ module.exports = {
type: 'string',
minLength: 1,
},
propertyName: {
type: 'string',
minLength: 1,
},
moduleNames: {
type: 'array',
items: {
enum: [
'Component',
'GlimmerComponent',
'Controller',
'Mixin',
'Route',
'Service',
'ArrayProxy',
'ObjectProxy',
'EmberObject',
'Helper',
],
enum: MODULE_TYPES,
},
},
},
Expand Down Expand Up @@ -101,7 +114,7 @@ module.exports = {

// State being tracked for this file.
let serviceInjectImportName = undefined;
let serviceNamesWithImplicitInjection = denyList;
const normalizedConfiguration = normalizeConfiguration(denyList);

// State being tracked for the current class we're inside.
const classStack = new Stack();
Expand All @@ -116,32 +129,30 @@ module.exports = {

function onModuleFound(node) {
// Get the name of services for the current module type
const serviceNames = denyList
.filter((serviceConfig) => {
return (
serviceConfig.moduleNames === undefined ||
serviceConfig.moduleNames.some((moduleName) =>
emberUtils.isEmberCoreModule(context, node, moduleName)
)
);
})
.map((a) => camelCase(a.service));
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
serviceNamesWithImplicitInjection = modulePropertyDeclarations.reduce((accum, n) => {
configToCheckFor = modulePropertyDeclarations.reduce((accum, n) => {
if (propertyDefintionTypes.has(n.type)) {
return accum.filter((serviceName) => !(serviceName === n.key.name));
return accum.filter((config) => !(config.propertyName === n.key.name));
}
return accum;
}, serviceNames);
}, configToCheckFor);

classStack.push({
node,
isEmberModule: true,
serviceNamesWithImplicitInjection,
configToCheckFor,
});
}

Expand Down Expand Up @@ -191,36 +202,31 @@ module.exports = {
}

if (types.isThisExpression(node.object) && types.isIdentifier(node.property)) {
const serviceMissingInjection = serviceNamesWithImplicitInjection.find(
(s) => s === node.property.name
const failedConfig = currentClass.configToCheckFor.find(
(s) => s.propertyName === node.property.name
);

if (serviceMissingInjection) {
if (failedConfig) {
context.report({
node,
messageId: 'main',
data: {
serviceName: kebabCase(serviceMissingInjection),
serviceName: kebabCase(failedConfig.service),
},
fix(fixer) {
const sourceCode = context.getSourceCode();

// service inject is already declared
if (serviceInjectImportName) {
return fixService(
fixer,
currentClass,
serviceInjectImportName,
serviceMissingInjection
);
return fixService(fixer, currentClass, serviceInjectImportName, failedConfig);
}

return [
fixer.insertTextBefore(
sourceCode.ast,
"import { inject as service } from '@ember/service';\n"
),
fixService(fixer, currentClass, 'service', serviceMissingInjection),
fixService(fixer, currentClass, 'service', failedConfig),
];
},
});
Expand Down
51 changes: 48 additions & 3 deletions tests/lib/rules/no-implicit-injections.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ const FLASH_MESSAGES_CONFIG = {
const MEDIA_CONFIG = {
denyList: [{ service: 'media', moduleNames: ['Component', 'Controller'] }],
};
const FEATURE_CHECKER_CONFIG = {
denyList: [{ service: 'feature', propertyName: 'featureChecker' }],
};

function createClassUsage(serviceDefinition) {
return {
Expand Down Expand Up @@ -196,15 +199,30 @@ ruleTester.run('no-implicit-injections', rule, {
}`,
options: [MEDIA_CONFIG],
},
// Can ignore non-matching module types for custom 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 Route from '@ember/routing/route';
export default class IndexRoute extends Route {
get isSmallScreen() {
return this.media.isXs;
featureChecker = null;
get canVisitCheckout() {
return this.featureChecker.isEnabled('checkout');
}
}`,
options: [MEDIA_CONFIG],
Expand Down Expand Up @@ -685,6 +703,33 @@ async loadData() {
{ 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' }],
},

// Check use and fix in legacy ember components
{
filename: 'pods/index.js',
code: `
Expand Down

0 comments on commit 61ebd4f

Please sign in to comment.