Skip to content

Commit

Permalink
Improve S6964: Include array types and nested collection types (#9239)
Browse files Browse the repository at this point in the history
  • Loading branch information
zsolt-kolbay-sonarsource committed May 2, 2024
1 parent c298484 commit 24ec493
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ private static void ProcessControllerMethods(SonarSyntaxNodeReportingContext con
{
var modelParameterTypes = method.Parameters
.Where(x => !HasValidateNeverAttribute(x))
.Select(x => x.Type)
.OfType<INamedTypeSymbol>()
.SelectMany(RelatedTypesToExamine)
.SelectMany(x => RelatedTypesToExamine(x.Type, method.ContainingType))
.Distinct();
foreach (var modelParameterType in modelParameterTypes)
{
Expand Down Expand Up @@ -134,10 +132,16 @@ private static void GetAllDeclaredProperties(ITypeSymbol type, ConcurrentDiction
}
}

private static IEnumerable<INamedTypeSymbol> RelatedTypesToExamine(INamedTypeSymbol type) =>
type.DerivesOrImplements(KnownType.System_Collections_Generic_IEnumerable_T)
? type.TypeArguments.OfType<INamedTypeSymbol>()
: [type];
// We only consider Model types that are in the same assembly as the Controller, because Roslyn can't raise an issue when the location is in a different assembly than the one being analyzed.
private static IEnumerable<INamedTypeSymbol> RelatedTypesToExamine(ITypeSymbol type, ITypeSymbol controllerType) =>
type switch
{
IArrayTypeSymbol array => RelatedTypesToExamine(array.ElementType, controllerType),
INamedTypeSymbol collection when collection.DerivesOrImplements(KnownType.System_Collections_Generic_IEnumerable_T) =>
collection.TypeArguments.SelectMany(x => RelatedTypesToExamine(x, controllerType)),
INamedTypeSymbol namedType when type.IsInSameAssembly(controllerType) => [namedType],
_ => []
};

private static bool HasValidateNeverAttribute(ISymbol symbol) =>
symbol.HasAttribute(KnownType.Microsoft_AspNetCore_Mvc_ModelBinding_Validation_ValidateNeverAttribute);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ namespace SonarAnalyzer.Test.Rules;
[TestClass]
public class AvoidUnderPostingTest
{
private readonly VerifierBuilder builder = new VerifierBuilder<AvoidUnderPosting>()
.WithBasePath("AspNet")
.AddReferences([
private static readonly IEnumerable<MetadataReference> AspNetReferences = [
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcAbstractions,
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcCore,
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcViewFeatures,
..NuGetMetadataReference.SystemComponentModelAnnotations(Constants.NuGetLatestVersion)
]);
..NuGetMetadataReference.SystemComponentModelAnnotations(Constants.NuGetLatestVersion)];

private readonly VerifierBuilder builder = new VerifierBuilder<AvoidUnderPosting>()
.WithBasePath("AspNet")
.AddReferences(AspNetReferences);

[TestMethod]
public void AvoidUnderPosting_CSharp() =>
Expand Down Expand Up @@ -103,6 +104,43 @@ public class ControllerClass : Controller
[{{attribute}}] public IActionResult Create(Model model) => View(model);
}
""").Verify();

[TestMethod]
public void AvoidUnderPosting_ModelInDifferentProject_CSharp()
{
const string modelCode = """
namespace Models
{
public class Person
{
public int Age { get; set; } // FN - Roslyn can't raise an issue when the location is in different project than the one being analyzed
}
}
""";
const string controllerCode = """
using Microsoft.AspNetCore.Mvc;
using Models;

namespace Controllers
{
public class PersonController : Controller
{
[HttpPost] public IActionResult Post(Person person) => View(person);
}
}
""";
var solution = SolutionBuilder.Create()
.AddProject(AnalyzerLanguage.CSharp)
.AddSnippet(modelCode)
.Solution
.AddProject(AnalyzerLanguage.CSharp)
.AddProjectReference(x => x.ProjectIds[0])
.AddReferences(AspNetReferences)
.AddSnippet(controllerCode)
.Solution;
var compiledAspNetProject = solution.Compile()[1];
DiagnosticVerifier.Verify(compiledAspNetProject, new AvoidUnderPosting());
}
}

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,24 @@ public class ControllerClass : Controller
}
}

namespace GenericCollections
namespace Collections
{
public class ArrayItem { public int Property { get; set; } } // Noncompliant
public class NestedArrayItem { public int Property { get; set; } } // Noncompliant
public class EnumerableItem { public int Property { get; set; } } // Noncompliant
public class ListItem { public int Property { get; set; } } // Noncompliant
public class DictionaryKeyItem { public int Property { get; set; } } // Noncompliant
public class DictionaryValueItem { public int Property { get; set; } } // Noncompliant
public class NestedCollectionItem { public int Property { get; set; } } // Noncompliant

public class ControllerClass : Controller
{
[HttpPost] public IActionResult CreateArray(ArrayItem[] model) => View(model);
[HttpPost] public IActionResult CreateNestedArray(NestedArrayItem[] model) => View(model);
[HttpPost] public IActionResult CreateEnumerable(IEnumerable<EnumerableItem> model) => View(model);
[HttpPost] public IActionResult CreateList(List<ListItem> model) => View(model);
[HttpPost] public IActionResult CreateDictionary(Dictionary<DictionaryKeyItem, DictionaryValueItem> model) => View(model);
[HttpPost] public IActionResult CreateNestedCollection(Dictionary<string, IEnumerable<NestedCollectionItem[]>> model) => View(model);
}
}

Expand Down Expand Up @@ -270,3 +276,13 @@ public class CustomController : Controller
[HttpPost] public IActionResult Post(Model model) => View(model);
}
}

namespace GeneralTypes
{
public class CustomController : Controller
{
[HttpPost] public IActionResult PostObject(object model) => View(model);
[HttpPost] public IActionResult PostString(string model) => View(model);
[HttpPost] public IActionResult PostDynamic(dynamic model) => View(model);
}
}

0 comments on commit 24ec493

Please sign in to comment.