From c29527f992c565f7792d50b3bfffc8d2bbe0eb2a Mon Sep 17 00:00:00 2001 From: Pranav K Date: Wed, 6 Jun 2018 18:51:05 -0700 Subject: [PATCH] Add some assertions for controllers and controller actions --- .../MvcFacts.cs | 137 ++++++++++ .../MvcFactsTest.cs | 235 ++++++++++++++++++ .../MvcFactsTest/IsControllerActionTests.cs | 75 ++++++ .../MvcFactsTest/IsControllerTests.cs | 44 ++++ 4 files changed, 491 insertions(+) create mode 100644 src/Microsoft.AspNetCore.Mvc.Analyzers/MvcFacts.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Analyzers.Test/MvcFactsTest.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Analyzers.Test/TestFiles/MvcFactsTest/IsControllerActionTests.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Analyzers.Test/TestFiles/MvcFactsTest/IsControllerTests.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Analyzers/MvcFacts.cs b/src/Microsoft.AspNetCore.Mvc.Analyzers/MvcFacts.cs new file mode 100644 index 0000000000..90651247c4 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Analyzers/MvcFacts.cs @@ -0,0 +1,137 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Diagnostics; +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + internal static class MvcFacts + { + public static bool IsController(INamedTypeSymbol type, INamedTypeSymbol controllerAttribute, INamedTypeSymbol nonControllerAttribute) + { + Debug.Assert(type != null); + Debug.Assert(controllerAttribute != null); + Debug.Assert(nonControllerAttribute != null); + + if (type.TypeKind != TypeKind.Class) + { + return false; + } + + if (type.IsAbstract) + { + return false; + } + + // We only consider public top-level classes as controllers. + if (type.DeclaredAccessibility != Accessibility.Public) + { + return false; + } + + if (type.ContainingType != null) + { + return false; + } + + if (type.IsGenericType || type.IsUnboundGenericType) + { + return false; + } + + if (type.HasAttribute(nonControllerAttribute, inherit: true)) + { + return false; + } + + if (!type.Name.EndsWith("Controller", StringComparison.OrdinalIgnoreCase) && + !type.HasAttribute(controllerAttribute, inherit: true)) + { + return false; + } + + return true; + } + + public static bool IsControllerAction(IMethodSymbol method, INamedTypeSymbol nonActionAttribute, IMethodSymbol disposableDispose) + { + Debug.Assert(method != null); + Debug.Assert(nonActionAttribute != null); + + if (method.MethodKind != MethodKind.Ordinary) + { + return false; + } + + if (method.HasAttribute(nonActionAttribute, inherit: true)) + { + return false; + } + + // Overridden methods from Object class, e.g. Equals(Object), GetHashCode(), etc., are not valid. + if (GetDeclaringType(method).SpecialType == SpecialType.System_Object) + { + return false; + } + + if (IsIDisposableDispose(method, disposableDispose)) + { + return false; + } + + if (method.IsStatic) + { + return false; + } + + if (method.IsAbstract) + { + return false; + } + + if (method.IsGenericMethod) + { + return false; + } + + return method.DeclaredAccessibility == Accessibility.Public; + } + + private static INamedTypeSymbol GetDeclaringType(IMethodSymbol method) + { + while (method.IsOverride) + { + method = method.OverriddenMethod; + } + + return method.ContainingType; + } + + private static bool IsIDisposableDispose(IMethodSymbol method, IMethodSymbol disposableDispose) + { + if (method.Name != disposableDispose.Name) + { + return false; + } + + if (method.Parameters.Length != disposableDispose.Parameters.Length) + { + return false; + } + + // Explicit implementation + for (var i = 0; i < method.ExplicitInterfaceImplementations.Length; i++) + { + if (method.ExplicitInterfaceImplementations[i].ContainingType.SpecialType == SpecialType.System_IDisposable) + { + return true; + } + } + + var implementedMethod = method.ContainingType.FindImplementationForInterfaceMember(disposableDispose); + return implementedMethod == method; + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/MvcFactsTest.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/MvcFactsTest.cs new file mode 100644 index 0000000000..1d81f7e41d --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/MvcFactsTest.cs @@ -0,0 +1,235 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Analyzer.Testing; +using Microsoft.AspNetCore.Mvc.Analyzers.Infrastructure; +using Microsoft.CodeAnalysis; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public class MvcFactsTest + { + private static readonly string ControllerAttribute = typeof(ControllerAttribute).FullName; + private static readonly string NonControllerAttribute = typeof(NonControllerAttribute).FullName; + private static readonly string NonActionAttribute = typeof(NonActionAttribute).FullName; + private static readonly Type TestIsControllerActionType = typeof(TestIsControllerAction); + + #region IsController + [Fact] + public Task IsController_ReturnsFalseForInterfaces() => IsControllerReturnsFalse(typeof(ITestController)); + + [Fact] + public Task IsController_ReturnsFalseForAbstractTypes() => IsControllerReturnsFalse(typeof(AbstractController)); + + [Fact] + public Task IsController_ReturnsFalseForValueType() => IsControllerReturnsFalse(typeof(ValueTypeController)); + + + [Fact] + public Task IsController_ReturnsFalseForGenericType() => IsControllerReturnsFalse(typeof(OpenGenericController<>)); + + [Fact] + public Task IsController_ReturnsFalseForPocoType() => IsControllerReturnsFalse(typeof(PocoType)); + + [Fact] + public Task IsController_ReturnsFalseForTypeDerivedFromPocoType() => IsControllerReturnsFalse(typeof(DerivedPocoType)); + + [Fact] + public Task IsController_ReturnsTrueForTypeDerivingFromController() => IsControllerReturnsTrue(typeof(TypeDerivingFromController)); + + [Fact] + public Task IsController_ReturnsTrueForTypeDerivingFromControllerBase() => IsControllerReturnsTrue(typeof(TypeDerivingFromControllerBase)); + + [Fact] + public Task IsController_ReturnsTrueForTypeDerivingFromController_WithoutSuffix() => IsControllerReturnsTrue(typeof(NoSuffix)); + + [Fact] + public Task IsController_ReturnsTrueForTypeWithSuffix_ThatIsNotDerivedFromController() => IsControllerReturnsTrue(typeof(PocoController)); + + [Fact] + public Task IsController_ReturnsTrueForTypeWithoutSuffix_WithControllerAttribute() => IsControllerReturnsTrue(typeof(CustomBase)); + + [Fact] + public Task IsController_ReturnsTrueForTypeDerivingFromCustomBaseThatHasControllerAttribute() => IsControllerReturnsTrue(typeof(ChildOfCustomBase)); + + [Fact] + public Task IsController_ReturnsFalseForTypeWithNonControllerAttribute() => IsControllerReturnsFalse(typeof(BaseNonController)); + + [Fact] + public Task IsController_ReturnsFalseForTypesDerivingFromTypeWithNonControllerAttribute() => IsControllerReturnsFalse(typeof(BasePocoNonControllerChildController)); + + [Fact] + public Task IsController_ReturnsFalseForTypesDerivingFromTypeWithNonControllerAttributeWithControllerAttribute() => + IsControllerReturnsFalse(typeof(ControllerAttributeDerivingFromNonController)); + + private async Task IsControllerReturnsFalse(Type type) + { + var compilation = await GetIsControllerCompilation(); + var controllerAttribute = compilation.GetTypeByMetadataName(ControllerAttribute); + var nonControllerAttribute = compilation.GetTypeByMetadataName(NonControllerAttribute); + var typeSymbol = compilation.GetTypeByMetadataName(type.FullName); + + // Act + var isController = MvcFacts.IsController(typeSymbol, controllerAttribute, nonControllerAttribute); + + // Assert + Assert.False(isController); + } + + private async Task IsControllerReturnsTrue(Type type) + { + var compilation = await GetIsControllerCompilation(); + var controllerAttribute = compilation.GetTypeByMetadataName(ControllerAttribute); + var nonControllerAttribute = compilation.GetTypeByMetadataName(NonControllerAttribute); + var typeSymbol = compilation.GetTypeByMetadataName(type.FullName); + + // Act + var isController = MvcFacts.IsController(typeSymbol, controllerAttribute, nonControllerAttribute); + + // Assert + Assert.True(isController); + } + + #endregion + + #region IsControllerAction + [Fact] + public Task IsAction_ReturnsFalseForConstructor() => IsActionReturnsFalse(TestIsControllerActionType, ".ctor"); + + [Fact] + public Task IsAction_ReturnsFalseForStaticConstructor() => IsActionReturnsFalse(TestIsControllerActionType, ".cctor"); + + [Fact] + public Task IsAction_ReturnsFalseForPrivateMethod() => IsActionReturnsFalse(TestIsControllerActionType, "PrivateMethod"); + + [Fact] + public Task IsAction_ReturnsFalseForProtectedMethod() => IsActionReturnsFalse(TestIsControllerActionType, "ProtectedMethod"); + + [Fact] + public Task IsAction_ReturnsFalseForInternalMethod() => IsActionReturnsFalse(TestIsControllerActionType, nameof(TestIsControllerAction.InternalMethod)); + + [Fact] + public Task IsAction_ReturnsFalseForGenericMethod() => IsActionReturnsFalse(TestIsControllerActionType, nameof(TestIsControllerAction.GenericMethod)); + + [Fact] + public Task IsAction_ReturnsFalseForStaticMethod() => IsActionReturnsFalse(TestIsControllerActionType, nameof(TestIsControllerAction.StaticMethod)); + + [Fact] + public Task IsAction_ReturnsFalseForNonActionMethod() => IsActionReturnsFalse(TestIsControllerActionType, nameof(TestIsControllerAction.NonAction)); + + [Fact] + public Task IsAction_ReturnsFalseForOverriddenNonActionMethod() => IsActionReturnsFalse(TestIsControllerActionType, nameof(TestIsControllerAction.NonActionBase)); + + [Fact] + public Task IsAction_ReturnsFalseForDisposableDispose() => IsActionReturnsFalse(TestIsControllerActionType, nameof(TestIsControllerAction.Dispose)); + + [Fact] + public Task IsAction_ReturnsFalseForExplicitDisposableDispose() => IsActionReturnsFalse(typeof(ExplicitIDisposable), "System.IDisposable.Dispose"); + + [Fact] + public Task IsAction_ReturnsFalseForAbstractMethods() => IsActionReturnsFalse(typeof(TestIsControllerActionBase), nameof(TestIsControllerActionBase.AbstractMethod)); + + [Fact] + public Task IsAction_ReturnsFalseForObjectEquals() => IsActionReturnsFalse(typeof(object), nameof(object.Equals)); + + [Fact] + public Task IsAction_ReturnsFalseForObjectHashCode() => IsActionReturnsFalse(typeof(object), nameof(object.GetHashCode)); + + [Fact] + public Task IsAction_ReturnsFalseForObjectToString() => IsActionReturnsFalse(typeof(object), nameof(object.ToString)); + + [Fact] + public Task IsAction_ReturnsFalseForOverriddenObjectEquals() => + IsActionReturnsFalse(typeof(OverridesObjectMethods), nameof(OverridesObjectMethods.Equals)); + + [Fact] + public Task IsAction_ReturnsFalseForOverriddenObjectHashCode() => + IsActionReturnsFalse(typeof(OverridesObjectMethods), nameof(OverridesObjectMethods.GetHashCode)); + + private async Task IsActionReturnsFalse(Type type, string methodName) + { + var compilation = await GetIsControllerActionCompilation(); + var nonActionAttribute = compilation.GetTypeByMetadataName(NonActionAttribute); + var disposableDispose = GetDisposableDispose(compilation); + var typeSymbol = compilation.GetTypeByMetadataName(type.FullName); + var method = (IMethodSymbol)typeSymbol.GetMembers(methodName).First(); + + // Act + var isControllerAction = MvcFacts.IsControllerAction(method, nonActionAttribute, disposableDispose); + + // Assert + Assert.False(isControllerAction); + } + + [Fact] + public Task IsAction_ReturnsTrueForNewMethodsOfObject() => IsActionReturnsTrue(typeof(OverridesObjectMethods), nameof(OverridesObjectMethods.ToString)); + + [Fact] + public Task IsAction_ReturnsTrueForNotDisposableDispose() => IsActionReturnsTrue(typeof(NotDisposable), nameof(NotDisposable.Dispose)); + + [Fact] + public Task IsAction_ReturnsTrueForNotDisposableDisposeOnTypeWithExplicitImplementation() => + IsActionReturnsTrue(typeof(NotDisposableWithExplicitImplementation), nameof(NotDisposableWithExplicitImplementation.Dispose)); + + [Fact] + public Task IsAction_ReturnsTrueForOrdinaryAction() => IsActionReturnsTrue(TestIsControllerActionType, nameof(TestIsControllerAction.Ordinary)); + + [Fact] + public Task IsAction_ReturnsTrueForOverriddenMethod() => IsActionReturnsTrue(TestIsControllerActionType, nameof(TestIsControllerAction.AbstractMethod)); + + [Fact] + public async Task IsAction_ReturnsTrueForNotDisposableDisposeOnTypeWithImplicitImplementation() + { + var compilation = await GetIsControllerActionCompilation(); + var nonActionAttribute = compilation.GetTypeByMetadataName(NonActionAttribute); + var disposableDispose = GetDisposableDispose(compilation); + var typeSymbol = compilation.GetTypeByMetadataName(typeof(NotDisposableWithDisposeThatIsNotInterfaceContract).FullName); + var method = typeSymbol.GetMembers(nameof(IDisposable.Dispose)).OfType().First(f => !f.ReturnsVoid); + + // Act + var isControllerAction = MvcFacts.IsControllerAction(method, nonActionAttribute, disposableDispose); + + // Assert + Assert.True(isControllerAction); + } + + private async Task IsActionReturnsTrue(Type type, string methodName) + { + var compilation = await GetIsControllerActionCompilation(); + var nonActionAttribute = compilation.GetTypeByMetadataName(NonActionAttribute); + var disposableDispose = GetDisposableDispose(compilation); + var typeSymbol = compilation.GetTypeByMetadataName(type.FullName); + var method = (IMethodSymbol)typeSymbol.GetMembers(methodName).First(); + + // Act + var isControllerAction = MvcFacts.IsControllerAction(method, nonActionAttribute, disposableDispose); + + // Assert + Assert.True(isControllerAction); + } + + private IMethodSymbol GetDisposableDispose(Compilation compilation) + { + var type = compilation.GetSpecialType(SpecialType.System_IDisposable); + return (IMethodSymbol)type.GetMembers(nameof(IDisposable.Dispose)).First(); + } + #endregion + + + private Task GetIsControllerCompilation() => GetCompilation("IsControllerTests"); + + private Task GetIsControllerActionCompilation() => GetCompilation("IsControllerActionTests"); + + private Task GetCompilation(string test) + { + var testSource = MvcTestSource.Read(GetType().Name, test); + var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source }); + + return project.GetCompilationAsync(); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/TestFiles/MvcFactsTest/IsControllerActionTests.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/TestFiles/MvcFactsTest/IsControllerActionTests.cs new file mode 100644 index 0000000000..607715bcb7 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/TestFiles/MvcFactsTest/IsControllerActionTests.cs @@ -0,0 +1,75 @@ +using System; + +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public abstract class TestIsControllerActionBase : ControllerBase + { + public abstract IActionResult AbstractMethod(); + + public virtual IActionResult VirtualMethod() => null; + + public virtual IActionResult MethodInBase() => null; + + [NonAction] + public virtual IActionResult NonActionBase() => null; + } + + public class TestIsControllerAction : TestIsControllerActionBase, IDisposable + { + static TestIsControllerAction() { } + + public override IActionResult AbstractMethod() => null; + + private IActionResult PrivateMethod() => null; + + protected IActionResult ProtectedMethod() => null; + + internal IActionResult InternalMethod() => null; + + public IActionResult GenericMethod() => null; + + public static IActionResult StaticMethod() => null; + + [NonAction] + public IActionResult NonAction() => null; + + public override IActionResult NonActionBase() => null; + + public IActionResult Ordinary() => null; + + public void Dispose() { } + } + + public class OverridesObjectMethods : ControllerBase + { + public override bool Equals(object obj) => false; + + public override int GetHashCode() => 0; + + public new string ToString() => null; + } + + public class ExplicitIDisposable : ControllerBase, IDisposable + { + void IDisposable.Dispose() { } + } + + public class NotDisposable + { + public IActionResult Dispose() => null; + } + + public class NotDisposableWithExplicitImplementation : IDisposable + { + public IActionResult Dispose() => null; + + void IDisposable.Dispose() { } + } + + public class NotDisposableWithDisposeThatIsNotInterfaceContract : IDisposable + { + public IActionResult Dispose(int id) => null; + + void IDisposable.Dispose() { } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/TestFiles/MvcFactsTest/IsControllerTests.cs b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/TestFiles/MvcFactsTest/IsControllerTests.cs new file mode 100644 index 0000000000..1837ac712b --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Analyzers.Test/TestFiles/MvcFactsTest/IsControllerTests.cs @@ -0,0 +1,44 @@ +namespace Microsoft.AspNetCore.Mvc.Analyzers +{ + public interface ITestController { } + + public abstract class AbstractController : Controller { } + + public class DerivedAbstractController : AbstractController { } + + public struct ValueTypeController { } + + public class OpenGenericController : Controller { } + + public class PocoType { } + + public class DerivedPocoType : PocoType { } + + public class TypeDerivingFromController : Controller { } + + public class TypeDerivingFromControllerBase : ControllerBase { } + + public abstract class NoControllerAttributeBaseController { } + + public class NoSuffixNoControllerAttribute : NoControllerAttributeBaseController { } + + public class DerivedGenericController : OpenGenericController { } + + public class NoSuffix : Controller { } + + public class PocoController { } + + [Controller] + public class CustomBase { } + + [Controller] + public class ChildOfCustomBase : CustomBase { } + + [NonController] + public class BaseNonController { } + + [Controller] + public class ControllerAttributeDerivingFromNonController : BaseNonController { } + + public class BasePocoNonControllerChildController : BaseNonController { } +}