Enable UDF on REPL (#2559)

Adds ability to define User-defined functions in REPL.

Parsing UDFs requires a different parser from the Texl Expression parser
and this is addressed by using the DefinitionsParser as a fallback to
check when regular parsing fails. If definitions parsing is successful
and we find UDF, we add it to the Engine.


![image](https://github.com/user-attachments/assets/ee86d499-2ee6-4232-a7cd-3b5ca9e09d1e)

Fixes #2546

src/libraries/Microsoft.PowerFx.Core/Public/Config/SymbolTable.cs

@@ -208,63 +208,39 @@ namespace Microsoft.PowerFx
         }
 
         /// <summary>
-        /// Adds an user defined function.
+        /// Adds user defined functions in the script.
         /// </summary>
         /// <param name="script">String representation of the user defined function.</param>
         /// <param name="parseCulture">CultureInfo to parse the script againts. Default is invariant.</param>
         /// <param name="symbolTable">Extra symbols to bind UDF. Commonly coming from Engine.</param>
         /// <param name="extraSymbolTable">Additional symbols to bind UDF.</param>
         /// <param name="allowSideEffects">Allow for curly brace parsing.</param>
-        internal void AddUserDefinedFunction(string script, CultureInfo parseCulture = null, ReadOnlySymbolTable symbolTable = null, ReadOnlySymbolTable extraSymbolTable = null, bool allowSideEffects = false)
+        internal DefinitionsCheckResult AddUserDefinedFunction(string script, CultureInfo parseCulture = null, ReadOnlySymbolTable symbolTable = null, ReadOnlySymbolTable extraSymbolTable = null, bool allowSideEffects = false)
         {
             // Phase 1: Side affects are not allowed.
             // Phase 2: Introduces side effects and parsing of function bodies.
             var options = new ParserOptions() 
             { 
                 AllowsSideEffects = allowSideEffects, 
-                Culture = parseCulture ?? CultureInfo.InvariantCulture 
+                Culture = parseCulture ?? CultureInfo.InvariantCulture,
             };
-            var sb = new StringBuilder();
 
-            var parseResult = UserDefinitions.Parse(script, options);
-
-            // Compose will handle null symbols
             var composedSymbols = Compose(this, symbolTable, extraSymbolTable);
 
-            var udfs = UserDefinedFunction.CreateFunctions(parseResult.UDFs.Where(udf => udf.IsParseValid), composedSymbols, out var errors);
+            var checkResult = new DefinitionsCheckResult();
 
-            errors.AddRange(parseResult.Errors ?? Enumerable.Empty<TexlError>());
+            var udfs = checkResult.SetText(script, options)
+                .SetBindingInfo(composedSymbols)
+                .ApplyCreateUserDefinedFunctions();
 
-            if (errors.Any(error => error.Severity > DocumentErrorSeverity.Warning))
+            Contracts.AssertValue(udfs);
+
+            if (checkResult.IsSuccess)
             {
-                sb.AppendLine("Something went wrong when parsing user defined functions.");
-
-                foreach (var error in errors)
-                {
-                    error.FormatCore(sb);
-                }
-
-                throw new InvalidOperationException(sb.ToString());
+                AddFunctions(udfs);
             }
 
-            foreach (var udf in udfs)
-            {
-                AddFunction(udf);
-                var config = new BindingConfig(allowsSideEffects: allowSideEffects, useThisRecordForRuleScope: false, numberIsFloat: false);
-                var binding = udf.BindBody(composedSymbols, new Glue2DocumentBinderGlue(), config);
-
-                List<TexlError> bindErrors = new List<TexlError>();
-
-                if (binding.ErrorContainer.GetErrors(ref bindErrors))
-                {
-                    sb.AppendLine(string.Join(", ", errors.Select(err => err.ToString())));
-                }
-            }
-
-            if (sb.Length > 0)
-            {
-                throw new InvalidOperationException(sb.ToString());
-            }
+            return checkResult;
         }
 
         /// <summary>

src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs

@@ -12,6 +12,9 @@ using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.PowerFx.Core.Binding;
 using Microsoft.PowerFx.Core.Errors;
+using Microsoft.PowerFx.Core.Functions;
+using Microsoft.PowerFx.Core.Glue;
+using Microsoft.PowerFx.Core.Localization;
 using Microsoft.PowerFx.Core.Parser;
 using Microsoft.PowerFx.Core.Types;
 using Microsoft.PowerFx.Core.Utils;
@@ -30,11 +33,16 @@ namespace Microsoft.PowerFx
 
         private IReadOnlyDictionary<DName, FormulaType> _resolvedTypes;
 
+        private TexlFunctionSet _userDefinedFunctions;
+
         private CultureInfo _defaultErrorCulture;
         private ParserOptions _parserOptions;
 
         private ParseUserDefinitionResult _parse;
 
+        // Local symboltable to store new symbols in a given script and use in binding.
+        private readonly SymbolTable _localSymbolTable;
+
         // Power Fx expression containing definitions
         private string _definitions;
 
@@ -43,6 +51,7 @@ namespace Microsoft.PowerFx
 
         public DefinitionsCheckResult()
         {
+            _localSymbolTable = new SymbolTable { DebugName = "LocalUserDefinitions" };
         }
 
         internal DefinitionsCheckResult SetBindingInfo(ReadOnlySymbolTable symbols)
@@ -59,7 +68,7 @@ namespace Microsoft.PowerFx
             return this;
         }
 
