Lack of expected compiler error with type declaration involving a type union

[BrianYoung27]

Bug Report

With types AC = {a: string, c: string} and B = {b: string}, setting a variable to {b: 'value of b', c: 'value of c'} with type declaration of AC fails and with B fails but AC|B succeeds.

Maybe I'm missing something here but if the value fails to satisfy AC and B shouldn't it fail to satisfy AC|B and result in a compiler error? Is the type union stripping out the Excess Property Checks for Object literals and letting it satisfy B?

Thanks!

🔎 Search Terms

Type union

🕗 Version & Regression Information

Just noticed it today and repro'd with 5.1.3, Nightly, and all the way back to 3.x in Playground (which probably means this is either known or working as designed). =/

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about "union" and "declaration"

⏯ Playground Link

Playground link with relevant code

💻 Code

type AC = {
    a: string, 
    c: string
};
type B = {
    b: string
};

const ShouldAndDoesFail: AC = {
        b: 'value for b',
        c: 'value for c'
};
const ShouldAndDoesFailToo: B = {
        b: 'value for b',
        c: 'value for c'
};
const ShouldFailButWorks: AC|B = {
        b: 'value for b',
        c: 'value for c'
};

🙁 Actual behavior

Setting ShouldAndDoesFail and ShouldAndDoesFailToo results in compiler errors but ShouldFailButWorks does not.

🙂 Expected behavior

Expected ShouldFailButWorks to have a compiler error based on it being declared as AC|B and it fails when declared as either AC or B.

[MartinJohns]

Duplicate of #39050. In general you should not rely on excess property checks for your code to work.

[BrianYoung27]

Duplicate of #39050. In general you should not rely on excess property checks for your code to work.

Agreed, though in this case we're not relying on the check for the code to work but rather for the code to break. The hope is to have a compile error prevent someone from adding a bogus Object literal. If they do and the compiler doesn't flag it it compiles and runs fine, other than the bogus literal being ignored (which is harder to track down).

Also, thanks for the link to the earlier issue. Sorry for the duplicate.

[fatcerberus]

The hope is to have a compile error prevent someone from adding a bogus Object literal. If they do and the compiler doesn't flag it it compiles and runs fine, other than the bogus literal being ignored

Yeah, you shouldn’t rely on the excess property check to enforce runtime invariants - it’s designed primarily as a lint-type feature and is intentionally not comprehensive.

[yin]

I think preserving excess property checks would be useful in certain situations, like in a class setting, or if you want to build a small self-consistent model, like behavior in a logic game, which is well described by algebraic types.

It will not hurt if the type system is self-consistent.

[fatcerberus]

I think preserving excess property checks would be useful in certain situations, like …

If you mean “preserving excess property checks” in the sense of making EPC part of the type system proper, then that’s #12936.

[yin]

@fatcerberus Yes, I was just about to ask for a formal spec. I'll give it a read. I'd appreciate any suggestions here.

I tracked down function hasExcessProperties: src/compiler/checker.ts#20883

I'll report back what I find out.

[yin]

We should at least discuss how the excess property check should work regardless of whether bugfix this or complete Exact Types. It helps with building a consensus.

I have done an experiment with the flow to see what the expectations might be from the community:

See: https://flow.org/try/#1N4Igxg9gdgZglgcxALlAJwKYEMwBcD6aArlLnALYYrgA2WAzvXGCADQgYAeOBARgJ74AJhhhYiNXClzEM7DFCLl602QF92kEdQD0AKgAEAARg0IAdwN6dAHVL8ADhgMBBAwF4DwLMkXleGGhqANx2uI7OAEIeXry+SgFBoVB2NBi4BlAAjMiuAD7RMd7IAAwhqemZAEy5LgUGRXFlyWkZUADMtfVFYKXlUK2ZACxdhZ7FJawGTf2DUACsow3jPpMGvc0VbQBsS42lUxv9dluZOfljXquzlVA1F8uxfS23nQ89z6dQI+8rB9OfAa3Ra-K7-I4vHZ7cZNQ6AtggABugSY0GoiJKADoqiVtpiSiA1EA

I find that some cases could be improved but for the cost of time complexity. For example:

let n5: A|B  = {a:0, c:0};

gives a plausibly correct error message:

Cannot assign object literal to n5 because: [incompatible-type] Either property c is missing in A [1] but exists in object literal [2]. Or property a is missing in B [3] but exists in object literal [2].../-

Notice that property c is missing from both A and B, which is omitted. Seems they enumerate all union types reporting an error for each type. They don't focus on individual properties but on individual types and report on them.

When boiled to the simplest case:

type A = {a:number};
type B = {b:number};

let n4: A|B  = {a:0, b:0};

And then there's the case, where there's the slightly more complicated case:

type A = {a:number};
type B = {b:number};

let n4: A|B  = {a:0, b:0, x:0};

We should pay attention so we don't cause error masking in our case, as that's not the case elsewhere.

I'll play with the current code to get a feel for the codebase and try to fix excess property checks, it might be worth giving it a try.

[yin]

I have coded up the change quickly and played with it a bit. I took many shortcuts to get there so quickly, but it made it worth the effort.

One thing which probably needs optimizing would be the nested for loop which has O(N*M) best complexity. I did not bother providing a detailed error message, because I don't know proper error reporting in the checker well.

If we took a different path with a recursive approach, we can make it faster. We would need to collect error reports for each type and store them. We can get away with reporting only one property problem per union type to speed it up even more.

Lastly, if none of the object literal is assignable to any of the union types, report all problems under one report listing alternative resolution problems.

@fatcerberus I think we should align on how the error should be reported. Otherwise, users might experience a change in tsc interface. Do you have anything in mind? Do you want to propose an error message here? Can you take a look at my Flow example?

---

Mental note:

• Add test cases for Discriminating Unions

[yin]

Most baselines have been unaffected by my changes, except contextualTypeWithUnionTypeMembers.ts, which now fails with errors. Here you have an excerpt from the relevant test code:

interface I1<T> {
  // I1 members...
}
interface I2<T> {
  // I2 members...
}
var i1: I1<number>;
var i2: I2<number>;
var i1Ori2: I1<number> | I2<number> = i1;
var i1Ori2: I1<number> | I2<number> = i2;
var i1Ori2: I1<number> | I2<number> = { // Like i1
  // I1 properties...
};
var i1Ori2: I1<number> | I2<number> = { // Like i2
  // I2 properties...
};
var i1Ori2: I1<number> | I2<number> = { // Like i1 and i2 both
  // Common properties...
  // I1 properties...
  // I2 properties...
};

I would reason, the test case was in the baseline to provide coverage rather than lock down behavior and guarantee it won't change.

I'd remove the now unsupported case and extend the baseline. We will break TypeScript code which was compiled previously. People will be fixing their codebase after upgrading. Therefore we should pre-emptively include their solutions in new baselines.

This brings us to this demonic construction from hell:

var i1Ori2OrEither: I1<number> | I2<number> | (I1<number> & I2<number>) = ...

...

[typescript-bot]

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.