darraghoriordan / eslint-plugin-nestjs-typed

Some eslint rules for working with NestJs projects
http://www.darraghoriordan.com
171 stars 34 forks source link

validated-non-primitive-property-needs-type-decorator is erroring for optional enum properties #36

Closed iddan closed 1 year ago

iddan commented 1 year ago

The rule validated-non-primitive-property-needs-type-decorator is erroring for optional enum properties. This is a regression from previous versions which prevents me from updating the package for quite some time now.

For example:

enum Bar {}

export class Foo {
    @ApiProperty({
      enum: Bar,
      enumName: "Bar",
    })
    @IsOptional()
    baz!: Bar | null;
  }

Or:

enum Baz {}

class Foo {
  @ApiPropertyOptional({
    enum: Baz,
    enumName: "Baz",
    required: false,
    isArray: true,
  })
  @IsOptional()
  bar?: Baz[];
}
darraghoriordan commented 1 year ago

The rule uses the enum validation to detect if the non-primitive type is an enum and ignore the rule. So you can prevent the lint error by applying @IsEnum(Baz) or @IsEnum(Bar) in the above cases. If i get a chance i'll try changing it to check the type of Bar/Baz is an actual enum. But it might take me a while 😂

(it's a good idea to validate that the client supplied values are valid enum values anyway)

darraghoriordan commented 1 year ago

I improved the error message for the rule to suggest adding "IsEnum()" for enums

i noticed that i do (try to) check the property type in the rule for enums but maybe it's not working for some reason. The unit tests pass with your scenario, but your scenario also errors for me when in a real nestjs project. So it's likely something to do with the parser? but i'm not 100% sure. IsEnum solves it anyway so i never noticed

iddan commented 1 year ago

Sorry for the dumb question, but can I use IsEnum() for nullable / array properties?

darraghoriordan commented 1 year ago

Sorry for the dumb question, but can I use IsEnum() for nullable / array properties?

you'll have to test the "null" one works in your scenario. i'm not sure how you're using it.

For the array you must tell the validation decorator it is an enum array: @IsEnum(Baz, { each: true })

It's described here in the class validator docs: https://github.com/typestack/class-validator#validating-arrays

iddan commented 1 year ago

Can we add an auto-fix for using the IsEnum() decorator?

darraghoriordan commented 1 year ago

i believe this is resolved