Error: Property is overwritten by later spread

[sandersn]

Move the checking from JSX to getSpreadType so that normal objects get the error too.

Also reword the missing semicolon related error to be less silly.

Fixes #27355 (probably)

[sandersn]

Note that the related span didn't work out because getSpreadType doesn't have easy access to the usage of properties, just their declarations. For spread properties, the declaration is a really bad site for a related span. I decided that the additional complexity of determining the usage site wasn't worth it.

[RyanCavanaugh]

Error message is inaccurate here

[weswigham]

..."later spread"? Methinks this needs a different error message, since it's not being overridden by a spread, but by the actual children tags in the AST.

[weswigham]

I don't think it's safe to emit this error outside of strictNullChecks in normal circumstances (JSX children being one of the few where it is).

[weswigham]

For spread properties, the declaration is a really bad site for a related span. I decided that the additional complexity of determining the usage site wasn't worth it.

We should have a related span that calls out which following spread (just in case there are multiple) is the one which overrides the property.

[ghost]

False positive:

function f(obj: { x: number } | undefined): { x: number } {
    return { x: 1, ...obj };
}

(mentioned by @ajafff in #23340)

[sandersn]

@andy-ms Actually, the related example that you gave in that issue also fails, with obj: { x: number | undefined }. I have a commit that fixes these two problems, but it does so by not issuing the error for any union-distributed spread (fixes the first example) and not issuing the error for any nullable-containing property type (fixes the second example). It also reverts the JsX test changes, although that issue isn’t really fixed.

Basically, this fix is more complicated than I hoped, so I’m going to put it on hold until I address more urgent 3.2work.

[matthargett]

3.2 has been out for a while -- any updates on this fix, or one that supersedes it? :D if there were complications, can they be described in a comment so we can understand better?

[sandersn]

#36727 is the revived version of this, which is now merged.