more strict check for aspect method signatures

vescon · Jan 31, 2022 · 3608d6b · 3608d6b
1 parent 5528fe3
commit 3608d6b
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 13 deletions.
diff --git a/src/MethodBoundaryAspect.Fody.UnitTests.NetFramework/DuplicateEntryMethodsAspectTests.cs b/src/MethodBoundaryAspect.Fody.UnitTests.NetFramework/DuplicateEntryMethodsAspectTests.cs
@@ -0,0 +1,40 @@
+using System;
+using FluentAssertions;
+using MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework;
+using Xunit;
+
+namespace MethodBoundaryAspect.Fody.UnitTests.NetFramework
+{
+    public class DuplicateEntryMethodsAspectTests : MethodBoundaryAspectNetFrameworkTestBase
+    {
+        private static readonly Type TestMethodsType = typeof(DuplicateEntryMethodsAspectMethods);
+
+        [Fact]
+        public void IfStaticMethodIsCalled_ThenTheOnMethodBoundaryAspectShouldBeCalled()
+        {
+            // Arrange
+            const string testMethodName = "StaticMethodCall";
+            WeaveAssemblyMethodAndLoad(TestMethodsType, testMethodName);
+
+            // Act
+            var result = AssemblyLoader.InvokeMethod(TestMethodsType.TypeInfo(), testMethodName);
+
+            // Assert
+            result.Should().Be("-OnEntry()-OnExit()");
+        }
+
+        [Fact]
+        public void IfInstanceMethodIsCalled_ThenTheOnMethodBoundaryAspectShouldBeCalled()
+        {
+            // Arrange
+            const string testMethodName = "InstanceMethodCall";
+            WeaveAssemblyMethodAndLoad(TestMethodsType, testMethodName);
+
+            // Act
+            var result = AssemblyLoader.InvokeMethod(TestMethodsType.TypeInfo(), testMethodName);
+
+            // Assert
+            result.Should().Be("-OnEntry()-OnExit()");
+        }
+    }
+}
diff --git a/...aryAspect.Fody.UnitTests.TestAssembly.NetFramework/Aspects/DuplicateEntryMethodsAspect.cs b/...aryAspect.Fody.UnitTests.TestAssembly.NetFramework/Aspects/DuplicateEntryMethodsAspect.cs
@@ -0,0 +1,55 @@
+using MethodBoundaryAspect.Fody.Attributes;
+
+namespace MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework.Aspects
+{
+    /// <summary>
+    /// remark: weaved methods with parameter MethodExecutionArgs should be ordered last
+    /// </summary>
+    public class DuplicateEntryMethodsAspect : OnMethodBoundaryAspect
+    {
+        public virtual void OnEntry(MethodExecutionArgs arg, bool b)
+        {
+            DuplicateEntryMethodsAspectMethods.Result += "-OnEntry(bool)";
+        }
+
+        public override void OnEntry(MethodExecutionArgs arg)
+        {
+            DuplicateEntryMethodsAspectMethods.Result += "-OnEntry()";
+        }
+
+        public virtual void OnEntry<T>(MethodExecutionArgs arg)
+        {
+            DuplicateEntryMethodsAspectMethods.Result += "-OnEntry<T>";
+        }
+
+        public virtual void OnExit(MethodExecutionArgs arg, bool b)
+        {
+            DuplicateEntryMethodsAspectMethods.Result += "-OnExit(bool)";
+        }
+
+        public virtual void OnExit<T>(MethodExecutionArgs arg)
+        {
+            DuplicateEntryMethodsAspectMethods.Result += "-OnExit<T>";
+        }
+
+        public override void OnExit(MethodExecutionArgs arg)
+        {
+            DuplicateEntryMethodsAspectMethods.Result += "-OnExit()";
+        }
+
+        public virtual void OnException(MethodExecutionArgs arg, bool b)
+        {
+            DuplicateEntryMethodsAspectMethods.Result += "-OnException(bool)";
+        }
+
+        public virtual void OnException<T>(MethodExecutionArgs arg)
+        {
+            DuplicateEntryMethodsAspectMethods.Result += "-OnException<T>";
+        }
+
+        public override void OnException(MethodExecutionArgs arg)
+        {
+            DuplicateEntryMethodsAspectMethods.Result += "-OnException()";
+        }
+    }
+}
diff --git a/...daryAspect.Fody.UnitTests.TestAssembly.NetFramework/DuplicateEntryMethodsAspectMethods.cs b/...daryAspect.Fody.UnitTests.TestAssembly.NetFramework/DuplicateEntryMethodsAspectMethods.cs
@@ -0,0 +1,19 @@
+using MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework.Aspects;
+
+namespace MethodBoundaryAspect.Fody.UnitTests.TestAssembly.NetFramework
+{
+    public class DuplicateEntryMethodsAspectMethods
+    {
+        public static object Result { get; set; }
+
+        [DuplicateEntryMethodsAspect]
+        public static void StaticMethodCall()
+        {
+        }
+
+        [DuplicateEntryMethodsAspect]
+        public void InstanceMethodCall()
+        {
+        }
+    }
+}
diff --git a/src/MethodBoundaryAspect.Fody/AspectDataOnAsyncMethod.cs b/src/MethodBoundaryAspect.Fody/AspectDataOnAsyncMethod.cs
@@ -83,7 +83,7 @@ public InstructionBlockChain LoadTagInMoveNext(IPersistable executionArgs)
         public InstructionBlockChain CallOnExceptionInMoveNext(IPersistable executionArgs, VariableDefinition exceptionLocal)
         {
             var onExceptionMethodRef = _referenceFinder.GetMethodReference(Info.AspectAttribute.AttributeType,
-                md => md.Name == "OnException");
+                AspectMethodCriteria.IsOnExceptionMethod);
 
             var setMethod = _referenceFinder.GetMethodReference(executionArgs.PersistedType,
                 md => md.Name == "set_Exception");

