Merge pull request #5 from darraghoriordan/feat/two-new-swagger-rules

feat: two new swagger rules

README.md

@@ -28,6 +28,8 @@ Nest Swagger
 -   api-property-matches-property-optionality
 -   controllers-should-supply-api-tags
 -   api-method-should-specify-api-operation
+-   api-enum-property-best-practices
+-   api-property-returning-array-should-set-array
 
 ## To install
 
@@ -185,3 +187,81 @@ class TestClass {
     }
 }
 ```
+
+### Rule: api-enum-property-best-practices
+
+If you use enums you should set the correct properties in the ApiProperty decorator. Note I don't actually check the types on the property, I only check properties where `enum: EnumType` is already set to make sure they are set correctly.
+
+If you don't use enumName then Open api will create a new enum for each api method. This is awful to use in a generated client.
+You don't need to use type any more. This used to be necessary in old versions to get enum strings correctly output.
+
+This is perfect
+
+```ts
+class TestClass {
+    @ApiPropertyOptional({enum: MyEnum, enumName: "MyEnum"})
+    thisIsAnEnumProp!: MyEnum;
+}
+```
+
+Fails - you don't need type
+
+```ts
+class TestClass {
+    @ApiPropertyOptional({type: MyEnum, enum: MyEnum, enumName: "MyEnum"})
+    thisIsAnEnumProp!: MyEnum;
+}
+```
+
+Fails - you need to add a name
+
+```ts
+class TestClass {
+    @ApiPropertyOptional({enum: MyEnum})
+    thisIsAnEnumProp!: MyEnum;
+}
+```
+
+### Rule: api-property-returning-array-should-set-array
+
+If you return an array you should indicate this in the api property. There are two ways to do this
+
+`ApiProperty({type:[String]}) OR ApiProperty({type:String, isArray:true})`
+
+I enforce the second long way! You can turn this off if you prefer the shorthand way but you won't get warned if you missed the array specification.
+
+This passes
+
+```ts
+class TestClass {
+    @ApiPropertyOptional({enumName: "MyEnum" isArray:true})
+    thisIsAProp!: MyEnum[];
+}
+```
+
+This passes
+
+```ts
+class TestClass {
+    @ApiPropertyOptional({type: String, isArray: true})
+    thisIsAProp!: Array<string>;
+}
+```
+
+This FAILS - missing isArray
+
+```ts
+class TestClass {
+    @ApiPropertyOptional({type: String})
+    thisIsAProp!: Array<string>;
+}
+```
+
+This FAILS - doesn't need isArray
+
+```ts
+class TestClass {
+    @ApiPropertyOptional({type: String, isArray: true})
+    thisIsAProp!: string;
+}
+```

TODO.md

@@ -2,7 +2,6 @@ A list of things to work on
 
 ## swagger rules
 
--   enum properties should specify enum param, not specify a type and should specify an enum name
 -   if method returns array or promise array then the api response should have isArray:true or a type of [type]
 -   objects returned or in the body input should have attributes for swagger
 

src/configs/noSwagger.ts

@@ -8,5 +8,8 @@ export = {
         "@darraghor/nestjs-typed/api-method-should-specify-api-operation":
             "off",
         "@darraghor/nestjs-typed/controllers-should-supply-api-tags": "off",
+        "@darraghor/nestjs-typed/api-enum-property-best-practices": "off",
+        "@darraghor/nestjs-typed/api-property-returning-array-should-set-array":
+            "off",
     },
 };

src/configs/recommended.ts

@@ -17,5 +17,8 @@ export = {
         "@darraghor/nestjs-typed/api-method-should-specify-api-operation":
             "error",
         "@darraghor/nestjs-typed/controllers-should-supply-api-tags": "error",
+        "@darraghor/nestjs-typed/api-enum-property-best-practices": "error",
+        "@darraghor/nestjs-typed/api-property-returning-array-should-set-array":
+            "error",
     },
 };

src/rules/apiEnumPropertyBestPractices/apiEnumPropertyBestPractices.test-data.ts

@@ -0,0 +1,96 @@
+export const testCases = [
+    {
+        moduleCode: `enum MyEnum{
+            ValA,
+            ValB
+        }
+        
+        class MyClass {
+            @ApiProperty({
+                type: BizTaskUiCollectionEnum,
+                enum: BizTaskUiCollectionEnum,
+            })
+            public myProperty!:MyEnum
+        }`,
+        needsEnumNameProperty: true,
+        needsTypeRemoved: true,
+        message: "type is present, no enum name",
+    },
+    {
+        moduleCode: `enum MyEnum{
+            ValA,
+            ValB
+        }
+        
+        class MyClass {
+            @ApiProperty({
+                enumName: BizTaskUiCollectionEnum,
+                enum: BizTaskUiCollectionEnum,
+            })
+            public myProperty!:MyEnum
+        }`,
+        needsEnumNameProperty: false,
+        needsTypeRemoved: false,
+        message: "perfect",
+    },
+    {
+        moduleCode: `enum MyEnum{
+            ValA,
+            ValB
+        }
+        
+        class MyClass {
+            @ApiProperty({
+                enum: BizTaskUiCollectionEnum,
+            })
+            public myProperty!:MyEnum
+        }`,
+        needsEnumNameProperty: true,
+        needsTypeRemoved: false,
+        message: "optional everywhere",
+    },
+    {
+        moduleCode: `enum MyEnum{
+            ValA,
+            ValB
+        }
+        
+        class MyClass {
+            @ApiProperty({
+                type: BizTaskUiCollectionEnum,
+            })
+            public myProperty!:MyEnum
+        }`,
+        needsEnumNameProperty: false,
+        needsTypeRemoved: false,
+        message: "not an enum",
+    },
+    {
+        moduleCode: `enum MyEnum{
+            ValA,
+            ValB
+        }
+        
+        class MyClass {
+            @ApiProperty()
+            public myProperty!:MyEnum
+        }`,
+        needsEnumNameProperty: false,
+        needsTypeRemoved: false,
+        message: "not an enum",
+    },
+    {
+        moduleCode: `enum MyEnum{
+            ValA,
+            ValB
+        }
+        
+        class MyClass {
+            @ApiProperty({})
+            public myProperty!:MyEnum
+        }`,
+        needsEnumNameProperty: false,
+        needsTypeRemoved: false,
+        message: "not an enum",
+    },
+];

src/rules/apiEnumPropertyBestPractices/apiEnumPropertyBestPractices.test.ts

@@ -0,0 +1,39 @@
+import {hasEnumSpecifiedCorrectly} from "./apiEnumPropertyBestPractices";
+import {testCases} from "./apiEnumPropertyBestPractices.test-data";
+import {typedTokenHelpers} from "../../utils/typedTokenHelpers";
+import {
+    fakeContext,
+    fakeFilePath,
+} from "../../utils/nestModules/nestProvidedInjectableMapper.test-date";
+import {TSESTree} from "@typescript-eslint/types";
+
+// should probably be split up into multiple tests
+describe("apiEnumPropertyBestPractices", () => {
+    test.each(testCases)(
+        "is an expected response for %#",
+        (testCase: {
+            moduleCode: string;
+            needsEnumNameProperty: boolean;
+            needsTypeRemoved: boolean;
+            message: string;
+        }) => {
+            const ast = typedTokenHelpers.parseStringToAst(
+                testCase.moduleCode,
+                fakeFilePath,
+                fakeContext
+            );
+
+            const hasValidEnumSpecResult = hasEnumSpecifiedCorrectly(
+                // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                (ast.body[1] as TSESTree.ClassDeclaration).body
+                    .body[0] as TSESTree.ClassProperty
+            );
+            expect(hasValidEnumSpecResult.needsEnumNameAdded).toEqual(
+                testCase.needsEnumNameProperty
+            );
+            expect(hasValidEnumSpecResult.needsTypeRemoved).toEqual(
+                testCase.needsTypeRemoved
+            );
+        }
+    );
+});

src/rules/apiEnumPropertyBestPractices/apiEnumPropertyBestPractices.ts

@@ -0,0 +1,95 @@
+import {TSESTree} from "@typescript-eslint/types";
+import {createRule} from "../../utils/createRule";
+import {typedTokenHelpers} from "../../utils/typedTokenHelpers";
+import {EnumTestResultModel} from "./enumTestResultModel";
+
+export const hasEnumSpecifiedCorrectly = (
+    node: TSESTree.ClassProperty
+): EnumTestResultModel => {
+    const decorators = typedTokenHelpers.getDecoratorsNamed(node, [
+        "ApiPropertyOptional",
+        "ApiProperty",
+    ]);
+
+    if (decorators.length === 0) {
+        return new EnumTestResultModel(false, false);
+    }
+
+    const firstArgument = (decorators[0].expression as TSESTree.CallExpression)
+        .arguments[0] as TSESTree.ObjectExpression;
+    if (!firstArgument) {
+        return new EnumTestResultModel(false, false);
+    }
+    // is it an enum prop?
+    // OK so this could be changed later to actually parse the property type for an enum
+    // and so check if enum: is needed too. However we don't do this yet so we depend on enum having
+    // been added already and this rule just recommends keeping it tidy. So, you will still have to at
+    // least remember to add "enum: Blah" to your api decorator.
+    const hasEnumProperty =
+        firstArgument.properties.find(
+            (p) =>
+                ((p as TSESTree.Property).key as TSESTree.Identifier).name ===
+                "enum"
+        ) !== undefined;
+
+    if (!hasEnumProperty) {
+        return new EnumTestResultModel(false, false);
+    }
+    const hasTypeProperty =
+        firstArgument.properties.find(
+            (p) =>
+                ((p as TSESTree.Property).key as TSESTree.Identifier).name ===
+                "type"
+        ) !== undefined;
+    const hasEnumNameProperty =
+        firstArgument.properties.find(
+            (p) =>
+                ((p as TSESTree.Property).key as TSESTree.Identifier).name ===
+                "enumName"
+        ) !== undefined;
+
+    return new EnumTestResultModel(hasTypeProperty, !hasEnumNameProperty);
+};
+
+const rule = createRule({
+    name: "api-enum-property-best-practices",
+    meta: {
+        docs: {
+            description:
+                "Enums should use the best practices for api documentation",
+            category: "Best Practices",
+            recommended: false,
+            requiresTypeChecking: false,
+        },
+        messages: {
+            needsEnumNameAdded: `Properties with enum should also specify an enumName property to keep generated models clean`,
+            needsTypeRemoved: `Properties with enum should not specify a type property`,
+        },
+        schema: [],
+        type: "suggestion",
+    },
+    defaultOptions: [],
+
+    create(context) {
+        return {
+            // eslint-disable-next-line @typescript-eslint/naming-convention
+            ClassProperty(node: TSESTree.ClassProperty): void {
+                const result = hasEnumSpecifiedCorrectly(node);
+                if (result.needsEnumNameAdded) {
+                    context.report({
+                        node: node,
+                        messageId: "needsEnumNameAdded",
+                    });
+                }
+                if (result.needsTypeRemoved) {
+                    context.report({
+                        node: node,
+                        messageId: "needsTypeRemoved",
+                    });
+                }
+            },
+        };
+    },
+});
+
+export default rule;

src/rules/apiEnumPropertyBestPractices/enumTestResultModel.ts

@@ -0,0 +1,6 @@
+export class EnumTestResultModel {
+    constructor(
+        public needsTypeRemoved: boolean,
+        public needsEnumNameAdded: boolean
+    ) {}
+}

src/rules/apiMethodsShouldSpecifyApiOperation/apiMethodsShouldSpecifyApiOperation.ts

@@ -57,7 +57,7 @@ const rule = createRule({
                 "Api methods should at least specify the expected OK response with @ApiOkResponse. But also add any error responses that might not be expected (e.g. 429)",
             category: "Best Practices",
             recommended: false,
-            requiresTypeChecking: true,
+            requiresTypeChecking: false,
         },
         messages: {
             shouldSpecifyApiOperation: `A method decorated with @Get, @Post etc. should specify the expected ApiOperation e.g. @ApiOkResponse(type: MyType)`,

src/rules/apiPropertyMatchesPropertyOptionality/apiPropertyMatchesPropertyOptionality.ts

@@ -41,7 +41,7 @@ const rule = createRule({
                 "Properties should have correct @ApiProperty decorators",
             category: "Best Practices",
             recommended: false,
-            requiresTypeChecking: true,
+            requiresTypeChecking: false,
         },
         messages: {
             shouldUseOptionalDecorator: `Property marked as optional should use @ApiPropertyOptional decorator`,