WIP: 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.Warning, 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/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)
+Table({F1:{N1F1:{N2F1:1}}})
+
+>> Filter(Table({F1: {N1F1:  {N2F1 : 1} } } ), F1.N1F1.N2F1 = ThisRecord.F1.N1F1.N2F1)
+Table({F1:{N1F1:{N2F1:1}}})
+
+>> Filter(Table({F1: {N1F1:  {N2F1 : 1} } } ), F1.N1F1.N2F1 = F1.N1F1.N2F1)
+Table({F1:{N1F1:{N2F1:1}}})

src/tests/Microsoft.PowerFx.Core.Tests/TexlTests.cs

@@ -2221,12 +2221,12 @@ public void TestTexlFunctionGetSignaturesApi()
         [Fact]
         public void TestWarningsOnEqualWithIncompatibleTypes()
         {
-            TestBindingWarning("1 = \"hello\"", DType.Boolean, expectedErrorCount: 1);
-            TestBindingWarning("1 <> \"hello\"", DType.Boolean, expectedErrorCount: 1);
-            TestBindingWarning("true = 123", DType.Boolean, expectedErrorCount: 1);
-            TestBindingWarning("true <> 123", DType.Boolean, expectedErrorCount: 1);
-            TestBindingWarning("false = \"false\"", DType.Boolean, expectedErrorCount: 1);
-            TestBindingWarning("false <> \"false\"", DType.Boolean, expectedErrorCount: 1);
+            TestBindingWarning("1 = \"hello\"", expectedErrorCount: 1, expectedType: DType.Boolean);
+            TestBindingWarning("1 <> \"hello\"", expectedErrorCount: 1, expectedType: DType.Boolean);
+            TestBindingWarning("true = 123", expectedErrorCount: 1, expectedType: DType.Boolean);
+            TestBindingWarning("true <> 123", expectedErrorCount: 1, expectedType: DType.Boolean);
+            TestBindingWarning("false = \"false\"", expectedErrorCount: 1, expectedType: DType.Boolean);
+            TestBindingWarning("false <> \"false\"", expectedErrorCount: 1, expectedType: DType.Boolean);
         }
 
         [Fact]
@@ -3024,8 +3024,8 @@ public void TestWarningOnLiteralPredicate(string script, string expectedType)
             symbol.AddVariable("TW", new TableType(TestUtils.DT("*[Item:w]")));
             TestBindingWarning(
                 script,
-                TestUtils.DT(expectedType),
                 expectedErrorCount: null,
+                expectedType: TestUtils.DT(expectedType),
                 symbolTable: symbol);
         }
 
@@ -3043,8 +3043,8 @@ public void TestWarningOnLiteralPredicate_NumberIsFloat(string script, string ex
             symbol.AddVariable("TW", new TableType(TestUtils.DT("*[Item:w]")));
             TestBindingWarning(
                 script,
-                TestUtils.DT(expectedType),
-                expectedErrorCount: null,
+                expectedErrorCount: null,
+                expectedType: TestUtils.DT(expectedType),
                 symbolTable: symbol,
                 numberIsFloat: true);
         }
@@ -4196,12 +4196,46 @@ public void TexlFunctionTypeSemanticsTable_Delegation(string script, string expe
 
             TestBindingWarning(
                 script,
-                TestUtils.DT(expectedSchema),
                 errorCount,
+                expectedType: TestUtils.DT(expectedSchema),
                 symbol,
                 features: Features.PowerFxV1);
         }
 
+        [Theory]
+
+        [InlineData("logicalNum = logicalNum")]
+        [InlineData("ThisRecord.logicalNum = logicalNum")]
+        [InlineData("logicalNum = ThisRecord.logicalNum")]
+        [InlineData("ThisRecord.logicalNum = ThisRecord.logicalNum")]
+
+        [InlineData("DisplayNum = DisplayNum")]
+        [InlineData("ThisRecord.DisplayNum = DisplayNum")]
+        [InlineData("DisplayNum = ThisRecord.DisplayNum")]
+        [InlineData("ThisRecord.DisplayNum = ThisRecord.DisplayNum")]
+
+        [InlineData("Filter(tableVar, f1 = f1)")]
+        [InlineData("Filter(tableVar, ThisRecord.f1 = f1)")]
+        [InlineData("Filter(tableVar, f1 = ThisRecord.f1)")]
+        [InlineData("Filter(tableVar, ThisRecord.f1 = ThisRecord.f1)")]
+
+        [InlineData("Filter(tableVar, F1 = F1)")]
+        [InlineData("Filter(tableVar, ThisRecord.F1 = F1)")]
+        [InlineData("Filter(tableVar, F1 = ThisRecord.F1)")]
+        [InlineData("Filter(tableVar, ThisRecord.F1 = ThisRecord.F1)")]
+        public void SameFirstNameNodeComparisonWarning(string script)
+        {
+            var tableVarType = RecordType.Empty().Add("f1", FormulaType.String, "F1").ToTable();
+            var symbolRecord = RecordType.Empty().Add("logicalNum", FormulaType.Number, "DisplayNum").Add("tableVar", tableVarType, "TableVar");
+            var symbol = new SymbolTableOverRecordType(symbolRecord, allowThisRecord: true);
+
+            TestBindingWarning(
+                script,
+                expectedErrorCount: 1,
+                symbolTable: symbol,
+                features: Features.PowerFxV1);
+        }
+
         private void TestBindingPurity(string script, bool isPure, SymbolTable symbolTable = null)
         {
             var config = new PowerFxConfig
@@ -4216,7 +4250,7 @@ private void TestBindingPurity(string script, bool isPure, SymbolTable symbolTab
             Assert.Equal(isPure, result.Binding.IsPure(result.Parse.Root));
         }
 
-        private void TestBindingWarning(string script, DType expectedType, int? expectedErrorCount, SymbolTable symbolTable = null, bool numberIsFloat = false, Features features = null)
+        private void TestBindingWarning(string script, int? expectedErrorCount, DType expectedType = null, ReadOnlySymbolTable symbolTable = null, bool numberIsFloat = false, Features features = null)
         {
             var config = features != null ? new PowerFxConfig(features) : new PowerFxConfig(Features.None);
             var parserOptions = new ParserOptions()
@@ -4225,9 +4259,13 @@ private void TestBindingWarning(string script, DType expectedType, int? expected
             };
 
             var engine = new Engine(config);
-            var result = engine.Check(script, parserOptions, symbolTable);
-
-            Assert.Equal(expectedType, result.Binding.ResultType);
+            var result = engine.Check(script, parserOptions, symbolTable);
+
+            if (expectedType != null)
+            {
+                Assert.Equal(expectedType, result.Binding.ResultType);
+            }
+
             Assert.True(result.Binding.ErrorContainer.HasErrors());
             if (expectedErrorCount != null)
             {

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) ? "=" : "<>");