From 4c7918821ced60b9c76e50f5176bc17658159223 Mon Sep 17 00:00:00 2001 From: Muqiu Han Date: Mon, 5 Aug 2024 16:06:31 +0800 Subject: [PATCH 1/3] Diagnostics: Clearer information style * Add file content to message. * Use more symbols and line breaks to separate messages. --- src/Compiler/Driver/CompilerDiagnostics.fs | 40 ++++++++++++++++++--- src/Compiler/Driver/CompilerDiagnostics.fsi | 3 +- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/Compiler/Driver/CompilerDiagnostics.fs b/src/Compiler/Driver/CompilerDiagnostics.fs index 5ac8f448448..d779ebb0292 100644 --- a/src/Compiler/Driver/CompilerDiagnostics.fs +++ b/src/Compiler/Driver/CompilerDiagnostics.fs @@ -2039,6 +2039,7 @@ type FormattedDiagnosticDetailedInfo = Location: FormattedDiagnosticLocation option Canonical: FormattedDiagnosticCanonicalInformation Message: string + Context: string option } [] @@ -2046,7 +2047,7 @@ type FormattedDiagnostic = | Short of FSharpDiagnosticSeverity * string | Long of FSharpDiagnosticSeverity * FormattedDiagnosticDetailedInfo -let FormatDiagnosticLocation (tcConfig: TcConfig) m : FormattedDiagnosticLocation = +let FormatDiagnosticLocation (tcConfig: TcConfig) (m: Range) : FormattedDiagnosticLocation = if equals m rangeStartup || equals m rangeCmdArgs then { Range = m @@ -2073,7 +2074,7 @@ let FormatDiagnosticLocation (tcConfig: TcConfig) m : FormattedDiagnosticLocatio | DiagnosticStyle.Default -> let file = file.Replace('/', Path.DirectorySeparatorChar) let m = withStart (mkPos m.StartLine (m.StartColumn + 1)) m - (sprintf "%s(%d,%d): " file m.StartLine m.StartColumn), m, file + (sprintf "◦→ %s (%d,%d)" file m.StartLine m.StartColumn), m, file // We may also want to change Test to be 1-based | DiagnosticStyle.Test -> @@ -2150,7 +2151,7 @@ let CollectFormattedDiagnostics (tcConfig: TcConfig, severity: FSharpDiagnosticS match tcConfig.diagnosticStyle with // Show the subcategory for --vserrors so that we can fish it out in Visual Studio and use it to determine error stickiness. | DiagnosticStyle.VisualStudio -> sprintf "%s %s FS%04d: " subcategory message errorNumber - | _ -> sprintf "%s FS%04d: " message errorNumber + | _ -> sprintf "\n◦→ %s FS%04d: " message errorNumber let canonical: FormattedDiagnosticCanonicalInformation = { @@ -2159,11 +2160,37 @@ let CollectFormattedDiagnostics (tcConfig: TcConfig, severity: FSharpDiagnosticS TextRepresentation = text } - let message = diagnostic.FormatCore(tcConfig.flatErrors, suggestNames) + let message = + diagnostic.FormatCore(tcConfig.flatErrors, suggestNames).Split([| '\n' |]) + |> Array.map (fun msg -> "\n◦ " + msg) + |> String.Concat + + let context = + match diagnostic.Range with + | Some m -> + let content = + m.FileName + |> FileSystem.GetFullFilePathInDirectoryShim tcConfig.implicitIncludeDir + |> System.IO.File.ReadAllLines + + if m.StartLine = m.EndLine then + $"\n◦ {m.StartLine} |{content[m.StartLine - 1]}\n" + + $"""◦ {String.init (m.StartColumn + 3) (fun _ -> " ")}^{String.init (m.EndColumn - m.StartColumn) (fun _ -> "~")}""" + |> Some + else + None + // Untested multi-line support: + // content + // |> fun lines -> Array.sub lines (m.StartLine - 1) (m.EndLine - m.StartLine - 1) + // |> Array.fold (fun (context, lineNumber) line -> (context + $"\n◦ {lineNumber} |{line}", lineNumber + 1)) ("", (m.StartLine)) + // |> fst + // |> Some + | None -> None let entry: FormattedDiagnosticDetailedInfo = { Location = where + Context = context Canonical = canonical Message = message } @@ -2194,7 +2221,10 @@ type PhasedDiagnostic with | FormattedDiagnostic.Short(_, txt) -> buf.AppendString txt | FormattedDiagnostic.Long(_, details) -> match details.Location with - | Some l when not l.IsEmpty -> buf.AppendString l.TextRepresentation + | Some l when not l.IsEmpty -> + buf.AppendString l.TextRepresentation + // Because details.Context depends on the value of details.Location, if details.Location is not None, details.Context can be accessed directly. + buf.AppendString details.Context.Value | _ -> () buf.AppendString details.Canonical.TextRepresentation diff --git a/src/Compiler/Driver/CompilerDiagnostics.fsi b/src/Compiler/Driver/CompilerDiagnostics.fsi index 8e0890d4418..729245f81d9 100644 --- a/src/Compiler/Driver/CompilerDiagnostics.fsi +++ b/src/Compiler/Driver/CompilerDiagnostics.fsi @@ -113,7 +113,8 @@ type FormattedDiagnosticCanonicalInformation = type FormattedDiagnosticDetailedInfo = { Location: FormattedDiagnosticLocation option Canonical: FormattedDiagnosticCanonicalInformation - Message: string } + Message: string + Context: string option } /// Used internally and in LegacyHostedCompilerForTesting [] From 8d26aeb48bac6fcef4951282a6cb4688670d6ee0 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Mon, 26 Aug 2024 17:25:24 +0200 Subject: [PATCH 2/3] Added flag --- src/Compiler/Driver/CompilerDiagnostics.fs | 78 ++++++++++++------- src/Compiler/Facilities/DiagnosticsLogger.fs | 3 +- src/Compiler/Facilities/DiagnosticsLogger.fsi | 3 +- 3 files changed, 55 insertions(+), 29 deletions(-) diff --git a/src/Compiler/Driver/CompilerDiagnostics.fs b/src/Compiler/Driver/CompilerDiagnostics.fs index a8e92b9b5c8..afcc06aed11 100644 --- a/src/Compiler/Driver/CompilerDiagnostics.fs +++ b/src/Compiler/Driver/CompilerDiagnostics.fs @@ -2079,7 +2079,7 @@ let FormatDiagnosticLocation (tcConfig: TcConfig) (m: Range) : FormattedDiagnost | DiagnosticStyle.Default -> let file = file.Replace('/', Path.DirectorySeparatorChar) let m = withStart (mkPos m.StartLine (m.StartColumn + 1)) m - (sprintf "◦→ %s (%d,%d)" file m.StartLine m.StartColumn), m, file + (sprintf "%s(%d,%d): " file m.StartLine m.StartColumn), m, file // We may also want to change Test to be 1-based | DiagnosticStyle.Test -> @@ -2115,6 +2115,10 @@ let FormatDiagnosticLocation (tcConfig: TcConfig) (m: Range) : FormattedDiagnost sprintf "%s(%d,%d,%d,%d): " file m.StartLine m.StartColumn m.EndLine m.EndColumn, m, file else "", m, file + | DiagnosticStyle.Rich -> + let file = file.Replace('/', Path.DirectorySeparatorChar) + let m = withStart (mkPos m.StartLine (m.StartColumn + 1)) m + (sprintf "◦→ %s (%d,%d)" file m.StartLine m.StartColumn), m, file { Range = m @@ -2155,8 +2159,15 @@ let CollectFormattedDiagnostics (tcConfig: TcConfig, severity: FSharpDiagnosticS let text = match tcConfig.diagnosticStyle with // Show the subcategory for --vserrors so that we can fish it out in Visual Studio and use it to determine error stickiness. - | DiagnosticStyle.VisualStudio -> sprintf "%s %s FS%04d: " subcategory message errorNumber - | _ -> sprintf "\n◦→ %s FS%04d: " message errorNumber + | DiagnosticStyle.Emacs + | DiagnosticStyle.Gcc + | DiagnosticStyle.Default + | DiagnosticStyle.Test -> + sprintf "%s FS%04d: " message errorNumber + | DiagnosticStyle.VisualStudio -> + sprintf "%s %s FS%04d: " subcategory message errorNumber + | DiagnosticStyle.Rich -> + sprintf "\n◦→ %s FS%04d: " message errorNumber let canonical: FormattedDiagnosticCanonicalInformation = { @@ -2166,31 +2177,43 @@ let CollectFormattedDiagnostics (tcConfig: TcConfig, severity: FSharpDiagnosticS } let message = - diagnostic.FormatCore(tcConfig.flatErrors, suggestNames).Split([| '\n' |]) - |> Array.map (fun msg -> "\n◦ " + msg) - |> String.Concat + match tcConfig.diagnosticStyle with + | DiagnosticStyle.Emacs + | DiagnosticStyle.Gcc + | DiagnosticStyle.Default + | DiagnosticStyle.Test + | DiagnosticStyle.VisualStudio -> diagnostic.FormatCore(tcConfig.flatErrors, suggestNames) + | DiagnosticStyle.Rich -> + diagnostic.FormatCore(tcConfig.flatErrors, suggestNames).Split([| '\n' |]) + |> Array.map (fun msg -> "\n◦ " + msg) + |> String.Concat let context = - match diagnostic.Range with - | Some m -> - let content = - m.FileName - |> FileSystem.GetFullFilePathInDirectoryShim tcConfig.implicitIncludeDir - |> System.IO.File.ReadAllLines - - if m.StartLine = m.EndLine then - $"\n◦ {m.StartLine} |{content[m.StartLine - 1]}\n" - + $"""◦ {String.init (m.StartColumn + 3) (fun _ -> " ")}^{String.init (m.EndColumn - m.StartColumn) (fun _ -> "~")}""" - |> Some - else - None - // Untested multi-line support: - // content - // |> fun lines -> Array.sub lines (m.StartLine - 1) (m.EndLine - m.StartLine - 1) - // |> Array.fold (fun (context, lineNumber) line -> (context + $"\n◦ {lineNumber} |{line}", lineNumber + 1)) ("", (m.StartLine)) - // |> fst - // |> Some - | None -> None + match tcConfig.diagnosticStyle with + | DiagnosticStyle.Emacs + | DiagnosticStyle.Gcc + | DiagnosticStyle.Default + | DiagnosticStyle.Test + | DiagnosticStyle.VisualStudio -> None + | DiagnosticStyle.Rich -> + match diagnostic.Range with + | Some m -> + let content = + m.FileName + |> FileSystem.GetFullFilePathInDirectoryShim tcConfig.implicitIncludeDir + |> System.IO.File.ReadAllLines + + if m.StartLine = m.EndLine then + $"\n◦ {m.StartLine} |{content[m.StartLine - 1]}\n" + + $"""◦ {String.init (m.StartColumn + 3) (fun _ -> " ")}^{String.init (m.EndColumn - m.StartColumn) (fun _ -> "~")}""" + |> Some + else + content + |> fun lines -> Array.sub lines (m.StartLine - 1) (m.EndLine - m.StartLine - 1) + |> Array.fold (fun (context, lineNumber) line -> (context + $"\n◦ {lineNumber} |{line}", lineNumber + 1)) ("", (m.StartLine)) + |> fst + |> Some + | None -> None let entry: FormattedDiagnosticDetailedInfo = { @@ -2229,7 +2252,8 @@ type PhasedDiagnostic with | Some l when not l.IsEmpty -> buf.AppendString l.TextRepresentation // Because details.Context depends on the value of details.Location, if details.Location is not None, details.Context can be accessed directly. - buf.AppendString details.Context.Value + if details.Context.IsSome then + buf.AppendString details.Context.Value | _ -> () buf.AppendString details.Canonical.TextRepresentation diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fs b/src/Compiler/Facilities/DiagnosticsLogger.fs index af19ae2f617..84e1cb55360 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fs +++ b/src/Compiler/Facilities/DiagnosticsLogger.fs @@ -18,13 +18,14 @@ open Internal.Utilities.Library.Extras open System.Threading.Tasks /// Represents the style being used to format errors -[] +[] type DiagnosticStyle = | Default | Emacs | Test | VisualStudio | Gcc + | Rich /// Thrown when we want to add some range information to a .NET exception exception WrappedError of exn * range with diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fsi b/src/Compiler/Facilities/DiagnosticsLogger.fsi index 0f9b2432403..6cf3a5f2184 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fsi +++ b/src/Compiler/Facilities/DiagnosticsLogger.fsi @@ -10,13 +10,14 @@ open System.Runtime.CompilerServices open System.Runtime.InteropServices /// Represents the style being used to format errors -[] +[] type DiagnosticStyle = | Default | Emacs | Test | VisualStudio | Gcc + | Rich /// Thrown when we want to add some range information to a .NET exception exception WrappedError of exn * range From bc16d11b7941456b8a737cd1f1ee055f9d3f32e2 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Mon, 26 Aug 2024 17:30:39 +0200 Subject: [PATCH 3/3] Fantomas --- src/Compiler/Driver/CompilerDiagnostics.fs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Compiler/Driver/CompilerDiagnostics.fs b/src/Compiler/Driver/CompilerDiagnostics.fs index afcc06aed11..df5a27cabbe 100644 --- a/src/Compiler/Driver/CompilerDiagnostics.fs +++ b/src/Compiler/Driver/CompilerDiagnostics.fs @@ -2162,12 +2162,9 @@ let CollectFormattedDiagnostics (tcConfig: TcConfig, severity: FSharpDiagnosticS | DiagnosticStyle.Emacs | DiagnosticStyle.Gcc | DiagnosticStyle.Default - | DiagnosticStyle.Test -> - sprintf "%s FS%04d: " message errorNumber - | DiagnosticStyle.VisualStudio -> - sprintf "%s %s FS%04d: " subcategory message errorNumber - | DiagnosticStyle.Rich -> - sprintf "\n◦→ %s FS%04d: " message errorNumber + | DiagnosticStyle.Test -> sprintf "%s FS%04d: " message errorNumber + | DiagnosticStyle.VisualStudio -> sprintf "%s %s FS%04d: " subcategory message errorNumber + | DiagnosticStyle.Rich -> sprintf "\n◦→ %s FS%04d: " message errorNumber let canonical: FormattedDiagnosticCanonicalInformation = { @@ -2210,7 +2207,9 @@ let CollectFormattedDiagnostics (tcConfig: TcConfig, severity: FSharpDiagnosticS else content |> fun lines -> Array.sub lines (m.StartLine - 1) (m.EndLine - m.StartLine - 1) - |> Array.fold (fun (context, lineNumber) line -> (context + $"\n◦ {lineNumber} |{line}", lineNumber + 1)) ("", (m.StartLine)) + |> Array.fold + (fun (context, lineNumber) line -> (context + $"\n◦ {lineNumber} |{line}", lineNumber + 1)) + ("", (m.StartLine)) |> fst |> Some | None -> None