Wrong documentation on hover #344

ulugbekna · 2020-12-08T12:16:18Z

On hover, I sometimes get documentation for a completely different thing than the identifier I hover over. The "different thing" is usually something that comes from stdlib such as > operator or as in the screenshot, where I hover over err_pos_range and get correct type but not doc.

(might be a merlin issue, but the last issue found with ocaml-lsp I raised in merlin repo turned out to be ocaml-lsp related, so creating the issue here)

The text was updated successfully, but these errors were encountered:

tmattio · 2020-12-08T12:18:08Z

Related to ocamllabs/vscode-ocaml-platform#111

ulugbekna · 2020-12-08T12:52:37Z

That issue could be closed since the problem cannot be fixed on the vscode extension side.

…

On Tue, 8 Dec 2020 at 17:18, Thibaut Mattio ***@***.***> wrote: Related to ocamllabs/vscode-ocaml-platform#111 <ocamllabs/vscode-ocaml-platform#111> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#344 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD4YR67VJCYXJLGBL6V2IUTSTYKQ7ANCNFSM4UR4IECQ> .

ulugbekna · 2021-02-27T13:54:29Z

I think I got a reproducible case.

When one types a module name List, example :

(* file s.ml *) 
module M = struct 
  let u = () 
  let f = List.<cursor>
end

for some reason the documentation on hover at position p in file s.ml is fetched from the file where List is defined, when it should actually fetch the documentation from file s.ml. I have only noticed the issue with stdlib modules.

In the video I need to push the module def down to reproduce the bug, because the List module documentation in list.mli occurs on lines ~20; hence, we see that the documentation is fetched for things at position p from list.mli instead of the currently open file.

Screen.Recording.2021-02-27.at.4.59.15.PM.mov

Phrased differently, if we fetch documentation for a stdlib module (by typing the module name and hence triggering auto-completion which shows documentation, or hovering over the module, which also brings docs), we start fetching docs from the file instead of the currently open one.

ulugbekna · 2021-04-20T19:36:49Z

This is fixed on master by updating to the latest merlin 4.2

edit: NVM I confused this issue with another one

rgrinberg · 2021-04-22T04:02:21Z

@ulugbekna can you make a regression test for this? I'll try and have a look.

rgrinberg · 2021-04-28T01:10:14Z

I'm unable to reproduce this issue with the example above.

ulugbekna · 2021-04-28T09:14:59Z

Sorry, I missed your first message. I'll try to come up with a test. I can still reproduce it manually though.

…

On Wed, 28 Apr 2021 at 06:10, Rudi Grinberg ***@***.***> wrote: I'm unable to reproduce this issue with the example above. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#344 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD4YR6YZEYR47GD3H43AC4DTK5OANANCNFSM4UR4IECQ> .

sir4ur0n · 2021-05-04T19:13:53Z

I never know if this kind of message is useful, but just in case: I regularly encounter this problem too, and so do several of my teammates (to confirm this is not an isolated issue for OP).

Edit: one of my teammates rightfully mentioned that it seems to often replace the doc with the doc of an Stdlib function/type (in case this helps). I noticed the same thing

rgrinberg · 2021-05-04T19:17:17Z

What's blocking me from making progress on this issue is a (preferably small) reproducible test case. Next time you run into this issue, see if you can isolate it. Rudi.

…

On May 4, 2021, 12:14 PM -0700, Julien Debon ***@***.***>, wrote: I never know if this kind of message is useful, but just in case: I regularly encounter this problem too, and so do several of my teammates (to confirm this is not an isolated issue for OP). — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

smelc · 2021-05-04T19:24:10Z

@rgrinberg> Thanks for your awesome work on ocaml-lsp. As I'm regularly affected by this issue, I'll keep in mind that a reproduction procedure is welcome.

ulugbekna · 2021-05-05T13:30:35Z

Since the bug is triggered by "pulling in" an stdlib module, for example, for auto-complete (List.ma<|>) or hovering over List.map, AND after pulling in all consequent requests result in wrong documentation, I think there is some kind of a mutable cache in merlin for documentation or "the current module" cache from which the documentation is extracted.

