Fix rename

vtl-engine/src/main/java/fr/insee/vtl/engine/VtlScriptEngine.java

@@ -20,6 +20,7 @@
 import java.util.stream.Stream;
 import javax.script.*;
 import org.antlr.v4.runtime.*;
+import org.antlr.v4.runtime.misc.Interval;
 import org.antlr.v4.runtime.tree.ParseTree;
 import org.antlr.v4.runtime.tree.TerminalNode;
 
@@ -58,16 +59,37 @@ public VtlScriptEngine(ScriptEngineFactory factory) {
     this.factory = factory;
   }
 
+  public static Positioned toPositioned(ParseTree tree) {
+    return fromContext(tree);
+  }
+
+  public static Positioned toPositioned(Token tree) {
+    return fromToken(tree);
+  }
+
+  /**
+   * Convert a Token to Positioned.
+   *
+   * @deprecated This method is no longer acceptable to compute time between versions.
+   *     <p>Use {@link VtlScriptEngine#toPositioned(Token)} instead.
+   */
   public static Positioned fromToken(Token token) {
     Positioned.Position position =
         new Positioned.Position(
+            token.getText(),
             token.getLine() - 1,
             token.getLine() - 1,
             token.getCharPositionInLine(),
             token.getCharPositionInLine() + (token.getStopIndex() - token.getStartIndex() + 1));
     return () -> position;
   }
 