diff --git a/src/MethodBoundaryAspect.Fody/AspectMethodCriteria.cs b/src/MethodBoundaryAspect.Fody/AspectMethodCriteria.cs
@@ -0,0 +1,36 @@
+using System.Linq;
+using Mono.Cecil;
+
+namespace MethodBoundaryAspect.Fody
+{
+    public static class AspectMethodCriteria
+    {
+        public static readonly string OnEntryMethodName = "OnEntry";
+        public static readonly string OnExitMethodName = "OnExit";
+        public static readonly string OnExceptionMethodName = "OnException";
+
+        public static bool MatchesSignature(MethodDefinition method)
+        {
+            return method.IsVirtual
+                   && !method.HasGenericParameters
+                   && method.Parameters.Count == 1
+                   && method.Parameters.Single().ParameterType.FullName ==
+                   "MethodBoundaryAspect.Fody.Attributes.MethodExecutionArgs";
+        }
+
+        public static bool IsOnEntryMethod(MethodDefinition method)
+        {
+            return method.Name == OnEntryMethodName && MatchesSignature(method);
+        }
+
+        public static bool IsOnExitMethod(MethodDefinition method)
+        {
+            return method.Name == OnExitMethodName && MatchesSignature(method);
+        }
+
+        public static bool IsOnExceptionMethod(MethodDefinition method)
+        {
+            return method.Name == OnExceptionMethodName && MatchesSignature(method);
+        }
+    }
+}
diff --git a/src/MethodBoundaryAspect.Fody/InstructionBlockChainCreator.cs b/src/MethodBoundaryAspect.Fody/InstructionBlockChainCreator.cs
@@ -114,7 +114,7 @@ public NamedInstructionBlockChain CreateMethodExecutionArgsInstance(
 
             // MethodExecutionArgs instance
             var onEntryMethodTypeRef =
-                anyAspectTypeDefinition.Resolve().BaseType.Resolve().Methods.Single(x => x.Name == "OnEntry");
+                anyAspectTypeDefinition.Resolve().BaseType.Resolve().Methods.Single(AspectMethodCriteria.IsOnEntryMethod);
             var firstParameterType = onEntryMethodTypeRef.Parameters.Single().ParameterType;
             var methodExecutionArgsTypeRef = _moduleDefinition.ImportReference(firstParameterType);
 
@@ -272,11 +272,13 @@ public NamedInstructionBlockChain SaveThrownException()
             return block;
         }
 
