Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/enforce custom provider type #25

Merged
merged 23 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
728efed
feat: create enforce custom provider type rule
thiagomini Feb 5, 2024
061b85b
feat: ensure the provider type is factory
thiagomini Feb 5, 2024
81f5303
test: provider type is class or useExisting
thiagomini Feb 9, 2024
a4009e9
feat: support aliasing provider type
tolgap Feb 10, 2024
ee1dadc
feat: ensure we only check for Provider imported from nestjs package
thiagomini Feb 15, 2024
e84a24c
Merge branch 'feat/enforce-custom-provider-type' of github.com:Trilon…
thiagomini Feb 15, 2024
8583c37
refactor: use filter and forEach to get specifiers
thiagomini Feb 16, 2024
e4608a0
refactor: remove unnecessary type assertion
thiagomini Feb 16, 2024
0e767f0
docs: added comments to tests
thiagomini Feb 16, 2024
a2b9f7d
feat: report error when provider type in providers array is invalid
thiagomini Feb 21, 2024
93553a2
test: add test for all types of providers
thiagomini Feb 21, 2024
51a7e69
refactor: simplify getProviderType code
thiagomini Feb 21, 2024
5167f01
refactor: extract common logic to method
thiagomini Feb 22, 2024
6555624
chore: update node version
thiagomini Feb 22, 2024
b03a987
style: fix indentation
thiagomini Feb 26, 2024
58647c7
docs: add docs for new rule and update README
thiagomini Feb 26, 2024
a9b0473
chore: update node version hint
thiagomini Feb 26, 2024
509c9ba
refactor: make preferred type an array
thiagomini Feb 27, 2024
c815a6a
test: create tests for multiple preferred types
thiagomini Feb 27, 2024
fbad230
docs: update docs to include array of preferred types
thiagomini Feb 27, 2024
55a03f6
chore: change node version hint to 18 to make it less strict
thiagomini Feb 27, 2024
7d5cd24
chore: delete nvmrc file
thiagomini Feb 28, 2024
02c2f7b
chore: add symlink to .node-version
thiagomini Feb 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .node-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v20
v20.11.1
1 change: 1 addition & 0 deletions .nvmrc
9 changes: 5 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,12 @@ The "recommended" preset contains the rules listed below. If you need custom con

## Rules

| Rule | Description | Recommended |
| Rule | Description | Type |
| ------------------------------------------------------------------------------------ | -------------------------------------------------------------- | ----------- |
| [`@trilon/enforce-close-testing-module`](docs/rules/enforce-close-testing-module.md) | Ensures NestJS testing modules are closed properly after tests | ✅ |
| [`@trilon/check-inject-decorator`](docs/rules/check-inject-decorator.md) | Detects incorrect usage of `@Inject(TOKEN)` decorator | ✅ |
| [`@trilon/detect-circular-reference`](docs/rules/detect-circular-reference.md) | Detects usage of `forwardRef()` method | ✅ |
| [`@trilon/enforce-close-testing-module`](docs/rules/enforce-close-testing-module.md) | Ensures NestJS testing modules are closed properly after tests | Recommended ✅ |
| [`@trilon/check-inject-decorator`](docs/rules/check-inject-decorator.md) | Detects incorrect usage of `@Inject(TOKEN)` decorator | Recommended ✅ |
| [`@trilon/detect-circular-reference`](docs/rules/detect-circular-reference.md) | Detects usage of `forwardRef()` method | Recommended ✅ |
| [`@trilon/enforce-custom-provider-type`](docs/rules/enforce-custom-provider-type.md) | Enforces a styleguide for provider types | Strict ⚠️ |
---

# Trilon Consulting
Expand Down
55 changes: 55 additions & 0 deletions docs/rules/enforce-custom-provider-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
description: 'Enforces a styleguide for provider types'
---

Large teams can have the desire to limit or enforce a particular style of creating [custom providers](https://docs.nestjs.com/fundamentals/custom-providers); e.g. banning request-scoped providers to avoid potential circular dependencies, or [preferring factory providers over value providers to significantly increase performance](https://github.com/nestjs/nest/pull/12753). This rule enforces a particular type of provider to be used.

## Options

This rule accepts an object with the "prefer" property, which is an array containing one or more of the following values:

- `value`: Enforces the use of value providers.
- `factory`: Enforces the use of factory providers.
- `class`: Enforces the use of class providers.
- `existing`: Enforces the use of existing providers.


### Example of Options

```json
"rules": {
"@trilon/enforce-custom-provider-type": [
"warn", {
"prefer": ["factory", "value"]
}
]
}
```

## Examples
Considering the options above, the following examples will show how the rule behaves when the `prefer` option is set to `factory`.

### ❌ Incorrect

```ts
const customValueProvider: Provider = {
provide: 'TOKEN',
useExisting: 'some-value' // ⚠️ provider is not of type ["factory", "value"]
}

const customClassProvider: Provider = {
provide: AbstractClass,
useClass: SomeClass // ⚠️ provider is not of type ["factory", "value"]
}
```

### ✅ Correct

const factoryProvider: Provider = {
provide: 'TOKEN',
useFactory: () => 'some-value'
}

## When Not To Use It

If you don't want to enforce a particular style of provider, you can disable this rule.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"LICENSE"
],
"engines": {
"node": ">=20.9.0",
"node": ">=18.*.*",
"yarn": ">=4.0.2"
},
"scripts": {
Expand Down
10 changes: 10 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import enforceCloseTestingModuleRule from './rules/enforce-close-testing-module.rule';
import checkInjectDecoratorRule from './rules/check-inject-decorator.rule';
import detectCircularReferenceRule from './rules/detect-circular-reference.rule';
import enforceCustomProviderTypeRule from './rules/enforce-custom-provider-type.rule';
// TODO: we should type this as ESLint.Plugin but there's a type incompatibilities with the utils package
const plugin = {
configs: {
Expand All @@ -11,11 +12,20 @@ const plugin = {
'@trilon/detect-circular-reference': 'warn',
},
},
strict: {
rules: {
'@trilon/enforce-close-testing-module': 'error',
'@trilon/check-inject-decorator': 'error',
'@trilon/detect-circular-reference': 'error',
'@trilon/enforce-custom-provider-type': 'error',
},
},
},
rules: {
'enforce-close-testing-module': enforceCloseTestingModuleRule,
'check-inject-decorator': checkInjectDecoratorRule,
'detect-circular-reference': detectCircularReferenceRule,
'@trilon/enforce-custom-provider-type': enforceCustomProviderTypeRule,
},
};

