Skip to content

Commit

Permalink
First implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
zsolt-kolbay-sonarsource committed Apr 17, 2024
1 parent 57e84e4 commit f010cbb
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 15 deletions.
56 changes: 49 additions & 7 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/AvoidUnderPosting.cs
Expand Up @@ -18,26 +18,68 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Collections.Concurrent;

namespace SonarAnalyzer.Rules.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class AvoidUnderPosting : SonarDiagnosticAnalyzer
{
private const string DiagnosticId = "S6964";
private const string MessageFormat = "FIXME";
private const string MessageFormat = "Property used as input in a controller action should be nullable or annotated with the Required attribute to avoid under-posting.";

private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterNodeAction(c =>
context.RegisterCompilationStartAction(compilationStart =>
{
if (!compilationStart.Compilation.ReferencesControllers())
{
return;
}
var actionParameters = new ConcurrentStack<IParameterSymbol>();
compilationStart.RegisterSymbolStartAction(symbolStart =>
{
var node = c.Node;
if (true)
var method = (IMethodSymbol)symbolStart.Symbol;
if (method.IsControllerMethod())
{
c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation()));
actionParameters.PushRange([.. method.Parameters]);
}
},
SyntaxKind.InvocationExpression);
}, SymbolKind.Method);
compilationStart.RegisterCompilationEndAction(compilationEnd =>
{
var examinedTypes = new HashSet<ITypeSymbol>();
foreach (var parameter in actionParameters)
{
if (!examinedTypes.Contains(parameter.Type))
{
examinedTypes.Add(parameter.Type);
CheckInvalidProperties(parameter, compilationEnd);
}
}
});
});

private static void CheckInvalidProperties(IParameterSymbol actionParameter, SonarCompilationReportingContext context)
{
if (actionParameter.Type.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax() is { } parameterSyntax)
{
var invalidProperties = actionParameter.Type.GetMembers()
.Where(x => x is IPropertySymbol property
&& !property.Type.CanBeNull()
&& property.GetEffectiveAccessibility() == Accessibility.Public
&& property.SetMethod?.DeclaredAccessibility is Accessibility.Public
&& !property.HasAttribute(KnownType.System_ComponentModel_DataAnnotations_RequiredAttribute)
&& !property.HasAttribute(KnownType.Microsoft_AspNetCore_Mvc_ModelBinding_Validation_ValidateNeverAttribute));
foreach (var property in invalidProperties)
{
var propertySyntax = property.DeclaringSyntaxReferences[0].GetSyntax();
context.ReportIssue(CSharpGeneratedCodeRecognizer.Instance, Diagnostic.Create(Rule, propertySyntax.GetLocation(), parameterSyntax.GetLocation()));
}
}
}
}
2 changes: 2 additions & 0 deletions analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs
Expand Up @@ -81,6 +81,7 @@ public sealed partial class KnownType
public static readonly KnownType Microsoft_AspNetCore_Mvc_HttpPutAttribute = new("Microsoft.AspNetCore.Mvc.HttpPutAttribute");
public static readonly KnownType Microsoft_AspNetCore_Mvc_IActionResult = new("Microsoft.AspNetCore.Mvc.IActionResult");
public static readonly KnownType Microsoft_AspNetCore_Mvc_IgnoreAntiforgeryTokenAttribute = new("Microsoft.AspNetCore.Mvc.IgnoreAntiforgeryTokenAttribute");
public static readonly KnownType Microsoft_AspNetCore_Mvc_ModelBinding_Validation_ValidateNeverAttribute = new("Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidateNeverAttribute");
public static readonly KnownType Microsoft_AspNetCore_Mvc_NonActionAttribute = new("Microsoft.AspNetCore.Mvc.NonActionAttribute");
public static readonly KnownType Microsoft_AspNetCore_Mvc_NonControllerAttribute = new("Microsoft.AspNetCore.Mvc.NonControllerAttribute");
public static readonly KnownType Microsoft_AspNetCore_Mvc_RazorPages_PageModel = new("Microsoft.AspNetCore.Mvc.RazorPages.PageModel");
Expand Down Expand Up @@ -279,6 +280,7 @@ public sealed partial class KnownType
public static readonly KnownType System_ComponentModel_Composition_PartCreationPolicyAttribute = new("System.ComponentModel.Composition.PartCreationPolicyAttribute");
public static readonly KnownType System_ComponentModel_DataAnnotations_KeyAttribute = new("System.ComponentModel.DataAnnotations.KeyAttribute");
public static readonly KnownType System_ComponentModel_DataAnnotations_RegularExpressionAttribute = new("System.ComponentModel.DataAnnotations.RegularExpressionAttribute");
public static readonly KnownType System_ComponentModel_DataAnnotations_RequiredAttribute = new("System.ComponentModel.DataAnnotations.RequiredAttribute");
public static readonly KnownType System_ComponentModel_DefaultValueAttribute = new("System.ComponentModel.DefaultValueAttribute");
public static readonly KnownType System_ComponentModel_EditorBrowsableAttribute = new("System.ComponentModel.EditorBrowsableAttribute");
public static readonly KnownType System_ComponentModel_LocalizableAttribute = new("System.ComponentModel.LocalizableAttribute");
Expand Down
17 changes: 14 additions & 3 deletions analyzers/tests/SonarAnalyzer.Test/Rules/AvoidUnderPostingTest.cs
Expand Up @@ -18,16 +18,27 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

