Skip to content

Commit

Permalink
Merge pull request #95 from getlarge/fix-handle-more-lint-cases
Browse files Browse the repository at this point in the history 
fix: improve ESlint plugin’s rule
  • Loading branch information
@getlarge
getlarge authored Sep 18, 2024
2 parents 1405d35 + 5a706b5 commit c2cb59c
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 88 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ This repository contains several helpful packages for NestJS that I have develop
| [AMQP-Transport](./packages/amqp-transport/README.md) | A NestJS microservice adapter around [AMQPlib] supporting exchanges. | [![npm](https://img.shields.io/npm/v/@getlarge/nestjs-tools-amqp-transport.svg?style=flat)](https://npmjs.org/package/@getlarge/nestjs-tools-amqp-transport) |
| [Async-Local-Storage](./packages/async-local-storage/README.md) | A NestJS module to track context with [AsyncLocalStorage]. | [![npm](https://img.shields.io/npm/v/@getlarge/nestjs-tools-async-local-storage?style=flat)](https://npmjs.org/package/@getlarge/nestjs-tools-async-local-storage) |
| [Cluster](./packages/cluster/README.md) | A class to manage workers' lifecycle in a (Node.js) [cluster]. | [![npm](https://img.shields.io/npm/v/@getlarge/nestjs-tools-cluster?style=flat)](https://npmjs.org/package/@getlarge/nestjs-tools-cluster) |
| [ESLint-Plugin](./packages/eslint-plugin/README.md) | A set of ESLint rules for NestJS applications. | [![npm](https://img.shields.io/npm/v/@getlarge/eslint-plugin-nestjs-tools?style=flat)](https://npmjs.org/package/@getlarge/eslint-plugin-nestjs-tools) |
| [ESLint-Plugin](./packages/eslint-plugin/README.md) | A set of ESLint rules for NestJS applications. | [![npm](https://img.shields.io/npm/v/@getlarge/eslint-plugin-nestjs-tools?style=flat)](https://npmjs.org/package/@getlarge/eslint-plugin-nestjs-tools) |
| [File-Storage](./packages/file-storage/README.md) | A NestJS module supporting [FS], [S3] and [GCP] strategies. | [![npm](https://img.shields.io/npm/v/@getlarge/nestjs-tools-file-storage?style=flat)](https://npmjs.org/package/@getlarge/nestjs-tools-file-storage) |
| [Lock](./packages/lock/README.md) | A NestJS module to provide a distributed lock for your application. | [![npm](https://img.shields.io/npm/v/@getlarge/nestjs-tools-lock?style=flat)](https://npmjs.org/package/@getlarge/nestjs-tools-lock) |
| [Fastify-Upload](./packages/fastify-upload/README.md) | A NestJS module to provide file upload support for Fastify. | [![npm](https://img.shields.io/npm/v/@getlarge/nestjs-tools-fastify-upload?style=flat)](https://npmjs.org/package/@getlarge/nestjs-tools-fastify-upload) |
Expand Down
6 changes: 5 additions & 1 deletion packages/eslint-plugin/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@ npm install --save @getlarge/eslint-plugin-nestjs-tools
This rule enforces that all public Service methods return a class instance of the same return type.
The purpose of this rule is to ensure you return class instances instead of plain objects, which is essentials when using the [`ClassSerializerInterceptor`](https://docs.nestjs.com/techniques/serialization#class-serializer-interceptor).

In order to use this rule, add it to your ESLint configuration file:

```json
{
"files": ["*.ts", "*.tsx"],
"plugins": ["@getlarge/nestjs-tools"],
"rules": {
"@getlarge/eslint-plugin-nestjs-tools/return-class-instance": "error"
"@getlarge/nestjs-tools/return-class-instance": "error"
}
}
```
3 changes: 2 additions & 1 deletion packages/eslint-plugin/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ export = {
configs: {
recommended: {
parser: '@typescript-eslint/parser',
plugins: ['@getlarge/nestjs-tools'],
rules: {
[`${name}/${returnClassInstanceName}`]: 'error',
[`@getlarge/nestjs-tools/${returnClassInstanceName}`]: 'error',
},
},
},
Expand Down
31 changes: 31 additions & 0 deletions packages/eslint-plugin/src/lib/return-class-instance.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,20 @@ ruleTester.run(RULE_NAME, rule, {
return new Test({ message: 'Hello API' });
}
}`,
// return an array of class instances
`import { Injectable } from '@nestjs/common';
class Test {
constructor(params) {
Object.assign(this, params);
}
message: string;
}
@Injectable()
export class AppService {
getData(): Test[] {
return [new Test({ message: 'Hello API' })]
}
}`,
// promise test case
`import { Injectable } from '@nestjs/common';
class Test {
Expand Down Expand Up @@ -158,5 +172,22 @@ ruleTester.run(RULE_NAME, rule, {
}
}`,
},
{
errors: [{ messageId: 'returnClassInstance' }],
code: `
import { Injectable } from '@nestjs/common';
class Test {
constructor(params) {
Object.assign(this, params);
}
message: string;
}
@Injectable()
export class AppService {
getData(): Test {
return new Test().toJSON();
}
}`,
},
],
});
211 changes: 126 additions & 85 deletions packages/eslint-plugin/src/lib/return-class-instance.ts
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,53 @@
/* eslint-disable max-lines-per-function */
import { ESLintUtils } from '@typescript-eslint/utils';
import { ESLintUtils, TSESTree } from '@typescript-eslint/utils';

export const RULE_NAME = 'return-class-instance';

const isNestServiceMethod = (node: TSESTree.MethodDefinition) => {
const ignoredKinds = new Set(['constructor', 'get', 'set']);
const ignoredMethods = new Set([
'onModuleInit',
'onModuleDestroy',
'onApplicationBootstrap',
'beforeApplicationShutdown',
'onApplicationShutdown',
]);

return (
node.static === false &&
node.type === 'MethodDefinition' &&
node.key.type === 'Identifier' &&
ignoredMethods.has(node.key.name) === false &&
ignoredKinds.has(node.kind) === false &&
node.computed === false &&
node.accessibility !== 'private' &&
node.accessibility !== 'protected' &&
node.parent?.type === 'ClassBody' &&
node.parent.parent.type === 'ClassDeclaration' &&
node.parent.parent.decorators?.some(
(decorator) =>
decorator.expression.type === 'CallExpression' &&
decorator.expression.callee.type === 'Identifier' &&
decorator.expression.callee.name === 'Injectable',
)
);
};

const getTypeNameFromReturnType = (rawReturnType: string) => {
return rawReturnType.replaceAll(/Promise<([^<>]*)>/g, '$1').replace(': ', '');
return rawReturnType
.replaceAll(/Promise<([^<>]*)>/g, '$1')
.replace(': ', '')
.replaceAll('[]', '');
};

const isPrimitive = (value: string) => {
return value === 'string' || value === 'number' || value === 'boolean';
};

const isBuffer = (value: string) => {
return value === 'Buffer';
};

const isVoid = (value: string) => {
return value === 'void';
};
Expand All @@ -24,7 +61,68 @@ const isNullable = (type: string) => ['null', 'undefined'].includes(type);
const extractUnionTypes = (typeName: string) => typeName.split('|').map((type) => type.trim());

const isValidReturnType = (type: string) =>
isPrimitive(type) || isVoid(type) || isLiteralType(type) || isNullable(type);
isPrimitive(type) || isVoid(type) || isLiteralType(type) || isNullable(type) || isBuffer(type);

// eslint-disable-next-line complexity
const isReturnClassInstance = (
returnStatement: TSESTree.ReturnStatement,
typeName: string,
extractedTypes: string[],
) => {
const returnArgument = returnStatement.argument;

// Handle class instance
if (returnArgument?.type === 'NewExpression' && returnArgument.callee?.type === 'Identifier') {
return returnArgument.callee.name === typeName;
}

// Handle array of class instances
if (returnArgument?.type === 'ArrayExpression' && returnArgument.elements[0]?.type === 'NewExpression') {
return (
returnArgument.elements[0].callee?.type === 'Identifier' && returnArgument.elements[0].callee.name === typeName
);
}

if (returnArgument?.type === 'CallExpression') {
const calleeName =
returnArgument.callee?.type === 'Identifier'
? returnArgument.callee.name
: returnArgument.callee.type === 'MemberExpression' && returnArgument.callee.object.type === 'Identifier'
? returnArgument.callee.object.name
: '';

// Handle Promise.resolve cases
if (
calleeName === 'Promise' &&
returnArgument.callee.type === 'MemberExpression' &&
returnArgument.callee.property.type === 'Identifier' &&
returnArgument.callee.property.name === 'resolve' &&
returnArgument.arguments[0].type === 'NewExpression' &&
returnArgument.arguments[0].callee?.type === 'Identifier'
) {
return returnArgument.arguments[0].callee.name === typeName;
}

// Handle this.service.method() cases
if (
returnArgument.callee?.type === 'MemberExpression' &&
returnArgument.callee.object.type === 'MemberExpression' &&
returnArgument.callee.object.object.type === 'ThisExpression' &&
returnArgument.callee.object.property.type === 'Identifier'
) {
return true;
}

return false;
}
// Handle literal types and null/undefined
if (returnArgument?.type === 'Literal' || returnArgument === null) {
return returnArgument?.value?.toString()
? extractedTypes.includes(returnArgument.value.toString())
: extractedTypes.includes('null');
}
return false;
};

export const rule = ESLintUtils.RuleCreator(() => __filename)({
name: RULE_NAME,
Expand All @@ -42,93 +140,36 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)({
},
defaultOptions: [],
create(context) {
const ignoredKinds = new Set(['constructor', 'get', 'set']);
const ignoredMethods = new Set([
'onModuleInit',
'onModuleDestroy',
'onApplicationBootstrap',
'beforeApplicationShutdown',
'onApplicationShutdown',
]);

return {
MethodDefinition(node) {
if (
node.static === false &&
node.type === 'MethodDefinition' &&
node.key.type === 'Identifier' &&
ignoredMethods.has(node.key.name) === false &&
ignoredKinds.has(node.kind) === false &&
node.computed === false &&
node.accessibility !== 'private' &&
node.accessibility !== 'protected' &&
node.parent?.type === 'ClassBody' &&
node.parent.parent.type === 'ClassDeclaration' &&
node.parent.parent.decorators?.some(
(decorator) =>
decorator.expression.type === 'CallExpression' &&
decorator.expression.callee.type === 'Identifier' &&
decorator.expression.callee.name === 'Injectable',
)
) {
const returnType = node.value.returnType;
if (!returnType) {
if (!isNestServiceMethod(node)) {
return;
}
const returnType = node.value.returnType;
if (!returnType) {
context.report({
node,
messageId: 'missingReturnType',
});
} else {
const returnTypeText = context.sourceCode.getText(returnType);
const typeName = getTypeNameFromReturnType(returnTypeText);
const extractedTypes = extractUnionTypes(getTypeNameFromReturnType(returnTypeText));

// If all union types are primitive, void, or literals, we return early
if (extractedTypes.every(isValidReturnType)) {
return;
}
const returnStatements = node.value?.body?.body?.filter((node) => node.type === 'ReturnStatement') ?? [];

const returnClassInstance = returnStatements.every((returnStatement) =>
isReturnClassInstance(returnStatement, typeName, extractedTypes),
);
if (!returnClassInstance) {
context.report({
node,
messageId: 'missingReturnType',
messageId: 'returnClassInstance',
});
} else {
const returnTypeText = context.sourceCode.getText(returnType);
const typeName = getTypeNameFromReturnType(returnTypeText);
const extractedTypes = extractUnionTypes(getTypeNameFromReturnType(returnTypeText));

// If all union types are primitive, void, or literals, we return early
if (extractedTypes.every(isValidReturnType)) {
return;
}
const returnStatements = node.value?.body?.body?.filter((node) => node.type === 'ReturnStatement') ?? [];

const returnClassInstance = returnStatements.every((returnStatement) => {
const returnArgument = returnStatement.argument;

if (returnArgument?.type === 'NewExpression' && returnArgument.callee?.type === 'Identifier') {
return returnArgument.callee.name === typeName;
}

if (returnArgument?.type === 'CallExpression') {
const calleeName =
returnArgument.callee?.type === 'Identifier'
? returnArgument.callee.name
: returnArgument.callee.type === 'MemberExpression' &&
returnArgument.callee.property.type === 'Identifier'
? returnArgument.callee.property.name
: '';

// Handle Promise.resolve cases
if (
calleeName === 'Promise' &&
returnArgument.arguments[0].type === 'NewExpression' &&
returnArgument.arguments[0].callee?.type === 'Identifier'
) {
return returnArgument.arguments[0].callee.name === typeName;
}
return true;
}
// Handle literal types and null/undefined
if (returnArgument?.type === 'Literal' || returnArgument === null) {
return returnArgument?.value?.toString()
? extractedTypes.includes(returnArgument.value.toString())
: extractedTypes.includes('null');
}
return false;
});

if (!returnClassInstance) {
context.report({
node,
messageId: 'returnClassInstance',
});
}
}
}
},
Expand Down

0 comments on commit c2cb59c

Please sign in to comment.