Fix crash from missing valueDeclaration on intersection property

[andrewbranch]

Fixes #37015

The error was occurring when trying to serialize a JS symbol into a class declaration, because one of its inherited property symbols was missing a valueDeclaration, which was the target of a Debug.assertIsDefined.

When two object types were unioned/intersectioned, the order of types was having an observable effect on whether the resulting property symbol had a valueDeclaration set. If the first type’s property didn’t have a valueDeclaration but a later type’s property did, we’d take the later one. But in the inverse case, we’d say that there was a non-uniform value declaration so we wouldn’t attach one at all.

Fixing this fixed the real-world repro at #37015 and improved one types baseline, but my minimal repro was still failing because the intersection property for toString had two different defined declarations, which seemed like a legitimate violation of the assertion’s assumption, so I removed the assertion. My understanding of this code is that inherited properties could all be filtered out, and if valueDeclaration is undefined, the property must be inherited.

[sandersn]

We still want to serialise properties that come from an intersection but only have one source property, right? If I understand correctly, { a } & { b } should always produce a and b -- whereas { a: number } & { a: string } is the dual-defined case, and is OK to skip a. Is that correct? Is that what this PR does?

[sandersn]

I thought there was a reliable way to differentiate between properties that are just passed through an intersection vs those that combine two source properties. It might be having a defined containingType, but I thought there was something else.

Combined, these two statements:

But in the inverse case, we’d say that there was a non-uniform value declaration so we wouldn’t attach one at all.

my minimal repro was still failing because the intersection property for toString had two different defined declarations

make me think that the rules for setting valueDeclaration on intersection properties should change first and then the serialisation code can change if there's still a need.

[sandersn]

Let's talk about the possibility of changing the rules for setting valueDeclaration in createUnionOrIntersectionProperty.

[andrewbranch]

We still want to serialise properties that come from an intersection but only have one source property, right? If I understand correctly, { a } & { b } should always produce a and b -- whereas { a: number } & { a: string } is the dual-defined case, and is OK to skip a. Is that correct? Is that what this PR does?

Yep, that’s what this PR does. But in my test case, the inherited toString property still came from an intersection of Object and CoreObject, each with their own toString declaration, so the serialization had to change too.

[andrewbranch]

I should also be more clear that I think my change to createUnionOrIntersectionProperty was just fixing a mistake, not changing the rules. It was dependent on the order of types in the union/intersection, which seems like it must have been unintentional.