Skip to content

Commit

Permalink
The missing comment (#2491)
Browse files Browse the repository at this point in the history
* Debugging CodePrinter.

* The Missing Comment

* Apply suggestions from code review

Co-authored-by: dawe <[email protected]>

Co-authored-by: dawe <[email protected]>
  • Loading branch information
nojaf and dawedawe authored Sep 10, 2022
1 parent c1a16e6 commit 1d1cda7
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 1 deletion.
2 changes: 1 addition & 1 deletion docs/docs/contributors/How Can I Contribute.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,4 +192,4 @@ This update to the tree made it very straightforward to fix the original bug in

Another example of code that could benefit from a better representation is [extern](https://github.com/fsprojects/fantomas/issues?q=is%3Aissue+is%3Aopen+extern).

<fantomas-nav previous="./Conditional%20Compilation%20Directives.html"></fantomas-nav>
<fantomas-nav previous="./Conditional%20Compilation%20Directives.html" next="./The%20Missing%20Comment.html"></fantomas-nav>
19 changes: 19 additions & 0 deletions docs/docs/contributors/Print AST with Context.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,23 @@ Instead we can use various helper functions that take the `Context` as parameter
Please take a moment to debug the unit tests in [CodePrinterHelperFunctionsTests.fs](https://github.com/fsprojects/fantomas/blob/master/src/Fantomas.Core.Tests/CodePrinterHelperFunctionsTests.fs).
This will give you a better understanding of how we capture events in `CodePrinter`.

### Debugging CodePrinter

One thing that is a bit harder to grasp initially, is what is happening when you put a breakpoint in `CodePrinter.fs`.
In `CodePrinter.fs` we compose a format function that takes a `Context` and returns a `Context`.
We do this by traversing the syntax tree, and when you put a breakpoint in `genTypeDefn` for example:

![Breakpoint in genTypeDefn](../../images/debugging-code-printer-1.png)

we are still in the process of composing the format function.
**The Context has not been going through our format function yet!**

If we want to debug when the `Context` is traveling through the format function, we can easily, temporarily, insert an additional function to inspect the `Context` content:

![Breakpoint in Context -> Context](../../images/debugging-code-printer-2.png)

The `dumpAndContinue` helper function can be used to inspect the `Context`.
Please remove all usages when submitting a PR 😸.


<fantomas-nav previous="./Prepare%20Context.html" next="./Formatted%20Code.html"></fantomas-nav>
124 changes: 124 additions & 0 deletions docs/docs/contributors/The Missing Comment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
---
category: Contributors
categoryindex: 2
index: 10
---
# The Missing Comment

Code comments can literally exist between every single F# token. I'm looking at you block comment `(* ... *)`.
As explained in [Detecting trivia](./Prepare%20Context.html#Detecting-trivia), we need to do quite some processing to restore code comments.
In this guide, we would like to give you seven tips to restore a missing comment!

## Breathe

We understand it very well that losing a code comment after formatting can be extremely frustrating.
There is no easy fix that will solve all the missing comments overnight. Each case is very individual and can be complex to solve.
May these steps help towards fixing your problem!

## Isolate the problem

Before we can commence our murder mystery, it is best to narrow down our problem space.

Example ([#2490](https://github.com/fsprojects/fantomas/issues/2490)):

```fsharp
let (* this comment disappears after formatting *) a = []
type A =
{
X : int
...
}
while true do ()
```

Using the [online tool](https://fsprojects.github.io/fantomas-tools/#/fantomas/preview), we can remove any code that isn't relevant.
The `type` and `while` code can be trimmed and the problem still exists.

## Check the syntax tree

Every code comment should be present on the root level of the syntax tree.
[ParsedImplFileInputTrivia](../../reference/fsharp-compiler-syntaxtrivia-parsedimplfileinputtrivia.html) or [ParsedSigFileInputTrivia](../../reference/fsharp-compiler-syntaxtrivia-parsedsigfileinputtrivia.html) should contain the comment.

```fsharp
ImplFile
(ParsedImplFileInput
("tmp.fsx", true, QualifiedNameOfFile Tmp$fsx, [], [],
[ ... ], (false, false),
{ ConditionalDirectives = []
CodeComments = [BlockComment tmp.fsx (1,4--1,50)] }))
```

If the comment is not there this means the F# lexer and parser didn't pick up the comment. In the unlikely event this happened, this should be fixed first over at [dotnet/fsharp](https://github.com/dotnet/fsharp).

## Was the comment detected as trivia?

Fantomas grabs the comments straight from the syntax tree, and transforms them as `Trivia`.
This is a fairly straightforward process, and you can easily visually inspect this using the [online tool](https://fsprojects.github.io/fantomas-tools/#/trivia).

![Trivia in online tool](../../images/online-tool-trivia-1.png)

If your comment does not show up there, it means there is a problem between getting the information from the syntax tree and constructing the `Trivia` in `Trivia.fs`.

## Was the trivia assigned to a trivia node?

The `Trivia` needs to be assigned to a `TriviaNode` to be stored in the `Context`.
Using the center tab in the tool, we can see the tree structure of all available `TriviaNodes`.

![Root node in online tool](../../images/online-tool-trivia-2.png)

Choosing the best suitable node can be quite tricky, there are different strategies for different `TriviaTypes`.
In this example `SynBindingKind_Normal` and `SynPat_Named` are good candidates as they appear right after the comment.
Once the `Trivia` is assigned to a `TriviaNode`, a `TriviaInstruction` will be created. This is the link between them and is what will be stored in the `Context`.

The left-most tab in the tool shows us the result of the assigment:

![Trivia Instructions in online tool](../../images/online-tool-trivia-3.png)

In this example, at the time of writing, there are no `TriviaInstructions`. This means the assignment in `Trivia.fs` failed and we would need to investigate why.

## Was the trivia assigned to the best possible node?

Sometimes the selected `TriviaNode` isn't really the best possible candidate.
In [#640](https://github.com/fsprojects/fantomas/issues/640), the `Directive` trivia are assigned to `SynBindingKind_Normal`.

![Wrong node assignment in online tool](../../images/online-tool-trivia-4.png)

Notice that `SynBindingKind_Normal` starts at line `6`, while the `Trivia` is wrapped around the `inline` keyword on line `4`.
It would be ideal if we could use the `inline` keyword as a `TriviaNode`, but looking at the tree, it doesn't appear to be present.
Why is that? Well, the syntax tree does, at the time of writing, not provide a `range` for the keyword.
This would be a great addition as to [SyntaxTreeTrivia](../../reference/fsharp-compiler-syntaxtrivia.html), and can be done by submitting a pull request to [dotnet/fsharp](https://github.com/dotnet/fsharp).

## Printing the TriviaInstruction

The last piece of the puzzle is printing the actual `TriviaInstruction` in `CodePrinter`.
If everything up to this point went well, and the comment is still missing after formatting, it means it was not printed.

Every `TriviaInstruction` has a type (`FsAstType`) and a `range`. The `range` is taken from an actual AST node, so when we encounter this `range` in `CodePrinter`, we need to act upon it.
We typically do this by calling `genTriviaFor`, passing down the `type`, the `range` and a function `f` that should be composed in between.

Example:

```fsharp
genTriviaFor Ident_ (* : FsAstType *) ident.idRange (* range : *) genIdent (* : Context -> Context *)
```

If we look at the definition of `genTriviaFor`:

```fsharp
and genTriviaFor (mainNodeName: FsAstType) (range: Range) f ctx =
(enterNodeFor mainNodeName range +> f +> leaveNodeFor mainNodeName range) ctx
```

We can derive that the composition will be as follows:

```fsharp
enterNodeFor Ident_ ident.idRange
+> genIdent
+> leaveNodeFor Ident_ ident.idRange
```

`enterNodeFor` and `leaveNodeFor` will print the `TriviaInstruction` it finds for the `range` and `type` inputs in the `Context`.

<fantomas-nav previous="./How%20Can%20I%20Contribute.html"></fantomas-nav>
Binary file added docs/images/debugging-code-printer-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/images/debugging-code-printer-2.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/images/online-tool-trivia-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/images/online-tool-trivia-2.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/images/online-tool-trivia-3.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/images/online-tool-trivia-4.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 1d1cda7

Please sign in to comment.