Skip to content

Commit

Permalink
Merge pull request #198 from Smaug123/fail-on-failure
Browse files Browse the repository at this point in the history
Fail the tool when analyzers fail to load
  • Loading branch information
nojaf authored Jan 26, 2024
2 parents 49a5a20 + 3d8351e commit 1aa73ad
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 12 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 16 additions & 5 deletions src/FSharp.Analyzers.Cli/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -596,11 +596,14 @@ let main argv =

let client = Client<CliAnalyzerAttribute, CliContext>(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)
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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<CliAnalyzerAttribute, CliContext>()
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 |]
Expand Down
29 changes: 26 additions & 3 deletions src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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$")
Expand Down Expand Up @@ -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) ->
Expand All @@ -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,
Expand Down Expand Up @@ -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<AnalyzerMessage list> =
async {
Expand Down
12 changes: 11 additions & 1 deletion src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ type AnalysisResult =
Output: Result<Message list, exn>
}

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)
Expand All @@ -23,7 +33,7 @@ type Client<'TAttribute, 'TContext when 'TAttribute :> AnalyzerAttribute and 'TC
/// Analyzers are filtered according to the ExcludeInclude set, if provided.
/// </summary>
/// <returns>number of found dlls matching `*Analyzer*.dll` and number of registered analyzers</returns>
member LoadAnalyzers: dir: string * ?excludeInclude: ExcludeInclude -> int * int
member LoadAnalyzers: dir: string * ?excludeInclude: ExcludeInclude -> AssemblyLoadStats
/// <summary>Runs all registered analyzers for given context (file).</summary>
/// <returns>list of messages. Ignores errors from the analyzers</returns>
member RunAnalyzers: ctx: 'TContext -> Async<AnalyzerMessage list>
Expand Down

0 comments on commit 1aa73ad

Please sign in to comment.