-
Notifications
You must be signed in to change notification settings - Fork 0
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
Changes from 9 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 061b85b
feat: ensure the provider type is factory
thiagomini 81f5303
test: provider type is class or useExisting
thiagomini a4009e9
feat: support aliasing provider type
tolgap ee1dadc
feat: ensure we only check for Provider imported from nestjs package
thiagomini e84a24c
Merge branch 'feat/enforce-custom-provider-type' of github.com:Trilon…
thiagomini 8583c37
refactor: use filter and forEach to get specifiers
thiagomini e4608a0
refactor: remove unnecessary type assertion
thiagomini 0e767f0
docs: added comments to tests
thiagomini a2b9f7d
feat: report error when provider type in providers array is invalid
thiagomini 93553a2
test: add test for all types of providers
thiagomini 51a7e69
refactor: simplify getProviderType code
thiagomini 5167f01
refactor: extract common logic to method
thiagomini 6555624
chore: update node version
thiagomini b03a987
style: fix indentation
thiagomini 58647c7
docs: add docs for new rule and update README
thiagomini a9b0473
chore: update node version hint
thiagomini 509c9ba
refactor: make preferred type an array
thiagomini c815a6a
test: create tests for multiple preferred types
thiagomini fbad230
docs: update docs to include array of preferred types
thiagomini 55a03f6
chore: change node version hint to 18 to make it less strict
thiagomini 7d5cd24
chore: delete nvmrc file
thiagomini 02c2f7b
chore: add symlink to .node-version
thiagomini File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,160 @@ | ||
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: 'factory', | ||
}, | ||
]; | ||
|
||
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: 'string', | ||
enum: ['class', 'factory', 'value'], | ||
}, | ||
}, | ||
}, | ||
], | ||
messages: { | ||
providerTypeMismatch: 'Provider is not of type {{ preferred }}', | ||
}, | ||
}, | ||
defaultOptions, | ||
create(context) { | ||
const options = context.options[0] || defaultOptions[0]; | ||
const preferredType = 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) | ||
); | ||
}, | ||
|
||
'Identifier[typeAnnotation.typeAnnotation.type="TSTypeReference"]': ( | ||
node: TSESTree.Identifier | ||
) => { | ||
const typeName = ( | ||
node.typeAnnotation?.typeAnnotation as TSESTree.TSTypeReference | ||
).typeName; | ||
|
||
if ( | ||
ASTUtils.isIdentifier(typeName) && | ||
providerTypesImported.includes(typeName.name) | ||
) { | ||
const providerType = getProviderType(node); | ||
if (providerType && providerType !== preferredType) { | ||
context.report({ | ||
node, | ||
messageId: 'providerTypeMismatch', | ||
data: { | ||
preferred: preferredType, | ||
}, | ||
}); | ||
} | ||
} | ||
}, | ||
}; | ||
}, | ||
}); | ||
|
||
function getProviderType(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 && | ||
ASTUtils.isIdentifier(property.key) && | ||
property.key.name === 'useFactory' | ||
) { | ||
type = 'factory'; | ||
break; | ||
} | ||
|
||
if ( | ||
property.type === AST_NODE_TYPES.Property && | ||
ASTUtils.isIdentifier(property.key) && | ||
property.key.name === 'useClass' | ||
) { | ||
type = 'class'; | ||
break; | ||
} | ||
|
||
if ( | ||
property.type === AST_NODE_TYPES.Property && | ||
ASTUtils.isIdentifier(property.key) && | ||
property.key.name === 'useValue' | ||
) { | ||
type = 'value'; | ||
break; | ||
} | ||
|
||
if ( | ||
property.type === AST_NODE_TYPES.Property && | ||
ASTUtils.isIdentifier(property.key) && | ||
property.key.name === 'useExisting' | ||
) { | ||
type = 'existing'; | ||
break; | ||
} | ||
} | ||
} | ||
|
||
return type; | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
import { RuleTester } from '@typescript-eslint/rule-tester'; | ||
import enforceCustomProviderTypeRule from '../../src/rules/enforce-custom-provider-type.rule'; | ||
|
||
// This test required changes to the tsconfig file to allow importing from the rule-tester package. | ||
// See https://github.com/typescript-eslint/typescript-eslint/issues/7284 | ||
|
||
const ruleTester = new RuleTester({ | ||
parserOptions: { | ||
project: './tsconfig.json', | ||
}, | ||
parser: '@typescript-eslint/parser', | ||
defaultFilenames: { | ||
// We need to specify a filename that will be used by the rule parser. | ||
// Since the test process starts at the root of the project, we need to point to the sub folder containing it. | ||
ts: './tests/rules/file.ts', | ||
tsx: '', | ||
}, | ||
}); | ||
|
||
ruleTester.run('enforce-custom-provider-type', enforceCustomProviderTypeRule, { | ||
valid: [ | ||
// Test for when the provider is of the preferred type | ||
{ | ||
code: ` | ||
import { Provider } from '@nestjs/common'; | ||
|
||
const factoryProvider: Provider = { | ||
provide: 'TOKEN', | ||
useFactory: () => 'some-value' | ||
} | ||
`, | ||
options: [ | ||
{ | ||
prefer: 'factory', | ||
}, | ||
], | ||
}, | ||
// Test for when the Provider type is not imported from '@nestjs/common' | ||
{ | ||
code: ` | ||
import { Provider } from 'some-other-package'; | ||
const customValueProvider: Provider = { | ||
useValue: 'some-value', | ||
provide: 'TOKEN' | ||
} | ||
`, | ||
options: [ | ||
{ | ||
prefer: 'factory', | ||
}, | ||
], | ||
}, | ||
], | ||
invalid: [ | ||
// Test for when the provider is not of the preferred type | ||
{ | ||
code: ` | ||
import { Provider } from '@nestjs/common'; | ||
const customValueProvider: Provider = { | ||
provide: 'TOKEN', | ||
useValue: 'some-value' // ⚠️ provider is not of type "factory" | ||
} | ||
`, | ||
options: [ | ||
{ | ||
prefer: 'factory', | ||
}, | ||
], | ||
errors: [ | ||
{ | ||
messageId: 'providerTypeMismatch', | ||
}, | ||
], | ||
}, | ||
{ | ||
code: ` | ||
import { Provider } from '@nestjs/common'; | ||
import { SomeClass } from './some-class'; | ||
const customValueProvider: Provider = { | ||
provide: 'TOKEN', | ||
useClass: SomeClass // ⚠️ provider is not of type "factory" | ||
} | ||
`, | ||
options: [ | ||
{ | ||
prefer: 'factory', | ||
}, | ||
], | ||
errors: [ | ||
{ | ||
messageId: 'providerTypeMismatch', | ||
}, | ||
], | ||
}, | ||
{ | ||
code: ` | ||
import { Provider } from '@nestjs/common'; | ||
import { EXISTING_TOKEN } from './token'; | ||
const customValueProvider: Provider = { | ||
provide: 'TOKEN', | ||
useExisting: EXISTING_TOKEN // ⚠️ provider is not of type "factory" | ||
} | ||
`, | ||
options: [ | ||
{ | ||
prefer: 'factory', | ||
}, | ||
], | ||
errors: [ | ||
{ | ||
messageId: 'providerTypeMismatch', | ||
}, | ||
], | ||
}, | ||
// Test for when the provider is not of the preferred type and was renamed | ||
{ | ||
code: ` | ||
import { Provider as NestProvider } from '@nestjs/common'; | ||
import { EXISTING_TOKEN } from './token'; | ||
const customValueProvider: NestProvider = { | ||
provide: 'TOKEN', | ||
useExisting: EXISTING_TOKEN // ⚠️ provider is not of type "factory" | ||
} | ||
`, | ||
options: [ | ||
{ | ||
prefer: 'factory', | ||
}, | ||
], | ||
errors: [ | ||
{ | ||
messageId: 'providerTypeMismatch', | ||
}, | ||
], | ||
}, | ||
], | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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: [] })
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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!