From 5734aae991173f7238151bad3aa51c6f76a4c673 Mon Sep 17 00:00:00 2001 From: gpanaitescu Date: Wed, 1 Oct 2025 15:00:17 +0300 Subject: [PATCH] Improve out arguments assignability, for both validation and expression compilation (STUD-76892) --- ...proveAssignabilityOutArgumentActivity.xaml | 65 ++++++++++++++++ .../TestXamls/TestHelper.cs | 1 + .../WF4Samples/Expressions.cs | 6 +- ...ImproveAssignabilityOutArgumentActivity.cs | 60 +++++++++++++++ .../ImproveAssignabilityOutArgumentTests.cs | 33 ++++++++ .../WF4Samples/ValueSpecialCharacterTests.cs | 2 +- src/Test/TestCases.Workflows/XamlTests.cs | 16 +++- .../Activities/Utils/CSharpCompilerHelper.cs | 26 +++---- .../CSharp/CSharpExpressionCompiler.cs | 21 ++++- .../Utils/CSharpValidatorCommon.cs | 51 ++++++++++++ .../Validation/CSharpExpressionValidator.cs | 23 +----- .../XamlIntegration/TextExpressionCompiler.cs | 77 ++++++++++++++++++- 12 files changed, 333 insertions(+), 48 deletions(-) create mode 100644 src/Test/TestCases.Workflows/TestXamls/ImproveAssignabilityOutArgumentActivity.xaml create mode 100644 src/Test/TestCases.Workflows/WF4Samples/ImproveAssignabilityOutArgumentActivity.cs create mode 100644 src/Test/TestCases.Workflows/WF4Samples/ImproveAssignabilityOutArgumentTests.cs create mode 100644 src/UiPath.Workflow/Utils/CSharpValidatorCommon.cs diff --git a/src/Test/TestCases.Workflows/TestXamls/ImproveAssignabilityOutArgumentActivity.xaml b/src/Test/TestCases.Workflows/TestXamls/ImproveAssignabilityOutArgumentActivity.xaml new file mode 100644 index 000000000..17048dabb --- /dev/null +++ b/src/Test/TestCases.Workflows/TestXamls/ImproveAssignabilityOutArgumentActivity.xaml @@ -0,0 +1,65 @@ + + + + + + + + C# + + + + + + + + + + + + myEnumerable + + + + + + + + dateTimeOutArgument + + + + + + + + nullableDateTimeOutArgument + + + + + + + + + + big + + + + + new string('A', 100) ; + + + + + \ No newline at end of file diff --git a/src/Test/TestCases.Workflows/TestXamls/TestHelper.cs b/src/Test/TestCases.Workflows/TestXamls/TestHelper.cs index bf6c0a652..7d1a80987 100644 --- a/src/Test/TestCases.Workflows/TestXamls/TestHelper.cs +++ b/src/Test/TestCases.Workflows/TestXamls/TestHelper.cs @@ -46,5 +46,6 @@ public enum TestXamls SpecialCharacters, ValueSpecialCharacterCSharp, ValueSpecialCharacterVb, + ImproveAssignabilityOutArgumentActivity } } diff --git a/src/Test/TestCases.Workflows/WF4Samples/Expressions.cs b/src/Test/TestCases.Workflows/WF4Samples/Expressions.cs index e28c386af..6899bf164 100644 --- a/src/Test/TestCases.Workflows/WF4Samples/Expressions.cs +++ b/src/Test/TestCases.Workflows/WF4Samples/Expressions.cs @@ -21,7 +21,7 @@ namespace TestCases.Workflows.WF4Samples { using StringDictionary = Dictionary; - public abstract class ExpressionsBase + public abstract class ExpressionsBaseCommon { protected abstract bool CompileExpressions { get; } protected Activity GetActivityFromXamlResource(TestXamls xamlName) => TestHelper.GetActivityFromXamlResource(xamlName, CompileExpressions); @@ -31,6 +31,10 @@ protected Activity Compile(TestXamls xamlName) Compiler.Run(activity); return activity; } + } + + public abstract class ExpressionsBase : ExpressionsBaseCommon + { protected const string CorrectOutput = @"John Doe earns $55000.00 Frank Kimono earns $89000.00 Salary statistics: minimum salary is $55000.00, maximum salary is $89000.00, average salary is $72000.00 diff --git a/src/Test/TestCases.Workflows/WF4Samples/ImproveAssignabilityOutArgumentActivity.cs b/src/Test/TestCases.Workflows/WF4Samples/ImproveAssignabilityOutArgumentActivity.cs new file mode 100644 index 000000000..3d57c3cf4 --- /dev/null +++ b/src/Test/TestCases.Workflows/WF4Samples/ImproveAssignabilityOutArgumentActivity.cs @@ -0,0 +1,60 @@ +using System; +using System.Activities; +using System.Collections.Generic; + +namespace TestCases.Workflows.WF4Samples; + +public class ImproveAssignabilityOutArgumentActivity : NativeActivity +{ + // OutArgument of type List + public OutArgument> Result { get; set; } + + // Additional OutArguments + public OutArgument DateTimeOutArgument { get; set; } + public OutArgument NullableDateTimeOutArgument { get; set; } + + public ImproveAssignabilityOutArgumentActivity() : base() + { + } + + protected override void CacheMetadata(NativeActivityMetadata metadata) + { + // Result + var resultArg = new RuntimeArgument( + "Result", + typeof(List), + ArgumentDirection.Out); + metadata.Bind(this.Result, resultArg); + metadata.AddArgument(resultArg); + + // DateTimeOutArgument + var dateTimeArg = new RuntimeArgument( + "DateTimeOutArgument", + typeof(DateTime), + ArgumentDirection.Out); + metadata.Bind(this.DateTimeOutArgument, dateTimeArg); + metadata.AddArgument(dateTimeArg); + + // NullableDateTimeOutArgument + var nullableDateTimeArg = new RuntimeArgument( + "NullableDateTimeOutArgument", + typeof(DateTime?), + ArgumentDirection.Out); + metadata.Bind(this.NullableDateTimeOutArgument, nullableDateTimeArg); + metadata.AddArgument(nullableDateTimeArg); + } + + protected override void Execute(NativeActivityContext context) + { + var result = new List + { + "aaa", + "bbb", + "ccc" + }; + + Result.Set(context, result); + DateTimeOutArgument.Set(context, DateTime.Now); + NullableDateTimeOutArgument.Set(context, DateTime.Now); + } +} diff --git a/src/Test/TestCases.Workflows/WF4Samples/ImproveAssignabilityOutArgumentTests.cs b/src/Test/TestCases.Workflows/WF4Samples/ImproveAssignabilityOutArgumentTests.cs new file mode 100644 index 000000000..f92e09cb2 --- /dev/null +++ b/src/Test/TestCases.Workflows/WF4Samples/ImproveAssignabilityOutArgumentTests.cs @@ -0,0 +1,33 @@ +using Shouldly; +using System; +using System.Activities; +using System.Activities.XamlIntegration; +using System.Collections.Generic; +using Xunit; + +namespace TestCases.Workflows.WF4Samples; + +public class ImproveAssignabilityOutArgumentTests : ExpressionsBaseCommon +{ + protected override bool CompileExpressions => true; + + [Fact] + public void CompileImproveAssignabilityOutArgumentActivity() + { + Activity activity = null; + + // Assert that ActivityXamlServices.Load does not throw + Should.NotThrow(() => + { + activity = ActivityXamlServices.Load(TestHelper.GetXamlStream(TestXamls.ImproveAssignabilityOutArgumentActivity), + new ActivityXamlServicesSettings + { + CompileExpressions = true + }); + }); + + var invoker = new WorkflowInvoker(activity); + var result = invoker.Invoke(); + result["myEnumerable"].ShouldBe(new List { "aaa", "bbb", "ccc" }); + } +} diff --git a/src/Test/TestCases.Workflows/WF4Samples/ValueSpecialCharacterTests.cs b/src/Test/TestCases.Workflows/WF4Samples/ValueSpecialCharacterTests.cs index 8646557a2..7ee099b6e 100644 --- a/src/Test/TestCases.Workflows/WF4Samples/ValueSpecialCharacterTests.cs +++ b/src/Test/TestCases.Workflows/WF4Samples/ValueSpecialCharacterTests.cs @@ -19,7 +19,7 @@ namespace TestCases.Workflows.WF4Samples //4. Assign: output = Dict.First.Key. => we expect the key is "KeyName" //Results: the output has the unexpected value of "ValueName", instead of "KeyName", but if we enable parameter rename, //everything works as expected - public class ValueSpecialCharacterTests : ExpressionsBase + public class ValueSpecialCharacterTests : ExpressionsBaseCommon { protected override bool CompileExpressions => true; diff --git a/src/Test/TestCases.Workflows/XamlTests.cs b/src/Test/TestCases.Workflows/XamlTests.cs index 10fd267b0..a7049e3df 100644 --- a/src/Test/TestCases.Workflows/XamlTests.cs +++ b/src/Test/TestCases.Workflows/XamlTests.cs @@ -225,6 +225,16 @@ public async Task CSharp_CompileAnonymousTypes(Type targetType) Assert.Equal(typeof(IEnumerable), compilationResult.ReturnType); } + [Fact] + public async Task CSharp_CompileReferenceType() + { + SetupCompilation(out var location, out var namespaces, out var assemblyReferences); + + var result = await CSharpDesignerHelper.CreatePrecompiledReferenceAsync(typeof(List), "myEnumerable", namespaces, assemblyReferences, location); + + Assert.Equal(typeof(IEnumerable), result.ReturnType); + } + private static void SetupCompilation(out ActivityLocationReferenceEnvironment location, out string[] namespaces, out AssemblyReference[] assemblyReferences) { var seq = new Sequence(); @@ -233,7 +243,8 @@ private static void SetupCompilation(out ActivityLocationReferenceEnvironment lo WorkflowInspectionServices.CacheMetadata(seq, location); location.Declare(new Variable("in_CountryName"), seq, ref errors); location.Declare(new Variable("in_dt_OrderExport"), seq, ref errors); - namespaces = ["System", "System.Linq", "System.Data"]; + location.Declare(new Variable>("myEnumerable"), seq, ref errors); + namespaces = ["System", "System.Linq", "System.Data", "System.Collections.Generic"]; assemblyReferences = [ new AssemblyReference() { Assembly = typeof(string).Assembly }, @@ -242,7 +253,8 @@ private static void SetupCompilation(out ActivityLocationReferenceEnvironment lo new AssemblyReference() { Assembly = typeof(System.ComponentModel.TypeConverter).Assembly }, new AssemblyReference() { Assembly = typeof(IServiceProvider).Assembly }, new AssemblyReference() { Assembly = Assembly.Load("System.Xml.ReaderWriter") }, - new AssemblyReference() { Assembly = Assembly.Load("System.Private.Xml") } + new AssemblyReference() { Assembly = Assembly.Load("System.Private.Xml") }, + new AssemblyReference() { Assembly = typeof(IEnumerable<>).Assembly } ]; } diff --git a/src/UiPath.Workflow/Activities/Utils/CSharpCompilerHelper.cs b/src/UiPath.Workflow/Activities/Utils/CSharpCompilerHelper.cs index 2c356e949..2a998bf97 100644 --- a/src/UiPath.Workflow/Activities/Utils/CSharpCompilerHelper.cs +++ b/src/UiPath.Workflow/Activities/Utils/CSharpCompilerHelper.cs @@ -4,14 +4,11 @@ using Microsoft.CodeAnalysis.Scripting.Hosting; using ReflectionMagic; using System.Runtime.InteropServices; -using System.Text; -using System.Threading; namespace System.Activities { public sealed class CSharpCompilerHelper : CompilerHelper { - private static int crt = 0; private static readonly dynamic s_typeNameFormatter = GetTypeNameFormatter(); private static readonly dynamic s_typeOptions = GetTypeOptions(); @@ -39,22 +36,19 @@ public override string CreateExpressionCode(string[] types, string[] names, stri return $"{myDelegate} \n public static Expression<{name}<{typesStr}>> CreateExpression() => ({namesStr}) => {code};"; } - protected override (string, string) DefineDelegateCommon(int argumentsCount) + internal string CreateReferenceCode(string[] types, string returnType, string[] names, string code) { - var crtValue = Interlocked.Add(ref crt, 1); - - var part1 = new StringBuilder(); - var part2 = new StringBuilder(); - for (var i = 0; i < argumentsCount; i++) - { - part1.Append($"in T{i}, "); - part2.Append($" T{i} arg{i},"); - } - part2.Remove(part2.Length - 1, 1); - var name = $"Func{crtValue}"; - return ($"public delegate TResult {name}<{part1} out TResult>({part2});", name); + var strTypes = string.Join(Comma, types); + var strNames = string.Join(Comma, names); + return CSharpValidatorCommon.CreateReferenceCode(strTypes, returnType, strNames, code, string.Empty, 0); } + internal string CreateValueCode(string[] types, string[] names, string code) + => CSharpValidatorCommon.CreateValueCode(types, string.Join(Comma, names), code, string.Empty, 0); + + protected override (string, string) DefineDelegateCommon(int argumentsCount) + => CSharpValidatorCommon.DefineDelegateCommon(argumentsCount); + private static object GetTypeNameFormatter() { return typeof(CSharpScript) diff --git a/src/UiPath.Workflow/Microsoft/CSharp/CSharpExpressionCompiler.cs b/src/UiPath.Workflow/Microsoft/CSharp/CSharpExpressionCompiler.cs index b9a27d98d..0a1f320d3 100644 --- a/src/UiPath.Workflow/Microsoft/CSharp/CSharpExpressionCompiler.cs +++ b/src/UiPath.Workflow/Microsoft/CSharp/CSharpExpressionCompiler.cs @@ -50,8 +50,23 @@ protected override SyntaxTree GetSyntaxTreeForExpression(string expression, bool .ToArray(); var names = resolvedIdentifiers.Select(var => var.Name).ToArray(); - var types = resolvedIdentifiers.Select(var => var.Type).Concat(new[] { returnType }).Select(_compilerHelper.GetTypeName).ToArray(); - var lambdaFuncCode = _compilerHelper.CreateExpressionCode(types, names, expression); - return CSharpSyntaxTree.ParseText(lambdaFuncCode, _compilerHelper.ScriptParseOptions); + var types = resolvedIdentifiers.Select(var => var.Type).Select(_compilerHelper.GetTypeName).ToArray(); + string expressionCode; + if (isLocation) + { + expressionCode = _compilerHelper.CreateReferenceCode(types: types, + returnType: _compilerHelper.GetTypeName(returnType), + names: names, + code: expression); + } + else + { + types = types.Concat(new[] { _compilerHelper.GetTypeName(returnType) }).ToArray(); + expressionCode = _compilerHelper.CreateValueCode(types: types, + names: names, + code: expression); + } + + return CSharpSyntaxTree.ParseText(expressionCode, _compilerHelper.ScriptParseOptions); } } diff --git a/src/UiPath.Workflow/Utils/CSharpValidatorCommon.cs b/src/UiPath.Workflow/Utils/CSharpValidatorCommon.cs new file mode 100644 index 000000000..4768b7c5a --- /dev/null +++ b/src/UiPath.Workflow/Utils/CSharpValidatorCommon.cs @@ -0,0 +1,51 @@ +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading; +namespace System.Activities; + +internal static class CSharpValidatorCommon +{ + private static int crt = 0; + + // This is used in case the expression does not properly close (e.g. missing quotes, or multiline comment not closed) + private const string _expressionEnder = "// */ // \""; + + private const string _valueValidationTemplate = "public static System.Linq.Expressions.Expression> CreateExpression{1}()//activityId:{4}\n => ({2}) => {3}; {5}"; + private const string _delegateValueValidationTemplate = "{0}\npublic static System.Linq.Expressions.Expression<{1}<{2}>> CreateExpression{3}()//activityId:{6}\n => ({4}) => {5}; {7}"; + private const string _referenceValidationTemplate = "public static {0} IsLocation{1}()//activityId:{5}\n => ({2}) => {3} = default({4}); {6}"; + + internal static string CreateReferenceCode(string types, string returnType, string names, string code, string activityId, int index) + { + var actionDefinition = !string.IsNullOrWhiteSpace(types) + ? $"System.Action<{string.Join(CompilerHelper.Comma, types)}>" + : "System.Action"; + return string.Format(_referenceValidationTemplate, actionDefinition, index, names, code, returnType, activityId, _expressionEnder); + } + + internal static string CreateValueCode(IEnumerable types, string names, string code, string activityId, int index) + { + var serializedArgumentTypes = string.Join(CompilerHelper.Comma, types); + if (types.Count() <= 16) // .net defines Func...Func({part2});", name); + } +} diff --git a/src/UiPath.Workflow/Validation/CSharpExpressionValidator.cs b/src/UiPath.Workflow/Validation/CSharpExpressionValidator.cs index 592772967..1ff9151d5 100644 --- a/src/UiPath.Workflow/Validation/CSharpExpressionValidator.cs +++ b/src/UiPath.Workflow/Validation/CSharpExpressionValidator.cs @@ -18,13 +18,6 @@ namespace System.Activities.Validation; /// public class CSharpExpressionValidator : RoslynExpressionValidator { - // This is used in case the expression does not properly close (e.g. missing quotes, or multiline comment not closed) - private const string _expressionEnder = "// */ // \""; - - private const string _valueValidationTemplate = "public static System.Linq.Expressions.Expression> CreateExpression{1}()//activityId:{4}\n => ({2}) => {3}; {5}"; - private const string _delegateValueValidationTemplate = "{0}\npublic static System.Linq.Expressions.Expression<{1}<{2}>> CreateExpression{3}()//activityId:{6}\n => ({4}) => {5}; {7}"; - private const string _referenceValidationTemplate = "public static {0} IsLocation{1}()//activityId:{5}\n => ({2}) => {3} = default({4}); {6}"; - private static readonly Lazy s_instance = new(() => new()); public override string Language => CSharpHelper.Language; @@ -66,22 +59,10 @@ protected override Compilation GetCompilation(IReadOnlyCollection asse } protected override string CreateValueCode(IEnumerable types, string names, string code, string activityId, int index) - { - var serializedArgumentTypes = string.Join(Comma, types); - if (types.Count() <= 16) // .net defines Func...Funct CSharpValidatorCommon.CreateValueCode(types, names, code, activityId, index); protected override string CreateReferenceCode(string types, string names, string code, string activityId, string returnType, int index) - { - var actionDefinition = !string.IsNullOrWhiteSpace(types) - ? $"System.Action<{string.Join(Comma, types)}>" - : "System.Action"; - return string.Format(_referenceValidationTemplate, actionDefinition, index, names, code, returnType, activityId, _expressionEnder); - } + => CSharpValidatorCommon.CreateReferenceCode(types, returnType, names, code, activityId, index); protected override SyntaxTree GetSyntaxTreeForExpression(string expressionText) => CSharpSyntaxTree.ParseText(expressionText, CompilerHelper.ScriptParseOptions); diff --git a/src/UiPath.Workflow/XamlIntegration/TextExpressionCompiler.cs b/src/UiPath.Workflow/XamlIntegration/TextExpressionCompiler.cs index f8309329b..67de09b20 100644 --- a/src/UiPath.Workflow/XamlIntegration/TextExpressionCompiler.cs +++ b/src/UiPath.Workflow/XamlIntegration/TextExpressionCompiler.cs @@ -65,6 +65,8 @@ public class TextExpressionCompiler private bool? _isVb; private int _nextContextId; + private readonly Lazy _lazyProvider; + public TextExpressionCompiler(TextExpressionCompilerSettings settings) { if (settings == null) @@ -99,6 +101,8 @@ public TextExpressionCompiler(TextExpressionCompilerSettings settings) _lineNumbersForNSes = new Dictionary(); _lineNumbersForNSesForImpl = new Dictionary(); + + _lazyProvider = new Lazy(() => CodeDomProvider.CreateProvider(_settings.Language)); } private bool IsCs @@ -1458,7 +1462,7 @@ private bool TryGenerateExpressionCode(Activity activity, CompiledDataContextDes if (isValue || isReference) { - var expressionGetMethod = GenerateGetMethod(activity, resultType, expressionText, nextExpressionId); + var expressionGetMethod = GenerateGetMethod(activity, resultType, expressionText, nextExpressionId, isReference); typeDeclaration.Members.Add(expressionGetMethod); var expressionGetValueTypeAccessorMethod = GenerateGetMethodWrapper(expressionGetMethod); @@ -1525,7 +1529,15 @@ private void GenerateExpressionGetTreeMethod(Activity activity, CompiledExpressi } else if (IsCs) { - expressionText = string.Concat(CSharpLambdaString, coreExpressionText); + if (!isValue) + { + var safeCastText = SafeCastText(coreExpressionText, expressionDescriptor.ResultType); + expressionText = string.Concat(CSharpLambdaString, safeCastText); + } + else + { + expressionText = string.Concat(CSharpLambdaString, coreExpressionText); + } } if (expressionText != null) @@ -1558,7 +1570,7 @@ private void GenerateExpressionGetTreeMethod(Activity activity, CompiledExpressi } private CodeMemberMethod GenerateGetMethod(Activity activity, Type resultType, string expressionText, - int nextExpressionId) + int nextExpressionId, bool isReference) { var expressionMethod = new CodeMemberMethod { @@ -1570,7 +1582,16 @@ private CodeMemberMethod GenerateGetMethod(Activity activity, Type resultType, s new CodeAttributeDeclaration(new CodeTypeReference(typeof(DebuggerHiddenAttribute)))); AlignText(activity, ref expressionText, out var pragma); - CodeStatement statement = new CodeMethodReturnStatement(new CodeSnippetExpression(expressionText)); + CodeStatement statement; + if (IsCs && isReference) + { + statement = new CodeMethodReturnStatement(SafeCast(expressionText, resultType)); + } + else + { + statement = new CodeMethodReturnStatement(new CodeSnippetExpression(expressionText)); + } + statement.LinePragma = pragma; expressionMethod.Statements.Add(statement); @@ -2512,6 +2533,54 @@ private string GetActivityFullName(TextExpressionCompilerSettings settings) return activityFullName; } + /// + /// Creates a CodeSnippetExpression like: "variableName as TargetType". + /// If variableName or targetType is null/empty, falls back to "null". + /// + private CodeSnippetExpression SafeCast(string variableName, Type targetType) + => new CodeSnippetExpression(SafeCastText(variableName, targetType)); + + /// + /// Safely formats a string representation of a variable name and its target type. + /// Validates that the 'as' operator is only applied to reference types or nullable value types. + /// For non-nullable value types, returns the variable name without any casting. + /// + /// The name of the variable to be cast. If null or whitespace, "null" is used instead. + /// The target to which the variable is being cast. + /// A string in the format " as " for valid types, otherwise just the variable name. + private string SafeCastText(string variableName, Type targetType) + { + string varName = string.IsNullOrWhiteSpace(variableName) ? "null" : variableName; + + // Early exit if targetType is null or if it's a non-nullable value type + if (targetType == null || (targetType.IsValueType && !IsNullableValueType(targetType))) + { + return varName; + } + + string typeName = GetFriendlyTypeName(targetType); + if (typeName == null) + { + return varName; + } + + // Use 'as' operator for reference types and nullable value types + return $"{varName} as {typeName}"; + } + + /// + /// Helper method to check if a type is a nullable value type (e.g., int?, DateTime?) + /// + private static bool IsNullableValueType(Type type) + { + return type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>); + } + + private string GetFriendlyTypeName(Type type) + { + return type is null ? null : _lazyProvider.Value.GetTypeOutput(new CodeTypeReference(type)); + } + private class ExpressionCompilerActivityVisitor : CompiledExpressionActivityVisitor { private readonly TextExpressionCompiler _compiler;