-        internal DefinitionsCheckResult SetText(string definitions, ParserOptions parserOptions = null)
+        public DefinitionsCheckResult SetText(string definitions, ParserOptions parserOptions = null)
         {
             Contracts.AssertValue(definitions);
 
@@ -97,6 +106,8 @@ namespace Microsoft.PowerFx
 
         public IReadOnlyDictionary<DName, FormulaType> ResolvedTypes => _resolvedTypes;
 
+        public bool ContainsUDF => _parse.UDFs.Any();
+
         internal IReadOnlyDictionary<DName, FormulaType> ApplyResolveTypes()
         {
             if (_parse == null)
@@ -114,6 +125,7 @@ namespace Microsoft.PowerFx
                 if (_parse.DefinedTypes.Any())
                 {
                     this._resolvedTypes = DefinedTypeResolver.ResolveTypes(_parse.DefinedTypes.Where(dt => dt.IsParseValid), _symbols, out var errors);
+                    this._localSymbolTable.AddTypes(this._resolvedTypes);
                     _errors.AddRange(ExpressionError.New(errors, _defaultErrorCulture));
                 }
                 else
@@ -125,16 +137,79 @@ namespace Microsoft.PowerFx
             return this._resolvedTypes;
         }
 
+        internal TexlFunctionSet ApplyCreateUserDefinedFunctions()
+        {
+            if (_parse == null)
+            {
+                this.ApplyParse();
+            }
+
+            if (_symbols == null)
+            {
+                throw new InvalidOperationException($"Must call {nameof(SetBindingInfo)} before calling ApplyCreateUserDefinedFunctions().");
+            }
+
+            if (_resolvedTypes == null)
+            {
+                this.ApplyResolveTypes();
+            }
+
+            if (_userDefinedFunctions == null)
+            {
+                _userDefinedFunctions = new TexlFunctionSet();
+
+                var partialUDFs = UserDefinedFunction.CreateFunctions(_parse.UDFs.Where(udf => udf.IsParseValid), _symbols, out var errors);
+
+                if (errors.Any())
+                {
+                    _errors.AddRange(ExpressionError.New(errors, _defaultErrorCulture));
+                }
+
+                var composedSymbols = ReadOnlySymbolTable.Compose(_localSymbolTable, _symbols);
+                foreach (var udf in partialUDFs)
+                {
+                    var config = new BindingConfig(allowsSideEffects: _parserOptions.AllowsSideEffects, useThisRecordForRuleScope: false, numberIsFloat: false);
+                    var binding = udf.BindBody(composedSymbols, new Glue2DocumentBinderGlue(), config);
+
+                    List<TexlError> bindErrors = new List<TexlError>();
+
+                    if (binding.ErrorContainer.HasErrors())
+                    {
+                        _errors.AddRange(ExpressionError.New(binding.ErrorContainer.GetErrors(), _defaultErrorCulture));
+                    }
+                    else
+                    {
+                        _localSymbolTable.AddFunction(udf);
+                        _userDefinedFunctions.Add(udf);
+                    }
+                }
+
+                return this._userDefinedFunctions;
+            }
+
+            return this._userDefinedFunctions;
+        }
+
         internal IEnumerable<ExpressionError> ApplyErrors()
         {
             if (_resolvedTypes == null)
             {
-                ApplyResolveTypes();
+                this.ApplyCreateUserDefinedFunctions();
             }
 
             return this.Errors;
         }
 
+        public IEnumerable<ExpressionError> ApplyParseErrors()
+        {
+            if (_parse == null)
+            {
+                this.ApplyParse();
+            }
+
+            return ExpressionError.New(_parse.Errors, _defaultErrorCulture);
+        }
+
         /// <summary>
         /// List of all errors and warnings. Check <see cref="ExpressionError.IsWarning"/>.
         /// This can include Parse, ResolveType errors />,

src/libraries/Microsoft.PowerFx.Core/Public/Engine.cs

@@ -546,10 +546,10 @@ namespace Microsoft.PowerFx
             return ExpressionLocalizationHelper.ConvertExpression(expressionText, ruleScope, GetDefaultBindingConfig(), CreateResolverInternal(symbolTable), CreateBinderGlue(), culture, Config.Features, toDisplay: true);
         }
 
