Check if predicate contains scope reference

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

@@ -123,7 +123,7 @@ internal sealed partial class TexlBinding
         // This is set when a First Name node or child First Name node contains itself in its variable weight
         // and can be read by the back end to determine whether it may generate code that lifts or caches an
         // expression
-        private readonly BitArray _isUnliftable;
+        private readonly BitArray _isUnliftable;
 
         public bool HasLocalScopeReferences { get; private set; }
 
@@ -1124,6 +1124,21 @@ public void CheckAndMarkAsPageable(CallNode node, TexlFunction func)
                     TransitionsFromAsyncToSync = true;
                 }
             }
+        }
+
+        public void CheckPredicateUsage(CallNode node, TexlFunction func)
+        {
+            Contracts.AssertValue(node);
+            Contracts.AssertValue(func);
+
+            if (func.ScopeInfo != null &&
+                func.ScopeInfo.CheckPredicateUsage &&
+                !node.Args.ChildNodes.Any(child => child is ErrorNode) &&
+                TryGetCall(node.Id, out var callInfo))
+            {
+                //func.ScopeInfo.CheckPredicateFields(GetUsedScopeFields(callInfo), node, GetLambdaParamNames(callInfo.ScopeNest + 1), ErrorContainer);
+                func.ScopeInfo.CheckPredicateFields(this, callInfo);
+            }
         }
 
         public void CheckAndMarkAsPageable(FirstNameNode node)