Expand Down
172 changes: 172 additions & 0 deletions src/rules/enforce-custom-provider-type.rule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
import {
ASTUtils,
AST_NODE_TYPES,
ESLintUtils,
type TSESTree,
} from '@typescript-eslint/utils';

const createRule = ESLintUtils.RuleCreator(
(name) => `https://eslint.org/docs/latest/rules/${name}`
);

type ProviderType = 'class' | 'factory' | 'value' | 'existing' | 'unknown';

export type Options = [
{
prefer: ProviderType[];
},
];

const defaultOptions: Options = [
{
prefer: [],
},
];

export type MessageIds = 'providerTypeMismatch';

export default createRule<Options, MessageIds>({
name: 'enforce-custom-provider-type',
meta: {
type: 'suggestion',
docs: {
description: 'Ensure that custom providers are of the preferred type',
},
fixable: undefined,
schema: [
{
type: 'object',
properties: {
prefer: {
type: 'array',
items: {
type: 'string',
enum: ['class', 'factory', 'value', 'existing'],
},
},
},
},
],
messages: {
providerTypeMismatch: 'Provider is not of type {{ preferred }}',
},
},
defaultOptions,
create(context) {
const options = context.options[0] || defaultOptions[0];
const preferredTypes = options.prefer;
const providerTypesImported: string[] = [];
return {
'ImportDeclaration[source.value="@nestjs/common"]': (
node: TSESTree.ImportDeclaration
) => {
const specifiers = node.specifiers;

const isImportSpecifier = (
node: TSESTree.ImportClause
): node is TSESTree.ImportSpecifier =>
node.type === AST_NODE_TYPES.ImportSpecifier;

const isProviderImport = (spec: TSESTree.ImportSpecifier) =>
[
'Provider',
'ClassProvider',
'FactoryProvider',
'ValueProvider',
].includes(spec.imported.name);

specifiers
.filter(isImportSpecifier)
.filter(isProviderImport)
.forEach((spec) =>
providerTypesImported.push(spec.local.name ?? spec.imported.name)
);
},

'Property[key.name="providers"] > ArrayExpression > ObjectExpression': (
node: TSESTree.ObjectExpression
) => {
for (const property of node.properties) {
if (property.type === AST_NODE_TYPES.Property) {
const providerType = providerTypeOfProperty(property);

if (
providerType &&
!preferredTypes.includes(providerType) &&
preferredTypes.length > 0
) {
context.report({
node: property,
messageId: 'providerTypeMismatch',
data: {
preferred: preferredTypes,
},
});
}
}
}
},

'Identifier[typeAnnotation.typeAnnotation.type="TSTypeReference"]': (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also capture providers inside @Module({ providers: [] })

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question Rick, I don't think so haha I'll have to create another test for that

Copy link
Contributor Author

@thiagomini thiagomini Feb 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is now fixed, @tuxmachine , could you look at the tests and see if I'm missing something? thanks!

node: TSESTree.Identifier
) => {
const typeName = (
node.typeAnnotation?.typeAnnotation as TSESTree.TSTypeReference
).typeName;

if (
ASTUtils.isIdentifier(typeName) &&
providerTypesImported.includes(typeName.name) &&
preferredTypes.length > 0
) {
const providerType = providerTypeOfIdentifier(node);
if (providerType && !preferredTypes.includes(providerType)) {
context.report({
node,
messageId: 'providerTypeMismatch',
data: {
preferred: preferredTypes,
},
});
}
}
},
};
},
});

function providerTypeOfIdentifier(
node: TSESTree.Identifier
): ProviderType | undefined {
const parent = node.parent;

if (ASTUtils.isVariableDeclarator(parent)) {
const init = parent.init;
let type: ProviderType | undefined;
if (init?.type === AST_NODE_TYPES.ObjectExpression) {
const properties = init.properties;
for (const property of properties) {
if (property.type === AST_NODE_TYPES.Property) {
type = providerTypeOfProperty(property);
}
}
}

return type;
}
}

function providerTypeOfProperty(
node: TSESTree.Property
): ProviderType | undefined {
const propertyKey = (node.key as TSESTree.Identifier)?.name;
return propertyKey === 'useClass'
? 'class'
: propertyKey === 'useFactory'
? 'factory'
: propertyKey === 'useValue'
? 'value'
: propertyKey === 'useExisting'
? 'existing'
: undefined;
};
Loading
Loading