Fix code generation for a list of lists inside a conditional or switch

rune-integration-tests/src/test/java/com/regnosys/rosetta/generator/java/expression/ListOfListsTest.java

@@ -0,0 +1,107 @@
+package com.regnosys.rosetta.generator.java.expression;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.eclipse.xtext.testing.InjectWith;
+import org.eclipse.xtext.testing.extensions.InjectionExtension;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+import com.regnosys.rosetta.generator.java.types.JavaTypeUtil;
+import com.regnosys.rosetta.tests.RosettaTestInjectorProvider;
+import com.regnosys.rosetta.tests.testmodel.JavaTestModel;
+import com.regnosys.rosetta.tests.testmodel.RosettaTestModelService;
+
+/**
+ * Tests for expressions that produce a list of lists, in particular when such an expression
+ * appears in a branch of a conditional or of a switch.
+ */
+@ExtendWith(InjectionExtension.class)
+@InjectWith(RosettaTestInjectorProvider.class)
+public class ListOfListsTest {
+    @Inject
+    private RosettaTestModelService modelService;
+    @Inject
+    private JavaTypeUtil typeUtil;
+
+    private static final String FOOS = """
+            [Foo { xs: ["a", "b"] }, Foo { xs: ["c"] }]
+            """;
+
+    @SuppressWarnings("unchecked")
+    private List<String> evaluateStringList(JavaTestModel model, String expr) {
+        return (List<String>) model.evaluateExpression(typeUtil.wrap(typeUtil.LIST, typeUtil.STRING), expr);
+    }
+
+    @Test
+    void flattenConditionalContainingListOfLists() {
+        JavaTestModel model = modelService.toJavaTestModel("""
+                type Foo:
+                    xs string (0..*)
+
+                func GetStrings:
+                    inputs:
+                        foos Foo (0..*)
+                        test boolean (1..1)
+                    output:
+                        result string (0..*)
+
+                    add result:
+                        (if test then foos extract item -> xs) flatten
+                """).compile();
+
+        assertEquals(List.of("a", "b", "c"), evaluateStringList(model, "GetStrings(" + FOOS + ", True)"));
+        assertEquals(List.of(), evaluateStringList(model, "GetStrings(" + FOOS + ", False)"));
+    }
+
+    @Test
+    void thenFlattenAfterConditionalContainingListOfLists() {
+        JavaTestModel model = modelService.toJavaTestModel("""
+                type Foo:
+                    xs string (0..*)
+
+                func GetStrings:
+                    inputs:
+                        foos Foo (0..*)
+                        test boolean (1..1)
+                    output:
+                        result string (0..*)
+
+                    add result:
+                        if test
+                        then foos extract item -> xs
+                        then flatten
+                """).compile();
+
+        assertEquals(List.of("a", "b", "c"), evaluateStringList(model, "GetStrings(" + FOOS + ", True)"));
+        assertEquals(List.of(), evaluateStringList(model, "GetStrings(" + FOOS + ", False)"));
+    }
+
+    @Test
+    void flattenSwitchContainingListOfLists() {
+        JavaTestModel model = modelService.toJavaTestModel("""
+                type Foo:
+                    xs string (0..*)
+
+                func GetStrings:
+                    inputs:
+                        foos Foo (0..*)
+                        mode string (1..1)
+                    output:
+                        result string (0..*)
+
+                    add result:
+                        (mode switch
+                            "all" then foos extract item -> xs,
+                            default empty)
+                            flatten
+                """).compile();
+
+        assertEquals(List.of("a", "b", "c"), evaluateStringList(model, "GetStrings(" + FOOS + ", \"all\")"));
+        assertEquals(List.of(), evaluateStringList(model, "GetStrings(" + FOOS + ", \"none\")"));
+    }
+}

rune-integration-tests/src/test/java/com/regnosys/rosetta/validation/ExpressionValidatorTest.java

@@ -17,7 +17,7 @@
 
 @ExtendWith(InjectionExtension.class)
 @InjectWith(RosettaTestInjectorProvider.class)
