Skip to content

Commit fb3353a

Browse files
kzukzu
committed
Remove all cross-process deduping and simplify
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).
1 parent 6be92c4 commit fb3353a

2 files changed

Lines changed: 38 additions & 67 deletions

File tree

‎samples/dotnet/SponsorLink/DiagnosticsManager.cs‎

Lines changed: 28 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,9 @@
44
using System.Collections.Concurrent;
55
using System.Collections.Generic;
66
using System.Collections.Immutable;
7-
using System.Diagnostics;
87
using System.Globalization;
98
using System.IO;
10-
using System.IO.MemoryMappedFiles;
119
using System.Linq;
12-
using System.Runtime.InteropServices;
13-
using System.Threading;
1410
using Humanizer;
1511
using Humanizer.Localisation;
1612
using Microsoft.CodeAnalysis;
@@ -44,32 +40,6 @@ class DiagnosticsManager
4440
ConcurrentDictionary<string, Diagnostic> Diagnostics
4541
=> AppDomainDictionary.Get<ConcurrentDictionary<string, Diagnostic>>(appDomainDiagnosticsKey.ToString());
4642

47-
/// <summary>
48-
/// Attemps to remove a diagnostic for the given product.
49-
/// </summary>
50-
/// <param name="product">The product diagnostic that might have been pushed previously.</param>
51-
/// <returns>The removed diagnostic, or <see langword="null" /> if none was previously pushed.</returns>
52-
public void ReportOnce(Action<Diagnostic> report, string product = Funding.Product)
53-
{
54-
if (Diagnostics.TryRemove(product, out var diagnostic) &&
55-
GetStatus(diagnostic) != SponsorStatus.Grace)
56-
{
57-
// Ensure only one such diagnostic is reported per product for the entire process,
58-
// so that we can avoid polluting the error list with duplicates across multiple projects.
59-
var id = string.Concat(SessionId, product, diagnostic.Id);
60-
using var mutex = new Mutex(false, "mutex" + id);
61-
mutex.WaitOne();
62-
using var mmf = CreateOrOpenMemoryMappedFile(product, 1);
63-
using var accessor = mmf.CreateViewAccessor();
64-
if (accessor.ReadByte(0) == 0)
65-
{
66-
accessor.Write(0, (byte)1);
67-
report(diagnostic);
68-
Tracing.Trace($"👈{diagnostic.Severity.ToString().ToLowerInvariant()}:{Process.GetCurrentProcess().Id}:{Process.GetCurrentProcess().ProcessName}:{product}:{diagnostic.Id}");
69-
}
70-
}
71-
}
72-
7343
/// <summary>
7444
/// Gets the status of the given product based on a previously stored diagnostic.
7545
/// To ensure the value is always set before returning, use <see cref="GetOrSetStatus"/>.
@@ -100,6 +70,34 @@ public SponsorStatus GetOrSetStatus(ImmutableArray<AdditionalText> manifests, An
10070
public SponsorStatus GetOrSetStatus(Func<AnalyzerOptions?> options)
10171
=> GetOrSetStatus(() => options().GetSponsorAdditionalFiles(), () => options()?.AnalyzerConfigOptionsProvider.GlobalOptions);
10272

73+
/// <summary>
74+
/// Attemps to remove a diagnostic for the given product.
75+
/// </summary>
76+
/// <param name="product">The product diagnostic that might have been pushed previously.</param>
77+
/// <returns>The removed diagnostic, or <see langword="null" /> if none was previously pushed.</returns>
78+
public Diagnostic? Pop(string product = Funding.Product)
79+
{
80+
if (Diagnostics.TryRemove(product, out var diagnostic) &&
81+
GetStatus(diagnostic) != SponsorStatus.Grace)
82+
{
83+
return diagnostic;
84+
}
85+
86+
return null;
87+
}
88+
89+
/// <summary>
90+
/// Pushes a diagnostic for the given product.
91+
/// </summary>
92+
/// <returns>The same diagnostic that was pushed, for chained invocations.</returns>
93+
Diagnostic Push(Diagnostic diagnostic, string product = Funding.Product)
94+
{
95+
// We only expect to get one warning per sponsorable+product
96+
// combination, and first one to set wins.
97+
Diagnostics.TryAdd(product, diagnostic);
98+
return diagnostic;
99+
}
100+
103101
SponsorStatus GetOrSetStatus(Func<ImmutableArray<AdditionalText>> getAdditionalFiles, Func<AnalyzerConfigOptions?> getGlobalOptions)
104102
{
105103
if (GetStatus() is { } status)
@@ -166,29 +164,6 @@ SponsorStatus GetOrSetStatus(Func<ImmutableArray<AdditionalText>> getAdditionalF
166164
}
167165
}
168166

169-
/// <summary>
170-
/// Pushes a diagnostic for the given product.
171-
/// </summary>
172-
/// <returns>The same diagnostic that was pushed, for chained invocations.</returns>
173-
Diagnostic Push(Diagnostic diagnostic, string product = Funding.Product)
174-
{
175-
// We only expect to get one warning per sponsorable+product
176-
// combination, and first one to set wins.
177-
if (Diagnostics.TryAdd(product, diagnostic))
178-
{
179-
// Reset the process-wide flag for this diagnostic.
180-
var id = string.Concat(SessionId, product, diagnostic.Id);
181-
using var mutex = new Mutex(false, "mutex" + id);
182-
mutex.WaitOne();
183-
using var mmf = CreateOrOpenMemoryMappedFile(id, 1);
184-
using var accessor = mmf.CreateViewAccessor();
185-
accessor.Write(0, (byte)0);
186-
Tracing.Trace($"👉{diagnostic.Severity.ToString().ToLowerInvariant()}:{Process.GetCurrentProcess().Id}:{Process.GetCurrentProcess().ProcessName}:{product}:{diagnostic.Id}");
187-
}
188-
189-
return diagnostic;
190-
}
191-
192167
SponsorStatus? GetStatus(Diagnostic? diagnostic) => diagnostic?.Properties.TryGetValue(nameof(SponsorStatus), out var value) == true
193168
? value switch
194169
{
@@ -201,19 +176,6 @@ Diagnostic Push(Diagnostic diagnostic, string product = Funding.Product)
201176
}
202177
: null;
203178

204-
static MemoryMappedFile CreateOrOpenMemoryMappedFile(string mapName, int capacity)
205-
{
206-
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
207-
return MemoryMappedFile.CreateOrOpen(mapName, capacity);
208-
209-
// On *nix, use a file-based memory-mapped file
210-
var filePath = $"/tmp/{mapName}";
211-
using (var fs = new FileStream(filePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.ReadWrite))
212-
fs.Write(new byte[capacity], 0, capacity);
213-
214-
return MemoryMappedFile.CreateFromFile(filePath, FileMode.OpenOrCreate);
215-
}
216-
217179
internal static DiagnosticDescriptor CreateSponsor(string[] sponsorable, string prefix) => new(
218180
$"{prefix}100",
219181
Resources.Sponsor_Title,

‎samples/dotnet/SponsorLink/SponsorLinkAnalyzer.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,16 @@ public override void Initialize(AnalysisContext context)
4141
// so it's expected to NOT get a diagnostic back. Also, we don't want to report
4242
// multiple diagnostics for each project in a solution that uses the same product.
4343
ctx.RegisterCompilationEndAction(ctx =>
44-
Diagnostics.ReportOnce(diagnostic => ctx.ReportDiagnostic(diagnostic)));
44+
{
45+
var prop = Funding.PackageId.Replace('.', '_');
46+
// Only report if the package is directly referenced in the project. See SL_CollectDependencies in buildTransitive\Devlooped.Sponsors.targets
47+
if (ctx.Options.AnalyzerConfigOptionsProvider.GlobalOptions.TryGetValue("build_property." + prop, out var package) &&
48+
package?.Length > 0 &&
49+
Diagnostics.Pop() is { } diagnostic)
50+
{
51+
ctx.ReportDiagnostic(diagnostic);
52+
}
53+
});
4554
}
4655
});
4756
#pragma warning restore RS1013 // Start action has no registered non-end actions

0 commit comments

Comments
 (0)