Internal: simplify FSharpDiagnostics.CreateFromException (#18610)

Author: Martin521

docs/release-notes/.FSharp.Compiler.Service/10.0.100.md

@@ -20,3 +20,4 @@
 ### Breaking Changes
 
 * Scoped Nowarn: Add the #warnon compiler directive ([Language suggestion #278](https://github.com/fsharp/fslang-suggestions/issues/278), [RFC FS-1146 PR](https://github.com/fsharp/fslang-design/pull/782), [PR #18049](https://github.com/dotnet/fsharp/pull/18049) and [PR #18637](https://github.com/dotnet/fsharp/pull/18637))
+* Simplify creation of `FSharpDiagnostics`. In a few cases, errors without ranges were assigned to the currently checked file, while in other cases they carried an empty range. The latter is now true in all cases. In a few cases, ranges at eof were corrected, while in others they were not. They are now always left uncorrected. This is a prerequisit for [#18553](https://github.com/dotnet/fsharp/issues/18553). ([PR #18610](https://github.com/dotnet/fsharp/pull/18610)).

src/Compiler/Service/BackgroundCompiler.fs

@@ -1370,16 +1370,10 @@ type internal BackgroundCompiler
             scriptClosureCache.Set(AnyCallerThread, options, loadClosure) // Save the full load closure for later correlation.
 
             let diags =
+                let flatErrors = options.OtherOptions |> Array.contains "--flaterrors"
+
                 loadClosure.LoadClosureRootFileDiagnostics
-                |> List.map (fun (exn, isError) ->
-                    FSharpDiagnostic.CreateFromException(
-                        exn,
-                        isError,
-                        range.Zero,
-                        false,
-                        options.OtherOptions |> Array.contains "--flaterrors",
-                        None
-                    ))
+                |> List.map (fun (exn, isError) -> FSharpDiagnostic.CreateFromException(exn, isError, false, flatErrors, None))
 
             return options, (diags @ diagnostics.Diagnostics)
         }

src/Compiler/Service/FSharpCheckerResults.fs

@@ -2797,21 +2797,11 @@ module internal ParseAndCheckFile =
 
     /// Diagnostics handler for parsing & type checking while processing a single file
     type DiagnosticsHandler
-        (
-            reportErrors,
-            mainInputFileName,
-            diagnosticsOptions: FSharpDiagnosticOptions,
-            sourceText: ISourceText,
-            suggestNamesForErrors: bool,
-            flatErrors: bool
-        ) =
+        (reportErrors, mainInputFileName, diagnosticsOptions: FSharpDiagnosticOptions, suggestNamesForErrors: bool, flatErrors: bool) =
         let mutable options = diagnosticsOptions
         let diagnosticsCollector = ResizeArray<_>()
         let mutable errorCount = 0
 
-        // We'll need number of lines for adjusting error messages at EOF
-        let fileInfo = sourceText.GetLastCharacterPosition()
-
         let collectOne severity diagnostic =
             // 1. Extended diagnostic data should be created after typechecking because it requires a valid SymbolEnv
             // 2. Diagnostic message should be created during the diagnostic sink, because after typechecking
@@ -2881,7 +2871,6 @@ module internal ParseAndCheckFile =
                             options,
                             false,
                             mainInputFileName,
-                            fileInfo,
                             diagnostic,
                             severity,
                             suggestNamesForErrors,
@@ -2960,7 +2949,7 @@ module internal ParseAndCheckFile =
 
         usingLexbufForParsing (createLexbuf options.LangVersionText options.StrictIndentation sourceText, fileName) (fun lexbuf ->
             let errHandler =
-                DiagnosticsHandler(false, fileName, options.DiagnosticOptions, sourceText, suggestNamesForErrors, false)
+                DiagnosticsHandler(false, fileName, options.DiagnosticOptions, suggestNamesForErrors, false)
 
             let lexfun = createLexerFunction fileName options lexbuf errHandler ct
 
@@ -3064,7 +3053,7 @@ module internal ParseAndCheckFile =
             Activity.start "ParseAndCheckFile.parseFile" [| Activity.Tags.fileName, fileName |]
 
         let errHandler =
-            DiagnosticsHandler(true, fileName, options.DiagnosticOptions, sourceText, suggestNamesForErrors, flatErrors)
+            DiagnosticsHandler(true, fileName, options.DiagnosticOptions, suggestNamesForErrors, flatErrors)
 
         use _ = UseDiagnosticsLogger errHandler.DiagnosticsLogger
 
@@ -3233,14 +3222,7 @@ module internal ParseAndCheckFile =
 
             // Initialize the error handler
             let errHandler =
-                DiagnosticsHandler(
-                    true,
-                    mainInputFileName,
-                    tcConfig.diagnosticsOptions,
-                    sourceText,
-                    suggestNamesForErrors,
-                    tcConfig.flatErrors
-                )
+                DiagnosticsHandler(true, mainInputFileName, tcConfig.diagnosticsOptions, suggestNamesForErrors, tcConfig.flatErrors)
 
             use _ = UseDiagnosticsLogger errHandler.DiagnosticsLogger
 

src/Compiler/Service/FSharpCheckerResults.fsi

@@ -593,7 +593,6 @@ module internal ParseAndCheckFile =
             reportErrors: bool *
             mainInputFileName: string *
             diagnosticsOptions: FSharpDiagnosticOptions *
-            sourceText: ISourceText *
             suggestNamesForErrors: bool *
             flatErrors: bool ->
                 DiagnosticsHandler

src/Compiler/Service/IncrementalBuild.fs

@@ -1666,7 +1666,7 @@ type IncrementalBuilder(initialState: IncrementalBuilderInitialState, state: Inc
                     Array.ofList delayedLogger.Diagnostics, false
             diagnostics
             |> Array.map (fun (diagnostic, severity) ->
-                FSharpDiagnostic.CreateFromException(diagnostic, severity, range.Zero, suggestNamesForErrors, flatErrors, None))
+                FSharpDiagnostic.CreateFromException(diagnostic, severity, suggestNamesForErrors, flatErrors, None))
 
         return builderOpt, diagnostics
       }

src/Compiler/Service/TransparentCompiler.fs

@@ -1128,7 +1128,7 @@ type internal TransparentCompiler
                             |> Option.map (fun bootstrapInfo -> bootstrapInfo.TcConfig.flatErrors)
                             |> Option.defaultValue false // TODO: do we need to figure this out?
 
-                        FSharpDiagnostic.CreateFromException(diagnostic, severity, range.Zero, suggestNamesForErrors, flatErrors, None))
+                        FSharpDiagnostic.CreateFromException(diagnostic, severity, suggestNamesForErrors, flatErrors, None))
 
                 return bootstrapInfoOpt, diagnostics
             }
