Clarify error and instructions when user uses behavior function in non-behavior UDF (#2653)

Add an informative error for behavior functions used in non-imperative
UDF

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

@@ -4827,8 +4827,15 @@ namespace Microsoft.PowerFx.Core.Binding
 
                 // Invalid datasources always result in error
                 if (func.IsBehaviorOnly && !_txb.BindingConfig.AllowsSideEffects)
-                {
-                    _txb.ErrorContainer.EnsureError(node, TexlStrings.ErrBehaviorPropertyExpected);
+                {
+                    if (_txb.BindingConfig.UserDefinitionsMode)
+                    {
+                        _txb.ErrorContainer.EnsureError(node, TexlStrings.ErrBehaviorFunctionInDataUDF);
+                    }
+                    else
+                    {
+                        _txb.ErrorContainer.EnsureError(node, TexlStrings.ErrBehaviorPropertyExpected);
+                    }
                 }
 
                 // Test-only functions can only be used within test cases.

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

@@ -23,15 +23,18 @@ namespace Microsoft.PowerFx.Core.Binding
 
         public bool AnalysisMode { get; }
 
-        public bool MarkAsAsyncOnLazilyLoadedControlRef { get; } = false;
+        public bool MarkAsAsyncOnLazilyLoadedControlRef { get; } = false;
+
+        public bool UserDefinitionsMode { get; }
 
-        public BindingConfig(bool allowsSideEffects = false, bool useThisRecordForRuleScope = false, bool numberIsFloat = false, bool analysisMode = false, bool markAsAsyncOnLazilyLoadedControlRef = false)
+        public BindingConfig(bool allowsSideEffects = false, bool useThisRecordForRuleScope = false, bool numberIsFloat = false, bool analysisMode = false, bool markAsAsyncOnLazilyLoadedControlRef = false, bool userDefinitionsMode = false)
         {
             AllowsSideEffects = allowsSideEffects;
             UseThisRecordForRuleScope = useThisRecordForRuleScope;
             NumberIsFloat = numberIsFloat;
             AnalysisMode = analysisMode;
-            MarkAsAsyncOnLazilyLoadedControlRef = markAsAsyncOnLazilyLoadedControlRef;
+            MarkAsAsyncOnLazilyLoadedControlRef = markAsAsyncOnLazilyLoadedControlRef;
+            UserDefinitionsMode = userDefinitionsMode;
         }
     }
 }

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

@@ -147,7 +147,7 @@ namespace Microsoft.PowerFx.Core.Functions
                 throw new InvalidOperationException($"Body should only get bound once: {this.Name}");
             }
 
-            bindingConfig = bindingConfig ?? new BindingConfig(this._isImperative);
+            bindingConfig = bindingConfig ?? new BindingConfig(this._isImperative, userDefinitionsMode: true);
             _binding = TexlBinding.Run(documentBinderGlue, UdfBody, UserDefinitionsNameResolver.Create(nameResolver, _args, ParamTypes), bindingConfig, features: features, rule: rule);
 
             CheckTypesOnDeclaration(_binding.CheckTypesContext, _binding.ResultType, _binding);

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

@@ -677,6 +677,7 @@ namespace Microsoft.PowerFx.Core.Localization
         public static ErrorResourceKey ErrColonExpected = new ErrorResourceKey("ErrColonExpected");
         public static ErrorResourceKey ErrExpectedDataSourceRestriction = new ErrorResourceKey("ErrExpectedDataSourceRestriction");
         public static ErrorResourceKey ErrBehaviorPropertyExpected = new ErrorResourceKey("ErrBehaviorPropertyExpected");
+        public static ErrorResourceKey ErrBehaviorFunctionInDataUDF = new ErrorResourceKey("ErrBehaviorFunctionInDataUDF");
         public static ErrorResourceKey ErrTestPropertyExpected = new ErrorResourceKey("ErrTestPropertyExpected");
         public static ErrorResourceKey ErrStringExpected = new ErrorResourceKey("ErrStringExpected");
         public static ErrorResourceKey ErrDateExpected = new ErrorResourceKey("ErrDateExpected");

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