#if NET

using SonarAnalyzer.Rules.CSharp;

namespace SonarAnalyzer.Test.Rules;

[TestClass]
public class AvoidUnderPostingTest
{
private readonly VerifierBuilder builder = new VerifierBuilder<AvoidUnderPosting>();
private readonly VerifierBuilder builder = new VerifierBuilder<AvoidUnderPosting>()
.WithOptions(ParseOptionsHelper.FromCSharp12)
.AddReferences([
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcAbstractions,
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcCore,
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcViewFeatures,
..NuGetMetadataReference.SystemComponentModelAnnotations(Constants.NuGetLatestVersion)
]);

[TestMethod]
public void AvoidUnderPosting_CS() =>
builder.AddPaths("AvoidUnderPosting.cs").Verify();
public void AvoidUnderPosting_CSharp12() =>
builder.AddPaths("AvoidUnderPosting.CSharp12.cs").Verify();
}

#endif
@@ -0,0 +1,76 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
using System.ComponentModel.DataAnnotations;

public class ClassNotUsedInRequests
{
int ValueProperty { get; set; } // Compliant
}

public struct Struct { public int Foo { get; set; } }
public record struct RecordStruct { public int Foo { get; set; } }

public class ModelUsedInController
{
public int ValueProperty { get; set; } // Noncompliant
public int? NullableValueProperty { get; set; } // Compliant
[Required] public int RequiredValueProperty { get; set; } // Compliant
[ValidateNever] public int NotValidatedValueProperty { get; set; } // Compliant
[Range(0, 10)] public int ValuePropertyWithRangeValidation { get; set; } // Noncompliant
[Required] public int? RequiredNullableValueProperty { get; set; } // Compliant
public int PropertyWithPrivateSetter { get; private set; } // Compliant
protected int ProtectedProperty { get; set; } // Compliant
internal int InternalProperty { get; set; } // Compliant
protected internal int ProtectedInternalProperty { get; set; } // Compliant
private int PrivateProperty { get; set; } // Compliant
public int ReadOnlyProperty => 42; // Compliant
public int field = 42; // Compliant

public Struct StructValueProperty { get; set; } // Noncompliant
public RecordStruct RecordStructValueProperty { get; set; } // Noncompliant

#nullable enable
public string NonNullableReferenceProperty { get; set; } // Noncompliant
[Required] public string RequiredNonNullableReferenceProperty { get; set; } // Compliant
public string? NullableReferenceProperty { get; set; } // Compliant
#nullable disable
public string ReferenceProperty { get; set; } // Compliant
}

public class DerivedFromController : Controller
{
[HttpPost]
public IActionResult Create(ModelUsedInController model)
{
return View(model);
}
}

[Controller]
public class DecoratedWithControllerAttribute // better suited for parameterized UTs
{
[HttpGet] public IActionResult Get(ModelUsedInController model) => null;
[HttpPost] public IActionResult Post(ModelUsedInController model) => null;
[HttpPut] public IActionResult Put(ModelUsedInController model) => null;
[HttpDelete] public IActionResult Delete(ModelUsedInController model) => null;
}

[ApiController]
[Route("api/[controller]")]
public class DecoratedWithApiControlerAttribute : ControllerBase
{
[HttpGet]
public int Single(ModelUsedInController model) => 42;

[HttpGet]
[HttpPost]
[HttpPut]
[HttpDelete]
public int Multiple(ModelUsedInController model) => 42;

[AcceptVerbs("POST")]
public int Verb(ModelUsedInController model) => 42;

[AcceptVerbs("GET", "POST", "PUT", "DELETE")]
public int MultipleVerbs(ModelUsedInController model) => 42;
}

This file was deleted.

0 comments on commit f010cbb

Please sign in to comment.