Remove extra checkDefined in visitEachChildOfJsxExpression

[jakebailey]

Fixes #52479

[jakebailey]

Checked all of the other checkDefineds in this section; this was the only wrong one. I'll have to look into something we can do to checkDefined that makes it fail if you give it something that's definitely defined.

EDIT: No, that's the opposite of the problem; we would need something which checks whether or not a result of a checkDefined is used only in a place which already accepts undefined. That's not really easily doable.

(I was able to find one other place that calls checkDefined on a definitely-defined value, but that is harmless and not what I really want.)

[DanielRosenwasser]

we would need something which checks whether or not a result of a checkDefined is used only in a place which already accepts undefined.

Would this fit the bill?

export function checkDefined<T, U extends T | null | undefined>(
    value: T | null | undefined,
    message?: string,
    stackCrawlMark?: AnyFunction
): U extends NonNullable<U> ? U : unknown {
    assertIsDefined(value, message, stackCrawlMark || checkDefined);
    return (value satisfies NonNullable<T> as any);
}

[DanielRosenwasser]

FWIW, if I do that, here are the errors I see in visitorPublic.ts:

src/compiler/visitorPublic.ts:655:13 - error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'string | BindingName'.

655             Debug.checkDefined(nodeVisitor(node.name, visitor, isBindingName)),
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/compiler/visitorPublic.ts:678:13 - error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'string | PropertyName'.

678             Debug.checkDefined(nodeVisitor(node.name, visitor, isPropertyName)),
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/compiler/visitorPublic.ts:1055:13 - error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'BinaryOperator | BinaryOperatorToken'.

1055             tokenVisitor ? Debug.checkDefined(nodeVisitor(node.operatorToken, tokenVisitor, isBinaryOperatorToken)) : node.operatorToken,
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/compiler/visitorPublic.ts:1457:13 - error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'Expression | undefined'.

1457             Debug.checkDefined(nodeVisitor(node.expression, visitor, isExpression)));
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[jakebailey]

The first three appear to be cases where our factories accept string or SyntaxKind for convenience (but can't actually appear during visit), so there's something weird happening with that signature, I think.

The last one is this bug, so it did catch that at least. But I'm guessing if there were more, it would have caught them too (with the false positives present).

[jakebailey]

Another way to do this would be to transform all Debug.checkDefined calls to ! and then see if eslint complains, but that is of course a bit more work.

[jakebailey]

I wrote the above transform and then ran eslint; this is in fact the only place we use checkDefined on a definitely defined value. Good to know that we don't have any more of these kinds of potential bugs.