Walk all outer expression kinds before comitting to the return type of a symbol call

[Andarist]

fixes #61070

[jakebailey]

Doesn't walkUpOuterExpressions do things other than satisfies and parens?

[Andarist]

Doesn't walkUpOuterExpressions do things other than satisfies and parens?

Yes, I could control what it is supposed to handle with a bitmask but I decided that there is no harm to just use the default (All). Cause at the end of the day - what harm can be caused by it? One of the changes would be a changed error like:

-const bar = Symbol() as string; // Conversion of type 'symbol' to type 'string' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.(2352)
+const bar = Symbol() as string; // Conversion of type 'typeof bar' to type 'string' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.(2352)

[RyanCavanaugh]

@typescript-bot pack this

[typescript-bot]

Starting jobs; this comment will be updated as builds start and complete.

Command	Status	Results
pack this	✅ Started	✅ Results

[typescript-bot]

Hey @RyanCavanaugh, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/164676/artifacts?artifactName=tgz&fileId=6AC0B4FA6225C679120D6D095ADE93E18CE2BB508C80F6B2148B4273E613A69102&fileName=/typescript-5.8.0-insiders.20250129.tgz"
    }
}

and then running npm install.

---

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.8.0-pr-61071-4".;

[jakebailey]

Yes, I could control what it is supposed to handle with a bitmask but I decided that there is no harm to just use the default (All). Cause at the end of the day - what harm can be caused by it? One of the changes would be a changed error like:

Is there a test for this? I don't see it in the PR.

[Andarist]

@jakebailey there is one now 😉

[jakebailey]

Yeah, this I'm not satisfied with (pun intended).

The complaint is about Symbol() as string, but the fact that it's in the context of a const declaration, its type now changes. It seems very odd to me that if I were to write something like (() => Symbol() as string)() that the message would be different.

I feel less weird about this error in the context of satisfies, maybe, but even that I think is strange to be talking about the actual declaration name....

[Andarist]

Ok, I changed it to walkUpOuterExpressions(node, OuterExpressionKinds.Parentheses | OuterExpressionKinds.Satisfies) now

[jakebailey]

Thanks; can you also update the test with the other outer expression kinds, just to illustrate the differences? (Mainly just parens, satisfies, maybe !).

[Andarist]

It seems very odd to me that if I were to write something like (() => Symbol() as string)() that the message would be different.

The type is already different between those 2 though:

const sym1 = Symbol();
const sym2 = (() => Symbol())();

I see your point though and I don't mind either solution ;p

[jakebailey]

The types are different, absolutely, I just view const foo = Symbol() to be a very specific syntactic construct, which this PR is saying should be extended to satisfies.

That being said, #61070 is closed as "not a defect", so I'm not sure if we're actually going to accept this?

[Andarist]

That being said, #61070 is closed as "not a defect", so I'm not sure if we're actually going to accept this?

That's fine. I opened this PR before that :P Feel free to close it. It's obviously not a real-world problem, but I sympathized with the OP's view that this is surprising in the context of the satisfies role. OTOH, Symbol() is just special so 🤷