Fix S4529: Should not highlight constructors, properties and methods with NonActionAttribute

sonaranalyzer-dotnet/src/SonarAnalyzer.Common/Helpers/AspNetMvcHelper.cs

@@ -29,8 +29,13 @@ public static class AspNetMvcHelper
         private static readonly ImmutableArray<KnownType> controllerTypes =
             ImmutableArray.Create(
                 KnownType.Microsoft_AspNetCore_Mvc_ControllerBase,
-                KnownType.System_Web_Mvc_Controller
-            );
+                KnownType.System_Web_Mvc_Controller);
+
+        private static readonly ImmutableArray<KnownType> nonActionTypes =
+            ImmutableArray.Create(
+                KnownType.Microsoft_AspNetCore_Mvc_NonActionAttribute,
+                KnownType.System_Web_Mvc_NonActionAttribute);
+
 
         private static readonly ImmutableArray<KnownType> nonControllerAttributeTypes =
             ImmutableArray.Create(KnownType.Microsoft_AspNetCore_Mvc_NonControllerAttribute);
@@ -43,7 +48,9 @@ public static class AspNetMvcHelper
         /// controller method.
         /// </summary>
         public static bool IsControllerMethod(this IMethodSymbol methodSymbol) =>
+            methodSymbol.MethodKind == MethodKind.Ordinary &&
             methodSymbol.GetEffectiveAccessibility() == Accessibility.Public &&
+            !methodSymbol.GetAttributes().Any(d => d.AttributeClass.IsAny(nonActionTypes)) &&
             IsControllerType(methodSymbol?.ContainingType);
 
         /// <summary>

sonaranalyzer-dotnet/src/SonarAnalyzer.Common/Helpers/KnownType.cs

@@ -45,6 +45,7 @@ internal sealed class KnownType
         internal static readonly KnownType Microsoft_AspNetCore_Mvc_Controller = new KnownType("Microsoft.AspNetCore.Mvc.Controller");
         internal static readonly KnownType Microsoft_AspNetCore_Mvc_ControllerBase = new KnownType("Microsoft.AspNetCore.Mvc.ControllerBase");
         internal static readonly KnownType Microsoft_AspNetCore_Mvc_ControllerAttribute = new KnownType("Microsoft.AspNetCore.Mvc.ControllerAttribute");
+        internal static readonly KnownType Microsoft_AspNetCore_Mvc_NonActionAttribute = new KnownType("Microsoft.AspNetCore.Mvc.NonActionAttribute");
         internal static readonly KnownType Microsoft_AspNetCore_Mvc_NonControllerAttribute = new KnownType("Microsoft.AspNetCore.Mvc.NonControllerAttribute");
         internal static readonly KnownType Microsoft_AspNetCore_Mvc_VirtualFileResult = new KnownType("Microsoft.AspNetCore.Mvc.VirtualFileResult");
         internal static readonly KnownType Microsoft_AspNetCore_Routing_VirtualPathData = new KnownType("Microsoft.AspNetCore.Routing.VirtualPathData");
@@ -346,6 +347,7 @@ internal sealed class KnownType
         internal static readonly KnownType System_Web_HttpRequestBase = new KnownType("System.Web.HttpRequestBase");
         internal static readonly KnownType System_Web_HttpResponseBase = new KnownType("System.Web.HttpResponseBase");
         internal static readonly KnownType System_Web_HttpServerUtilityBase = new KnownType("System.Web.HttpServerUtilityBase");
+        internal static readonly KnownType System_Web_Mvc_NonActionAttribute = new KnownType("System.Web.Mvc.NonActionAttribute");
         internal static readonly KnownType System_Web_Mvc_Controller = new KnownType("System.Web.Mvc.Controller");
         internal static readonly KnownType System_Web_Mvc_HttpPostAttribute = new KnownType("System.Web.Mvc.HttpPostAttribute");
         internal static readonly KnownType System_Web_Mvc_UrlHelper = new KnownType("System.Web.Mvc.UrlHelper");

sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/Helpers/AspNetMvcHelper_IsControllerMethod.cs

@@ -40,10 +40,12 @@ public void PublicFoo() { }
     protected void ProtectedFoo() { }
     internal void InternalFoo() { }
     private void PrivateFoo() { }
-    private class Bar : System.Web.Mvb.Controller
+    private class Bar : System.Web.Mvc.Controller
     {
         public void InnerFoo() { }
     }
