Fix S1104 FP: Do not report in Unity serializable classes (#9514)

Author: sebastien-marichal

analyzers/src/SonarAnalyzer.CSharp/Rules/FieldsShouldBeEncapsulatedInProperties.cs

@@ -18,54 +18,59 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-namespace SonarAnalyzer.Rules.CSharp
+namespace SonarAnalyzer.Rules.CSharp;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class FieldsShouldBeEncapsulatedInProperties : SonarDiagnosticAnalyzer
 {
-    [DiagnosticAnalyzer(LanguageNames.CSharp)]
-    public sealed class FieldsShouldBeEncapsulatedInProperties : SonarDiagnosticAnalyzer
-    {
-        private const string DiagnosticId = "S1104";
-        private const string MessageFormat = "Make this field 'private' and encapsulate it in a 'public' property.";
+    private const string DiagnosticId = "S1104";
+    private const string MessageFormat = "Make this field 'private' and encapsulate it in a 'public' property.";
+
+    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
 
-        private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
+    private static readonly ISet<SyntaxKind> ValidModifiers = new HashSet<SyntaxKind>
+    {
+        SyntaxKind.PrivateKeyword,
+        SyntaxKind.ProtectedKeyword,
+        SyntaxKind.InternalKeyword,
+        SyntaxKind.ReadOnlyKeyword,
+        SyntaxKind.ConstKeyword
+    };
 
-        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
+    private static readonly ImmutableArray<KnownType> IgnoredTypes = ImmutableArray.Create(
+        KnownType.UnityEngine_MonoBehaviour,
+        KnownType.UnityEngine_ScriptableObject);
 
-        private static readonly ISet<SyntaxKind> ValidModifiers = new HashSet<SyntaxKind>
-        {
-            SyntaxKind.PrivateKeyword,
-            SyntaxKind.ProtectedKeyword,
-            SyntaxKind.InternalKeyword,
-            SyntaxKind.ReadOnlyKeyword,
-            SyntaxKind.ConstKeyword
-        };
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
 
-        protected override void Initialize(SonarAnalysisContext context) =>
-            context.RegisterNodeAction(
-                c =>
+    protected override void Initialize(SonarAnalysisContext context) =>
+        context.RegisterNodeAction(
+            c =>
+            {
+                var fieldDeclaration = (FieldDeclarationSyntax)c.Node;
+                if (fieldDeclaration.Modifiers.Any(m => ValidModifiers.Contains(m.Kind())))
                 {
-                    var fieldDeclaration = (FieldDeclarationSyntax)c.Node;
-                    if (fieldDeclaration.Modifiers.Any(m => ValidModifiers.Contains(m.Kind())))
-                    {
-                        return;
-                    }
+                    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) || Serializable(symbol, parentSymbol))
-                    {
-                        return;
-                    }
+                var firstVariable = fieldDeclaration.Declaration.Variables[0];
+                var symbol = c.SemanticModel.GetDeclaredSymbol(firstVariable);
+                var parentSymbol = c.SemanticModel.GetDeclaredSymbol(fieldDeclaration.Parent);
+                if (symbol.ContainingType.DerivesFromAny(IgnoredTypes)
+                    || parentSymbol.HasAttribute(KnownType.System_Runtime_InteropServices_StructLayoutAttribute)
+                    || Serializable(symbol, parentSymbol))
+                {
+                    return;
+                }
 
-                    if (symbol.GetEffectiveAccessibility() == Accessibility.Public)
-                    {
-                        c.ReportIssue(Rule, firstVariable);
-                    }
-                },
-                SyntaxKind.FieldDeclaration);
+                if (symbol.GetEffectiveAccessibility() == Accessibility.Public)
+                {
+                    c.ReportIssue(Rule, firstVariable);
+                }
+            },
+            SyntaxKind.FieldDeclaration);
 
-        private static bool Serializable(ISymbol symbol, ISymbol parentSymbol) =>
-            parentSymbol.HasAttribute(KnownType.System_SerializableAttribute)
-            && !symbol.HasAttribute(KnownType.System_NonSerializedAttribute);
-    }
+    private static bool Serializable(ISymbol symbol, ISymbol parentSymbol) =>
+        parentSymbol.HasAttribute(KnownType.System_SerializableAttribute)
+        && !symbol.HasAttribute(KnownType.System_NonSerializedAttribute);
 }

analyzers/tests/SonarAnalyzer.Test/Rules/FieldsShouldBeEncapsulatedInPropertiesTest.cs

@@ -31,6 +31,13 @@ public class FieldsShouldBeEncapsulatedInPropertiesTest
     public void FieldsShouldBeEncapsulatedInProperties() =>
         builder.AddPaths("FieldsShouldBeEncapsulatedInProperties.cs").Verify();
 
+    [TestMethod]
+    public void FieldsShouldBeEncapsulatedInProperties_Unity3D_Ignored() =>
+        builder.AddPaths("FieldsShouldBeEncapsulatedInProperties.Unity3D.cs")
+            // Concurrent analysis puts fake Unity3D class into the Concurrent namespace
+            .WithConcurrentAnalysis(false)
+            .Verify();
+
 #if NET
 
     [TestMethod]

analyzers/tests/SonarAnalyzer.Test/TestCases/FieldsShouldBeEncapsulatedInProperties.Unity3D.cs

@@ -0,0 +1,32 @@
+using System;
+
+// https://github.com/SonarSource/sonar-dotnet/issues/7522
+public class UnityMonoBehaviour : UnityEngine.MonoBehaviour
+{
+    public int Field1; // Compliant
+}
+
+public class UnityScriptableObject : UnityEngine.ScriptableObject
+{
+    public int Field1; // Compliant
+}
+
+public class InvalidCustomSerializableClass1 : UnityEngine.Object
+{
+    public int Field1; // Noncompliant
+}
+
+[Serializable]
+public class ValidCustomSerializableClass : UnityEngine.Object
+{
+    public int Field1; // Compliant
+}
+
+// Unity3D does not seem to be available as a nuget package and we cannot use the original classes
+// Cannot run this test case in Concurrent mode because of the Concurrent namespace
+namespace UnityEngine
+{
+    public class MonoBehaviour { }
+    public class ScriptableObject { }
+    public class Object { }
+}