Call hierarchy support #332

July541 · 2021-05-27T04:57:56Z

As part of my GSoC 2021 works, this pr adds call hierarchy supporting according to lsp spec.

jneira · 2021-05-27T05:19:36Z

@July541 great work, i'll review it asap
//cc @michaelpj

michaelpj

LGTM, although I'm never sure all the places that need to be updated with capabilities etc!

michaelpj · 2021-05-27T08:03:16Z

lsp-types/src/Language/LSP/Types/CallHierarchy.hs

+
+data CallHierarchyItem =
+  CallHierarchyItem
+    { _name :: Text


Can we include haddock corresponding to the doc in the LSP spec for each item? Tedious, but matching the spec is a simple policy and more field documentation is almost always better.

-- <https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_prepareCallHierarchy> data CallHierarchyItem = ...

Should it look like this? I have little knowledge about haddock.

lsp-types/src/Language/LSP/Types/CallHierarchy.hs

lsp-test/test/DummyServer.hs

July541 · 2021-05-27T14:46:58Z

although I'm never sure all the places that need to be updated with capabilities etc!

It turns out I updated the capabilities following the compile errors/warnings.

banacorn · 2021-05-27T17:16:01Z

This looks great!
Actually I have a branch with call hierarchy implemented, and I planned to make a PR after #327 is accepted ... It'd be nice it someone can merge it sooner ...

July541 · 2021-05-28T04:56:03Z

@banacorn I am sooo sorry to see about that, I checked the issue list and find no one explicitly intends to do this. To avoid the recurrence of such incidents, I add a comment to make what I'm going to do more public in this repo.

jneira · 2021-05-28T05:45:01Z

Ouch, that is unfortunate indeed, i was not aware of @banacorn plan neither 😟

July541 · 2021-05-28T06:21:23Z

@banacorn I added some missing tests after compared both solutions of you and me, thank you for your inspiration:)

banacorn · 2021-05-28T06:54:46Z

No, it's not your fault !!
I should've explicitly stated my intent and commented at that issue just like what you've done.
No one on earth would've known what I'm brewing in a secluded branch of my fork.

Anyway, my branch doesn't work. I was actually gonna make a PR and ask for help.
I hit some problem like in #314, none of my call hierarchy handlers are responding to the editor.

jneira · 2021-05-28T06:57:56Z

Ok, what do you think about:

@banacorn could review in deep this pr and propose changes (even including code changes that can be commited in the web ui)
As @banacorn's branch already has been useful for this one, we would set @banacorn as co-author in the merge commit

July541 · 2021-05-28T07:07:54Z

As @banacorn's branch already has been useful for this one, we would set @banacorn as co-author in the merge commit

I agree with this.

@banacorn Out of curiosity, what do you mean about no call hierarchy response? I can navigate the call hierarchy on my client(VSCode) without any explicit configuration.

banacorn · 2021-05-28T08:51:35Z

@July541 that means it's just broken, as if no call hierarchy supported has been added.
I think I'll get to know what's wrong with my code after reviewing your PR. 🙂

lukel97

Hi, I'm not too active here anymore but just chiming in here to say this looks great! Thanks for this PR and I hope you have a great summer of code :)

July541 · 2021-05-29T10:40:00Z

It seems #331 has the same confliction problem that failed on Windows with 8.10.4, I don't have any idea about it for the time being:(

jneira · 2021-05-29T11:47:24Z

will try to take a look in my windows laptop

jneira · 2021-05-30T08:09:10Z

it seems to be a dependency solver issue, related with dependent-map and some, maybe we would need to add some constraints in cabal.project similar to the stack.yaml of his itself

jneira · 2021-05-30T21:05:43Z

I've got to build it adding to cabal.project

constraints: some == 1.0.1,
             dependent-sum == 0.7.1.0

max-backjumps: 10000

But i dont like the workaround

banacorn · 2021-05-31T06:57:30Z

lsp-types/src/Language/LSP/Types/Message.hs

+  ResponseResult TextDocumentPrepareCallHierarchy = Maybe (List CallHierarchyItem)
+  ResponseResult CallHierarchyIncomingCalls    = Maybe (List CallHierarchyIncomingCall)
+  ResponseResult CallHierarchyOutgoingCalls    = Maybe (List CallHierarchyOutgoingCall)


I think the Maybe is not necessary

ResponseResult TextDocumentCallHierarchy = List CallHierarchyItem ResponseResult CallHierarchyIncomingCalls = List CallHierarchyIncomingCall ResponseResult CallHierarchyOutgoingCalls = List CallHierarchyOutgoingCall