+    [System.Web.Mvc.NonActionAttribute]
+    public void PublicNonAction() { }
 }
 ";
             var compilation = TestHelper.Compile(code,
@@ -54,12 +56,14 @@ public void InnerFoo() { }
             var internalFoo = compilation.GetMethodSymbol("InternalFoo");
             var privateFoo = compilation.GetMethodSymbol("PrivateFoo");
             var innerFoo = compilation.GetMethodSymbol("InnerFoo");
+            var publicNonAction = compilation.GetMethodSymbol("PublicNonAction");
 
             AspNetMvcHelper.IsControllerMethod(publicFoo).Should().Be(true);
             AspNetMvcHelper.IsControllerMethod(protectedFoo).Should().Be(false);
             AspNetMvcHelper.IsControllerMethod(internalFoo).Should().Be(false);
             AspNetMvcHelper.IsControllerMethod(privateFoo).Should().Be(false);
             AspNetMvcHelper.IsControllerMethod(innerFoo).Should().Be(false);
+            AspNetMvcHelper.IsControllerMethod(publicNonAction).Should().Be(false);
         }
 
         [DataTestMethod]
@@ -71,6 +75,8 @@ public void Controller_Methods_Are_EntryPoints(string aspNetMvcVersion)
 public class Foo : System.Web.Mvc.Controller
 {
     public void PublicFoo() { }
+    [System.Web.Mvc.NonActionAttribute]
+    public void PublicNonAction() { }
 }
 public class Controller
 {
@@ -87,52 +93,58 @@ public void PublicDiz() { }
             var publicFoo = compilation.GetMethodSymbol("PublicFoo");
             var publicBar = compilation.GetMethodSymbol("PublicBar");
             var publicDiz = compilation.GetMethodSymbol("PublicDiz");
+            var publicNonAction = compilation.GetMethodSymbol("PublicNonAction");
 
             AspNetMvcHelper.IsControllerMethod(publicFoo).Should().Be(true);
             AspNetMvcHelper.IsControllerMethod(publicBar).Should().Be(false);
             AspNetMvcHelper.IsControllerMethod(publicDiz).Should().Be(false);
+            AspNetMvcHelper.IsControllerMethod(publicNonAction).Should().Be(false);
         }
 
         [DataTestMethod]
-        [DataRow("3.0.20105.1")]
+        [DataRow("2.1.3")]
         [DataRow(Constants.NuGetLatestVersion)]
         public void Methods_In_Classes_With_ControllerAttribute_Are_EntryPoints(string aspNetMvcVersion)
         {
             const string code = @"
-// The Attribute suffix is required because we don't have a reference
-// to ASP.NET Core and we cannot do type checking in the test project.
-// We will need to convert this test project to .NET Core to do that.
 [Microsoft.AspNetCore.Mvc.ControllerAttribute]
 public class Foo
 {
     public void PublicFoo() { }
+    [Microsoft.AspNetCore.Mvc.NonActionAttribute]
+    public void PublicNonAction() { }
 }
 ";
             var compilation = TestHelper.Compile(code,
-                additionalReferences:  NuGetMetadataReference.MicrosoftAspNetMvc(aspNetMvcVersion).ToArray());
+                additionalReferences:
+                    FrameworkMetadataReference.Netstandard
+                        .Union(NuGetMetadataReference.MicrosoftAspNetCoreMvcCore(aspNetMvcVersion))
+                        .ToArray());
 
             var publicFoo = compilation.GetMethodSymbol("PublicFoo");
+            var publicNonAction = compilation.GetMethodSymbol("PublicNonAction");
 
             AspNetMvcHelper.IsControllerMethod(publicFoo).Should().Be(true);
+            AspNetMvcHelper.IsControllerMethod(publicNonAction).Should().Be(false);
         }
 
         [DataTestMethod]
-        [DataRow("3.0.20105.1")]
+        [DataRow("2.1.3")]
         [DataRow(Constants.NuGetLatestVersion)]
         public void Methods_In_Classes_With_NonControllerAttribute_Are_Not_EntryPoints(string aspNetMvcVersion)
         {
             const string code = @"
-// The Attribute suffix is required because we don't have a reference
-// to ASP.NET Core and we cannot do type checking in the test project.
-// We will need to convert this test project to .NET Core to do that.
 [Microsoft.AspNetCore.Mvc.NonControllerAttribute]
 public class Foo : Microsoft.AspNetCore.Mvc.ControllerBase
 {
     public void PublicFoo() { }
 }
 ";
             var compilation = TestHelper.Compile(code,
-                additionalReferences:  NuGetMetadataReference.MicrosoftAspNetMvc(aspNetMvcVersion).ToArray());
+                additionalReferences:
+                    FrameworkMetadataReference.Netstandard
+                        .Union(NuGetMetadataReference.MicrosoftAspNetCoreMvcCore(aspNetMvcVersion))
+                        .ToArray());
 
             var publicFoo = compilation.GetMethodSymbol("PublicFoo");
 

sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/NuGetMetadataReference.cs

@@ -294,6 +294,9 @@ public static IEnumerable<MetadataReference> MicrosoftAspNetCoreRoutingAbstracti
         public static IEnumerable<MetadataReference> MicrosoftAspNetMvc(string packageVersion) =>
             Create("Microsoft.AspNet.Mvc", packageVersion);
 
