Don't emit enum members as evaluated Infinity/NaN when their symbols are shadowed

[Andarist]

fixes #55091

[Andarist]

alternatively, I could just not use the constant value in those cases and fallback to the other branch - that would likely work. But I feel that NaN and Infinity should not be passed to factory.createNumericLiteral so some changes to this branch of code here would have to be done anyway. I also like that those evaluated, yet shadowed, Infinity and NaN are normalized~ by the proposed changes.

[fatcerberus]

Just for completeness’ sake, how does this interact with const enum?

[Andarist]

Just for completeness’ sake, how does this interact with const enum?

const enum members have to be finite. TS still inlines Infinity and NaN in the emit's output despite the fact that it errors on them. This won't fix it in those instances but since the input code is considered to be invalid, it's likely OK to ignore this bug in those emits.

[DanielRosenwasser]

What do you think of this alternative way to write it?

if (!isFinite(value) {
    let result: Node = resolver.isNameReferencingGlobalValueAtLocation("Infinity", member) ?
        factory.createIdentifier("Infinity") :
        factory.createBinaryExpression(factory.createNumericLiteral(0), SyntaxKind.SlashToken, factory.createNumericLiteral(0));
    if (value < 0) {
        result = factory.createPrefixUnaryExpression(SyntaxKind.MinusToken, result);
    }
    return result;
}

[DanielRosenwasser]

I guess that would emit as -(1/0)?

[Andarist]

What do you think of this alternative way to write it?

That's what I started with :p
ed2cc57

I guess that would emit as -(1/0)?

and why I changed it to the current version 😉

[sandersn]

Are there any more convincing global names that could get shadowed than Infinity and NaN? It's a stretch for people to create local names Infinity or NaN.

[Andarist]

I think those are the only 2 that can be produced by the „constant evaluation”

It's a stretch for people to create local names Infinity or NaN.

cant disagree :p the issue was labeled as a bug and not a wontfix, it’s similar to the merged: #55018