Hover info for local bindings
Overview
- Emits Type InfoNotes from the typecheecker containing information about the types of local bindings.
- Wires up Hover in LSP to provide this type info when hovering on references to local-bindings
Implementation notes
- Rather than only emitting notes for top-level bindings in the type-checker, emit notes for ALL bindings under different tags accordingly.
- Wire up hover so if the hover is a var reference, check the local binding types and find a match.
Interesting/controversial decisions
- Local var names don't need to be unique within a file (and you can shadow them anyways) so there may be many types emitted for the same var name, to account for this I also emit the scope of each binding as an annotation. When looking up the type of a binding you just find the smallest enclosing scope associated to that var name. This handles shadowing and multiple vars of the same name in the file.
Test coverage
- [ ] Add tests for hover info.
Loose ends
TODO:
- [x] Support hovering the name of a binding:
- The LHS of bindings are converted into
Absnodes in the resulting tree, and then are moved around all over the place in the tree as part of normalization, This makes it tricky to determine which variable we're referencing when hovering over the var at the binding site; ironically we can find the type of a var, just not at it's definition point 😆 . I can probably fix this either by keeping more info in the tree, or adding some special cases to how the LSP crawls the ABT, but it'll take a little more time to sort out.
- The LHS of bindings are converted into
- [x] Support hovering variables bound as function arguments
- I should be able to emit notes about these during typechecking too. Just need to track down the right spot for it.
- [x] Ensure we handle letrec's correctly, I suspect it doesn't work in some cases.
Unlocks new features:
- Will allow things like inline type-hints on local vars in your editor
Hmm I feel like I'm doing something wrong here, but when I try this out I get a totally different type than in your screenshot.
Also I just get "No information found" for local variables in the more involved definition that I tried. Any idea why our results might differ? I'm running a local build of 00d32cc4f .
@aryairani P.s. I don't think this is ready for review from anyone yet btw; I still have to fix let-recs, figure out what's up with Cody's issue, and fix some annotations 😄
@ceedubs Hrmm, I get your result if I remove the Nat. from the a Nat.+ 2;
Looks like I might be grabbing types from before TNDR has done its thing, probably fixable 😄
Oh I didn't realize that I failed to copy verbatim. Something something jpegs are lossy when copy/pasting code. But I guess that it was good because I might have caught an issue :)
No matter how much I work on something I can always trust @ceedubs to find something I missed, which I actually really appreciated 😄 ; Something to be said for having someone really unlucky on the team 😅 😆
@ChrisPenner I don't have a sense of how much work it is to get this to a state in which you think that it is ready for review. It's certainly not a high priority, but I just wanted to let you know that this would be a very welcome feature if you think that it's not much work to push it through :)
@ChrisPenner I've been using this branch (with some updates from trunk) for a couple of days and it has been great!
Here is an example that I noticed where the type doesn't look right. It shows '{IO} 𝕣, but the actual type should be Signal [Text], which is annotated a few lines above. I thought that maybe it was finding a stdout term in base, but that doesn't seem to be the case because those are named stdOut and StdOut (and don't have this type signature either).
@aryairani ugh, I think the diff got all mixed up; there's definitely not this many actual changes.