From 124a3dd1ea4c94698244527c7cf16db2416f01a2 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Tue, 28 May 2024 16:25:43 +0200 Subject: [PATCH 1/2] Fix FP for properties with default values --- .../Rules/AspNet/AvoidUnderPosting.cs | 13 +++++++++++-- .../TestCases/AspNet/AvoidUnderPosting.CSharp9.cs | 5 +++-- .../TestCases/AspNet/AvoidUnderPosting.cs | 1 + 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs index e3ded471da3..1573dd570c1 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs @@ -76,10 +76,12 @@ private static void CheckInvalidProperties(INamedTypeSymbol parameterType, Sonar var declaredProperties = new List(); GetAllDeclaredProperties(parameterType, examinedTypes, declaredProperties); var invalidProperties = declaredProperties - .Where(x => !CanBeNull(x.Type) && !x.HasAttribute(KnownType.System_Text_Json_Serialization_JsonRequiredAttribute) && !x.IsRequired()); + .Where(x => !CanBeNull(x.Type) && !x.HasAttribute(KnownType.System_Text_Json_Serialization_JsonRequiredAttribute) && !x.IsRequired()) + .Select(x => x.GetFirstSyntaxRef()) + .Where(x => !HasDefaultValue(x)); foreach (var property in invalidProperties) { - context.ReportIssue(Rule, property.GetFirstSyntaxRef().GetIdentifier()?.GetLocation()); + context.ReportIssue(Rule, property.GetIdentifier()?.GetLocation()); } } @@ -144,4 +146,11 @@ INamedTypeSymbol namedType when type.IsInSameAssembly(controllerType) => [namedT private static bool HasValidateNeverAttribute(ISymbol symbol) => symbol.HasAttribute(KnownType.Microsoft_AspNetCore_Mvc_ModelBinding_Validation_ValidateNeverAttribute); + + private static bool HasDefaultValue(SyntaxNode node) => node switch + { + ParameterSyntax { Default: not null } => true, + PropertyDeclarationSyntax { Initializer: not null } property => property.Parent.ParameterList() is null, + _ => false + }; } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp9.cs index 25d984a5cbb..c9677bed495 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp9.cs @@ -7,8 +7,9 @@ namespace CSharp9 // https://learn.microsoft.com/en-us/aspnet/core/mvc/models/model-binding#constructor-binding-and-record-types public record RecordModel( int ValueProperty, // Noncompliant - [property: JsonRequired] int RequiredValueProperty, // without the property prefix the attribute would have been applied to the constructor parameter instead - int? NullableValueProperty); + [property: JsonRequired] int RequiredValueProperty, // without the property prefix the attribute would have been applied to the constructor parameter instead + int? NullableValueProperty, + int PropertyWithDefaultValue = 42); public class ModelUsedInController { diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.cs index 6947fe1feea..c5f5a6e82b0 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.cs @@ -24,6 +24,7 @@ public class ModelUsedInController internal int InternalProperty { get; set; } protected internal int ProtectedInternalProperty { get; set; } private int PrivateProperty { get; set; } + public int PropertyWithDefaultValue { get; set; } = 42; public int ReadOnlyProperty => 42; public int field = 42; From c41fc6cada1609ca188d3955045e2dd124cfd767 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Wed, 29 May 2024 10:52:01 +0200 Subject: [PATCH 2/2] Review --- .../Rules/AspNet/AvoidUnderPosting.cs | 10 +++------- .../TestCases/AspNet/AvoidUnderPosting.CSharp12.cs | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs index 1573dd570c1..1b4d72415dd 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs @@ -78,7 +78,7 @@ private static void CheckInvalidProperties(INamedTypeSymbol parameterType, Sonar var invalidProperties = declaredProperties .Where(x => !CanBeNull(x.Type) && !x.HasAttribute(KnownType.System_Text_Json_Serialization_JsonRequiredAttribute) && !x.IsRequired()) .Select(x => x.GetFirstSyntaxRef()) - .Where(x => !HasDefaultValue(x)); + .Where(x => !IsInitialized(x)); foreach (var property in invalidProperties) { context.ReportIssue(Rule, property.GetIdentifier()?.GetLocation()); @@ -147,10 +147,6 @@ INamedTypeSymbol namedType when type.IsInSameAssembly(controllerType) => [namedT private static bool HasValidateNeverAttribute(ISymbol symbol) => symbol.HasAttribute(KnownType.Microsoft_AspNetCore_Mvc_ModelBinding_Validation_ValidateNeverAttribute); - private static bool HasDefaultValue(SyntaxNode node) => node switch - { - ParameterSyntax { Default: not null } => true, - PropertyDeclarationSyntax { Initializer: not null } property => property.Parent.ParameterList() is null, - _ => false - }; + private static bool IsInitialized(SyntaxNode node) => + node is ParameterSyntax { Default: not null } or PropertyDeclarationSyntax { Initializer: not null }; } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp12.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp12.cs index 5f37cf7af16..73b26a7ce32 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp12.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp12.cs @@ -13,7 +13,7 @@ public class ModelWithPrimaryConstructor(int vp, int rvp, int nvp) public class ModelWithPrimaryAndParameterlessConstructor(int vp, int rvp, int nvp) { public ModelWithPrimaryAndParameterlessConstructor() : this(0, 0, 0) { } - public int ValueProperty { get; set; } = vp; // Noncompliant + public int ValueProperty { get; set; } = vp; // Compliant - the property has default value } public class DerivedFromController : Controller