From 28b4989d93cb06969604d75e87302e6104588862 Mon Sep 17 00:00:00 2001 From: Smaug123 Date: Thu, 25 Jan 2024 20:23:52 +0000 Subject: [PATCH 1/2] Fail the tool on failure to load analyzers --- src/FSharp.Analyzers.Cli/Program.fs | 21 ++++++++++---- .../FSharp.Analyzers.SDK.Testing.fs | 9 ++++-- .../FSharp.Analyzers.SDK.Client.fs | 29 +++++++++++++++++-- .../FSharp.Analyzers.SDK.Client.fsi | 12 +++++++- 4 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/FSharp.Analyzers.Cli/Program.fs b/src/FSharp.Analyzers.Cli/Program.fs index 65fa85b..6931e68 100644 --- a/src/FSharp.Analyzers.Cli/Program.fs +++ b/src/FSharp.Analyzers.Cli/Program.fs @@ -596,11 +596,14 @@ let main argv = let client = Client(logger) - let dlls, analyzers = - ((0, 0), analyzersPaths) - ||> List.fold (fun (accDlls, accAnalyzers) analyzersPath -> - let dlls, analyzers = client.LoadAnalyzers(analyzersPath, exclInclAnalyzers) - (accDlls + dlls), (accAnalyzers + analyzers) + let dlls, analyzers, failedAssemblies = + ((0, 0, 0), analyzersPaths) + ||> List.fold (fun (accDlls, accAnalyzers, accFailed) analyzersPath -> + let loadedDlls = client.LoadAnalyzers(analyzersPath, exclInclAnalyzers) + + (accDlls + loadedDlls.AnalyzerAssemblies), + (accAnalyzers + loadedDlls.Analyzers), + (accFailed + loadedDlls.FailedAssemblies) ) logger.LogInformation("Registered {0} analyzers from {1} dlls", analyzers, dlls) @@ -639,4 +642,12 @@ let main argv = results |> Option.iter printMessages report |> Option.iter (writeReport results codeRoot) + 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 + calculateExitCode results 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 6769c5e..30b0762 100644 --- a/src/FSharp.Analyzers.SDK.Testing/FSharp.Analyzers.SDK.Testing.fs +++ b/src/FSharp.Analyzers.SDK.Testing/FSharp.Analyzers.SDK.Testing.fs @@ -224,16 +224,19 @@ let getContextFor (opts: FSharpProjectOptions) isSignature source = let fcs = Utils.createFCS (Some documentSource) let pathToAnalyzerDlls = Path.GetFullPath(".") - let foundDlls, registeredAnalyzers = + let assemblyLoadStats = let client = Client() client.LoadAnalyzers pathToAnalyzerDlls - if foundDlls = 0 then + if assemblyLoadStats.AnalyzerAssemblies = 0 then failwith $"no Dlls found in {pathToAnalyzerDlls}" - if registeredAnalyzers = 0 then + if assemblyLoadStats.Analyzers = 0 then failwith $"no Analyzers found in {pathToAnalyzerDlls}" + if assemblyLoadStats.FailedAssemblies > 0 then + failwith $"failed to load %i{assemblyLoadStats.FailedAssemblies} Analyzers in {pathToAnalyzerDlls}" + let opts = { opts with SourceFiles = [| fileName |] diff --git a/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fs b/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fs index 6ac5a39..7f28016 100644 --- a/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fs +++ b/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fs @@ -127,6 +127,13 @@ module Client = |> Seq.choose (analyzerFromMember<'TAnalyzerAttribute, 'TContext> path) |> Seq.toList +type AssemblyLoadStats = + { + AnalyzerAssemblies: int + Analyzers: int + FailedAssemblies: int + } + type ExcludeInclude = | ExcludeFilter of (string -> bool) | IncludeFilter of (string -> bool) @@ -139,7 +146,7 @@ type Client<'TAttribute, 'TContext when 'TAttribute :> AnalyzerAttribute and 'TC new() = Client(Abstractions.NullLogger.Instance) - member x.LoadAnalyzers(dir: string, ?excludeInclude: ExcludeInclude) : int * int = + member x.LoadAnalyzers(dir: string, ?excludeInclude: ExcludeInclude) : AssemblyLoadStats = if Directory.Exists dir then let analyzerAssemblies = let regex = Regex(@".*test.*\.dll$") @@ -174,6 +181,8 @@ type Client<'TAttribute, 'TContext when 'TAttribute :> AnalyzerAttribute and 'TC let fas = references |> Array.find (fun ra -> ra.Name = "FSharp.Analyzers.SDK") fas.Version + let skippedAssemblies = ref 0 + let analyzers = analyzerAssemblies |> Array.filter (fun (name, analyzerAssembly) -> @@ -185,6 +194,8 @@ type Client<'TAttribute, 'TContext when 'TAttribute :> AnalyzerAttribute and 'TC then true else + System.Threading.Interlocked.Increment skippedAssemblies |> ignore + logger.LogError( "Trying to load {Name} which was built using SDK version {Version}. Expect {SdkVersion} instead. Assembly will be skipped.", name, @@ -233,10 +244,22 @@ type Client<'TAttribute, 'TContext when 'TAttribute :> AnalyzerAttribute and 'TC registeredAnalyzers.AddOrUpdate(path, analyzers, (fun _ _ -> analyzers)) |> ignore - Array.length analyzers, analyzers |> Seq.collect snd |> Seq.length + let assemblyCount = Array.length analyzers + let analyzerCount = analyzers |> Seq.sumBy (snd >> Seq.length) + + { + AnalyzerAssemblies = assemblyCount + Analyzers = analyzerCount + FailedAssemblies = skippedAssemblies.Value + } else logger.LogWarning("Analyzer path {analyzerPath} does not exist", dir) - 0, 0 + + { + AnalyzerAssemblies = 0 + Analyzers = 0 + FailedAssemblies = 0 + } member x.RunAnalyzers(ctx: 'TContext) : Async = async { diff --git a/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fsi b/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fsi index 2e24b25..837bdd2 100644 --- a/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fsi +++ b/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fsi @@ -8,6 +8,16 @@ type AnalysisResult = Output: Result } +type AssemblyLoadStats = + { + /// The number of DLLs from which we tried to load analyzers. + AnalyzerAssemblies: int + /// The total number of analyzers loaded across all attempted DLLs. + Analyzers: int + /// The number of assemblies from which we tried and failed to load any DLLs. + FailedAssemblies: int + } + type ExcludeInclude = /// A predicate function to exclude Analyzers. | ExcludeFilter of (string -> bool) @@ -23,7 +33,7 @@ type Client<'TAttribute, 'TContext when 'TAttribute :> AnalyzerAttribute and 'TC /// Analyzers are filtered according to the ExcludeInclude set, if provided. /// /// number of found dlls matching `*Analyzer*.dll` and number of registered analyzers - member LoadAnalyzers: dir: string * ?excludeInclude: ExcludeInclude -> int * int + member LoadAnalyzers: dir: string * ?excludeInclude: ExcludeInclude -> AssemblyLoadStats /// Runs all registered analyzers for given context (file). /// list of messages. Ignores errors from the analyzers member RunAnalyzers: ctx: 'TContext -> Async From 3d8351ed5e084c6cc3ea0a93a01bcb04bf763858 Mon Sep 17 00:00:00 2001 From: Smaug123 Date: Fri, 26 Jan 2024 11:01:15 +0000 Subject: [PATCH 2/2] Add changelog entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b8419c..b4669ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Changed +* [Fail the tool when any analyzers fail to load](https://github.com/ionide/FSharp.Analyzers.SDK/pull/198) (thanks @Smaug123!) + ## [0.23.0] - 2024-01-05 ### Changed