Report parse errors with their position and the source

CHANGELOG.md

@@ -12,7 +12,8 @@
 - Breaking: warnings and errors are written to standard error instead of standard out. Informational output stays on standard out, including `--version` and the files `--check` reports as needing formatting, so a caller can tell the tool's output apart from its diagnostics by stream. Scripts that capture standard out to detect failures need to capture standard error as well. [#3399](https://github.com/fsprojects/fantomas/pull/3399)
 - Update FCS to 'Parser: recover on missing when conditions', commit d05075e098278aedcea3379159504d664628a495 [#3400](https://github.com/fsprojects/fantomas/pull/3400)
 - Breaking: the `--help` page is written by Fantomas instead of by Argu. It carries the version, worked examples, what an input path may be, and links to the documentation, the F# Discord and the `llms.txt` files an LLM can read. Colours are used when the terminal supports them and dropped when standard out is redirected. `-h` is now accepted alongside `--help`. An argument error reports the complaint on standard error followed by a pointer to `--help`, where it used to print Argu's usage block. [#3402](https://github.com/fsprojects/fantomas/pull/3402)
-- Breaking: a run over a single file reports the path it was given instead of only the file name, so `fantomas src/A.fs` prints `src/A.fs was formatted.` where it printed `A.fs was formatted.`. The same applies to the unchanged, ignored and `Failed to format file` messages. A run over several files already reported the path, so the two now agree. [#3404](https://github.com/fsprojects/fantomas/pull/3404)
+- Breaking: a run over a single file reports the path it was given instead of only the file name, so `fantomas src/A.fs` prints `src/A.fs was formatted.` where it printed `A.fs was formatted.`. The same applies to the unchanged, ignored and failure messages. A run over several files already reported the path, so the two now agree. [#3404](https://github.com/fsprojects/fantomas/pull/3404)
+- Breaking: a file that cannot be parsed is reported with the position of every diagnostic the parser produced, one MSBuild style line each, followed by a snippet of the source with two lines of context either side and a caret under the offending range. This replaces `Could not parse the file.` for a format run, the `%A` record dump and stack trace for a `--check` run, and the same `%A` dump the daemon used to hand editors. Diagnostics are ordered by position and columns are one based, matching what the F# compiler prints for the same file. [#3405](https://github.com/fsprojects/fantomas/pull/3405)
 
 ### Fixed
 

docs/docs/end-users/UpgradeGuide.md

@@ -98,7 +98,8 @@ fsharp_experimental_stroustrup_style = true
 ### console application
 - Target framework is now `net10.0`.
 - Warnings and errors are written to standard error instead of standard out. A script that captured standard out to detect failures needs to capture standard error as well. Informational output stays on standard out, including `--version` and the files `--check` reports as needing formatting.
-- A run over a single file reports the path it was given rather than only the file name, so `fantomas src/A.fs` prints `src/A.fs was formatted.` where it printed `A.fs was formatted.`. The same applies to the unchanged, ignored and `Failed to format file` messages. A run over several files already reported the path, so a script that handled both cases can now treat them alike.
+- A run over a single file reports the path it was given rather than only the file name, so `fantomas src/A.fs` prints `src/A.fs was formatted.` where it printed `A.fs was formatted.`. The same applies to the unchanged, ignored and failure messages. A run over several files already reported the path, so a script that handled both cases can now treat them alike.
+- A file that cannot be parsed is reported with the position of each diagnostic instead of `Could not parse the file.`, one line per diagnostic in the shape `src/A.fs(3,9): error FS0583: Unmatched '('`, followed by the source around the failure with a caret under it. `--check` reported the same failure as an exception dump with a stack trace and now uses this as well. A script that matched on `Could not parse the file.` needs to match on the new text.
 - The `--help` page is written by Fantomas instead of by Argu, and `-h` is accepted alongside `--help`. An argument error prints its complaint on standard error followed by a pointer to `--help`, where it used to print Argu's usage block.
 - `--out` now mirrors the structure of the input folder, and creates the folders it needs.
 

src/Fantomas.Tests/DiagnosticsTests.fs

@@ -0,0 +1,221 @@
+module Fantomas.Tests.DiagnosticsTests
+
+open NUnit.Framework
+open FsUnit
+open Fantomas
+open Fantomas.FCS.Diagnostics
+open Fantomas.FCS.Parse
+open Fantomas.FCS.Text
+
+// Diagnostics are constructed here rather than parsed from source on purpose: the messages and
+// numbers the parser produces move when the vendored compiler is bumped, and what these tests
+// are about is the rendering.
+let private diagnostic severity errorNumber message (startLine, startColumn) (endLine, endColumn) =
+    { Severity = severity
+      SubCategory = "parse"
+      Range = Some(Range.mkRange "tmp.fsx" (Position.mkPos startLine startColumn) (Position.mkPos endLine endColumn))
+      ErrorNumber = Some errorNumber
+      Message = message }
+
+let private error errorNumber message start finish =
+    diagnostic FSharpDiagnosticSeverity.Error errorNumber message start finish
+
+let private warning errorNumber message start finish =
+    diagnostic FSharpDiagnosticSeverity.Warning errorNumber message start finish
+
+let private source = "module A\n\nlet a = (1 + 2\n\nlet b = 2\nlet c = 3\n"
+
+let private lines (text: string) =
+    text.Replace("\r\n", "\n").Split('\n') |> Array.toList
+
+[<Test>]
+let ``a diagnostic is reported as an MSBuild style line with a one based column`` () =
+    let rendered =
+        Diagnostics.renderParseFailure "/tmp/bad.fs" "" [ error 583 "Unmatched '('" (3, 8) (3, 9) ]
+
+    lines rendered
+    |> should
+        equal
+        [ "Fantomas could not parse /tmp/bad.fs:"
+          ""
+          "/tmp/bad.fs(3,9): error FS0583: Unmatched '('"
+          "" ]
+
+[<Test>]
+let ``the file the caller names is reported, not the one the parser was handed`` () =
+    let rendered =
+        Diagnostics.renderParseFailure "/tmp/bad.fs" "" [ error 583 "Unmatched '('" (3, 8) (3, 9) ]
+
+    rendered |> should not' (contain "tmp.fsx")
+
+[<Test>]
+let ``diagnostics are ordered by position, not by the order the parser produced them`` () =
+    let rendered =
+        Diagnostics.renderParseFailure
+            "bad.fs"
+            ""
+            [ error 58 "Offside" (4, 0) (4, 3)
+              error 3118 "Incomplete value or function definition" (3, 0) (3, 3) ]
+
+    lines rendered
+    |> should
+        equal
+        [ "Fantomas could not parse bad.fs:"
+          ""
+          "bad.fs(3,1): error FS3118: Incomplete value or function definition"
+          "bad.fs(4,1): error FS0058: Offside"
+          "" ]
+
+[<Test>]
+let ``a warning keeps its severity`` () =
+    let rendered =
+        Diagnostics.renderParseFailure
+            "bad.fs"
+            ""
+            [ warning 1104 "Identifiers containing '@' are reserved" (2, 4) (2, 11) ]
+
+    rendered
+    |> should contain "bad.fs(2,5): warning FS1104: Identifiers containing '@' are reserved"
+
+[<Test>]
+let ``a message that spans lines is collapsed, so one diagnostic stays one line`` () =
+    let rendered =
+        Diagnostics.renderParseFailure "bad.fs" "" [ error 10 "First part.\nSecond part." (1, 0) (1, 1) ]
+
+    rendered |> should contain "bad.fs(1,1): error FS0010: First part. Second part."
+
+[<Test>]
+let ``a diagnostic without a range is still reported`` () =
+    let rendered =
+        Diagnostics.renderParseFailure
+            "bad.fs"
+            ""
+            [ { Severity = FSharpDiagnosticSeverity.Error
+                SubCategory = "parse"
+                Range = None
+                ErrorNumber = None
+                Message = "Something went wrong" } ]
+
+    rendered |> should contain "bad.fs: error FS0000: Something went wrong"
+
+[<Test>]
+let ``the snippet shows two lines either side with a caret under the range`` () =
+    let rendered =
+        Diagnostics.renderParseFailure "bad.fs" source [ error 583 "Unmatched '('" (3, 8) (3, 9) ]
+
+    lines rendered
+    |> should
+        equal
+        [ "Fantomas could not parse bad.fs:"
+          ""
+          "bad.fs(3,9): error FS0583: Unmatched '('"
+          ""
+          "1 | module A"
+          "2 | "
+          "3 | let a = (1 + 2"
+          "  |         ^"
+          "4 | "
+          "5 | let b = 2"
+          "" ]
+
+[<Test>]
+let ``the caret goes on the first error, not on a warning that sorts ahead of it`` () =
+    let rendered =
+        Diagnostics.renderParseFailure
+            "bad.fs"
+            source
+            [ warning 1104 "Reserved" (1, 0) (1, 6)
+              error 583 "Unmatched '('" (3, 8) (3, 9) ]
+
+    rendered |> should contain "3 | let a = (1 + 2"
+    rendered |> should contain "  |         ^"
+
+[<Test>]
+let ``the window is clipped at the start and the end of the file`` () =
+    let rendered =
+        Diagnostics.renderParseFailure "bad.fs" "let a = 1\n" [ error 10 "Unexpected" (1, 0) (1, 3) ]
+
+    lines rendered
+    |> should
+        equal
+        [ "Fantomas could not parse bad.fs:"
+          ""
+          "bad.fs(1,1): error FS0010: Unexpected"
+          ""
+          "1 | let a = 1"
+          "  | ^^^"
+          "2 | "
+          "" ]
+
+[<Test>]
+let ``tabs are expanded in the line and under the caret, so the two stay aligned`` () =
+    let rendered =
+        Diagnostics.renderParseFailure
+            "bad.fs"
+            "module A\n\n\tlet a = (1\n"
+            [ error 583 "Unmatched '('" (3, 9) (3, 10) ]
+
+    lines rendered
+    |> should
+        equal
+        [ "Fantomas could not parse bad.fs:"
+          ""
+          "bad.fs(3,10): error FS0583: Unmatched '('"
+          ""
+          "1 | module A"
+          "2 | "
+          "3 |     let a = (1"
+          "  |             ^"
+          "4 | "
+          "" ]
+
+[<Test>]
+let ``a tab inside the range widens the caret run by as much as it widened the line`` () =
+    let rendered =
+        Diagnostics.renderParseFailure "bad.fs" "module A\n\nlet a\t= 1\n" [ error 10 "Unexpected" (3, 3) (3, 7) ]
+
+    lines rendered
+    |> should
+        equal
+        [ "Fantomas could not parse bad.fs:"
+          ""
+          "bad.fs(3,4): error FS0010: Unexpected"
+          ""
+          "1 | module A"
+          "2 | "
+          "3 | let a    = 1"
+          "  |    ^^^^^^^"
+          "4 | "
+          "" ]
+
+[<Test>]
+let ``a range that runs past its first line is underlined to the end of that line`` () =
+    let rendered =
+        Diagnostics.renderParseFailure "bad.fs" source [ error 3118 "Incomplete" (3, 0) (5, 9) ]
+
+    rendered |> should contain "  | ^^^^^^^^^^^^^^"
+
+[<Test>]
+let ``a range beyond the end of the file leaves the snippet out rather than throwing`` () =
+    let rendered =
+        Diagnostics.renderParseFailure "bad.fs" "let a = 1\n" [ error 10 "Unexpected" (40, 0) (40, 3) ]
+
+    lines rendered
+    |> should
+        equal
+        [ "Fantomas could not parse bad.fs:"
+          ""
+          "bad.fs(40,1): error FS0010: Unexpected"
+          "" ]
+
+[<Test>]
+let ``without source there is no snippet`` () =
+    let rendered =
+        Diagnostics.renderParseFailure "bad.fs" "" [ error 583 "Unmatched '('" (3, 8) (3, 9) ]
+
+    lines rendered |> List.length |> should equal 4
+
+[<Test>]
+let ``an exception that is not a parse failure is not this module's to describe`` () =
+    Diagnostics.describeParseFailure "bad.fs" source (exn "boom")
+    |> should equal None

src/Fantomas.Tests/Fantomas.Tests.fsproj

@@ -16,6 +16,7 @@
   </ItemGroup>
   <ItemGroup>
     <Compile Include="TestHelpers.fs" />
+    <Compile Include="DiagnosticsTests.fs" />
     <Compile Include="CheckTests.fs" />
     <Compile Include="IgnoreFileTests.fs" />
     <Compile Include="EditorConfigurationTests.fs" />

src/Fantomas.Tests/Integration/ReportedPathTests.fs

@@ -62,7 +62,7 @@ let ``a single file that could not be parsed is reported with the path it was gi
     let { ExitCode = exitCode; Error = error } = formatCode [ path ]
 
     exitCode |> should equal 1
-    error |> should contain $"Failed to format file: %s{path}"
+    error |> should contain $"Fantomas could not parse %s{path}:"
 
 [<Test>]
 let ``a file that could not be parsed is reported the same way among several files`` () =
@@ -80,4 +80,4 @@ let ``a file that could not be parsed is reported the same way among several fil
         formatCode [ path; otherFixture.Filename ]
 
     exitCode |> should equal 1
-    error |> should contain $"Failed to format file: %s{path}"
+    error |> should contain $"Fantomas could not parse %s{path}:"

src/Fantomas.Tests/Integration/StandardStreamTests.fs

@@ -18,7 +18,7 @@ let ``errors are written to standard error and not to standard out`` () =
 
     exitCode |> should equal 1
     output |> should equal ""
-    Assert.That(error, Does.Contain "Could not parse the file.")
+    Assert.That(error, Does.Contain "error FS0010:")
 
 [<Test>]
 let ``progress messages are written to standard out and not to standard error`` () =
@@ -45,3 +45,31 @@ let ``version is written to standard out`` () =
     exitCode |> should equal 0
     Assert.That(output, Does.Contain "Fantomas v")
     error |> should equal ""
+
+// A parse failure is the one error an agent or a CI job can act on without opening the file,
+// provided it is told where the failure is.
+[<Test>]
+let ``a parse failure is reported with its position and the source around it`` () =
+    use fileFixture = new TemporaryFileCodeSample("module A\n\nlet a = (1 + 2\n")
+
+    let { ExitCode = exitCode
+          Output = output
+          Error = error } =
+        formatCode [ fileFixture.Filename ]
+
+    exitCode |> should equal 1
+    output |> should equal ""
+    Assert.That(error, Does.Contain $"%s{fileFixture.Filename}(3,9): error FS0583: Unmatched '('")
+    Assert.That(error, Does.Contain "3 | let a = (1 + 2")
+    Assert.That(error, Does.Contain "  |         ^")
+
+[<Test>]
+let ``--check reports a parse failure the same way a format run does`` () =
+    use fileFixture = new TemporaryFileCodeSample("module A\n\nlet a = (1 + 2\n")
+
+    let { ExitCode = exitCode; Error = error } =
+        runFantomasTool [ "--check"; fileFixture.Filename ]
+
+    exitCode |> should equal 1
+    Assert.That(error, Does.Contain $"%s{fileFixture.Filename}(3,9): error FS0583: Unmatched '('")
+    Assert.That(error, Does.Not.Contain "at Fantomas.Core.CodeFormatterImpl")

src/Fantomas/Daemon.fs

@@ -84,7 +84,13 @@ type FantomasDaemon(sender: Stream, reader: Stream) as this =
 
                         return FormatDocumentResponse.Formatted(request.FilePath, formatResponse.Code, cursor)
                 with ex ->
-                    return FormatDocumentResponse.Error(request.FilePath, ex.Message)
+                    // A ParseException's own Message is an %A dump of the diagnostic records, and
+                    // it is the editor's user who would have been shown it.
+                    let message =
+                        Diagnostics.describeParseFailure request.FilePath request.SourceCode ex
+                        |> Option.defaultValue ex.Message
+
+                    return FormatDocumentResponse.Error(request.FilePath, message)
         }
 
     [<JsonRpcMethod(Methods.FormatSelection, UseSingleObjectParameterDeserialization = true)>]
@@ -119,7 +125,11 @@ type FantomasDaemon(sender: Stream, reader: Stream) as this =
 
                 return FormatSelectionResponse.Formatted(request.FilePath, formatted, actualSelection)
             with ex ->
-                return FormatSelectionResponse.Error(request.FilePath, ex.Message)
+                let message =
+                    Diagnostics.describeParseFailure request.FilePath request.SourceCode ex
+                    |> Option.defaultValue ex.Message
+
+                return FormatSelectionResponse.Error(request.FilePath, message)
         }
 
     [<JsonRpcMethod(Methods.Configuration)>]