~~Uglify~~ Format Code

Author: Booksbaum

issue.md

@@ -0,0 +1,171 @@
+
+# InlayHints: LSP `3.17` & `TextEdit`s
+
+Add & Implement `InlayHint`s from LSP `3.17` (as mentioned in #935)  
+* Note: LSP `3.17` was just released (on `2022-05-10`) -- but in this PR I'm still treating it as still in preview: most code I wrote before `3.17` and Types aren't yet in `Ionide.LanguageServerProtocol` -> in here still temporary
+* Note: InlayHint capabilities: not handled at all
+
+Both `textDocument/inlayHint` and `inlayHint/resolve` are implemented -- with exception of `Tooltip`s: That's still always `None`.
+* `inlayHint/resolve`: resolves `Tooltip` & `TextEdits` when missing from `InlayHint` -- but in reality it's actually completely unused & unnecessary:
+  * As mentioned above: Tooltips aren't currently generated at all
+  * `TextEdits` are always created when collecting all InlayHints and not delayed to be `resolve`d later.  
+    Reason: I changed ident detection & validation (-> "should get a hint?") slightly for type hints. In the process I collect all data necessary for `TextEdit`s too -> no reason to resolve later again...
+
+<br/>
+<br/>
+
+Use `TextEdit`s instead of just "insert string at Inlay Hint Location" (-> necessary for parens)
+* Note: `fsharp/inlayHints` still exists (-> compatibility with older version) -- but its `InsertText` can only insert at InlayHint location (-> no parens) -> might be wrong
+
+<br/>
+<br/>
+
+Use same logic for type `InlayHint`s & `AddExplicitTypeAnnotation` CodeFix  
+
+Enhancements around existing type annotation detection, like
+* already type annotated?
+* needs parens when type annotation gets inserted? 
+* where must parens go
+* And some other special cases like `?a` -> optional parameter, but type annotation cannot have `option` (`: int` instead of `: int option`)
+
+I'm using this (at least in part) to decide if type Hints should be emitted.  
+-> This replaces some other checks  
+Though I'm not yet sure it still behaves the same -> there are still some additional tests necessary (-> PR is still *WIP*)  
+
+<br/>
+
+On enhancement is to handle function values:
+```fsharp
+let map f v = f (v+1)
+//      ^
+
+type A(a) =
+  member _.F(b, f) = a + b |> f
+//              ^
+
+let f = fun a b -> a + b
+//  ^
+```
+-> `AddExplicitType` can add type annotations to `f`.  
+But should we show type hints too? (currently disabled)  
+It's nice for short cases -- but when it accepts many args that hint might get quite large and distracting.  
+(Ideal solution would be to make it a setting for user -- but not now. And even then we should use reasonable defaults)
+
+(Note: currently not for functions (`let f a b = a + b`). Maybe someday -- but not now. Getting a function's return type is unfortunately not trivial (at least wasn't last time I tried))
+
+<br/>
+<br/>
+
+------------------------
+
+<br/>
+
+Still WIP because:
+* No tests yet for `textDocument/inlayHint`
+* Still some testing to do around change in InlayHint triggering
+* Some cleanup (I left some unused code and TODOs)
+
+* Considering LSP `3.17` was just released: Maybe wait till update of `Ionide.LanguageServerProtocol`?
+  -> Then we don't have to remember to remove `LSP.Preview.fs` sometime later (but instead do that directly in this PR here)
+
+
+
+
+//TODO: show type:
+```fsharp
+let f a = fun b -> a + b
+
+let map f v = f (v+1)
+//      ^
+```
+currently: no (main as well as this PR)
+should show?
+
+first f: currently shows `: int`
+
+```fsharp
+let f a = fun b -> a + b
+
+let map f v = f (v+1)
+
+type A(a) =
+  member _.F(b, f) = a + b |> f
+
+let f2: int -> int -> int = fun a b -> a + b
+```
+
+
+-------------------------------------
+
+InlayHint-Data-> Generic
+
+`Data` is of type [`LSPAny`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#lspAny):
+```typescript
+export type LSPAny = LSPObject | LSPArray | string | integer | uinteger |
+	decimal | boolean | null;
+export type LSPObject = { [key: string]: LSPAny };
+export type LSPArray = LSPAny[];
+```
+Not really representable in F#  
+-> I decided to use a Generic instead
+
+But then of course correctness is in user hand and not type checked any more. I still think it's way easier to use than anything else.
+
+--------------------------
+InlayHints -> tryCreateTypeHint -> Text
+
+`Type` and `Type necessary for annotation` don't necessary need to match:
+```fsharp
+static member F(?value) = value |> Option.map ((+) 1)
+//               ^^^^^ type: `int option`
+// with type annotation:
+static member F(?value: int) = value |> Option.map ((+) 1)
+//                      ^^^ just `int` without `option`
+```
+What should we use as type hint? (when inserted it correctly uses just `int`)
+
+Currently I'm showing `int option` -- but the sole reason is: That's what was used before.  
+I'm always confused when type in annotation is without option -- but it's confusing too to show type hint with option but when it gets inserted the option disappears
+
+<br/>
+
+Additional:  
+I'm currently only handle trailing `option`.  
+Does `type.Format` always produce a trailing `option`, or is `Option<...>` (maybe even with namespace) possible?
+
+
+-----------------
+
+InlayHints -> VisitSimplePats -> SyntaxNode.SynTypeDefn 
+
+When does this happen?
+Case above is normal primary ctor (`type A(v) = ...`) -- but when does this here occur?
+
+
+----------------
+
+`ServiceParseTreeWalk`
+
+Copied from `dotnet/fsharp`
+
+Current nuget package has a bug when traversing match cases (fixed in `dotnet/fsharp` main) & doesn't walk into all `SynPat`s (dotnet/fsharp#13114)
+
+Once there's a release of FCS with match case traversal fixed, **we must remember to remove this file again!**
+
+
+
+
+-----------
+
+//TODO: lambda -> IsFunction, but not included
+
+//TODO: rename AddExplTyToParameter to `AddExplTy` -> not limited to param any more
+---------------------------------
+
+
+(
+  In the future this may be expanded to additional identify where a binding is. And then hide and show based on location & user settings.  
+  (location like: `let`, `let!`, local `let`, global `let`, parameter, ...; and even further into bind)
+)
+
+//TODO: mention: enhancement: extend to determine location (like let, binding, in match, nested in Record, Union, ...) -> for filtering (user settings)