@@ -2050,10 +2065,18 @@ public DType GetUsedScopeFields(CallInfo call)
 
             foreach (var name in GetLambdaParamNames(call.ScopeNest + 1))
             {
-                var fError = false;
+                var fError = false;
+                var fieldName = name.Name;
+                var foundLogicalName = call.CursorType.TryGetLogicalName(name.Name, out var logicalName);
+
+                if (foundLogicalName)
+                {
+                    fieldName = logicalName;
+                }
+
                 if (!name.Node.InTree(arg0) &&
-                    name.Node.InTree(call.Node) &&
-                    call.CursorType.TryGetType(name.Name, out var lambdaParamType))
+                    name.Node.InTree(call.Node) &&
+                    call.CursorType.TryGetType(fieldName, out var lambdaParamType))
                 {
                     if (name.Node.Parent is DottedNameNode dotted)
                     {
@@ -3114,8 +3137,8 @@ private bool IsRowScopeField(FirstNameNode node, out Scope scope, out bool fErro
                     scope = default;
                     return false;
                 }
-
-                var nodeName = node.Ident.Name;
+
+                var nodeName = node.Ident.Name;
 
                 // Look up the name in the current scopes, innermost to outermost.
                 // The logic here is as follows:
@@ -3137,13 +3160,12 @@ private bool IsRowScopeField(FirstNameNode node, out Scope scope, out bool fErro
                         // If scope type is a data source, the node may be a display name instead of logical.
                         // Attempt to get the logical name to use for type checking.
                         // If this is executed amidst a metadata refresh then the reference may refer to an old
-                        // display name, so we need to check the old mapping as well as the current mapping.
-                        var usesDisplayName =
-                            DType.TryGetConvertedDisplayNameAndLogicalNameForColumn(scope.Type, nodeName.Value, out var maybeLogicalName, out _) ||
-                            DType.TryGetLogicalNameForColumn(scope.Type, nodeName.Value, out maybeLogicalName);
-                        if (usesDisplayName)
-                        {
-                            nodeName = new DName(maybeLogicalName);
+                        // display name, so we need to check the old mapping as well as the current mapping.
+                        var foundLoficalName = scope.Type.TryGetLogicalName(node.Ident.Name, out var logicalName);
+
+                        if (foundLoficalName)
+                        {
+                            nodeName = logicalName;
                         }
 
                         if (scope.Type.TryGetType(nodeName, out var typeTmp))
@@ -4862,7 +4884,8 @@ private void FinalizeCall(CallNode node)
                 }
 
                 _txb.CheckAndMarkAsDelegatable(node);
-                _txb.CheckAndMarkAsPageable(node, func);
+                _txb.CheckAndMarkAsPageable(node, func);
+                _txb.CheckPredicateUsage(node, func);
 
                 // A function will produce a constant output (and have no side-effects, which is important for
                 // caching/precomputing the result) iff the function is pure and its arguments are constant.

src/libraries/Microsoft.PowerFx.Core/Functions/FunctionScopeInfo.cs

@@ -2,8 +2,11 @@
 // Licensed under the MIT license.
 
 using System;
+using System.Collections.Generic;
+using System.Linq;
 using Microsoft.PowerFx.Core.App.ErrorContainers;
 using Microsoft.PowerFx.Core.Binding;
+using Microsoft.PowerFx.Core.Binding.BindInfo;
 using Microsoft.PowerFx.Core.Errors;
 using Microsoft.PowerFx.Core.Localization;
 using Microsoft.PowerFx.Core.Types;
@@ -39,7 +42,7 @@ internal class FunctionScopeInfo
         /// If false, the author will be warned when inputting predicates that
         /// do not reference the input table.
         /// </summary>
-        public bool AcceptsLiteralPredicates { get; }
+        public bool CheckPredicateUsage { get; }
 
         /// <summary>
         /// True indicates that the function performs some sort of iteration over
@@ -72,15 +75,15 @@ public FunctionScopeInfo(
             TexlFunction function,
             bool usesAllFieldsInScope = true,
             bool supportsAsyncLambdas = true,
-            bool acceptsLiteralPredicates = true,
+            bool checkPredicateUsage = false,
             bool iteratesOverScope = true,
             DType scopeType = null,
             Func<int, bool> appliesToArgument = null,
             bool canBeCreatedByRecord = false)
         {
             UsesAllFieldsInScope = usesAllFieldsInScope;
             SupportsAsyncLambdas = supportsAsyncLambdas;
-            AcceptsLiteralPredicates = acceptsLiteralPredicates;
+            CheckPredicateUsage = checkPredicateUsage;
             IteratesOverScope = iteratesOverScope;
             ScopeType = scopeType;
             _function = function;
@@ -192,29 +195,6 @@ public virtual bool CheckInput(Features features, TexlNode inputNode, DType inpu
         public virtual bool CheckInput(Features features, CallNode callNode, TexlNode inputNode, DType inputSchema, out DType typeScope)
         {
             return CheckInput(features, inputNode, inputSchema, TexlFunction.DefaultErrorContainer, out typeScope);
-        }
-
-        public void CheckLiteralPredicates(TexlNode[] args, IErrorContainer errors)
-        {
-            Contracts.AssertValue(args);
-            Contracts.AssertValue(errors);
-
-            if (!AcceptsLiteralPredicates)
-            {
-                for (var i = 0; i < args.Length; i++)
-                {
-                    if (_function.IsLambdaParam(args[i], i))
-                    {
-                        if (args[i].Kind == NodeKind.BoolLit ||
-                            args[i].Kind == NodeKind.NumLit ||
-                            args[i].Kind == NodeKind.DecLit ||
-                            args[i].Kind == NodeKind.StrLit)
-                        {
-                            errors.EnsureError(DocumentErrorSeverity.Warning, args[i], TexlStrings.WarnLiteralPredicate);
-                        }
-                    }
-                }
-            }
         }
 
         /// <summary>
@@ -235,10 +215,54 @@ public virtual bool GetScopeIdent(TexlNode[] nodes, out DName[] scopeIdents)
             }
 
             return false;
+        }
+
+        public virtual void CheckPredicateFields(TexlBinding binding, CallInfo callInfo)
+        {
+            var fields = binding.GetUsedScopeFields(callInfo);
+            var lambdaParamNames = binding.GetLambdaParamNames(callInfo.ScopeNest + 1);
+
+            if (fields == DType.Error || fields.GetAllNames(DPath.Root).Any())
+            {
+                return;
+            }
+
+            GetScopeIdent(callInfo.Node.Args.ChildNodes.ToArray(), out var idents);
+
+            if (!lambdaParamNames.Any(lambdaName => idents.Contains(lambdaName.Name)))
+            {
+                binding.ErrorContainer.EnsureError(DocumentErrorSeverity.Warning, callInfo.Node, TexlStrings.WarnCheckPredicateUsage);
+            }
         }
     }
 
-    internal class FunctionThisGroupScopeInfo : FunctionScopeInfo
+    internal class FunctionFilterScopeInfo : FunctionScopeInfo
+    {
+        public FunctionFilterScopeInfo(TexlFunction function)
+            : base(function, checkPredicateUsage: true)
+        {
+        }
+
+        public override void CheckPredicateFields(TexlBinding binding, CallInfo callInfo)
+        {
+            // Filter can also accept a view as argument.
+            // In the event of any argN (where N > 1) is a view, the binder will validate if the view is valid or not.
+            // We check if there is any view as argument. If there is, we just skip the predicate checking.
+            foreach (var childNode in callInfo.Node.Args.ChildNodes)
+            {
+                var argType = binding.GetType(childNode);
+
+                if (argType.Kind == DKind.ViewValue)
+                {
+                    return;
+                }
+            }
+
+            base.CheckPredicateFields(binding, callInfo);
+        }
+    }
+
+    internal class FunctionThisGroupScopeInfo : FunctionScopeInfo
     {
         public static DName ThisGroup => new DName("ThisGroup");
 
@@ -283,14 +307,14 @@ public override bool CheckInput(Features features, CallNode callNode, TexlNode i
         }
     }
 
-    internal class FunctionJoinScopeInfo : FunctionScopeInfo
+    internal class FunctionJoinScopeInfo : FunctionScopeInfo
     {
         public static DName LeftRecord => new DName("LeftRecord");
 
         public static DName RightRecord => new DName("RightRecord");
 
         public FunctionJoinScopeInfo(TexlFunction function)
-            : base(function, appliesToArgument: (argIndex) => argIndex > 1)
+            : base(function, appliesToArgument: (argIndex) => argIndex > 1, checkPredicateUsage: true)
         {
         }
 
@@ -324,19 +348,65 @@ public override bool GetScopeIdent(TexlNode[] nodes, out DName[] scopeIdents)
             {
                 scopeIdents[0] = LeftRecord;
             }
-
+
             if (nodes.Length > 1 && nodes[1] is AsNode rightAsNode)
             {
                 scopeIdents[1] = rightAsNode.Right.Name;
             }
             else
             {
                 scopeIdents[1] = RightRecord;
-            }
+            }
 
             // Returning false to indicate that the scope is not a whole scope.
-            // Meaning that the scope is a record type and we are accessing the fields directly.
+            // Meaning that the scope is a record type and we are accessing the fields directly.
             return false;
         }
+
+        public override void CheckPredicateFields(TexlBinding binding, CallInfo callInfo)
+        {
+            var fields = binding.GetUsedScopeFields(callInfo);
+            var lambdaParamNames = binding.GetLambdaParamNames(callInfo.ScopeNest + 1);
+
+            // If Join call node has less than 5 records, we are possibly looking for suggestions.
+            if (callInfo.Node.Args.ChildNodes.Count < 5 || fields == DType.Error)
+            {
+                return;
+            }
+
+            GetScopeIdent(callInfo.Node.Args.ChildNodes.ToArray(), out var idents);
+
+            var foundIdents = new HashSet<DName>();
+            var predicate = callInfo.Node.Args.ChildNodes[2];
+
+            // In the Join function, arg2 and argN > 3 are lambdas nodes.
+            // We need to check if scope identifiers are used arg2 (predicate).
+            foreach (var lambda in lambdaParamNames)
+            {
+                var parent = lambda.Node.Parent;
+
+                while (parent != null)
+                {
+                    if (parent.Id == predicate.Id)
+                    {
+                        foundIdents.Add(lambda.Name);
+                        break;
+                    }
+
+                    parent = parent.Parent;
+                }
+            }
+
+            var foundIdentsArray = foundIdents.ToArray();
+
+            if (foundIdents.Count == 2 && 
+                fields.TryGetType(foundIdentsArray[0], out _) && 
+                fields.TryGetType(foundIdentsArray[1], out _))
+            {                
+                return;
+            }
+
+            binding.ErrorContainer.EnsureError(DocumentErrorSeverity.Warning, callInfo.Node, TexlStrings.WarnCheckPredicateUsage);
+        }
     }
 }

