Added argument index to UDF params (#1439)

Added arg index to udf params, we need this during code gen on the host.
Created a new info type for UDF parameters.
Modified UserDefinitionsNameResolver to not use record type, since I
didn't find a way to get the index of a pram.

src/libraries/Microsoft.PowerFx.Core/Binding/BindInfo/UDFParameterInfo.cs

@@ -0,0 +1,29 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+using Microsoft.PowerFx.Core.Types;
+using Microsoft.PowerFx.Core.Utils;
+
+namespace Microsoft.PowerFx.Core.Binding.BindInfo
+{
+    /// <summary>
+    /// Lookup info for parameters in user defined functions.
+    /// </summary>
+    internal sealed class UDFParameterInfo
+    {
+        public readonly DType Type;
+        public readonly int ArgIndex;
+        public readonly DName Name;
+
+        public UDFParameterInfo(DType type, int argIndex, DName name)
+        {
+            Contracts.AssertValid(type);
+            Contracts.AssertValid(name);
+            Contracts.Assert(argIndex >= 0);
+
+            Type = type;
+            ArgIndex = argIndex;
+            Name = name;
+        }
+    }
+}

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

@@ -8,6 +8,7 @@ using System.Numerics;
 using System.Text;
 using System.Xml.Linq;
 using Microsoft.PowerFx.Core.App;
+using Microsoft.PowerFx.Core.App.Controls;
 using Microsoft.PowerFx.Core.App.ErrorContainers;
 using Microsoft.PowerFx.Core.Binding;
 using Microsoft.PowerFx.Core.Binding.BindInfo;
@@ -29,25 +30,48 @@ namespace Microsoft.PowerFx.Core.Functions
     {
         private readonly bool _isImperative;
         private readonly IdentToken _returnTypeToken;
-        private readonly ISet<UDFArg> _args;
-        private readonly RecordType _parametersRecordType;
+        private readonly IEnumerable<UDFArg> _args;
+        private TexlBinding _binding;
+
+        public override bool IsAsync => _binding?.IsAsync(UdfBody) ?? false;
 
         public TexlNode UdfBody { get; }
 
         public override bool IsSelfContained => !_isImperative;
 
-        public UserDefinedFunction(string name, IdentToken returnTypeToken, TexlNode body, bool isImperative, ISet<UDFArg> args, RecordType parametersRecordType)
-        : base(DPath.Root, name, name, SG("Custom func " + name), FunctionCategories.UserDefined, returnTypeToken.GetFormulaType()._type, 0, args.Count, args.Count, args.Select(a => a.VarType.GetFormulaType()._type).ToArray())
+        public UserDefinedFunction(string name, IdentToken returnTypeToken, TexlNode body, bool isImperative, ISet<UDFArg> args)
+        : base(DPath.Root, name, name, SG("Custom func " + name), FunctionCategories.UserDefined, returnTypeToken.GetFormulaType()._type, 0, args.Count, args.Count, args.Select(a => a.TypeIdent.GetFormulaType()._type).ToArray())
         {
             this._returnTypeToken = returnTypeToken;
             this._args = args;
-            this._parametersRecordType = parametersRecordType;
             this._isImperative = isImperative;
 
             this.UdfBody = body;
         }
 
-        public TexlBinding BindBody(INameResolver nameResolver, IBinderGlue documentBinderGlue, BindingConfig bindingConfig = null, IUserDefinitionSemanticsHandler userDefinitionSemanticsHandler = null, Features features = null, INameResolver functionNameResolver = null)
+        public bool TryGetArgIndex(string argName, out int argIndex)
+        {
+            argIndex = -1;
+
+            if (string.IsNullOrEmpty(argName))
+            {
+                return false;
+            }
+
+            foreach (var arg in _args)
+            {
+                if (arg.NameIdent.Name.Value == argName)
+                {
+                    argIndex = arg.ArgIndex;
+
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        public TexlBinding BindBody(INameResolver nameResolver, IBinderGlue documentBinderGlue, BindingConfig bindingConfig = null, IUserDefinitionSemanticsHandler userDefinitionSemanticsHandler = null, Features features = null, INameResolver functionNameResolver = null, IExternalRule rule = null)
         {
             if (nameResolver is null)
             {
@@ -60,12 +84,12 @@ namespace Microsoft.PowerFx.Core.Functions
             }
 
             bindingConfig = bindingConfig ?? new BindingConfig(this._isImperative);
-            var binding = TexlBinding.Run(documentBinderGlue, UdfBody, UserDefinitionsNameResolver.Merge(nameResolver, _parametersRecordType, functionNameResolver), bindingConfig, features: features);
+            _binding = TexlBinding.Run(documentBinderGlue, UdfBody, UserDefinitionsNameResolver.Create(nameResolver, _args, functionNameResolver), bindingConfig, features: features, rule: rule);
 
-            CheckTypesOnDeclaration(binding.CheckTypesContext, binding.ResultType, binding.ErrorContainer);
-            userDefinitionSemanticsHandler?.CheckSemanticsOnDeclaration(binding, _args, binding.ErrorContainer);
+            CheckTypesOnDeclaration(_binding.CheckTypesContext, _binding.ResultType, _binding.ErrorContainer);
+            userDefinitionSemanticsHandler?.CheckSemanticsOnDeclaration(_binding, _args, _binding.ErrorContainer);
 
-            return binding;
+            return _binding;
         }
 
         /// <summary>
@@ -92,7 +116,7 @@ namespace Microsoft.PowerFx.Core.Functions
 
         public override IEnumerable<StringGetter[]> GetSignatures()
         {
-            yield return new[] { SG("Arg 1") };
+            return new[] { _args.Select<UDFArg, TexlStrings.StringGetter>(key => _ => key.NameIdent.Name.Value).ToArray() };
         }
 
         /// <summary>
@@ -101,18 +125,18 @@ namespace Microsoft.PowerFx.Core.Functions
         private partial class UserDefinitionsNameResolver : INameResolver
         {
             private readonly INameResolver _globalNameResolver;
-            private readonly INameResolver _parameterNamedResolver;
+            private readonly IReadOnlyDictionary<string, UDFArg> _args;
             private readonly INameResolver _functionNameResolver;
 
-            public static INameResolver Merge(INameResolver globalNameResolver, RecordType parametersRecordType, INameResolver functionNameResolver)
+            public static INameResolver Create(INameResolver globalNameResolver, IEnumerable<UDFArg> args, INameResolver functionNameResolver)
             {
-                return new UserDefinitionsNameResolver(globalNameResolver, ReadOnlySymbolTable.NewFromRecord(parametersRecordType), functionNameResolver);
+                return new UserDefinitionsNameResolver(globalNameResolver, args, functionNameResolver);
             }
 
-            private UserDefinitionsNameResolver(INameResolver globalNameResolver, INameResolver parameterNamedResolver, INameResolver functionNameResolver = null)
+            private UserDefinitionsNameResolver(INameResolver globalNameResolver, IEnumerable<UDFArg> args, INameResolver functionNameResolver = null)
             {
                 this._globalNameResolver = globalNameResolver;
-                this._parameterNamedResolver = parameterNamedResolver;
+                this._args = args.ToDictionary(arg => arg.NameIdent.Name.Value, arg => arg);
                 this._functionNameResolver = functionNameResolver;
             }
 
@@ -133,7 +157,15 @@ namespace Microsoft.PowerFx.Core.Functions
             public bool Lookup(DName name, out NameLookupInfo nameInfo, NameLookupPreferences preferences = NameLookupPreferences.None)
             {
                 // lookup in the local scope i.e., function params & body and then look in global scope.
-                return _parameterNamedResolver.Lookup(name, out nameInfo, preferences) || _globalNameResolver.Lookup(name, out nameInfo, preferences);
+                if (_args.TryGetValue(name, out var value))
+                {
+                    var type = value.TypeIdent.GetFormulaType()._type;
+                    nameInfo = new NameLookupInfo(BindKind.PowerFxResolvedObject, type, DPath.Root, 0, new UDFParameterInfo(type, value.ArgIndex, value.NameIdent.Name));
+
+                    return true;
+                }
+
+                return _globalNameResolver.Lookup(name, out nameInfo, preferences);
             }
 
             public bool LookupDataControl(DName name, out NameLookupInfo lookupInfo, out DName dataControlName)

src/libraries/Microsoft.PowerFx.Core/Parser/ParseFormulasResult.cs

@@ -93,13 +93,15 @@ namespace Microsoft.PowerFx.Core.Parser
 
     internal class UDFArg
     {
-        internal IdentToken VarIdent;
-        internal IdentToken VarType;
+        internal IdentToken NameIdent;
+        internal IdentToken TypeIdent;
+        internal int ArgIndex;
 
-        public UDFArg(IdentToken varIdent, IdentToken varType)
+        public UDFArg(IdentToken nameIdent, IdentToken typeIdent, int argIndex)
         {
-            VarIdent = varIdent;
-            VarType = varType;
+            NameIdent = nameIdent;
+            TypeIdent = typeIdent;
+            ArgIndex = argIndex;
         } 
     }
 }

src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs

@@ -121,6 +121,7 @@ namespace Microsoft.PowerFx.Core.Parser
         private bool ParseUDFArgs(out HashSet<UDFArg> args)
         {
             args = new HashSet<UDFArg>();
+            int argIndex = 0;
             if (TokEat(TokKind.ParenOpen) == null)
             {
                 return false;
@@ -147,7 +148,7 @@ namespace Microsoft.PowerFx.Core.Parser
 
                 ParseTrivia();
 
-                args.Add(new UDFArg(varIdent.As<IdentToken>(), varType.As<IdentToken>()));
+                args.Add(new UDFArg(varIdent.As<IdentToken>(), varType.As<IdentToken>(), argIndex++));
                 if (_curs.TokCur.Kind != TokKind.ParenClose && _curs.TokCur.Kind != TokKind.Comma)
                 {
                     ErrorTid(_curs.TokCur, TokKind.Comma);

src/libraries/Microsoft.PowerFx.Core/Syntax/UserDefinitions.cs

@@ -64,7 +64,7 @@ namespace Microsoft.PowerFx.Syntax
             return TexlParser.ParseUserDefinitionScript(script, numberAsFloat, loc);
         }
 
-        public static bool ProcessUserDefnitions(string script, INameResolver globalNameResolver, IBinderGlue documentBinderGlue, BindingConfig bindingConfig, out UserDefinitionResult userDefinitionResult, IUserDefinitionSemanticsHandler userDefinitionSemanticsHandler = null, CultureInfo loc = null, bool numberAsFloat = false, Features features = null)
+        public static bool ProcessUserDefinitions(string script, INameResolver globalNameResolver, IBinderGlue documentBinderGlue, BindingConfig bindingConfig, out UserDefinitionResult userDefinitionResult, IUserDefinitionSemanticsHandler userDefinitionSemanticsHandler = null, CultureInfo loc = null, bool numberAsFloat = false, Features features = null)
         {
             var userDefinitions = new UserDefinitions(script, globalNameResolver, documentBinderGlue, bindingConfig, loc, numberAsFloat, userDefinitionSemanticsHandler, features);
 
@@ -99,18 +99,20 @@ namespace Microsoft.PowerFx.Syntax
 
             foreach (var udf in uDFs)
             {
-                CheckParameters(udf.Args, errors, out var paramsRecordType);
-                CheckReturnType(udf.ReturnType, errors);
-
                 var udfName = udf.Ident.Name;
-                var func = new UserDefinedFunction(udfName.Value, udf.ReturnType, udf.Body, udf.IsImperative, udf.Args, paramsRecordType);
-
                 if (texlFunctionSet.AnyWithName(udfName) || BuiltinFunctionsCore._library.AnyWithName(udfName))
                 {
                     errors.Add(new TexlError(udf.Ident, DocumentErrorSeverity.Severe, TexlStrings.ErrUDF_FunctionAlreadyDefined, udfName));
                     continue;
                 }
 
+                if (!(CheckParameters(udf.Args, errors) & CheckReturnType(udf.ReturnType, errors)))
+                {
+                    continue;
+                }
+                
+                var func = new UserDefinedFunction(udfName.Value, udf.ReturnType, udf.Body, udf.IsImperative, udf.Args);
+
                 texlFunctionSet.Add(func);
                 userDefinedFunctions.Add(func);
             }
@@ -130,39 +132,45 @@ namespace Microsoft.PowerFx.Syntax
             }
         }
 
-        private void CheckParameters(ISet<UDFArg> args, List<TexlError> errors, out RecordType paramRecordType)
+        private bool CheckParameters(ISet<UDFArg> args, List<TexlError> errors)
         {
-            paramRecordType = RecordType.Empty();
+            var isParamCheckSuccessful = true;
             var argsAlreadySeen = new HashSet<string>();
 
             foreach (var arg in args)
             {
-                if (argsAlreadySeen.Contains(arg.VarIdent.Name))
+                if (argsAlreadySeen.Contains(arg.NameIdent.Name))
                 {
-                    errors.Add(new TexlError(arg.VarIdent, DocumentErrorSeverity.Severe, TexlStrings.ErrUDF_DuplicateParameter, arg.VarIdent.Name));
+                    errors.Add(new TexlError(arg.NameIdent, DocumentErrorSeverity.Severe, TexlStrings.ErrUDF_DuplicateParameter, arg.NameIdent.Name));
+                    isParamCheckSuccessful = false;
                 }
                 else
                 {
-                    argsAlreadySeen.Add(arg.VarIdent.Name);
+                    argsAlreadySeen.Add(arg.NameIdent.Name);
 
-                    if (arg.VarType.GetFormulaType()._type.Kind.Equals(DType.Unknown.Kind))
+                    if (arg.TypeIdent.GetFormulaType()._type.Kind.Equals(DType.Unknown.Kind))
                     {
-                        errors.Add(new TexlError(arg.VarType, DocumentErrorSeverity.Severe, TexlStrings.ErrUDF_UnknownType, arg.VarType.Name));
+                        errors.Add(new TexlError(arg.TypeIdent, DocumentErrorSeverity.Severe, TexlStrings.ErrUDF_UnknownType, arg.TypeIdent.Name));
+                        isParamCheckSuccessful = false;
                     }
-
-                    paramRecordType = paramRecordType.Add(new NamedFormulaType(arg.VarIdent.ToString(), arg.VarType.GetFormulaType()));
                 }
             }
+
+            return isParamCheckSuccessful;
         }
 
-        private void CheckReturnType(IdentToken returnType, List<TexlError> errors)
+        private bool CheckReturnType(IdentToken returnType, List<TexlError> errors)
         {
             var returnTypeFormulaType = returnType.GetFormulaType()._type;
+            var isReturnTypeCheckSuccessful = true;
 
             if (returnTypeFormulaType.Kind.Equals(DType.Unknown.Kind))
             {
                 errors.Add(new TexlError(returnType, DocumentErrorSeverity.Severe, TexlStrings.ErrUDF_UnknownType, returnType.Name));
+                isReturnTypeCheckSuccessful = false;
             }
+
+            return isReturnTypeCheckSuccessful;
         }
     }
 }

src/libraries/Microsoft.PowerFx.Interpreter/RecalcEngine.cs

@@ -191,7 +191,7 @@ namespace Microsoft.PowerFx
                 udf.ReturnType.GetFormulaType(),
                 udf.IsImperative,
                 udf.NumberIsFloat,
-                udf.Args.Select(arg => new NamedFormulaType(arg.VarIdent.ToString(), arg.VarType.GetFormulaType())).ToArray())).ToArray();
+                udf.Args.Select(arg => new NamedFormulaType(arg.NameIdent.ToString(), arg.TypeIdent.GetFormulaType())).ToArray())).ToArray();
 
             return DefineFunctions(udfDefinitions);
         }

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

