Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Implement signature help #4626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jian-lin merged 47 commits into haskell:master from linj-fork:pr/signature-help
Sep 6, 2025
Merged

Conversation

Copy link
Collaborator

@jian-lin jian-lin commented Jun 8, 2025
edited
Loading

Introduction

Haskell language server (HLS) implements the server part of Language Server Protocol (LSP) to improve user experience of reading and writing Haskell. Signature help is a LSP language feature which shows function signatures and documentation when the cursor is inside a function application. It can also highlight part of the signature related to the current parameter the cursor is at. This is a useful feature and users want it.

This PR aims to implement the signature help feature for HLS.

My work

I have implemented most parts of the signature help feature for HLS in this PR. The last commit during GSoC is d0200a4.

The signature help feature works well in most cases, especially when a user is reading code.

Click for a video demo
hls-signature-help-gsoc-demo.webm

When I worked on this PR, a bug of other parts of HLS was found and I created a PR to fix it.

Besides HLS proper, I also created 2 PRs for lsp-test to add a needed function for tests.

Click for more detailed work

2025年06月02日 - 2025年06月08日

  • Add basic boilerplate for signature help plugin

2025年06月09日 - 2025年07月13日

  • Finish signature help plugin MVP: show function signature and highlight the current parameter
    • commit: 9168b74

    • click for video demo
      Screencast.From.2025年07月13日.02-11-44.webm

2025年07月14日 - 2025年07月20日

  • Add basic tests. Some of them are not passed for now.

2025年07月21日 - 2025年07月27日

  • Change expected test results considering the cursor shape (a6635ca)
  • Replace maybe with case for better readability (bf0b4d5)
  • Call extractInfoFromSmallestContainingFunctionApplicationAst once (c95d6e4)

2025年07月28日 - 2025年08月03日

  • Show more types: each type as one signature help (d603ec4)
  • Add tests for kind signatures and higher-order function (dca1311) (471958f)

2025年08月04日 - 2025年08月10日

2025年08月11日 - 2025年08月17日

  • Show function documentation in signature help (a522e88)

  • Show function argument documentation in signature help (d826d06)

  • Do not error if doc is not available (35399e7)

  • click for screenshots image image

2025年08月18日 - 2025年08月24日

  • Upstream getSignatureHelp to lsp-test: Add signature help request to lsp-test lsp#621
  • Make signature helps reproducible in tests (eeeb283)
  • Do not show uris in the argument documentation (f1d19ba)
  • Add tests
    • imported function with argument doc (433a8ad)
    • imported function with no doc (d3d7d12)
  • Remember previous active signature (e6702b7)
  • Show signature help even when there are type applications (800f908)
  • Fix tests for ghc > 9.8
  • Add myself as codeowner of hls-signature-help-plugin (b6d4617)
  • Add hls-signature-help-plugin to some documentation files (9a1de4e)

2025年08月25日 - 2025年08月31日

2025年09月01日 - 2025年09月02日

  • Use record syntax for data with many fields (b5c13a3)

Future work

  • Fix a known bug of wrong highlight in the signature in some cases.
  • Show signature and highlight the related part of the next parameter when a user is writing code.
  • Highlight the type variable of forall when the cursor is at a type application such as @Int.
  • Support OpApp (operator application) of HsApp such as f x $ y|.
  • Trigger a signature help request after a space char is typed.

Acknowledgements

This is a Google Summer of Code (GSoC) project. Thanks to all the people involved. Special thanks to my mentors, @michaelpj and @fendor, for their help.


Closes #3598

Related to #2348 since we make mkDocMap expose the argument doc map

fishtreesugar and sidkshatriya reacted with hooray emoji
TODO:
- handle more cases
- add successful and (currently failed) tests
- show documentation
Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think really worth trying to start getting some tests in place!

jian-lin reacted with thumbs up emoji
jian-lin added 9 commits July 25, 2025 15:45
This improves performance. It also improves correctness because
functionName, functionType and argumentNumber are extracted from the
same AST.
Vscode and Emacs(eglot) seems to treat newline as a normal char.
See comment for a comparison with alternative methods.

This comment was marked as outdated.

Comment on lines 122 to 161
findArgumentRanges :: Type -> [(UInt, UInt)]
findArgumentRanges functionType =
let functionTypeString = printOutputableOneLine functionType
functionTypeStringLength = fromIntegral $ T.length functionTypeString
splitFunctionTypes = filter notTypeConstraint $ splitFunTysIgnoringForAll functionType
splitFunctionTypeStrings = printOutputableOneLine . fst <$> splitFunctionTypes
-- reverse to avoid matching "a" of "forall a" in "forall a. a -> a"
reversedRanges =
drop 1 $ -- do not need the range of the result (last) type
findArgumentStringRanges
0
(T.reverse functionTypeString)
(T.reverse <$> reverse splitFunctionTypeStrings)
in reverse $ modifyRange functionTypeStringLength <$> reversedRanges
where
modifyRange functionTypeStringLength (start, end) =
(functionTypeStringLength - end, functionTypeStringLength - start)