-        internal void AddUserDefinedFunction(string script, CultureInfo parseCulture = null, ReadOnlySymbolTable symbolTable = null, bool allowSideEffects = false)
+        public DefinitionsCheckResult AddUserDefinedFunction(string script, CultureInfo parseCulture = null, ReadOnlySymbolTable symbolTable = null, bool allowSideEffects = false)
         {
             var engineTypesAndFunctions = ReadOnlySymbolTable.Compose(PrimitiveTypes, SupportedFunctions);
-            Config.SymbolTable.AddUserDefinedFunction(script, parseCulture, engineTypesAndFunctions, symbolTable, allowSideEffects);
+            return Config.SymbolTable.AddUserDefinedFunction(script, parseCulture, engineTypesAndFunctions, symbolTable, allowSideEffects);
         }
     }
 }

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

@@ -460,15 +460,18 @@ namespace Microsoft.PowerFx
 
             foreach (var udf in udfs)
             {
-                Config.SymbolTable.AddFunction(udf);
                 var binding = udf.BindBody(nameResolver, new Glue2DocumentBinderGlue(), BindingConfig.Default, Config.Features);
 
                 List<TexlError> bindErrors = new List<TexlError>();
 
-                if (binding.ErrorContainer.GetErrors(ref errors))
+                if (binding.ErrorContainer.GetErrors(ref bindErrors))
                 {
                     sb.AppendLine(string.Join(", ", bindErrors.Select(err => err.ToString())));
                 }
+                else
+                {
+                    Config.SymbolTable.AddFunction(udf);
+                }
             }
 
             if (sb.Length > 0)

src/libraries/Microsoft.PowerFx.Repl/Repl.cs

@@ -34,6 +34,9 @@ namespace Microsoft.PowerFx
         // Allow repl to create new definitions, such as Set(). 
         public bool AllowSetDefinitions { get; set; }
 
+        // Allow repl to create new UserDefinedFunctions. 
+        public bool AllowUserDefinedFunctions { get; set; }
+
         // Do we print each command before evaluation?
         // Useful if we're running a file and are debugging, or if input UI is separated from output UI. 
         public bool Echo { get; set; } = false;
