Support nullability check even when type is not specified

[mprisehk]

First off: this is a neat plugin, thanks for making it :) We have a ton of entities and we can really use this to save some headache. I encountered an issue/feature request that I would like raise which I think should make it more complete and intuitive to use. Here goes:

I have an Entity where I made the nullability wrong on purpose:

@Entity('containers')
export class Container {
  @Column({ nullable: true })
  code: string //should error, missing `| null`

  @Column({ nullable: false })
  name: string | null //should error, column isn't nullable
}

However, the plugin doesn't show an error. It took me a bit to figure out why. As it turns out, errors will only start showing once you specify the type:

However, not specifying a type is perfectly fine in TypeORM (it will pick a basic column type based on the Typescript type) and most of our entities are defined that way. It does not make sense to add hundreds of type properties on the fields of all of our entities just to make this check work. However, we would like to use this check to catch nullability inconsistencies. It seems to me that, even though the plugin has no opinion on whether the column is (e.g.) number | null or string | null, it should still be able to assess the nullability as an issue on its own.

[daniel7grant]

Thanks for using the library! I only used some TypeORM features, so I was not familiar that you can omit the type parameter. I changed the code that it falls back to the TypeScript types, it is in next release: v0.2.6. Can you try if this solves your problem?

[mprisehk]

Awesome, thanks for the swift fix! I tested it and it works, already caught more than two dozen errors in our entities 😅

I played around with some more values though, and I noticed one more thing. Using an empty Column() operator should mean the column acts as a nullable: false, however as shown in the pictures below, adding | null to the type there does not show an error:

while when explicitly setting it to nullable: false, it does:

[daniel7grant]

I had to fix a condition, I updated master with this change. I wait and if you don't find any more bugs I can put out a new release. 😄

[mprisehk]

Nice! I went over all our entities and I did find some more stuff:

1. We have two columns of type time without time zone and timestamp with time zone, marked explicitly as nullable: true, yet the plugin gives an error on the string | null type:
   
   Edit: This seems to have to do with the ColumnOptions being ignored when it is the second parameter. The following should give an error but doesn't:
   

2. A small thing (and not something we had in practice, but still), I am able to mark a @PrimaryGeneratedColumn() as | null without an error, which does not make sense:
   

3. I have one other thing but it's more a feature request. I'll make a separate issue for it.
   Edit: logged #9

[daniel7grant]

Thanks for the list, you are 100% correct, I didn't have in mind that you can set types as strings. I fixed this so now you can use multiple parameters in a decorators as it usually works in TypeORM.

Also you reminded me a feature I wanted to add, which is the odd behaviour that DeleteDateColumn is nullable by default. To make these work, I added primary columns (PrimaryColumn, PrimaryGeneratedColumn) and special columns ( CreateDateColumn, UpdateDateColumn , DeleteDateColumn). Feel free to test these unreleased changes on master.

[mprisehk]

Great! I haven't built/used the code locally yet, will have to look into that. Planning to do so after the weekend!

[mprisehk]

Sorry for the delay, I only got to testing it now. But good news, for items 1 & 2 and all variations on their cases I could think of, it all works as expected! Thank you 😊

[daniel7grant]

Thanks for testing, if all's good in v0.3.0, you can close this issue.

[mprisehk]

All is good, thanks again! Closing issue 👍