+        public static IEnumerable<MetadataReference> MicrosoftAspNetMvcCore(string packageVersion) =>
+            Create("Microsoft.AspNet.Mvc.Core", packageVersion);
+
         public static IEnumerable<MetadataReference> MicrosoftExtensionsConfigurationAbstractions(string packageVersion) =>
             Create("Microsoft.Extensions.Configuration.Abstractions", packageVersion);
 

sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ExposingEndpoints_Net46.cs

@@ -1,7 +1,26 @@
-using System.Web.Mvc;
+using System;
+using System.Web.Mvc;
 
 public class Foo : Controller
 {
+    public Foo() { } // Compliant, ctors should not be highlighted
+
+    ~Foo() { } // Compliant, destructors should not be highlighted
+
+    public int MyProperty // Compliant, properties should not be highlighted
+    {
+        get { return 0; }
+        set { }
+    }
+
+    public static Foo operator +(Foo a, Foo b) => null; // Compliant, operators should not be highlighted
+
+    public event EventHandler<EventArgs> MyEvent // Compliant, events should not be highlighted
+    {
+        add { }
+        remove { }
+    }
+
     public void PublicFoo() { } // Noncompliant {{Make sure that exposing this HTTP endpoint is safe here.}}
 //              ^^^^^^^^^
 
@@ -11,6 +30,9 @@ internal void InternalFoo() { }
 
     private void PrivateFoo() { }
 
+    [NonAction]
+    public void PublicNonAction() { } // Compliant, methods decorated with NonAction are not entrypoints
+
     private class Bar : Controller
     {
         public void InnerFoo() { }

sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ExposingEndpoints_Net46.vb

@@ -3,6 +3,20 @@
 Public Class Foo
     Inherits Controller
 
+    Public sub New () ' Compliant, ctors should not be highlighted
+    End sub
+
+    Protected Overrides Sub Finalize() ' Compliant, desctructors should not be highlighted; in addition, this protected and will not be highlighted anyway
+    End Sub
+
+    Public Property MyProperty As Integer  ' Compliant, properties should not be highlighted
+        Get
+            Return 0
+        End Get
+        Set(ByVal value As Integer)
+        End Set
+    End Property
+
     Public Sub PublicFoo() ' Noncompliant {{Make sure that exposing this HTTP endpoint is safe here.}}
 '              ^^^^^^^^^
     End Sub
@@ -16,6 +30,10 @@ Public Class Foo
     Private Sub PrivateFoo()
     End Sub
 
+    <NonAction>
+    Public Sub PublicNonAction() ' Compliant, methods decorated with NonAction are not entrypoints
+    End Sub
+
     Private Class Bar
         Inherits Controller
 

sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ExposingEndpoints_NetCore.cs

@@ -2,6 +2,14 @@
 
 public class SomeController : ControllerBase
 {
+    public SomeController() { } // Compliant, ctors should not be highlighted
+
+    public int MyProperty // Compliant, properties should not be highlighted
+    {
+        get { return 0; }
+        set { }
+    }
+
     public void PublicFoo() { } // Noncompliant {{Make sure that exposing this HTTP endpoint is safe here.}}
 //              ^^^^^^^^^
 
@@ -11,6 +19,9 @@ internal void InternalFoo() { }
 
     private void PrivateFoo() { }
 
+    [NonAction]
+    public void PublicNonAction() { } // Compliant, methods decorated with NonAction are not entrypoints
+
     private class Bar : ControllerBase
     {
         public void InnerFoo() { }
@@ -20,6 +31,8 @@ public void InnerFoo() { }
 [Controller]
 public class MyController
 {
+    public MyController() { } // Compliant, ctor should not be highlighted
+
     public void PublicFoo() { } // Noncompliant
 }
 

sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/ExposingEndpoints_NetCore.vb

@@ -3,6 +3,18 @@
 Public Class SomeController
     Inherits ControllerBase
 
+    Public Sub New() ' Compliant, ctors should not be highlighted
+    End sub
+
+    Public Property MyProperty As Integer  ' Compliant, properties should not be highlighted
+        Get
+            Return 0
+        End Get
+        Set(ByVal value As Integer)
+        End Set
+    End Property
+
+
     Public Sub PublicFoo() ' Noncompliant {{Make sure that exposing this HTTP endpoint is safe here.}}
 '              ^^^^^^^^^
     End Sub
@@ -16,6 +28,10 @@ Public Class SomeController
     Private Sub PrivateFoo()
     End Sub
 
+    <NonAction>
+    Public Sub PublicNonAction() ' Compliant, methods decorated with NonAction are not entrypoints
+    End Sub
+
     Private Class Bar
         Inherits ControllerBase
 
@@ -26,6 +42,9 @@ End Class
 
 <Controller>
 Public Class MyController
+    Public Sub New() ' Compliant, ctor should not be highlighted
+    End sub
+
     Public Sub PublicFoo() ' Noncompliant
     End Sub
 End Class