@@ -168,7 +168,7 @@ namespace Microsoft.PowerFx
                 var composedSymbols = ReadOnlySymbolTable.Compose(_localSymbolTable, _symbols);
                 foreach (var udf in partialUDFs)
                 {
-                    var config = new BindingConfig(allowsSideEffects: _parserOptions.AllowsSideEffects, useThisRecordForRuleScope: false, numberIsFloat: false);
+                    var config = new BindingConfig(allowsSideEffects: _parserOptions.AllowsSideEffects, useThisRecordForRuleScope: false, numberIsFloat: false, userDefinitionsMode: true);
                     var binding = udf.BindBody(composedSymbols, new Glue2DocumentBinderGlue(), config);
 
                     List<TexlError> bindErrors = new List<TexlError>();

src/strings/PowerFxResources.en-US.resx

@@ -3621,6 +3621,18 @@
     <value>https://go.microsoft.com/fwlink/?linkid=2132570</value>
     <comment>{Locked}</comment>
   </data>
+  <data name="ErrorResource_ErrBehaviorFunctionInDataUDF_ShortMessage" xml:space="preserve">
+    <value>Behavior function in a non-behavior user-defined function (UDF). Please wrap the user-defined function body with curly braces ({...}) to declare a behavior UDF.</value>
+    <comment>Error Message.</comment>
+  </data>
+  <data name="ErrorResource_ErrBehaviorFunctionInDataUDF_LongMessage" xml:space="preserve">
+    <value>Behavior functions change the current session state. 'Clear', 'Collect', 'Patch', and 'Refresh' are common behavior functions.</value>
+    <comment>{Locked=Clear}{Locked=Collect}{Locked=Patch}{Locked=Refresh}</comment>
+  </data>
+  <data name="ErrorResource_ErrBehaviorFunctionInDataUDF_HowToFix_1" xml:space="preserve">
+    <value>Edit the user-defined function expression so that it is inside curly braces ({...}).</value>
+    <comment>1 How to fix the error. </comment>
+  </data>
   <data name="ErrorResource_ErrTestPropertyExpected_ShortMessage" xml:space="preserve">
     <value>Test function in a non-test property. You can't use this property to invoke test-only functions.</value>
     <comment>Error Message. The term 'Test' is an adjective ('Test function' = 'function for testing').</comment>

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

@@ -634,7 +634,39 @@ namespace Microsoft.PowerFx.Tests
             result = check.GetEvaluator().Eval();
             Assert.Equal(3, result.AsDouble());
         }
+
+        [Theory]
+
+        // Behavior function in non-imperative udf
+        [InlineData(
+            "TestFunc():Void = Set(a, 123);",
+            true,
+            "Behavior function in a non-behavior user-defined function",
+            false)]
+
+        // Behavior function in imperative udf
+        [InlineData(
+            "TestFunc():Void = { Set(a, 123); };",
+            false,
+            null,
+            true)]
 
+        public void BehaviorFunctionInImperativeUDF(string udfExpression, bool expectedError, string expectedErrorKey, bool allowSideEffects)
+        {
+            var config = new PowerFxConfig();
+            config.EnableSetFunction();
+            var engine = new RecalcEngine(config);
+            engine.UpdateVariable("a", 1m);
+
+            var result = engine.AddUserDefinedFunction(udfExpression, CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: allowSideEffects);
+            Assert.True(expectedError ? result.Errors.Count() > 0 : result.Errors.Count() == 0);
+
+            if (expectedError)
+            {
+                result.Errors.Any(error => error.MessageKey == expectedErrorKey);
+            }
+        }
+
         [Theory]
 
         // Return value with side effectful UDF
@@ -1776,7 +1808,7 @@ namespace Microsoft.PowerFx.Tests
             var parserOptions = new ParserOptions()
             {
                 AllowsSideEffects = false,
-                AllowParseAsTypeLiteral = true
+                AllowParseAsTypeLiteral = true,
             };
 
             if (isValid)