+  /**
+   * Convert a ParseTree to Positioned.
+   *
+   * @deprecated This method is no longer acceptable to compute time between versions.
+   *     <p>Use {@link VtlScriptEngine#toPositioned(ParseTree)} instead.
+   */
   public static Positioned fromContext(ParseTree tree) {
     if (tree instanceof ParserRuleContext parserRuleContext) {
       return fromTokens(parserRuleContext.getStart(), parserRuleContext.getStop());
@@ -82,8 +104,11 @@ public static Positioned fromTokens(Token from, Token to) {
     if (to == null) {
       to = from;
     }
+    var stream = from.getInputStream();
+    var text = stream.getText(new Interval(from.getStartIndex(), to.getStopIndex()));
     var position =
         new Positioned.Position(
+            text,
             from.getLine() - 1,
             to.getLine() - 1,
             from.getCharPositionInLine(),
@@ -218,7 +243,8 @@ public void syntaxError(
                   errors.add(new VtlSyntaxException(msg, fromToken(offendingSymbolToken)));
                 } else {
                   var pos =
-                      new Positioned.Position(startLine, startLine, startColumn, startColumn + 1);
+                      new Positioned.Position(
+                          "", startLine, startLine, startColumn, startColumn + 1);
                   errors.add(new VtlScriptException(msg, () -> pos));
                 }
               }

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

@@ -18,4 +18,15 @@ public class UndefinedVariableException extends VtlScriptException {
   public UndefinedVariableException(String name, Positioned position) {
     super("undefined variable %s".formatted(name), position);
   }
+
+  public UndefinedVariableException(Positioned identifier, Positioned container) {
+    super(
+        "undefined variable '%s' in '%s'"
+            .formatted(identifier.getPosition().text(), container.getPosition().text()),
+        identifier);
+  }
+
+  public UndefinedVariableException(Positioned identifier) {
+    super("undefined variable '%s'".formatted(identifier.getPosition().text()), identifier);
+  }
 }

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

@@ -1,10 +1,12 @@
 package fr.insee.vtl.engine.visitors;
 
 import static fr.insee.vtl.engine.VtlScriptEngine.fromContext;
+import static fr.insee.vtl.engine.VtlScriptEngine.toPositioned;
 import static fr.insee.vtl.engine.utils.TypeChecking.assertBasicScalarType;
 import static fr.insee.vtl.engine.utils.TypeChecking.assertNumber;
 
 import fr.insee.vtl.engine.VtlScriptEngine;
+import fr.insee.vtl.engine.exceptions.AlreadyDefinedException;
 import fr.insee.vtl.engine.exceptions.InvalidArgumentException;
 import fr.insee.vtl.engine.exceptions.UndefinedVariableException;
 import fr.insee.vtl.engine.exceptions.VtlRuntimeException;
@@ -124,7 +126,6 @@ public DatasetExpression visitKeepOrDropClause(VtlParser.KeepOrDropClauseContext
     var structure = datasetExpression.getDataStructure();
 
     // Evaluate that all requested columns must exist in the dataset or raise an error
-    // TODO: Is that no handled already?
     for (String col : columns.keySet()) {
       if (!structure.containsKey(col)) {
         throw new VtlRuntimeException(
@@ -133,7 +134,6 @@ public DatasetExpression visitKeepOrDropClause(VtlParser.KeepOrDropClauseContext
     }
 
     // VTL specification: identifiers must not appear explicitly in KEEP
-    // TODO: Use multi errors that noah created?
     for (String col : columns.keySet()) {
       if (structure.get(col).isIdentifier()) {
         throw new VtlRuntimeException(
@@ -166,45 +166,106 @@ public DatasetExpression visitKeepOrDropClause(VtlParser.KeepOrDropClauseContext
   @Override
   public DatasetExpression visitCalcClause(VtlParser.CalcClauseContext ctx) {
 
-    var expressions = new LinkedHashMap<String, ResolvableExpression>();
-    var expressionStrings = new LinkedHashMap<String, String>();
-    var roles = new LinkedHashMap<String, Dataset.Role>();
-    var currentDatasetExpression = datasetExpression;
-    // TODO: Refactor so we call the executeCalc for each CalcClauseItemContext the same way we call
-    // the
-    //  analytics functions.
+    // Dataset structure (ordered) and quick lookups
+    final List<Dataset.Component> componentsInOrder =
+        new ArrayList<>(datasetExpression.getDataStructure().values());
+
+    final Map<String, Dataset.Component> byName =
+        componentsInOrder.stream()
+            .collect(
+                Collectors.toMap(
+                    Dataset.Component::getName, c -> c, (a, b) -> a, LinkedHashMap::new));
+
+    // Accumulators for non-analytic calc items
+    final LinkedHashMap<String, ResolvableExpression> expressions = new LinkedHashMap<>();
+    final LinkedHashMap<String, String> expressionStrings = new LinkedHashMap<>();
+    final LinkedHashMap<String, Dataset.Role> roles = new LinkedHashMap<>();
+
+    // Tracks duplicates in the same clause (target names)
+    final Set<String> targetsSeen = new LinkedHashSet<>();
+
+    // We need a rolling dataset expression to chain analytics items
+    DatasetExpression currentDatasetExpression = datasetExpression;
+
+    // TODO: Refactor so we call executeCalc per CalcClauseItemContext (as analytics do).
     for (VtlParser.CalcClauseItemContext calcCtx : ctx.calcClauseItem()) {
-      var columnName = getName(calcCtx.componentID());
-      var columnRole =
-          calcCtx.componentRole() == null
+
+      // ----  Resolve target name and desired role ----
+      final String columnName = getName(calcCtx.componentID());
+      final Dataset.Role columnRole =
+          (calcCtx.componentRole() == null)
               ? Dataset.Role.MEASURE
               : Dataset.Role.valueOf(calcCtx.componentRole().getText().toUpperCase());
 
-      if ((calcCtx.expr() instanceof VtlParser.FunctionsExpressionContext)
-          && ((VtlParser.FunctionsExpressionContext) calcCtx.expr()).functions()
-              instanceof VtlParser.AnalyticFunctionsContext) {
-        AnalyticsVisitor analyticsVisitor =
+      // If the target already exists in the dataset, check its role
+      final Dataset.Component existing = byName.get(columnName);
+      if (existing != null) {
+        // Explicitly block overwriting identifiers (already handled above if role==IDENTIFIER).
+        if (existing.getRole() == Dataset.Role.IDENTIFIER) {
+          final String meta =
+              String.format(
+                  "(role=%s, type=%s)",
+                  existing.getRole(), existing.getType() != null ? existing.getType() : "n/a");
+          throw new VtlRuntimeException(
+              new InvalidArgumentException(
+                  // TODO: see if other cases are the same error (already defined in assignment for
+                  // example).
+                  String.format("CALC cannot overwrite IDENTIFIER '%s' %s.", columnName, meta),
+                  fromContext(ctx)));
+        }
+      }
+
+      // ---- Dispatch: analytics vs. regular calc ----
+      final boolean isAnalytic =
+          (calcCtx.expr() instanceof VtlParser.FunctionsExpressionContext)
+              && ((VtlParser.FunctionsExpressionContext) calcCtx.expr()).functions()
+                  instanceof VtlParser.AnalyticFunctionsContext;
+
+      if (isAnalytic) {
+        // Analytics are executed immediately and update the rolling dataset expression
+        final AnalyticsVisitor analyticsVisitor =
             new AnalyticsVisitor(processingEngine, currentDatasetExpression, columnName);
-        VtlParser.FunctionsExpressionContext functionExprCtx =
+        final VtlParser.FunctionsExpressionContext functionExprCtx =
             (VtlParser.FunctionsExpressionContext) calcCtx.expr();
-        VtlParser.AnalyticFunctionsContext anFuncCtx =
+        final VtlParser.AnalyticFunctionsContext anFuncCtx =
             (VtlParser.AnalyticFunctionsContext) functionExprCtx.functions();
+
         currentDatasetExpression = analyticsVisitor.visit(anFuncCtx);
       } else {
-        ResolvableExpression calc = componentExpressionVisitor.visit(calcCtx);
+        // Regular calc expression – build resolvable expression and capture its source text
+        final ResolvableExpression calc = componentExpressionVisitor.visit(calcCtx);
+
+        final String exprSource = getSource(calcCtx.expr());
+        if (exprSource == null || exprSource.isEmpty()) {
+          throw new VtlRuntimeException(
+              new InvalidArgumentException(
+                  String.format(
+                      "empty or unavailable source expression for '%s' in CALC.", columnName),
+                  fromContext(ctx)));
+        }
 
+        // Store in insertion order (deterministic column creation)
         expressions.put(columnName, calc);
-        expressionStrings.put(columnName, getSource(calcCtx.expr()));
+        expressionStrings.put(columnName, exprSource);
         roles.put(columnName, columnRole);
       }
     }
 
+    // ---- Consistency checks before execution ----
+    if (!(expressions.keySet().equals(expressionStrings.keySet())
+        && expressions.keySet().equals(roles.keySet()))) {
+      throw new VtlRuntimeException(
+          new InvalidArgumentException(
+              "internal CALC maps out of sync (expressions/expressionStrings/roles)",
+              fromContext(ctx)));
+    }
+
+    // ---- Execute the batch calc if any non-analytic expressions were collected ----
     if (!expressionStrings.isEmpty()) {
       currentDatasetExpression =
           processingEngine.executeCalc(
               currentDatasetExpression, expressions, roles, expressionStrings);
     }
-
     return currentDatasetExpression;
   }
 
@@ -216,18 +277,74 @@ public DatasetExpression visitFilterClause(VtlParser.FilterClauseContext ctx) {
 
   @Override
   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));
+
+    var structure = datasetExpression.getDataStructure();
+
+    // Parse the RENAME clause and validate
     Map<String, String> fromTo = new LinkedHashMap<>();
-    Set<String> renamed = new HashSet<>();
+    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()) {
-      var toNameString = getName(renameCtx.toName);
-      var fromNameString = getName(renameCtx.fromName);
-      if (!renamed.add(toNameString)) {
+      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);
+
+      // Validate: no duplicate "from" names inside the clause
+      if (!fromSeen.add(fromNameString)) {
         throw new VtlRuntimeException(
             new InvalidArgumentException(
-                "duplicate column: %s".formatted(toNameString), fromContext(renameCtx)));
+                "duplicate from name '%s'".formatted(renameCtx.fromName.getText()),
+                toPositioned(renameCtx.fromName)));
+      }
+
+      // Validate: "from" must exist in dataset
+      if (!structure.containsKey(fromNameString)) {
+        throw new VtlRuntimeException(
+            new UndefinedVariableException(toPositioned(renameCtx.fromName), datasetExpression));
+      }
+
+      // Validate: no duplicate "to" names inside the clause
+      if (!toSeen.add(toNameString)) {
+        throw new VtlRuntimeException(
+            new AlreadyDefinedException(toPositioned(renameCtx.toName), datasetExpression));
       }
+
       fromTo.put(fromNameString, toNameString);
     }
+
+    // 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 =
+        structure.keySet().stream()
+            .filter(c -> !fromTo.containsKey(c))
+            .collect(Collectors.toCollection(LinkedHashSet::new));
+
+    for (Map.Entry<String, String> e : fromTo.entrySet()) {
+      final String from = e.getKey();
+      final String to = e.getValue();
+
+      // If target already exists as untouched, it would cause a collision
+      if (untouched.contains(to)) {
+        throw new VtlRuntimeException(
+            new AlreadyDefinedException(toPositioned(toCtxMap.get(to)), datasetExpression));
+      }
+    }
+
+    // Execute rename in processing engine
     return processingEngine.executeRename(datasetExpression, fromTo);
   }
 

vtl-engine/src/test/java/fr/insee/vtl/engine/utils/TypeCheckingUtilsTest.java

@@ -14,7 +14,7 @@ public class TypeCheckingUtilsTest {
       new Positioned() {
         @Override
         public Position getPosition() {
-          return new Position(1, 1, 1, 1);
+          return new Position("", 1, 1, 1, 1);
         }
       };
 

vtl-engine/src/test/java/fr/insee/vtl/engine/utils/dag/DagTest.java

@@ -148,7 +148,7 @@ private static Positioned.Position getPositionOfStatementInScript(
       }
     }
 
-    return new Positioned.Position(startLine, endLine, startColumn, endColumn);
+    return new Positioned.Position(statement, startLine, endLine, startColumn, endColumn);
   }
 
   @BeforeEach