From 5decea08ba0af5439232e072841c7e8ce2b48efe Mon Sep 17 00:00:00 2001 From: Tim Pohlmann Date: Mon, 15 Jan 2024 16:23:44 +0100 Subject: [PATCH] Fix S1104 FN: In types marked as [Serializable], do not ignore fields marked as [NonSerialized] (#8531) --- .../FieldsShouldBeEncapsulatedInProperties.cs | 12 +++--- .../SonarAnalyzer.Common/Helpers/KnownType.cs | 1 + ...ldsShouldBeEncapsulatedInPropertiesTest.cs | 37 +++++++++---------- .../FieldsShouldBeEncapsulatedInProperties.cs | 3 +- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/FieldsShouldBeEncapsulatedInProperties.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/FieldsShouldBeEncapsulatedInProperties.cs index 806b4e71e46..818aeb1af87 100644 --- 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 index b5ae682b470..260978c93e2 100644 --- 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 index ed2ca5b5c99..75b7761bed2 100644 --- 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(); + private readonly VerifierBuilder builder = new VerifierBuilder(); - [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 index 9d4866213f5..329e33e3e48 100644 --- 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 } }