List should have that null covered

I just keep it to satisfy the spec. And I still notice that we have several dealings about null, for example:

Use Maybe to represent null.

lsp/lsp-types/src/Language/LSP/Types/Message.hs

Line 168 in 3e688d9

ResponseResult TextDocumentHover = Maybe Hover

https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_hover

Ignore null.

lsp/lsp-types/src/Language/LSP/Types/Message.hs

Line 169 in 3e688d9

ResponseResult TextDocumentSignatureHelp = SignatureHelp

https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_signatureHelp

Strictly follow the spec even like me it looks stupid.

lsp/lsp-types/src/Language/LSP/Types/Message.hs

Line 205 in 3e688d9

ResponseResult WorkspaceWorkspaceFolders = Maybe (List WorkspaceFolder)

https://microsoft.github.io/language-server-protocol/specifications/specification-current/#workspace_workspaceFolders

Maybe we need to unify them under one specific rule.

jneira · 2021-05-31T07:27:26Z

But i dont like the workaround

I dont like it but maybe can be applied to make progress until we have a proper solution

jneira · 2021-05-31T20:09:19Z

Merged! many thanks to @July541 and @banacorn too of course, amazing work

anka-213 · 2021-06-05T10:02:45Z

Stupid question, but what is a Call Hierarchy? I couldn't figure it out from the spec, which just seemed to describe the protocol for it, not what the actual thing is.

Edit: Oh, I just saw that the issue this PR is working on fixing explains it: haskell/haskell-language-server#738

July541 added 2 commits May 23, 2021 14:15

Add call hierarchy support

ec01720

Reformat

e95835b

michaelpj approved these changes May 27, 2021

View reviewed changes

July541 added 3 commits May 27, 2021 22:30

Fix typo

a540245

Add test for incoming/outgoing calls

6d165ed

Fix comment

cd501e5

Add missing tests

4998fcc

lukel97 approved these changes May 28, 2021

View reviewed changes

banacorn suggested changes May 31, 2021

View reviewed changes

Try to fix CI on Windows with 8.10.4

ef59c28

banacorn approved these changes May 31, 2021

View reviewed changes

jneira merged commit 907a697 into haskell:master May 31, 2021

michaelpj mentioned this pull request Jun 19, 2021

Semantic tokens support #314

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Call hierarchy support #332

Call hierarchy support #332

July541 commented May 27, 2021

jneira commented May 27, 2021

michaelpj left a comment

michaelpj May 27, 2021

July541 May 27, 2021

July541 commented May 27, 2021

banacorn commented May 27, 2021

July541 commented May 28, 2021

jneira commented May 28, 2021

July541 commented May 28, 2021

banacorn commented May 28, 2021

jneira commented May 28, 2021

July541 commented May 28, 2021

banacorn commented May 28, 2021

lukel97 left a comment

July541 commented May 29, 2021

jneira commented May 29, 2021

jneira commented May 30, 2021 •

edited

Loading

jneira commented May 30, 2021 •

edited

Loading

banacorn May 31, 2021

July541 May 31, 2021 •

edited

Loading

jneira commented May 31, 2021

jneira commented May 31, 2021

anka-213 commented Jun 5, 2021 •

edited

Loading

Call hierarchy support #332

Call hierarchy support #332

Conversation

July541 commented May 27, 2021

jneira commented May 27, 2021

michaelpj left a comment

Choose a reason for hiding this comment

michaelpj May 27, 2021

Choose a reason for hiding this comment

July541 May 27, 2021

Choose a reason for hiding this comment

July541 commented May 27, 2021

banacorn commented May 27, 2021

July541 commented May 28, 2021

jneira commented May 28, 2021

July541 commented May 28, 2021

banacorn commented May 28, 2021

jneira commented May 28, 2021

July541 commented May 28, 2021

banacorn commented May 28, 2021

lukel97 left a comment

Choose a reason for hiding this comment

July541 commented May 29, 2021

jneira commented May 29, 2021

jneira commented May 30, 2021 • edited Loading

jneira commented May 30, 2021 • edited Loading

banacorn May 31, 2021

Choose a reason for hiding this comment

July541 May 31, 2021 • edited Loading

Choose a reason for hiding this comment

jneira commented May 31, 2021

jneira commented May 31, 2021

anka-213 commented Jun 5, 2021 • edited Loading

jneira commented May 30, 2021 •

edited

Loading

jneira commented May 30, 2021 •

edited

Loading

July541 May 31, 2021 •

edited

Loading

anka-213 commented Jun 5, 2021 •

edited

Loading