better error when embedding comparisons in value measured (#398)

Jamiras · Jun 8, 2023 · b5d0572 · b5d0572
1 parent 6fd08ab
commit b5d0572
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 5 deletions.
diff --git a/Source/Parser/Expressions/Trigger/MeasuredRequirementExpression.cs b/Source/Parser/Expressions/Trigger/MeasuredRequirementExpression.cs
@@ -95,8 +95,14 @@ public override RequirementExpressionBase Optimize(TriggerBuilderContext context
 
         public override ErrorExpression BuildTrigger(TriggerBuilderContext context)
         {
-            if (Format == RequirementType.MeasuredPercent && context is ValueBuilderContext)
-                return new ErrorExpression("Value fields only support raw measured values", this);
+            if (context is ValueBuilderContext)
+            {
+                if (Format == RequirementType.MeasuredPercent)
+                    return new ErrorExpression("Value fields only support raw measured values", this);
+
+                if (Condition is RequirementConditionExpression || Condition is RequirementClauseExpression)
+                    return new ErrorExpression("Comparison must be wrapped in tally(0,...) to be used in measured value expression", this);
+            }
 
             Debug.Assert(Condition != null);
             var clause = Condition as RequirementClauseExpression;

diff --git a/Tests/Parser/Expressions/Trigger/TriggerExpressionTests.cs b/Tests/Parser/Expressions/Trigger/TriggerExpressionTests.cs
@@ -85,6 +85,17 @@ public static void AssertBuildTriggerError(string input, string message)
             ExpressionTests.AssertError(error, message);
         }
 
+        public static void AssertBuildValueError(string input, string message)
+        {
+            var clause = Parse<RequirementExpressionBase>(input);
+
+            var requirements = new List<Requirement>();
+            var context = new ValueBuilderContext() { Trigger = requirements };
+
+            var error = clause.BuildTrigger(context);
+            ExpressionTests.AssertError(error, message);
+        }
+
         public static string Serialize(ITriggerExpression expression)
         {
             var requirements = new List<Requirement>();

diff --git a/Tests/Parser/Functions/MeasuredFunctionTests.cs b/Tests/Parser/Functions/MeasuredFunctionTests.cs
@@ -73,6 +73,7 @@ public void TestBuildTrigger(string input, string expected)
         [TestCase("measured(byte(word(0x1234) + 16))", "I:0x 001234_M:0xH000010")]
         [TestCase("measured(repeated(0, byte(0x1234) == 5))", "M:0xH001234=5")]
         [TestCase("measured(tally(0, byte(0x1234) == 20, byte(0x1234) == 67))", "C:0xH001234=20_M:0xH001234=67")]
+        [TestCase("measured(tally(0, byte(0x1234) == 20 && byte(0x2345) == 67))", "N:0xH001234=20_M:0xH002345=67")]
         public void TestBuildValue(string input, string expected)
         {
             var clause = TriggerExpressionTests.Parse<MeasuredRequirementExpression>(input);
@@ -110,5 +111,22 @@ public void TestTallyZero()
                 "measured(tally(0, byte(0x1234) == 20, byte(0x1234) == 67))",
                 "Unbounded count is only supported in measured value expressions");
         }
+
+        [Test]
+        public void TestValueComparison()
+        {
+            TriggerExpressionTests.AssertBuildValueError(
+                "measured(byte(0x1234) == 20)",
+                "Comparison must be wrapped in tally(0,...) to be used in measured value expression");
+        }
+
+
+        [Test]
+        public void TestValueComparisonAndNext()
+        {
+            TriggerExpressionTests.AssertBuildValueError(
+                "measured(byte(0x1234) == 20 && byte(0x2345) == 67)",
+                "Comparison must be wrapped in tally(0,...) to be used in measured value expression");
+        }
     }
 }
diff --git a/Tests/Parser/ValueBuilderContextTests.cs b/Tests/Parser/ValueBuilderContextTests.cs
@@ -54,10 +54,10 @@ class ValueBuilderContextTests
         [TestCase("byte(0x1234 + byte(0x2345)) - byte(0x1235 + byte(0x2345))", "I:0xH002345_B:0xH001235_I:0xH002345_M:0xH001234")]
         [TestCase("byte(0x1234 + byte(0x2345)) * 2", "I:0xH002345_M:0xH001234*2")]
         [TestCase("byte(0x1234 + byte(0x2345)) / 2", "I:0xH002345_M:0xH001234/2")]
-        [TestCase("measured(dword(0x1234) == 0xFFFFFFFF)", "M:0xX001234=4294967295")]
+        [TestCase("measured(tally(0, dword(0x1234) == 0xFFFFFFFF))", "M:0xX001234=4294967295")]
         [TestCase("measured(byte(0x1234 + byte(0x2345)) / 2)", "I:0xH002345_M:0xH001234/2")]
-        [TestCase("measured(byte(0x1234) != prev(byte(0x1234)))", "M:0xH001234!=d0xH001234")]
-        [TestCase("measured(byte(0x1234) != prev(byte(0x1234))) && never(byte(0x2345) == 1)", "M:0xH001234!=d0xH001234_R:0xH002345=1")]
+        [TestCase("measured(tally(0, byte(0x1234) != prev(byte(0x1234))))", "M:0xH001234!=d0xH001234")]
+        [TestCase("measured(tally(0, byte(0x1234) != prev(byte(0x1234)))) && never(byte(0x2345) == 1)", "M:0xH001234!=d0xH001234_R:0xH002345=1")]
         [TestCase("tally(20, byte(0x1234) != prev(byte(0x1234))) && never(byte(0x2345) == 1)", "M:0xH001234!=d0xH001234.20._R:0xH002345=1")]
         [TestCase("byte(byte(0x1234) - 10)", "I:0xH001234_M:0xHfffffff6")]
         [TestCase("measured(repeated(10, byte(0x2345 + word(0x1234) * 4) == 6)))", "I:0x 001234*4_M:0xH002345=6.10.")]