From cc8c3b1b4428ed86febaa3750c5a9fa6d8907a80 Mon Sep 17 00:00:00 2001 From: Mike Stall Date: Wed, 22 Jan 2025 13:54:58 -0800 Subject: [PATCH] Unify IAsyncTexlFunction (#2812) Address #2751. This unifies most of them. To scope this PR, there are subissues on 2751 for remaining issues. We have a plethora of IAsyncTexlFunction* interfaces. Want to consolidate on 1, and make it public. This Ttake the parameters on the various IAsyncTexlFunction* overloads and move them as properties on a new public `FunctionInvokerInfo` class. And then have a new IAsyncTexlFunction* that takes the invoker. And remove all the others, migrating them onto this - and fixing the various bugs/hacks along the way that would impede migration. Most fundamentally, this FunctionInvokerInfo must have access to interpreter state and so must live in the interpreter. (Whereas some of the IAsyncFunction* interfaces live in core). Successfully removes several here so we get a net reduction, and shows a path to remove the rest. This problem quickly touches other problems: - **How to map from the TexlFunction to the invoker**. This is made more complicated because we register on PowerFxConfig, but that lives in Fx.Core and can't refer to interpreter implementations. - **dll layering around Fx.Json**: Some function implementations live in Fx.Json, but that specifically _does not_ depend on interpreter (because it is included by PowerApps). - **Standard Pipeline**: how to handle default args, common error handling scenarios, etc. Today, that's still in interpreter. Whereas we really want those checks to be in the IR and shared across front-ends (and available to designer). - **Split up Library.cs and class** - we have one giant class, partial over many files, containing 100s of function impls. Just give each impl its own file, similar to how we did for TexlFunction and the static declarations. - **lack of unit testing**: Today, most of our interpreter coverage comes from .txt files. But we should have C# unit tests on interpreter classes that provide coverage on core classes, like EvalVisitor. Some related prereq fixes that will directly help here: - Can we remove runner/context from Join? and leverage LambdaValue - which already closes over these. - fix Json layering problem. - IsType/AsType _UO shouldn't live in Json. They should work on any UO. - Remove _additionalFunctions from the serviceProvider. --------- Co-authored-by: Mike --- .../Texl/ConnectorTexlFunction.cs | 8 +- .../Functions/IAsyncTexlFunction.cs | 1 + .../Functions/IAsyncTexlFunction3.cs | 16 -- .../Functions/IAsyncTexlFunction4.cs | 1 + .../Functions/IAsyncTexlFunction5.cs | 4 +- .../Public/Config/PowerFxConfig.cs | 1 + .../CustomFunction/CustomTexlFunction.cs | 7 +- .../EvalVisitor.cs | 190 ++++++++++++------ .../Functions/FileInfoImpl.cs | 7 +- .../Functions/FunctionInvokeInfo.cs | 79 ++++++++ .../Functions/IAsyncConnectorTexlFunction.cs | 16 -- .../Functions/IAsyncTexlFunction2.cs | 18 -- .../Functions/IAsyncTexlFunctionJoin.cs | 18 -- .../Functions/IFunctionInvoker.cs | 17 ++ .../Functions/LibraryMutation.cs | 77 ++++--- .../Functions/LibraryTable.cs | 16 +- .../Values/LambdaFormulaValue.cs | 2 + .../ClearFunctionTests.cs | 5 +- .../EvalVisitorTests.cs | 50 +++++ .../PatchFunctionTests.cs | 12 +- .../RecalcEngineTests.cs | 18 +- 21 files changed, 383 insertions(+), 180 deletions(-) delete mode 100644 src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction3.cs create mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/FunctionInvokeInfo.cs delete mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncConnectorTexlFunction.cs delete mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunction2.cs delete mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunctionJoin.cs create mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/IFunctionInvoker.cs create mode 100644 src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/EvalVisitorTests.cs diff --git a/src/libraries/Microsoft.PowerFx.Connectors/Texl/ConnectorTexlFunction.cs b/src/libraries/Microsoft.PowerFx.Connectors/Texl/ConnectorTexlFunction.cs index b139be1570..12effc3ce1 100644 --- a/src/libraries/Microsoft.PowerFx.Connectors/Texl/ConnectorTexlFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Connectors/Texl/ConnectorTexlFunction.cs @@ -12,13 +12,14 @@ using Microsoft.PowerFx.Core.Localization; using Microsoft.PowerFx.Core.Types; using Microsoft.PowerFx.Core.Utils; +using Microsoft.PowerFx.Functions; using Microsoft.PowerFx.Intellisense; using Microsoft.PowerFx.Types; using static Microsoft.PowerFx.Connectors.ConnectorHelperFunctions; namespace Microsoft.PowerFx.Connectors { - internal class ConnectorTexlFunction : TexlFunction, IAsyncConnectorTexlFunction, IHasUnsupportedFunctions + internal class ConnectorTexlFunction : TexlFunction, IFunctionInvoker, IHasUnsupportedFunctions { public ConnectorFunction ConnectorFunction { get; } @@ -85,8 +86,11 @@ public override bool TryGetParamDescription(string paramName, out string paramDe public override bool HasSuggestionsForParam(int argumentIndex) => argumentIndex <= MaxArity; - public async Task InvokeAsync(FormulaValue[] args, IServiceProvider serviceProvider, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + FormulaValue[] args = invokeInfo.Args.ToArray(); // https://github.com/microsoft/Power-Fx/issues/2817 + var serviceProvider = invokeInfo.FunctionServices; + cancellationToken.ThrowIfCancellationRequested(); BaseRuntimeConnectorContext runtimeContext = serviceProvider.GetService(typeof(BaseRuntimeConnectorContext)) as BaseRuntimeConnectorContext ?? throw new InvalidOperationException("RuntimeConnectorContext is missing from service provider"); diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction.cs index cb15c3e963..0ecc98c78e 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction.cs @@ -8,6 +8,7 @@ namespace Microsoft.PowerFx.Core.Functions { // A Texl function capable of async invokes. + // This only lives in Core to enable Fx.Json funcs impl (which doesn't depend on interpreter). internal interface IAsyncTexlFunction { Task InvokeAsync(FormulaValue[] args, CancellationToken cancellationToken); diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction3.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction3.cs deleted file mode 100644 index e4ac6bfbe8..0000000000 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction3.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.PowerFx.Types; - -namespace Microsoft.PowerFx.Core.Functions -{ - // A Texl function capable of async invokes, using IRContext. - internal interface IAsyncTexlFunction3 - { - Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken); - } -} diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction4.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction4.cs index c68908571f..0ab689bd91 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction4.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction4.cs @@ -9,6 +9,7 @@ namespace Microsoft.PowerFx.Core.Functions { // A Texl function capable of async invokes, using TimeZoneInfo and IRContext. + // Remove this: https://github.com/microsoft/Power-Fx/issues/2818 internal interface IAsyncTexlFunction4 { Task InvokeAsync(TimeZoneInfo timezoneInfo, FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken); diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction5.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction5.cs index 9dff48e73a..69b408d9a1 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction5.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction5.cs @@ -7,8 +7,10 @@ using Microsoft.PowerFx.Types; namespace Microsoft.PowerFx.Core.Functions -{ +{ // Texl function interface with IServiceProvider + // Only product impl is JsonFunctionImpl. + // Remove this: https://github.com/microsoft/Power-Fx/issues/2818 internal interface IAsyncTexlFunction5 { Task InvokeAsync(IServiceProvider runtimeServiceProvider, FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken); diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/Config/PowerFxConfig.cs b/src/libraries/Microsoft.PowerFx.Core/Public/Config/PowerFxConfig.cs index d873582a56..8942026dde 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/Config/PowerFxConfig.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/Config/PowerFxConfig.cs @@ -52,6 +52,7 @@ public SymbolTable SymbolTable set => _symbolTable = value; } + // Remove this: https://github.com/microsoft/Power-Fx/issues/2821 internal readonly Dictionary AdditionalFunctions = new (); [Obsolete("Use Config.EnumStore or symboltable directly")] diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/CustomFunction/CustomTexlFunction.cs b/src/libraries/Microsoft.PowerFx.Interpreter/CustomFunction/CustomTexlFunction.cs index d0db543958..44c38a1f17 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/CustomFunction/CustomTexlFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/CustomFunction/CustomTexlFunction.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Numerics; using System.Threading; using System.Threading.Tasks; @@ -22,7 +23,7 @@ namespace Microsoft.PowerFx /// /// Internal adapter for adding custom functions. /// - internal class CustomTexlFunction : TexlFunction + internal class CustomTexlFunction : TexlFunction, IFunctionInvoker { public Func> _impl; @@ -53,8 +54,10 @@ public CustomTexlFunction(DPath ns, string name, FunctionCategories functionCate yield return CustomFunctionUtility.GenerateArgSignature(_argNames, ParamTypes); } - public virtual Task InvokeAsync(IServiceProvider serviceProvider, FormulaValue[] args, CancellationToken cancellationToken) + public virtual Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var serviceProvider = invokeInfo.FunctionServices; + var args = invokeInfo.Args.ToArray(); // remove ToArray: https://github.com/microsoft/Power-Fx/issues/2817 return _impl(serviceProvider, args, cancellationToken); } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/EvalVisitor.cs b/src/libraries/Microsoft.PowerFx.Interpreter/EvalVisitor.cs index 7c93430b9b..621f62d28a 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/EvalVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/EvalVisitor.cs @@ -265,6 +265,100 @@ private async Task TryHandleSetProperty(CallNode node, EvalVisitor return result; } + + // Given a TexlFunction, get the implementation to invoke. + private IFunctionInvoker GetInvoker(TexlFunction func) + { + if (func is IFunctionInvoker invoker) + { + return invoker; + } + + if (func is UserDefinedFunction userDefinedFunc) + { + return new UserDefinedFunctionAdapter(userDefinedFunc); + } + + if (FunctionImplementations.TryGetValue(func, out AsyncFunctionPtr ptr)) + { + return new AsyncFunctionPtrAdapter(ptr); + } + + return null; + } + + // Adapter for AsyncFunctionPtr to common invoker interface. + private class AsyncFunctionPtrAdapter : IFunctionInvoker + { + private readonly AsyncFunctionPtr _ptr; + + public AsyncFunctionPtrAdapter(AsyncFunctionPtr ptr) + { + _ptr = ptr; + } + + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) + { + var args = invokeInfo.Args.ToArray(); + var context = invokeInfo.Context; + var evalVisitor = invokeInfo.Runner; + var irContext = invokeInfo.IRContext; + + var result = await _ptr(evalVisitor, context, irContext, args).ConfigureAwait(false); + + return result; + } + } + + // Adapter for UDF to common invoker. + // This still ensures that *invoking* a UDF has the same semantics as invoking other function calls. + private class UserDefinedFunctionAdapter : IFunctionInvoker + { + private readonly UserDefinedFunction _udf; + + public UserDefinedFunctionAdapter(UserDefinedFunction udf) + { + _udf = udf; + } + + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) + { + var args = invokeInfo.Args.ToArray(); + var context = invokeInfo.Context; + var evalVisitor = invokeInfo.Runner; + + var udfStack = evalVisitor._udfStack; + + UDFStackFrame frame = new UDFStackFrame(_udf, args); + UDFStackFrame framePop = null; + FormulaValue result = null; + + try + { + // Push this so that we have access to args. + udfStack.Push(frame); + + // https://github.com/microsoft/Power-Fx/issues/2822 + // This repeats IRTranslator each time. Do once and save. + (var irnode, _) = _udf.GetIRTranslator(); + + evalVisitor.CheckCancel(); + + result = await irnode.Accept(evalVisitor, context).ConfigureAwait(false); + } + finally + { + framePop = udfStack.Pop(); + } + + if (frame != framePop) + { + throw new Exception("Something went wrong. UDF stack values didn't match."); + } + + return result; + } + } public override async ValueTask Visit(CallNode node, EvalVisitorContext context) { @@ -300,7 +394,8 @@ public override async ValueTask Visit(CallNode node, EvalVisitorCo args[i] = await child.Accept(this, context.IncrementStackDepthCounter()).ConfigureAwait(false); } else - { + { + // This is where Lambdas are created. They close over key values to invoke. args[i] = new LambdaFormulaValue(node.IRContext, child, this, context); } } @@ -308,32 +403,50 @@ public override async ValueTask Visit(CallNode node, EvalVisitorCo var childContext = context.SymbolContext.WithScope(node.Scope); FormulaValue result; + + // Remove this: https://github.com/microsoft/Power-Fx/issues/2821 IReadOnlyDictionary extraFunctions = _services.GetService>(); try { - if (func is IAsyncTexlFunction asyncFunc || extraFunctions?.TryGetValue(func, out asyncFunc) == true) + IFunctionInvoker invoker = GetInvoker(func); + + // Standard invoke path. Make everything go through here. + // Eventually collapse all cases to this. + if (invoker != null) { - result = await asyncFunc.InvokeAsync(args, _cancellationToken).ConfigureAwait(false); + var invokeInfo = new FunctionInvokeInfo + { + Args = args, + FunctionServices = _services, + Runner = this, + Context = context.IncrementStackDepthCounter(childContext), + IRContext = node.IRContext, + }; + + result = await invoker.InvokeAsync(invokeInfo, _cancellationToken).ConfigureAwait(false); } -#pragma warning disable CS0618 // Type or member is obsolete - else if (func is IAsyncTexlFunction2 asyncFunc2) -#pragma warning restore CS0618 // Type or member is obsolete + else if (func is IAsyncTexlFunction asyncFunc) { - result = await asyncFunc2.InvokeAsync(this.GetFormattingInfo(), args, _cancellationToken).ConfigureAwait(false); + result = await asyncFunc.InvokeAsync(args, _cancellationToken).ConfigureAwait(false); } - else if (func is IAsyncTexlFunction3 asyncFunc3) + else if (extraFunctions?.TryGetValue(func, out asyncFunc) == true) { - result = await asyncFunc3.InvokeAsync(node.IRContext.ResultType, args, _cancellationToken).ConfigureAwait(false); + result = await asyncFunc.InvokeAsync(args, _cancellationToken).ConfigureAwait(false); } else if (func is IAsyncTexlFunction4 asyncFunc4) { + // https://github.com/microsoft/Power-Fx/issues/2818 + // This is used for Json() functions. IsType, AsType result = await asyncFunc4.InvokeAsync(TimeZoneInfo, node.IRContext.ResultType, args, _cancellationToken).ConfigureAwait(false); } else if (func is IAsyncTexlFunction5 asyncFunc5) { + // https://github.com/microsoft/Power-Fx/issues/2818 + // This is used for Json() functions. BasicServiceProvider services2 = new BasicServiceProvider(_services); + // Invocation should not get its own provider. if (services2.GetService(typeof(TimeZoneInfo)) == null) { services2.AddService(TimeZoneInfo); @@ -346,63 +459,18 @@ public override async ValueTask Visit(CallNode node, EvalVisitorCo result = await asyncFunc5.InvokeAsync(services2, node.IRContext.ResultType, args, _cancellationToken).ConfigureAwait(false); } - else if (func is IAsyncConnectorTexlFunction asyncConnectorTexlFunction) - { - return await asyncConnectorTexlFunction.InvokeAsync(args, _services, _cancellationToken).ConfigureAwait(false); - } - else if (func is CustomTexlFunction customTexlFunc) - { - // If custom function throws an exception, don't catch it - let it propagate up to the host. - result = await customTexlFunc.InvokeAsync(FunctionServices, args, _cancellationToken).ConfigureAwait(false); - } - else if (func is UserDefinedFunction userDefinedFunc) + else { - UDFStackFrame frame = new UDFStackFrame(userDefinedFunc, args); - UDFStackFrame framePop = null; - - try - { - _udfStack.Push(frame); - - (var irnode, _) = userDefinedFunc.GetIRTranslator(); - - this.CheckCancel(); - - result = await irnode.Accept(this, context).ConfigureAwait(false); - } - finally - { - framePop = _udfStack.Pop(); - } - - if (frame != framePop) - { - throw new Exception("Something went wrong. UDF stack values didn't match."); - } + result = CommonErrors.NotYetImplementedFunctionError(node.IRContext, func.Name); } -#pragma warning disable CS0618 // Type or member is obsolete - - // This is a temporary solution to release the Join function for host that want to use it. - else if (func is IAsyncTexlFunctionJoin asyncJoin) + // https://github.com/microsoft/Power-Fx/issues/2820 + // We should remove this check that limits to just Adapter1, so we apply this check to all impls. + if (invoker is AsyncFunctionPtrAdapter) { - result = await asyncJoin.InvokeAsync(this, context.IncrementStackDepthCounter(childContext), node.IRContext, args).ConfigureAwait(false); - } -#pragma warning restore CS0618 // Type or member is obsolete - else - { - if (FunctionImplementations.TryGetValue(func, out AsyncFunctionPtr ptr)) - { - result = await ptr(this, context.IncrementStackDepthCounter(childContext), node.IRContext, args).ConfigureAwait(false); - - if (!(result.IRContext.ResultType._type == node.IRContext.ResultType._type || result is ErrorValue || result.IRContext.ResultType is BlankType)) - { - throw CommonExceptions.RuntimeMisMatch; - } - } - else + if (!(result.IRContext.ResultType._type == node.IRContext.ResultType._type || result is ErrorValue || result.IRContext.ResultType is BlankType)) { - result = CommonErrors.NotYetImplementedFunctionError(node.IRContext, func.Name); + throw CommonExceptions.RuntimeMisMatch; } } } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FileInfoImpl.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FileInfoImpl.cs index 5ce5fe1f96..dff6e02834 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FileInfoImpl.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FileInfoImpl.cs @@ -6,15 +6,18 @@ using System.Threading.Tasks; using Microsoft.PowerFx.Core.Functions; using Microsoft.PowerFx.Core.Texl.Builtins; +using Microsoft.PowerFx.Functions; using Microsoft.PowerFx.Interpreter; using Microsoft.PowerFx.Types; namespace Microsoft.PowerFx { - internal class FileInfoFunctionImpl : FileInfoFunction, IAsyncTexlFunction3 + internal class FileInfoFunctionImpl : FileInfoFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var arg0 = args[0]; if (arg0 is BlankValue || arg0 is ErrorValue) { diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FunctionInvokeInfo.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FunctionInvokeInfo.cs new file mode 100644 index 0000000000..f59553cac3 --- /dev/null +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FunctionInvokeInfo.cs @@ -0,0 +1,79 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System; +using System.Collections.Generic; +using Microsoft.PowerFx.Core.IR; +using Microsoft.PowerFx.Types; + +namespace Microsoft.PowerFx.Functions +{ + /// + /// The parameters to use with . + /// The information necessary to invoke a function via . + /// The arguments here may be closured over an instance of EvalVisitor/EvalContext, so this + /// is only valid for a specific invocation and can't be reused across invokes. + /// + [ThreadSafeImmutable] + public class FunctionInvokeInfo + { + /// + /// The arguments passed to this function. + /// These may be closed over context and specific to this invocation, so they should + /// not be saved or used outside of this invocation. + /// + public IReadOnlyList Args { get; init; } + + /// + /// The expected return type of this function. This is computed by the binder. + /// Some functions are polymorphic and the return type depends on the intput argument types. + /// + public FormulaType ReturnType => this.IRContext.ResultType; + + /// + /// services for function invocation. This is the set from . + /// + public IServiceProvider FunctionServices { get; init; } + + // Since every method impl would get this, consider add other useful operators on here, like: + // - CheckCancel? + // - error checks, blank checks. + + // Can we find a way to get rid of these ones? Keep internal to limit usage. + // Keep internal until we kind a way to remove. + #region Remove these + + // IrContext has a mutability flag. + internal IRContext IRContext { get; init; } + + // https://github.com/microsoft/Power-Fx/issues/2819 + // Get rid of these... Capture in them in the closure of a lambdaValue. + internal EvalVisitor Runner { get; init; } + + internal EvalVisitorContext Context { get; init; } + #endregion + + // Since this is immutable, clone to get adjusted parameters. + public FunctionInvokeInfo CloneWith(IReadOnlyList newArgs) + { + return new FunctionInvokeInfo + { + Args = newArgs, + FunctionServices = this.FunctionServices, + IRContext = this.IRContext, + Runner = this.Runner, + Context = this.Context + }; + } + + // Helper to create simple case, primarily for testing. + internal static FunctionInvokeInfo New(FormulaType returnType, params FormulaValue[] args) + { + return new FunctionInvokeInfo + { + Args = args, + IRContext = IRContext.NotInSource(returnType) + }; + } + } +} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncConnectorTexlFunction.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncConnectorTexlFunction.cs deleted file mode 100644 index 3fe07d7bbe..0000000000 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncConnectorTexlFunction.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.PowerFx.Types; - -namespace Microsoft.PowerFx.Core.Functions -{ - // A Texl function capable of async invokes. - internal interface IAsyncConnectorTexlFunction - { - Task InvokeAsync(FormulaValue[] args, IServiceProvider context, CancellationToken cancellationToken); - } -} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunction2.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunction2.cs deleted file mode 100644 index fa2313c38a..0000000000 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunction2.cs +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.PowerFx.Functions; -using Microsoft.PowerFx.Types; - -namespace Microsoft.PowerFx.Core.Functions -{ - // A Texl function capable of async invokes. - [Obsolete("This interface is obsolete and will be removed in a future release. Please use IAsyncConnectorTexlFunction instead.")] - internal interface IAsyncTexlFunction2 - { - Task InvokeAsync(FormattingInfo context, FormulaValue[] args, CancellationToken cancellationToken); - } -} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunctionJoin.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunctionJoin.cs deleted file mode 100644 index cd9a2b08cf..0000000000 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunctionJoin.cs +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.PowerFx.Core.IR; -using Microsoft.PowerFx.Functions; -using Microsoft.PowerFx.Types; - -namespace Microsoft.PowerFx.Core.Functions -{ - [Obsolete("This interface is temporary and its meant only to support Join function. It will be soon removed.")] - internal interface IAsyncTexlFunctionJoin - { - Task InvokeAsync(EvalVisitor runner, EvalVisitorContext context, IRContext irContext, FormulaValue[] args); - } -} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IFunctionInvoker.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IFunctionInvoker.cs new file mode 100644 index 0000000000..506e07568f --- /dev/null +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IFunctionInvoker.cs @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Threading; +using System.Threading.Tasks; +using Microsoft.PowerFx.Types; + +namespace Microsoft.PowerFx.Functions +{ + /// + /// Invoker to execute a function via the interpreter. + /// + public interface IFunctionInvoker + { + Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken); + } +} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs index cc8c2599d5..a6e986a855 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs @@ -15,10 +15,12 @@ namespace Microsoft.PowerFx.Core.Texl.Builtins { // Patch(dataSource:*[], Record, Updates1, Updates2,…) - internal class PatchImpl : PatchFunction, IAsyncTexlFunction3 + internal class PatchImpl : PatchFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var arg0 = args[0]; if (args[0] is LambdaFormulaValue arg0lazy) @@ -57,10 +59,12 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ // If arg1 is pure PFx record, it will return a runtime not supported error. // Patch(DS, record_with_keys_and_updates) - internal class PatchSingleRecordImpl : PatchSingleRecordFunction, IAsyncTexlFunction3 + internal class PatchSingleRecordImpl : PatchSingleRecordFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var arg0 = args[0]; if (args[0] is LambdaFormulaValue arg0lazy) @@ -93,10 +97,12 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ } // Patch(DS, table_of_rows, table_of_updates) - internal class PatchAggregateImpl : PatchAggregateFunction, IAsyncTexlFunction3 + internal class PatchAggregateImpl : PatchAggregateFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var arg0 = args[0]; if (args[0] is LambdaFormulaValue arg0lazy) @@ -114,7 +120,7 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ if (arg1Rows.Count() != arg2Rows.Count()) { - return CommonErrors.InvalidArgumentError(IRContext.NotInSource(irContext), RuntimeStringResources.ErrAggregateArgsSameNumberOfRecords); + return CommonErrors.InvalidArgumentError(IRContext.NotInSource(invokeInfo.ReturnType), RuntimeStringResources.ErrAggregateArgsSameNumberOfRecords); } List> resultRows = new List>(); @@ -170,10 +176,12 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ // If arg1 is pure PFx record, it will return a runtime not supported error. // Patch(DS, table_of_rows_with_updates) - internal class PatchAggregateSingleTableImpl : PatchAggregateSingleTableFunction, IAsyncTexlFunction3 + internal class PatchAggregateSingleTableImpl : PatchAggregateSingleTableFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var arg0 = args[0]; if (args[0] is LambdaFormulaValue arg0lazy) @@ -229,28 +237,34 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ } } - internal class CollectImpl : CollectFunction, IAsyncTexlFunction3 + internal class CollectImpl : CollectFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { - return await CollectProcess.Process(irContext, args, cancellationToken).ConfigureAwait(false); + var args = invokeInfo.Args; + var returnType = invokeInfo.ReturnType; + + return await CollectProcess.Process(returnType, args, cancellationToken).ConfigureAwait(false); } } - internal class CollectScalarImpl : CollectScalarFunction, IAsyncTexlFunction3 + internal class CollectScalarImpl : CollectScalarFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { - return await CollectProcess.Process(irContext, args, cancellationToken).ConfigureAwait(false); + var args = invokeInfo.Args; + var returnType = invokeInfo.ReturnType; + + return await CollectProcess.Process(returnType, args, cancellationToken).ConfigureAwait(false); } } internal class CollectProcess { - internal static async Task Process(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + internal static async Task Process(FormulaType irContext, IReadOnlyList args, CancellationToken cancellationToken) { FormulaValue arg0; - var argc = args.Length; + var argc = args.Count; // Need to check if the Lazy first argument has been evaluated since it may have already been // evaluated in the ClearCollect case. @@ -341,10 +355,13 @@ internal static async Task Process(FormulaType irContext, FormulaV } // Clear(collection_or_table) - internal class ClearImpl : ClearFunction, IAsyncTexlFunction3 + internal class ClearImpl : ClearFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var returnType = invokeInfo.ReturnType; + if (args[0] is ErrorValue errorValue) { return errorValue; @@ -352,13 +369,13 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ if (args[0] is BlankValue) { - return irContext == FormulaType.Void ? FormulaValue.NewVoid() : FormulaValue.NewBlank(FormulaType.Boolean); + return returnType == FormulaType.Void ? FormulaValue.NewVoid() : FormulaValue.NewBlank(FormulaType.Boolean); } var datasource = (TableValue)args[0]; var ret = await datasource.ClearAsync(cancellationToken).ConfigureAwait(false); - if (irContext == FormulaType.Void) + if (returnType == FormulaType.Void) { return ret.IsError ? FormulaValue.NewError(ret.Error.Errors, FormulaType.Void) : FormulaValue.NewVoid(); } @@ -370,18 +387,22 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ } // ClearCollect(table_or_collection, table|record, ...) - internal class ClearCollectImpl : ClearCollectFunction, IAsyncTexlFunction3 + internal class ClearCollectImpl : ClearCollectFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args.ToArray(); + if (args[0] is LambdaFormulaValue arg0lazy) { args[0] = await arg0lazy.EvalAsync().ConfigureAwait(false); } + invokeInfo = invokeInfo.CloneWith(args); + var clearFunction = new ClearImpl(); - var cleared = await clearFunction.InvokeAsync(FormulaType.Void, args, cancellationToken).ConfigureAwait(false); + var cleared = await clearFunction.InvokeAsync(invokeInfo, cancellationToken).ConfigureAwait(false); if (cleared is ErrorValue) { @@ -390,16 +411,16 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ var collectFunction = new CollectImpl(); - return await collectFunction.InvokeAsync(irContext, args, cancellationToken).ConfigureAwait(false); + return await collectFunction.InvokeAsync(invokeInfo, cancellationToken).ConfigureAwait(false); } } // ClearCollect(table_or_collection, scalar, ...) - internal class ClearCollectScalarImpl : ClearCollectScalarFunction, IAsyncTexlFunction3 + internal class ClearCollectScalarImpl : ClearCollectScalarFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { - return await new ClearCollectImpl().InvokeAsync(irContext, args, cancellationToken).ConfigureAwait(false); + return await new ClearCollectImpl().InvokeAsync(invokeInfo, cancellationToken).ConfigureAwait(false); } } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs index a89e8a0f98..e3896aa133 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.PowerFx.Core.Entities; using Microsoft.PowerFx.Core.Functions; @@ -524,8 +525,13 @@ public static async ValueTask FilterTable(EvalVisitor runner, Eval } // Join(t1, t2, LeftRecord.Id = RightRecord.RefId, JoinType.Inner) - public static async ValueTask JoinTables(EvalVisitor runner, EvalVisitorContext context, IRContext irContext, FormulaValue[] args) + public static async ValueTask JoinTables(FunctionInvokeInfo invokeInfo) { + var args = invokeInfo.Args; + var runner = invokeInfo.Runner; + var context = invokeInfo.Context; + var irContext = invokeInfo.IRContext; + if (args[0] is not TableValue leftTable) { return args[0]; @@ -1575,15 +1581,13 @@ public static NamedLambda[] Parse(FormulaValue[] args) } } -#pragma warning disable CS0618 // Type or member is obsolete - internal class JoinImpl : JoinFunction, IAsyncTexlFunctionJoin -#pragma warning restore CS0618 // Type or member is obsolete + internal class JoinImpl : JoinFunction, IFunctionInvoker { public override Type DeclarationType => typeof(JoinFunction); - public async Task InvokeAsync(EvalVisitor runner, EvalVisitorContext context, IRContext irContext, FormulaValue[] args) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { - return await Library.JoinTables(runner, context, irContext, args).ConfigureAwait(false); + return await Library.JoinTables(invokeInfo).ConfigureAwait(false); } } } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Values/LambdaFormulaValue.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Values/LambdaFormulaValue.cs index 2380b874eb..bd8e2f506e 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Values/LambdaFormulaValue.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Values/LambdaFormulaValue.cs @@ -34,6 +34,8 @@ public async ValueTask EvalAsync() return await EvalInRowScopeAsync(_context).ConfigureAwait(false); } + // Can we remove this and replace with safer implementations? + // https://github.com/microsoft/Power-Fx/issues/2819 public async ValueTask EvalInRowScopeAsync(EvalVisitorContext context) { _runner.CheckCancel(); diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/ClearFunctionTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/ClearFunctionTests.cs index cb9e2a267c..ef5e545dda 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/ClearFunctionTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/ClearFunctionTests.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using Microsoft.PowerFx.Core.Tests; using Microsoft.PowerFx.Core.Texl.Builtins; +using Microsoft.PowerFx.Functions; using Microsoft.PowerFx.Types; using Xunit; @@ -34,7 +35,7 @@ public async Task CheckArgsTestAsync() foreach (var arg in faultyArs) { - var result = await function.InvokeAsync(FormulaType.Void, new FormulaValue[] { arg }, CancellationToken.None); + var result = await function.InvokeAsync(FunctionInvokeInfo.New(FormulaType.Void, arg), CancellationToken.None); if (arg is ErrorValue) { @@ -66,7 +67,7 @@ public async Task CheckArgsTestAsync_V1CompatDisabled() foreach (var arg in faultyArs) { - var result = await function.InvokeAsync(FormulaType.Boolean, new FormulaValue[] { arg }, CancellationToken.None); + var result = await function.InvokeAsync(FunctionInvokeInfo.New(FormulaType.Boolean, arg), CancellationToken.None); if (arg is ErrorValue) { diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/EvalVisitorTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/EvalVisitorTests.cs new file mode 100644 index 0000000000..38deb077a6 --- /dev/null +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/EvalVisitorTests.cs @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Text; +using Xunit; + +namespace Microsoft.PowerFx.Interpreter.Tests +{ + // This file should eventually get 100% code-coverage on EvalVistor. + // Exercise via RecalcEngine + public class EvalVistitorTests + { + [Theory] + [InlineData( + "Join([10,20],[1,2], LeftRecord.Value = RightRecord.Value*10, JoinType.Left, RightRecord.Value As R2)", "Table({R2:1,Value:10},{R2:2,Value:20})")] + [InlineData( + "Filter([10,30,20], ThisRecord.Value > 15)", + "Table({Value:30},{Value:20})")] + public void Dispatch(string expr, string expected) + { + string answer = Run(expr); + + Assert.Equal(expected, answer); + } + + private static readonly RecalcEngine _engine = NewEngine(); + + private static RecalcEngine NewEngine() + { + var config = new PowerFxConfig(); +#pragma warning disable CS0618 // Type or member is obsolete + config.EnableJoinFunction(); +#pragma warning restore CS0618 // Type or member is obsolete + var engine = new RecalcEngine(config); + + return engine; + } + + private static string Run(string expr) + { + var result = _engine.Eval(expr); + + var sb = new StringBuilder(); + result.ToExpression(sb, new FormulaValueSerializerSettings { UseCompactRepresentation = true }); + + var answer = sb.ToString(); + return answer; + } + } +} diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PatchFunctionTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PatchFunctionTests.cs index 7aa55c7492..0054beb15f 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PatchFunctionTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PatchFunctionTests.cs @@ -5,8 +5,10 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.PowerFx.Core.Functions; +using Microsoft.PowerFx.Core.IR; using Microsoft.PowerFx.Core.Tests; using Microsoft.PowerFx.Core.Texl.Builtins; +using Microsoft.PowerFx.Functions; using Microsoft.PowerFx.Types; using Xunit; @@ -36,8 +38,14 @@ public async Task CheckArgsTestAsync(Type type) innerServices.AddService(Features.PowerFxV1); - var function = Activator.CreateInstance(type) as IAsyncTexlFunction3; - var result = await function.InvokeAsync(FormulaType.Unknown, args, CancellationToken.None); + var function = Activator.CreateInstance(type) as IFunctionInvoker; + var invokeInfo = new FunctionInvokeInfo + { + Args = args, + IRContext = IRContext.NotInSource(FormulaType.Unknown) + }; + + var result = await function.InvokeAsync(invokeInfo, CancellationToken.None); Assert.IsType(result); } diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index 5efafa90d1..ab6a942972 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -61,7 +61,7 @@ public void PublicSurfaceTests() $"{ns}.{nameof(TableMarshallerProvider)}", $"{ns}.{nameof(TypeMarshallerCache)}", $"{ns}.{nameof(TypeMarshallerCacheExtensions)}", - $"{ns}.{nameof(SymbolExtensions)}", + $"{ns}.{nameof(SymbolExtensions)}", $"{nsType}.{nameof(ObjectRecordValue)}", #pragma warning disable CS0618 // Type or member is obsolete $"{nsType}.{nameof(QueryableTableValue)}", @@ -71,7 +71,9 @@ public void PublicSurfaceTests() $"{ns}.Interpreter.{nameof(CustomFunctionErrorException)}", $"{ns}.{nameof(TypeCoercionProvider)}", - // Services for functions. + // Services for functions. + "Microsoft.PowerFx.Functions.IFunctionInvoker", + "Microsoft.PowerFx.Functions.FunctionInvokeInfo", $"{ns}.Functions.IRandomService", $"{ns}.Functions.IClockService" }; @@ -1216,8 +1218,10 @@ public TestFunctionMultiply() yield return new[] { TexlStrings.IsBlankArg1 }; } - public override Task InvokeAsync(IServiceProvider serviceProvider, FormulaValue[] args, CancellationToken cancellationToken) - { + public override Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) + { + var args = invokeInfo.Args; + var arg0 = args[0] as NumberValue; var arg1 = args[1] as StringValue; @@ -1239,8 +1243,10 @@ public TestFunctionSubstract() yield return new[] { TexlStrings.IsBlankArg1 }; } - public override Task InvokeAsync(IServiceProvider serviceProvider, FormulaValue[] args, CancellationToken cancellationToken) - { + public override Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) + { + var args = invokeInfo.Args; + var arg0 = args[0] as StringValue; var arg1 = args[1] as NumberValue;