getSymbolAtLocation now supports class expressions #51049

sandersn · 2022-10-03T22:54:21Z

Discovered while writing #51016; after that merges I'll update this PR to remove its special-case code.

Many baselines change because our symbol baselines apparently use getSymbolAtLocation. I believe that class expressions will also show up better in quick info, although I'm not sure. This is a big change, but I think it's good.
Some special-case code in find-all-refs is no longer relevant. (and more from Fix crash in goto-def on @override #51016 will be when that merges.)
Using the real symbol gives names to more class expressions, so I made the expression more complex to keep the class truly anonymous in
goToImplementationInterface_07. The quick info display also changes, from

(anonymous local class)

to

(local class) (Anonymous class)

This is more consistent with named class expressions ((local class) C), so I think it's a good change.

And accept baselines for later inspection. One fourslash test changes, indicating that quick info relied on getTypeAtLocation not returning anything for class expressions to distinguish named from anonymous class expressions. This predicate will have to be improved.

1. Some special-case code in find-all-refs is no longer relevant. 2. Using the real symbol gives names to more class expressions, so I made the expression more complex to keep the class truly anonymous in goToImplementationInterface_07. The quick info display also changes, from ``` (anonymous local class) ``` to ``` (local class) (Anonymous class) ``` This is more consistent internally and externally, so I think it's a good change.

sandersn · 2022-10-03T23:00:48Z

@amcasey and @andrewbranch I think you know the most about how this will affect language service results.

tests/cases/fourslash/goToImplementationInterface_07.ts

Only display `(local class)` if the class expression is named and has no alias. Other named classes use the normal `class C` format; anonymous classes display `(Anonymous class)`.

amcasey · 2022-10-05T17:36:26Z

@typescript-bot test tsserver top100
@typescript-bot user test tsserver

typescript-bot · 2022-10-05T17:36:30Z

Heya @amcasey, I've started to run the diff-based user code test suite (tsserver) on this PR at aaa3240. You can monitor the build here.

Update: The results are in!

typescript-bot · 2022-10-05T17:36:30Z

Heya @amcasey, I've started to run the diff-based top-repos suite (tsserver) on this PR at aaa3240. You can monitor the build here.

Update: The results are in!

typescript-bot · 2022-10-05T17:47:18Z

@amcasey Here are the results of running the user test suite comparing main and refs/pull/51049/merge:

Everything looks good!

amcasey · 2022-10-05T17:58:10Z

So, I have basically no idea what impact this will have but, superficially, it looks completely out of place in the list of pre-existing syntax kind checks. If it was always an oversight (doesn't really look like it?), why wouldn't we do the same for function expressions?

typescript-bot · 2022-10-05T19:45:51Z

@amcasey Here are the results of running the top-repos suite comparing main and refs/pull/51049/merge:

Something interesting changed - please have a look.

Details

felixrieseberg/windows95

⚠️ Note that built also had errors ⚠️

Req #432 - references
    at formatMessage (/typescript-main/built/local/tsserver.js:178948:29)
    at IOSession.Session.writeMessage (/typescript-main/built/local/tsserver.js:180023:31)
    at IOSession.Session.send (/typescript-main/built/local/tsserver.js:180020:22)
    at IOSession.Session.doOutput (/typescript-main/built/local/tsserver.js:180073:22)
    at IOSession.Session.onMessage (/typescript-main/built/local/tsserver.js:181718:30)
    at Interface.<anonymous> (/typescript-main/built/local/tsserver.js:185825:31)

Req #432 - references
    at formatMessage (/typescript-51049/built/local/tsserver.js:178949:29)
    at IOSession.Session.writeMessage (/typescript-51049/built/local/tsserver.js:180024:31)
    at IOSession.Session.send (/typescript-51049/built/local/tsserver.js:180021:22)
    at IOSession.Session.doOutput (/typescript-51049/built/local/tsserver.js:180074:22)
    at IOSession.Session.onMessage (/typescript-51049/built/local/tsserver.js:181719:30)
    at Interface.<anonymous> (/typescript-51049/built/local/tsserver.js:185826:31)

Last few requests

{"seq":429,"type":"request","command":"completionInfo","arguments":{"file":"@PROJECT_ROOT@/src/renderer/lib/_libwabt.js","line":13,"offset":76180,"includeExternalModuleExports":false,"includeInsertTextCompletions":true,"triggerKind":1}}
{"seq":430,"type":"request","command":"completionEntryDetails","arguments":{"file":"@PROJECT_ROOT@/src/renderer/lib/_libwabt.js","line":13,"offset":76180,"entryNames":["_"]}}
{"seq":431,"type":"request","command":"definitionAndBoundSpan","arguments":{"file":"@PROJECT_ROOT@/src/renderer/lib/_libwabt.js","line":13,"offset":77944}}
{"seq":432,"type":"request","command":"references","arguments":{"file":"@PROJECT_ROOT@/src/renderer/lib/_libwabt.js","line":13,"offset":78089}}

Repro Steps

git clone https://github.com/felixrieseberg/windows95 --recurse-submodules
In dir windows95, run git reset --hard c483871df905ac8889090d03afeae4f864d2c313
In dir windows95, run yarn install --ignore-engines --ignore-scripts --silent
Back in the initial folder, download RepoResults4/felixrieseberg.windows95.replay.txt from the artifact folder
npm install --no-save @typescript/server-replay
npx tsreplay ./windows95 ./felixrieseberg.windows95.replay.txt path/to/tsserver.js
npx tsreplay --help to learn about helpful switches for debugging, logging, etc

palantir/blueprint

⚠️ Note that built also had errors ⚠️

Req #12285 - references
    at formatMessage (/typescript-main/built/local/tsserver.js:178948:29)
    at IOSession.Session.writeMessage (/typescript-main/built/local/tsserver.js:180023:31)
    at IOSession.Session.send (/typescript-main/built/local/tsserver.js:180020:22)
    at IOSession.Session.doOutput (/typescript-main/built/local/tsserver.js:180073:22)
    at IOSession.Session.onMessage (/typescript-main/built/local/tsserver.js:181718:30)
    at Interface.<anonymous> (/typescript-main/built/local/tsserver.js:185825:31)

Req #12285 - references
    at formatMessage (/typescript-51049/built/local/tsserver.js:178949:29)
    at IOSession.Session.writeMessage (/typescript-51049/built/local/tsserver.js:180024:31)
    at IOSession.Session.send (/typescript-51049/built/local/tsserver.js:180021:22)
    at IOSession.Session.doOutput (/typescript-51049/built/local/tsserver.js:180074:22)
    at IOSession.Session.onMessage (/typescript-51049/built/local/tsserver.js:181719:30)
    at Interface.<anonymous> (/typescript-51049/built/local/tsserver.js:185826:31)

Last few requests

{"seq":12282,"type":"request","command":"completionInfo","arguments":{"file":"@PROJECT_ROOT@/site/docs/versions/1/docs-app.js","line":1,"offset":403366,"includeExternalModuleExports":false,"includeInsertTextCompletions":true,"triggerKind":2,"triggerCharacter":"."}}
{"seq":12283,"type":"request","command":"definitionAndBoundSpan","arguments":{"file":"@PROJECT_ROOT@/site/docs/versions/1/docs-app.js","line":1,"offset":404323}}
{"seq":12284,"type":"request","command":"definitionAndBoundSpan","arguments":{"file":"@PROJECT_ROOT@/site/docs/versions/1/docs-app.js","line":1,"offset":405053}}
{"seq":12285,"type":"request","command":"references","arguments":{"file":"@PROJECT_ROOT@/site/docs/versions/1/docs-app.js","line":1,"offset":405913}}

Repro Steps

git clone https://github.com/palantir/blueprint --recurse-submodules
In dir blueprint, run git reset --hard 5b06fd3e9740675869c043ee8505c4927afe6e72
In dir blueprint, run yarn install --ignore-engines --ignore-scripts --silent
Back in the initial folder, download RepoResults4/palantir.blueprint.replay.txt from the artifact folder
npm install --no-save @typescript/server-replay
npx tsreplay ./blueprint ./palantir.blueprint.replay.txt path/to/tsserver.js
npx tsreplay --help to learn about helpful switches for debugging, logging, etc

amcasey · 2022-10-05T20:00:59Z

@sandersn The new server errors aren't related or interesting.

sandersn · 2022-10-07T00:24:32Z

it looks completely out of place in the list of pre-existing syntax kind checks

From discussion on teams, getSymbolAtLocation currently only handles identifiers and some other keywords -- all single tokens. It seems likely that some other entrypoint handles symbols of other things like expressions, and that that's the right place for class expressions.

andrewbranch · 2022-10-13T18:28:17Z

I’d be interested to read the discussion on Teams I missed, because my assumption would have been that this:

getSymbolAtLocation currently only handles identifiers and some other keywords -- all single tokens

is just an artifact of its usage in the services for things like quick info, where the cursor is necessarily confined to a single token. As an API entrypoint, I always understood its failure on higher nodes to be undesirable.

amcasey · 2022-10-19T16:30:14Z

@andrewbranch We just spoke 1:1. Neither of us had any context on what the function was supposed to do, but I observed that all the existing checks were for tokens, suggesting that expressions were handled elsewhere (or, alternatively, that several kinds of expressions needed to be supported).

sandersn · 2022-10-19T16:54:09Z

Overall, the benefit of getting rid of a couple of special cases in the services vs. changing the way getSymbolAtLocation works isn't justified. I'm going to close this for now.

sandersn · 2022-10-19T16:54:10Z

Overall, the benefit of getting rid of a couple of special cases in the services vs. changing the way getSymbolAtLocation works isn't justified. I'm going to close this for now.

sandersn added 2 commits October 3, 2022 09:08

typescript-bot assigned sandersn Oct 3, 2022

typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Oct 3, 2022

sandersn requested review from amcasey and andrewbranch October 3, 2022 23:00

DanielRosenwasser reviewed Oct 3, 2022

View reviewed changes

tests/cases/fourslash/goToImplementationInterface_07.ts Outdated Show resolved Hide resolved

sandersn added 4 commits October 3, 2022 16:08

Merge branch 'main' into getTypeAtLocation-support-class-expr

44c86a3

getDefinitionFromOverriddenMember: remove class-expr special case

2940e08

Merge branch 'main' into getTypeAtLocation-support-class-expr

71c990e

Improve class expression display

aaa3240

Only display `(local class)` if the class expression is named and has no alias. Other named classes use the normal `class C` format; anonymous classes display `(Anonymous class)`.

sandersn added this to Not started in PR Backlog Oct 18, 2022

sandersn closed this Oct 19, 2022

PR Backlog automation moved this from Not started to Done Oct 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

getSymbolAtLocation now supports class expressions #51049

getSymbolAtLocation now supports class expressions #51049

sandersn commented Oct 3, 2022

sandersn commented Oct 3, 2022

amcasey commented Oct 5, 2022

typescript-bot commented Oct 5, 2022 •

edited

typescript-bot commented Oct 5, 2022 •

edited

typescript-bot commented Oct 5, 2022

amcasey commented Oct 5, 2022

typescript-bot commented Oct 5, 2022

felixrieseberg/windows95

Last few requests

Repro Steps

palantir/blueprint

Last few requests

Repro Steps

amcasey commented Oct 5, 2022

sandersn commented Oct 7, 2022 •

edited

andrewbranch commented Oct 13, 2022

amcasey commented Oct 19, 2022

sandersn commented Oct 19, 2022

sandersn commented Oct 19, 2022

getSymbolAtLocation now supports class expressions #51049

getSymbolAtLocation now supports class expressions #51049

Conversation

sandersn commented Oct 3, 2022

sandersn commented Oct 3, 2022

amcasey commented Oct 5, 2022

typescript-bot commented Oct 5, 2022 • edited

typescript-bot commented Oct 5, 2022 • edited

typescript-bot commented Oct 5, 2022

amcasey commented Oct 5, 2022

typescript-bot commented Oct 5, 2022

felixrieseberg/windows95

Last few requests

Repro Steps

palantir/blueprint

Last few requests

Repro Steps

amcasey commented Oct 5, 2022

sandersn commented Oct 7, 2022 • edited

andrewbranch commented Oct 13, 2022

amcasey commented Oct 19, 2022

sandersn commented Oct 19, 2022

sandersn commented Oct 19, 2022

typescript-bot commented Oct 5, 2022 •

edited

typescript-bot commented Oct 5, 2022 •

edited

sandersn commented Oct 7, 2022 •

edited