From 5986c5d39d6f05b2c40882df06ead38d2912a294 Mon Sep 17 00:00:00 2001
From: stakx <stakx@eml.cc>
Date: Fri, 17 Apr 2020 17:59:22 +0200
Subject: [PATCH] Make `conditionalSetup.Verifiable()` an error

---
 CHANGELOG.md                             |  4 ++++
 src/Moq/Properties/Resources.Designer.cs |  9 +++++++++
 src/Moq/Properties/Resources.resx        |  3 +++
 src/Moq/Setup.cs                         |  7 +++++++
 tests/Moq.Tests/VerifyFixture.cs         | 15 +++++++--------
 5 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 20dea276e..2db4d9cc4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,10 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
  * Two new public methods in `Times`: `bool Validate(int count)` and `string ToString()` (@stakx, 975)
 
+#### Changed
+
+ * Attempts to mark conditionals setup as verifiable are now considered an error, since conditional setups are ignored during verification. Calls to `.Verifiable()` on conditional setups are no-ops and can be safely removed. (@stakx, #997)
+
 #### Fixed
 
  * Regression: Restored `Capture.In` use in `mock.Verify(expression, ...)` to extract arguments of previously recorded invocations. (@vgriph, #968; @stakx, #974)
diff --git a/src/Moq/Properties/Resources.Designer.cs b/src/Moq/Properties/Resources.Designer.cs
index 4c3e18794..c6f840e81 100644
--- a/src/Moq/Properties/Resources.Designer.cs
+++ b/src/Moq/Properties/Resources.Designer.cs
@@ -114,6 +114,15 @@ internal static string CantSetReturnValueForVoid {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to This call to &apos;Verifiable&apos; will have no effect because conditional setups are ignored by both &apos;Verify&apos; and &apos;VerifyAll&apos;. This might indicate an error in your code..
+        /// </summary>
+        internal static string ConditionalSetupsAreNotVerifiable {
+            get {
+                return ResourceManager.GetString("ConditionalSetupsAreNotVerifiable", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Constructor arguments cannot be passed for delegate mocks..
         /// </summary>
diff --git a/src/Moq/Properties/Resources.resx b/src/Moq/Properties/Resources.resx
index 75a2c37c0..b84d019ea 100644
--- a/src/Moq/Properties/Resources.resx
+++ b/src/Moq/Properties/Resources.resx
@@ -351,4 +351,7 @@ This could be caused by an unrecognized type conversion, coercion, narrowing, or
   <data name="UseItIsOtherOverload" xml:space="preserve">
     <value>It is impossible to call the provided strongly-typed predicate due to the use of a type matcher. Provide a weakly-typed predicate with two parameters (object, Type) instead.</value>
   </data>
+  <data name="ConditionalSetupsAreNotVerifiable" xml:space="preserve">
+    <value>This call to 'Verifiable' will have no effect because conditional setups are ignored by both 'Verify' and 'VerifyAll'. This might indicate an error in your code.</value>
+  </data>
 </root>
\ No newline at end of file
diff --git a/src/Moq/Setup.cs b/src/Moq/Setup.cs
index 8ef963e51..f7d7da0a4 100644
--- a/src/Moq/Setup.cs
+++ b/src/Moq/Setup.cs
@@ -7,6 +7,8 @@
 using System.Reflection;
 using System.Text;
 
+using Moq.Properties;
+
 namespace Moq
 {
 	internal abstract class Setup : ISetup
@@ -86,6 +88,11 @@ public void MarkAsOverridden()
 
 		public void MarkAsVerifiable()
 		{
+			if (this.IsConditional)
+			{
+				throw new InvalidOperationException(Resources.ConditionalSetupsAreNotVerifiable);
+			}
+
 			this.flags |= Flags.Verifiable;
 		}
 
diff --git a/tests/Moq.Tests/VerifyFixture.cs b/tests/Moq.Tests/VerifyFixture.cs
index 5b691596a..ef6f9f9d4 100644
--- a/tests/Moq.Tests/VerifyFixture.cs
+++ b/tests/Moq.Tests/VerifyFixture.cs
@@ -1047,17 +1047,16 @@ public void CanVerifyMethodThatIsNamedLikeEventRemoveAccessor()
 		}
 
 		[Fact]
-		public void Verify_ignores_conditional_setups()
+		public void Verify_ignores_conditional_setups_so_calling_Verifiable_is_a_user_error()
 		{
 			var mock = new Mock<IFoo>();
-			mock.When(() => true).Setup(m => m.Submit()).Verifiable();
+			var setup = mock.When(() => true).Setup(m => m.Submit());
 
-			var exception = Record.Exception(() =>
-			{
-				mock.Verify();
-			});
-
-			Assert.Null(exception);
+			// Conditional setups are completely ignored by `Verify[All]`.
+			// Making such a setup verifiable is therefore a no-op, but
+			// may be indicative of the user making an incorrect assumption;
+			// best to warn the user so they revise their code:
+			Assert.Throws<InvalidOperationException>(() => setup.Verifiable());
 		}
 
 		[Fact]