@@ -1382,15 +1382,13 @@ type internal TransparentCompiler
                 let tcImports = bootstrapInfo.TcImports
 
                 let mainInputFileName = file.FileName
-                let sourceText = file.SourceText
 
                 // Initialize the error handler
                 let errHandler =
                     ParseAndCheckFile.DiagnosticsHandler(
                         true,
                         mainInputFileName,
                         tcConfig.diagnosticsOptions,
-                        sourceText,
                         suggestNamesForErrors,
                         tcConfig.flatErrors
                     )
@@ -2466,16 +2464,10 @@ type internal TransparentCompiler
                     )
 
                 let diags =
+                    let flaterrors = otherFlags |> List.contains "--flaterrors"
+
                     loadClosure.LoadClosureRootFileDiagnostics
-                    |> List.map (fun (exn, isError) ->
-                        FSharpDiagnostic.CreateFromException(
-                            exn,
-                            isError,
-                            range.Zero,
-                            false,
-                            otherFlags |> List.contains "--flaterrors",
-                            None
-                        ))
+                    |> List.map (fun (exn, isError) -> FSharpDiagnostic.CreateFromException(exn, isError, false, flaterrors, None))
 
                 return snapshot, (diags @ diagnostics.Diagnostics)
             }

src/Compiler/Service/service.fs

