Fix S1144 FP: Nested types decorated with StructLayout attribute (#9441)

SonarSource · Jun 21, 2024 · f0a203a · f0a203a
1 parent cc797dd
commit f0a203a
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 14 deletions.
diff --git a/analyzers/its/expected/akka.net/S1144-Akka-netstandard2.0.json b/analyzers/its/expected/akka.net/S1144-Akka-netstandard2.0.json
@@ -54,18 +54,6 @@
       "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/akka.net/src/core/Akka/Event/DeadLetterListener.cs#L255",
       "Location": "Line 255 Position 29-37"
     },
-    {
-      "Id": "S1144",
-      "Message": "Remove the unused private field \u0027m_padding1\u0027.",
-      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/akka.net/src/core/Akka/Helios.Concurrency.DedicatedThreadPool.cs#L567",
-      "Location": "Line 567 Position 13-58"
-    },
-    {
-      "Id": "S1144",
-      "Message": "Remove the unused private field \u0027m_padding2\u0027.",
-      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/akka.net/src/core/Akka/Helios.Concurrency.DedicatedThreadPool.cs#L569",
-      "Location": "Line 569 Position 13-58"
-    },
     {
       "Id": "S1144",
       "Message": "Remove the unused private setter \u0027set_ExceptionHandler\u0027.",

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs
@@ -115,8 +115,7 @@ private static bool GatherSymbols(INamedTypeSymbol namedType,
         foreach (var declaration in PrivateNestedMembersFromNonGeneratedCode(namedType, context))
         {
             if (compilation.GetSemanticModel(declaration.SyntaxTree) is { } semanticModel
-                && semanticModel.GetDeclaredSymbol(declaration) is { } declarationSymbol
-                && !declarationSymbol.HasAttribute(KnownType.System_Runtime_InteropServices_StructLayoutAttribute))
+                && semanticModel.GetDeclaredSymbol(declaration) is { } declarationSymbol)
             {
                 var symbolsCollector = RetrieveRemovableSymbols(declarationSymbol, compilation, context);
                 CopyRetrievedSymbols(symbolsCollector, privateSymbols, internalSymbols, fieldLikeSymbols);
@@ -534,6 +533,7 @@ private static bool IsRemovable(ISymbol symbol) =>
             && !HasAttributes(symbol)
             && !symbol.IsSerializableMember()
             && !symbol.ContainingType.IsInterface()
+            && !(symbol.Kind is SymbolKind.Field && symbol.ContainingType.HasAttribute(KnownType.System_Runtime_InteropServices_StructLayoutAttribute))
             && symbol.GetInterfaceMember() is null
             && symbol.GetOverriddenMember() is null;
 

diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.Batch.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.Batch.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Runtime.InteropServices;
 using System.Runtime.Serialization;
 using System.Threading.Tasks;
 
@@ -404,3 +405,25 @@ public bool AttributeOnPropertyNoncompliant
         get;
     }
 }
+
+// https://github.com/SonarSource/sonar-dotnet/issues/9432
+namespace Repro_9432
+{
+    partial class OuterClass
+    {
+        public void MethodUsesNestedStruct()
+        {
+            NestedClass.NestedStruct x;
+        }
+
+        private static class NestedClass
+        {
+            // https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.structlayoutattribute
+            [StructLayout(LayoutKind.Sequential)]
+            internal struct NestedStruct
+            {
+                public int Field;                       // Compliant: the unused field can be used to control the physical layout of struct in the memory
+            }
+        }
+    }
+}
diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Runtime.InteropServices;
 using System.Runtime.Serialization;
 using System.Threading.Tasks;
 
@@ -388,3 +389,25 @@ public bool AttributeOnPropertyNoncompliant
         get;
     }
 }
+
+// https://github.com/SonarSource/sonar-dotnet/issues/9432
+namespace Repro_9432
+{
+    partial class OuterClass
+    {
+        public void MethodUsesNestedStruct()
+        {
+            NestedClass.NestedStruct x;
+        }
+
+        private static class NestedClass
+        {
+            // https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.structlayoutattribute
+            [StructLayout(LayoutKind.Sequential)]
+            internal struct NestedStruct
+            {
+                public int Field;                       // Compliant: the unused field can be used to control the physical layout of struct in the memory
+            }
+        }
+    }
+}
diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Runtime.InteropServices;
 using System.Runtime.Serialization;
 using System.Threading.Tasks;
 
@@ -495,3 +496,29 @@ public bool AttributeOnPropertyNoncompliant
         private set; // Noncompliant
     }
 }
+
+// https://github.com/SonarSource/sonar-dotnet/issues/9432
+namespace Repro_9432
+{
+    partial class OuterClass
+    {
+        public void MethodUsesNestedStruct()
+        {
+            NestedClass.NestedStruct x;
+        }
+
+        private static class NestedClass
+        {
+            // https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.structlayoutattribute
+            [StructLayout(LayoutKind.Sequential)]
+            internal struct NestedStruct
+            {
+                public int Field;                       // Compliant: the unused field can be used to control the physical layout of struct in the memory
+                public int Property { get; set; }       // Noncompliant: https://stackoverflow.com/questions/28488057/what-is-the-structlayoutattribute-effect-on-properties-in-c
+                public int CalculatedProperty => 42;    // Noncompliant
+                public void Method() { }                // Noncompliant
+                public struct UnusedNestedType { }      // Noncompliant
+            }
+        }
+    }
+}