Adds warning on comparing same FirstNameNode

src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs

@@ -3862,6 +3862,12 @@ public override void PostVisit(BinaryOpNode node)
                 var leftType = _txb.GetType(node.Left);
                 var rightType = _txb.GetType(node.Right);
 
+                // Helps warn no op comparison, e.g. Filter(table, ThisRecord.Value = Value)) or Filter(table, Value = Value)
+                if (IsNoOPFirstNameComparison(node))
+                {
+                    _txb.ErrorContainer.EnsureError(DocumentErrorSeverity.Severe, node, TexlStrings.WrnNoOpFieldComparison);
+                }
+
                 var res = CheckBinaryOpCore(_txb.ErrorContainer, node, _txb.Features, leftType, rightType, _txb.BindingConfig.NumberIsFloat);
 
                 foreach (var coercion in res.Coercions)
@@ -3882,6 +3888,48 @@ public override void PostVisit(BinaryOpNode node)
                 _txb.SetIsUnliftable(node, _txb.IsUnliftable(node.Left) || _txb.IsUnliftable(node.Right));
             }
 
+            private static bool IsNoOPFirstNameComparison(BinaryOpNode node)
+            {
+                if ((node.Op == BinaryOp.Equal || 
+                    node.Op == BinaryOp.NotEqual || 
+                    node.Op == BinaryOp.Less || 
+                    node.Op == BinaryOp.LessEqual || 
+                    node.Op == BinaryOp.Greater || 
+                    node.Op == BinaryOp.GreaterEqual) && 
+                    IsNoOPFirstNameComparison(node.Left, node.Right))
+                {
+                    return true;
+                }
+
+                return false;
+            }
+
+            private static bool IsNoOPFirstNameComparison(TexlNode left, TexlNode right)
+            {
+                if (left is FirstNameNode lfn1 && right is FirstNameNode rfn1)
+                {
+                    // This means a field comparison e.g. Value = Value
+                    return lfn1.Ident.Name == rfn1.Ident.Name;
+                }
+                else if (left is FirstNameNode leftFirstName && right is DottedNameNode rightDn && rightDn.Left is FirstNameNode rightfn)
+                {
+                    // This means a field comparison with ThisRecord, where field is on left hand side. e.g. Value = ThisRecord.Value
+                    return leftFirstName.Ident.Name == rightDn.Right.Name && rightfn.Ident.Name == ThisRecordDefaultName;
+                }
+                else if (right is FirstNameNode rightFirstName && left is DottedNameNode leftDn && leftDn.Left is FirstNameNode leftfn)
+                {
+                    // This means a field comparison with ThisRecord, where field is on right hand side. e.g. ThisRecord.Value = Value
+                    return rightFirstName.Ident.Name == leftDn.Right.Name && leftfn.Ident.Name == ThisRecordDefaultName;
+                }
+                else if (left is DottedNameNode leftDottedName && right is DottedNameNode rightDottedName)
+                {
+                    // This means field is a part of dotted name node and we need to compare the field names and recursively check for the left and right nodes.
+                    return leftDottedName.Right.Name == rightDottedName.Right.Name && IsNoOPFirstNameComparison(leftDottedName.Left, rightDottedName.Left);
+                }
+
+                return false;
+            }
+
             public override void PostVisit(AsNode node)
             {
                 Contracts.AssertValue(node);

src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs

@@ -802,5 +802,6 @@ internal static class TexlStrings
         public static ErrorResourceKey ErrUnknownPartialOp = new ErrorResourceKey("ErrUnknownPartialOp");
 
         public static ErrorResourceKey ErrTruncatedArgWarning = new ErrorResourceKey("ErrTruncatedArgWarning");
+        public static ErrorResourceKey WrnNoOpFieldComparison = new ErrorResourceKey("WrnNoOpFieldComparison");
     }
 }

src/strings/PowerFxResources.en-US.resx

@@ -4575,4 +4575,8 @@
     <value>Delegation warning. The result of this argument '{0}' may be truncated for large data sets before being passed to the '{1}' function.</value>
     <comment>Error message when an argument to non-delegable function has possible delegation and resulting rows may be truncated</comment>
   </data>
+  <data name="WrnNoOpFieldComparison" xml:space="preserve">
+    <value>This comparison will always be constant. (Always yields true or false). Please consider using 'As' keyword for aliasing scoped variable if this not what you want.</value>
+    <comment>Warning when comparing same first name node.</comment>
+  </data>
 </root>

src/tests/Microsoft.PowerFx.Core.Tests/ExpressionTestCases/Blob.txt

@@ -52,7 +52,7 @@ false
 >> Blob("Abc") <> Blob("Abc")
 true
 
->> With({ file: Blob("Abc") }, file = file)
+>> With({ file: Blob("Abc") }, file <> Blob("Abc"))
 true
 
 >> IsBlank(Blob(""))

src/tests/Microsoft.PowerFx.Core.Tests/ExpressionTestCases/ConcatenateT.txt

@@ -6,5 +6,5 @@ Table({Value:"JaneDoe"},{Value:"JaneSmith"},{Value:"JaneWilliams"})
 >> Concatenate("777", ["abc"], Table({a: "1"}, {a: "2"}))
 Table({Value:"777abc1"},{Value:Error({Kind:ErrorKind.NotApplicable})})
 
->> Concatenate("777", Filter(["empty table"], Value <> Value), ["1", "2"])
+>> Concatenate("777", Filter(["empty table"], Value <> "empty table"), ["1", "2"])
 Table({Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})})

src/tests/Microsoft.PowerFx.Core.Tests/ExpressionTestCases/ErrorKinds.txt

@@ -200,7 +200,7 @@ Error({Kind:ErrorKind.Div0})
 >> ForAll(Exp([0, 1000000000, 0, 1/0, 0, If(Char(0) = "a",-1)]), IfError(Value, -FirstError.Kind))
 Table({Value:1},{Value:-24},{Value:1},{Value:-13},{Value:1},{Value:-25})
 
->> Filter(If(1/0<2,[1,2,3]), Value = Value)
+>> Filter(If(1/0<2,[1,2,3]), Value = 1)
 Error({Kind:ErrorKind.Div0})
 
 >> Filter([2,-1,0,1,-2], Sqrt(Value) > 0)

src/tests/Microsoft.PowerFx.Core.Tests/ExpressionTestCases/FilterFunctions.txt

@@ -66,3 +66,12 @@ Table(Blank())
 
 >> Filter(Table({a:1},{a:2},{a:4},{a:5}), IsBlank(ThisRecord))
 Table()
+
+>> Filter(Table({F1: {N1F1:  {N2F1 : 1} } } ), ThisRecord.F1.N1F1.N2F1 = F1.N1F1.N2F1)
+Errors: Error 68-69: This comparison will always be constant. (Always yields true or false). Please consider using 'As' keyword for aliasing scoped variable if this not what you want.
+
+>> Filter(Table({F1: {N1F1:  {N2F1 : 1} } } ), F1.N1F1.N2F1 = ThisRecord.F1.N1F1.N2F1)
+Errors: Error 57-58: This comparison will always be constant. (Always yields true or false). Please consider using 'As' keyword for aliasing scoped variable if this not what you want.
+
+>> Filter(Table({F1: {N1F1:  {N2F1 : 1} } } ), F1.N1F1.N2F1 = F1.N1F1.N2F1)
+Errors: Error 57-58: This comparison will always be constant. (Always yields true or false). Please consider using 'As' keyword for aliasing scoped variable if this not what you want.

src/tests/Microsoft.PowerFx.Core.Tests/ExpressionTestCases/FindT.txt

@@ -26,7 +26,7 @@ Table({Value:2},{Value:Error({Kind:ErrorKind.NotApplicable})})
 >> Find(["b"], ["abc", "cde"], If(false, [0], Blank()))
 Blank()
 
->> Find(["b"], ["abc", "cde"], Filter([0], Value <> Value))
+>> Find(["b"], ["abc", "cde"], Filter([0], Value <> 0))
 Table({Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})})
 
 >> Find([Blank(), "", ","], If(false, ["blank table"], Blank()), [1, 2, 3])
@@ -89,37 +89,37 @@ Table({Value:9},{Value:2},{Value:Error({Kind:ErrorKind.NotApplicable})})
 >> Find(",", [Blank(), "lastName,firstName", "lastName,firstName"], [1, 2])
 Table({Value:Blank()},{Value:9},{Value:Error({Kind:ErrorKind.NotApplicable})})
 
->> Find(Filter(["empty table"], Value <> Value), "", 1)
+>> Find(Filter(["empty table"], Value <> "empty table"), "", 1)
 Table()
 
->> Find("", Filter(["empty table"], Value <> Value), 1)
+>> Find("", Filter(["empty table"], Value <> "empty table"), 1)
 Table()
 
->> Find("textToFind", Filter(["empty table"], Value <> Value), 2)
+>> Find("textToFind", Filter(["empty table"], Value <> "empty table"), 2)
 Table()
 
->> Find(Filter(["empty table"], Value <> Value), Filter(["empty table"], Value <> Value))
+>> Find(Filter(["empty table"], Value <> "empty table"), Filter(["empty table"], Value <> "empty table"))
 Table()
 
->> Find(Filter(["empty table"], Value <> Value), Filter(["empty table"], Value <> Value), Filter(["empty table"], Value <> Value))
+>> Find(Filter(["empty table"], Value <> "empty table"), Filter(["empty table"], Value <> "empty table"), Filter(["empty table"], Value <> "empty table"))
 Table()
 
->> Find(Filter(["empty table"], Value <> Value), [Blank(), "lastName,firstName", "lastName,firstName"], 2)
+>> Find(Filter(["empty table"], Value <> "empty table"), [Blank(), "lastName,firstName", "lastName,firstName"], 2)
 Table({Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})})
 
->> Find(Filter(["empty table"], Value <> Value), "textToSearch", [1, 2])
+>> Find(Filter(["empty table"], Value <> "empty table"), "textToSearch", [1, 2])
 Table({Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})})
 
->> Find([",", Blank(), ""], Filter(["empty table"], Value <> Value), 2)
+>> Find([",", Blank(), ""], Filter(["empty table"], Value <> "empty table"), 2)
 Table({Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})})
 
->> Find(",", Filter(["empty table"], Value <> Value), [1, 1])
+>> Find(",", Filter(["empty table"], Value <> "empty table"), [1, 1])
 Table({Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})})
 
->> Find([Blank(), ",", "", "findMe"], [Blank(), "lastName,firstName", "lastName,firstName"], Filter(["empty table"], Value <> Value))
+>> Find([Blank(), ",", "", "findMe"], [Blank(), "lastName,firstName", "lastName,firstName"], Filter(["empty table"], Value <> "empty table"))
 Table({Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})})
 
->> Find(",", [Blank(), "lastName,firstName"], Filter(["empty table"], Value <> Value))
+>> Find(",", [Blank(), "lastName,firstName"], Filter(["empty table"], Value <> "empty table"))
 Table({Value:Error({Kind:ErrorKind.NotApplicable})},{Value:Error({Kind:ErrorKind.NotApplicable})})
 
 >> Find("c", Table({ within: "abc" }, { within: Error({Kind: ErrorKind.Validation}) }, { within: "cde" }))

src/tests/Microsoft.PowerFx.Core.Tests/ExpressionTestCases/OptionSet.txt

@@ -35,8 +35,8 @@ OptionSet.'option-3'
 >> OptionSet.Option1 <> OptionSet.Option2
 true
 
->> OptionSet.Option2 = OptionSet.Option2
-true
+>> OptionSet.Option2 = OptionSet.Option1
+false
 
 >> "Coerces to " & OptionSet.Option2
 "Coerces to Option2"

src/tests/Microsoft.PowerFx.Core.Tests/ExpressionTestCases/TabularOverloadsBlanksAndErrors.txt

@@ -187,7 +187,7 @@ Error({Kind:ErrorKind.Div0})
 >> Concatenate( If(Sqrt(-1)<0,["Hi", "hello"]), If(1/0<2, [" world", "people"]) )
 Error({Kind:ErrorKind.Numeric})
 
->> Concatenate(Filter([1,2], Value<>Value), If(1/0<0, "test"))
+>> Concatenate(Filter([1,2], Value = 3), If(1/0<0, "test"))
 Table()
 
 >> Concatenate(["hello", "hi"], If(1/0<2," world"))

src/tests/Microsoft.PowerFx.Performance.Tests/PvaPerformance.cs

@@ -128,7 +128,13 @@ private static string GetRandomExpression()
             {
                 int j = rnd.Next(0, 10000); // [0 - 9999]
                 int k = rnd.Next(0, 10);    // [0 -9]
-                int l = rnd.Next(0, 10);    // [0 -9]
+
+                int l;
+                do
+                {
+                    l = rnd.Next(0, 10);    // [0 - 9] not same as k.
+                } 
+                while (l == k);
 
                 expr.Append($"(OptionSet{j:0000}.Logical{k} ");
                 expr.Append(rnd.Next() > (1 << 30) ? "=" : "<>");