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

Improve Inlay Hints Handling with [@merlin.hide] #1894

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mlasson
Copy link
Contributor

@mlasson mlasson commented Feb 5, 2025

Description

Previously, inlay hints logic relied on ghost locations to avoid annotating generated code. But it lead to some inconsistencies because ghost locations are used in non-generated code, for instance:

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

This is because

let f _ = ...

is syntactic sugar for:

let f = fun _ -> ...

where fun _ -> introduces a ghost location (the whole fun node).

This is why you needed to disable the check on ghost location in your samples.t test.

Proposed Fix

This PR replaces ghost location handling with the [@merlin.hide] attribute. This allows for more precise control over which parts of the AST should be ignored for inlay hints.

To make this approach work correctly, I refactored the main AST iterator to use Ast_iterators.iter_only_visible. The iterator now ensures attributes are not skipped by recursing on non-immediate sub-terms.

Also, this restructuring simplifies the code and makes inlay hints more consistent across different contexts (e.g., top-level let bindings, class-level let bindings, and expression let bindings).

Additional Changes

  • Updated samples.t with new test cases illustrating the improved inlay hints behavior.
  • Removed support for the avoid-ghost-location option (both in the CLI and protocol).

Related

Screenshot

image

@mlasson mlasson force-pushed the mlasson-improve-inlay-hints branch from c110cb7 to 1440a86 Compare February 5, 2025 17:18
@mlasson
Copy link
Contributor Author

mlasson commented Feb 5, 2025

Is it ok to change the protocol/library in a non-backward compatible way ?

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.

1 participant