From e975e35d0b911de0a9abfd5e1ab58f96ae3feaf3 Mon Sep 17 00:00:00 2001 From: Smaug123 Date: Fri, 26 Jan 2024 11:07:24 +0000 Subject: [PATCH 1/2] Remove spurious option type --- src/FSharp.Analyzers.Cli/Program.fs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/FSharp.Analyzers.Cli/Program.fs b/src/FSharp.Analyzers.Cli/Program.fs index f347dab..c6c571f 100644 --- a/src/FSharp.Analyzers.Cli/Program.fs +++ b/src/FSharp.Analyzers.Cli/Program.fs @@ -171,12 +171,11 @@ let runProjectAux |> Async.Parallel return - Some - [ - for messages in messagesPerAnalyzer do - let mappedMessages = messages |> List.map (mapMessageToSeverity mappings) - yield! mappedMessages - ] + [ + for messages in messagesPerAnalyzer do + let mappedMessages = messages |> List.map (mapMessageToSeverity mappings) + yield! mappedMessages + ] } let runProject @@ -623,6 +622,7 @@ let main argv = | [], Some fscArgs -> runFscArgs client fscArgs exclInclFiles severityMapping |> Async.RunSynchronously + |> Some | projects, None -> for projPath in projects do if not (File.Exists(projPath)) then @@ -635,7 +635,6 @@ let main argv = ) |> Async.Sequential |> Async.RunSynchronously - |> Array.choose id |> List.concat |> Some From 25ab689f9285a81ff1c782e9ac62abed0eab785c Mon Sep 17 00:00:00 2001 From: Smaug123 Date: Fri, 26 Jan 2024 11:37:41 +0000 Subject: [PATCH 2/2] Fail on analysis failure --- .../FSharp.Analyzers.Cli.fsproj | 1 + src/FSharp.Analyzers.Cli/Program.fs | 87 +++++++++++-------- src/FSharp.Analyzers.Cli/Result.fs | 15 ++++ .../FSharp.Analyzers.SDK.Testing.fs | 4 +- .../FSharp.Analyzers.SDK.fs | 7 +- .../FSharp.Analyzers.SDK.fsi | 8 +- 6 files changed, 82 insertions(+), 40 deletions(-) create mode 100644 src/FSharp.Analyzers.Cli/Result.fs diff --git a/src/FSharp.Analyzers.Cli/FSharp.Analyzers.Cli.fsproj b/src/FSharp.Analyzers.Cli/FSharp.Analyzers.Cli.fsproj index eb2711a..9fd4150 100644 --- a/src/FSharp.Analyzers.Cli/FSharp.Analyzers.Cli.fsproj +++ b/src/FSharp.Analyzers.Cli/FSharp.Analyzers.Cli.fsproj @@ -14,6 +14,7 @@ + diff --git a/src/FSharp.Analyzers.Cli/Program.fs b/src/FSharp.Analyzers.Cli/Program.fs index c6c571f..ecadd79 100644 --- a/src/FSharp.Analyzers.Cli/Program.fs +++ b/src/FSharp.Analyzers.Cli/Program.fs @@ -12,6 +12,7 @@ open Microsoft.CodeAnalysis.Sarif open Microsoft.CodeAnalysis.Sarif.Writers open Microsoft.Extensions.Logging open Ionide.ProjInfo +open FSharp.Analyzers.Cli open FSharp.Analyzers.Cli.CustomLogging type Arguments = @@ -136,6 +137,7 @@ let runProjectAux (fsharpOptions: FSharpProjectOptions) (excludeIncludeFiles: Choice) (mappings: SeverityMappings) + : Async list> = async { let! checkProjectResults = fcs.ParseAndCheckProject(fsharpOptions) @@ -157,25 +159,33 @@ let runProjectAux true | None -> false ) - |> Array.choose (fun fileName -> + |> Array.map (fun fileName -> let fileContent = File.ReadAllText fileName let sourceText = SourceText.ofString fileContent Utils.typeCheckFile fcs logger fsharpOptions fileName (Utils.SourceOfSource.SourceText sourceText) - |> Option.map (Utils.createContext checkProjectResults fileName sourceText) + |> Result.map (Utils.createContext checkProjectResults fileName sourceText) ) |> Array.map (fun ctx -> - logger.LogInformation("Running analyzers for {0}", ctx.FileName) - client.RunAnalyzers ctx + match ctx with + | Error e -> async.Return(Error e) + | Ok ctx -> + async { + logger.LogInformation("Running analyzers for {0}", ctx.FileName) + let! results = client.RunAnalyzers ctx + return Ok results + } ) |> Async.Parallel return - [ - for messages in messagesPerAnalyzer do - let mappedMessages = messages |> List.map (mapMessageToSeverity mappings) - yield! mappedMessages - ] + messagesPerAnalyzer + |> Seq.map (fun messages -> + match messages with + | Error e -> Error e + | Ok messages -> messages |> List.map (mapMessageToSeverity mappings) |> Ok + ) + |> Seq.toList } let runProject @@ -266,7 +276,7 @@ let printMessages (msgs: AnalyzerMessage list) = let msgLogger = factory.CreateLogger("") msgs - |> Seq.iter (fun analyzerMessage -> + |> List.iter (fun analyzerMessage -> let m = analyzerMessage.Message msgLogger.Log( @@ -283,7 +293,7 @@ let printMessages (msgs: AnalyzerMessage list) = () -let writeReport (results: AnalyzerMessage list option) (codeRoot: string option) (report: string) = +let writeReport (results: AnalyzerMessage list) (codeRoot: string option) (report: string) = try let codeRoot = match codeRoot with @@ -318,7 +328,7 @@ let writeReport (results: AnalyzerMessage list option) (codeRoot: string option) sarifLogger.AnalysisStarted() - for analyzerResult in (Option.defaultValue List.empty results) do + for analyzerResult in results do let reportDescriptor = ReportingDescriptor() reportDescriptor.Id <- analyzerResult.Message.Code reportDescriptor.Name <- analyzerResult.Message.Message @@ -374,20 +384,6 @@ let writeReport (results: AnalyzerMessage list option) (codeRoot: string option) logger.LogError(ex, "Could not write sarif to {report}", report) logger.LogInformation("{0}", ex) -let calculateExitCode (msgs: AnalyzerMessage list option) : int = - match msgs with - | None -> -1 - | Some msgs -> - let check = - msgs - |> List.exists (fun analyzerMessage -> - let message = analyzerMessage.Message - - message.Severity = Severity.Error - ) - - if check then -2 else 0 - /// If multiple MSBuild properties are given in one -p flag like -p:prop1="val1a;val1b;val1c";prop2="1;2;3";prop3=val3 /// argu will think it means prop1 has the value: "val1a;val1b;val1c";prop2="1;2;3";prop3=val3 /// so this function expands the value into multiple key-value properties @@ -638,15 +634,36 @@ let main argv = |> List.concat |> Some - results |> Option.iter printMessages - report |> Option.iter (writeReport results codeRoot) + match results with + | None -> -1 + | Some results -> + let results, hasError = + match Result.allOkOrError results with + | Ok results -> results, false + | Error(results, _errors) -> results, true - if failedAssemblies > 0 then - logger.LogError( - "Because we failed to load some assemblies to obtain analyzers from them, exiting (failure count: {FailedAssemblyLoadCount})", - failedAssemblies - ) + let results = results |> List.concat + + printMessages results + + report |> Option.iter (writeReport results codeRoot) + + let check = + results + |> List.exists (fun analyzerMessage -> + let message = analyzerMessage.Message + + message.Severity = Severity.Error + ) + + if failedAssemblies > 0 then + logger.LogError( + "Because we failed to load some assemblies to obtain analyzers from them, exiting (failure count: {FailedAssemblyLoadCount})", + failedAssemblies + ) - exit -3 + exit -3 - calculateExitCode results + if check then -2 + elif hasError then -4 + else 0 diff --git a/src/FSharp.Analyzers.Cli/Result.fs b/src/FSharp.Analyzers.Cli/Result.fs new file mode 100644 index 0000000..f83daeb --- /dev/null +++ b/src/FSharp.Analyzers.Cli/Result.fs @@ -0,0 +1,15 @@ +module FSharp.Analyzers.Cli.Result + +let allOkOrError<'ok, 'err> (results: Result<'ok, 'err> list) : Result<'ok list, 'ok list * 'err list> = + let oks, errs = + (([], []), results) + ||> List.fold (fun (oks, errs) result -> + match result with + | Ok ok -> ok :: oks, errs + | Error err -> oks, err :: errs + ) + + let oks = List.rev oks + let errs = List.rev errs + + if List.isEmpty errs then Ok oks else Error(oks, errs) diff --git a/src/FSharp.Analyzers.SDK.Testing/FSharp.Analyzers.SDK.Testing.fs b/src/FSharp.Analyzers.SDK.Testing/FSharp.Analyzers.SDK.Testing.fs index 30b0762..064e949 100644 --- a/src/FSharp.Analyzers.SDK.Testing/FSharp.Analyzers.SDK.Testing.fs +++ b/src/FSharp.Analyzers.SDK.Testing/FSharp.Analyzers.SDK.Testing.fs @@ -257,7 +257,7 @@ let getContextFor (opts: FSharpProjectOptions) isSignature source = fileName (Utils.SourceOfSource.DiscreteSource source) with - | Some(parseFileResults, checkFileResults) -> + | Ok(parseFileResults, checkFileResults) -> let diagErrors = checkFileResults.Diagnostics |> Array.filter (fun d -> d.Severity = FSharpDiagnosticSeverity.Error) @@ -267,7 +267,7 @@ let getContextFor (opts: FSharpProjectOptions) isSignature source = let sourceText = SourceText.ofString source Utils.createContext checkProjectResults fileName sourceText (parseFileResults, checkFileResults) - | None -> failwith "typechecking file failed" + | Error e -> failwith $"typechecking file failed: %O{e}" let getContext (opts: FSharpProjectOptions) source = getContextFor opts false source let getContextForSignature (opts: FSharpProjectOptions) source = getContextFor opts true source diff --git a/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fs b/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fs index c0b9414..ada105d 100644 --- a/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fs +++ b/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fs @@ -176,6 +176,8 @@ type AnalyzerMessage = HelpUri: string option } +type AnalysisFailure = | Aborted + module Utils = let currentFSharpAnalyzersSDKVersion = Assembly.GetExecutingAssembly().GetName().Version @@ -230,6 +232,7 @@ module Utils = (options: FSharpProjectOptions) (fileName: string) (source: SourceOfSource) + : Result = let sourceText = @@ -247,5 +250,5 @@ module Utils = match checkAnswer with | FSharpCheckFileAnswer.Aborted -> logger.LogError("Checking of file {0} aborted", fileName) - None - | FSharpCheckFileAnswer.Succeeded result -> Some(parseRes, result) + Error AnalysisFailure.Aborted + | FSharpCheckFileAnswer.Succeeded result -> Ok(parseRes, result) diff --git a/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fsi b/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fsi index 5748001..6afca2e 100644 --- a/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fsi +++ b/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fsi @@ -152,6 +152,12 @@ type AnalyzerMessage = HelpUri: string option } +/// Represents a failure to run FSC analysis. +[] +type AnalysisFailure = + /// The F# compiler service aborted during analysis. + | Aborted + module Utils = [] @@ -171,7 +177,7 @@ module Utils = options: FSharpProjectOptions -> fileName: string -> source: SourceOfSource -> - option + Result val createContext: checkProjectResults: FSharpCheckProjectResults ->