Fix logic

Author: hadrienk

vtl-engine/src/main/java/fr/insee/vtl/engine/exceptions/AlreadyDefinedException.java

@@ -0,0 +1,28 @@
+package fr.insee.vtl.engine.exceptions;
+
+import fr.insee.vtl.model.Positioned;
+import fr.insee.vtl.model.exceptions.VtlScriptException;
+import java.util.Optional;
+
+public class AlreadyDefinedException extends VtlScriptException {
+
+  private static String format(Positioned identifier, Optional<Positioned> container) {
+    var msg = "'%s', is already defined".formatted(identifier.getPosition().text());
+
+    msg += container.map(c -> " in '%s'".formatted(c.getPosition().text())).orElse("");
+
+    return msg;
+  }
+
+  private AlreadyDefinedException(Positioned identifier, Optional<Positioned> container) {
+    super(format(identifier, container), identifier);
+  }
+
+  public AlreadyDefinedException(Positioned identifier) {
+    super(format(identifier, Optional.empty()), identifier);
+  }
+
+  public AlreadyDefinedException(Positioned identifier, Positioned container) {
+    super(format(identifier, Optional.of(container)), identifier);
+  }
+}

vtl-engine/src/main/java/fr/insee/vtl/engine/exceptions/UndefinedVariableException.java

@@ -21,7 +21,7 @@ public UndefinedVariableException(String name, Positioned position) {
 
   public UndefinedVariableException(Positioned identifier, Positioned container) {
     super(
-        "undefined variable '%s' in '%'"
+        "undefined variable '%s' in '%s'"
             .formatted(identifier.getPosition().text(), container.getPosition().text()),
         identifier);
   }

vtl-engine/src/main/java/fr/insee/vtl/engine/visitors/ClauseVisitor.java

@@ -279,19 +279,27 @@ public DatasetExpression visitFilterClause(VtlParser.FilterClauseContext ctx) {
   public DatasetExpression visitRenameClause(VtlParser.RenameClauseContext ctx) {
 
     // Dataset structure in order + lookup maps
-    final List<Dataset.Component> componentsInOrder =
-        new ArrayList<>(datasetExpression.getDataStructure().values());
-    final Set<String> availableColumns =
-        componentsInOrder.stream()
-            .map(Dataset.Component::getName)
-            .collect(Collectors.toCollection(LinkedHashSet::new));
+    //final List<Dataset.Component> componentsInOrder =
+    //    new ArrayList<>(datasetExpression.getDataStructure().values());
+    // final Set<String> availableColumns =
+    //    componentsInOrder.stream()
+    //        .map(Dataset.Component::getName)
+    //        .collect(Collectors.toCollection(LinkedHashSet::new));
+
+    var structure = datasetExpression.getDataStructure();
 
     // Parse the RENAME clause and validate
     Map<String, String> fromTo = new LinkedHashMap<>();
     Set<String> toSeen = new LinkedHashSet<>();
     Set<String> fromSeen = new LinkedHashSet<>();
 
+    Map<String, ParserRuleContext> toCtxMap = new HashMap<>();
+    Map<String, ParserRuleContext> fromCtxMap = new HashMap<>();
+
     for (VtlParser.RenameClauseItemContext renameCtx : ctx.renameClauseItem()) {
+      toCtxMap.put(getName(renameCtx.toName), renameCtx.toName);
+      fromCtxMap.put(getName(renameCtx.fromName), renameCtx.fromName);
+
       final String toNameString = getName(renameCtx.toName);
       final String fromNameString = getName(renameCtx.fromName);
 
@@ -304,7 +312,7 @@ public DatasetExpression visitRenameClause(VtlParser.RenameClauseContext ctx) {
       }
 
       // Validate: "from" must exist in dataset
-      if (!availableColumns.contains(fromNameString)) {
+      if (!structure.containsKey(fromNameString)) {
         throw new VtlRuntimeException(
             new UndefinedVariableException(toPositioned(renameCtx.fromName), datasetExpression));
       }
@@ -318,10 +326,10 @@ public DatasetExpression visitRenameClause(VtlParser.RenameClauseContext ctx) {
       fromTo.put(fromNameString, toNameString);
     }
 
-    // Validate collisions with untouched dataset columns ("Untouched" = columns that are not
-    // being renamed)
+    // Check that the renamed columns do not collide with the remaining columns. This
+    // is done so that swapping variables works: a -> b, b -> a.
     final Set<String> untouched =
-        availableColumns.stream()
+        structure.keySet().stream()
             .filter(c -> !fromTo.containsKey(c))
             .collect(Collectors.toCollection(LinkedHashSet::new));
 
@@ -331,20 +339,12 @@ public DatasetExpression visitRenameClause(VtlParser.RenameClauseContext ctx) {
 
       // If target already exists as untouched, it would cause a collision
       if (untouched.contains(to)) {
-        Dataset.Component comp = datasetExpression.getDataStructure().get(to);
-        String meta =
-            (comp != null)
-                ? String.format(
-                    " (role=%s, type=%s)",
-                    comp.getRole(), comp.getType() != null ? comp.getType() : "n/a")
-                : "";
-
         throw new VtlRuntimeException(
-            new InvalidArgumentException(
-                String.format(
-                    "Error: target name '%s'%s already exists in dataset and is not being renamed.",
-                    to, meta),
-                fromContext(ctx)));
+            new AlreadyDefinedException(
+                toPositioned(toCtxMap.get(to)),
+                datasetExpression
+            )
+        );
       }
     }