@@ -405,6 +408,30 @@ namespace Microsoft.PowerFx
             var errors = check.ApplyErrors();
             if (!check.IsSuccess)
             {
+                var definitionsCheckResult = new DefinitionsCheckResult();
+
+                definitionsCheckResult.SetText(expression, this.ParserOptions)
+                    .ApplyParseErrors();
+
+                if (this.AllowUserDefinedFunctions && definitionsCheckResult.IsSuccess && definitionsCheckResult.ContainsUDF)
+                {
+                    var defCheckResult = this.Engine.AddUserDefinedFunction(expression, this.ParserOptions.Culture, extraSymbolTable);
+
+                    if (!defCheckResult.IsSuccess)
+                    {
+                        foreach (var error in defCheckResult.Errors)
+                        {
+                            var kind = error.IsWarning ? OutputKind.Warning : OutputKind.Error;
+                            var msg = error.ToString();
+
+                            await this.Output.WriteLineAsync(lineError + msg, kind, cancel)
+                                .ConfigureAwait(false);
+                        }
+                    }
+
+                    return new ReplResult();
+                }
+
                 foreach (var error in check.Errors)
                 {
                     var kind = error.IsWarning ? OutputKind.Warning : OutputKind.Error;

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

@@ -425,7 +425,7 @@ namespace Microsoft.PowerFx.Core.Tests
             // Empty symbol table doesn't get builtins. 
             var st = SymbolTable.WithPrimitiveTypes();
             st.AddUserDefinedFunction("Foo1(x: Number): Number = x;"); // ok 
-            Assert.Throws<InvalidOperationException>(() => st.AddUserDefinedFunction("Foo2(x: Number): Number = Abs(x);"));
+            Assert.False(st.AddUserDefinedFunction("Foo2(x: Number): Number = Abs(x);").IsSuccess);
         }
 
         // Show definitions on public symbol tables

src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs

@@ -447,11 +447,6 @@ namespace Microsoft.PowerFx.Tests
             "func1(x:Number/*comment*/): Number = x * 10;\nfunc2(x:Number): Number = y1 * 10;",
             null,
             true)]
-        [InlineData(
-            "foo(x:Number):Number = If(x=0,foo(1),If(x=1,foo(2),If(x=2,Float(2))));",
-            "foo(Float(0))",
-            false,
-            2.0)]
         [InlineData(
             "foo():Blank = foo();",
             "foo()",
@@ -472,7 +467,11 @@ namespace Microsoft.PowerFx.Tests
             false,
             14.0)]
 
-        // Recursive calls are not allowed
+        // Recursive calls are not allowed
+        [InlineData(
+            "foo(x:Number):Number = If(x=0,foo(1),If(x=1,foo(2),If(x=2,Float(2))));",
+            "foo(Float(0))",
+            true)]
         [InlineData(
             "hailstone(x:Number):Number = If(Not(x = 1), If(Mod(x, 2)=0, hailstone(x/2), hailstone(3*x+1)), x);",
             "hailstone(Float(192))",
@@ -574,7 +573,7 @@ namespace Microsoft.PowerFx.Tests
         {
             var engine = new RecalcEngine();
 
-            Assert.Throws<InvalidOperationException>(() => engine.AddUserDefinedFunction(script, CultureInfo.InvariantCulture));
+            Assert.False(engine.AddUserDefinedFunction(script, CultureInfo.InvariantCulture).IsSuccess);
         }
 
         // Overloads and conflict 
@@ -651,30 +650,32 @@ namespace Microsoft.PowerFx.Tests
             "F1(x:Number) : Boolean = { Set(a, x); Today(); };",
             null,
             true,
-            "AddUserDefinedFunction",
+            "ErrUDF_ReturnTypeDoesNotMatch",
             0)]
 
-        public void ImperativeUserDefinedFunctionTest(string udfExpression, string expression, bool expectedError, string expectedMethodFailure, double expected)
+        public void ImperativeUserDefinedFunctionTest(string udfExpression, string expression, bool expectedError, string errorKey, double expected)
         {
             var config = new PowerFxConfig();
             config.EnableSetFunction();
             var recalcEngine = new RecalcEngine(config);
             recalcEngine.UpdateVariable("a", 1m);
+
+            var definitionsCheckResult = recalcEngine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true);
 
-            try
-            {
-                recalcEngine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true);
+            if (!expectedError)
+            { 
+                Assert.True(definitionsCheckResult.IsSuccess);
 
                 var result = recalcEngine.Eval(expression, options: _opts);
                 var fvExpected = FormulaValue.New(expected);
 
                 Assert.Equal(fvExpected.AsDecimal(), result.AsDecimal());
-                Assert.False(expectedError);
             }
-            catch (Exception ex)
+            else 
             {
-                Assert.True(expectedError, ex.Message);
-                Assert.Contains(expectedMethodFailure, ex.StackTrace);
+                Assert.False(definitionsCheckResult.IsSuccess);
+                Assert.Single(definitionsCheckResult.Errors);
+                Assert.Contains(definitionsCheckResult.Errors, err => err.MessageKey == errorKey);
             }
         }
 
@@ -708,7 +709,7 @@ namespace Microsoft.PowerFx.Tests
 
             var recalcEngine = new RecalcEngine(config);
 
-            recalcEngine.AddUserDefinedFunction("A():MyDataSourceTableType = Filter(MyDataSource, Value > 10);C():MyDataSourceTableType = A(); B():MyDataSourceTableType = Filter(C(), Value > 11); D():MyDataSourceTableType = { Filter(B(), Value > 12); }; E():Void = { E(); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true);
+            recalcEngine.AddUserDefinedFunction("A():MyDataSourceTableType = Filter(MyDataSource, Value > 10);C():MyDataSourceTableType = A(); B():MyDataSourceTableType = Filter(C(), Value > 11); D():MyDataSourceTableType = { Filter(B(), Value > 12); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true);
             var func = recalcEngine.Functions.WithName("A").First() as UserDefinedFunction;
 
             Assert.True(func.IsAsync);
@@ -730,11 +731,8 @@ namespace Microsoft.PowerFx.Tests
             Assert.True(func.IsAsync);
             Assert.True(!func.IsDelegatable);
 
-            func = recalcEngine.Functions.WithName("E").First() as UserDefinedFunction;
-
-            // Imperative function is not delegable
-            // E():Void = { E() }; ---> binding will be null so no attempt to get datasource should happen
-            Assert.True(!func.IsDelegatable);
+            // Binding fails for recursive definitions and hence function is not added.
+            Assert.False(recalcEngine.AddUserDefinedFunction("E():Void = { E(); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true).IsSuccess);
         }
 
         // Binding to inner functions does not impact outer functions. 
@@ -1836,7 +1834,7 @@ namespace Microsoft.PowerFx.Tests
             }
             else
             {
-                Assert.Throws<InvalidOperationException>(() => recalcEngine.AddUserDefinedFunction(udf, CultureInfo.InvariantCulture, extraSymbols, true));
+                Assert.False(recalcEngine.AddUserDefinedFunction(udf, CultureInfo.InvariantCulture, extraSymbols, true).IsSuccess);
             }
         }
 

src/tests/Microsoft.PowerFx.Repl.Tests.Shared/ReplTests.cs

@@ -34,6 +34,8 @@ namespace Microsoft.PowerFx.Repl.Tests
                 Engine = engine,
                 Output = _output,
                 AllowSetDefinitions = true,
+                AllowUserDefinedFunctions = true,
+                ParserOptions = new ParserOptions() { AllowsSideEffects = true }
             };
         }
 
@@ -268,6 +270,29 @@ Notify(z)
             Assert.True(log.Length > 0);
         }
 
+        [Fact]
+        public void UserDefinedFunctions()
+        {
+            _repl.HandleLine("F(x: Number): Number = x;");
+            _repl.HandleLine("F(42)");
+            var log = _output.Get(OutputKind.Repl);
+            Assert.Equal("42", log);
+
+            // we do not have a clear semantics defined yet for the below test
+            // should be addressed in future
+            /*
+            _repl.HandleLine("F(x: Text): Text = x;");
+            var error1 = _output.Get(OutputKind.Error);
+            Assert.Equal("Error 0-1: Function F is already defined.", error1);
+            */
+
+            _repl.HandleLine("G(x: Currency): Currency = x;");
+            var error2 = _output.Get(OutputKind.Error);
+            Assert.Equal(
+                @"Error 5-13: Unknown type Currency.
+Error 16-24: Unknown type Currency.", error2);
+        }
+
         // test that Exit() informs the host that an exit has been requested
         [Fact]
         public void Exit()

src/tools/Repl/Program.cs

@@ -40,6 +40,9 @@ namespace Microsoft.PowerFx
         private const string OptionTextFirst = "TextFirst";
         private static bool _textFirst = false;
 
+        private const string OptionUDF = "UserDefinedFunctions";
+        private static bool _enableUDFs = true;
+
         private static readonly Features _features = Features.PowerFxV1;
 
         private static StandardFormatter _standardFormatter;
@@ -64,7 +67,8 @@ namespace Microsoft.PowerFx
                 { OptionPowerFxV1, OptionPowerFxV1 },
                 { OptionHashCodes, OptionHashCodes },
                 { OptionStackTrace, OptionStackTrace },
-                { OptionTextFirst, OptionTextFirst }
+                { OptionTextFirst, OptionTextFirst },
+                { OptionUDF, OptionUDF },
             };
 
             foreach (var featureProperty in typeof(Features).GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic))
@@ -133,6 +137,7 @@ namespace Microsoft.PowerFx
                 this.HelpProvider = new MyHelpProvider();
 
                 this.AllowSetDefinitions = true;
+                this.AllowUserDefinedFunctions = _enableUDFs;
                 this.EnableSampleUserObject();
                 this.AddPseudoFunction(new IRPseudoFunction());
                 this.AddPseudoFunction(new SuggestionsPseudoFunction());
@@ -255,6 +260,7 @@ namespace Microsoft.PowerFx
                 sb.Append(CultureInfo.InvariantCulture, $"{"LargeCallDepth:",-42}{_largeCallDepth}\n");
                 sb.Append(CultureInfo.InvariantCulture, $"{"StackTrace:",-42}{_stackTrace}\n");
                 sb.Append(CultureInfo.InvariantCulture, $"{"TextFirst:",-42}{_textFirst}\n");
+                sb.Append(CultureInfo.InvariantCulture, $"{"UserDefinedFunctions:",-42}{_enableUDFs}\n");
 
                 foreach (var prop in typeof(Features).GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic))
                 {
@@ -303,6 +309,11 @@ namespace Microsoft.PowerFx
                     return BooleanValue.New(_stackTrace);
                 }
 
+                if (string.Equals(option.Value, OptionUDF, StringComparison.OrdinalIgnoreCase))
+                {
+                    return BooleanValue.New(_enableUDFs);
+                }
+
                 return FormulaValue.NewError(new ExpressionError()
                 {
                     Kind = ErrorKind.InvalidArgument,
@@ -343,6 +354,13 @@ namespace Microsoft.PowerFx
                     return value;
                 }
 
+                if (string.Equals(option.Value, OptionUDF, StringComparison.OrdinalIgnoreCase))
+                {
+                    _enableUDFs = value.Value;
+                    _reset = true;
+                    return value;
+                }
+
                 if (string.Equals(option.Value, OptionLargeCallDepth, StringComparison.OrdinalIgnoreCase))
                 {
                     _largeCallDepth = value.Value;
@@ -441,6 +459,9 @@ Options.PowerFxV1
 Options.None
     Removed all the feature flags, which is even less than Canvas uses.
 
+Options.EnableUDFs
+    Enables UserDefinedFunctions to be added.
+
 ";
 
                     await WriteAsync(repl, msg, cancel)