Skip to content

Commit

Permalink
fix null reference comparing dictionary to value (#438)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jamiras authored Mar 4, 2024
1 parent 6107873 commit 728973e
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 15 deletions.
17 changes: 13 additions & 4 deletions Source/Parser/Expressions/ArrayExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,25 @@ public override bool ReplaceVariables(InterpreterScope scope, out ExpressionBase
}

/// <summary>
/// Determines whether the specified <see cref="DictionaryExpression" /> is equal to this instance.
/// Determines whether the specified <see cref="ArrayExpression" /> is equal to this instance.
/// </summary>
/// <param name="obj">The <see cref="DictionaryExpression" /> to compare with this instance.</param>
/// <param name="obj">The <see cref="ArrayExpression" /> to compare with this instance.</param>
/// <returns>
/// <c>true</c> if the specified <see cref="DictionaryExpression" /> is equal to this instance; otherwise, <c>false</c>.
/// <c>true</c> if the specified <see cref="ArrayExpression" /> is equal to this instance; otherwise, <c>false</c>.
/// </returns>
protected override bool Equals(ExpressionBase obj)
{
var that = obj as ArrayExpression;
return that != null && Entries == that.Entries;
if (that == null || Entries.Count != that.Entries.Count)
return false;

for (int i = 0; i < Entries.Count; i++)
{
if (Entries[i] != that.Entries[i])
return false;
}

return true;
}

IEnumerable<ExpressionBase> INestedExpressions.NestedExpressions
Expand Down
39 changes: 31 additions & 8 deletions Source/Parser/Expressions/ComparisonExpression.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using RATools.Parser.Expressions.Trigger;
using RATools.Parser.Internal;
using System;
using System.Diagnostics;
using System.Text;

Expand Down Expand Up @@ -323,28 +324,50 @@ public static ExpressionBase NormalizeFloatComparisonForInteger(ExpressionBase l
error = null;

var normalizeComparison = left as IComparisonNormalizeExpression;
if (left != null)
if (normalizeComparison != null)
{
var result = normalizeComparison.NormalizeComparison(right, Operation, true);
var boolResult = result as BooleanConstantExpression;
return (boolResult != null) ? boolResult.Value : null;
if (boolResult != null)
return boolResult.Value;

// memory reference (or something similar) that can't be determined at processing time
if (!left.IsLiteralConstant)
return null;
}

if (left == right)
// type doesn't implement IComparisonNormalizeExpression, or comparison didn't collapse to
// a boolean expression. if both sides have the same type, do a strict equality comparison
if (left.Type == right.Type)
{
switch (Operation)
{
case ComparisonOperation.Equal:
case ComparisonOperation.GreaterThanOrEqual:
case ComparisonOperation.LessThanOrEqual:
return true;
return (left == right);

case ComparisonOperation.NotEqual:
return !(left == right);

default:
return false;
error = new ErrorExpression(String.Format("Cannot perform relative comparison on {0}", left.Type.ToLowerString()), this);
return null;
}
}

return null;
// different types are always not equal to each other, even if they could be coerced.
// allow a direct equality/inequality check, but error if a relative comparison is being attemped.
switch (Operation)
{
case ComparisonOperation.Equal:
return false;

case ComparisonOperation.NotEqual:
return true;

default:
error = new ErrorExpression(String.Format("Cannot compare {0} and {1}", left.Type.ToLowerString(), right.Type.ToLowerString()), this);
return null;
}
}

/// <summary>
Expand Down
69 changes: 66 additions & 3 deletions Tests/Parser/Expressions/ComparisonExpressionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ public void TestReplaceVariables(string input, string expected)
[TestCase("\"bbb\" <= \"bbbb\"", true)]
[TestCase("\"bbb\" > \"bbbb\"", false)]
[TestCase("\"bbb\" >= \"bbbb\"", false)]
[TestCase("\"bbb\" == 0", null)]
[TestCase("\"bbb\" == -2.0", null)]
[TestCase("1 == \"bbb\"", null)]
[TestCase("\"bbb\" == 0", false)]
[TestCase("\"bbb\" == -2.0", false)]
[TestCase("1 == \"bbb\"", false)]
[TestCase("2.0 == -2.0", false)]
public void TestIsTrue(string input, bool? expected)
{
Expand All @@ -162,6 +162,69 @@ public void TestIsTrue(string input, bool? expected)
Assert.That(error, Is.Null);
}

[Test]
[TestCase("a == a", true, null)] // 1 == 1
[TestCase("a == b", false, null)] // 1 == 2
[TestCase("a == c", true, null)] // 1 == 1
[TestCase("a == d", false, null)] // 1 == "1"
[TestCase("a == x", false, null)] // 1 == [1]
[TestCase("x == x", true, null)] // [1] == [1]
[TestCase("x == y", true, null)] // [1] == [1]
[TestCase("x == z", false, null)] // [1] == [1,2]
[TestCase("x == a", false, null)] // [1] == 1
[TestCase("x == d", false, null)] // [1] == "1"
[TestCase("a != b", true, null)] // 1 != 2
[TestCase("a != c", false, null)] // 1 != 1
[TestCase("a != d", true, null)] // 1 != "1"
[TestCase("a != x", true, null)] // 1 != [1]
[TestCase("a <= b", true, null)] // 1 <= 2
[TestCase("a <= c", true, null)] // 1 <= 1
[TestCase("x != z", true, null)] // [1] != [1,2]
[TestCase("x < z", null, "Cannot perform relative comparison on array")] // [1] < [1,2]
[TestCase("x > z", null, "Cannot perform relative comparison on array")] // [1] > [1,2]
[TestCase("x <= z", null, "Cannot perform relative comparison on array")] // [1] <= [1,2]
[TestCase("x >= z", null, "Cannot perform relative comparison on array")] // [1] >= [1,2]
[TestCase("a <= d", null, "Cannot compare integer and string")] // 1 <= "1"
[TestCase("a <= x", null, "Cannot compare integer and array")] // 1 <= [1]
[TestCase("a == g", null, "Unknown variable: g")]
[TestCase("g == a", null, "Unknown variable: g")]
public void TestIsTrueVariables(string input, bool? expected, string expectedError)
{
var tokenizer = Tokenizer.CreateTokenizer(input);
var expr = ExpressionBase.Parse(new PositionalTokenizer(tokenizer));

var scope = new InterpreterScope(AchievementScriptInterpreter.GetGlobalScope());
scope.Context = new TriggerBuilderContext();
scope.DefineVariable(new VariableDefinitionExpression("a"), new IntegerConstantExpression(1));
scope.DefineVariable(new VariableDefinitionExpression("b"), new IntegerConstantExpression(2));
scope.DefineVariable(new VariableDefinitionExpression("c"), new IntegerConstantExpression(1));
scope.DefineVariable(new VariableDefinitionExpression("d"), new StringConstantExpression("1"));

var array1 = new ArrayExpression();
array1.Entries.Add(new IntegerConstantExpression(1));
scope.DefineVariable(new VariableDefinitionExpression("x"), array1);
var array2 = new ArrayExpression();
array2.Entries.Add(new IntegerConstantExpression(1));
scope.DefineVariable(new VariableDefinitionExpression("y"), array2);
var array3 = new ArrayExpression();
array3.Entries.Add(new IntegerConstantExpression(1));
array3.Entries.Add(new IntegerConstantExpression(2));
scope.DefineVariable(new VariableDefinitionExpression("z"), array3);

ErrorExpression error;
Assert.That(expr.IsTrue(scope, out error), Is.EqualTo(expected));

if (expectedError != null)
{
Assert.That(error, Is.Not.Null);
Assert.That(error.Message, Is.EqualTo(expectedError));
}
else
{
Assert.That(error, Is.Null);
}
}

[Test]
public void TestCompareFunctionReferences()
{
Expand Down

0 comments on commit 728973e

Please sign in to comment.