ulugbekna · 2021-05-05T16:55:20Z

update: I was able to reproduce the bug with any library module (not only stdlib), but the bug still doesn't occur with modules of the project itself, only libraries.

ulugbekna · 2021-05-06T22:07:25Z

Removing these lines fixes the issue without regressions on the doc-on-hover tests in merlin.

I could create a PR but I'm having problems with jq in merlin test suite, where I keep getting parse errors (from this issue I guess). I'm not sure how to get around this without changing the tests.

   $ $MERLIN single document -position 7:10 -filename doc.ml < doc.ml |
   > jq '.value'
-  "second function"
+  parse error: Invalid string: control characters from U+0000 through U+001F must be escaped at line 9, column 1
+  [4]

cc @rgrinberg

edit: on this line, cmt_path is "lib/ocaml/stdlib__list.cmti" for the test case I created in #437. But we really shouldn't be taking any documentation from that cmti file. Maybe I'm missing some other case when we do need to include such cmti's.

use the current file fixes ocaml/ocaml-lsp#344

CHANGES: ## Features - Add a new code action `Add missing rec keyword`, which is available when adding a `rec` keyword can fix `Unbound value ...` error, e.g., ```ocaml let fact n = if n = 0 then 1 else n * fact (n - 1) (* ^^^^ Unbound value fact *) ``` Adding `rec` to the definition of `fact` will fix the problem. The new code action offers adding `rec`. - Jump to the first hole on calling `Destruct` code action (only with client VSCode OCaml Platform) (ocaml/ocaml-lsp#468) Example: when a user invokes `Destruct` code action on `Some 1`, this code is replaced by `match Some 1 with None -> _ | Some _ -> _`, where the 1st and 3rd underscores are "typed holes", a concept created by Merlin to be able to put "holes" in OCaml code. With this change, now for VSCode OCaml Platform users, on such invocation of `Destruct`, the cursor will jump to the first typed hole and select it, so that user can start editing right away. - Use ocamlformat to properly format type snippets. This feature requires the `ocamlformat-rpc` opam package to be installed. (ocaml/ocaml-lsp#386) - Add completion support for polymorphic variants, when it is possible to pin down the precise type. Examples (`<|>` stands for the cursor) when completion will work (ocaml/ocaml-lsp#473) Function application: ``` let foo (a: [`Alpha | `Beta]) = () foo `A<|> ``` Type explicitly shown: ``` let a : [`Alpha | `Beta] = `B<|> ``` Note: this is actually a bug fix, since we were ignoring the backtick when constructing the prefix for completion. - Parse merlin errors (best effort) into a more structured form. This allows reporting all locations as "related information" (ocaml/ocaml-lsp#475) - Add support for Merlin `Construct` command as completion suggestions, i.e., show complex expressions that could complete the typed hole. (ocaml/ocaml-lsp#472) - Add a code action `Construct an expression` that is shown when the cursor is at the end of the typed hole, i.e., `_|`, where `|` is the cursor. The code action simply triggers the client (currently only VS Code is supported) to show completion suggestions. (ocaml/ocaml-lsp#472) - Change the formatting-on-save error notification to a warning notification (ocaml/ocaml-lsp#472) - Code action to qualify ("put module name in identifiers") and unqualify ("remove module name from identifiers") module names in identifiers (ocaml/ocaml-lsp#399) Starting from: ```ocaml open Unix let times = Unix.times () let f x = x.Unix.tms_stime, x.Unix.tms_utime ``` Calling "remove module name from identifiers" with the cursor on the open statement will produce: ```ocaml open Unix let times = times () let f x = x.tms_stime, x.tms_utime ``` Calling "put module name in identifiers" will restore: ```ocaml open Unix let times = Unix.times () let f x = x.Unix.tms_stime, x.Unix.tms_utime ``` ## Fixes - Do not show "random" documentation on hover - fixed by [merlin#1364](ocaml/merlin#1364) - fixes duplicate: - [ocaml-lsp#344](ocaml/ocaml-lsp#344) - [vscode-ocaml-platform#111](ocamllabs/vscode-ocaml-platform#111) - Correctly rename a variable used as a named/optional argument (ocaml/ocaml-lsp#478) - When reporting an error at the beginning of the file, use the first line not the second (ocaml/ocaml-lsp#489)

