Skip to content
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

Inlay hints: traverse ghost locations #1892

Closed
wants to merge 4 commits into from

Conversation

mlasson
Copy link
Contributor

@mlasson mlasson commented Feb 4, 2025

Description

Consider the following code:

let f = fun x -> x ^ ""
let g _ = fun x -> x ^ ""

In this case, an inlay hint appears for x in f, but no hint is shown for g.

In VS Code, this results in the following display:
image

Issue

The inlay hints implementation includes logic to stop traversing "ghost locations" to avoid annotating generated code. However, a simple definition like:

let f x = ...

is syntactic sugar for:

let f = fun x -> ...

where fun x -> introduces a ghost location. This explains why the ghost location check had to be explicitly disabled in your test.

Proposed Fix

This PR modifies the traversal logic to allow ghost locations even if they originate from "generated code." Instead of blocking traversal, ghost locations are filtered when generating inlay hints.

@mlasson mlasson marked this pull request as draft February 4, 2025 18:02
@mlasson
Copy link
Contributor Author

mlasson commented Feb 4, 2025

I tested my "fix" with generated code and got the following result:
image

This highlights why simply filtering the output isn't a viable solution. Code generators actively use locations, sometimes reusing them multiple times, which makes it difficult to distinguish between meaningful and redundant hints.

@mlasson
Copy link
Contributor Author

mlasson commented Feb 4, 2025

Here is the PR that introduced the "avoid-ghost-location" ocaml/ocaml-lsp#1290 for context.

@mlasson
Copy link
Contributor Author

mlasson commented Feb 4, 2025

In commit [9a977d7](9a977d7), I reverted my previous fix and implemented a new approach.

The new fix stops traversal for structure items with a ghost location that are likely produced by deriving code generators while still allowing traversal inside expressions with ghost locations.

@mlasson
Copy link
Contributor Author

mlasson commented Feb 4, 2025

In commit 598e863 I proposed yet another fix. We can rely on the merlin.hide attribute introduced by well-behaved code generators !

@voodoos
Copy link
Collaborator

voodoos commented Feb 5, 2025

Seems reasonnable to rely on merlin.hide attributes for this indeed.

I wonder what happens in other cases were the compiler introduces ghost locations. Standard example of these are the use of punning in record's label names and function labels. Could you add such examples to your test ?

@mlasson
Copy link
Contributor Author

mlasson commented Feb 5, 2025

Closed in favor of #1894

@mlasson mlasson closed this Feb 5, 2025
@mlasson
Copy link
Contributor Author

mlasson commented Feb 5, 2025

@voodoos I added the tests #1894 (but they were already broken even when we were not checking for ghost locations)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants