Report diagnostics for bad opt-in marshallers at MarshalUsing use-sit…

…e if the marshaler is in a different compilation than the current compilation. (dotnet/runtimelab#1300) Commit migrated from dotnet/runtimelab@d4b5fd0
jkoritzinsky · Jul 7, 2021 · adf085f · adf085f
1 parent d7b3dc2
commit adf085f
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 9 deletions.
diff --git a/...Runtime.InteropServices/gen/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs b/...Runtime.InteropServices/gen/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs
@@ -334,7 +334,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
                 if (!nativeType.IsValueType)
                 {
                     context.ReportDiagnostic(
-                        nativeType.CreateDiagnostic(
+                        GetDiagnosticLocations(context, nativeType, nativeMarshalerAttributeData).CreateDiagnostic(
                             NativeTypeMustHaveRequiredShapeRule,
                             nativeType.ToDisplayString(),
                             type.ToDisplayString()));
@@ -379,7 +379,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
                         if (stackAllocSizeField is null or { DeclaredAccessibility: not Accessibility.Public } or { IsConst: false } or { Type: not { SpecialType: SpecialType.System_Int32 } })
                         {
                             context.ReportDiagnostic(
-                                ctor.CreateDiagnostic(
+                                GetDiagnosticLocations(context, ctor, nativeMarshalerAttributeData).CreateDiagnostic(
                                     StackallocConstructorMustHaveStackBufferSizeConstantRule,
                                     nativeType.ToDisplayString()));
                         }
@@ -392,7 +392,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
                 if (!hasConstructor && !hasStackallocConstructor && !hasToManaged)
                 {
                     context.ReportDiagnostic(
-                        marshalerType.CreateDiagnostic(
+                        GetDiagnosticLocations(context, marshalerType, nativeMarshalerAttributeData).CreateDiagnostic(
                             NativeTypeMustHaveRequiredShapeRule,
                             marshalerType.ToDisplayString(),
                             type.ToDisplayString()));
@@ -402,7 +402,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
                 if (validateAllScenarioSupport && hasStackallocConstructor && !hasConstructor)
                 {
                     context.ReportDiagnostic(
-                        marshalerType.CreateDiagnostic(
+                        GetDiagnosticLocations(context, marshalerType, nativeMarshalerAttributeData).CreateDiagnostic(
                             StackallocMarshallingShouldSupportAllocatingMarshallingFallbackRule,
                             marshalerType.ToDisplayString()));
                 }
@@ -415,7 +415,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
                     if (valuePropertyIsRefReturn)
                     {
                         context.ReportDiagnostic(
-                            valueProperty.CreateDiagnostic(
+                            GetDiagnosticLocations(context, valueProperty, nativeMarshalerAttributeData).CreateDiagnostic(
                                 RefValuePropertyUnsupportedRule,
                                 marshalerType.ToDisplayString()));
                     }
@@ -430,14 +430,14 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
                     if ((hasConstructor || hasStackallocConstructor) && valueProperty.GetMethod is null)
                     {
                         context.ReportDiagnostic(
-                            valueProperty.CreateDiagnostic(
+                            GetDiagnosticLocations(context, valueProperty, nativeMarshalerAttributeData).CreateDiagnostic(
                                 ValuePropertyMustHaveGetterRule,
                                 marshalerType.ToDisplayString()));
                     }
                     if (hasToManaged && valueProperty.SetMethod is null)
                     {
                         context.ReportDiagnostic(
-                            valueProperty.CreateDiagnostic(
+                            GetDiagnosticLocations(context, valueProperty, nativeMarshalerAttributeData).CreateDiagnostic(
                                 ValuePropertyMustHaveSetterRule,
                                 marshalerType.ToDisplayString()));
                     }
@@ -446,7 +446,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
                 if (!nativeType.IsConsideredBlittable())
                 {
                     context.ReportDiagnostic(
-                        nativeTypeDiagnosticsTargetSymbol.CreateDiagnostic(
+                        GetDiagnosticLocations(context, nativeTypeDiagnosticsTargetSymbol, nativeMarshalerAttributeData).CreateDiagnostic(
                             NativeTypeMustBeBlittableRule,
                             nativeType.ToDisplayString(),
                             type.ToDisplayString()));
@@ -486,12 +486,24 @@ IPointerTypeSymbol _ or
                     IMethodSymbol getPinnableReferenceMethodToMention = getPinnableReferenceMethods.Managed ?? getPinnableReferenceMethods.Marshaler!;
 
                     context.ReportDiagnostic(
-                        nativeTypeDiagnosticsTargetSymbol.CreateDiagnostic(
+                        GetDiagnosticLocations(context, nativeTypeDiagnosticsTargetSymbol, nativeMarshalerAttributeData).CreateDiagnostic(
                             NativeTypeMustBePointerSizedRule,
                             nativeType.ToDisplayString(),
                             getPinnableReferenceMethodToMention.ContainingType.ToDisplayString()));
                 }
             }
+
+            private ImmutableArray<Location> GetDiagnosticLocations(SymbolAnalysisContext context, ISymbol targetSymbol, AttributeData marshallingAttribute)
+            {
+                // If we're using a compilation that references another compilation, the symbol locations can be in source in the wrong compilation,
+                // which can cause exceptions when reporting diagnostics. Make sure the symbol is defined in the current Compilation's source module before using its locations.
+                // If the symbol is not defined in the current Compilation's source module, report the diagnostic at the marshalling attribute's location.
+                if (SymbolEqualityComparer.Default.Equals(context.Compilation.SourceModule, targetSymbol.ContainingModule))
+                {
+                    return targetSymbol.Locations;
+                }
+                return ImmutableArray.Create(marshallingAttribute.ApplicationSyntaxReference?.GetSyntax().GetLocation() ?? Location.None);
+            }
         }
     }
 }
diff --git a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/GeneratorDiagnostics.cs b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/GeneratorDiagnostics.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
 
@@ -25,6 +26,22 @@ public static Diagnostic CreateDiagnostic(
                 messageArgs: args);
         }
 
+        public static Diagnostic CreateDiagnostic(
+            this ImmutableArray<Location> locations,
+            DiagnosticDescriptor descriptor,
+            params object[] args)
+        {
+            IEnumerable<Location> locationsInSource = locations.Where(l => l.IsInSource);
+            if (!locationsInSource.Any())
+                return Diagnostic.Create(descriptor, Location.None, args);
+
+            return Diagnostic.Create(
+                descriptor,
+                location: locationsInSource.First(),
+                additionalLocations: locationsInSource.Skip(1),
+                messageArgs: args);
+        }
+
         public static Diagnostic CreateDiagnostic(
             this AttributeData attributeData,
             DiagnosticDescriptor descriptor,

diff --git a/....InteropServices/tests/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs b/....InteropServices/tests/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs
@@ -1,3 +1,5 @@
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Testing;
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Xunit;
@@ -827,6 +829,57 @@ await VerifyCS.VerifyAnalyzerAsync(source,
                 VerifyCS.Diagnostic(NativeTypeMustBeBlittableRule).WithLocation(0).WithArguments("Native", "S"));
         }
 
+        [Fact]
+        public async Task NonBlittableNativeTypeOnMarshalUsingParameter_MultipleCompilations_ReportsDiagnostic_WithLocation()
+        {
+            string source1 = @"
+using System;
+using System.Runtime.InteropServices;
+
+public struct S
+{
+    public string s;
+}
+
+public struct Native
+{
+    private string value;
+
+    public Native(S s) : this()
+    {
+    }
+
+    public S ToManaged() => new S();
+}
+";
+            Compilation compilation1 = await TestUtils.CreateCompilation(source1);
+
+            string source2 = @"
+using System;
+using System.Runtime.InteropServices;
+
+static class Test
+{
+    static void Foo([{|#0:MarshalUsing(typeof(Native))|}] S s)
+    {}
+}
+";
+            var test = new Verifiers.CSharpCodeFixVerifier<Microsoft.Interop.Analyzers.ManualTypeMarshallingAnalyzer, EmptyCodeFixProvider>.Test
+            {
+                ExpectedDiagnostics =
+                {
+                    VerifyCS.Diagnostic(NativeTypeMustBeBlittableRule).WithLocation(0).WithArguments("Native", "S")
+                },
+                SolutionTransforms =
+                {
+                    (solution, projectId) => solution.AddMetadataReference(projectId, compilation1.ToMetadataReference())
+                },
+                TestCode = source2
+            };
+
+            await test.RunAsync();
+        }
+
         [Fact]
         public async Task NonBlittableNativeTypeOnMarshalUsingReturn_ReportsDiagnostic()
         {