CHANGES: ## Features - Add a new code action `Add missing rec keyword`, which is available when adding a `rec` keyword can fix `Unbound value ...` error, e.g., ```ocaml let fact n = if n = 0 then 1 else n * fact (n - 1) (* ^^^^ Unbound value fact *) ``` Adding `rec` to the definition of `fact` will fix the problem. The new code action offers adding `rec`. - Use ocamlformat to properly format type snippets. This feature requires the `ocamlformat-rpc` opam package to be installed. (ocaml/ocaml-lsp#386) - Add completion support for polymorphic variants, when it is possible to pin down the precise type. Examples (`<|>` stands for the cursor) when completion will work (ocaml/ocaml-lsp#473) Function application: ``` let foo (a: [`Alpha | `Beta]) = () foo `A<|> ``` Type explicitly shown: ``` let a : [`Alpha | `Beta] = `B<|> ``` Note: this is actually a bug fix, since we were ignoring the backtick when constructing the prefix for completion. - Parse merlin errors (best effort) into a more structured form. This allows reporting all locations as "related information" (ocaml/ocaml-lsp#475) - Add support for Merlin `Construct` command as completion suggestions, i.e., show complex expressions that could complete the typed hole. (ocaml/ocaml-lsp#472) - Add a code action `Construct an expression` that is shown when the cursor is at the end of the typed hole, i.e., `_|`, where `|` is the cursor. The code action simply triggers the client (currently only VS Code is supported) to show completion suggestions. (ocaml/ocaml-lsp#472) - Change the formatting-on-save error notification to a warning notification (ocaml/ocaml-lsp#472) - Code action to qualify ("put module name in identifiers") and unqualify ("remove module name from identifiers") module names in identifiers (ocaml/ocaml-lsp#399) Starting from: ```ocaml open Unix let times = Unix.times () let f x = x.Unix.tms_stime, x.Unix.tms_utime ``` Calling "remove module name from identifiers" with the cursor on the open statement will produce: ```ocaml open Unix let times = times () let f x = x.tms_stime, x.tms_utime ``` Calling "put module name in identifiers" will restore: ```ocaml open Unix let times = Unix.times () let f x = x.Unix.tms_stime, x.Unix.tms_utime ``` ## Fixes - Do not show "random" documentation on hover - fixed by [merlin#1364](ocaml/merlin#1364) - fixes duplicate: - [ocaml-lsp#344](ocaml/ocaml-lsp#344) - [vscode-ocaml-platform#111](ocamllabs/vscode-ocaml-platform#111) - Correctly rename a variable used as a named/optional argument (ocaml/ocaml-lsp#478) - When reporting an error at the beginning of the file, use the first line not the second (ocaml/ocaml-lsp#489)

tmattio added the bug Something isn't working label Dec 8, 2020

ulugbekna changed the title ~~Wrong doc on hover~~ Wrong documentation on hover Feb 27, 2021

ulugbekna closed this as completed Apr 20, 2021

ulugbekna reopened this Apr 20, 2021

ulugbekna added a commit to ulugbekna/ocaml-lsp that referenced this issue May 5, 2021

reproduce ocaml#344

0b020d2

ulugbekna mentioned this issue May 5, 2021

reproduce wrong-doc-on-hover bug from #344 #437

Merged

ulugbekna added a commit to ulugbekna/merlin that referenced this issue May 7, 2021

add a test to show issue ocaml/ocaml-lsp#344

018dd6f

ulugbekna added a commit to ulugbekna/merlin that referenced this issue May 7, 2021