-public class ExpressionValidatorTest {
+public class ExpressionValidatorTest extends AbstractValidatorTest {
     @Inject
     private RosettaValidationTestHelper validationTestHelper;
     @Inject
@@ -417,4 +417,137 @@ void asOnUnsupportedTypeShouldError() {
         validationTestHelper.assertError(expr, AS_OPERATION, null,
                 "Operator `as` is not supported for type `string`. Supported argument types are complex types and choice types");
     }
+
+    @Test
+    void listOfListsInConditionalBranchShouldError() {
+        assertIssues("""
+                type Foo:
+                    xs string (0..*)
+
+                func GetStrings:
+                    inputs:
+                        foos Foo (0..*)
+                        test boolean (1..1)
+                    output:
+                        result string (0..*)
+
+                    add result:
+                        if test
+                        then foos extract item -> xs
+                """, """
+                ERROR (null) 'Assign expression contains a list of lists, use flatten to create a list' at 15:9, length 44, on Operation
+                """);
+    }
+
+    @Test
+    void listOfListsInSwitchCaseShouldError() {
+        assertIssues("""
+                type Foo:
+                    xs string (0..*)
+
+                func GetStrings:
+                    inputs:
+                        foos Foo (0..*)
+                        mode string (1..1)
+                    output:
+                        result string (0..*)
+
+                    add result:
+                        mode switch
+                            "all" then foos extract item -> xs,
+                            default empty
+                """, """
+                ERROR (null) 'Assign expression contains a list of lists, use flatten to create a list' at 15:9, length 85, on Operation
+                """);
+    }
+
+    @Test
+    void listOfListsInOnlyOneConditionalBranchShouldError() {
+        RosettaExpression expr = modelService.toTestModel("""
+                type Foo:
+                    xs string (0..*)
+                """).parseExpression("""
+                (if test then foos extract item -> xs else foos -> xs) flatten
+                """, "foos Foo (0..*)", "test boolean (1..1)");
+
+        validationTestHelper.assertError(expr, ROSETTA_CONDITIONAL_EXPRESSION, null,
+                "Branch contains a list of lists, use flatten to create a list.");
+    }
+
+    @Test
+    void listOfListsInOnlyOneSwitchCaseShouldError() {
+        RosettaExpression expr = modelService.toTestModel("""
+                type Foo:
+                    xs string (0..*)
+                """).parseExpression("""
+                (mode switch
+                    "all" then foos extract item -> xs,
+                    default foos -> xs)
+                    flatten
+                """, "foos Foo (0..*)", "mode string (1..1)");
+
+        validationTestHelper.assertError(expr, SWITCH_CASE_OR_DEFAULT, null,
+                "Branch contains a list of lists, use flatten to create a list.");
+    }
+
+    @Test
+    void listOfListsAsListLiteralElementShouldError() {
+        RosettaExpression expr = modelService.toTestModel("""
+                type Foo:
+                    xs string (0..*)
+                """).parseExpression("""
+                ["a", foos extract item -> xs]
+                """, "foos Foo (0..*)");
+
+        validationTestHelper.assertError(expr, LIST_LITERAL, null,
+                "List element contains a list of lists, use flatten to create a list.");
+    }
+
+    @Test
+    void listOfListsAsConstructorValueShouldError() {
+        RosettaExpression expr = modelService.toTestModel("""
+                type Foo:
+                    xs string (0..*)
+                """).parseExpression("""
+                Foo { xs: foos extract item -> xs }
+                """, "foos Foo (0..*)");
+
+        validationTestHelper.assertError(expr, CONSTRUCTOR_KEY_VALUE_PAIR, null,
+                "Attribute value contains a list of lists, use flatten to create a list.");
+    }
+
+    @Test
+    void listOfListsAsOperandOfDefaultOperationShouldError() {
+        RosettaExpression expr = modelService.toTestModel("""
+                type Foo:
+                    xs string (0..*)
+                """).parseExpression("""
+                (foos extract item -> xs) default empty
+                """, "foos Foo (0..*)");
+
+        validationTestHelper.assertError(expr, DEFAULT_OPERATION, null,
+                "Left operand contains a list of lists, use flatten to create a list.");
+    }
+
+    @Test
+    void listOfListsAsFunctionArgumentShouldError() {
+        RosettaExpression expr = modelService.toTestModel("""
+                type Foo:
+                    xs string (0..*)
+
+                func Identity:
+                    inputs:
+                        strings string (0..*)
+                    output:
+                        result string (0..*)
+
+                    add result:
+                        strings
+                """).parseExpression("""
+                Identity(foos extract item -> xs)
+                """, "foos Foo (0..*)");
+
+        validationTestHelper.assertError(expr, ROSETTA_SYMBOL_REFERENCE, null,
+                "Argument contains a list of lists, use flatten to create a list.");
+    }
 }

rune-lang/src/main/java/com/regnosys/rosetta/generator/java/expression/ExpressionGenerator.xtend

@@ -1156,7 +1156,7 @@ class ExpressionGenerator extends RosettaExpressionSwitch<JavaStatementBuilder,
 	}
 
 	override protected caseThenOperation(ThenOperation expr, Context context) {
-		val thenArgCode = expr.argument.javaCode(context.withExpected(expr.argument.isMulti ? MAPPER_C.wrapExtends(expr.argument) as JavaType : MAPPER_S.wrapExtends(expr.argument)))
+		val thenArgCode = expr.argument.javaCode(context.withExpected(expr.argument.wrapperTypeOfExpression))
 		val thenAsVarCode = thenArgCode.declareAsVariable(true, "thenArg", context.scope)
 		if (expr.function.parameters.size == 0) {
 			context.scope.createKeySynonym(expr.function.implicitVarInContext, thenArgCode)
@@ -1165,11 +1165,24 @@ class ExpressionGenerator extends RosettaExpressionSwitch<JavaStatementBuilder,
 		}
 		thenAsVarCode
 			.then(
-				expr.function.body.javaCode(context.withExpected(expr.isMulti ? MAPPER_C.wrapExtends(expr) as JavaType : MAPPER_S.wrapExtends(expr))),
+				expr.function.body.javaCode(context.withExpected(expr.wrapperTypeOfExpression)),
 				[a, b| b],
 				context.scope
 			)
 	}
