From 14d9696d75a44305dabfb13779db2ef648e2c767 Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Mon, 3 Nov 2025 13:26:43 +0100 Subject: [PATCH 1/5] Delay throwing exceptions for invalid expressions untill thy actually gets evalutated. --- .../Expressions/ExpressionEvaluator.cs | 64 ++++++++++----- .../Models/Expressions/ComponentContext.cs | 6 +- .../Models/Expressions/Expression.cs | 80 +++++++++++++------ .../Models/Expressions/ExpressionConverter.cs | 41 ++++++---- .../Models/Expressions/ExpressionFunction.cs | 7 ++ .../CommonTests/TestInvalid.cs | 2 +- ...ouldNotChange_Unintentionally.verified.txt | 23 +++--- 7 files changed, 145 insertions(+), 78 deletions(-) diff --git a/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs b/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs index a7658c8c1a..a670e4c1af 100644 --- a/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs +++ b/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using System.Globalization; using System.Text.Json; using System.Text.RegularExpressions; @@ -80,12 +81,13 @@ internal static async Task EvaluateExpression_internal( ExpressionValue[]? positionalArguments = null ) { - if (!expr.IsFunctionExpression) + if (expr.IsLiteralValue) { return expr.ValueUnion; } + ValidateExpressionArgs(expr); - var args = new ExpressionValue[expr.Args.Count]; + var args = new ExpressionValue[expr.Args.Length]; for (var i = 0; i < args.Length; i++) { args[i] = await EvaluateExpression_internal(state, expr.Args[i], context, positionalArguments); @@ -93,6 +95,7 @@ internal static async Task EvaluateExpression_internal( ExpressionValue ret = expr.Function switch { + //ExpressionFunction.LITERAL_VALUE => expr.ValueUnion, // Handled above ExpressionFunction.dataModel => await DataModel(args, context, state), ExpressionFunction.component => await Component(args, context, state), ExpressionFunction.countDataElements => CountDataElements(args, state), @@ -129,27 +132,33 @@ internal static async Task EvaluateExpression_internal( ExpressionFunction.argv => Argv(args, positionalArguments), ExpressionFunction.gatewayAction => state.GetGatewayAction(), ExpressionFunction.language => state.GetLanguage(), - _ => throw new ExpressionEvaluatorTypeErrorException("Function not implemented", expr.Function, args), + ExpressionFunction.INVALID => throw new ExpressionEvaluatorTypeErrorException( + $"Function {expr.Args.FirstOrDefault()} not implemented in backend" + expr.ToString() + ), + _ => throw new UnreachableException( + $"Function {(int)expr.Function} not a valid enum value" + expr.ToString() + ), }; return ret; } private static void ValidateExpressionArgs(Expression expr) { + // Some functions have restrictions that arguments must be literal values and not subexpressions. switch (expr) { - case { Function: ExpressionFunction.dataModel, Args: [_, { IsFunctionExpression: true }] }: + case { Function: ExpressionFunction.dataModel, Args: [_, { IsLiteralValue: false }] }: throw new ExpressionEvaluatorTypeErrorException( "The data type must be a string (expressions cannot be used here)" ); - case { Function: ExpressionFunction.@if, Args: [_, _, { IsFunctionExpression: true }, _] }: + case { Function: ExpressionFunction.@if, Args: [_, _, { IsLiteralValue: false }, _] }: throw new ExpressionEvaluatorTypeErrorException("Expected third argument to be \"else\""); - case { Function: ExpressionFunction.compare, Args: [_, { IsFunctionExpression: true }, _] }: - case { Function: ExpressionFunction.compare, Args: [_, _, { IsFunctionExpression: true }, _] }: + case { Function: ExpressionFunction.compare, Args: [_, { IsLiteralValue: false }, _] }: + case { Function: ExpressionFunction.compare, Args: [_, _, { IsLiteralValue: false }, _] }: throw new ExpressionEvaluatorTypeErrorException( "Invalid operator (it cannot be an expression or null)" ); - case { Function: ExpressionFunction.compare, Args: [_, { IsFunctionExpression: true }, _, _] }: + case { Function: ExpressionFunction.compare, Args: [_, { IsLiteralValue: false }, _, _] }: throw new ExpressionEvaluatorTypeErrorException( "Second argument must be \"not\" when providing 4 arguments in total" ); @@ -333,7 +342,7 @@ private static bool Contains(ExpressionValue[] args) date = TimeZoneInfo.ConvertTime(date.Value, timezone); } - string? language = state.GetLanguage(); + string language = state.GetLanguage(); return UnicodeDateTimeTokenConverter.Format( date, args.Length == 2 ? args[1].ToStringForEquals() : null, @@ -653,12 +662,7 @@ private static string Round(ExpressionValue[] args) ); } - var number = PrepareNumericArg(args[0]); - - if (number is null) - { - number = 0; - } + var number = PrepareNumericArg(args[0]) ?? 0; int precision = 0; @@ -667,7 +671,7 @@ private static string Round(ExpressionValue[] args) precision = (int)(PrepareNumericArg(args[1]) ?? 0); } - return number.Value.ToString($"N{precision}", CultureInfo.InvariantCulture); + return number.ToString($"N{precision}", CultureInfo.InvariantCulture); } private static string? UpperCase(ExpressionValue[] args) @@ -770,9 +774,17 @@ private static bool PrepareBooleanArg(ExpressionValue arg) throw new ExpressionEvaluatorTypeErrorException("Expected 1+ argument(s), got 0"); } - var preparedArgs = args.Select(arg => PrepareBooleanArg(arg)).ToArray(); - // Ensure all args gets converted, because they might throw an Exception - return preparedArgs.All(a => a); + var all = true; + foreach (var arg in args) + { + // Ensure all args gets converted, because they might throw an Exception + if (!PrepareBooleanArg(arg)) + { + all = false; + } + } + + return all; } private static async Task Text( @@ -804,9 +816,17 @@ ExpressionValue[] args throw new ExpressionEvaluatorTypeErrorException("Expected 1+ argument(s), got 0"); } - var preparedArgs = args.Select(arg => PrepareBooleanArg(arg)).ToArray(); - // Ensure all args gets converted, because they might throw an Exception - return preparedArgs.Any(a => a); + bool any = false; + foreach (var arg in args) + { + // Ensure all args gets converted, because they might throw an Exception + if (PrepareBooleanArg(arg)) + { + any = true; + } + } + + return any; } private static bool? Not(ExpressionValue[] args) diff --git a/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs b/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs index f9aec5e3f8..c58c761a89 100644 --- a/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs +++ b/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs @@ -220,9 +220,9 @@ public DebuggerEvaluatedExpression(Expression expression, ComponentContext conte public IEnumerable? Args => _expression.Args?.Select(e => new DebuggerEvaluatedExpression(e, _context)); public Task EvaluationResult => - _expression.IsFunctionExpression - ? ExpressionEvaluator.EvaluateExpression_internal(_context.State, _expression, _context, null) - : Task.FromResult(_expression.ValueUnion); + _expression.IsLiteralValue + ? Task.FromResult(_expression.ValueUnion) + : ExpressionEvaluator.EvaluateExpression_internal(_context.State, _expression, _context, null); public override string ToString() { diff --git a/src/Altinn.App.Core/Models/Expressions/Expression.cs b/src/Altinn.App.Core/Models/Expressions/Expression.cs index 0b1e421d1e..32da7d3867 100644 --- a/src/Altinn.App.Core/Models/Expressions/Expression.cs +++ b/src/Altinn.App.Core/Models/Expressions/Expression.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Serialization; @@ -15,60 +16,87 @@ namespace Altinn.App.Core.Models.Expressions; public readonly struct Expression : IEquatable { /// - /// Construct a value expression with the given value + /// Construct a value expression with the given literal value /// /// public Expression(ExpressionValue value) { + Function = ExpressionFunction.LITERAL_VALUE; + Args = null; ValueUnion = value; } /// - /// Construct a value expression with the given value + /// Construct a function expression with the given function and arguments /// - /// - [Obsolete("Use the constructor with ExpressionValue instead")] - public Expression(object? value) + public Expression(ExpressionFunction function, Expression[] args) { - ValueUnion = ExpressionValue.FromObject(value); + Debug.Assert( + function != ExpressionFunction.LITERAL_VALUE, + "Function cannot be LITERAL_VALUE when Args are provided" + ); + Debug.Assert(args != null, "Args cannot be null when constructing a function expression"); + Function = function; + Args = args; } /// /// Construct a function expression with the given function and arguments /// - public Expression(ExpressionFunction function, List? args) - { - Function = function; - Args = args; - } + [Obsolete("Use the constructor with Expression[] instead")] + public Expression(ExpressionFunction function, List args) + : this(function, args.ToArray()) { } + + /// + /// Construct a value expression with the given value + /// + /// + [Obsolete("Use the constructor with ExpressionValue instead")] + public Expression(object? value) + : this(ExpressionValue.FromObject(value)) { } /// /// Construct a function expression with the given function and arguments /// public Expression(ExpressionFunction function, Expression arg1) - { - Function = function; - Args = [arg1]; - } + : this(function, new[] { arg1 }) { } /// /// Construct a function expression with the given function and arguments /// public Expression(ExpressionFunction function, Expression arg1, Expression arg2) - { - Function = function; - Args = [arg1, arg2]; - } + : this(function, new[] { arg1, arg2 }) { } /// /// Test function to see if this is representing a function with args. /// - [MemberNotNullWhen(true, nameof(Function), nameof(Args))] -#pragma warning disable CS0618 // Type or member is obsolete - [MemberNotNullWhen(false, nameof(Value))] -#pragma warning restore CS0618 // Type or member is obsolete - [MemberNotNullWhen(false, nameof(ValueUnion))] - public bool IsFunctionExpression => Function != ExpressionFunction.INVALID && Args != null; + [MemberNotNullWhen(true, nameof(Args))] + [Obsolete("Use IsLiteralExpression instead")] + public bool IsFunctionExpression => !IsLiteralValue; + + /// + /// Test to see if this expression must be evaluated or is just a literal value. + /// + [MemberNotNullWhen(false, nameof(Args))] + public bool IsLiteralValue + { + get + { + if (Args == null) + { + Debug.Assert( + Function == ExpressionFunction.LITERAL_VALUE, + "If Args is null, Function must be LITERAL_VALUE" + ); + return true; + } + Debug.Assert( + Function != ExpressionFunction.LITERAL_VALUE, + "If Args is not null, Function must not be LITERAL_VALUE" + ); + return false; + } + } /// /// Name of the function. Must be one those actually implemented in @@ -78,7 +106,7 @@ public Expression(ExpressionFunction function, Expression arg1, Expression arg2) /// /// List of arguments to the function. These expressions will be evaluated before passed to the function. /// - public List? Args { get; } + public Expression[]? Args { get; } /// /// Get the object value for backwards compatibility diff --git a/src/Altinn.App.Core/Models/Expressions/ExpressionConverter.cs b/src/Altinn.App.Core/Models/Expressions/ExpressionConverter.cs index 97539fcd4d..3095ad950b 100644 --- a/src/Altinn.App.Core/Models/Expressions/ExpressionConverter.cs +++ b/src/Altinn.App.Core/Models/Expressions/ExpressionConverter.cs @@ -69,16 +69,21 @@ private static Expression ReadArray(JsonElement element) { throw new JsonException("Function name in expression must be string"); } + var args = new List(); + var functionString = enumerator.Current.GetString(); - var functionEnum = ExpressionFunction(enumerator.Current.GetString()); + var functionEnum = ExpressionFunction(functionString); + if (functionEnum == Expressions.ExpressionFunction.INVALID) + { + args.Add(new Expression(functionString)); + } - var args = new List(); while (enumerator.MoveNext()) { args.Add(ReadStatic(enumerator.Current)); } - return new Expression(functionEnum, args); + return new Expression(functionEnum, args.ToArray()); } private static Expression ReadArray(ref Utf8JsonReader reader, JsonSerializerOptions options) @@ -93,24 +98,29 @@ private static Expression ReadArray(ref Utf8JsonReader reader, JsonSerializerOpt { throw new JsonException("Function name in expression should be string"); } + var args = new List(); - var functionEnum = ExpressionFunction(reader.GetString()); + var functionString = reader.GetString(); + var functionEnum = ExpressionFunction(functionString); - var args = new List(); + if (functionEnum == Expressions.ExpressionFunction.INVALID) + { + args.Add(new Expression(functionString)); + } while (reader.Read() && reader.TokenType != JsonTokenType.EndArray) { args.Add(ReadStatic(ref reader, options)); } - return new Expression(functionEnum, args); + return new Expression(functionEnum, args.ToArray()); } private static ExpressionFunction ExpressionFunction(string? stringFunction) { if (!Enum.TryParse(stringFunction, ignoreCase: false, out var functionEnum)) { - throw new JsonException($"Function \"{stringFunction}\" not implemented"); + return Expressions.ExpressionFunction.INVALID; } return functionEnum; @@ -119,21 +129,24 @@ private static ExpressionFunction ExpressionFunction(string? stringFunction) /// public override void Write(Utf8JsonWriter writer, Expression value, JsonSerializerOptions options) { - if (value.IsFunctionExpression) + if (value.IsLiteralValue) + { + // Just serialize the literal value + JsonSerializer.Serialize(writer, value.ValueUnion.ToObject(), options); + } + else { // Serialize with as an array expression ["functionName", arg1, arg2, ...] writer.WriteStartArray(); - writer.WriteStringValue(value.Function.ToString()); + if (value.Function != Expressions.ExpressionFunction.INVALID) + { + writer.WriteStringValue(value.Function.ToString()); + } foreach (var arg in value.Args) { JsonSerializer.Serialize(writer, arg, options); } writer.WriteEndArray(); } - else - { - // Just serialize the literal value - JsonSerializer.Serialize(writer, value.ValueUnion.ToObject(), options); - } } } diff --git a/src/Altinn.App.Core/Models/Expressions/ExpressionFunction.cs b/src/Altinn.App.Core/Models/Expressions/ExpressionFunction.cs index c4082ab03a..f799c95a9f 100644 --- a/src/Altinn.App.Core/Models/Expressions/ExpressionFunction.cs +++ b/src/Altinn.App.Core/Models/Expressions/ExpressionFunction.cs @@ -12,6 +12,13 @@ namespace Altinn.App.Core.Models.Expressions; [JsonConverter(typeof(JsonStringEnumConverter))] public enum ExpressionFunction { + /// + /// Expressions that hold a literal value have this as function + /// +#pragma warning disable CA1707 // Identifiers should not contain underscores + LITERAL_VALUE = -1, +#pragma warning restore CA1707 // Identifiers should not contain underscores + /// /// Value for all unknown functions. /// diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestInvalid.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestInvalid.cs index 9c75e5e6ad..c935f44554 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestInvalid.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestInvalid.cs @@ -55,7 +55,7 @@ public async Task Simple_Theory(string testName, string folder) await ExpressionEvaluator.EvaluateExpression(state, test.Expression, await test.GetContextOrNull(state)); }; - (await act.Should().ThrowAsync()).WithMessage(testCase.ExpectsFailure); + (await act.Should().ThrowAsync()).WithMessage(testCase.ExpectsFailure + "*"); } private static async Task LoadData(string testName, string folder) diff --git a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt index bec0cac792..f51228b590 100644 --- a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt +++ b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt @@ -4428,22 +4428,20 @@ namespace Altinn.App.Core.Models.Expressions [System.Obsolete("Use the constructor with ExpressionValue instead")] public Expression(object? value) { } public Expression(Altinn.App.Core.Models.Expressions.ExpressionFunction function, Altinn.App.Core.Models.Expressions.Expression arg1) { } - public Expression(Altinn.App.Core.Models.Expressions.ExpressionFunction function, System.Collections.Generic.List? args) { } + public Expression(Altinn.App.Core.Models.Expressions.ExpressionFunction function, Altinn.App.Core.Models.Expressions.Expression[] args) { } + [System.Obsolete("Use the constructor with Expression[] instead")] + public Expression(Altinn.App.Core.Models.Expressions.ExpressionFunction function, System.Collections.Generic.List args) { } public Expression(Altinn.App.Core.Models.Expressions.ExpressionFunction function, Altinn.App.Core.Models.Expressions.Expression arg1, Altinn.App.Core.Models.Expressions.Expression arg2) { } - public System.Collections.Generic.List? Args { get; } + public Altinn.App.Core.Models.Expressions.Expression[]? Args { get; } public Altinn.App.Core.Models.Expressions.ExpressionFunction Function { get; } - [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Value")] - [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "ValueUnion")] - [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, new string?[]?[] { - "Function", - "Args"})] - [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Value")] - [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "ValueUnion")] - [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, new string?[]?[] { - "Function", - "Args"})] + [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Args")] + [System.Obsolete("Use IsLiteralExpression instead")] + [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Args")] public bool IsFunctionExpression { get; } public bool IsLiteralString { get; } + [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Args")] + [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Args")] + public bool IsLiteralValue { get; } [System.Obsolete("Use ValueUnion instead")] public object? Value { get; } public Altinn.App.Core.Internal.Expressions.ExpressionValue ValueUnion { get; } @@ -4468,6 +4466,7 @@ namespace Altinn.App.Core.Models.Expressions [System.Text.Json.Serialization.JsonConverter(typeof(System.Text.Json.Serialization.JsonStringEnumConverter))] public enum ExpressionFunction { + LITERAL_VALUE = -1, INVALID = 0, dataModel = 1, component = 2, From 6b3155c1cde85c53e76bd40f7f5791e1f31d5faa Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Mon, 3 Nov 2025 20:03:08 +0100 Subject: [PATCH 2/5] Coderabbit suggesions --- .../Expressions/ExpressionEvaluator.cs | 10 ++--- .../Expressions/LayoutEvaluatorState.cs | 2 +- .../Models/Expressions/Expression.cs | 44 +++++++------------ .../ExpressionTests.cs | 6 +-- 4 files changed, 24 insertions(+), 38 deletions(-) diff --git a/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs b/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs index a670e4c1af..d3264af095 100644 --- a/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs +++ b/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs @@ -133,11 +133,9 @@ internal static async Task EvaluateExpression_internal( ExpressionFunction.gatewayAction => state.GetGatewayAction(), ExpressionFunction.language => state.GetLanguage(), ExpressionFunction.INVALID => throw new ExpressionEvaluatorTypeErrorException( - $"Function {expr.Args.FirstOrDefault()} not implemented in backend" + expr.ToString() - ), - _ => throw new UnreachableException( - $"Function {(int)expr.Function} not a valid enum value" + expr.ToString() + $"Function {expr.Args.FirstOrDefault()} not implemented in backend {expr}" ), + _ => throw new UnreachableException($"Function {(int)expr.Function} not a valid enum value {expr}"), }; return ret; } @@ -777,7 +775,7 @@ private static bool PrepareBooleanArg(ExpressionValue arg) var all = true; foreach (var arg in args) { - // Ensure all args gets converted, because they might throw an Exception + // the LINQ All() method would short-circuit and not evaluate all args, so we do it manually to ensure exceptions are thrown correctly if (!PrepareBooleanArg(arg)) { all = false; @@ -819,7 +817,7 @@ ExpressionValue[] args bool any = false; foreach (var arg in args) { - // Ensure all args gets converted, because they might throw an Exception + // the LINQ Any() method would short-circuit and not evaluate all args, so we do it manually to ensure exceptions are thrown correctly if (PrepareBooleanArg(arg)) { any = true; diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs index 7249aa990c..c0c6564a24 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs @@ -401,7 +401,7 @@ public async Task TranslateText(string textKey, ComponentContext context // private void GetModelErrorsForExpression(Expression expr, BaseComponent component, List errors) // { - // if (!expr.IsFunctionExpression) + // if (expr.IsLiteralValue) // { // return; // } diff --git a/src/Altinn.App.Core/Models/Expressions/Expression.cs b/src/Altinn.App.Core/Models/Expressions/Expression.cs index 32da7d3867..c7b180690f 100644 --- a/src/Altinn.App.Core/Models/Expressions/Expression.cs +++ b/src/Altinn.App.Core/Models/Expressions/Expression.cs @@ -31,11 +31,17 @@ public Expression(ExpressionValue value) /// public Expression(ExpressionFunction function, Expression[] args) { - Debug.Assert( - function != ExpressionFunction.LITERAL_VALUE, - "Function cannot be LITERAL_VALUE when Args are provided" - ); - Debug.Assert(args != null, "Args cannot be null when constructing a function expression"); + if (function == ExpressionFunction.LITERAL_VALUE) + { + throw new ArgumentException("Function LITERAL_VALUE cannot have arguments", nameof(function)); + } + if (args == null) + { + throw new ArgumentNullException( + nameof(args), + "Args cannot be null when constructing a function expression" + ); + } Function = function; Args = args; } @@ -71,32 +77,14 @@ public Expression(ExpressionFunction function, Expression arg1, Expression arg2) /// Test function to see if this is representing a function with args. /// [MemberNotNullWhen(true, nameof(Args))] - [Obsolete("Use IsLiteralExpression instead")] + [Obsolete("Use !IsLiteralValue instead")] public bool IsFunctionExpression => !IsLiteralValue; /// /// Test to see if this expression must be evaluated or is just a literal value. /// [MemberNotNullWhen(false, nameof(Args))] - public bool IsLiteralValue - { - get - { - if (Args == null) - { - Debug.Assert( - Function == ExpressionFunction.LITERAL_VALUE, - "If Args is null, Function must be LITERAL_VALUE" - ); - return true; - } - Debug.Assert( - Function != ExpressionFunction.LITERAL_VALUE, - "If Args is not null, Function must not be LITERAL_VALUE" - ); - return false; - } - } + public bool IsLiteralValue => Args == null; /// /// Name of the function. Must be one those actually implemented in @@ -143,7 +131,7 @@ public bool IsLiteralValue public bool IsLiteralString => ValueUnion.ValueKind == JsonValueKind.String; /// - /// Overridden for better debugging experience + /// We have a custom JsonSerializer so this will return the json array representation of this expression /// public override string ToString() { @@ -159,7 +147,7 @@ public bool Equals(Expression other) // return false; // // // For function expressions, compare arguments - // if (IsFunctionExpression) + // if (!IsLiteralValue) // { // if (other.Args == null || Args.Count != other.Args.Count) // return false; @@ -181,7 +169,7 @@ public override bool Equals(object? obj) public override int GetHashCode() { throw new NotImplementedException(); - // if (IsFunctionExpression) + // if (!IsLiteralValue) // { // var hash = Function.GetHashCode(); // if (Args != null) diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/ExpressionEvaluatorTests/ExpressionTests.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/ExpressionEvaluatorTests/ExpressionTests.cs index cf2c05f622..9ac8a44545 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/ExpressionEvaluatorTests/ExpressionTests.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/ExpressionEvaluatorTests/ExpressionTests.cs @@ -61,14 +61,14 @@ public void TestShortcuts() { var trueExpression = Expression.True; Assert.True(trueExpression.ValueUnion.Bool); - Assert.False(trueExpression.IsFunctionExpression); + Assert.True(trueExpression.IsLiteralValue); var falseExpression = Expression.False; Assert.False(falseExpression.ValueUnion.Bool); - Assert.False(falseExpression.IsFunctionExpression); + Assert.True(falseExpression.IsLiteralValue); var nullExpression = Expression.Null; Assert.Equal(JsonValueKind.Null, nullExpression.ValueUnion.ValueKind); - Assert.False(nullExpression.IsFunctionExpression); + Assert.True(nullExpression.IsLiteralValue); } } From 24c36b57ff56b57fe95f6d092fc2629f495cb5c9 Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Mon, 3 Nov 2025 20:21:44 +0100 Subject: [PATCH 3/5] Remvoe extra using --- src/Altinn.App.Core/Models/Expressions/Expression.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Altinn.App.Core/Models/Expressions/Expression.cs b/src/Altinn.App.Core/Models/Expressions/Expression.cs index c7b180690f..17f00d853e 100644 --- a/src/Altinn.App.Core/Models/Expressions/Expression.cs +++ b/src/Altinn.App.Core/Models/Expressions/Expression.cs @@ -1,4 +1,3 @@ -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Serialization; From 584f48a6af8792c4e45520bdc79c4bae74cd7bac Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Mon, 3 Nov 2025 20:38:54 +0100 Subject: [PATCH 4/5] Update public api verification --- .../Internal/Expressions/LayoutEvaluatorState.cs | 2 +- ...Tests.PublicApi_ShouldNotChange_Unintentionally.verified.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs index c0c6564a24..2a58f33800 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs @@ -270,7 +270,7 @@ public async Task AddInidicies(ModelBinding binding, ComponentCon $"Failed to add indexes to path {binding.Field} with indexes " + $"{(context.RowIndices is null ? "null" : string.Join(", ", context.RowIndices))} on {dataElementId}" ); - ; + return new DataReference() { Field = field, DataElementIdentifier = dataElementId }; } diff --git a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt index f51228b590..ecec455594 100644 --- a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt +++ b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt @@ -4435,7 +4435,7 @@ namespace Altinn.App.Core.Models.Expressions public Altinn.App.Core.Models.Expressions.Expression[]? Args { get; } public Altinn.App.Core.Models.Expressions.ExpressionFunction Function { get; } [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Args")] - [System.Obsolete("Use IsLiteralExpression instead")] + [System.Obsolete("Use !IsLiteralValue instead")] [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Args")] public bool IsFunctionExpression { get; } public bool IsLiteralString { get; } From 231708e59e20ab0a306fc7f0d45a2e5727a764a2 Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Mon, 3 Nov 2025 21:06:08 +0100 Subject: [PATCH 5/5] Kudos for Coderabbit to notice a bug in a comment! --- src/Altinn.App.Core/Models/Expressions/Expression.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Altinn.App.Core/Models/Expressions/Expression.cs b/src/Altinn.App.Core/Models/Expressions/Expression.cs index 17f00d853e..28ed5a517b 100644 --- a/src/Altinn.App.Core/Models/Expressions/Expression.cs +++ b/src/Altinn.App.Core/Models/Expressions/Expression.cs @@ -130,7 +130,8 @@ public Expression(ExpressionFunction function, Expression arg1, Expression arg2) public bool IsLiteralString => ValueUnion.ValueKind == JsonValueKind.String; /// - /// We have a custom JsonSerializer so this will return the json array representation of this expression + /// The custom is a + /// that serializes the expression to JSON (array for function or literal value). /// public override string ToString() {