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

Fix type annotation on function name in declaration #801

Open
ulugbekna opened this issue Jul 25, 2022 · 3 comments
Open

Fix type annotation on function name in declaration #801

ulugbekna opened this issue Jul 25, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@ulugbekna
Copy link
Collaborator

We shouldn't produce syntactically incorrect code, e.g.,

image

@3Rafal
Copy link
Collaborator

3Rafal commented Mar 17, 2023

I have tried to solve this with #1048, but found a problem when dealing with higher order functions.

Example:
We have two functions:

let my_fun x = fun y z -> 1

let my_fun' x y z = 1

We would probably want to annotate them like this:

let my_fun x : `a -> `b -> int = fun y z -> 1

let my_fun' x y z : int = 1

or this:

let my_fun x = (fun y z -> 1) : `a -> `b -> int

let my_fun' x y z = 1 : int

but they are represented almost identically in both parsetree and typedtree.
Parsetree dump diff:

-  structure_item (*buffer*[1,0+0]..[1,0+29])
+  structure_item (*buffer*[3,31+0]..[3,31+21])
     Pstr_value Nonrec
     [
       <def>
-        pattern (*buffer*[1,0+4]..[1,0+10])
-          Ppat_var \"my_fun\" (*buffer*[1,0+4]..[1,0+10])
-        expression (*buffer*[1,0+11]..[1,0+29]) ghost
+        pattern (*buffer*[3,31+4]..[3,31+11])
+          Ppat_var \"my_fun'\" (*buffer*[3,31+4]..[3,31+11])
+        expression (*buffer*[3,31+12]..[3,31+21]) ghost
           Pexp_fun
           Nolabel
           None
-          pattern (*buffer*[1,0+11]..[1,0+12])
-            Ppat_var \"x\" (*buffer*[1,0+11]..[1,0+12])
-          expression (*buffer*[1,0+15]..[1,0+29])
+          pattern (*buffer*[3,31+12]..[3,31+13])
+            Ppat_var \"x\" (*buffer*[3,31+12]..[3,31+13])
+          expression (*buffer*[3,31+14]..[3,31+21]) ghost
             Pexp_fun
             Nolabel
             None
-            pattern (*buffer*[1,0+20]..[1,0+21])
-              Ppat_var \"y\" (*buffer*[1,0+20]..[1,0+21])
-            expression (*buffer*[1,0+22]..[1,0+28]) ghost
+            pattern (*buffer*[3,31+14]..[3,31+15])
+              Ppat_var \"y\" (*buffer*[3,31+14]..[3,31+15])
+            expression (*buffer*[3,31+16]..[3,31+21]) ghost
               Pexp_fun
               Nolabel
               None
-              pattern (*buffer*[1,0+22]..[1,0+23])
-                Ppat_var \"z\" (*buffer*[1,0+22]..[1,0+23])
-              expression (*buffer*[1,0+27]..[1,0+28])
-                attribute \"merlin.loc\"
-                  []
+              pattern (*buffer*[3,31+16]..[3,31+17])
+                Ppat_var \"z\" (*buffer*[3,31+16]..[3,31+17])
+              expression (*buffer*[3,31+20]..[3,31+21])
                 Pexp_constant PConst_int (1,None)
     ]

The only difference in dump is attribute "merlin.loc" in the higher-order one.
Our current approach for code-action analysis is based on typedtree analysis, which is insufficient for this case. I have discussed this with @voodoos , and we've agreed to disable type annotation for now.
In future, we may have an access to an intermediate parsetree which will reflect the syntactic structures. Currently, in order for this code action to work in all cases, we would probably need to parse it on our own.

@ulugbekna
Copy link
Collaborator Author

Yeah, having concrete syntax tree (CST, or lossless syntax tree) for OCaml source would actually unlock many opportunities for olsp [*]. I know that Thomas Refis did some work on CSTs for OCaml programs for ocamlformat (you can look up his talk proposal in Tarides slack)

[*] Lossless syntax trees help language servers on many fronts code refactoring (btw, there's someone at Jane St. who has recently started work on refactoring -- I wonder if there could be some collaboration on this front), semantic highlighting. See:

[1] https://github.com/rust-lang/rust-analyzer/blob/master/docs/dev/syntax.md
[2] https://users.rust-lang.org/t/why-would-someone-use-rowan/78480
[3] We could improve our semantic highlighting support, e.g., I do some manual lexing for semantic highlighting (which has problems #1034), so this could be improved.

@ulugbekna
Copy link
Collaborator Author

For me, one of the very helpful workflows for type-annotation & de-annotation is

During development, it can be quite helpful to pin down the parameter types of a function for better IDE support (eg completions) and then remove the types. So type-annot & de-annot is one of the most crucial parts of this code action (for me, at least).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants