From b1f0348dabdae6d997be831fa4c28d83fae879e3 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 1 Aug 2022 13:17:18 -0700 Subject: [PATCH 1/6] Implement the code-fix for the stateful shape missing member diagnostics. --- .../CustomMarshallerAttributeFixer.cs | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs index 74413228d4dab9..9e815eeb09b7ac 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs @@ -200,6 +200,10 @@ private static void AddMissingMembers( { AddMissingMembersToStatelessMarshaller(editor, declaringSyntax, marshallerType, managedType, missingMemberNames, isLinearCollectionMarshaller); } + if (marshallerType.IsValueType) + { + AddMissingMembersToStatefulMarshaller(editor, declaringSyntax, marshallerType, managedType, missingMemberNames, isLinearCollectionMarshaller); + } } private static void AddMissingMembersToStatelessMarshaller(DocumentEditor editor, SyntaxNode declaringSyntax, INamedTypeSymbol marshallerType, ITypeSymbol managedType, HashSet missingMemberNames, bool isLinearCollectionMarshaller) @@ -398,6 +402,174 @@ ITypeSymbol CreateManagedElementTypeSymbol() } } + private static void AddMissingMembersToStatefulMarshaller(DocumentEditor editor, SyntaxNode declaringSyntax, INamedTypeSymbol marshallerType, ITypeSymbol managedType, HashSet missingMemberNames, bool isLinearCollectionMarshaller) + { + SyntaxGenerator gen = editor.Generator; + // Get the methods of the shape so we can use them to determine what types to use in signatures that are not obvious. + var (_, methods) = StatefulMarshallerShapeHelper.GetShapeForType(marshallerType, managedType, isLinearCollectionMarshaller, editor.SemanticModel.Compilation); + INamedTypeSymbol spanOfT = editor.SemanticModel.Compilation.GetBestTypeByMetadataName(TypeNames.System_Span_Metadata)!; + INamedTypeSymbol readOnlySpanOfT = editor.SemanticModel.Compilation.GetBestTypeByMetadataName(TypeNames.System_ReadOnlySpan_Metadata)!; + var (typeParameters, _) = marshallerType.GetAllTypeArgumentsIncludingInContainingTypes(); + + // Use a lazy factory for the type syntaxes to avoid re-checking the various methods and reconstructing the syntax. + Lazy unmanagedTypeSyntax = new(CreateUnmanagedTypeSyntax, isThreadSafe: false); + Lazy managedElementTypeSymbol = new(CreateManagedElementTypeSymbol, isThreadSafe: false); + + List newMembers = new(); + + if (missingMemberNames.Contains(ShapeMemberNames.Value.Stateful.FromManaged)) + { + newMembers.Add( + gen.MethodDeclaration( + ShapeMemberNames.Value.Stateful.FromManaged, + parameters: new[] { gen.ParameterDeclaration("managed", gen.TypeExpression(managedType)) }, + accessibility: Accessibility.Public, + statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); + } + + if (missingMemberNames.Contains(ShapeMemberNames.Value.Stateful.ToUnmanaged)) + { + newMembers.Add( + gen.MethodDeclaration( + ShapeMemberNames.Value.Stateful.ToUnmanaged, + returnType: unmanagedTypeSyntax.Value, + accessibility: Accessibility.Public, + statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); + } + + if (missingMemberNames.Contains(ShapeMemberNames.Value.Stateful.FromUnmanaged)) + { + newMembers.Add( + gen.MethodDeclaration( + ShapeMemberNames.Value.Stateful.FromUnmanaged, + parameters: new[] { gen.ParameterDeclaration("unmanaged", unmanagedTypeSyntax.Value) }, + accessibility: Accessibility.Public, + statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); + } + + if (missingMemberNames.Contains(ShapeMemberNames.Value.Stateful.ToManaged)) + { + newMembers.Add( + gen.MethodDeclaration( + ShapeMemberNames.Value.Stateful.ToManaged, + returnType: gen.TypeExpression(managedType), + accessibility: Accessibility.Public, + statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); + } + + if (missingMemberNames.Contains(ShapeMemberNames.BufferSize)) + { + newMembers.Add( + gen.WithAccessorDeclarations( + gen.PropertyDeclaration(ShapeMemberNames.BufferSize, + gen.TypeExpression(editor.SemanticModel.Compilation.GetSpecialType(SpecialType.System_Int32)), + Accessibility.Public, + DeclarationModifiers.Static), + gen.GetAccessorDeclaration(statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) }))); + } + + if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateless.GetManagedValuesSource)) + { + newMembers.Add( + gen.MethodDeclaration( + ShapeMemberNames.LinearCollection.Stateless.GetManagedValuesSource, + returnType: gen.TypeExpression(readOnlySpanOfT.Construct(managedElementTypeSymbol.Value)), + accessibility: Accessibility.Public, + statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); + } + + if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateless.GetUnmanagedValuesDestination)) + { + newMembers.Add( + gen.MethodDeclaration( + ShapeMemberNames.LinearCollection.Stateless.GetUnmanagedValuesDestination, + returnType: gen.TypeExpression(spanOfT.Construct(typeParameters[typeParameters.Length - 1])), + accessibility: Accessibility.Public, + statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); + } + + if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateless.GetUnmanagedValuesSource)) + { + newMembers.Add( + gen.MethodDeclaration( + ShapeMemberNames.LinearCollection.Stateless.GetUnmanagedValuesSource, + parameters: new[] + { + gen.ParameterDeclaration("numElements", gen.TypeExpression(SpecialType.System_Int32)) + }, + returnType: gen.TypeExpression(readOnlySpanOfT.Construct(typeParameters[typeParameters.Length - 1])), + accessibility: Accessibility.Public, + statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); + } + + if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateless.GetManagedValuesDestination)) + { + newMembers.Add( + gen.MethodDeclaration( + ShapeMemberNames.LinearCollection.Stateless.GetManagedValuesDestination, + parameters: new[] + { + gen.ParameterDeclaration("numElements", gen.TypeExpression(SpecialType.System_Int32)) + }, + returnType: gen.TypeExpression(spanOfT.Construct(managedElementTypeSymbol.Value)), + accessibility: Accessibility.Public, + modifiers: DeclarationModifiers.Static, + statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); + } + + if (missingMemberNames.Contains(ShapeMemberNames.Free)) + { + newMembers.Add( + gen.MethodDeclaration( + ShapeMemberNames.Value.Stateful.Free, + accessibility: Accessibility.Public, + statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); + } + + editor.ReplaceNode(declaringSyntax, (declaringSyntax, gen) => gen.AddMembers(declaringSyntax, newMembers)); + + SyntaxNode CreateUnmanagedTypeSyntax() + { + ITypeSymbol? unmanagedType = null; + if (methods.ToUnmanaged is not null) + { + unmanagedType = methods.ToUnmanaged.ReturnType; + } + else if (methods.FromUnmanaged is not null) + { + unmanagedType = methods.FromUnmanaged.Parameters[0].Type; + } + else if (methods.UnmanagedValuesSource is not null) + { + unmanagedType = methods.UnmanagedValuesSource.Parameters[0].Type; + } + else if (methods.UnmanagedValuesDestination is not null) + { + unmanagedType = methods.UnmanagedValuesDestination.Parameters[0].Type; + } + + if (unmanagedType is not null) + { + return gen.TypeExpression(unmanagedType); + } + return gen.TypeExpression(editor.SemanticModel.Compilation.GetSpecialType(SpecialType.System_IntPtr)); + } + + ITypeSymbol CreateManagedElementTypeSymbol() + { + if (methods.ManagedValuesSource is not null) + { + return ((INamedTypeSymbol)methods.ManagedValuesSource.ReturnType).TypeArguments[0]; + } + if (methods.ManagedValuesDestination is not null) + { + return ((INamedTypeSymbol)methods.ManagedValuesDestination.ReturnType).TypeArguments[0]; + } + + return editor.SemanticModel.Compilation.GetSpecialType(SpecialType.System_IntPtr); + } + } + private static SyntaxNode DefaultMethodStatement(SyntaxGenerator generator, Compilation compilation) { return generator.ThrowStatement(generator.ObjectCreationExpression( From 75da51074438cffc29c1b2f1179543c8ce9cc311 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 2 Aug 2022 17:37:37 -0700 Subject: [PATCH 2/6] Update value tests to test codefix (pending roslyn-sdk fix to enable deterministic ordering) --- ...FixerTests_StatefulValueShapeValidation.cs | 161 +++++++++++++++++- 1 file changed, 156 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs index f3e6710d8bf8c9..934ee91207fb77 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs @@ -33,8 +33,35 @@ struct MarshallerType } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedIn, typeof(MarshallerType))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedOut, typeof(MarshallerType))] + struct MarshallerType + { + public void FromManaged(ManagedType managed) + { + throw new System.NotImplementedException(); + } + + public nint ToUnmanaged() + { + throw new System.NotImplementedException(); + } + + public void Free() + { + throw new System.NotImplementedException(); + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(StatefulMarshallerRequiresFromManagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType"), VerifyCS.Diagnostic(StatefulMarshallerRequiresFromManagedRule).WithLocation(1).WithArguments("MarshallerType", MarshalMode.UnmanagedToManagedOut, "ManagedType"), VerifyCS.Diagnostic(StatefulMarshallerRequiresToUnmanagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType"), @@ -58,8 +85,35 @@ struct MarshallerType } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedOut, typeof(MarshallerType))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedIn, typeof(MarshallerType))] + struct MarshallerType + { + public void FromUnmanaged(nint unmanaged) + { + throw new System.NotImplementedException(); + } + + public ManagedType ToManaged() + { + throw new System.NotImplementedException(); + } + + public void Free() + { + throw new System.NotImplementedException(); + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(StatefulMarshallerRequiresFromUnmanagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType"), VerifyCS.Diagnostic(StatefulMarshallerRequiresFromUnmanagedRule).WithLocation(1).WithArguments("MarshallerType", MarshalMode.UnmanagedToManagedIn, "ManagedType"), VerifyCS.Diagnostic(StatefulMarshallerRequiresToManagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType"), @@ -106,8 +160,44 @@ struct MarshallerType } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedRef, typeof(MarshallerType))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedRef, typeof(MarshallerType))] + struct MarshallerType + { + public void FromManaged(ManagedType managed) + { + throw new System.NotImplementedException(); + } + + public nint ToUnmanaged() + { + throw new System.NotImplementedException(); + } + public void FromUnmanaged(nint unmanaged) + { + throw new System.NotImplementedException(); + } + + public ManagedType ToManaged() + { + throw new System.NotImplementedException(); + } + + public void Free() + { + throw new System.NotImplementedException(); + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(StatefulMarshallerRequiresFreeRule).WithLocation(0).WithArguments("MarshallerType"), VerifyCS.Diagnostic(StatefulMarshallerRequiresFromManagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedRef, "ManagedType"), VerifyCS.Diagnostic(StatefulMarshallerRequiresFromManagedRule).WithLocation(1).WithArguments("MarshallerType", MarshalMode.UnmanagedToManagedRef, "ManagedType"), @@ -212,8 +302,43 @@ struct MarshallerType } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.Default, typeof(MarshallerType))] + struct MarshallerType + { + public void FromManaged(ManagedType managed) + { + throw new System.NotImplementedException(); + } + + public nint ToUnmanaged() + { + throw new System.NotImplementedException(); + } + public void FromUnmanaged(nint unmanaged) + { + throw new System.NotImplementedException(); + } + + public ManagedType ToManaged() + { + throw new System.NotImplementedException(); + } + + public void Free() + { + throw new System.NotImplementedException(); + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(StatefulMarshallerRequiresFreeRule).WithLocation(0).WithArguments("MarshallerType"), VerifyCS.Diagnostic(StatefulMarshallerRequiresFromManagedRule).WithSeverity(DiagnosticSeverity.Info).WithLocation(0).WithArguments("MarshallerType", MarshalMode.Default, "ManagedType"), VerifyCS.Diagnostic(StatefulMarshallerRequiresToUnmanagedRule).WithSeverity(DiagnosticSeverity.Info).WithLocation(0).WithArguments("MarshallerType", MarshalMode.Default, "ManagedType"), @@ -241,8 +366,34 @@ public void Free() {} } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System; + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedIn, typeof({|#0:MarshallerType|}))] + struct MarshallerType + { + public static int BufferSize + { + get + { + throw new System.NotImplementedException(); + } + } + + public void FromManaged(ManagedType m, Span b) {} + + public int ToUnmanaged() => default; + + public void Free() {} + } + """; + + await VerifyCS.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(CallerAllocFromManagedMustHaveBufferSizeRule).WithLocation(0).WithArguments("MarshallerType", "byte")); } } From 950875deff4b5b551105af8de7d462b29f940f12 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 3 Aug 2022 13:40:53 -0700 Subject: [PATCH 3/6] Upgrade testing platform and re-enable disabled test. --- eng/Versions.props | 2 +- ...rAttributeFixerTests_StatelessValueShapeValidation.cs | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/eng/Versions.props b/eng/Versions.props index cec00875b841a5..76feae1dbfb9ed 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -166,7 +166,7 @@ 2.14.3 - 1.1.2-beta1.22205.2 + 1.1.2-beta1.22403.2 7.0.0-preview-20220721.1 diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessValueShapeValidation.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessValueShapeValidation.cs index dbfe4d8d2e9e90..5f35c7f628f6ae 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessValueShapeValidation.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessValueShapeValidation.cs @@ -422,14 +422,11 @@ public static nint ConvertToUnmanaged(ManagedType managed) } [Fact] - [ActiveIssue("https://github.com/dotnet/roslyn-sdk/issues/1000")] public async Task ModeThatUsesManagedToUnmanagedShape_Missing_ConvertToUnmanagedMethod_Marshaller_DifferentProject_ReportsDiagnostic() { string entryPointTypeSource = """ using System.Runtime.InteropServices.Marshalling; - class ManagedType {} - [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedIn, typeof({|SYSLIB1057:OtherMarshallerType|}))] [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedOut, typeof({|SYSLIB1057:OtherMarshallerType|}))] [CustomMarshaller(typeof(ManagedType), MarshalMode.ElementIn, typeof({|SYSLIB1057:OtherMarshallerType|}))] @@ -439,12 +436,14 @@ static class MarshallerType """; string otherMarshallerTypeOriginalSource = """ + public class ManagedType {} public static class OtherMarshallerType { } """; string otherMarshallerTypeFixedSource = """ + public class ManagedType {} public static class OtherMarshallerType { public static nint ConvertToUnmanaged(ManagedType managed) @@ -472,8 +471,10 @@ public static nint ConvertToUnmanaged(ManagedType managed) test.FixedState.Sources.Add(entryPointTypeSource); test.FixedState.AdditionalProjects.Add(otherProjectName, otherProjectFixedState); test.FixedState.AdditionalProjectReferences.Add(otherProjectName); - test.MarkupOptions = MarkupOptions.UseFirstDescriptor; test.FixedState.MarkupHandling = MarkupMode.IgnoreFixable; + + test.NumberOfFixAllIterations = 1; + test.MarkupOptions = MarkupOptions.UseFirstDescriptor; await test.RunAsync(); } From 1d50cc7ddf91a0918335ce0cf35499f06ff9250e Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 3 Aug 2022 15:21:17 -0700 Subject: [PATCH 4/6] Add custom ordering test harness to ensure our tests are deterministic for the code-fix testing --- .../CustomMarshallerAttributeAnalyzer.cs | 10 +- .../CustomMarshallerAttributeFixerTest.cs | 116 ++++++++++++++++++ ...FixerTests_StatefulValueShapeValidation.cs | 38 ++++-- 3 files changed, 147 insertions(+), 17 deletions(-) create mode 100644 src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTest.cs diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeAnalyzer.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeAnalyzer.cs index 9603ec32cbc2cd..d9b513cc1769a1 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeAnalyzer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeAnalyzer.cs @@ -468,7 +468,7 @@ public static class DefaultMarshalModeDiagnostics description: GetResourceString(nameof(SR.StatelessLinearCollectionRequiresTwoParameterAllocateContainerForManagedElementsDescription))); /// - public static readonly DiagnosticDescriptor StatefulMarshallerRequiresFromManagedRule = + private static readonly DiagnosticDescriptor StatefulMarshallerRequiresFromManagedRule = new DiagnosticDescriptor( Ids.CustomMarshallerTypeMustHaveRequiredShape, GetResourceString(nameof(SR.CustomMarshallerTypeMustHaveRequiredShapeTitle)), @@ -479,7 +479,7 @@ public static class DefaultMarshalModeDiagnostics description: GetResourceString(nameof(SR.StatefulMarshallerRequiresFromManagedDescription))); /// - public static readonly DiagnosticDescriptor StatefulMarshallerRequiresToUnmanagedRule = + private static readonly DiagnosticDescriptor StatefulMarshallerRequiresToUnmanagedRule = new DiagnosticDescriptor( Ids.CustomMarshallerTypeMustHaveRequiredShape, GetResourceString(nameof(SR.CustomMarshallerTypeMustHaveRequiredShapeTitle)), @@ -490,7 +490,7 @@ public static class DefaultMarshalModeDiagnostics description: GetResourceString(nameof(SR.StatefulMarshallerRequiresToUnmanagedDescription))); /// - public static readonly DiagnosticDescriptor StatefulMarshallerRequiresToManagedRule = + private static readonly DiagnosticDescriptor StatefulMarshallerRequiresToManagedRule = new DiagnosticDescriptor( Ids.CustomMarshallerTypeMustHaveRequiredShape, GetResourceString(nameof(SR.CustomMarshallerTypeMustHaveRequiredShapeTitle)), @@ -501,7 +501,7 @@ public static class DefaultMarshalModeDiagnostics description: GetResourceString(nameof(SR.StatefulMarshallerRequiresToManagedDescription))); /// - public static readonly DiagnosticDescriptor StatefulMarshallerRequiresFromUnmanagedRule = + private static readonly DiagnosticDescriptor StatefulMarshallerRequiresFromUnmanagedRule = new DiagnosticDescriptor( Ids.CustomMarshallerTypeMustHaveRequiredShape, GetResourceString(nameof(SR.CustomMarshallerTypeMustHaveRequiredShapeTitle)), @@ -511,7 +511,7 @@ public static class DefaultMarshalModeDiagnostics isEnabledByDefault: true, description: GetResourceString(nameof(SR.StatefulMarshallerRequiresFromUnmanagedDescription))); - internal static DiagnosticDescriptor GetDefaultMarshalModeDiagnostic(DiagnosticDescriptor errorDescriptor) + public static DiagnosticDescriptor GetDefaultMarshalModeDiagnostic(DiagnosticDescriptor errorDescriptor) { if (ReferenceEquals(errorDescriptor, CustomMarshallerAttributeAnalyzer.StatelessValueInRequiresConvertToUnmanagedRule)) { diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTest.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTest.cs new file mode 100644 index 00000000000000..95afe794310c7d --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTest.cs @@ -0,0 +1,116 @@ +// 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.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading.Tasks; +using LibraryImportGenerator.UnitTests.Verifiers; +using Microsoft.CodeAnalysis; +using Microsoft.Interop.Analyzers; +using static Microsoft.Interop.Analyzers.CustomMarshallerAttributeAnalyzer; + +namespace LibraryImportGenerator.UnitTests +{ + internal class CustomMarshallerAttributeFixerTest : CSharpCodeFixVerifier.Test + { + protected override ImmutableArray<(Project project, Diagnostic diagnostic)> SortDistinctDiagnostics(IEnumerable<(Project project, Diagnostic diagnostic)> diagnostics) + => diagnostics.OrderBy(d => d.diagnostic.Location.GetLineSpan().Path, StringComparer.Ordinal) + .ThenBy(d => d.diagnostic.Location.SourceSpan.Start) + .ThenBy(d => d.diagnostic.Location.SourceSpan.End) + .ThenBy(d => d.diagnostic.Id) + .ThenBy(d => d.diagnostic.Descriptor, Comparer.Instance) + .ToImmutableArray(); + + private class Comparer : IComparer + { + public static readonly Comparer Instance = new(); + + /// + /// Checks if the provided descriptor matches the expected descriptor or the default marshal mode equivalent of the expected descriptor. + /// + /// + /// + /// + private static bool IsEquivalentDescriptor(DiagnosticDescriptor descriptor, DiagnosticDescriptor expected) + { + return descriptor.Equals(expected) || descriptor.Equals(DefaultMarshalModeDiagnostics.GetDefaultMarshalModeDiagnostic(expected)); + } + + private static int GetOrderIndexFromDescriptor(DiagnosticDescriptor descriptor) + { + // We'll order the descriptors in the following order for testing: + // - BufferSize + // - FromManaged/ConvertToUnmanaged/AllocateContainerForUnmanagedElements + // - GetManagedValuesSource/GetUnmanagedValuesDestination + // - ToUnmanaged + // - FromUnmanaged/ConvertToManaged/AllocateContainerForManagedElements + // - GetUnmanagedValuesSource/GetManagedValuesDestination + // - ToManaged + // - Free + // This order corresponds to the order that the fix-all provider will add the methods. + + if (IsEquivalentDescriptor(descriptor, CallerAllocFromManagedMustHaveBufferSizeRule) + || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionCallerAllocFromManagedMustHaveBufferSizeRule)) + { + return 0; + } + + if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresFromManagedRule) + || IsEquivalentDescriptor(descriptor, StatelessValueInRequiresConvertToUnmanagedRule) + || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionRequiresTwoParameterAllocateContainerForUnmanagedElementsRule)) + { + return 1; + } + if (IsEquivalentDescriptor(descriptor, LinearCollectionInRequiresCollectionMethodsRule) + || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionInRequiresCollectionMethodsRule)) + { + return 2; + } + if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresToUnmanagedRule)) + { + return 3; + } + if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresFromUnmanagedRule) + || IsEquivalentDescriptor(descriptor, StatelessRequiresConvertToManagedRule) + || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionRequiresTwoParameterAllocateContainerForManagedElementsRule)) + { + return 4; + } + if (IsEquivalentDescriptor(descriptor, LinearCollectionOutRequiresCollectionMethodsRule) + || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionOutRequiresCollectionMethodsRule)) + { + return 5; + } + if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresToManagedRule)) + { + return 6; + } + if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresFreeRule)) + { + return 7; + } + // Sort all unknown diagnostic descriptors later. + return 8; + } + + public int Compare(DiagnosticDescriptor? x, DiagnosticDescriptor? y) + { + // Sort null as less than non-null. + if (x is null) + { + return y is null ? 0 : -1; + } + if (y is null) + { + return 1; + } + + return GetOrderIndexFromDescriptor(x) - GetOrderIndexFromDescriptor(y); + } + } + } +} diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs index 934ee91207fb77..45b07be8173d57 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs @@ -5,6 +5,7 @@ using Microsoft.CodeAnalysis.Testing; using Microsoft.Interop; using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; using Xunit; using static Microsoft.Interop.Analyzers.CustomMarshallerAttributeAnalyzer; @@ -59,7 +60,7 @@ public void Free() } """; - await VerifyCS.VerifyCodeFixAsync( + await VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatefulMarshallerRequiresFromManagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType"), @@ -111,7 +112,7 @@ public void Free() } """; - await VerifyCS.VerifyCodeFixAsync( + await VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatefulMarshallerRequiresFromUnmanagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType"), @@ -178,6 +179,7 @@ public nint ToUnmanaged() { throw new System.NotImplementedException(); } + public void FromUnmanaged(nint unmanaged) { throw new System.NotImplementedException(); @@ -195,7 +197,7 @@ public void Free() } """; - await VerifyCS.VerifyCodeFixAsync( + await VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatefulMarshallerRequiresFreeRule).WithLocation(0).WithArguments("MarshallerType"), @@ -319,6 +321,7 @@ public nint ToUnmanaged() { throw new System.NotImplementedException(); } + public void FromUnmanaged(nint unmanaged) { throw new System.NotImplementedException(); @@ -336,7 +339,7 @@ public void Free() } """; - await VerifyCS.VerifyCodeFixAsync( + await VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatefulMarshallerRequiresFreeRule).WithLocation(0).WithArguments("MarshallerType"), @@ -375,26 +378,37 @@ class ManagedType {} [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedIn, typeof({|#0:MarshallerType|}))] struct MarshallerType { + public void FromManaged(ManagedType m, Span b) {} + + public int ToUnmanaged() => default; + + public void Free() {} + public static int BufferSize { get { - throw new System.NotImplementedException(); + throw new NotImplementedException(); } } - - public void FromManaged(ManagedType m, Span b) {} - - public int ToUnmanaged() => default; - - public void Free() {} } """; - await VerifyCS.VerifyCodeFixAsync( + await VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(CallerAllocFromManagedMustHaveBufferSizeRule).WithLocation(0).WithArguments("MarshallerType", "byte")); } + private static async Task VerifyCodeFixAsync(string source, string fixedSource, params DiagnosticResult[] expected) + { + var test = new CustomMarshallerAttributeFixerTest + { + TestCode = source, + FixedCode = fixedSource, + }; + + test.ExpectedDiagnostics.AddRange(expected); + await test.RunAsync(CancellationToken.None); + } } } From 5134711c5d124662898f9bd05438fb85f4676d13 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 3 Aug 2022 15:56:07 -0700 Subject: [PATCH 5/6] Update tests to use the new harness type. Add tests for the code fix for the stateful shapes. --- .../CustomMarshallerAttributeFixer.cs | 1 - .../CustomMarshallerAttributeFixerTest.cs | 69 +++-- ...StatefulLinearCollectionShapeValidation.cs | 271 +++++++++++++++++- ...FixerTests_StatefulValueShapeValidation.cs | 21 +- ...tatelessLinearCollectionShapeValidation.cs | 18 +- ...ixerTests_StatelessValueShapeValidation.cs | 16 +- 6 files changed, 332 insertions(+), 64 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs index 9e815eeb09b7ac..14a9bdc123b505 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs @@ -513,7 +513,6 @@ private static void AddMissingMembersToStatefulMarshaller(DocumentEditor editor, }, returnType: gen.TypeExpression(spanOfT.Construct(managedElementTypeSymbol.Value)), accessibility: Accessibility.Public, - modifiers: DeclarationModifiers.Static, statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); } diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTest.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTest.cs index 95afe794310c7d..c56b8aa792f212 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTest.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTest.cs @@ -10,6 +10,7 @@ using System.Threading.Tasks; using LibraryImportGenerator.UnitTests.Verifiers; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; using Microsoft.Interop.Analyzers; using static Microsoft.Interop.Analyzers.CustomMarshallerAttributeAnalyzer; @@ -17,6 +18,10 @@ namespace LibraryImportGenerator.UnitTests { internal class CustomMarshallerAttributeFixerTest : CSharpCodeFixVerifier.Test { + // Sort the diagnostics in a deterministic order even when they only differ by diagnostic message. + // In particular, sort the equivalent subgroups by their diagnostic descriptor in the order that the fixer's fix-all provider + // will add the methods. + // This ensures that the iterative code-fix test will produce the same (deterministic) output as the fix-all tests. protected override ImmutableArray<(Project project, Diagnostic diagnostic)> SortDistinctDiagnostics(IEnumerable<(Project project, Diagnostic diagnostic)> diagnostics) => diagnostics.OrderBy(d => d.diagnostic.Location.GetLineSpan().Path, StringComparer.Ordinal) .ThenBy(d => d.diagnostic.Location.SourceSpan.Start) @@ -43,58 +48,67 @@ private static bool IsEquivalentDescriptor(DiagnosticDescriptor descriptor, Diag private static int GetOrderIndexFromDescriptor(DiagnosticDescriptor descriptor) { // We'll order the descriptors in the following order for testing: + // - FromManaged/ConvertToUnmanaged + // - ToUnmanaged + // - FromUnmanaged/ConvertToManaged + // - ToManaged // - BufferSize - // - FromManaged/ConvertToUnmanaged/AllocateContainerForUnmanagedElements + // - AllocateContainerForUnmanagedElements + // - AllocateContainerForManagedElements // - GetManagedValuesSource/GetUnmanagedValuesDestination - // - ToUnmanaged - // - FromUnmanaged/ConvertToManaged/AllocateContainerForManagedElements // - GetUnmanagedValuesSource/GetManagedValuesDestination - // - ToManaged // - Free // This order corresponds to the order that the fix-all provider will add the methods. - if (IsEquivalentDescriptor(descriptor, CallerAllocFromManagedMustHaveBufferSizeRule) - || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionCallerAllocFromManagedMustHaveBufferSizeRule)) - { - return 0; - } - if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresFromManagedRule) || IsEquivalentDescriptor(descriptor, StatelessValueInRequiresConvertToUnmanagedRule) || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionRequiresTwoParameterAllocateContainerForUnmanagedElementsRule)) + { + return 0; + } + if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresToUnmanagedRule)) { return 1; } - if (IsEquivalentDescriptor(descriptor, LinearCollectionInRequiresCollectionMethodsRule) - || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionInRequiresCollectionMethodsRule)) + if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresFromUnmanagedRule) + || IsEquivalentDescriptor(descriptor, StatelessRequiresConvertToManagedRule) + || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionRequiresTwoParameterAllocateContainerForManagedElementsRule)) { return 2; } - if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresToUnmanagedRule)) + if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresToManagedRule)) { return 3; } - if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresFromUnmanagedRule) - || IsEquivalentDescriptor(descriptor, StatelessRequiresConvertToManagedRule) - || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionRequiresTwoParameterAllocateContainerForManagedElementsRule)) + if (IsEquivalentDescriptor(descriptor, CallerAllocFromManagedMustHaveBufferSizeRule) + || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionCallerAllocFromManagedMustHaveBufferSizeRule)) { return 4; } - if (IsEquivalentDescriptor(descriptor, LinearCollectionOutRequiresCollectionMethodsRule) - || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionOutRequiresCollectionMethodsRule)) + if (IsEquivalentDescriptor(descriptor, StatelessLinearCollectionRequiresTwoParameterAllocateContainerForUnmanagedElementsRule)) { return 5; } - if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresToManagedRule)) + if (IsEquivalentDescriptor(descriptor, StatelessLinearCollectionRequiresTwoParameterAllocateContainerForManagedElementsRule)) { return 6; } - if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresFreeRule)) + if (IsEquivalentDescriptor(descriptor, LinearCollectionInRequiresCollectionMethodsRule) + || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionInRequiresCollectionMethodsRule)) { return 7; } + if (IsEquivalentDescriptor(descriptor, LinearCollectionOutRequiresCollectionMethodsRule) + || IsEquivalentDescriptor(descriptor, StatelessLinearCollectionOutRequiresCollectionMethodsRule)) + { + return 8; + } + if (IsEquivalentDescriptor(descriptor, StatefulMarshallerRequiresFreeRule)) + { + return 9; + } // Sort all unknown diagnostic descriptors later. - return 8; + return 10; } public int Compare(DiagnosticDescriptor? x, DiagnosticDescriptor? y) @@ -112,5 +126,18 @@ public int Compare(DiagnosticDescriptor? x, DiagnosticDescriptor? y) return GetOrderIndexFromDescriptor(x) - GetOrderIndexFromDescriptor(y); } } + + public static async Task VerifyCodeFixAsync(string source, string fixedSource, params DiagnosticResult[] diagnostics) + { + CustomMarshallerAttributeFixerTest test = new() + { + TestCode = source, + FixedCode = fixedSource, + }; + + test.ExpectedDiagnostics.AddRange(diagnostics); + + await test.RunAsync(); + } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulLinearCollectionShapeValidation.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulLinearCollectionShapeValidation.cs index 13f79c7e2f2937..62d599fea5494d 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulLinearCollectionShapeValidation.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulLinearCollectionShapeValidation.cs @@ -34,8 +34,46 @@ struct MarshallerType } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedIn, typeof(MarshallerType<>))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedOut, typeof(MarshallerType<>))] + [ContiguousCollectionMarshaller] + struct MarshallerType + { + public void FromManaged(ManagedType managed) + { + throw new System.NotImplementedException(); + } + + public nint ToUnmanaged() + { + throw new System.NotImplementedException(); + } + + public System.ReadOnlySpan GetManagedValuesSource() + { + throw new System.NotImplementedException(); + } + + public System.Span GetUnmanagedValuesDestination() + { + throw new System.NotImplementedException(); + } + + public void Free() + { + throw new System.NotImplementedException(); + } + } + """; + + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.DiagnosticWithArguments(StatefulMarshallerRequiresFreeRule, "MarshallerType").WithLocation(0), VerifyCS.DiagnosticWithArguments(StatefulMarshallerRequiresFromManagedRule, "MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType").WithLocation(0), VerifyCS.DiagnosticWithArguments(StatefulMarshallerRequiresFromManagedRule, "MarshallerType", MarshalMode.UnmanagedToManagedOut, "ManagedType").WithLocation(1), @@ -65,8 +103,35 @@ public void Free() {} } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedIn, typeof(MarshallerType<>))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedOut, typeof(MarshallerType<>))] + [ContiguousCollectionMarshaller] + struct MarshallerType + { + public void FromManaged(ManagedType m) {} + public nint ToUnmanaged() => default; + public void Free() {} + + public System.ReadOnlySpan GetManagedValuesSource() + { + throw new System.NotImplementedException(); + } + + public System.Span GetUnmanagedValuesDestination() + { + throw new System.NotImplementedException(); + } + } + """; + + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(LinearCollectionInRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType"), VerifyCS.Diagnostic(LinearCollectionInRequiresCollectionMethodsRule).WithLocation(1).WithArguments("MarshallerType", MarshalMode.UnmanagedToManagedOut, "ManagedType")); } @@ -92,8 +157,32 @@ public void Free() {} } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System; + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedIn, typeof(MarshallerType<>))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedOut, typeof(MarshallerType<>))] + [ContiguousCollectionMarshaller] + struct MarshallerType + { + public void FromManaged(ManagedType m) {} + public nint ToUnmanaged() => default; + public void Free() {} + public Span GetUnmanagedValuesDestination() => default; + + public ReadOnlySpan GetManagedValuesSource() + { + throw new NotImplementedException(); + } + } + """; + + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(LinearCollectionInRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType"), VerifyCS.Diagnostic(LinearCollectionInRequiresCollectionMethodsRule).WithLocation(1).WithArguments("MarshallerType", MarshalMode.UnmanagedToManagedOut, "ManagedType")); } @@ -119,8 +208,32 @@ public void Free() {} } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System; + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedIn, typeof(MarshallerType<>))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedOut, typeof(MarshallerType<>))] + [ContiguousCollectionMarshaller] + struct MarshallerType + { + public void FromManaged(ManagedType m) {} + public nint ToUnmanaged() => default; + public void Free() {} + public ReadOnlySpan GetManagedValuesSource() => default; + + public Span GetUnmanagedValuesDestination() + { + throw new NotImplementedException(); + } + } + """; + + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(LinearCollectionInRequiresCollectionMethodsRule).WithLocation(1).WithArguments("MarshallerType", MarshalMode.UnmanagedToManagedOut, "ManagedType"), VerifyCS.Diagnostic(LinearCollectionInRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType")); } @@ -196,8 +309,46 @@ struct MarshallerType } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedOut, typeof(MarshallerType<>))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedIn, typeof(MarshallerType<>))] + [ContiguousCollectionMarshaller] + struct MarshallerType + { + public void FromUnmanaged(nint unmanaged) + { + throw new System.NotImplementedException(); + } + + public ManagedType ToManaged() + { + throw new System.NotImplementedException(); + } + + public System.ReadOnlySpan GetUnmanagedValuesSource(int numElements) + { + throw new System.NotImplementedException(); + } + + public System.Span GetManagedValuesDestination(int numElements) + { + throw new System.NotImplementedException(); + } + + public void Free() + { + throw new System.NotImplementedException(); + } + } + """; + + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.DiagnosticWithArguments(StatefulMarshallerRequiresFreeRule, "MarshallerType").WithLocation(0), VerifyCS.DiagnosticWithArguments(StatefulMarshallerRequiresToManagedRule, "MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType").WithLocation(0), VerifyCS.DiagnosticWithArguments(StatefulMarshallerRequiresToManagedRule, "MarshallerType", MarshalMode.UnmanagedToManagedIn, "ManagedType").WithLocation(1), @@ -227,8 +378,35 @@ public void Free() {} } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedOut, typeof(MarshallerType<>))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedIn, typeof(MarshallerType<>))] + [ContiguousCollectionMarshaller] + struct MarshallerType + { + public void FromUnmanaged(int f) {} + public ManagedType ToManaged() => default; + public void Free() {} + + public System.ReadOnlySpan GetUnmanagedValuesSource(int numElements) + { + throw new System.NotImplementedException(); + } + + public System.Span GetManagedValuesDestination(int numElements) + { + throw new System.NotImplementedException(); + } + } + """; + + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(LinearCollectionOutRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType"), VerifyCS.Diagnostic(LinearCollectionOutRequiresCollectionMethodsRule).WithLocation(1).WithArguments("MarshallerType", MarshalMode.UnmanagedToManagedIn, "ManagedType")); } @@ -254,8 +432,32 @@ public void Free() {} } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System; + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedOut, typeof(MarshallerType<>))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedIn, typeof(MarshallerType<>))] + [ContiguousCollectionMarshaller] + struct MarshallerType + { + public void FromUnmanaged(int f) {} + public ManagedType ToManaged() => default; + public void Free() {} + public Span GetManagedValuesDestination(int numElements) => default; + + public ReadOnlySpan GetUnmanagedValuesSource(int numElements) + { + throw new NotImplementedException(); + } + } + """; + + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(LinearCollectionOutRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType"), VerifyCS.Diagnostic(LinearCollectionOutRequiresCollectionMethodsRule).WithLocation(1).WithArguments("MarshallerType", MarshalMode.UnmanagedToManagedIn, "ManagedType")); } @@ -281,8 +483,32 @@ public void Free() {} } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System; + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedOut, typeof(MarshallerType<>))] + [CustomMarshaller(typeof(ManagedType), MarshalMode.UnmanagedToManagedIn, typeof(MarshallerType<>))] + [ContiguousCollectionMarshaller] + struct MarshallerType + { + public void FromUnmanaged(int f) {} + public ManagedType ToManaged() => default; + public void Free() {} + public ReadOnlySpan GetUnmanagedValuesSource(int numElements) => default; + + public Span GetManagedValuesDestination(int numElements) + { + throw new NotImplementedException(); + } + } + """; + + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(LinearCollectionOutRequiresCollectionMethodsRule).WithLocation(1).WithArguments("MarshallerType", MarshalMode.UnmanagedToManagedIn, "ManagedType"), VerifyCS.Diagnostic(LinearCollectionOutRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType")); } @@ -362,8 +588,35 @@ public void Free() {} } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedSource = """ + using System; + using System.Runtime.InteropServices.Marshalling; + + class ManagedType {} + + [CustomMarshaller(typeof(ManagedType), MarshalMode.ManagedToUnmanagedIn, typeof(MarshallerType<>))] + [ContiguousCollectionMarshaller] + struct MarshallerType + { + public void FromManaged(ManagedType m, Span buffer) {} + public nint ToUnmanaged() => default; + public void Free() {} + public ReadOnlySpan GetManagedValuesSource() => default; + public Span GetUnmanagedValuesDestination() => default; + + public static int BufferSize + { + get + { + throw new NotImplementedException(); + } + } + } + """; + + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, + fixedSource, VerifyCS.Diagnostic(CallerAllocFromManagedMustHaveBufferSizeRule).WithLocation(0).WithArguments("MarshallerType", "byte")); } diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs index 23d11ec14f08af..6cf2b96e619125 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatefulValueShapeValidation.cs @@ -60,7 +60,7 @@ public void Free() } """; - await VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.DiagnosticWithArguments(StatefulMarshallerRequiresFromManagedRule, "MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType").WithLocation(0), @@ -112,7 +112,7 @@ public void Free() } """; - await VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.DiagnosticWithArguments(StatefulMarshallerRequiresFromUnmanagedRule, "MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType").WithLocation(0), @@ -197,7 +197,7 @@ public void Free() } """; - await VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.DiagnosticWithArguments(StatefulMarshallerRequiresFreeRule, "MarshallerType").WithLocation(0), @@ -339,7 +339,7 @@ public void Free() } """; - await VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatefulMarshallerRequiresFreeRule).WithLocation(0).WithArguments("MarshallerType"), @@ -394,21 +394,10 @@ public static int BufferSize } """; - await VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(CallerAllocFromManagedMustHaveBufferSizeRule).WithLocation(0).WithArguments("MarshallerType", "byte")); } - private static async Task VerifyCodeFixAsync(string source, string fixedSource, params DiagnosticResult[] expected) - { - var test = new CustomMarshallerAttributeFixerTest - { - TestCode = source, - FixedCode = fixedSource, - }; - - test.ExpectedDiagnostics.AddRange(expected); - await test.RunAsync(CancellationToken.None); - } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessLinearCollectionShapeValidation.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessLinearCollectionShapeValidation.cs index 65ecd60049f429..28149a0f7864e4 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessLinearCollectionShapeValidation.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessLinearCollectionShapeValidation.cs @@ -63,7 +63,7 @@ public static System.Span GetUnmanagedValuesDestination(nint unmanaged, int n } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.DiagnosticWithArguments(StatelessLinearCollectionRequiresTwoParameterAllocateContainerForUnmanagedElementsRule, "MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType").WithLocation(0), @@ -117,7 +117,7 @@ public static System.Span GetUnmanagedValuesDestination(nint unmanaged, int n } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessLinearCollectionInRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType"), @@ -169,7 +169,7 @@ public static ReadOnlySpan GetManagedValuesSource(ManagedType managed) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessLinearCollectionInRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType"), @@ -221,7 +221,7 @@ public static Span GetUnmanagedValuesDestination(nint unmanaged, int numEleme } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessLinearCollectionInRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType"), @@ -362,7 +362,7 @@ public static System.Span GetManagedValuesDestination(ManagedType managed) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.DiagnosticWithArguments(StatelessLinearCollectionRequiresTwoParameterAllocateContainerForManagedElementsRule, "MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType").WithLocation(0), @@ -416,7 +416,7 @@ public static System.Span GetManagedValuesDestination(ManagedType managed) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessLinearCollectionOutRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType"), @@ -468,7 +468,7 @@ public static ReadOnlySpan GetUnmanagedValuesSource(nint unmanaged, int numEl } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessLinearCollectionOutRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType"), @@ -520,7 +520,7 @@ public static Span GetManagedValuesDestination(ManagedType managed) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessLinearCollectionOutRequiresCollectionMethodsRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType"), @@ -662,7 +662,7 @@ public static int BufferSize } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessLinearCollectionCallerAllocFromManagedMustHaveBufferSizeRule).WithLocation(0).WithArguments("MarshallerType", "byte")); diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessValueShapeValidation.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessValueShapeValidation.cs index 4376de8617f7e4..b855690efc2b3a 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessValueShapeValidation.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CustomMarshallerAttributeFixerTests_StatelessValueShapeValidation.cs @@ -51,7 +51,7 @@ public static nint ConvertToUnmanaged(ManagedType managed) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessValueInRequiresConvertToUnmanagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType"), @@ -113,7 +113,7 @@ public static ManagedType ConvertToManaged(nint unmanaged) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessRequiresConvertToManagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedOut, "ManagedType"), @@ -159,7 +159,7 @@ public static ManagedType ConvertToManaged(nint unmanaged) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.DiagnosticWithArguments(StatelessValueInRequiresConvertToUnmanagedRule, "MarshallerType", MarshalMode.ManagedToUnmanagedRef, "ManagedType").WithLocation(0), @@ -212,7 +212,7 @@ public static float ConvertToUnmanaged(ManagedType managed) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessValueInRequiresConvertToUnmanagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedRef, "ManagedType"), @@ -262,7 +262,7 @@ public static ManagedType ConvertToManaged(float unmanaged) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessRequiresConvertToManagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedRef, "ManagedType"), @@ -329,7 +329,7 @@ public static ManagedType ConvertToManaged(nint unmanaged) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessValueInRequiresConvertToUnmanagedRule).WithSeverity(DiagnosticSeverity.Info).WithLocation(0).WithArguments("MarshallerType", MarshalMode.Default, "ManagedType"), @@ -373,7 +373,7 @@ public static int BufferSize } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(CallerAllocFromManagedMustHaveBufferSizeRule).WithLocation(0).WithArguments("MarshallerType", "byte")); @@ -516,7 +516,7 @@ public static nint ConvertToUnmanaged(ManagedType2 managed) } """; - await VerifyCS.VerifyCodeFixAsync( + await CustomMarshallerAttributeFixerTest.VerifyCodeFixAsync( source, fixedSource, VerifyCS.Diagnostic(StatelessValueInRequiresConvertToUnmanagedRule).WithLocation(0).WithArguments("MarshallerType", MarshalMode.ManagedToUnmanagedIn, "ManagedType"), From be215c0492e8a3f9976b5138c5068cc7957e9a45 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 4 Aug 2022 17:20:10 -0700 Subject: [PATCH 6/6] Fix Stateless->Stateful --- .../Analyzers/CustomMarshallerAttributeFixer.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs index 14a9bdc123b505..22d34aa852c664 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/CustomMarshallerAttributeFixer.cs @@ -468,31 +468,31 @@ private static void AddMissingMembersToStatefulMarshaller(DocumentEditor editor, gen.GetAccessorDeclaration(statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) }))); } - if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateless.GetManagedValuesSource)) + if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateful.GetManagedValuesSource)) { newMembers.Add( gen.MethodDeclaration( - ShapeMemberNames.LinearCollection.Stateless.GetManagedValuesSource, + ShapeMemberNames.LinearCollection.Stateful.GetManagedValuesSource, returnType: gen.TypeExpression(readOnlySpanOfT.Construct(managedElementTypeSymbol.Value)), accessibility: Accessibility.Public, statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); } - if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateless.GetUnmanagedValuesDestination)) + if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateful.GetUnmanagedValuesDestination)) { newMembers.Add( gen.MethodDeclaration( - ShapeMemberNames.LinearCollection.Stateless.GetUnmanagedValuesDestination, + ShapeMemberNames.LinearCollection.Stateful.GetUnmanagedValuesDestination, returnType: gen.TypeExpression(spanOfT.Construct(typeParameters[typeParameters.Length - 1])), accessibility: Accessibility.Public, statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); } - if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateless.GetUnmanagedValuesSource)) + if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateful.GetUnmanagedValuesSource)) { newMembers.Add( gen.MethodDeclaration( - ShapeMemberNames.LinearCollection.Stateless.GetUnmanagedValuesSource, + ShapeMemberNames.LinearCollection.Stateful.GetUnmanagedValuesSource, parameters: new[] { gen.ParameterDeclaration("numElements", gen.TypeExpression(SpecialType.System_Int32)) @@ -502,11 +502,11 @@ private static void AddMissingMembersToStatefulMarshaller(DocumentEditor editor, statements: new[] { DefaultMethodStatement(gen, editor.SemanticModel.Compilation) })); } - if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateless.GetManagedValuesDestination)) + if (missingMemberNames.Contains(ShapeMemberNames.LinearCollection.Stateful.GetManagedValuesDestination)) { newMembers.Add( gen.MethodDeclaration( - ShapeMemberNames.LinearCollection.Stateless.GetManagedValuesDestination, + ShapeMemberNames.LinearCollection.Stateful.GetManagedValuesDestination, parameters: new[] { gen.ParameterDeclaration("numElements", gen.TypeExpression(SpecialType.System_Int32))