Fix hover on method calls

[voodoos]

Fixes #1552

Now object method types are correctly shown.

The issue was due to a missing case in the hover filtering logic (cc @awilliambauer).

[coveralls]

Pull Request Test Coverage Report for Build 4929

Details

• 1 of 4 (25.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.02%) to 23.423%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
ocaml-lsp-server/src/hover_req.ml	1	4	25.0%

Totals
Change from base Build 4922:	0.02%
Covered Lines:	5991
Relevant Lines:	25577

---

💛 - Coveralls

[trefis]

Perhaps the "filtering" match could be made non-fragile to avoid such issues in the future (and so every situation is properly considered)?

Side note: I feel like this code kind of duplicates what the browse tree is used for in merlin...

[voodoos]

Side note: I feel like this code kind of duplicates what the browse tree is used for in merlin...

My feeling is that the goal of that code is distinct from the one of mbrowse. Merlin was designed around a protocol and a set of editors that require a deliberate action from users to call the type-enclosing command: thus, if the user asked, she probably expects an answer. In visual LSP-based editors, such as vscode, type-enclosing is called continuously, everywhere around the code, as soon as the cursor moves. The best effort strategy of Merlin can result in a lot of clutter, and I think it makes sense to rely on stricter filtering before answering. (I do agree that it could be worth investigating how to re-conciliate both usages with the same code paths internally.)

It's also true that the opt-in nature of the current implementation make it sensible to syntax changes, additions (and omissions such as this one). However it is not a trivial task to reverse this behavior, and outside the scope of this PR.

Meanwhile, we might want to check that other parsetree constructions where an ident is not actually represented by a Pexp_ident node are not suffering from the same omission...

For reference, the PR that introduced this filtering is #1245

[xvw]

I agree with the fix
(Even, I think that, in a future, providing hover as part of the Merlin protocol make sense).

[xvw]

(I guess that just running dune fmt should make the CI happy?)