From 5ab07b33dbd821dcbc9d069db8b1362d1ad8501f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 29 Jan 2026 22:59:00 +0000 Subject: [PATCH 1/9] Initial plan From 87827ddb7b6b05a2e98614e0eea31c5eda463993 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 29 Jan 2026 23:28:59 +0000 Subject: [PATCH 2/9] Add LibraryImportDiagnosticsAnalyzer and update generator to not emit diagnostics Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com> --- .../LibraryImportDiagnosticsAnalyzer.cs | 276 ++++++++++++++++++ .../LibraryImportGenerator.cs | 21 +- .../DiagnosticOr.cs | 10 + .../CSharpSourceGeneratorVerifier.cs | 13 + 4 files changed, 303 insertions(+), 17 deletions(-) create mode 100644 src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs new file mode 100644 index 00000000000000..af07913e31009a --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs @@ -0,0 +1,276 @@ +// 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.Collections.Immutable; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.DotnetRuntime.Extensions; +using Microsoft.Interop; + +namespace Microsoft.Interop.Analyzers +{ + /// + /// Analyzer that reports diagnostics for LibraryImport methods. + /// This analyzer runs the same diagnostic logic as LibraryImportGenerator + /// but reports diagnostics separately from the source generator. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class LibraryImportDiagnosticsAnalyzer : DiagnosticAnalyzer + { + public override ImmutableArray SupportedDiagnostics { get; } = + ImmutableArray.Create( + GeneratorDiagnostics.InvalidAttributedMethodSignature, + GeneratorDiagnostics.InvalidAttributedMethodContainingTypeMissingModifiers, + GeneratorDiagnostics.InvalidStringMarshallingConfiguration, + GeneratorDiagnostics.ParameterTypeNotSupported, + GeneratorDiagnostics.ReturnTypeNotSupported, + GeneratorDiagnostics.ParameterTypeNotSupportedWithDetails, + GeneratorDiagnostics.ReturnTypeNotSupportedWithDetails, + GeneratorDiagnostics.ParameterConfigurationNotSupported, + GeneratorDiagnostics.ReturnConfigurationNotSupported, + GeneratorDiagnostics.MarshalAsParameterConfigurationNotSupported, + GeneratorDiagnostics.MarshalAsReturnConfigurationNotSupported, + GeneratorDiagnostics.ConfigurationNotSupported, + GeneratorDiagnostics.ConfigurationValueNotSupported, + GeneratorDiagnostics.MarshallingAttributeConfigurationNotSupported, + GeneratorDiagnostics.CannotForwardToDllImport, + GeneratorDiagnostics.RequiresAllowUnsafeBlocks, + GeneratorDiagnostics.UnnecessaryParameterMarshallingInfo, + GeneratorDiagnostics.UnnecessaryReturnMarshallingInfo, + GeneratorDiagnostics.SizeOfInCollectionMustBeDefinedAtCallOutParam, + GeneratorDiagnostics.SizeOfInCollectionMustBeDefinedAtCallReturnValue, + GeneratorDiagnostics.LibraryImportUsageDoesNotFollowBestPractices); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(context => + { + // Nothing to do if the LibraryImportAttribute is not in the compilation + if (context.Compilation.GetBestTypeByMetadataName(TypeNames.LibraryImportAttribute) is null) + return; + + StubEnvironment env = new StubEnvironment( + context.Compilation, + context.Compilation.GetEnvironmentFlags()); + + context.RegisterSymbolAction(symbolContext => AnalyzeMethod(symbolContext, env), SymbolKind.Method); + }); + } + + private static void AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment env) + { + IMethodSymbol method = (IMethodSymbol)context.Symbol; + + // Only analyze methods with LibraryImportAttribute + AttributeData? libraryImportAttr = null; + foreach (AttributeData attr in method.GetAttributes()) + { + if (attr.AttributeClass?.ToDisplayString() == TypeNames.LibraryImportAttribute) + { + libraryImportAttr = attr; + break; + } + } + + if (libraryImportAttr is null) + return; + + // Find the method syntax + foreach (SyntaxReference syntaxRef in method.DeclaringSyntaxReferences) + { + if (syntaxRef.GetSyntax(context.CancellationToken) is MethodDeclarationSyntax methodSyntax) + { + AnalyzeMethodSyntax(context, methodSyntax, method, libraryImportAttr, env); + break; + } + } + } + + private static void AnalyzeMethodSyntax( + SymbolAnalysisContext context, + MethodDeclarationSyntax methodSyntax, + IMethodSymbol method, + AttributeData libraryImportAttr, + StubEnvironment env) + { + // Check for invalid method signature + DiagnosticInfo? invalidMethodDiagnostic = GetDiagnosticIfInvalidMethodForGeneration(methodSyntax, method); + if (invalidMethodDiagnostic is not null) + { + context.ReportDiagnostic(invalidMethodDiagnostic.ToDiagnostic()); + return; // Don't continue analysis if the method is invalid + } + + // Check for unsafe blocks requirement + if (context.Compilation.Options is not CSharpCompilationOptions { AllowUnsafe: true }) + { + context.ReportDiagnostic(DiagnosticInfo.Create(GeneratorDiagnostics.RequiresAllowUnsafeBlocks, null).ToDiagnostic()); + } + + // Get generator options + LibraryImportGeneratorOptions options = new(context.Options.AnalyzerConfigOptionsProvider.GlobalOptions); + + // Calculate stub information and collect diagnostics + var diagnostics = CalculateDiagnostics(methodSyntax, method, libraryImportAttr, env, options, context.CancellationToken); + + foreach (DiagnosticInfo diagnostic in diagnostics) + { + context.ReportDiagnostic(diagnostic.ToDiagnostic()); + } + } + + private static ImmutableArray CalculateDiagnostics( + MethodDeclarationSyntax originalSyntax, + IMethodSymbol symbol, + AttributeData libraryImportAttr, + StubEnvironment environment, + LibraryImportGeneratorOptions options, + CancellationToken ct) + { + ct.ThrowIfCancellationRequested(); + + var locations = new MethodSignatureDiagnosticLocations(originalSyntax); + var generatorDiagnostics = new GeneratorDiagnosticsBag( + new DiagnosticDescriptorProvider(), + locations, + SR.ResourceManager, + typeof(FxResources.Microsoft.Interop.LibraryImportGenerator.SR)); + + // Process the LibraryImport attribute + LibraryImportCompilationData libraryImportData = + ProcessLibraryImportAttribute(libraryImportAttr) ?? + new LibraryImportCompilationData("INVALID_CSHARP_SYNTAX"); + + if (libraryImportData.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshalling)) + { + // User specified StringMarshalling.Custom without specifying StringMarshallingCustomType + if (libraryImportData.StringMarshalling == StringMarshalling.Custom && libraryImportData.StringMarshallingCustomType is null) + { + generatorDiagnostics.ReportInvalidStringMarshallingConfiguration( + libraryImportAttr, symbol.Name, SR.InvalidStringMarshallingConfigurationMissingCustomType); + } + + // User specified something other than StringMarshalling.Custom while specifying StringMarshallingCustomType + if (libraryImportData.StringMarshalling != StringMarshalling.Custom && libraryImportData.StringMarshallingCustomType is not null) + { + generatorDiagnostics.ReportInvalidStringMarshallingConfiguration( + libraryImportAttr, symbol.Name, SR.InvalidStringMarshallingConfigurationNotCustom); + } + } + + // Check for unsupported LCIDConversion attribute + INamedTypeSymbol? lcidConversionAttrType = environment.LcidConversionAttrType; + if (lcidConversionAttrType is not null) + { + foreach (AttributeData attr in symbol.GetAttributes()) + { + if (SymbolEqualityComparer.Default.Equals(attr.AttributeClass, lcidConversionAttrType)) + { + generatorDiagnostics.ReportConfigurationNotSupported(attr, nameof(TypeNames.LCIDConversionAttribute)); + break; + } + } + } + + // Create the signature context to collect marshalling-related diagnostics + _ = SignatureContext.Create( + symbol, + DefaultMarshallingInfoParser.Create(environment, generatorDiagnostics, symbol, libraryImportData, libraryImportAttr), + environment, + new CodeEmitOptions(SkipInit: true), + typeof(LibraryImportGenerator).Assembly); + + // If forwarders are not being generated, we need to run stub generation logic to get those diagnostics too + if (!options.GenerateForwarders) + { + var signatureContext = SignatureContext.Create( + symbol, + DefaultMarshallingInfoParser.Create(environment, generatorDiagnostics, symbol, libraryImportData, libraryImportAttr), + environment, + new CodeEmitOptions(SkipInit: true), + typeof(LibraryImportGenerator).Assembly); + + IMarshallingGeneratorResolver resolver = DefaultMarshallingGeneratorResolver.Create( + environment.EnvironmentFlags, + MarshalDirection.ManagedToUnmanaged, + TypeNames.LibraryImportAttribute_ShortName, + []); + + // Check marshalling generators - this collects diagnostics for marshalling issues + _ = new ManagedToNativeStubGenerator( + signatureContext.ElementTypeInformation, + LibraryImportData.From(libraryImportData).SetLastError, + generatorDiagnostics, + resolver, + new CodeEmitOptions(SkipInit: true)); + } + + return generatorDiagnostics.Diagnostics.ToImmutableArray(); + } + + private static LibraryImportCompilationData? ProcessLibraryImportAttribute(AttributeData attrData) + { + // Found the LibraryImport, but it has an error so report the error. + // This is most likely an issue with targeting an incorrect TFM. + if (attrData.AttributeClass?.TypeKind is null or TypeKind.Error) + { + return null; + } + + if (attrData.ConstructorArguments.Length == 0) + { + return null; + } + + ImmutableDictionary namedArguments = ImmutableDictionary.CreateRange(attrData.NamedArguments); + + string? entryPoint = null; + if (namedArguments.TryGetValue(nameof(LibraryImportCompilationData.EntryPoint), out TypedConstant entryPointValue)) + { + if (entryPointValue.Value is not string) + { + return null; + } + entryPoint = (string)entryPointValue.Value!; + } + + return new LibraryImportCompilationData(attrData.ConstructorArguments[0].Value!.ToString()) + { + EntryPoint = entryPoint, + }.WithValuesFromNamedArguments(namedArguments); + } + + private static DiagnosticInfo? GetDiagnosticIfInvalidMethodForGeneration(MethodDeclarationSyntax methodSyntax, IMethodSymbol method) + { + // Verify the method has no generic types or defined implementation + // and is marked static and partial. + if (methodSyntax.TypeParameterList is not null + || methodSyntax.Body is not null + || !methodSyntax.Modifiers.Any(SyntaxKind.StaticKeyword) + || !methodSyntax.Modifiers.Any(SyntaxKind.PartialKeyword)) + { + return DiagnosticInfo.Create(GeneratorDiagnostics.InvalidAttributedMethodSignature, methodSyntax.Identifier.GetLocation(), method.Name); + } + + // Verify that the types the method is declared in are marked partial. + if (methodSyntax.Parent is TypeDeclarationSyntax typeDecl && !typeDecl.IsInPartialContext(out var nonPartialIdentifier)) + { + return DiagnosticInfo.Create(GeneratorDiagnostics.InvalidAttributedMethodContainingTypeMissingModifiers, methodSyntax.Identifier.GetLocation(), method.Name, nonPartialIdentifier); + } + + // Verify the method does not have a ref return + if (method.ReturnsByRef || method.ReturnsByRefReadonly) + { + return DiagnosticInfo.Create(GeneratorDiagnostics.ReturnConfigurationNotSupported, methodSyntax.Identifier.GetLocation(), "ref return", method.ToDisplayString()); + } + + return null; + } + } +} diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs index bf0d61c87234c4..59c57c2f707b8d 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs @@ -59,7 +59,8 @@ public void Initialize(IncrementalGeneratorInitializationContext context) : DiagnosticOr<(MethodDeclarationSyntax Syntax, IMethodSymbol Symbol)>.From((data.Syntax, data.Symbol)); }); - var methodsToGenerate = context.FilterAndReportDiagnostics(methodsWithDiagnostics); + // Filter methods but don't report diagnostics - the analyzer will handle that + var methodsToGenerate = methodsWithDiagnostics.FilterWithoutReportingDiagnostics(); // Compute generator options IncrementalValueProvider stubOptions = context.AnalyzerConfigOptionsProvider @@ -67,21 +68,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) IncrementalValueProvider stubEnvironment = context.CreateStubEnvironmentProvider(); - // Validate environment that is being used to generate stubs. - context.RegisterDiagnostics( - context.CompilationProvider - .Select((comp, ct) => comp.Options is CSharpCompilationOptions { AllowUnsafe: true }) - .Combine(attributedMethods.Collect()) - .SelectMany((data, ct) => - { - if (data.Right.IsEmpty // no attributed methods - || data.Left) // Unsafe code enabled - { - return ImmutableArray.Empty; - } - - return ImmutableArray.Create(DiagnosticInfo.Create(GeneratorDiagnostics.RequiresAllowUnsafeBlocks, null)); - })); + // Note: RequiresAllowUnsafeBlocks diagnostic is now reported by the analyzer IncrementalValuesProvider<(MemberDeclarationSyntax, ImmutableArray)> generateSingleStub = methodsToGenerate .Combine(stubEnvironment) @@ -104,7 +91,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) .WithComparer(Comparers.GeneratedSyntax) .WithTrackingName(StepNames.GenerateSingleStub); - context.RegisterDiagnostics(generateSingleStub.SelectMany((stubInfo, ct) => stubInfo.Item2)); + // Note: Diagnostics are now reported by the analyzer, so we don't register them here context.RegisterConcatenatedSyntaxOutputs(generateSingleStub.Select((data, ct) => data.Item1), "LibraryImports.g.cs"); } diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/DiagnosticOr.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/DiagnosticOr.cs index 2fe420651f7338..5bd5a5a9e15c15 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/DiagnosticOr.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/DiagnosticOr.cs @@ -110,5 +110,15 @@ public static IncrementalValuesProvider FilterAndReportDiagnostics(this In ctx.RegisterDiagnostics(diagnostics); return values; } + + /// + /// Filters the by whether or not the element has a value, discarding diagnostics. + /// Use this when diagnostics are reported by an analyzer instead of the generator. + /// + public static IncrementalValuesProvider FilterWithoutReportingDiagnostics(this IncrementalValuesProvider> diagnosticOrValues) + { + var (values, _) = diagnosticOrValues.Split(); + return values; + } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs index 3836d0243951e1..ee4763a5447c6d 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; using System.Threading; @@ -11,6 +12,7 @@ using Microsoft.CodeAnalysis.CSharp.Testing; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; +using Microsoft.Interop.Analyzers; namespace Microsoft.Interop.UnitTests.Verifiers { @@ -107,6 +109,17 @@ public Test(bool referenceAncillaryInterop) SolutionTransforms.Add(CSharpVerifierHelper.GetAllDiagnosticsEnabledTransform(GetDiagnosticAnalyzers())); } + protected override IEnumerable GetDiagnosticAnalyzers() + { + // Include the LibraryImportDiagnosticsAnalyzer so it runs alongside the source generator. + // This is needed because diagnostics have been moved from the generator to the analyzer. + foreach (var analyzer in base.GetDiagnosticAnalyzers()) + { + yield return analyzer; + } + yield return new LibraryImportDiagnosticsAnalyzer(); + } + protected override CompilationWithAnalyzers CreateCompilationWithAnalyzers(Compilation compilation, ImmutableArray analyzers, AnalyzerOptions options, CancellationToken cancellationToken) { return new CompilationWithAnalyzers( From 4e3556ed9f5b82564c588215a76ce97a5d336ede Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 29 Jan 2026 23:33:42 +0000 Subject: [PATCH 3/9] Fix duplicate diagnostics in LibraryImportDiagnosticsAnalyzer Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com> --- .../Analyzers/LibraryImportDiagnosticsAnalyzer.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs index af07913e31009a..55aff9957dcc59 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs @@ -179,7 +179,7 @@ private static ImmutableArray CalculateDiagnostics( } // Create the signature context to collect marshalling-related diagnostics - _ = SignatureContext.Create( + var signatureContext = SignatureContext.Create( symbol, DefaultMarshallingInfoParser.Create(environment, generatorDiagnostics, symbol, libraryImportData, libraryImportAttr), environment, @@ -189,13 +189,6 @@ private static ImmutableArray CalculateDiagnostics( // If forwarders are not being generated, we need to run stub generation logic to get those diagnostics too if (!options.GenerateForwarders) { - var signatureContext = SignatureContext.Create( - symbol, - DefaultMarshallingInfoParser.Create(environment, generatorDiagnostics, symbol, libraryImportData, libraryImportAttr), - environment, - new CodeEmitOptions(SkipInit: true), - typeof(LibraryImportGenerator).Assembly); - IMarshallingGeneratorResolver resolver = DefaultMarshallingGeneratorResolver.Create( environment.EnvironmentFlags, MarshalDirection.ManagedToUnmanaged, From b0d4d926543bcea9cf32719e636e86a07c874031 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 29 Jan 2026 23:44:18 +0000 Subject: [PATCH 4/9] Address code review feedback - fix RequiresAllowUnsafeBlocks and conditional analyzer loading Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com> --- .../LibraryImportDiagnosticsAnalyzer.cs | 37 +++++++++++++++---- .../CSharpSourceGeneratorVerifier.cs | 12 ++++-- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs index 55aff9957dcc59..c3875f22d18d49 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs @@ -59,11 +59,34 @@ public override void Initialize(AnalysisContext context) context.Compilation, context.Compilation.GetEnvironmentFlags()); - context.RegisterSymbolAction(symbolContext => AnalyzeMethod(symbolContext, env), SymbolKind.Method); + // Track if we found any LibraryImport methods to report RequiresAllowUnsafeBlocks once + bool foundLibraryImportMethod = false; + bool unsafeEnabled = context.Compilation.Options is CSharpCompilationOptions { AllowUnsafe: true }; + + context.RegisterSymbolAction(symbolContext => + { + if (AnalyzeMethod(symbolContext, env)) + { + foundLibraryImportMethod = true; + } + }, SymbolKind.Method); + + // Report RequiresAllowUnsafeBlocks once per compilation if there are LibraryImport methods and unsafe is not enabled + context.RegisterCompilationEndAction(endContext => + { + if (foundLibraryImportMethod && !unsafeEnabled) + { + endContext.ReportDiagnostic(DiagnosticInfo.Create(GeneratorDiagnostics.RequiresAllowUnsafeBlocks, null).ToDiagnostic()); + } + }); }); } - private static void AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment env) + /// + /// Analyzes a method for LibraryImport diagnostics. + /// + /// True if the method has LibraryImportAttribute, false otherwise. + private static bool AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment env) { IMethodSymbol method = (IMethodSymbol)context.Symbol; @@ -79,7 +102,7 @@ private static void AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment } if (libraryImportAttr is null) - return; + return false; // Find the method syntax foreach (SyntaxReference syntaxRef in method.DeclaringSyntaxReferences) @@ -90,6 +113,8 @@ private static void AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment break; } } + + return true; } private static void AnalyzeMethodSyntax( @@ -107,11 +132,7 @@ private static void AnalyzeMethodSyntax( return; // Don't continue analysis if the method is invalid } - // Check for unsafe blocks requirement - if (context.Compilation.Options is not CSharpCompilationOptions { AllowUnsafe: true }) - { - context.ReportDiagnostic(DiagnosticInfo.Create(GeneratorDiagnostics.RequiresAllowUnsafeBlocks, null).ToDiagnostic()); - } + // Note: RequiresAllowUnsafeBlocks is reported once per compilation in Initialize method // Get generator options LibraryImportGeneratorOptions options = new(context.Options.AnalyzerConfigOptionsProvider.GlobalOptions); diff --git a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs index ee4763a5447c6d..2122feb66c14c3 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs @@ -12,6 +12,7 @@ using Microsoft.CodeAnalysis.CSharp.Testing; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; +using Microsoft.Interop; using Microsoft.Interop.Analyzers; namespace Microsoft.Interop.UnitTests.Verifiers @@ -111,13 +112,18 @@ public Test(bool referenceAncillaryInterop) protected override IEnumerable GetDiagnosticAnalyzers() { - // Include the LibraryImportDiagnosticsAnalyzer so it runs alongside the source generator. - // This is needed because diagnostics have been moved from the generator to the analyzer. + // Include the corresponding diagnostics analyzer when testing specific generators. + // This is needed because diagnostics have been moved from the generators to the analyzers. foreach (var analyzer in base.GetDiagnosticAnalyzers()) { yield return analyzer; } - yield return new LibraryImportDiagnosticsAnalyzer(); + + // Only add the LibraryImportDiagnosticsAnalyzer when testing LibraryImportGenerator + if (typeof(TSourceGenerator) == typeof(LibraryImportGenerator)) + { + yield return new LibraryImportDiagnosticsAnalyzer(); + } } protected override CompilationWithAnalyzers CreateCompilationWithAnalyzers(Compilation compilation, ImmutableArray analyzers, AnalyzerOptions options, CancellationToken cancellationToken) From deaee03b5a5ea028d53f38b6fccdc170a02ebb70 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 29 Jan 2026 23:50:29 +0000 Subject: [PATCH 5/9] Add LibraryImportDiagnosticsAnalyzer alongside generator (generator still reports diagnostics) Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com> --- .../LibraryImportGenerator.cs | 21 +++++++++++++++---- .../CSharpSourceGeneratorVerifier.cs | 11 ++-------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs index 59c57c2f707b8d..bf0d61c87234c4 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs @@ -59,8 +59,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) : DiagnosticOr<(MethodDeclarationSyntax Syntax, IMethodSymbol Symbol)>.From((data.Syntax, data.Symbol)); }); - // Filter methods but don't report diagnostics - the analyzer will handle that - var methodsToGenerate = methodsWithDiagnostics.FilterWithoutReportingDiagnostics(); + var methodsToGenerate = context.FilterAndReportDiagnostics(methodsWithDiagnostics); // Compute generator options IncrementalValueProvider stubOptions = context.AnalyzerConfigOptionsProvider @@ -68,7 +67,21 @@ public void Initialize(IncrementalGeneratorInitializationContext context) IncrementalValueProvider stubEnvironment = context.CreateStubEnvironmentProvider(); - // Note: RequiresAllowUnsafeBlocks diagnostic is now reported by the analyzer + // Validate environment that is being used to generate stubs. + context.RegisterDiagnostics( + context.CompilationProvider + .Select((comp, ct) => comp.Options is CSharpCompilationOptions { AllowUnsafe: true }) + .Combine(attributedMethods.Collect()) + .SelectMany((data, ct) => + { + if (data.Right.IsEmpty // no attributed methods + || data.Left) // Unsafe code enabled + { + return ImmutableArray.Empty; + } + + return ImmutableArray.Create(DiagnosticInfo.Create(GeneratorDiagnostics.RequiresAllowUnsafeBlocks, null)); + })); IncrementalValuesProvider<(MemberDeclarationSyntax, ImmutableArray)> generateSingleStub = methodsToGenerate .Combine(stubEnvironment) @@ -91,7 +104,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) .WithComparer(Comparers.GeneratedSyntax) .WithTrackingName(StepNames.GenerateSingleStub); - // Note: Diagnostics are now reported by the analyzer, so we don't register them here + context.RegisterDiagnostics(generateSingleStub.SelectMany((stubInfo, ct) => stubInfo.Item2)); context.RegisterConcatenatedSyntaxOutputs(generateSingleStub.Select((data, ct) => data.Item1), "LibraryImports.g.cs"); } diff --git a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs index 2122feb66c14c3..79a0609e490289 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs @@ -12,8 +12,6 @@ using Microsoft.CodeAnalysis.CSharp.Testing; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; -using Microsoft.Interop; -using Microsoft.Interop.Analyzers; namespace Microsoft.Interop.UnitTests.Verifiers { @@ -113,17 +111,12 @@ public Test(bool referenceAncillaryInterop) protected override IEnumerable GetDiagnosticAnalyzers() { // Include the corresponding diagnostics analyzer when testing specific generators. - // This is needed because diagnostics have been moved from the generators to the analyzers. + // Note: The analyzer is available alongside the generator but diagnostics are still + // reported by the generator for now. This will change as we migrate diagnostics to analyzers. foreach (var analyzer in base.GetDiagnosticAnalyzers()) { yield return analyzer; } - - // Only add the LibraryImportDiagnosticsAnalyzer when testing LibraryImportGenerator - if (typeof(TSourceGenerator) == typeof(LibraryImportGenerator)) - { - yield return new LibraryImportDiagnosticsAnalyzer(); - } } protected override CompilationWithAnalyzers CreateCompilationWithAnalyzers(Compilation compilation, ImmutableArray analyzers, AnalyzerOptions options, CancellationToken cancellationToken) From f8ec08c4d11b2dd8f897c6a4528378a3e71ad4ce Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 00:46:51 +0000 Subject: [PATCH 6/9] Update CSharpSourceGeneratorVerifier to use TAnalyzer type parameter per review feedback Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com> --- .../LibraryImportGenerator.cs | 21 ++++--------------- .../ByValueContentsMarshalling.cs | 2 +- .../ComClassGeneratorDiagnostics.cs | 2 +- .../ComClassGeneratorOutputShape.cs | 2 +- .../ComInterfaceGeneratorOutputShape.cs | 8 +++---- .../CompileFails.cs | 2 +- .../Compiles.cs | 6 +++--- .../TargetSignatureTests.cs | 2 +- .../VerifyCompilationTest.cs | 4 +++- .../CSharpSourceGeneratorVerifier.cs | 7 +++---- .../AdditionalAttributesOnStub.cs | 10 +++++---- .../AttributeForwarding.cs | 2 +- .../ByValueContentsMarshalling.cs | 2 +- .../CompileFails.cs | 2 +- .../Compiles.cs | 12 ++++++----- .../Diagnostics.cs | 4 ++-- 16 files changed, 40 insertions(+), 48 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs index bf0d61c87234c4..59c57c2f707b8d 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs @@ -59,7 +59,8 @@ public void Initialize(IncrementalGeneratorInitializationContext context) : DiagnosticOr<(MethodDeclarationSyntax Syntax, IMethodSymbol Symbol)>.From((data.Syntax, data.Symbol)); }); - var methodsToGenerate = context.FilterAndReportDiagnostics(methodsWithDiagnostics); + // Filter methods but don't report diagnostics - the analyzer will handle that + var methodsToGenerate = methodsWithDiagnostics.FilterWithoutReportingDiagnostics(); // Compute generator options IncrementalValueProvider stubOptions = context.AnalyzerConfigOptionsProvider @@ -67,21 +68,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) IncrementalValueProvider stubEnvironment = context.CreateStubEnvironmentProvider(); - // Validate environment that is being used to generate stubs. - context.RegisterDiagnostics( - context.CompilationProvider - .Select((comp, ct) => comp.Options is CSharpCompilationOptions { AllowUnsafe: true }) - .Combine(attributedMethods.Collect()) - .SelectMany((data, ct) => - { - if (data.Right.IsEmpty // no attributed methods - || data.Left) // Unsafe code enabled - { - return ImmutableArray.Empty; - } - - return ImmutableArray.Create(DiagnosticInfo.Create(GeneratorDiagnostics.RequiresAllowUnsafeBlocks, null)); - })); + // Note: RequiresAllowUnsafeBlocks diagnostic is now reported by the analyzer IncrementalValuesProvider<(MemberDeclarationSyntax, ImmutableArray)> generateSingleStub = methodsToGenerate .Combine(stubEnvironment) @@ -104,7 +91,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) .WithComparer(Comparers.GeneratedSyntax) .WithTrackingName(StepNames.GenerateSingleStub); - context.RegisterDiagnostics(generateSingleStub.SelectMany((stubInfo, ct) => stubInfo.Item2)); + // Note: Diagnostics are now reported by the analyzer, so we don't register them here context.RegisterConcatenatedSyntaxOutputs(generateSingleStub.Select((data, ct) => data.Item1), "LibraryImports.g.cs"); } diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ByValueContentsMarshalling.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ByValueContentsMarshalling.cs index fdd2d33d6e7d27..3bd2bae74d92d0 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ByValueContentsMarshalling.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ByValueContentsMarshalling.cs @@ -10,7 +10,7 @@ using Xunit; using static Microsoft.Interop.UnitTests.TestUtils; using StringMarshalling = System.Runtime.InteropServices.StringMarshalling; -using VerifyComInterfaceGenerator = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyComInterfaceGenerator = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace ComInterfaceGenerator.Unit.Tests { diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComClassGeneratorDiagnostics.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComClassGeneratorDiagnostics.cs index 13d63a83333e2d..f07e6d16b6322d 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComClassGeneratorDiagnostics.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComClassGeneratorDiagnostics.cs @@ -7,7 +7,7 @@ using Microsoft.CodeAnalysis.Testing; using Microsoft.Interop; using Xunit; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace ComInterfaceGenerator.Unit.Tests { diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComClassGeneratorOutputShape.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComClassGeneratorOutputShape.cs index c00ff13aae59f3..c61e00b0a3d7e0 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComClassGeneratorOutputShape.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComClassGeneratorOutputShape.cs @@ -8,7 +8,7 @@ using Microsoft.CodeAnalysis.Testing; using Xunit; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace ComInterfaceGenerator.Unit.Tests { diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComInterfaceGeneratorOutputShape.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComInterfaceGeneratorOutputShape.cs index efc4bc7f7c35f5..9ab81347a93dcb 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComInterfaceGeneratorOutputShape.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ComInterfaceGeneratorOutputShape.cs @@ -18,7 +18,7 @@ using SourceGenerators.Tests; using Xunit; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace ComInterfaceGenerator.Unit.Tests { @@ -149,7 +149,7 @@ partial interface J : I } """; - var test = new VerifyCompilationTest(false) + var test = new VerifyCompilationTest(false) { TestCode = source, TestBehaviors = TestBehaviors.SkipGeneratedSourcesCheck | TestBehaviors.SkipGeneratedCodeCheck, @@ -198,7 +198,7 @@ partial interface K : J } """; - var test = new VerifyCompilationTest(false) + var test = new VerifyCompilationTest(false) { TestCode = source, TestBehaviors = TestBehaviors.SkipGeneratedSourcesCheck | TestBehaviors.SkipGeneratedCodeCheck, @@ -256,7 +256,7 @@ partial interface IDerivedIface : IFoo } """; - var test = new VerifyCompilationTest(false) + var test = new VerifyCompilationTest(false) { TestCode = source, TestBehaviors = TestBehaviors.SkipGeneratedSourcesCheck | TestBehaviors.SkipGeneratedCodeCheck, diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/CompileFails.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/CompileFails.cs index 3e959e4d2ac744..a908d720673dfc 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/CompileFails.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/CompileFails.cs @@ -15,7 +15,7 @@ using Xunit; using static Microsoft.Interop.UnitTests.TestUtils; using StringMarshalling = System.Runtime.InteropServices.StringMarshalling; -using VerifyComInterfaceGenerator = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyComInterfaceGenerator = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace ComInterfaceGenerator.Unit.Tests { diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/Compiles.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/Compiles.cs index ba4928fec1d367..b255a2a29a44df 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/Compiles.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/Compiles.cs @@ -12,8 +12,8 @@ using Microsoft.CodeAnalysis.Testing; using Microsoft.Interop.UnitTests; using Xunit; -using VerifyComInterfaceGenerator = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; -using VerifyVTableGenerator = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyComInterfaceGenerator = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyVTableGenerator = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace ComInterfaceGenerator.Unit.Tests { @@ -410,7 +410,7 @@ public partial interface MyOtherInterface : IMyInterface } """; - var test = new VerifyCompilationTest(false) + var test = new VerifyCompilationTest(false) { TestCode = source, TestBehaviors = TestBehaviors.SkipGeneratedSourcesCheck | TestBehaviors.SkipGeneratedCodeCheck, diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/TargetSignatureTests.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/TargetSignatureTests.cs index dbd18badbb78a7..2e7bb5815a27fd 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/TargetSignatureTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/TargetSignatureTests.cs @@ -15,7 +15,7 @@ using Microsoft.Interop; using Xunit; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace ComInterfaceGenerator.Unit.Tests { diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/VerifyCompilationTest.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/VerifyCompilationTest.cs index 09d6b5e2a58aed..39c9fb01f1add6 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/VerifyCompilationTest.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/VerifyCompilationTest.cs @@ -3,12 +3,14 @@ using System; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.Interop.UnitTests; namespace ComInterfaceGenerator.Unit.Tests { - internal class VerifyCompilationTest : Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier.Test + internal class VerifyCompilationTest : Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier.Test where T : new() + where TAnalyzer : DiagnosticAnalyzer, new() { public required Action CompilationVerifier { get; init; } diff --git a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs index 79a0609e490289..f98b9a0abd344c 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs @@ -15,8 +15,9 @@ namespace Microsoft.Interop.UnitTests.Verifiers { - public static class CSharpSourceGeneratorVerifier + public static class CSharpSourceGeneratorVerifier where TSourceGenerator : new() + where TAnalyzer : DiagnosticAnalyzer, new() { public static DiagnosticResult Diagnostic(string diagnosticId) => new DiagnosticResult(diagnosticId, DiagnosticSeverity.Error); @@ -110,13 +111,11 @@ public Test(bool referenceAncillaryInterop) protected override IEnumerable GetDiagnosticAnalyzers() { - // Include the corresponding diagnostics analyzer when testing specific generators. - // Note: The analyzer is available alongside the generator but diagnostics are still - // reported by the generator for now. This will change as we migrate diagnostics to analyzers. foreach (var analyzer in base.GetDiagnosticAnalyzers()) { yield return analyzer; } + yield return new TAnalyzer(); } protected override CompilationWithAnalyzers CreateCompilationWithAnalyzers(Compilation compilation, ImmutableArray analyzers, AnalyzerOptions options, CancellationToken cancellationToken) diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AdditionalAttributesOnStub.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AdditionalAttributesOnStub.cs index e43937d75fa286..ebb295f6d3fe79 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AdditionalAttributesOnStub.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AdditionalAttributesOnStub.cs @@ -3,6 +3,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; using Microsoft.Interop; using Microsoft.Interop.UnitTests; @@ -12,7 +13,7 @@ using System.Threading.Tasks; using Xunit; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace LibraryImportGenerator.UnitTests { @@ -243,7 +244,7 @@ static class Marshaller private static Task VerifyDownlevelSourceGeneratorAsync(string source, string typeName, string methodName, string? attributeName, bool attributeAdded, TestTargetFramework targetFramework) { - AttributeAddedTest test = new(typeName, methodName, attributeName, attributeAdded, targetFramework) + AttributeAddedTest test = new(typeName, methodName, attributeName, attributeAdded, targetFramework) { TestCode = source, TestBehaviors = TestBehaviors.SkipGeneratedSourcesCheck @@ -253,7 +254,7 @@ private static Task VerifyDownlevelSourceGeneratorAsync(string source, string ty private static Task VerifySourceGeneratorAsync(string source, string typeName, string methodName, string? attributeName, bool attributeAdded) { - AttributeAddedTest test = new(typeName, methodName, attributeName, attributeAdded) + AttributeAddedTest test = new(typeName, methodName, attributeName, attributeAdded) { TestCode = source, TestBehaviors = TestBehaviors.SkipGeneratedSourcesCheck @@ -261,8 +262,9 @@ private static Task VerifySourceGeneratorAsync(string source, string typeName, s return test.RunAsync(); } - class AttributeAddedTest : Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier.Test + class AttributeAddedTest : Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier.Test where TSourceGenerator : new() + where TAnalyzer : DiagnosticAnalyzer, new() { private readonly string _typeName; private readonly string _methodName; diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AttributeForwarding.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AttributeForwarding.cs index 5a2b4ba19468f8..6a6de932786ad7 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AttributeForwarding.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AttributeForwarding.cs @@ -12,7 +12,7 @@ using System.Runtime.InteropServices; using System.Threading.Tasks; using Xunit; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace LibraryImportGenerator.UnitTests { diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/ByValueContentsMarshalling.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/ByValueContentsMarshalling.cs index e632481d52109f..90787cb7a02bd7 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/ByValueContentsMarshalling.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/ByValueContentsMarshalling.cs @@ -8,7 +8,7 @@ using Microsoft.Interop; using Xunit; using static Microsoft.Interop.UnitTests.TestUtils; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace LibraryImportGenerator.UnitTests { diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CompileFails.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CompileFails.cs index 952f8ef2890c81..8b6e94c043b7dc 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CompileFails.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CompileFails.cs @@ -16,7 +16,7 @@ using Xunit; using StringMarshalling = System.Runtime.InteropServices.StringMarshalling; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace LibraryImportGenerator.UnitTests { diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs index afa6af2bb4c9a6..225ae48288ebc1 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs @@ -13,12 +13,13 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; using Microsoft.CodeAnalysis.Text; using Microsoft.DotNet.XUnitExtensions.Attributes; using Microsoft.Interop.UnitTests; using Xunit; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace LibraryImportGenerator.UnitTests { @@ -578,7 +579,7 @@ public async Task ValidateSnippetsFallbackForwarder(string id, string source, Te await test.RunAsync(); } - class FallbackForwarderTest : Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier.Test + class FallbackForwarderTest : Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier.Test { private readonly bool _expectFallbackForwarder; @@ -747,7 +748,7 @@ public async Task ValidateNoGeneratedOutputForNoImport() public class Basic { } """; - var test = new NoChangeTest() + var test = new NoChangeTest() { TestCode = source, TestBehaviors = TestBehaviors.SkipGeneratedSourcesCheck @@ -767,7 +768,7 @@ public async Task ValidateNoGeneratedOutputForNoImportDownlevel(TestTargetFramew public class Basic { } """; - var test = new NoChangeTest(framework) + var test = new NoChangeTest(framework) { TestCode = source, TestBehaviors = TestBehaviors.SkipGeneratedSourcesCheck @@ -776,8 +777,9 @@ public class Basic { } await test.RunAsync(); } - class NoChangeTest : Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier.Test + class NoChangeTest : Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier.Test where TSourceGenerator : new() + where TAnalyzer : DiagnosticAnalyzer, new() { public NoChangeTest() : base(referenceAncillaryInterop: false) diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs index e064a4b951d290..67a4ea9a74fbd0 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs @@ -14,7 +14,7 @@ using Xunit; using StringMarshalling = Microsoft.Interop.StringMarshalling; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; namespace LibraryImportGenerator.UnitTests { @@ -269,7 +269,7 @@ public Native(string s) { } .WithArguments($"{nameof(TypeNames.LibraryImportAttribute)}{Type.Delimiter}{nameof(StringMarshalling)}={nameof(StringMarshalling)}{Type.Delimiter}{nameof(StringMarshalling.Custom)}") ]; - var test = new Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier.Test(TestTargetFramework.Standard2_0) + var test = new Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier.Test(TestTargetFramework.Standard2_0) { TestCode = source, TestBehaviors = TestBehaviors.SkipGeneratedSourcesCheck From 6232571264fde78a1769bc67efc896b3c7720ab4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 01:12:55 +0000 Subject: [PATCH 7/9] Address review feedback: use SymbolEqualityComparer, return early on null, remove helper method Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com> --- .../LibraryImportDiagnosticsAnalyzer.cs | 19 ++++++++++++------- .../LibraryImportGenerator.cs | 4 ++-- .../DiagnosticOr.cs | 10 ---------- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs index c3875f22d18d49..20fbeaf7daa0ff 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs @@ -52,7 +52,8 @@ public override void Initialize(AnalysisContext context) context.RegisterCompilationStartAction(context => { // Nothing to do if the LibraryImportAttribute is not in the compilation - if (context.Compilation.GetBestTypeByMetadataName(TypeNames.LibraryImportAttribute) is null) + INamedTypeSymbol? libraryImportAttrType = context.Compilation.GetBestTypeByMetadataName(TypeNames.LibraryImportAttribute); + if (libraryImportAttrType is null) return; StubEnvironment env = new StubEnvironment( @@ -65,7 +66,7 @@ public override void Initialize(AnalysisContext context) context.RegisterSymbolAction(symbolContext => { - if (AnalyzeMethod(symbolContext, env)) + if (AnalyzeMethod(symbolContext, env, libraryImportAttrType)) { foundLibraryImportMethod = true; } @@ -86,7 +87,7 @@ public override void Initialize(AnalysisContext context) /// Analyzes a method for LibraryImport diagnostics. /// /// True if the method has LibraryImportAttribute, false otherwise. - private static bool AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment env) + private static bool AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment env, INamedTypeSymbol libraryImportAttrType) { IMethodSymbol method = (IMethodSymbol)context.Symbol; @@ -94,7 +95,7 @@ private static bool AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment AttributeData? libraryImportAttr = null; foreach (AttributeData attr in method.GetAttributes()) { - if (attr.AttributeClass?.ToDisplayString() == TypeNames.LibraryImportAttribute) + if (SymbolEqualityComparer.Default.Equals(attr.AttributeClass, libraryImportAttrType)) { libraryImportAttr = attr; break; @@ -164,9 +165,13 @@ private static ImmutableArray CalculateDiagnostics( typeof(FxResources.Microsoft.Interop.LibraryImportGenerator.SR)); // Process the LibraryImport attribute - LibraryImportCompilationData libraryImportData = - ProcessLibraryImportAttribute(libraryImportAttr) ?? - new LibraryImportCompilationData("INVALID_CSHARP_SYNTAX"); + LibraryImportCompilationData? libraryImportData = ProcessLibraryImportAttribute(libraryImportAttr); + + // If we can't parse the attribute, we have an invalid compilation - stop processing + if (libraryImportData is null) + { + return generatorDiagnostics.Diagnostics.ToImmutableArray(); + } if (libraryImportData.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshalling)) { diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs index 59c57c2f707b8d..d7aef45d017a10 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs @@ -59,8 +59,8 @@ public void Initialize(IncrementalGeneratorInitializationContext context) : DiagnosticOr<(MethodDeclarationSyntax Syntax, IMethodSymbol Symbol)>.From((data.Syntax, data.Symbol)); }); - // Filter methods but don't report diagnostics - the analyzer will handle that - var methodsToGenerate = methodsWithDiagnostics.FilterWithoutReportingDiagnostics(); + // Filter methods - diagnostics are reported by the analyzer, not the generator + var (methodsToGenerate, _) = methodsWithDiagnostics.Split(); // Compute generator options IncrementalValueProvider stubOptions = context.AnalyzerConfigOptionsProvider diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/DiagnosticOr.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/DiagnosticOr.cs index 5bd5a5a9e15c15..2fe420651f7338 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/DiagnosticOr.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/DiagnosticOr.cs @@ -110,15 +110,5 @@ public static IncrementalValuesProvider FilterAndReportDiagnostics(this In ctx.RegisterDiagnostics(diagnostics); return values; } - - /// - /// Filters the by whether or not the element has a value, discarding diagnostics. - /// Use this when diagnostics are reported by an analyzer instead of the generator. - /// - public static IncrementalValuesProvider FilterWithoutReportingDiagnostics(this IncrementalValuesProvider> diagnosticOrValues) - { - var (values, _) = diagnosticOrValues.Split(); - return values; - } } } From 7a1d4df1ebd014039fadce38abb6a50a6eb6fa28 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 01:38:29 +0000 Subject: [PATCH 8/9] Filter diagnostics earlier in pipeline and run diagnostic tests with analyzer only Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com> --- .../LibraryImportGenerator.cs | 45 +++++++++++------ .../Verifiers/CSharpAnalyzerVerifier.cs | 9 +++- .../Diagnostics.cs | 50 +++++++------------ 3 files changed, 56 insertions(+), 48 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs index d7aef45d017a10..30c9aa027199b5 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs @@ -50,17 +50,9 @@ public void Initialize(IncrementalGeneratorInitializationContext context) .Where( static modelData => modelData is not null); - // Validate if attributed methods can have source generated - var methodsWithDiagnostics = attributedMethods.Select(static (data, ct) => - { - DiagnosticInfo? diagnostic = GetDiagnosticIfInvalidMethodForGeneration(data.Syntax, data.Symbol); - return diagnostic is not null - ? DiagnosticOr<(MethodDeclarationSyntax Syntax, IMethodSymbol Symbol)>.From(diagnostic) - : DiagnosticOr<(MethodDeclarationSyntax Syntax, IMethodSymbol Symbol)>.From((data.Syntax, data.Symbol)); - }); - - // Filter methods - diagnostics are reported by the analyzer, not the generator - var (methodsToGenerate, _) = methodsWithDiagnostics.Split(); + // Filter out methods that are invalid for generation (diagnostics are reported by the analyzer) + var methodsToGenerate = attributedMethods + .Where(static data => IsValidMethodForGeneration(data.Syntax, data.Symbol)); // Compute generator options IncrementalValueProvider stubOptions = context.AnalyzerConfigOptionsProvider @@ -68,8 +60,6 @@ public void Initialize(IncrementalGeneratorInitializationContext context) IncrementalValueProvider stubEnvironment = context.CreateStubEnvironmentProvider(); - // Note: RequiresAllowUnsafeBlocks diagnostic is now reported by the analyzer - IncrementalValuesProvider<(MemberDeclarationSyntax, ImmutableArray)> generateSingleStub = methodsToGenerate .Combine(stubEnvironment) .Combine(stubOptions) @@ -91,8 +81,6 @@ public void Initialize(IncrementalGeneratorInitializationContext context) .WithComparer(Comparers.GeneratedSyntax) .WithTrackingName(StepNames.GenerateSingleStub); - // Note: Diagnostics are now reported by the analyzer, so we don't register them here - context.RegisterConcatenatedSyntaxOutputs(generateSingleStub.Select((data, ct) => data.Item1), "LibraryImports.g.cs"); } @@ -537,6 +525,33 @@ static ExpressionSyntax CreateEnumExpressionSyntax(T value) where T : Enum } } + private static bool IsValidMethodForGeneration(MethodDeclarationSyntax methodSyntax, IMethodSymbol method) + { + // Verify the method has no generic types or defined implementation + // and is marked static and partial. + if (methodSyntax.TypeParameterList is not null + || methodSyntax.Body is not null + || !methodSyntax.Modifiers.Any(SyntaxKind.StaticKeyword) + || !methodSyntax.Modifiers.Any(SyntaxKind.PartialKeyword)) + { + return false; + } + + // Verify that the types the method is declared in are marked partial. + if (methodSyntax.Parent is TypeDeclarationSyntax typeDecl && !typeDecl.IsInPartialContext(out _)) + { + return false; + } + + // Verify the method does not have a ref return + if (method.ReturnsByRef || method.ReturnsByRefReadonly) + { + return false; + } + + return true; + } + private static DiagnosticInfo? GetDiagnosticIfInvalidMethodForGeneration(MethodDeclarationSyntax methodSyntax, IMethodSymbol method) { // Verify the method has no generic types or defined implementation diff --git a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpAnalyzerVerifier.cs b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpAnalyzerVerifier.cs index 79c12e1d0de624..cf3f06ff098823 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpAnalyzerVerifier.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpAnalyzerVerifier.cs @@ -40,6 +40,13 @@ public static async Task VerifyAnalyzerAsync(string source, params DiagnosticRes // Code fix tests support both analyzer and code fix testing. This test class is derived from the code fix test // to avoid the need to maintain duplicate copies of the customization work. internal class Test : CSharpCodeFixVerifier.Test - { } + { + public Test() + { + // Ignore compiler diagnostics since we're only testing the analyzer + // Without the generator, partial methods won't have implementations which causes CS8795 + CompilerDiagnostics = CompilerDiagnostics.None; + } + } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs index 67a4ea9a74fbd0..a440f935b7ffca 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs @@ -14,7 +14,7 @@ using Xunit; using StringMarshalling = Microsoft.Interop.StringMarshalling; -using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpSourceGeneratorVerifier; +using VerifyCS = Microsoft.Interop.UnitTests.Verifiers.CSharpAnalyzerVerifier; namespace LibraryImportGenerator.UnitTests { @@ -41,7 +41,7 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.ParameterTypeNotSupported) .WithLocation(0) .WithArguments("NS.MyClass", "c"), @@ -71,7 +71,7 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.ReturnTypeNotSupported) .WithLocation(0) .WithArguments("NS.MyClass", "Method1"), @@ -93,7 +93,7 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.ParameterTypeNotSupportedWithDetails) .WithLocation(0) .WithArguments("Runtime marshalling must be disabled in this project by applying the 'System.Runtime.CompilerServices.DisableRuntimeMarshallingAttribute' to the assembly to enable marshalling this type.", "c"), @@ -118,7 +118,7 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.ReturnTypeNotSupportedWithDetails) .WithLocation(0) .WithArguments("Runtime marshalling must be disabled in this project by applying the 'System.Runtime.CompilerServices.DisableRuntimeMarshallingAttribute' to the assembly to enable marshalling this type.", "Method1"), @@ -143,7 +143,7 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.MarshalAsParameterConfigurationNotSupported) .WithLocation(0) .WithArguments(nameof(MarshalAsAttribute), "i1"), @@ -170,7 +170,7 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.MarshalAsReturnConfigurationNotSupported) .WithLocation(0) .WithArguments(nameof(MarshalAsAttribute), "Method1"), @@ -196,7 +196,7 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.ConfigurationValueNotSupported) .WithLocation(0) .WithArguments(1, nameof(UnmanagedType)), @@ -228,7 +228,7 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.ConfigurationNotSupported) .WithLocation(0) .WithArguments($"{nameof(MarshalAsAttribute)}{Type.Delimiter}{nameof(MarshalAsAttribute.SafeArraySubType)}"), @@ -302,7 +302,7 @@ public Native(string s) { } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.InvalidStringMarshallingConfiguration) .WithLocation(0) .WithArguments("Method1", "'StringMarshallingCustomType' must be specified when 'StringMarshalling' is set to 'StringMarshalling.Custom'."), @@ -327,7 +327,7 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.InvalidAttributedMethodSignature) .WithLocation(0) .WithArguments("Method"), @@ -349,13 +349,10 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.InvalidAttributedMethodSignature) .WithLocation(0) - .WithArguments("Method"), - // Generator ignores the method - DiagnosticResult.CompilerError("CS8795") - .WithLocation(0)); + .WithArguments("Method")); } [Fact] @@ -374,18 +371,13 @@ partial class Test } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.InvalidAttributedMethodSignature) .WithLocation(0) .WithArguments("Method1"), VerifyCS.Diagnostic(GeneratorDiagnostics.InvalidAttributedMethodSignature) .WithLocation(1) - .WithArguments("Method2"), - // Generator ignores the method - DiagnosticResult.CompilerError("CS8795") - .WithLocation(0), - DiagnosticResult.CompilerError("CS8795") - .WithLocation(1)); + .WithArguments("Method2")); } [Theory] @@ -404,16 +396,10 @@ public async Task NonPartialParentType_Diagnostic(string typeKind) } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.InvalidAttributedMethodContainingTypeMissingModifiers) .WithLocation(0) - .WithArguments("Method", "Test"), - // Generator ignores the method - DiagnosticResult.CompilerError("CS8795") - .WithLocation(0), - // Also expect CS0751: A partial method must be declared within a partial type - DiagnosticResult.CompilerError("CS0751") - .WithLocation(0)); + .WithArguments("Method", "Test")); } [Theory] @@ -435,7 +421,7 @@ partial class TestInner } """; - await VerifyCS.VerifySourceGeneratorAsync(source, + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(GeneratorDiagnostics.InvalidAttributedMethodContainingTypeMissingModifiers) .WithLocation(0) .WithArguments("Method", "Test")); From 831d4313bf038e76da17b06885acca9cf375967b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 20:13:02 +0000 Subject: [PATCH 9/9] Address review feedback: share GetDiagnosticIfInvalidMethodForGeneration, simplify verifier, move options Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com> --- .../LibraryImportDiagnosticsAnalyzer.cs | 21 +++--- .../LibraryImportGenerator.cs | 66 ++----------------- .../Verifiers/CSharpAnalyzerVerifier.cs | 6 +- .../CSharpSourceGeneratorVerifier.cs | 8 +-- 4 files changed, 22 insertions(+), 79 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs index 20fbeaf7daa0ff..0800b375fc4f20 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/LibraryImportDiagnosticsAnalyzer.cs @@ -60,13 +60,16 @@ public override void Initialize(AnalysisContext context) context.Compilation, context.Compilation.GetEnvironmentFlags()); + // Get generator options once per compilation + LibraryImportGeneratorOptions options = new(context.Options.AnalyzerConfigOptionsProvider.GlobalOptions); + // Track if we found any LibraryImport methods to report RequiresAllowUnsafeBlocks once bool foundLibraryImportMethod = false; bool unsafeEnabled = context.Compilation.Options is CSharpCompilationOptions { AllowUnsafe: true }; context.RegisterSymbolAction(symbolContext => { - if (AnalyzeMethod(symbolContext, env, libraryImportAttrType)) + if (AnalyzeMethod(symbolContext, env, libraryImportAttrType, options)) { foundLibraryImportMethod = true; } @@ -87,7 +90,7 @@ public override void Initialize(AnalysisContext context) /// Analyzes a method for LibraryImport diagnostics. /// /// True if the method has LibraryImportAttribute, false otherwise. - private static bool AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment env, INamedTypeSymbol libraryImportAttrType) + private static bool AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment env, INamedTypeSymbol libraryImportAttrType, LibraryImportGeneratorOptions options) { IMethodSymbol method = (IMethodSymbol)context.Symbol; @@ -110,7 +113,7 @@ private static bool AnalyzeMethod(SymbolAnalysisContext context, StubEnvironment { if (syntaxRef.GetSyntax(context.CancellationToken) is MethodDeclarationSyntax methodSyntax) { - AnalyzeMethodSyntax(context, methodSyntax, method, libraryImportAttr, env); + AnalyzeMethodSyntax(context, methodSyntax, method, libraryImportAttr, env, options); break; } } @@ -123,7 +126,8 @@ private static void AnalyzeMethodSyntax( MethodDeclarationSyntax methodSyntax, IMethodSymbol method, AttributeData libraryImportAttr, - StubEnvironment env) + StubEnvironment env, + LibraryImportGeneratorOptions options) { // Check for invalid method signature DiagnosticInfo? invalidMethodDiagnostic = GetDiagnosticIfInvalidMethodForGeneration(methodSyntax, method); @@ -135,9 +139,6 @@ private static void AnalyzeMethodSyntax( // Note: RequiresAllowUnsafeBlocks is reported once per compilation in Initialize method - // Get generator options - LibraryImportGeneratorOptions options = new(context.Options.AnalyzerConfigOptionsProvider.GlobalOptions); - // Calculate stub information and collect diagnostics var diagnostics = CalculateDiagnostics(methodSyntax, method, libraryImportAttr, env, options, context.CancellationToken); @@ -265,7 +266,11 @@ private static ImmutableArray CalculateDiagnostics( }.WithValuesFromNamedArguments(namedArguments); } - private static DiagnosticInfo? GetDiagnosticIfInvalidMethodForGeneration(MethodDeclarationSyntax methodSyntax, IMethodSymbol method) + /// + /// Checks if a method is invalid for generation and returns a diagnostic if so. + /// + /// A diagnostic if the method is invalid, null otherwise. + internal static DiagnosticInfo? GetDiagnosticIfInvalidMethodForGeneration(MethodDeclarationSyntax methodSyntax, IMethodSymbol method) { // Verify the method has no generic types or defined implementation // and is marked static and partial. diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs index 30c9aa027199b5..130686c2db58dc 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs @@ -39,8 +39,9 @@ public static class StepNames public void Initialize(IncrementalGeneratorInitializationContext context) { - // Collect all methods adorned with LibraryImportAttribute - var attributedMethods = context.SyntaxProvider + // Collect all methods adorned with LibraryImportAttribute and filter out invalid ones + // (diagnostics for invalid methods are reported by the analyzer) + var methodsToGenerate = context.SyntaxProvider .ForAttributeWithMetadataName( TypeNames.LibraryImportAttribute, static (node, ct) => node is MethodDeclarationSyntax, @@ -48,11 +49,8 @@ public void Initialize(IncrementalGeneratorInitializationContext context) ? new { Syntax = (MethodDeclarationSyntax)context.TargetNode, Symbol = methodSymbol } : null) .Where( - static modelData => modelData is not null); - - // Filter out methods that are invalid for generation (diagnostics are reported by the analyzer) - var methodsToGenerate = attributedMethods - .Where(static data => IsValidMethodForGeneration(data.Syntax, data.Symbol)); + static modelData => modelData is not null + && Analyzers.LibraryImportDiagnosticsAnalyzer.GetDiagnosticIfInvalidMethodForGeneration(modelData.Syntax, modelData.Symbol) is null); // Compute generator options IncrementalValueProvider stubOptions = context.AnalyzerConfigOptionsProvider @@ -524,59 +522,5 @@ static ExpressionSyntax CreateEnumExpressionSyntax(T value) where T : Enum IdentifierName(value.ToString())); } } - - private static bool IsValidMethodForGeneration(MethodDeclarationSyntax methodSyntax, IMethodSymbol method) - { - // Verify the method has no generic types or defined implementation - // and is marked static and partial. - if (methodSyntax.TypeParameterList is not null - || methodSyntax.Body is not null - || !methodSyntax.Modifiers.Any(SyntaxKind.StaticKeyword) - || !methodSyntax.Modifiers.Any(SyntaxKind.PartialKeyword)) - { - return false; - } - - // Verify that the types the method is declared in are marked partial. - if (methodSyntax.Parent is TypeDeclarationSyntax typeDecl && !typeDecl.IsInPartialContext(out _)) - { - return false; - } - - // Verify the method does not have a ref return - if (method.ReturnsByRef || method.ReturnsByRefReadonly) - { - return false; - } - - return true; - } - - private static DiagnosticInfo? GetDiagnosticIfInvalidMethodForGeneration(MethodDeclarationSyntax methodSyntax, IMethodSymbol method) - { - // Verify the method has no generic types or defined implementation - // and is marked static and partial. - if (methodSyntax.TypeParameterList is not null - || methodSyntax.Body is not null - || !methodSyntax.Modifiers.Any(SyntaxKind.StaticKeyword) - || !methodSyntax.Modifiers.Any(SyntaxKind.PartialKeyword)) - { - return DiagnosticInfo.Create(GeneratorDiagnostics.InvalidAttributedMethodSignature, methodSyntax.Identifier.GetLocation(), method.Name); - } - - // Verify that the types the method is declared in are marked partial. - if (methodSyntax.Parent is TypeDeclarationSyntax typeDecl && !typeDecl.IsInPartialContext(out var nonPartialIdentifier)) - { - return DiagnosticInfo.Create(GeneratorDiagnostics.InvalidAttributedMethodContainingTypeMissingModifiers, methodSyntax.Identifier.GetLocation(), method.Name, nonPartialIdentifier); - } - - // Verify the method does not have a ref return - if (method.ReturnsByRef || method.ReturnsByRefReadonly) - { - return DiagnosticInfo.Create(GeneratorDiagnostics.ReturnConfigurationNotSupported, methodSyntax.Identifier.GetLocation(), "ref return", method.ToDisplayString()); - } - - return null; - } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpAnalyzerVerifier.cs b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpAnalyzerVerifier.cs index cf3f06ff098823..51b84dba0bc396 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpAnalyzerVerifier.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpAnalyzerVerifier.cs @@ -41,10 +41,10 @@ public static async Task VerifyAnalyzerAsync(string source, params DiagnosticRes // to avoid the need to maintain duplicate copies of the customization work. internal class Test : CSharpCodeFixVerifier.Test { - public Test() + public Test() : base() { - // Ignore compiler diagnostics since we're only testing the analyzer - // Without the generator, partial methods won't have implementations which causes CS8795 + // Ignore compiler diagnostics since we're only testing the analyzer. + // Without the generator, partial methods won't have implementations which causes CS8795. CompilerDiagnostics = CompilerDiagnostics.None; } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs index f98b9a0abd344c..25dda49d840902 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/Common/Verifiers/CSharpSourceGeneratorVerifier.cs @@ -110,13 +110,7 @@ public Test(bool referenceAncillaryInterop) } protected override IEnumerable GetDiagnosticAnalyzers() - { - foreach (var analyzer in base.GetDiagnosticAnalyzers()) - { - yield return analyzer; - } - yield return new TAnalyzer(); - } + => [..base.GetDiagnosticAnalyzers(), new TAnalyzer()]; protected override CompilationWithAnalyzers CreateCompilationWithAnalyzers(Compilation compilation, ImmutableArray analyzers, AnalyzerOptions options, CancellationToken cancellationToken) {