GeezerVison

I think this should be trueness -- it doesn't look like this test was intended to fail. Another 🏆 for this change's effectiveness.

https://stackoverflow.com/questions/50143250/how-excess-property-check-helps

Please define fresh object literal in the comment.

Why do we need a new flag here? This is set in the same place as ObjectLiteral. And when you get the regular type, you turn off FreshObjectLiteral, but not ObjectLiteral, and I don't really understand why. So a regular type is allowed to have excess properties, but it still does not need to have all the optional properties of the target in a subtype check?

Well, it's related to the issue of removing freshness after the first assignment, but still remembering that the source was an object literal. It may be that we can combine the two if we give up on the first assignment bit.

Yup, I think we should give up the first assignment bit. I think it is a strange rule.

So, I don't think we can get rid of the new flag. We use TypeFlag.ObjectLiteral to indicate that a type originated in an object literal and may contain null or undefined types. We can't turn off that flag unless we also widen the type. Yet, in type assertions and during subtype reduction we want to turn off freshness but not widen.

That said, we could still drop the assignment rule. They're really orthogonal issues.

Yes, let's drop the assignment rule.

I understand what you mean about TypeFlags.ObjectLiteral, but I still think we can use it. We actually use ContainsUndefinedOrNull and ContainsObjectLiteral for what you are talking about, not ObjectLiteral directly. For type assertions, I think it's fine to widen, we already do in the downcast direction, and I think it's fine to do it for the upcast (it uses assignability so we should be fine). For creation of a union type, we already give it TypeFlags.Union plus all the widening flags (which do not include ObjectLiteral anyway). So all the flags that getWidenedType checks for would still be there.

So I think it is safe to remove it.

To elaborate about the type assertion case, widening would essentially do two things:

1. null and undefined become any. They are bottom types anyway, so changing them to any should have no effect.
2. The object literal is changed with respect to the optional-properties-being-required rule. This rule only applies to subtype, and type assertions use assignability.

I admit it's still possible that I missed something, but if I did, I'd like to understand it before allowing us to have two flags that sound really similar and are easy to confuse.

Ah, so you do these checks on the target side as opposed to when contextually typing the object literal. Actually, this is probably a good thing, because not every assignability check is guaranteed to be accompanied by contextual typing. So if the object did not get contextually typed, the assignment would still be allowed, which is good I think.

But I would add some comments explaining the rationale behind these heuristics.

Thanks. This makes sense now