@@ -49,7 +49,7 @@ module CompileHelpers =
             { new DiagnosticsLogger("CompileAPI") with
 
                 member _.DiagnosticSink(diag, isError) =
-                    diagnostics.Add(FSharpDiagnostic.CreateFromException(diag, isError, range0, true, flatErrors, None)) // Suggest names for errors
+                    diagnostics.Add(FSharpDiagnostic.CreateFromException(diag, isError, true, flatErrors, None)) // Suggest names for errors
 
                 member _.ErrorCount =
                     diagnostics

src/Compiler/Symbols/FSharpDiagnostic.fs

@@ -191,8 +191,7 @@ type FSharpDiagnostic(m: range, severity: FSharpDiagnosticSeverity, message: str
         sprintf "%s (%d,%d)-(%d,%d) %s %s %s" fileName s.Line (s.Column + 1) e.Line (e.Column + 1) subcategory severity message
 
     /// Decompose a warning or error into parts: position, severity, message, error number
-    static member CreateFromException(diagnostic: PhasedDiagnostic, severity, fallbackRange: range, suggestNames: bool, flatErrors: bool, symbolEnv: SymbolEnv option) =
-        let m = match diagnostic.Range with Some m -> m | None -> fallbackRange
+    static member CreateFromException(diagnostic: PhasedDiagnostic, severity, suggestNames: bool, flatErrors: bool, symbolEnv: SymbolEnv option) =
         let extendedData: IFSharpDiagnosticExtendedData option =
             match symbolEnv with
             | None -> None
@@ -236,22 +235,9 @@ type FSharpDiagnostic(m: range, severity: FSharpDiagnosticSeverity, message: str
              | _ -> diagnostic.FormatCore(flatErrors, suggestNames)
 
         let errorNum = diagnostic.Number
+        let m = match diagnostic.Range with Some m -> m | None -> range.Zero
         FSharpDiagnostic(m, severity, msg, diagnostic.Subcategory(), errorNum, "FS", extendedData)
 
-    /// Decompose a warning or error into parts: position, severity, message, error number
-    static member CreateFromExceptionAndAdjustEof(diagnostic, severity, fallbackRange: range, (linesCount: int, lastLength: int), suggestNames: bool, flatErrors: bool, symbolEnv: SymbolEnv option) =
-        let diagnostic = FSharpDiagnostic.CreateFromException(diagnostic, severity, fallbackRange, suggestNames, flatErrors, symbolEnv)
-
-        // Adjust to make sure that diagnostics reported at Eof are shown at the linesCount
-        let startLine, startChanged = min (Line.toZ diagnostic.Range.StartLine, false) (linesCount, true)
-        let endLine, endChanged = min (Line.toZ diagnostic.Range.EndLine, false)  (linesCount, true)
-        
-        if not (startChanged || endChanged) then
-            diagnostic
-        else
-            let r = if startChanged then diagnostic.WithStart(mkPos startLine lastLength) else diagnostic
-            if endChanged then r.WithEnd(mkPos endLine (1 + lastLength)) else r
-
     static member NewlineifyErrorString(message) = NewlineifyErrorString(message)
 
     static member NormalizeErrorString(text) = NormalizeErrorString(text)
@@ -271,7 +257,7 @@ type DiagnosticsScope(flatErrors: bool)  =
             { new DiagnosticsLogger("DiagnosticsScope") with 
 
                 member _.DiagnosticSink(diagnostic, severity) = 
-                    let diagnostic = FSharpDiagnostic.CreateFromException(diagnostic, severity, range.Zero, false, flatErrors, None)
+                    let diagnostic = FSharpDiagnostic.CreateFromException(diagnostic, severity, false, flatErrors, None)
                     diags <- diagnostic :: diags
 
                 member _.ErrorCount = diags.Length }
@@ -344,21 +330,17 @@ type internal CompilationDiagnosticLogger (debugName: string, options: FSharpDia
 
 module DiagnosticHelpers =                            
 
-    let ReportDiagnostic (options: FSharpDiagnosticOptions, allErrors, mainInputFileName, fileInfo, diagnostic: PhasedDiagnostic, severity, suggestNames, flatErrors, symbolEnv) =
+    let ReportDiagnostic (options: FSharpDiagnosticOptions, allErrors, mainInputFileName, diagnostic: PhasedDiagnostic, severity, suggestNames, flatErrors, symbolEnv) =
         match diagnostic.AdjustSeverity(options, severity) with
         | FSharpDiagnosticSeverity.Hidden -> []
         | adjustedSeverity ->
 
-            // We use the first line of the file as a fallbackRange for reporting unexpected errors.
-            // Not ideal, but it's hard to see what else to do.
-            let fallbackRange = rangeN mainInputFileName 1
-            let diagnostic = FSharpDiagnostic.CreateFromExceptionAndAdjustEof (diagnostic, adjustedSeverity, fallbackRange, fileInfo, suggestNames, flatErrors, symbolEnv)
+            let diagnostic = FSharpDiagnostic.CreateFromException (diagnostic, adjustedSeverity, suggestNames, flatErrors, symbolEnv)
             let fileName = diagnostic.Range.FileName
             if allErrors || fileName = mainInputFileName || fileName = TcGlobals.DummyFileNameForRangesWithoutASpecificLocation then
                 [diagnostic]
             else []
 
     let CreateDiagnostics (options, allErrors, mainInputFileName, diagnostics, suggestNames, flatErrors, symbolEnv) =
-        let fileInfo = (Int32.MaxValue, Int32.MaxValue)
         [| for diagnostic, severity in diagnostics do 
-              yield! ReportDiagnostic (options, allErrors, mainInputFileName, fileInfo, diagnostic, severity, suggestNames, flatErrors, symbolEnv) |]
+              yield! ReportDiagnostic (options, allErrors, mainInputFileName, diagnostic, severity, suggestNames, flatErrors, symbolEnv) |]

src/Compiler/Symbols/FSharpDiagnostic.fsi

@@ -191,20 +191,9 @@ type public FSharpDiagnostic =
         ?subcategory: string ->
             FSharpDiagnostic
 
-    static member internal CreateFromExceptionAndAdjustEof:
-        diagnostic: PhasedDiagnostic *
-        severity: FSharpDiagnosticSeverity *
-        range *
-        lastPosInFile: (int * int) *
-        suggestNames: bool *
-        flatErrors: bool *
-        symbolEnv: SymbolEnv option ->
-            FSharpDiagnostic
-
     static member internal CreateFromException:
         diagnostic: PhasedDiagnostic *
         severity: FSharpDiagnosticSeverity *
-        range *
         suggestNames: bool *
         flatErrors: bool *
         symbolEnv: SymbolEnv option ->
@@ -251,7 +240,6 @@ module internal DiagnosticHelpers =
         FSharpDiagnosticOptions *
         allErrors: bool *
         mainInputFileName: string *
-        fileInfo: (int * int) *
         diagnostic: PhasedDiagnostic *
         severity: FSharpDiagnosticSeverity *
         suggestNames: bool *

tests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/codepage/codepage.fs

@@ -64,7 +64,7 @@ module Codepage =
         |> compile
         |> shouldFail
         |> withDiagnostics [
-            (Error 193, Line 1, Col 1, Line 1, Col 1, "No data is available for encoding 65535. For information on defining a custom encoding, see the documentation for the Encoding.RegisterProvider method.")
+            (Error 193, Line 0, Col 1, Line 0, Col 1, "No data is available for encoding 65535. For information on defining a custom encoding, see the documentation for the Encoding.RegisterProvider method.")
         ]
 
     //# Boundary case