From 213a272105dcf150ca6bd4e50a440169683eb67f Mon Sep 17 00:00:00 2001 From: Greg Lindhorst Date: Wed, 28 Feb 2024 18:26:31 -0800 Subject: [PATCH] Deprecate OptionSetInfo, prepare way for new ChoiceInfo function (#2235) OptionSetInfo only returns one value, the logical name of an option set choice. We'd like to redesign this function to accommodate: 1. The logical name is a text wrapped number for number and Boolean backed option sets coming from Dataverse. We recently added the ability to get at these values with the Value and Boolean functions which is a better way, strongly typed. https://github.com/microsoft/Power-Fx/pull/2230 1. There are other pieces of information that are worth knowing, for example the color of an option set choice, that can't be accommodated with this design. 1. Our preferred design is to return an extensible record of information. We could add an additional second argument with an enum to determine what information to return, with a default value of logical name, but this isn't our first choice. 1. We don't use the term OptionSet any longer in Dataverse, it has been replaced with Choice, which is more consistent with our existing Choices function. As some hosts may still be using this function, it will remain available as a function that can be enabled by calling config.EnableOptionSetInfo. Value/Boolean can be used immediately as a workaround, wrapped with Text if needed. We plan to add a new ChoiceInfo function with a separate PR. --- .../Texl/BuiltinFunctionsCore.cs | 1 - .../Texl/Builtins/OptionSetInfo.cs | 27 ---------- .../Environment/PowerFxConfigExtensions.cs | 7 +++ .../Functions/Library.cs | 11 ---- .../Functions/LibraryText.cs | 7 --- .../Functions/OptionSetInfo.cs | 50 +++++++++++++++++++ .../Docs/DocTests.cs | 1 + .../RecalcEngineTests.cs | 3 ++ src/tools/Repl/Program.cs | 3 ++ 9 files changed, 64 insertions(+), 46 deletions(-) delete mode 100644 src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/OptionSetInfo.cs create mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/OptionSetInfo.cs diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/BuiltinFunctionsCore.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/BuiltinFunctionsCore.cs index 54198afa89..0090b26361 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/BuiltinFunctionsCore.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/BuiltinFunctionsCore.cs @@ -253,7 +253,6 @@ internal class BuiltinFunctionsCore public static readonly TexlFunction Float = _featureGateFunctions.Add(new FloatFunction()); public static readonly TexlFunction Float_UO = _featureGateFunctions.Add(new FloatFunction_UO()); public static readonly TexlFunction IsUTCToday = _featureGateFunctions.Add(new IsUTCTodayFunction()); - public static readonly TexlFunction OptionsSetInfo = _featureGateFunctions.Add(new OptionSetInfoFunction()); public static readonly TexlFunction UTCNow = _featureGateFunctions.Add(new UTCNowFunction()); public static readonly TexlFunction UTCToday = _featureGateFunctions.Add(new UTCTodayFunction()); public static readonly TexlFunction BooleanL = _featureGateFunctions.Add(new BooleanLFunction()); diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/OptionSetInfo.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/OptionSetInfo.cs deleted file mode 100644 index 780bf69578..0000000000 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/OptionSetInfo.cs +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System.Collections.Generic; -using Microsoft.PowerFx.Core.Functions; -using Microsoft.PowerFx.Core.Localization; -using Microsoft.PowerFx.Core.Types; - -namespace Microsoft.PowerFx.Core.Texl.Builtins -{ - internal sealed class OptionSetInfoFunction : BuiltinFunction - { - public override bool IsSelfContained => true; - - public override bool SupportsParamCoercion => false; - - public OptionSetInfoFunction() - : base("OptionSetInfo", TexlStrings.AboutOptionSetInfo, FunctionCategories.Text, DType.String, 0, 1, 1, DType.OptionSetValue) - { - } - - public override IEnumerable GetSignatures() - { - yield return new[] { TexlStrings.AboutOptionSetInfoArg1 }; - } - } -} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Environment/PowerFxConfigExtensions.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Environment/PowerFxConfigExtensions.cs index 8ea79c6a0d..5c5064f8d0 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Environment/PowerFxConfigExtensions.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Environment/PowerFxConfigExtensions.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using Microsoft.PowerFx.Core.Functions; +using Microsoft.PowerFx.Core.Texl.Builtins; using Microsoft.PowerFx.Functions; using Microsoft.PowerFx.Interpreter; @@ -60,5 +61,11 @@ public static void EnableRegExFunctions(this PowerFxConfig config, TimeSpan regE config.AdditionalFunctions.Add(func.Key, func.Value); } } + + [Obsolete("OptionSetInfo function is deprecated. Use the Value function on an option set backed by a number and the Boolean function on an option set backed by a Boolean instead. A new ChoiceInfo function is in the works for access to logical names.")] + public static void EnableOptionSetInfo(this PowerFxConfig powerFxConfig) + { + powerFxConfig.AddFunction(new OptionSetInfoFunction()); + } } } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Library.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Library.cs index d4c9364620..fb2353b06d 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Library.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Library.cs @@ -1235,17 +1235,6 @@ static Library() BuiltinFunctionsCore.Now, NoErrorHandling(Now) }, - { - BuiltinFunctionsCore.OptionsSetInfo, - StandardErrorHandling( - BuiltinFunctionsCore.OptionsSetInfo.Name, - expandArguments: NoArgExpansion, - replaceBlankValues: DoNotReplaceBlank, - checkRuntimeTypes: ExactValueTypeOrBlank, - checkRuntimeValues: DeferRuntimeValueChecking, - returnBehavior: ReturnBehavior.ReturnEmptyStringIfAnyArgIsBlank, - targetFunction: OptionSetValueToLogicalName) - }, { BuiltinFunctionsCore.Or, Or diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryText.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryText.cs index 441853da85..7171521887 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryText.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryText.cs @@ -1169,13 +1169,6 @@ public static FormulaValue GuidPure(IRContext irContext, StringValue[] args) } } - public static FormulaValue OptionSetValueToLogicalName(IRContext irContext, OptionSetValue[] args) - { - var optionSet = args[0]; - var logicalName = optionSet.Option; - return new StringValue(irContext, logicalName); - } - public static FormulaValue PlainText(IRContext irContext, StringValue[] args) { string text = args[0].Value.Trim(); diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/OptionSetInfo.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/OptionSetInfo.cs new file mode 100644 index 0000000000..07fa066f56 --- /dev/null +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/OptionSetInfo.cs @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.PowerFx.Core.Functions; +using Microsoft.PowerFx.Core.IR; +using Microsoft.PowerFx.Core.Localization; +using Microsoft.PowerFx.Core.Types; +using Microsoft.PowerFx.Core.Utils; +using Microsoft.PowerFx.Functions; +using Microsoft.PowerFx.Types; + +namespace Microsoft.PowerFx.Interpreter +{ + internal sealed class OptionSetInfoFunction : TexlFunction, IAsyncTexlFunction + { + public override bool IsSelfContained => true; + + public override bool SupportsParamCoercion => false; + + public OptionSetInfoFunction() + : base(DPath.Root, "OptionSetInfo", "OptionSetInfo", TexlStrings.AboutOptionSetInfo, FunctionCategories.Text, DType.String, 0, 1, 1, DType.OptionSetValue) + { + } + + public override IEnumerable GetSignatures() + { + yield return new[] { TexlStrings.AboutOptionSetInfoArg1 }; + } + + public async Task InvokeAsync(FormulaValue[] args, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + switch (args[0]) + { + case ErrorValue errorValue: + return errorValue; + case BlankValue: + return new StringValue(IRContext.NotInSource(FormulaType.String), string.Empty); + case OptionSetValue osv: + return new StringValue(IRContext.NotInSource(FormulaType.String), osv.Option); + } + + return CommonErrors.GenericInvalidArgument(args[0].IRContext); + } + } +} diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests/Docs/DocTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests/Docs/DocTests.cs index e763242faa..d65e64ae86 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests/Docs/DocTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests/Docs/DocTests.cs @@ -16,6 +16,7 @@ public void Test() config.EnableJsonFunctions(); #pragma warning disable CS0618 // Type or member is obsolete config.EnableRegExFunctions(); + config.EnableOptionSetInfo(); #pragma warning restore CS0618 // Type or member is obsolete config.SymbolTable.EnableMutationFunctions(); diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests/RecalcEngineTests.cs index 4502e0e8eb..f4a5283762 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests/RecalcEngineTests.cs @@ -1032,6 +1032,9 @@ public async void OptionSetInfoTests(string expression, string expected) symValues.Set(option1Solt, option1); var config = new PowerFxConfig() { SymbolTable = symbol }; +#pragma warning disable CS0618 // Type or member is obsolete + config.EnableOptionSetInfo(); +#pragma warning restore CS0618 // Type or member is obsolete config.AddOptionSet(optionSet); var recalcEngine = new RecalcEngine(config); diff --git a/src/tools/Repl/Program.cs b/src/tools/Repl/Program.cs index 28c769d85a..e5afa6f52b 100644 --- a/src/tools/Repl/Program.cs +++ b/src/tools/Repl/Program.cs @@ -79,6 +79,9 @@ private static RecalcEngine ReplRecalcEngine() config.EnableSetFunction(); config.EnableJsonFunctions(); +#pragma warning disable CS0618 // Type or member is obsolete + config.EnableOptionSetInfo(); +#pragma warning restore CS0618 // Type or member is obsolete config.AddFunction(new ResetFunction()); config.AddFunction(new Option0Function());