-        public InstructionBlockChain CallAspectOnEntry(AspectData aspectInstance,
+        public InstructionBlockChain CallAspectOnEntry(
+            AspectData aspectInstance,
             IPersistable executionArgs)
         {
-            var onEntryMethodRef = _referenceFinder.GetMethodReference(aspectInstance.Info.AspectAttribute.AttributeType,
-                md => md.Name == "OnEntry");
+            var onEntryMethodRef = _referenceFinder.GetMethodReference(
+                aspectInstance.Info.AspectAttribute.AttributeType,
+                AspectMethodCriteria.IsOnEntryMethod);
             var callOnEntryBlock = _creator.CallVoidInstanceMethod(onEntryMethodRef,
                 aspectInstance.AspectPersistable, executionArgs);
 
@@ -288,8 +290,9 @@ public InstructionBlockChain CallAspectOnEntry(AspectData aspectInstance,
         public InstructionBlockChain CallAspectOnExit(AspectData aspectData,
             IPersistable executionArgs)
         {
-            var onExitMethodRef = _referenceFinder.GetMethodReference(aspectData.Info.AspectAttribute.AttributeType,
-                md => md.Name == "OnExit");
+            var onExitMethodRef = _referenceFinder.GetMethodReference(
+                aspectData.Info.AspectAttribute.AttributeType,
+                AspectMethodCriteria.IsOnExitMethod);
             var callOnExitBlock = _creator.CallVoidInstanceMethod(onExitMethodRef,
                 aspectData.AspectPersistable, executionArgs);
 
@@ -302,8 +305,9 @@ public InstructionBlockChain CallAspectOnException(
             AspectData aspectData,
             IPersistable executionArgs)
         {
-            var onExceptionMethodRef = _referenceFinder.GetMethodReference(aspectData.Info.AspectAttribute.AttributeType,
-                md => md.Name == "OnException");
+            var onExceptionMethodRef = _referenceFinder.GetMethodReference(
+                aspectData.Info.AspectAttribute.AttributeType,
+                AspectMethodCriteria.IsOnExceptionMethod);
             var callOnExceptionBlock = _creator.CallVoidInstanceMethod(onExceptionMethodRef,
                 aspectData.AspectPersistable, executionArgs);
 

diff --git a/src/MethodBoundaryAspect.Fody/MethodWeaverFactory.cs b/src/MethodBoundaryAspect.Fody/MethodWeaverFactory.cs
@@ -44,7 +44,7 @@ static AspectMethods GetUsedAspectMethods(TypeReference aspectTypeDefinition)
             {
                 var typeDefinition = currentType.Resolve();
                 var methods = typeDefinition.Methods
-                    .Where(x => x.IsVirtual)
+                    .Where(AspectMethodCriteria.MatchesSignature)
                     .ToList();
                 foreach (var method in methods)
                 {
@@ -58,11 +58,11 @@ static AspectMethods GetUsedAspectMethods(TypeReference aspectTypeDefinition)
             } while (currentType.FullName != AttributeFullNames.OnMethodBoundaryAspect);
 
             var aspectMethods = AspectMethods.None;
-            if (overloadedMethods.ContainsKey("OnEntry"))
+            if (overloadedMethods.ContainsKey(AspectMethodCriteria.OnEntryMethodName))
                 aspectMethods |= AspectMethods.OnEntry;
-            if (overloadedMethods.ContainsKey("OnExit"))
+            if (overloadedMethods.ContainsKey(AspectMethodCriteria.OnExitMethodName))
                 aspectMethods |= AspectMethods.OnExit;
-            if (overloadedMethods.ContainsKey("OnException"))
+            if (overloadedMethods.ContainsKey(AspectMethodCriteria.OnExceptionMethodName))
                 aspectMethods |= AspectMethods.OnException;
             return aspectMethods;
         }