Skip to content

Commit

Permalink
Remove all cross-process deduping and simplify
Browse files Browse the repository at this point in the history
We cannot actually prevent reporting across processes because ultimately that causes VS to drop diagnostics which it seems to expect to match across multiple processes (analyzer, compiler).

By making the analyzer properly detect direct dependencies only, we potentially eliminate most duplication which typically would come from transitive dependencies (except for development dependencies, say, which I'd say are the exception other than the norm).
  • Loading branch information
kzu committed Jul 10, 2024
1 parent 6be92c4 commit fb3353a
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 67 deletions.
94 changes: 28 additions & 66 deletions samples/dotnet/SponsorLink/DiagnosticsManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,9 @@
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.IO.MemoryMappedFiles;
using System.Linq;
using System.Runtime.InteropServices;
using System.Threading;
using Humanizer;
using Humanizer.Localisation;
using Microsoft.CodeAnalysis;
Expand Down Expand Up @@ -44,32 +40,6 @@ class DiagnosticsManager
ConcurrentDictionary<string, Diagnostic> Diagnostics
=> AppDomainDictionary.Get<ConcurrentDictionary<string, Diagnostic>>(appDomainDiagnosticsKey.ToString());

/// <summary>
/// Attemps to remove a diagnostic for the given product.
/// </summary>
/// <param name="product">The product diagnostic that might have been pushed previously.</param>
/// <returns>The removed diagnostic, or <see langword="null" /> if none was previously pushed.</returns>
public void ReportOnce(Action<Diagnostic> report, string product = Funding.Product)
{
if (Diagnostics.TryRemove(product, out var diagnostic) &&
GetStatus(diagnostic) != SponsorStatus.Grace)
{
// Ensure only one such diagnostic is reported per product for the entire process,
// so that we can avoid polluting the error list with duplicates across multiple projects.
var id = string.Concat(SessionId, product, diagnostic.Id);
using var mutex = new Mutex(false, "mutex" + id);
mutex.WaitOne();
using var mmf = CreateOrOpenMemoryMappedFile(product, 1);
using var accessor = mmf.CreateViewAccessor();
if (accessor.ReadByte(0) == 0)
{
accessor.Write(0, (byte)1);
report(diagnostic);
Tracing.Trace($"👈{diagnostic.Severity.ToString().ToLowerInvariant()}:{Process.GetCurrentProcess().Id}:{Process.GetCurrentProcess().ProcessName}:{product}:{diagnostic.Id}");
}
}
}

/// <summary>
/// Gets the status of the given product based on a previously stored diagnostic.
/// To ensure the value is always set before returning, use <see cref="GetOrSetStatus"/>.
Expand Down Expand Up @@ -100,6 +70,34 @@ public SponsorStatus GetOrSetStatus(ImmutableArray<AdditionalText> manifests, An
public SponsorStatus GetOrSetStatus(Func<AnalyzerOptions?> options)
=> GetOrSetStatus(() => options().GetSponsorAdditionalFiles(), () => options()?.AnalyzerConfigOptionsProvider.GlobalOptions);

/// <summary>
/// Attemps to remove a diagnostic for the given product.
/// </summary>
/// <param name="product">The product diagnostic that might have been pushed previously.</param>
/// <returns>The removed diagnostic, or <see langword="null" /> if none was previously pushed.</returns>
public Diagnostic? Pop(string product = Funding.Product)
{
if (Diagnostics.TryRemove(product, out var diagnostic) &&
GetStatus(diagnostic) != SponsorStatus.Grace)
{
return diagnostic;
}

return null;
}

/// <summary>
/// Pushes a diagnostic for the given product.
/// </summary>
/// <returns>The same diagnostic that was pushed, for chained invocations.</returns>
Diagnostic Push(Diagnostic diagnostic, string product = Funding.Product)
{
// We only expect to get one warning per sponsorable+product
// combination, and first one to set wins.
Diagnostics.TryAdd(product, diagnostic);
return diagnostic;
}

SponsorStatus GetOrSetStatus(Func<ImmutableArray<AdditionalText>> getAdditionalFiles, Func<AnalyzerConfigOptions?> getGlobalOptions)
{
if (GetStatus() is { } status)
Expand Down Expand Up @@ -166,29 +164,6 @@ SponsorStatus GetOrSetStatus(Func<ImmutableArray<AdditionalText>> getAdditionalF
}
}

/// <summary>
/// Pushes a diagnostic for the given product.
/// </summary>
/// <returns>The same diagnostic that was pushed, for chained invocations.</returns>
Diagnostic Push(Diagnostic diagnostic, string product = Funding.Product)
{
// We only expect to get one warning per sponsorable+product
// combination, and first one to set wins.
if (Diagnostics.TryAdd(product, diagnostic))
{
// Reset the process-wide flag for this diagnostic.
var id = string.Concat(SessionId, product, diagnostic.Id);
using var mutex = new Mutex(false, "mutex" + id);
mutex.WaitOne();
using var mmf = CreateOrOpenMemoryMappedFile(id, 1);
using var accessor = mmf.CreateViewAccessor();
accessor.Write(0, (byte)0);
Tracing.Trace($"👉{diagnostic.Severity.ToString().ToLowerInvariant()}:{Process.GetCurrentProcess().Id}:{Process.GetCurrentProcess().ProcessName}:{product}:{diagnostic.Id}");
}

return diagnostic;
}

SponsorStatus? GetStatus(Diagnostic? diagnostic) => diagnostic?.Properties.TryGetValue(nameof(SponsorStatus), out var value) == true
? value switch
{
Expand All @@ -201,19 +176,6 @@ Diagnostic Push(Diagnostic diagnostic, string product = Funding.Product)
}
: null;

static MemoryMappedFile CreateOrOpenMemoryMappedFile(string mapName, int capacity)
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
return MemoryMappedFile.CreateOrOpen(mapName, capacity);

// On *nix, use a file-based memory-mapped file
var filePath = $"/tmp/{mapName}";
using (var fs = new FileStream(filePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.ReadWrite))
fs.Write(new byte[capacity], 0, capacity);

return MemoryMappedFile.CreateFromFile(filePath, FileMode.OpenOrCreate);
}

internal static DiagnosticDescriptor CreateSponsor(string[] sponsorable, string prefix) => new(
$"{prefix}100",
Resources.Sponsor_Title,
Expand Down
11 changes: 10 additions & 1 deletion samples/dotnet/SponsorLink/SponsorLinkAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,16 @@ public override void Initialize(AnalysisContext context)
// so it's expected to NOT get a diagnostic back. Also, we don't want to report
// multiple diagnostics for each project in a solution that uses the same product.
ctx.RegisterCompilationEndAction(ctx =>
Diagnostics.ReportOnce(diagnostic => ctx.ReportDiagnostic(diagnostic)));
{
var prop = Funding.PackageId.Replace('.', '_');
// Only report if the package is directly referenced in the project. See SL_CollectDependencies in buildTransitive\Devlooped.Sponsors.targets
if (ctx.Options.AnalyzerConfigOptionsProvider.GlobalOptions.TryGetValue("build_property." + prop, out var package) &&
package?.Length > 0 &&
Diagnostics.Pop() is { } diagnostic)
{
ctx.ReportDiagnostic(diagnostic);
}
});
}
});
#pragma warning restore RS1013 // Start action has no registered non-end actions
Expand Down

0 comments on commit fb3353a

Please sign in to comment.