+	/**
+	 * The Java wrapper type that represents the value of the given expression:
+	 * a `MapperListOfLists` for a list of lists, a `MapperC` for a list, and a `MapperS` otherwise.
+	 */
+	private def JavaType wrapperTypeOfExpression(RosettaExpression expr) {
+		if (expr.isOutputListOfLists) {
+			MAPPER_LIST_OF_LISTS.wrapExtends(expr) as JavaType
+		} else if (expr.isMulti) {
+			MAPPER_C.wrapExtends(expr) as JavaType
+		} else {
+			MAPPER_S.wrapExtends(expr) as JavaType
+		}
+	}
 
 	private def JavaStatementBuilder conversionOperation(RosettaUnaryOperation expr, Context context, StringConcatenationClient conversion, Class<? extends Exception> errorClass) {
 		val argumentJavaType = typeProvider.getRMetaAnnotatedType(expr.argument).RType.toJavaReferenceType

rune-lang/src/main/java/com/regnosys/rosetta/generator/java/expression/TypeCoercionService.xtend

@@ -12,6 +12,7 @@ import com.regnosys.rosetta.generator.java.types.JavaTypeUtil
 import com.regnosys.rosetta.generator.java.types.RJavaWithMetaValue
 import com.rosetta.model.lib.expression.ComparisonResult
 import com.rosetta.model.lib.mapper.MapperC
+import com.rosetta.model.lib.mapper.MapperListOfLists
 import com.rosetta.model.lib.mapper.MapperS
 import com.rosetta.util.types.JavaPrimitiveType
 import com.rosetta.util.types.JavaReferenceType
@@ -382,6 +383,8 @@ class TypeCoercionService {
 			JavaExpression.from('''«MapperS».<«itemType»>ofNull()''', MAPPER_S.wrap(itemType))
 		} else if (expected.isMapperC) {
 			JavaExpression.from('''«MapperC».<«itemType»>ofNull()''', MAPPER_C.wrap(itemType))
+		} else if (expected.isMapperListOfLists) {
+			JavaExpression.from('''«MapperListOfLists».<«itemType»>of(«Collections».emptyList())''', MAPPER_LIST_OF_LISTS.wrap(itemType))
 		} else if (expected.isComparisonResult) {
 			JavaExpression.from('''«ComparisonResult».ofEmpty()''', COMPARISON_RESULT)
 		} else if (expected == JavaPrimitiveType.BOOLEAN) {

rune-lang/src/main/java/com/regnosys/rosetta/types/CardinalityProvider.java

@@ -217,7 +217,9 @@ public boolean isOutputListOfLists(RosettaExpression expr) {
         return safeIsOutputListOfLists(expr, new HashMap<>());
     }
 	private boolean safeIsOutputListOfLists(RosettaExpression expr, Map<RosettaSymbol, Boolean> cycleTracker) {
-        if (expr instanceof FlattenOperation) {
+        if (expr == null) {
+            return false;
+        } else if (expr instanceof FlattenOperation) {
             return false;
         } else if (expr instanceof MapOperation) {
             MapOperation mapOperation = (MapOperation) expr;
@@ -250,6 +252,19 @@ private boolean safeIsOutputListOfLists(RosettaExpression expr, Map<RosettaSymbo
         } else if (expr instanceof CanHandleListOfLists) {
             CanHandleListOfLists listExpression = (CanHandleListOfLists) expr;
             return safeIsOutputListOfLists(listExpression.getArgument(), cycleTracker);
+        } else if (expr instanceof RosettaConditionalExpression) {
+            // A conditional yields a list of lists if any of its branches does.
+            RosettaConditionalExpression conditional = (RosettaConditionalExpression) expr;
+            return safeIsOutputListOfLists(conditional.getIfthen(), cycleTracker)
+                    || safeIsOutputListOfLists(conditional.getElsethen(), cycleTracker);
+        } else if (expr instanceof SwitchOperation) {
+            // A switch yields a list of lists if any of its cases does.
+            for (SwitchCaseOrDefault switchCase : ((SwitchOperation) expr).getCases()) {
+                if (safeIsOutputListOfLists(switchCase.getExpression(), cycleTracker)) {
+                    return true;
+                }
+            }
+            return false;
         }
         return false;
     }

rune-lang/src/main/java/com/regnosys/rosetta/validation/expression/AbstractExpressionValidator.java

@@ -144,6 +144,25 @@ protected boolean isSingleCheck(RosettaExpression expr, EObject sourceObject, ES
 		return true;
 	}
 
+	/**
+	 * Check that the given expression is not a list of lists. Only the operations that explicitly
+	 * support a list of lists (see {@code CanHandleListOfLists}) and the branches of a conditional
+	 * or switch expression can handle one - anywhere else it must be flattened first.
+	 *
+	 * @param description how the expression is used, e.g. "Argument". It is the subject of the error message.
+	 */
+	protected boolean isNotListOfListsCheck(RosettaExpression expr, EObject sourceObject, EStructuralFeature feature, String description) {
+		return isNotListOfListsCheck(expr, sourceObject, feature, INSIGNIFICANT_INDEX, description);
+	}
+
+	protected boolean isNotListOfListsCheck(RosettaExpression expr, EObject sourceObject, EStructuralFeature feature, int featureIndex, String description) {
+		if (expr != null && cardinalityProvider.isOutputListOfLists(expr)) {
+			error(description + " contains a list of lists, use flatten to create a list.", sourceObject, feature, featureIndex);
+			return false;
+		}
+		return true;
+	}
+
 	protected boolean commonTypeCheck(RosettaExpression expr1, RosettaExpression expr2, EObject sourceObject, EStructuralFeature feature) {
 		if (expr1 == null || expr2 == null) {
 			return true;

rune-lang/src/main/java/com/regnosys/rosetta/validation/expression/ConstructorValidator.java

@@ -56,6 +56,7 @@ public void checkConstructorExpression(RosettaConstructorExpression ele) {
 				if (!cardinalityProvider.isFeatureMulti(feature)) {
 					isSingleCheck(expr, pair, CONSTRUCTOR_KEY_VALUE_PAIR__VALUE, "Cannot assign a list to a single value");
 				}
+				isNotListOfListsCheck(expr, pair, CONSTRUCTOR_KEY_VALUE_PAIR__VALUE, "Attribute value");
 			}
 		}