Fix S1104 FN: In types marked as [Serializable], do not ignore fields…

… marked as [NonSerialized] (#8531)
SonarSource · Jan 15, 2024 · 5decea0 · 5decea0
1 parent 20d4726
commit 5decea0
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 25 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/FieldsShouldBeEncapsulatedInProperties.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/FieldsShouldBeEncapsulatedInProperties.cs
@@ -49,21 +49,23 @@ protected override void Initialize(SonarAnalysisContext context) =>
                         return;
                     }
 
+                    var firstVariable = fieldDeclaration.Declaration.Variables[0];
+                    var symbol = c.SemanticModel.GetDeclaredSymbol(firstVariable);
                     var parentSymbol = c.SemanticModel.GetDeclaredSymbol(fieldDeclaration.Parent);
-                    if (parentSymbol.HasAttribute(KnownType.System_Runtime_InteropServices_StructLayoutAttribute)
-                        || parentSymbol.HasAttribute(KnownType.System_SerializableAttribute))
+                    if (parentSymbol.HasAttribute(KnownType.System_Runtime_InteropServices_StructLayoutAttribute) || Serializable(symbol, parentSymbol))
                     {
                         return;
                     }
 
-                    var firstVariable = fieldDeclaration.Declaration.Variables[0];
-                    var symbol = c.SemanticModel.GetDeclaredSymbol(firstVariable);
-
                     if (symbol.GetEffectiveAccessibility() == Accessibility.Public)
                     {
                         c.ReportIssue(Diagnostic.Create(Rule, firstVariable.GetLocation()));
                     }
                 },
                 SyntaxKind.FieldDeclaration);
+
+        private static bool Serializable(ISymbol symbol, ISymbol parentSymbol) =>
+            parentSymbol.HasAttribute(KnownType.System_SerializableAttribute)
+            && !symbol.HasAttribute(KnownType.System_NonSerializedAttribute);
     }
 }
diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs
@@ -356,6 +356,7 @@ public sealed partial class KnownType
         public static readonly KnownType System_Net_Sockets_TcpClient = new("System.Net.Sockets.TcpClient");
         public static readonly KnownType System_Net_Sockets_UdpClient = new("System.Net.Sockets.UdpClient");
         public static readonly KnownType System_Net_WebClient = new("System.Net.WebClient");
+        public static readonly KnownType System_NonSerializedAttribute = new("System.NonSerializedAttribute");
         public static readonly KnownType System_NotImplementedException = new("System.NotImplementedException");
         public static readonly KnownType System_NotSupportedException = new("System.NotSupportedException");
         public static readonly KnownType System_Nullable_T = new("System.Nullable", "T");

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/FieldsShouldBeEncapsulatedInPropertiesTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/FieldsShouldBeEncapsulatedInPropertiesTest.cs
@@ -20,32 +20,31 @@
 
 using SonarAnalyzer.Rules.CSharp;
 
-namespace SonarAnalyzer.UnitTest.Rules
+namespace SonarAnalyzer.UnitTest.Rules;
+
+[TestClass]
+public class FieldsShouldBeEncapsulatedInPropertiesTest
 {
-    [TestClass]
-    public class FieldsShouldBeEncapsulatedInPropertiesTest
-    {
-        private readonly VerifierBuilder builder = new VerifierBuilder<FieldsShouldBeEncapsulatedInProperties>();
+    private readonly VerifierBuilder builder = new VerifierBuilder<FieldsShouldBeEncapsulatedInProperties>();
 
-        [TestMethod]
-        public void FieldsShouldBeEncapsulatedInProperties() =>
-            builder.AddPaths("FieldsShouldBeEncapsulatedInProperties.cs").Verify();
+    [TestMethod]
+    public void FieldsShouldBeEncapsulatedInProperties() =>
+        builder.AddPaths("FieldsShouldBeEncapsulatedInProperties.cs").Verify();
 
 #if NET
 
-        [TestMethod]
-        public void FieldsShouldBeEncapsulatedInProperties_CSharp9() =>
-            builder.AddPaths("FieldsShouldBeEncapsulatedInProperties.CSharp9.cs")
-                .WithOptions(ParseOptionsHelper.FromCSharp9)
-                .Verify();
+    [TestMethod]
+    public void FieldsShouldBeEncapsulatedInProperties_CSharp9() =>
+        builder.AddPaths("FieldsShouldBeEncapsulatedInProperties.CSharp9.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp9)
+            .Verify();
 
-        [TestMethod]
-        public void FieldsShouldBeEncapsulatedInProperties_CSharp12() =>
-            builder.AddPaths("FieldsShouldBeEncapsulatedInProperties.CSharp12.cs")
-                .WithOptions(ParseOptionsHelper.FromCSharp12)
-                .Verify();
+    [TestMethod]
+    public void FieldsShouldBeEncapsulatedInProperties_CSharp12() =>
+        builder.AddPaths("FieldsShouldBeEncapsulatedInProperties.CSharp12.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp12)
+            .Verify();
 
 #endif
 
-    }
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/FieldsShouldBeEncapsulatedInProperties.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/FieldsShouldBeEncapsulatedInProperties.cs
@@ -90,6 +90,7 @@ public class Repro_8504
     {
         public string type;     // Compliant
         public string key;      // Compliant
-        public string value;    // Compliant
+        [NonSerialized]
+        public string value;    // Noncompliant
     }
 }