rhys-vdw / ts-auto-guard

Generate type guard functions from TypeScript interfaces
MIT License
501 stars 55 forks source link

Build should error with config that has `strictNullChecks: false` #152

Open nygrenh opened 3 years ago

nygrenh commented 3 years ago

Given the following interface:

export interface Example {
  name: string | null;
}

If I generate a type guard for this, it creates the following function:

import { Example } from "./test";

export function isExample(obj: any, _argumentName?: string): obj is Example {
    return (
        (obj !== null &&
            typeof obj === "object" ||
            typeof obj === "function") &&
        typeof obj.name === "string"
    )
}

Now if call the type guard like this:

const obj: Example = { name: null }
console.log(isExample(obj))

The output is false, but I'd expect this to output true.

rhys-vdw commented 3 years ago

@nygrenh hm... so there's no specific test cast for union type with null. If this is indeed the current behaviour then it's a terrible bug, and likely a regression. Can you confirm whether you have strictNullChecks enabled?

nygrenh commented 3 years ago

strictNullChecks was not enabled. If I switch it on, the output changes to the correct form:

export function isExample(obj: any, _argumentName?: string): obj is Example {
    return (
        (obj !== null &&
            typeof obj === "object" ||
            typeof obj === "function") &&
        (obj.name === null ||
            typeof obj.name === "string")
    )
}
rhys-vdw commented 3 years ago

@nygrenh this is known behaviour. There is a conceivable way that you could generate guards with strictNullChecks disabled, it would basically mean allowing any field or object to be null or undefined.

We could forcibly override this option before ts-auto-guard runs. However that would give incorrect results too. For example:

import { isMyInterface } from "./MyInterface.guard.ts"

// without null checks
interface MyInterface {
  value: int;
}

let valueInt: MyInterface = { value: 5 }; // valid
let valueUndefined: MyInterface = { value: undefined }; // valid
let valueNull: MyInterface = { value: null }; // valid

isMyInterface(valueInt); // true
isMyInterface(valueUndefined); // incorrectly returns false
isMyInterface(valueNull); // incorrectly returns false

But if we go the other way and permit any value to be undefined or null it's a lot of noise and the end validators are mostly useless. So it's not really clear how to handle this setting.

Generally speaking the goal of this project is to create type checks for people who want strict types, so the intersection with people working with strictNullChecks disabled is presumably small.

See #120, see #99

I think the solution is to warn or error when the program runs.

nygrenh commented 3 years ago

Yeah, I actually had strictNullChecks off only by accident -- was setting up a new project and didn't notice it.

I agree that a warning / error would be a good solution to this.

Thanks for your help!

rhys-vdw commented 3 years ago

Marking this issue as "help wanted". Further discussion notwithstanding, the accepted solution will error the build if structNullChecks is false. This must correctly handle the case where strict: true is enabled and strictNullChecks is not provided.

rhys-vdw commented 3 years ago

Further thoughts... The ideal behaviour here would be for ts-morph to handle this naturally by including null or undefined as a possible value for every single type with strictNullChecks disabled. I suspect this is the behaviour of typescript compiler itself and may be undesirable for other use cases.