@@ -25,8 +25,8 @@ namespace Microsoft.PowerFx.Core.Tests
             Assert.Equal("Abs(x)", udf.Body.ToString());
             Assert.Equal("Number", udf.ReturnType.ToString());
             var arg = udf.Args.First();
-            Assert.Equal("x", arg.VarIdent.ToString());
-            Assert.Equal("Number", arg.VarType.ToString());
+            Assert.Equal("x", arg.NameIdent.ToString());
+            Assert.Equal("Number", arg.TypeIdent.ToString());
         }
 
         [Theory]

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

@@ -0,0 +1,70 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using Microsoft.PowerFx.Core.Binding;
+using Microsoft.PowerFx.Core.Glue;
+using Microsoft.PowerFx.Core.Syntax;
+using Microsoft.PowerFx.Core.Texl;
+using Microsoft.PowerFx.Syntax;
+using Xunit;
+
+namespace Microsoft.PowerFx.Core.Tests
+{
+    public class UserDefinedFunctionTests : PowerFxTest
+    {
+        private bool ProcessUserDefinitions(string script, out UserDefinitionResult userDefinitionResult)
+        {
+            return UserDefinitions.ProcessUserDefinitions(script, ReadOnlySymbolTable.NewDefault(BuiltinFunctionsCore._library), new Glue2DocumentBinderGlue(), BindingConfig.Default, out userDefinitionResult);
+        }
+
+        [Theory]
+        [InlineData("Foo(x: Number): Number = Abs(x);")]
+        [InlineData("Foo(x: Number): Number{ Abs(x); };")]
+        public void TestUDFDeclaration(string script)
+        {
+            var userDefinitions = ProcessUserDefinitions(script, out var userDefinitionResult);
+            var udf = userDefinitionResult.UDFs.FirstOrDefault();
+
+            Assert.NotNull(udf);
+
+            Assert.Empty(userDefinitionResult.NamedFormulas);
+            Assert.Equal("Foo", udf.Name);
+            Assert.True(udf.ReturnType.IsPrimitive);
+            Assert.Empty(userDefinitionResult.Errors);
+        }
+
+        [Theory]
+        [InlineData("Foo(x: Number): Number = Abs(x);", 1, 0, false)]
+        [InlineData("Foo(x: Number): Number = Abs(x); x = 1;", 1, 1, false)]
+        [InlineData("x = 1; Foo(x: Number): Number = Abs(x);", 1, 1, false)]
+        [InlineData("/*this is a test*/ x = 1; Foo(x: Number): Number = Abs(x);", 1, 1, false)]
+        [InlineData("x = 1; Foo(x: Number): Number = Abs(x); y = 2;", 1, 2, false)]
+        [InlineData("Add(x: Number, y:Number): Number = x + y; Foo(x: Number): Number = Abs(x); y = 2;", 2, 1, false)]
+        [InlineData("Foo(x: Number): Number = /*this is a test*/ Abs(x); y = 2;", 1, 1, false)]
+        [InlineData("Add(x: Number, y:Number): Number = b + b; Foo(x: Number): Number = Abs(x); y = 2;", 2, 1, true)]
+        [InlineData("Add(x: Number, y:Number): Boolean = x + y;", 1, 0, true)]
+        [InlineData("Add(x: Number, y:Number): SomeType = x + y;", 0, 0, true)]
+        [InlineData("Add(x: SomeType, y:Number): Number = x + y;", 0, 0, true)]
+        [InlineData("Add(x: Number, y:Number): Number = x + y", 0, 0, true)]
+        [InlineData("x = 1; Add(x: Number, y:Number): Number = x + y", 0, 1, true)]
+        [InlineData("Add(x: Number, y:Number) = x + y;", 0, 0, true)]
+        [InlineData("Add(x): Number = x + 2;", 0, 0, true)]
+        [InlineData("Add(a:Number, b:Number): Number { a + b + 1; \n a + b; };", 1, 0, false)]
+        [InlineData("Add(a:Number, b:Number): Number { a + b; };", 1, 0, false)]
+        [InlineData("Add(a:Number, b:Number): Number { /*this is a test*/ a + b; };", 1, 0, false)]
+        [InlineData("Add(a:Number, b:Number): Number { /*this is a test*/ a + b; ;", 0, 0, true)]
+        [InlineData("Add(a:Number, a:Number): Number { a; };", 0, 0, true)]
+        public void TestUDFNamedFormulaCounts(string script, int udfCount, int namedFormulaCount, bool expectErrors)
+        {
+            var userDefinitions = UserDefinitions.ProcessUserDefinitions(script, ReadOnlySymbolTable.NewDefault(BuiltinFunctionsCore._library), new Glue2DocumentBinderGlue(), BindingConfig.Default, out var userDefinitionResult);
+
+            Assert.Equal(udfCount, userDefinitionResult.UDFs.Count());
+            Assert.Equal(namedFormulaCount, userDefinitionResult.NamedFormulas.Count());
+            Assert.Equal(expectErrors, userDefinitionResult.Errors.Any());
+        }
+    }
+}