Proposal / Suggestion : Excess Property Checking for Union Without Discriminant Properties.

[jack-williams]

I started playing around with excess property checking for unions without discriminant properties, proof-of-concept. (Currently it's a hack). Before ploughing on with any more work I thought it was best to get some feedback.

Related: #20863 #12745 #20060

Idea:

When computing known properties retain a set of type id's for pruned types that have failed previous excess property checks. When checking later properties we cannot use these pruned types as evidence that the property is not 'excess'.

Basic Example:

type Foo = {x: number} | {y: string};
let test: Foo = {x: 4, y: "hello"}; // error.

On checking property x we prune type {y: string} because x is excess. When checking y, we cannot use type {y: string} as it was previously pruned, therefore we only have {x: number} which is also excess. The snippet of the code that checks this can be found at the bottom (knowPropertyWithPruning).

Real Examples:

The proof-of-concept catches the error for the example in #20863, marking author as excess.

Using pruned set post excess property checks.

The pruned set can be used even when an excess property violation was not found. After the check, filter the target type by the pruned types to prevent assignment to branches in a union that have been violated by excess property checking. The p-o-c catches errors for the following examples:

In this example we get an error because PropBase gets pruned in excess property checking so the function convert must satisfy the type in PropsWithConvert.

In this example we get an error on passes because excess property checking prunes B, so z must satisfy boolean.

The example in #20060 is also flagged as an error.

Limitations

It does not catch the error here. The problem is that excess property checking is only 1-deep at a time. After the first level (which has no excess properties), the type is no longer marked as a fresh literal so the deeper checks don't look for excess properties (using getRegularTypeOfObjectLiteral). I tried changing this to only mark 1 level at a time, but it caused a perf regression.

I've not looked into perf impact with this. The main extra work that is being done is not short-circuiting the known property traversal for union types, as we want to prune all types. If there is a fast discriminant check then we can only add this extra cost when needed. I would be interested to know if there was a perf improvement in the isRelatedTo check for a union that has had some branches pruned by excess property checks.

Error Messages

I didn't do anything with the error messages and consequently they can be pretty awful. The problem is that it can be multiple properties that contribute to the error, but only the last gets flagged. E.g.:

type Foo = {x: number} | {y: string};
let test: Foo = {x: 4, y: "hello"}; // error.
// Type '{ x: number; y: string; }' is not assignable to type 'Foo'.
//   Object literal may only specify known properties,
//   and 'y' does not exist in type 'Foo'. [2322]

The property y gets flagged here because it's the last, but x could be equally responsible. Furthermore, the claim that y is not in Foo is completely misleading. I think good error reporting is really important here: I'm open to any suggestions.

Appendix

function knownPropertyWithPruning(targetType: Type, name: __String, isComparingJsxAttributes: boolean, pruned: {[id: number]: boolean}): boolean {
    if (targetType.flags & TypeFlags.Object) {
        const knownInObject = propInObject(<ObjectType>targetType, name, isComparingJsxAttributes);
        if(!knownInObject) {
            pruned[targetType.id] = true;
        }
        return knownInObject
    }
    else if (targetType.flags & TypeFlags.Union) {
        let res: boolean = false;
        for (const t of (<UnionOrIntersectionType>targetType).types) {
            if (!pruned[t.id] && knownPropertyWithPruning(t, name, isComparingJsxAttributes, pruned)) {
                res = true;
            }
        }
        return res;
    }
    else if (targetType.flags & TypeFlags.Intersection) {
        for (const t of (<UnionOrIntersectionType>targetType).types) {
            if (isKnownProperty(t, name, isComparingJsxAttributes)) {
                return true;
            }
        }
        pruned[targetType.id] = true;
    }
    return false;
}

[ghost]

CC @sandersn (who is working on #20863)

[jack-williams]

Looking back, the TLDR of my post is:

• in isRelatedTo when the source is an object literal
• let t = filterType(target, x => !hasExcessProperties(source, t, ....)).
• if t is never, signal excess property error, otherwise continue checking with the filtered type t (instead of target).

[jack-williams]

Closing this in favour of the proof-of-concept here: #23798.

The ideas I discussed in this issue are not complete and only handle a small section of cases.