src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs

@@ -609,8 +609,6 @@ private bool CheckTypesCore(CheckTypesContext context, TexlNode[] args, DType[]
                 nodeToCoercedTypeMap = null;
             }
 
-            ScopeInfo?.CheckLiteralPredicates(args, errors);
-
             // Default return type.
             returnType = ReturnType;
 

src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs

@@ -616,7 +616,7 @@ private static IntermediateNode ConcatenateArgs(IntermediateNode arg1, Intermedi
                     }
                 }
 
-                var concatenatedNode = new CallNode(irContext, BuiltinFunctionsCore.Concatenate, concatenateArgs);
+                var concatenatedNode = new CallNode(irContext, BuiltinFunctionsCore.Concatenate, concatenateArgs);
                 return concatenatedNode;
             }
 

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

@@ -804,7 +804,7 @@ internal static class TexlStrings
         public static ErrorResourceKey ErrAsNotInContext = new ErrorResourceKey("ErrAsNotInContext");
         public static ErrorResourceKey ErrValueMustBeFullyQualified = new ErrorResourceKey("ErrValueMustBeFullyQualified");
         public static ErrorResourceKey WarnColumnNameSpecifiedMultipleTimes_Name = new ErrorResourceKey("WarnColumnNameSpecifiedMultipleTimes_Name");