do not look for documentation in the last opened file,

7fe6190

use the current file fixes ocaml/ocaml-lsp#344

ulugbekna mentioned this issue May 7, 2021

Fixes random documentation fetched on hover ocaml/merlin#1339

Closed

ulugbekna added a commit that referenced this issue May 7, 2021

reproduce #344

88a7424

ulugbekna mentioned this issue May 10, 2021

Wrong doc comment displayed ocamllabs/vscode-ocaml-platform#597

Closed

ulugbekna added a commit to ulugbekna/merlin that referenced this issue Jun 9, 2021

add a test to show issue ocaml/ocaml-lsp#344

3397b89

ulugbekna added a commit to ulugbekna/merlin that referenced this issue Jun 9, 2021

do not look for documentation in the last opened file,

408b475

use the current file fixes ocaml/ocaml-lsp#344

ulugbekna added a commit to ulugbekna/merlin that referenced this issue Jun 29, 2021

add a test to show issue ocaml/ocaml-lsp#344

5bd473f

ulugbekna added a commit to ulugbekna/merlin that referenced this issue Jun 29, 2021

do not look for documentation in the last opened file,

bf1d3a3

use the current file fixes ocaml/ocaml-lsp#344

ulugbekna mentioned this issue Jul 2, 2021

Syntactic symbols are conflated with standard library functions in tooltips ocamllabs/vscode-ocaml-platform#654

Open

trefis pushed a commit to ocaml/merlin that referenced this issue Jul 13, 2021

add a test to show issue ocaml/ocaml-lsp#344

cb02ab6

ulugbekna mentioned this issue Jul 23, 2021

add support for refactor-open (qualify and unqualify) code action #399

Merged

5 tasks

ulugbekna linked a pull request Jul 23, 2021 that will close this issue

add support for refactor-open (qualify and unqualify) code action #399

Merged

5 tasks

ulugbekna closed this as completed in #399 Jul 26, 2021

rgrinberg mentioned this issue Aug 19, 2021

[new release] ocaml-lsp-server, lsp and jsonrpc (1.8.0) ocaml/opam-repository#19286

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrong documentation on hover #344

Wrong documentation on hover #344

ulugbekna commented Dec 8, 2020

tmattio commented Dec 8, 2020

ulugbekna commented Dec 8, 2020 via email

ulugbekna commented Feb 27, 2021 •

edited

Loading

ulugbekna commented Apr 20, 2021 •

edited

Loading

rgrinberg commented Apr 22, 2021

rgrinberg commented Apr 28, 2021

ulugbekna commented Apr 28, 2021 via email

sir4ur0n commented May 4, 2021 •

edited

Loading

rgrinberg commented May 4, 2021 via email

smelc commented May 4, 2021

ulugbekna commented May 5, 2021

ulugbekna commented May 5, 2021 •

edited

Loading

ulugbekna commented May 6, 2021 •

edited

Loading

Wrong documentation on hover #344

Wrong documentation on hover #344

Comments

ulugbekna commented Dec 8, 2020

tmattio commented Dec 8, 2020

ulugbekna commented Dec 8, 2020 via email

ulugbekna commented Feb 27, 2021 • edited Loading

ulugbekna commented Apr 20, 2021 • edited Loading

rgrinberg commented Apr 22, 2021

rgrinberg commented Apr 28, 2021

ulugbekna commented Apr 28, 2021 via email

sir4ur0n commented May 4, 2021 • edited Loading

rgrinberg commented May 4, 2021 via email

smelc commented May 4, 2021

ulugbekna commented May 5, 2021

ulugbekna commented May 5, 2021 • edited Loading

ulugbekna commented May 6, 2021 • edited Loading

ulugbekna commented Feb 27, 2021 •

edited

Loading

ulugbekna commented Apr 20, 2021 •

edited

Loading

sir4ur0n commented May 4, 2021 •

edited

Loading

ulugbekna commented May 5, 2021 •

edited

Loading

ulugbekna commented May 6, 2021 •

edited

Loading