{-
The implemented method uses both structured type and unstructured type string.
It provides good enough results and is easier to implement than alternative
method 1 or 2.
Alternative method 1: use only structured type
This method is hard to implement because we need to duplicate some logic of 'ppr' for 'Type'.
Some tricky cases are as follows:
- 'Eq a => Num b -> c' is shown as '(Eq a, Numb) => c'
- 'forall' can appear anywhere in a type when RankNTypes is enabled
f :: forall a. Maybe a -> forall b. (a, b) -> b
- '=>' can appear anywhere in a type
g :: forall a b. Eq a => a -> Num b => b -> b
- ppr the first argument type of '(a -> b) -> a -> b' is 'a -> b' (no parentheses)
- 'forall' is not always shown
Alternative method 2: use only unstructured type string
This method is hard to implement because we need to parse the type string.
Some tricky cases are as follows:
- h :: forall a (m :: Type -> Type). Monad m => a -> m a
-}
findArgumentStringRanges :: UInt -> Text -> [Text] -> [(UInt, UInt)]
Copy link
Collaborator Author

@jian-lin jian-lin Aug 7, 2025
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context:

@michaelpj

I implemented a mixed way which uses both structured type and type string. See the comment for the reason of doing it this way instead of using only structured type or only type string.

It seems to work pretty well. "3 out of 87 tests failed".


One failed test case is f :: Integer -> Num Integer => Integer -> Integer. We matched Num Integer as the first argument. This probably can be fixed by using regex.

Another similar failed test case is f :: forall l. l -> forall a. a -> a. When we should highlight the argument l, we highlight the l for the second forall.


Here is another failed test case.

f :: a -> forall a. a -> a
f = _

The printed type string for f is f :: forall a. a -> forall a1. a1 -> a1. This seems tricky to fix in the current implementation because it needs us to duplicate the renaming logic (the second forall a becomes forall a1) of ppr.

This case only happens when RankNTypes is used so I do not think it is very common.

Copy link
Collaborator

@michaelpj michaelpj Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering how bad it would be to just use the structured type and re-implement some pretty-printing logic. I think the only bit we'd need to duplicate would be the printing of ->, =>, and forall? Maybe that's okay?

Copy link
Collaborator

Another thing we should think about: type arguments.

id :: forall a . a -> a
id x = x
id @...
 ^^ --- we probably want to consider the "a" a parameter so we can highlight it here when the user is providing a type
-- with RequiredTypeArguments
id :: forall a -> a -> a
id _ x = x
id 
 ^^ -- here again we need to highlight the forall

Type arguments are kind of annoying because they might be optional (with normal foralls), or they might be required (with required type arguments).

jian-lin reacted with thumbs up emoji jian-lin reacted with eyes emoji

Copy link
Collaborator Author

@jian-lin jian-lin Aug 25, 2025
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: different hightlighting strategies for "reading code" and "writing code".

By "writing code", I mean the cursor is at the end of the function application and we intend to write more args to that function application.

Other cases are "reading code".


Currently, the highlight strategy for "reading code" works well.

For example, f x | y will highlight y. Note that in this case the cursor | is in the subtree of a HsApp AST node.


Currently, the highlight strategy for "writing code" is problematic.

Case 1: only the function itself is written f |. We want to highlight the first arg of f. Note that in this case there is no HsApp node and so the cursor | is not in the subtree of any HsApp AST node.

Case 2: the function itself and at least 1 arg have been written f x | (f :: a -> b -> c). We want to highlight the second arg of f. Note that in this case there are HsApp nodes but the cursor | is not in the subtree of any HsApp AST node.

Copy link
Collaborator

@michaelpj michaelpj Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds to me like your two strategies are the same? In both cases the strategy is "highlight the next argument after the cursor position (which may not exist)"?

Presumably in this situation we will highlight the second argument, though:

f one t|wo

jian-lin reacted with thumbs up emoji
Follows @fendor's suggestion to ignore some details in the doc string.
Newlines generated by neat-interpolation are sensitive[1] to
platforms, which may make many tests of hls-cabal-plugin-tests fail on
Windows.
To keep this PR simple and focused, neat-interpolation usage outside
of the signature help plugin is not changed. I plan to change that in
a future PR.
[1]: nikita-volkov/neat-interpolation#14 
Copy link
Collaborator Author

jian-lin commented Aug 31, 2025
edited
Loading

I think this PR is in a good shape now. All tests have passed. Please review.

@jian-lin jian-lin marked this pull request as ready for review August 31, 2025 05:18
Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Could maybe do with a few more comments but I think we should merge this and make some issues for later improvements.

@fendor fendor added the status: needs review This PR is ready for review label Sep 3, 2025
Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let's get this merged for the next HLS release :)

@jian-lin jian-lin merged commit 9b952c8 into haskell:master Sep 6, 2025
71 of 78 checks passed
@jian-lin jian-lin deleted the pr/signature-help branch September 6, 2025 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fendor fendor fendor approved these changes

@wz1000 wz1000 Awaiting requested review from wz1000 wz1000 is a code owner

@michaelpj michaelpj Awaiting requested review from michaelpj michaelpj is a code owner

Assignees
No one assigned
Labels
status: needs review This PR is ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Show function signature while providing the arguments

AltStyle によって変換されたページ (->オリジナル) /