-        public static ErrorResourceKey WarnLiteralPredicate = new ErrorResourceKey("WarnLiteralPredicate");
+        public static ErrorResourceKey WarnCheckPredicateUsage = new ErrorResourceKey("WarnCheckPredicateUsage");
         public static ErrorResourceKey WarnDynamicMetadata = new ErrorResourceKey("WarnDynamicMetadata");
         public static ErrorResourceKey WarnDeferredType = new ErrorResourceKey("WarnDeferredType");
         public static ErrorResourceKey ErrColRenamedTwice_Name = new ErrorResourceKey("ErrColRenamedTwice_Name");

src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Filter.cs

@@ -27,7 +27,7 @@ internal sealed class FilterFunction : FilterFunctionBase
         public FilterFunction()
             : base("Filter", TexlStrings.AboutFilter, FunctionCategories.Table, DType.EmptyTable, -2, 2, int.MaxValue, DType.EmptyTable)
         {
-            ScopeInfo = new FunctionScopeInfo(this, acceptsLiteralPredicates: false);
+            ScopeInfo = new FunctionFilterScopeInfo(this);
         }
 
         public override IEnumerable<TexlStrings.StringGetter[]> GetSignatures()
@@ -145,7 +145,7 @@ public override bool IsServerDelegatable(CallNode callNode, TexlBinding binding)
             Contracts.AssertValue(callNode);
             Contracts.AssertValue(binding);
 
-            if (!CheckArgsCount(callNode, binding))
+            if (!CheckArgsCount(callNode, binding, DocumentErrorSeverity.Moderate))
             {
                 return false;
             }

src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Lookup.cs

@@ -25,7 +25,7 @@ internal sealed class LookUpFunction : FilterFunctionBase
         public LookUpFunction()
             : base("LookUp", TexlStrings.AboutLookUp, FunctionCategories.Table, DType.Unknown, 0x6, 2, 3, DType.EmptyTable, DType.Boolean)
         {
-            ScopeInfo = new FunctionScopeInfo(this);
+            ScopeInfo = new FunctionScopeInfo(this, checkPredicateUsage: true);
         }
 
         public override IEnumerable<TexlStrings.StringGetter[]> GetSignatures()

src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/StatisticalTableFunction.cs

@@ -27,7 +27,7 @@ internal abstract class StatisticalTableFunction : FunctionWithTableInput
         public StatisticalTableFunction(string name, TexlStrings.StringGetter description, FunctionCategories fc, bool nativeDecimal = false)
             : base(name, description, fc, DType.Number, 0x02, 2, 2, DType.EmptyTable, DType.Number)
         {
-            ScopeInfo = new FunctionScopeInfo(this, usesAllFieldsInScope: false, acceptsLiteralPredicates: false);
+            ScopeInfo = new FunctionScopeInfo(this, usesAllFieldsInScope: false, checkPredicateUsage: true);
             _nativeDecimal = nativeDecimal;
         }
 
@@ -116,8 +116,6 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp
                 return false;
             }
 
-            ScopeInfo?.CheckLiteralPredicates(args, errors);
-
             return true;
         }
     }