diff --git a/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/Net7/S4200.cs b/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/Net7/S4200.cs new file mode 100644 index 00000000000..6fe7954b4ba --- /dev/null +++ b/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/Net7/S4200.cs @@ -0,0 +1,13 @@ +using System.Runtime.InteropServices; + +namespace Net7; + +public static partial class S4200 +{ + // Use Goto definition in VS on the methods to see the code generated by the Microsoft.Interop.LibraryImportGenerator + [LibraryImport("foo.dll")] + public static partial void DllImportAttributeAppliedToThisFunction(); // Noncompliant (S4200): Make this native method private and provide a wrapper. + + [LibraryImport("foo.dll", StringMarshalling = StringMarshalling.Utf8)] + public static partial void DllImportAttributeAppliedToGeneratedLocalFunction(string p); // Noncompliant +} diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/NativeMethodsShouldBeWrapped.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/NativeMethodsShouldBeWrapped.cs index 489cb169220..90586ff3021 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/NativeMethodsShouldBeWrapped.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/NativeMethodsShouldBeWrapped.cs @@ -34,20 +34,28 @@ public sealed class NativeMethodsShouldBeWrapped : SonarDiagnosticAnalyzer protected override void Initialize(SonarAnalysisContext context) { context.RegisterSymbolAction(ReportPublicExternalMethods, SymbolKind.Method); - context.RegisterSyntaxNodeAction(ReportTrivialWrappers, SyntaxKind.MethodDeclaration); + context.RegisterSyntaxNodeActionInNonGenerated(ReportTrivialWrappers, SyntaxKind.MethodDeclaration); } private static void ReportPublicExternalMethods(SymbolAnalysisContext c) { var methodSymbol = (IMethodSymbol)c.Symbol; - if (methodSymbol.IsExtern - && methodSymbol.IsPubliclyAccessible() - && methodSymbol.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax() is MethodDeclarationSyntax methodDeclaration) + if (IsExternMethod(methodSymbol) + && methodSymbol.IsPubliclyAccessible()) { - c.ReportIssue(Diagnostic.Create(Rule, methodDeclaration.Identifier.GetLocation(), MakeThisMethodPrivateMessage)); + foreach (var methodDeclaration in methodSymbol.DeclaringSyntaxReferences + .Where(x => !x.SyntaxTree.IsGenerated(CSharpGeneratedCodeRecognizer.Instance, c.Compilation)) + .Select(x => x.GetSyntax()) + .OfType()) + { + c.ReportIssue(Diagnostic.Create(Rule, methodDeclaration.Identifier.GetLocation(), MakeThisMethodPrivateMessage)); + } } } + private static bool IsExternMethod(IMethodSymbol methodSymbol) => + methodSymbol.IsExtern || methodSymbol.HasAttribute(KnownType.System_Runtime_InteropServices_LibraryImportAttribute); + private static void ReportTrivialWrappers(SyntaxNodeAnalysisContext c) { var methodDeclaration = (MethodDeclarationSyntax)c.Node; @@ -97,7 +105,7 @@ a.Expression is LiteralExpressionSyntax private static ISet GetExternalMethods(IMethodSymbol methodSymbol) => methodSymbol.ContainingType.GetMembers() .OfType() - .Where(m => m.IsExtern) + .Where(IsExternMethod) .ToHashSet(); private static IEnumerable GetBodyDescendants(MethodDeclarationSyntax methodDeclaration) => diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/NativeMethodsShouldBeWrappedTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/NativeMethodsShouldBeWrappedTest.cs index 29794d3a533..f4791f5b030 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/NativeMethodsShouldBeWrappedTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/NativeMethodsShouldBeWrappedTest.cs @@ -41,9 +41,19 @@ public void NativeMethodsShouldBeWrapped_CSharp9() => public void NativeMethodsShouldBeWrapped_CSharp10() => builder.AddPaths("NativeMethodsShouldBeWrapped.CSharp10.cs").WithOptions(ParseOptionsHelper.FromCSharp10).Verify(); + // NativeMethodsShouldBeWrapped.CSharp11.SourceGenerator.cs contains the code as generated by the SourceGenerator. To regenerate it: + // * Take the code from NativeMethodsShouldBeWrapped.CSharp11.cs + // * Copy it to a new .Net 7 project + // * Press F12 on any of the partial methods + // * Copy the result to NativeMethodsShouldBeWrapped.CSharp11.SourceGenerator.cs [TestMethod] public void NativeMethodsShouldBeWrapped_CSharp11() => - builder.AddPaths("NativeMethodsShouldBeWrapped.CSharp11.cs").WithOptions(ParseOptionsHelper.FromCSharp11).Verify(); + builder + .AddPaths("NativeMethodsShouldBeWrapped.CSharp11.cs") + .AddPaths("NativeMethodsShouldBeWrapped.CSharp11.SourceGenerator.cs") + .WithOptions(ParseOptionsHelper.FromCSharp11) + .WithConcurrentAnalysis(false) + .Verify(); #endif diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/NativeMethodsShouldBeWrapped.CSharp11.SourceGenerator.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/NativeMethodsShouldBeWrapped.CSharp11.SourceGenerator.cs new file mode 100644 index 00000000000..957d70b8152 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/NativeMethodsShouldBeWrapped.CSharp11.SourceGenerator.cs @@ -0,0 +1,86 @@ +// +namespace LibraryImportAttributeTests +{ + public static unsafe partial class ExternMethods + { + [System.Runtime.InteropServices.DllImportAttribute("foo.dll", EntryPoint = "DllImportAttributeAppliedToThisFunction", ExactSpelling = true)] + public static extern partial void DllImportAttributeAppliedToThisFunction(); + } +} +namespace LibraryImportAttributeTests +{ + public static unsafe partial class ExternMethods + { + [System.Runtime.InteropServices.DllImportAttribute("foo.dll", EntryPoint = "CompliantDllImportAttributeAppliedToThisFunction", ExactSpelling = true)] + private static extern partial void CompliantDllImportAttributeAppliedToThisFunction(int i); + } +} +namespace LibraryImportAttributeTests +{ + public static unsafe partial class ExternMethods + { + [System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Interop.LibraryImportGenerator", "7.0.7.6804")] + [System.Runtime.CompilerServices.SkipLocalsInitAttribute] + public static partial void DllImportAttributeAppliedToGeneratedLocalFunction(string p) + { + byte* __p_native = default; + // Setup - Perform required setup. + global::System.Runtime.InteropServices.Marshalling.Utf8StringMarshaller.ManagedToUnmanagedIn __p_native__marshaller = new(); + try + { + // Marshal - Convert managed data to native data. + byte* __p_native__stackptr = stackalloc byte[global::System.Runtime.InteropServices.Marshalling.Utf8StringMarshaller.ManagedToUnmanagedIn.BufferSize]; + __p_native__marshaller.FromManaged(p, new System.Span(__p_native__stackptr, global::System.Runtime.InteropServices.Marshalling.Utf8StringMarshaller.ManagedToUnmanagedIn.BufferSize)); + { + // PinnedMarshal - Convert managed data to native data that requires the managed data to be pinned. + __p_native = __p_native__marshaller.ToUnmanaged(); + __PInvoke(__p_native); + } + } + finally + { + // Cleanup - Perform required cleanup. + __p_native__marshaller.Free(); + } + + // Local P/Invoke + [System.Runtime.InteropServices.DllImportAttribute("foo.dll", EntryPoint = "DllImportAttributeAppliedToGeneratedLocalFunction", ExactSpelling = true)] + static extern unsafe void __PInvoke(byte* p); + } + } +} +namespace LibraryImportAttributeTests +{ + public static unsafe partial class ExternMethods + { + [System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Interop.LibraryImportGenerator", "7.0.7.6804")] + [System.Runtime.CompilerServices.SkipLocalsInitAttribute] + private static partial void CompliantDllImportAttributeAppliedToGeneratedLocalFunction(string p) + { + byte* __p_native = default; + // Setup - Perform required setup. + global::System.Runtime.InteropServices.Marshalling.Utf8StringMarshaller.ManagedToUnmanagedIn __p_native__marshaller = new(); + try + { + // Marshal - Convert managed data to native data. + byte* __p_native__stackptr = stackalloc byte[global::System.Runtime.InteropServices.Marshalling.Utf8StringMarshaller.ManagedToUnmanagedIn.BufferSize]; + __p_native__marshaller.FromManaged(p, new System.Span(__p_native__stackptr, global::System.Runtime.InteropServices.Marshalling.Utf8StringMarshaller.ManagedToUnmanagedIn.BufferSize)); + { + // PinnedMarshal - Convert managed data to native data that requires the managed data to be pinned. + __p_native = __p_native__marshaller.ToUnmanaged(); + __PInvoke(__p_native); + } + } + finally + { + // Cleanup - Perform required cleanup. + __p_native__marshaller.Free(); + } + + // Local P/Invoke + [System.Runtime.InteropServices.DllImportAttribute("foo.dll", EntryPoint = "CompliantDllImportAttributeAppliedToGeneratedLocalFunction", ExactSpelling = true)] + static extern unsafe void __PInvoke(byte* p); + } + } +} + diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/NativeMethodsShouldBeWrapped.CSharp11.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/NativeMethodsShouldBeWrapped.CSharp11.cs index 55fee14dd8a..19776f847d4 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/NativeMethodsShouldBeWrapped.CSharp11.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/NativeMethodsShouldBeWrapped.CSharp11.cs @@ -8,3 +8,40 @@ public interface IInterface [DllImport("kernel32.dll", CharSet = CharSet.Unicode)] public static extern virtual bool RemoveDirectory2(string name); // Noncompliant } + +namespace LibraryImportAttributeTests +{ + // Copy the class to a .Net 7 project in VS and use "goto definition" on the methods to extract the code generated by the Microsoft.Interop.LibraryImportGenerator + public static partial class ExternMethods + { + [LibraryImport("foo.dll")] + public static partial void DllImportAttributeAppliedToThisFunction(); // Noncompliant + + [LibraryImport("foo.dll")] + private static partial void CompliantDllImportAttributeAppliedToThisFunction(int i); // Compliant + + [LibraryImport("foo.dll", StringMarshalling = StringMarshalling.Utf8)] + public static partial void DllImportAttributeAppliedToGeneratedLocalFunction(string p); // Noncompliant + + [LibraryImport("foo.dll", StringMarshalling = StringMarshalling.Utf8)] + private static partial void CompliantDllImportAttributeAppliedToGeneratedLocalFunction(string p); // Compliant + + // Wrapper tests + + public static void CompliantDllImportAttributeAppliedToThisFunctionNoncompliantWrapper(int i) // Noncompliant {{Make this wrapper for native method 'CompliantDllImportAttributeAppliedToThisFunction' less trivial.}} + { + CompliantDllImportAttributeAppliedToThisFunction(i); + } + + public static void CompliantDllImportAttributeAppliedToThisFunctionCompliantWrapper(int i) // Compliant + { + if (i > 0) + { + CompliantDllImportAttributeAppliedToThisFunction(i); + } + } + + public static void CompliantDllImportAttributeAppliedToGeneratedLocalFunctionWrapper(string p) // Noncompliant + => CompliantDllImportAttributeAppliedToGeneratedLocalFunction(p); + } +}