diff --git a/src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs b/src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs deleted file mode 100644 index 74f44f99c62baa..00000000000000 --- a/src/libraries/Common/src/SourceGenerators/DiagnosticInfo.cs +++ /dev/null @@ -1,60 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Linq; -using System.Numerics.Hashing; -using Microsoft.CodeAnalysis; - -namespace SourceGenerators; - -/// -/// Descriptor for diagnostic instances using structural equality comparison. -/// Provides a work-around for https://github.com/dotnet/roslyn/issues/68291. -/// -internal readonly struct DiagnosticInfo : IEquatable -{ - public DiagnosticDescriptor Descriptor { get; private init; } - public object?[] MessageArgs { get; private init; } - public Location? Location { get; private init; } - - public static DiagnosticInfo Create(DiagnosticDescriptor descriptor, Location? location, object?[]? messageArgs) - { - Location? trimmedLocation = location is null ? null : GetTrimmedLocation(location); - - return new DiagnosticInfo - { - Descriptor = descriptor, - Location = trimmedLocation, - MessageArgs = messageArgs ?? Array.Empty() - }; - - // Creates a copy of the Location instance that does not capture a reference to Compilation. - static Location GetTrimmedLocation(Location location) - => Location.Create(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span); - } - - public Diagnostic CreateDiagnostic() - => Diagnostic.Create(Descriptor, Location, MessageArgs); - - public override readonly bool Equals(object? obj) => obj is DiagnosticInfo info && Equals(info); - - public readonly bool Equals(DiagnosticInfo other) - { - return Descriptor.Equals(other.Descriptor) && - MessageArgs.SequenceEqual(other.MessageArgs) && - Location == other.Location; - } - - public override readonly int GetHashCode() - { - int hashCode = Descriptor.GetHashCode(); - foreach (object? messageArg in MessageArgs) - { - hashCode = HashHelpers.Combine(hashCode, messageArg?.GetHashCode() ?? 0); - } - - hashCode = HashHelpers.Combine(hashCode, Location?.GetHashCode() ?? 0); - return hashCode; - } -} diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs index dc940d2cb07875..beb45cff274be5 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs @@ -30,13 +30,13 @@ internal sealed partial class Parser(CompilationData compilationData) private bool _emitEnumParseMethod; private bool _emitGenericParseEnum; - public List? Diagnostics { get; private set; } + public List? Diagnostics { get; private set; } public SourceGenerationSpec? GetSourceGenerationSpec(ImmutableArray invocations, CancellationToken cancellationToken) { if (!_langVersionIsSupported) { - RecordDiagnostic(DiagnosticDescriptors.LanguageVersionNotSupported, trimmedLocation: Location.None); + RecordDiagnostic(DiagnosticDescriptors.LanguageVersionNotSupported, location: Location.None); return null; } @@ -979,10 +979,10 @@ private void ReportContainingTypeDiagnosticIfRequired(TypeParseInfo typeParseInf } } - private void RecordDiagnostic(DiagnosticDescriptor descriptor, Location trimmedLocation, params object?[]? messageArgs) + private void RecordDiagnostic(DiagnosticDescriptor descriptor, Location location, params object?[]? messageArgs) { - Diagnostics ??= new List(); - Diagnostics.Add(DiagnosticInfo.Create(descriptor, trimmedLocation, messageArgs)); + Diagnostics ??= new List(); + Diagnostics.Add(Diagnostic.Create(descriptor, location, messageArgs)); } private void CheckIfToEmitParseEnumMethod() diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.cs index 4a3d5bbf7dea81..816bdff43f5c40 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.cs @@ -6,10 +6,10 @@ using System.Diagnostics; using System.Reflection; using System.Threading; +using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; -using SourceGenerators; namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration { @@ -37,7 +37,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) ? new CompilationData((CSharpCompilation)compilation) : null); - IncrementalValueProvider<(SourceGenerationSpec?, ImmutableEquatableArray?)> genSpec = context.SyntaxProvider + IncrementalValueProvider<(SourceGenerationSpec?, ImmutableArray)> genSpec = context.SyntaxProvider .CreateSyntaxProvider( (node, _) => BinderInvocation.IsCandidateSyntaxNode(node), BinderInvocation.Create) @@ -48,14 +48,16 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { if (tuple.Right is not CompilationData compilationData) { - return (null, null); + return (null, ImmutableArray.Empty); } try { Parser parser = new(compilationData); SourceGenerationSpec? spec = parser.GetSourceGenerationSpec(tuple.Left, cancellationToken); - ImmutableEquatableArray? diagnostics = parser.Diagnostics?.ToImmutableEquatableArray(); + ImmutableArray diagnostics = parser.Diagnostics is { } diags + ? diags.ToImmutableArray() + : ImmutableArray.Empty; return (spec, diagnostics); } catch (Exception ex) @@ -65,7 +67,26 @@ public void Initialize(IncrementalGeneratorInitializationContext context) }) .WithTrackingName(GenSpecTrackingName); - context.RegisterSourceOutput(genSpec, ReportDiagnosticsAndEmitSource); + // Project the combined pipeline result to just the equatable model, discarding diagnostics. + // SourceGenerationSpec implements value equality, so Roslyn's Select operator will compare + // successive model snapshots and only propagate changes downstream when the model structurally + // differs. This ensures source generation is fully incremental: re-emitting code only when + // the binding spec actually changes, not on every keystroke or positional shift. + IncrementalValueProvider sourceGenerationSpec = + genSpec.Select(static (t, _) => t.Item1); + + context.RegisterSourceOutput(sourceGenerationSpec, EmitSource); + + // Project to just the diagnostics, discarding the model. ImmutableArray does not + // implement value equality, so Roslyn's incremental pipeline uses reference equality for these + // values — the callback fires on every compilation change. This is by design: diagnostic + // emission is cheap, and we need fresh SourceLocation instances that are pragma-suppressible + // (cf. https://github.com/dotnet/runtime/issues/92509). + // No source code is generated from this pipeline — it exists solely to report diagnostics. + IncrementalValueProvider> diagnostics = + genSpec.Select(static (t, _) => t.Item2); + + context.RegisterSourceOutput(diagnostics, EmitDiagnostics); if (!s_hasInitializedInterceptorVersion) { @@ -136,17 +157,17 @@ internal static int DetermineInterceptableVersion() /// public Action? OnSourceEmitting { get; init; } - private void ReportDiagnosticsAndEmitSource(SourceProductionContext sourceProductionContext, (SourceGenerationSpec? SourceGenerationSpec, ImmutableEquatableArray? Diagnostics) input) + private static void EmitDiagnostics(SourceProductionContext context, ImmutableArray diagnostics) { - if (input.Diagnostics is ImmutableEquatableArray diagnostics) + foreach (Diagnostic diagnostic in diagnostics) { - foreach (DiagnosticInfo diagnostic in diagnostics) - { - sourceProductionContext.ReportDiagnostic(diagnostic.CreateDiagnostic()); - } + context.ReportDiagnostic(diagnostic); } + } - if (input.SourceGenerationSpec is SourceGenerationSpec spec) + private void EmitSource(SourceProductionContext sourceProductionContext, SourceGenerationSpec? spec) + { + if (spec is not null) { OnSourceEmitting?.Invoke(spec); Emitter emitter = new(spec); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Microsoft.Extensions.Configuration.Binder.SourceGeneration.csproj b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Microsoft.Extensions.Configuration.Binder.SourceGeneration.csproj index bf12a1fc225b81..a0fac6dbbfb626 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Microsoft.Extensions.Configuration.Binder.SourceGeneration.csproj +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Microsoft.Extensions.Configuration.Binder.SourceGeneration.csproj @@ -30,7 +30,6 @@ - diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs index 7eee48fdeb28ca..e15f9625a971a1 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs @@ -12,6 +12,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -414,5 +415,129 @@ public class AnotherGraphWithUnsupportedMembers Assert.True(result.Diagnostics.Any(diag => diag.Id == Diagnostics.TypeNotSupported.Id)); Assert.True(result.Diagnostics.Any(diag => diag.Id == Diagnostics.PropertyNotSupported.Id)); } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))] + public async Task Diagnostic_HasPragmaSuppressibleLocation() + { + // SYSLIB1103: ValueTypesInvalidForBind (Warning, configurable). + string source = """ + #pragma warning disable SYSLIB1103 + using System; + using Microsoft.Extensions.Configuration; + + public class Program + { + public static void Main() + { + ConfigurationBuilder configurationBuilder = new(); + IConfigurationRoot config = configurationBuilder.Build(); + + int myInt = 1; + config.Bind(myInt); + } + } + """; + + ConfigBindingGenRunResult result = await RunGeneratorAndUpdateCompilation(source); + var effective = CompilationWithAnalyzers.GetEffectiveDiagnostics(result.Diagnostics, result.OutputCompilation); + Diagnostic diagnostic = Assert.Single(effective, d => d.Id == "SYSLIB1103"); + Assert.True(diagnostic.IsSuppressed); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))] + public async Task Diagnostic_NoPragma_IsNotSuppressed() + { + string source = """ + using System; + using Microsoft.Extensions.Configuration; + + public class Program + { + public static void Main() + { + ConfigurationBuilder configurationBuilder = new(); + IConfigurationRoot config = configurationBuilder.Build(); + + int myInt = 1; + config.Bind(myInt); + } + } + """; + + ConfigBindingGenRunResult result = await RunGeneratorAndUpdateCompilation(source); + var effective = CompilationWithAnalyzers.GetEffectiveDiagnostics(result.Diagnostics, result.OutputCompilation); + Diagnostic diagnostic = Assert.Single(effective, d => d.Id == "SYSLIB1103"); + Assert.False(diagnostic.IsSuppressed); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))] + public async Task Diagnostic_MultipleDiagnostics_OnlySomeSuppressed() + { + string source = """ + using System; + using System.Collections.Immutable; + using System.Text; + using System.Text.Json; + using Microsoft.Extensions.Configuration; + + public class Program + { + public static void Main() + { + ConfigurationBuilder configurationBuilder = new(); + IConfigurationRoot config = configurationBuilder.Build(); + + // SYSLIB1103 suppressed for this call only. + #pragma warning disable SYSLIB1103 + int myInt = 1; + config.Bind(myInt); + #pragma warning restore SYSLIB1103 + + // SYSLIB1103 NOT suppressed for this call. + long myLong = 1; + config.Bind(myLong); + } + } + """; + + ConfigBindingGenRunResult result = await RunGeneratorAndUpdateCompilation(source); + var effective = CompilationWithAnalyzers.GetEffectiveDiagnostics(result.Diagnostics, result.OutputCompilation) + .Where(d => d.Id == "SYSLIB1103") + .ToList(); + + Assert.Equal(2, effective.Count); + Assert.Single(effective, d => d.IsSuppressed); + Assert.Single(effective, d => !d.IsSuppressed); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))] + public async Task Diagnostic_PragmaRestoreOutsideSpan_IsNotSuppressed() + { + string source = """ + using System; + using Microsoft.Extensions.Configuration; + + public class Program + { + public static void Main() + { + ConfigurationBuilder configurationBuilder = new(); + IConfigurationRoot config = configurationBuilder.Build(); + + // Suppress and restore BEFORE the diagnostic site. + #pragma warning disable SYSLIB1103 + #pragma warning restore SYSLIB1103 + + int myInt = 1; + config.Bind(myInt); + } + } + """; + + ConfigBindingGenRunResult result = await RunGeneratorAndUpdateCompilation(source); + var effective = CompilationWithAnalyzers.GetEffectiveDiagnostics(result.Diagnostics, result.OutputCompilation); + Diagnostic diagnostic = Assert.Single(effective, d => d.Id == "SYSLIB1103"); + Assert.False(diagnostic.IsSuppressed); + } } } diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs index c69c2db2c07e85..b98abaf7d04c99 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs @@ -38,7 +38,7 @@ internal sealed class Parser private readonly INamedTypeSymbol _stringSymbol; private readonly Action? _reportDiagnostic; - public List Diagnostics { get; } = new(); + public List Diagnostics { get; } = new(); public Parser( INamedTypeSymbol loggerMessageAttribute, @@ -811,12 +811,14 @@ private static string GenerateClassName(TypeDeclarationSyntax typeDeclaration) private void Diag(DiagnosticDescriptor desc, Location? location, params object?[]? messageArgs) { + Diagnostic diagnostic = Diagnostic.Create(desc, location, messageArgs); + // Report immediately if callback is provided (preserves pragma suppression with original locations) - _reportDiagnostic?.Invoke(Diagnostic.Create(desc, location, messageArgs)); + _reportDiagnostic?.Invoke(diagnostic); // Also collect for scenarios that need the diagnostics list; in Roslyn 4.0+ incremental generators, - // this list is exposed via parser.Diagnostics (as ImmutableEquatableArray) and reported in Execute. - Diagnostics.Add(DiagnosticInfo.Create(desc, location, messageArgs)); + // this list is exposed via parser.Diagnostics and reported in the diagnostic pipeline. + Diagnostics.Add(diagnostic); } private static bool IsBaseOrIdentity(ITypeSymbol source, ITypeSymbol dest, Compilation compilation) diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Roslyn4.0.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Roslyn4.0.cs index ef9fa6582b53b3..9a205f873a6d00 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Roslyn4.0.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Roslyn4.0.cs @@ -25,7 +25,7 @@ public static class StepNames public void Initialize(IncrementalGeneratorInitializationContext context) { - IncrementalValuesProvider<(LoggerClassSpec? LoggerClassSpec, ImmutableEquatableArray Diagnostics, bool HasStringCreate)> loggerClasses = context.SyntaxProvider + IncrementalValuesProvider<(LoggerClassSpec? LoggerClassSpec, ImmutableArray Diagnostics, bool HasStringCreate)> loggerClasses = context.SyntaxProvider .ForAttributeWithMetadataName( #if !ROSLYN4_4_OR_GREATER context, @@ -66,7 +66,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) if (exceptionSymbol == null) { - var diagnostics = new[] { DiagnosticInfo.Create(DiagnosticDescriptors.MissingRequiredType, null, new object?[] { "System.Exception" }) }.ToImmutableEquatableArray(); + var diagnostics = ImmutableArray.Create(Diagnostic.Create(DiagnosticDescriptors.MissingRequiredType, null, new object?[] { "System.Exception" })); return (null, diagnostics, false); } @@ -92,75 +92,110 @@ public void Initialize(IncrementalGeneratorInitializationContext context) // Convert to immutable spec for incremental caching LoggerClassSpec? loggerClassSpec = logClasses.Count > 0 ? logClasses[0].ToSpec() : null; - return (loggerClassSpec, parser.Diagnostics.ToImmutableEquatableArray(), hasStringCreate); + return (loggerClassSpec, parser.Diagnostics.ToImmutableArray(), hasStringCreate); }) #if ROSLYN4_4_OR_GREATER .WithTrackingName(StepNames.LoggerMessageTransform) #endif ; - context.RegisterSourceOutput(loggerClasses.Collect(), static (spc, items) => Execute(items, spc)); + // Single collect for all per-method results, then aggregate into an equatable source + // model (using ImmutableEquatableArray for deep value equality) plus flat diagnostics. + // Diagnostics are deduplicated here because each attributed method triggers parsing of + // the entire class, producing duplicate diagnostics. + IncrementalValueProvider<(ImmutableEquatableArray<(LoggerClassSpec LoggerClassSpec, bool HasStringCreate)> Specs, ImmutableArray Diagnostics)> collected = + loggerClasses.Collect().Select(static (items, _) => + { + ImmutableArray<(LoggerClassSpec, bool)>.Builder? specs = null; + ImmutableArray.Builder? diagnostics = null; + HashSet<(string Id, TextSpan? Span, string? FilePath, string Message)>? seen = null; + + foreach (var item in items) + { + if (item.LoggerClassSpec is not null) + { + (specs ??= ImmutableArray.CreateBuilder<(LoggerClassSpec, bool)>()).Add((item.LoggerClassSpec, item.HasStringCreate)); + } + foreach (Diagnostic diagnostic in item.Diagnostics) + { + if ((seen ??= new()).Add((diagnostic.Id, diagnostic.Location?.SourceSpan, diagnostic.Location?.SourceTree?.FilePath, diagnostic.GetMessage()))) + { + (diagnostics ??= ImmutableArray.CreateBuilder()).Add(diagnostic); + } + } + } + + return ( + specs?.ToImmutableEquatableArray() ?? ImmutableEquatableArray<(LoggerClassSpec, bool)>.Empty, + diagnostics?.ToImmutable() ?? ImmutableArray.Empty); + }); + + // Project to just the equatable source model, discarding diagnostics. + // ImmutableEquatableArray provides deep value equality, so Roslyn's Select operator + // compares successive model snapshots and only propagates changes downstream when the + // model structurally differs. This ensures source generation is fully incremental. + IncrementalValueProvider> sourceGenerationSpecs = + collected.Select(static (t, _) => t.Specs); + + context.RegisterSourceOutput(sourceGenerationSpecs, static (spc, items) => EmitSource(items, spc)); + + // Project to just the diagnostics, discarding the model. ImmutableArray does not + // implement value equality, so Roslyn's incremental pipeline uses reference equality for these + // values — the callback fires on every compilation change. This is by design: diagnostic + // emission is cheap, and we need fresh SourceLocation instances that are pragma-suppressible + // (cf. https://github.com/dotnet/runtime/issues/92509). + IncrementalValueProvider> diagnosticResults = + collected.Select(static (t, _) => t.Diagnostics); + + context.RegisterSourceOutput(diagnosticResults, EmitDiagnostics); + } + + private static void EmitDiagnostics(SourceProductionContext context, ImmutableArray diagnostics) + { + foreach (Diagnostic diagnostic in diagnostics) + { + context.ReportDiagnostic(diagnostic); + } } - private static void Execute(ImmutableArray<(LoggerClassSpec? LoggerClassSpec, ImmutableEquatableArray Diagnostics, bool HasStringCreate)> items, SourceProductionContext context) + private static void EmitSource(ImmutableEquatableArray<(LoggerClassSpec LoggerClassSpec, bool HasStringCreate)> items, SourceProductionContext context) { - if (items.IsDefaultOrEmpty) + if (items.Count == 0) { return; } bool hasStringCreate = false; - var allLogClasses = new Dictionary(); // Use dictionary to deduplicate by class key - var reportedDiagnostics = new HashSet(); // Track reported diagnostics to avoid duplicates + var allLogClasses = new Dictionary(); // Deduplicate by class key foreach (var item in items) { - // Report diagnostics (note: pragma suppression doesn't work with trimmed locations - known Roslyn limitation) - // Use HashSet to deduplicate - each attributed method triggers parsing of entire class, producing duplicate diagnostics - if (item.Diagnostics is not null) + hasStringCreate |= item.HasStringCreate; + + // Build unique key including parent class chain to handle nested classes + string classKey = BuildClassKey(item.LoggerClassSpec); + + // Each attributed method in a partial class file produces the same LoggerClassSpec with all methods in that file. + // However, different partial class files produce different LoggerClassSpecs with different methods. Merge them. + if (!allLogClasses.TryGetValue(classKey, out LoggerClass? existingClass)) { - foreach (var diagnostic in item.Diagnostics) - { - if (reportedDiagnostics.Add(diagnostic)) - { - context.ReportDiagnostic(diagnostic.CreateDiagnostic()); - } - } + allLogClasses[classKey] = FromSpec(item.LoggerClassSpec); } - - if (item.LoggerClassSpec != null) + else { - hasStringCreate |= item.HasStringCreate; - - // Build unique key including parent class chain to handle nested classes - string classKey = BuildClassKey(item.LoggerClassSpec); + var newClass = FromSpec(item.LoggerClassSpec); - // Each attributed method in a partial class file produces the same LoggerClassSpec with all methods in that file. - // However, different partial class files (e.g., LevelTestExtensions.cs and LevelTestExtensions.WithDiagnostics.cs) - // produce different LoggerClassSpecs with different methods. Merge them. - if (!allLogClasses.TryGetValue(classKey, out LoggerClass? existingClass)) + var existingMethodKeys = new HashSet<(string Name, int EventId)>(); + foreach (var method in existingClass.Methods) { - allLogClasses[classKey] = FromSpec(item.LoggerClassSpec); + existingMethodKeys.Add((method.Name, method.EventId)); } - else - { - // Merge methods from different partial class files - var newClass = FromSpec(item.LoggerClassSpec); - - // Use HashSet for O(1) lookup to avoid O(N×M) complexity - var existingMethodKeys = new HashSet<(string Name, int EventId)>(); - foreach (var method in existingClass.Methods) - { - existingMethodKeys.Add((method.Name, method.EventId)); - } - foreach (var method in newClass.Methods) + foreach (var method in newClass.Methods) + { + if (existingMethodKeys.Add((method.Name, method.EventId))) { - // Only add methods that don't already exist (avoid duplicates from same file) - if (existingMethodKeys.Add((method.Name, method.EventId))) - { - existingClass.Methods.Add(method); - } + existingClass.Methods.Add(method); } } } diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Microsoft.Extensions.Logging.Generators.targets b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Microsoft.Extensions.Logging.Generators.targets index f1a42f8831ecfa..096dc2f89a3709 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Microsoft.Extensions.Logging.Generators.targets +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Microsoft.Extensions.Logging.Generators.targets @@ -25,7 +25,6 @@ - diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs index a6cc6db209297d..a6bef486f6de2f 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs @@ -10,6 +10,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; using SourceGenerators.Tests; using Xunit; @@ -1426,5 +1427,35 @@ private static async Task> RunGenerator( return d; } + + [Fact] + public async Task Diagnostic_HasPragmaSuppressibleLocation() + { + // SYSLIB1017: MissingLogLevel (Error, but not NotConfigurable). + string code = """ + #pragma warning disable SYSLIB1017 + using Microsoft.Extensions.Logging; + + namespace Test + { + partial class C + { + [LoggerMessage(EventId = 0, Message = "M1")] + static partial void M1(ILogger logger); + } + } + """; + + Assembly[] refs = new[] { typeof(ILogger).Assembly, typeof(LoggerMessageAttribute).Assembly }; + using var workspace = RoslynTestUtils.CreateTestWorkspace(); + Project proj = RoslynTestUtils.CreateTestProject(workspace, refs) + .WithDocuments(new[] { code }); + Assert.True(proj.Solution.Workspace.TryApplyChanges(proj.Solution)); + Compilation comp = (await proj.GetCompilationAsync().ConfigureAwait(false))!; + var (diags, _) = RoslynTestUtils.RunGenerator(comp, new LoggerMessageGenerator()); + var effective = CompilationWithAnalyzers.GetEffectiveDiagnostics(diags, comp); + Diagnostic diagnostic = Assert.Single(effective, d => d.Id == "SYSLIB1017"); + Assert.True(diagnostic.IsSuppressed); + } } } diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs index 329a369b4a0135..a865466d5f40b8 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs @@ -47,7 +47,7 @@ private sealed class Parser private readonly Dictionary _generatedTypes = new(SymbolEqualityComparer.Default); #pragma warning restore - public List Diagnostics { get; } = new(); + public List Diagnostics { get; } = new(); private Location? _contextClassLocation; public void ReportDiagnostic(DiagnosticDescriptor descriptor, Location? location, params object?[]? messageArgs) @@ -60,7 +60,7 @@ public void ReportDiagnostic(DiagnosticDescriptor descriptor, Location? location location = _contextClassLocation; } - Diagnostics.Add(DiagnosticInfo.Create(descriptor, location, messageArgs)); + Diagnostics.Add(Diagnostic.Create(descriptor, location, messageArgs)); } public Parser(KnownTypeSymbols knownSymbols) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn3.11.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn3.11.cs index 965bf7fc210751..5397715ede9da8 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn3.11.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn3.11.cs @@ -75,9 +75,9 @@ public void Execute(GeneratorExecutionContext executionContext) } // Stage 2. Report any diagnostics gathered by the parser. - foreach (DiagnosticInfo diagnosticInfo in parser.Diagnostics) + foreach (Diagnostic diagnostic in parser.Diagnostics) { - executionContext.ReportDiagnostic(diagnosticInfo.CreateDiagnostic()); + executionContext.ReportDiagnostic(diagnostic); } if (contextGenerationSpecs is null) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn4.0.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn4.0.cs index 75f1f51269e7a6..79fab10a1cfc34 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn4.0.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Roslyn4.0.cs @@ -10,7 +10,6 @@ #if !ROSLYN4_4_OR_GREATER using Microsoft.CodeAnalysis.DotnetRuntime.Extensions; #endif -using SourceGenerators; namespace System.Text.Json.SourceGeneration { @@ -32,7 +31,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) IncrementalValueProvider knownTypeSymbols = context.CompilationProvider .Select((compilation, _) => new KnownTypeSymbols(compilation)); - IncrementalValuesProvider<(ContextGenerationSpec?, ImmutableEquatableArray)> contextGenerationSpecs = context.SyntaxProvider + IncrementalValuesProvider<(ContextGenerationSpec?, ImmutableArray)> contextGenerationSpecs = context.SyntaxProvider .ForAttributeWithMetadataName( #if !ROSLYN4_4_OR_GREATER context, @@ -54,7 +53,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) #pragma warning restore RS1035 Parser parser = new(tuple.Right); ContextGenerationSpec? contextGenerationSpec = parser.ParseContextGenerationSpec(tuple.Left.ContextClass, tuple.Left.SemanticModel, cancellationToken); - ImmutableEquatableArray diagnostics = parser.Diagnostics.ToImmutableEquatableArray(); + ImmutableArray diagnostics = parser.Diagnostics.ToImmutableArray(); return (contextGenerationSpec, diagnostics); #pragma warning disable RS1035 } @@ -69,23 +68,36 @@ public void Initialize(IncrementalGeneratorInitializationContext context) #endif ; - context.RegisterSourceOutput(contextGenerationSpecs, ReportDiagnosticsAndEmitSource); + // Project the combined pipeline result to just the equatable model, discarding diagnostics. + // ContextGenerationSpec implements value equality, so Roslyn's Select operator will compare + // successive model snapshots and only propagate changes downstream when the model structurally + // differs. This ensures source generation is fully incremental: re-emitting code only when + // the serialization spec actually changes, not on every keystroke or positional shift. + IncrementalValuesProvider sourceGenerationSpecs = + contextGenerationSpecs.Select(static (t, _) => t.Item1); + + context.RegisterSourceOutput(sourceGenerationSpecs, EmitSource); + + // Project to just the diagnostics, discarding the model. ImmutableArray does not + // implement value equality, so Roslyn's incremental pipeline uses reference equality for these + // values — the callback fires on every compilation change. This is by design: diagnostic + // emission is cheap, and we need fresh SourceLocation instances that are pragma-suppressible + // (cf. https://github.com/dotnet/runtime/issues/92509). + // No source code is generated from this pipeline — it exists solely to report diagnostics. + IncrementalValuesProvider> diagnostics = + contextGenerationSpecs.Select(static (t, _) => t.Item2); + + context.RegisterSourceOutput(diagnostics, EmitDiagnostics); } - private void ReportDiagnosticsAndEmitSource(SourceProductionContext sourceProductionContext, (ContextGenerationSpec? ContextGenerationSpec, ImmutableEquatableArray Diagnostics) input) + private void EmitSource(SourceProductionContext sourceProductionContext, ContextGenerationSpec? contextGenerationSpec) { - // Report any diagnostics ahead of emitting. - foreach (DiagnosticInfo diagnostic in input.Diagnostics) - { - sourceProductionContext.ReportDiagnostic(diagnostic.CreateDiagnostic()); - } - - if (input.ContextGenerationSpec is null) + if (contextGenerationSpec is null) { return; } - OnSourceEmitting?.Invoke(ImmutableArray.Create(input.ContextGenerationSpec)); + OnSourceEmitting?.Invoke(ImmutableArray.Create(contextGenerationSpec)); // Ensure the source generator emits number literals using invariant culture. // This prevents issues such as locale-specific negative signs (e.g., U+2212 in fi-FI) @@ -96,7 +108,7 @@ private void ReportDiagnosticsAndEmitSource(SourceProductionContext sourceProduc try { Emitter emitter = new(sourceProductionContext); - emitter.Emit(input.ContextGenerationSpec); + emitter.Emit(contextGenerationSpec); } finally { @@ -105,6 +117,14 @@ private void ReportDiagnosticsAndEmitSource(SourceProductionContext sourceProduc #pragma warning restore RS1035 } + private static void EmitDiagnostics(SourceProductionContext context, ImmutableArray diagnostics) + { + foreach (Diagnostic diagnostic in diagnostics) + { + context.ReportDiagnostic(diagnostic); + } + } + /// /// Instrumentation helper for unit tests. /// diff --git a/src/libraries/System.Text.Json/gen/System.Text.Json.SourceGeneration.targets b/src/libraries/System.Text.Json/gen/System.Text.Json.SourceGeneration.targets index aa7ae500ce7870..2d4351a3eab530 100644 --- a/src/libraries/System.Text.Json/gen/System.Text.Json.SourceGeneration.targets +++ b/src/libraries/System.Text.Json/gen/System.Text.Json.SourceGeneration.targets @@ -31,7 +31,6 @@ - diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorDiagnosticsTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorDiagnosticsTests.cs index b8e3073a857cf5..5712a52072a110 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorDiagnosticsTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorDiagnosticsTests.cs @@ -5,6 +5,7 @@ using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; using Xunit; namespace System.Text.Json.SourceGeneration.UnitTests @@ -755,5 +756,33 @@ public partial class MyContext : JsonSerializerContext CompilationHelper.AssertEqualDiagnosticMessages(expectedDiagnostics, result.Diagnostics); } #endif + + [Fact] + public void Diagnostic_HasPragmaSuppressibleLocation() + { + // SYSLIB1038: JsonInclude attribute on inaccessible member (Warning, configurable). + string source = """ + #pragma warning disable SYSLIB1038 + using System.Text.Json.Serialization; + + namespace Test + { + public class MyClass + { + [JsonInclude] + private int PrivateField; + } + + [JsonSerializable(typeof(MyClass))] + public partial class JsonContext : JsonSerializerContext { } + } + """; + + Compilation compilation = CompilationHelper.CreateCompilation(source); + JsonSourceGeneratorResult result = CompilationHelper.RunJsonSourceGenerator(compilation, disableDiagnosticValidation: true); + var effective = CompilationWithAnalyzers.GetEffectiveDiagnostics(result.Diagnostics, compilation); + Diagnostic diagnostic = Assert.Single(effective, d => d.Id == "SYSLIB1038"); + Assert.True(diagnostic.IsSuppressed); + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorIncrementalTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorIncrementalTests.cs index d08c07c2e741ea..7be4413b44befa 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorIncrementalTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorIncrementalTests.cs @@ -145,7 +145,6 @@ public static void SourceGenModelDoesNotEncapsulateSymbolsOrCompilationData(Func { JsonSourceGeneratorResult result = CompilationHelper.RunJsonSourceGenerator(factory(), disableDiagnosticValidation: true); WalkObjectGraph(result.ContextGenerationSpecs); - WalkObjectGraph(result.Diagnostics); static void WalkObjectGraph(object obj) { diff --git a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs index 947d0bc0b049b5..1d1df92c107d9b 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Immutable; @@ -20,7 +20,7 @@ public partial class RegexGenerator private const string GeneratedRegexAttributeName = "System.Text.RegularExpressions.GeneratedRegexAttribute"; /// - /// Returns null if nothing to do, if there's an error to report, + /// Returns null if nothing to do, a if there's an error to report, /// or if the type was analyzed successfully. /// private static object? GetRegexMethodDataOrFailureDiagnostic( @@ -32,7 +32,7 @@ public partial class RegexGenerator // of being able to flag invalid use when [GeneratedRegex] is applied incorrectly. // Otherwise, if the ForAttributeWithMetadataName call excluded these, [GeneratedRegex] // could be applied to them and we wouldn't be able to issue a diagnostic. - return new DiagnosticData(DiagnosticDescriptors.RegexMemberMustHaveValidSignature, GetComparableLocation(context.TargetNode)); + return Diagnostic.Create(DiagnosticDescriptors.RegexMemberMustHaveValidSignature, context.TargetNode.GetLocation()); } var memberSyntax = (MemberDeclarationSyntax)context.TargetNode; @@ -62,19 +62,19 @@ public partial class RegexGenerator ImmutableArray boundAttributes = context.Attributes; if (boundAttributes.Length != 1) { - return new DiagnosticData(DiagnosticDescriptors.MultipleGeneratedRegexAttributes, GetComparableLocation(memberSyntax)); + return Diagnostic.Create(DiagnosticDescriptors.MultipleGeneratedRegexAttributes, memberSyntax.GetLocation()); } AttributeData generatedRegexAttr = boundAttributes[0]; if (generatedRegexAttr.ConstructorArguments.Any(ca => ca.Kind == TypedConstantKind.Error)) { - return new DiagnosticData(DiagnosticDescriptors.InvalidGeneratedRegexAttribute, GetComparableLocation(memberSyntax)); + return Diagnostic.Create(DiagnosticDescriptors.InvalidGeneratedRegexAttribute, memberSyntax.GetLocation()); } ImmutableArray items = generatedRegexAttr.ConstructorArguments; if (items.Length is 0 or > 4) { - return new DiagnosticData(DiagnosticDescriptors.InvalidGeneratedRegexAttribute, GetComparableLocation(memberSyntax)); + return Diagnostic.Create(DiagnosticDescriptors.InvalidGeneratedRegexAttribute, memberSyntax.GetLocation()); } string? pattern = items[0].Value as string; @@ -106,7 +106,7 @@ public partial class RegexGenerator if (pattern is null || cultureName is null) { - return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), "(null)"); + return Diagnostic.Create(DiagnosticDescriptors.InvalidRegexArguments, memberSyntax.GetLocation(), "(null)"); } bool nullableRegex; @@ -118,7 +118,7 @@ public partial class RegexGenerator regexMethodSymbol.Arity != 0 || !SymbolEqualityComparer.Default.Equals(regexMethodSymbol.ReturnType, regexSymbol)) { - return new DiagnosticData(DiagnosticDescriptors.RegexMemberMustHaveValidSignature, GetComparableLocation(memberSyntax)); + return Diagnostic.Create(DiagnosticDescriptors.RegexMemberMustHaveValidSignature, memberSyntax.GetLocation()); } nullableRegex = regexMethodSymbol.ReturnNullableAnnotation == NullableAnnotation.Annotated; @@ -132,7 +132,7 @@ public partial class RegexGenerator regexPropertySymbol.SetMethod is not null || !SymbolEqualityComparer.Default.Equals(regexPropertySymbol.Type, regexSymbol)) { - return new DiagnosticData(DiagnosticDescriptors.RegexMemberMustHaveValidSignature, GetComparableLocation(memberSyntax)); + return Diagnostic.Create(DiagnosticDescriptors.RegexMemberMustHaveValidSignature, memberSyntax.GetLocation()); } nullableRegex = regexPropertySymbol.NullableAnnotation == NullableAnnotation.Annotated; @@ -154,7 +154,7 @@ regexPropertySymbol.SetMethod is not null || } catch (Exception e) { - return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), e.Message); + return Diagnostic.Create(DiagnosticDescriptors.InvalidRegexArguments, memberSyntax.GetLocation(), e.Message); } if ((regexOptionsWithPatternOptions & RegexOptions.IgnoreCase) != 0 && !string.IsNullOrEmpty(cultureName)) @@ -162,7 +162,7 @@ regexPropertySymbol.SetMethod is not null || if ((regexOptions & RegexOptions.CultureInvariant) != 0) { // User passed in both a culture name and set RegexOptions.CultureInvariant which causes an explicit conflict. - return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), "cultureName"); + return Diagnostic.Create(DiagnosticDescriptors.InvalidRegexArguments, memberSyntax.GetLocation(), "cultureName"); } try @@ -171,7 +171,7 @@ regexPropertySymbol.SetMethod is not null || } catch (CultureNotFoundException) { - return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), "cultureName"); + return Diagnostic.Create(DiagnosticDescriptors.InvalidRegexArguments, memberSyntax.GetLocation(), "cultureName"); } } @@ -189,13 +189,13 @@ regexPropertySymbol.SetMethod is not null || RegexOptions.Singleline; if ((regexOptions & ~SupportedOptions) != 0) { - return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), "options"); + return Diagnostic.Create(DiagnosticDescriptors.InvalidRegexArguments, memberSyntax.GetLocation(), "options"); } // Validate the timeout if (matchTimeout is 0 or < -1) { - return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), "matchTimeout"); + return Diagnostic.Create(DiagnosticDescriptors.InvalidRegexArguments, memberSyntax.GetLocation(), "matchTimeout"); } // Determine the namespace the class is declared in, if any @@ -214,7 +214,7 @@ regexPropertySymbol.SetMethod is not null || var result = new RegexPatternAndSyntax( regexType, IsProperty: regexMemberSymbol is IPropertySymbol, - GetComparableLocation(memberSyntax), + memberSyntax.GetLocation(), regexMemberSymbol.Name, memberSyntax.Modifiers.ToString(), nullableRegex, @@ -246,21 +246,13 @@ SyntaxKind.StructDeclaration or SyntaxKind.RecordDeclaration or SyntaxKind.RecordStructDeclaration or SyntaxKind.InterfaceDeclaration; - - // Get a Location object that doesn't store a reference to the compilation. - // That allows it to compare equally across compilations. - static Location GetComparableLocation(SyntaxNode syntax) - { - var location = syntax.GetLocation(); - return Location.Create(location.SourceTree?.FilePath ?? string.Empty, location.SourceSpan, location.GetLineSpan().Span); - } } /// Data about a regex directly from the GeneratedRegex attribute. internal sealed record RegexPatternAndSyntax(RegexType DeclaringType, bool IsProperty, Location DiagnosticLocation, string MemberName, string Modifiers, bool NullableRegex, string Pattern, RegexOptions Options, int? MatchTimeout, CultureInfo Culture, CompilationData CompilationData); /// Data about a regex, including a fully parsed RegexTree and subsequent analysis. - internal sealed record RegexMethod(RegexType DeclaringType, bool IsProperty, Location DiagnosticLocation, string MemberName, string Modifiers, bool NullableRegex, string Pattern, RegexOptions Options, int? MatchTimeout, RegexTree Tree, AnalysisResults Analysis, CompilationData CompilationData) + internal sealed record RegexMethod(RegexType DeclaringType, bool IsProperty, string MemberName, string Modifiers, bool NullableRegex, string Pattern, RegexOptions Options, int? MatchTimeout, RegexTree Tree, AnalysisResults Analysis, CompilationData CompilationData) { public string? GeneratedName { get; set; } public bool IsDuplicate { get; set; } diff --git a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs index 98e3f1ac35c036..69174c2a508efd 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs @@ -45,10 +45,13 @@ internal record struct CompilationData(bool AllowUnsafe, bool CheckOverflow, Lan public void Initialize(IncrementalGeneratorInitializationContext context) { // Produces one entry per generated regex. This may be: - // - DiagnosticData in the case of a failure that should end the compilation + // - Diagnostic in the case of a failure that should end the compilation // - (RegexMethod regexMethod, string runnerFactoryImplementation, Dictionary requiredHelpers) in the case of valid regex - // - (RegexMethod regexMethod, string reason, DiagnosticData diagnostic) in the case of a limited-support regex - IncrementalValueProvider> results = + // - (RegexMethod regexMethod, string reason, Diagnostic diagnostic) in the case of a limited-support regex + // + // Location is threaded separately from the records so that it doesn't participate in + // record equality — this allows the incremental pipeline to cache results by value. + IncrementalValueProvider<(ImmutableArray Results, ImmutableArray Diagnostics)> collected = context.SyntaxProvider // Find all MethodDeclarationSyntax nodes attributed with GeneratedRegex and gather the required information. @@ -67,8 +70,10 @@ public void Initialize(IncrementalGeneratorInitializationContext context) // Filter out any parsing errors that resulted in null objects being returned. .Where(static m => m is not null) - // The input here will either be a DiagnosticData (in the case of something erroneous detected in GetRegexMethodDataOrFailureDiagnostic) + // The input here will either be a Diagnostic (in the case of something erroneous detected in GetRegexMethodDataOrFailureDiagnostic) // or it will be a RegexPatternAndSyntax containing all of the successfully parsed data from the attribute/method. + // This step parses the regex tree and checks whether full code generation is supported. + // The DiagnosticLocation is consumed here for diagnostic creation and not propagated further. .Select((methodOrDiagnostic, _) => { if (methodOrDiagnostic is RegexPatternAndSyntax method) @@ -77,11 +82,20 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { RegexTree regexTree = RegexParser.Parse(method.Pattern, method.Options | RegexOptions.Compiled, method.Culture); // make sure Compiled is included to get all optimizations applied to it AnalysisResults analysis = RegexTreeAnalyzer.Analyze(regexTree); - return new RegexMethod(method.DeclaringType, method.IsProperty, method.DiagnosticLocation, method.MemberName, method.Modifiers, method.NullableRegex, method.Pattern, method.Options, method.MatchTimeout, regexTree, analysis, method.CompilationData); + RegexMethod regexMethod = new(method.DeclaringType, method.IsProperty, method.MemberName, method.Modifiers, method.NullableRegex, method.Pattern, method.Options, method.MatchTimeout, regexTree, analysis, method.CompilationData); + + // If we're unable to generate a full implementation for this regex, report a diagnostic. + // We'll still output a limited implementation that just caches a new Regex(...). + if (!SupportsCodeGeneration(regexMethod, regexMethod.CompilationData.LanguageVersion, out string? reason)) + { + return (object)(regexMethod, reason, Diagnostic.Create(DiagnosticDescriptors.LimitedSourceGeneration, method.DiagnosticLocation), regexMethod.CompilationData); + } + + return regexMethod; } catch (Exception e) { - return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, method.DiagnosticLocation, e.Message); + return Diagnostic.Create(DiagnosticDescriptors.InvalidRegexArguments, method.DiagnosticLocation, e.Message); } } @@ -93,17 +107,10 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { if (state is not RegexMethod regexMethod) { - Debug.Assert(state is DiagnosticData); + Debug.Assert(state is Diagnostic or ValueTuple); return state; } - // If we're unable to generate a full implementation for this regex, report a diagnostic. - // We'll still output a limited implementation that just caches a new Regex(...). - if (!SupportsCodeGeneration(regexMethod, regexMethod.CompilationData.LanguageVersion, out string? reason)) - { - return (regexMethod, reason, new DiagnosticData(DiagnosticDescriptors.LimitedSourceGeneration, regexMethod.DiagnosticLocation), regexMethod.CompilationData); - } - // Generate the core logic for the regex. Dictionary requiredHelpers = new(); var sw = new StringWriter(); @@ -115,30 +122,47 @@ public void Initialize(IncrementalGeneratorInitializationContext context) return (regexMethod, sw.ToString(), requiredHelpers, regexMethod.CompilationData); }) - // Combine all of the generated text outputs into a single batch. We then generate a single source output from that batch. + // Combine all of the generated text outputs into a single batch, then split + // the source model from diagnostics so they can be emitted independently. .Collect() - - // Apply sequence equality comparison on the result array for incremental caching. - .WithComparer(new ObjectImmutableArraySequenceEqualityComparer()); - - // When there something to output, take all the generated strings and concatenate them to output, - // and raise all of the created diagnostics. - context.RegisterSourceOutput(results, static (context, results) => - { - // Report any top-level diagnostics. - bool allFailures = true; - foreach (object result in results) + .Select(static (results, _) => { - if (result is DiagnosticData d) - { - context.ReportDiagnostic(d.ToDiagnostic()); - } - else + ImmutableArray.Builder? diagnostics = null; + ImmutableArray.Builder? filteredResults = null; + + foreach (object result in results) { - allFailures = false; + if (result is Diagnostic d) + { + (diagnostics ??= ImmutableArray.CreateBuilder()).Add(d); + } + else if (result is ValueTuple limitedSupportResult) + { + (diagnostics ??= ImmutableArray.CreateBuilder()).Add(limitedSupportResult.Item3); + (filteredResults ??= ImmutableArray.CreateBuilder()).Add( + (limitedSupportResult.Item1, limitedSupportResult.Item2, limitedSupportResult.Item4)); + } + else + { + (filteredResults ??= ImmutableArray.CreateBuilder()).Add(result); + } } - } - if (allFailures) + + return ( + Results: filteredResults?.ToImmutable() ?? ImmutableArray.Empty, + Diagnostics: diagnostics?.ToImmutable() ?? ImmutableArray.Empty); + }); + + // Project to just the source model, discarding diagnostics. + // ObjectImmutableArraySequenceEqualityComparer applies element-wise equality over + // the heterogeneous result array, enabling Roslyn's incremental pipeline to skip + // re-emitting source when the model has not changed. + IncrementalValueProvider> sourceModel = + collected.Select(static (t, _) => t.Results).WithComparer(new ObjectImmutableArraySequenceEqualityComparer()); + + context.RegisterSourceOutput(sourceModel, static (context, results) => + { + if (results.IsEmpty) { return; } @@ -168,17 +192,16 @@ public void Initialize(IncrementalGeneratorInitializationContext context) // pair is the implementation used for the key. var emittedExpressions = new Dictionary<(string Pattern, RegexOptions Options, int? Timeout), RegexMethod>(); - // If we have any (RegexMethod regexMethod, string generatedName, string reason, DiagnosticData diagnostic), these are regexes for which we have - // limited support and need to simply output boilerplate. We need to emit their diagnostics. - // If we have any (RegexMethod regexMethod, string generatedName, string runnerFactoryImplementation, Dictionary requiredHelpers), + // If we have any (RegexMethod regexMethod, string reason, CompilationData compilationData), these are regexes for which we have + // limited support and need to simply output boilerplate. + // If we have any (RegexMethod regexMethod, string runnerFactoryImplementation, Dictionary requiredHelpers, CompilationData compilationData), // those are generated implementations to be emitted. We need to gather up their required helpers. Dictionary requiredHelpers = new(); foreach (object? result in results) { RegexMethod? regexMethod = null; - if (result is ValueTuple limitedSupportResult) + if (result is ValueTuple limitedSupportResult) { - context.ReportDiagnostic(limitedSupportResult.Item3.ToDiagnostic()); regexMethod = limitedSupportResult.Item1; } else if (result is ValueTuple, CompilationData> regexImpl) @@ -238,11 +261,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context) writer.Indent++; foreach (object? result in results) { - if (result is ValueTuple limitedSupportResult) + if (result is ValueTuple limitedSupportResult) { if (!limitedSupportResult.Item1.IsDuplicate) { - EmitRegexLimitedBoilerplate(writer, limitedSupportResult.Item1, limitedSupportResult.Item2, limitedSupportResult.Item4.LanguageVersion); + EmitRegexLimitedBoilerplate(writer, limitedSupportResult.Item1, limitedSupportResult.Item2, limitedSupportResult.Item3.LanguageVersion); writer.WriteLine(); } } @@ -290,6 +313,22 @@ public void Initialize(IncrementalGeneratorInitializationContext context) // Save out the source context.AddSource("RegexGenerator.g.cs", sw.ToString()); }); + + // Project to just the diagnostics, discarding the model. ImmutableArray does not + // implement value equality, so Roslyn's incremental pipeline uses reference equality — + // the callback fires on every compilation change. This is by design: diagnostic emission + // is cheap, and we need fresh SourceLocation instances that are pragma-suppressible + // (cf. https://github.com/dotnet/runtime/issues/92509). + IncrementalValueProvider> diagnosticResults = + collected.Select(static (t, _) => t.Diagnostics); + + context.RegisterSourceOutput(diagnosticResults, static (context, diagnostics) => + { + foreach (Diagnostic diagnostic in diagnostics) + { + context.ReportDiagnostic(diagnostic); + } + }); } /// Determines whether the passed in node supports C# code generation. @@ -351,17 +390,6 @@ static bool HasCaseInsensitiveBackReferences(RegexNode node) } } - /// Stores the data necessary to create a Diagnostic. - /// - /// Diagnostics do not have value equality semantics. Storing them in an object model - /// used in the pipeline can result in unnecessary recompilation. - /// - private sealed record class DiagnosticData(DiagnosticDescriptor descriptor, Location location, object? arg = null) - { - /// Create a from the data. - public Diagnostic ToDiagnostic() => Diagnostic.Create(descriptor, location, arg is null ? [] : [arg]); - } - private sealed class ObjectImmutableArraySequenceEqualityComparer : IEqualityComparer> { public bool Equals(ImmutableArray left, ImmutableArray right) diff --git a/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/RegexGeneratorHelper.netcoreapp.cs b/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/RegexGeneratorHelper.netcoreapp.cs index 677bf16d32ff0a..946b1d2f1be6e9 100644 --- a/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/RegexGeneratorHelper.netcoreapp.cs +++ b/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/RegexGeneratorHelper.netcoreapp.cs @@ -69,7 +69,7 @@ internal static byte[] CreateAssemblyImage(string source, string assemblyName) throw new InvalidOperationException(); } - private static async Task<(Compilation, GeneratorDriverRunResult)> RunGeneratorCore( + internal static async Task<(Compilation, GeneratorDriverRunResult)> RunGeneratorCore( string code, LanguageVersion langVersion = LanguageVersion.Preview, MetadataReference[]? additionalRefs = null, bool allowUnsafe = false, bool checkOverflow = true, CancellationToken cancellationToken = default) { var proj = new AdhocWorkspace() diff --git a/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/RegexGeneratorParserTests.cs b/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/RegexGeneratorParserTests.cs index b4d46d8274f57a..3ab5a2923ccf90 100644 --- a/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/RegexGeneratorParserTests.cs +++ b/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/RegexGeneratorParserTests.cs @@ -3,8 +3,10 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.DotNet.RemoteExecutor; using System.Collections.Generic; +using System.Collections.Immutable; using System.Diagnostics; using System.Linq; using System.Globalization; @@ -398,6 +400,26 @@ partial class C Assert.Equal("SYSLIB1043", Assert.Single(diagnostics).Id); } + [Fact] + public async Task Diagnostic_HasPragmaSuppressibleLocation() + { + // SYSLIB1044 (LimitedSourceGeneration) is emitted for case-insensitive backreferences. + string code = """ + #pragma warning disable SYSLIB1044 + using System.Text.RegularExpressions; + partial class C + { + [GeneratedRegex("(a)\\1", RegexOptions.IgnoreCase)] + private static partial Regex Method(); + } + """; + + (Compilation comp, GeneratorDriverRunResult result) = await RegexGeneratorHelper.RunGeneratorCore(code); + var effective = CompilationWithAnalyzers.GetEffectiveDiagnostics(result.Diagnostics, comp); + Diagnostic diagnostic = Assert.Single(effective, d => d.Id == "SYSLIB1044"); + Assert.True(diagnostic.IsSuppressed); + } + [Fact] public async Task Diagnostic_PropertyMustHaveGetter() {