fix: DAG scopes #457

Author: noahboerger

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

@@ -9,6 +9,7 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import org.antlr.v4.runtime.RuleContext;
 import org.antlr.v4.runtime.Token;
 import org.antlr.v4.runtime.tree.RuleNode;
 import org.antlr.v4.runtime.tree.TerminalNode;
@@ -154,10 +155,10 @@ protected List<DAGStatement> defaultResult() {
    * <code>VarIDsExtractingVisitor</code> is the visitor for extracting the used VarIds from VTL
    * statements.
    */
-  private static class IdentifierExtractingVisitor
-      extends VtlBaseVisitor<Set<DAGStatement.Identifier>> {
+  static class IdentifierExtractingVisitor extends VtlBaseVisitor<Set<DAGStatement.Identifier>> {
 
     private final Set<String> ignoreInnerScopedVarIdentifiers;
+    private int componentContextDepth = 0;
 
     public IdentifierExtractingVisitor() {
       this(Set.of());
@@ -168,51 +169,127 @@ public IdentifierExtractingVisitor(Set<String> ignoreInnerScopedVarIdentifiers)
       this.ignoreInnerScopedVarIdentifiers = ignoreInnerScopedVarIdentifiers;
     }
 
+    private Set<DAGStatement.Identifier> enterRestrictedContext(RuleContext ctx) {
+      componentContextDepth++;
+      Set<DAGStatement.Identifier> result = super.visitChildren(ctx);
+      componentContextDepth--;
+      return result;
+    }
+
     @Override
     public Set<DAGStatement.Identifier> visitVarID(VtlParser.VarIDContext node) {
       final var currentVarIdentifier = node.IDENTIFIER().getSymbol().getText();
-      final Set<DAGStatement.Identifier> thisResult =
-          ignoreInnerScopedVarIdentifiers.contains(currentVarIdentifier)
-              ? Set.of()
-              : Set.of(
-                  new DAGStatement.Identifier(
-                      DAGStatement.Identifier.Type.VARIABLE, currentVarIdentifier));
-      final Set<DAGStatement.Identifier> subResult = this.visitChildren(node);
-      return aggregateResult(thisResult, subResult);
+
+      // If we are inside a component context (depth > 0), we ignore this identifier
+      // because it is a component name, not a dataset dependency, according to the unadjusted VTL
+      // syntax
+      if (componentContextDepth > 0) {
+        return Set.of();
+      }
+
+      return ignoreInnerScopedVarIdentifiers.contains(currentVarIdentifier)
+          ? Set.of()
+          : Set.of(
+              new DAGStatement.Identifier(
+                  DAGStatement.Identifier.Type.VARIABLE, currentVarIdentifier));
     }
 
+    // Workaround for https://github.com/InseeFr/Trevas/issues/457, as long the open points in here
+    // are not clarified and https://github.com/InseeFr/Trevas/issues/355 is not implemented
+    // If we are inside a component context (depth > 0), we ignore this identifier
+    // because it is a component name, not a dataset dependency, according to the unadjusted VTL
+    // syntax
     @Override
-    public Set<DAGStatement.Identifier> visitOperatorID(VtlParser.OperatorIDContext node) {
-      final Set<DAGStatement.Identifier> thisResult =
-          Set.of(
-              new DAGStatement.Identifier(
-                  DAGStatement.Identifier.Type.OPERATOR, node.IDENTIFIER().getSymbol().getText()));
-      final Set<DAGStatement.Identifier> subResult = this.visitChildren(node);
-      return aggregateResult(thisResult, subResult);
+    public Set<DAGStatement.Identifier> visitFilterClause(VtlParser.FilterClauseContext ctx) {
+      return enterRestrictedContext(ctx);
+    }
+
+    @Override
+    public Set<DAGStatement.Identifier> visitCalcClauseItem(VtlParser.CalcClauseItemContext ctx) {
+      return enterRestrictedContext(ctx);
+    }
+
+    @Override
+    public Set<DAGStatement.Identifier> visitHavingClause(VtlParser.HavingClauseContext ctx) {
+      return enterRestrictedContext(ctx);
+    }
+
+    @Override
+    public Set<DAGStatement.Identifier> visitJoinApplyClause(VtlParser.JoinApplyClauseContext ctx) {
+      return enterRestrictedContext(ctx);
+    }
+
+    @Override
+    public Set<DAGStatement.Identifier> visitAggrFunctionClause(
+        VtlParser.AggrFunctionClauseContext ctx) {
+      return enterRestrictedContext(ctx);
+    }
+
+    // Aggregate & Analytic Functions (First arg is Dataset, rest are components)
+    @Override
+    public Set<DAGStatement.Identifier> visitAggrDataset(VtlParser.AggrDatasetContext ctx) {
+      Set<DAGStatement.Identifier> datasetRef = visit(ctx.expr());
+      return aggregateResult(datasetRef, enterRestrictedContext(ctx));
+    }
+
+    @Override
+    public Set<DAGStatement.Identifier> visitAnSimpleFunction(
+        VtlParser.AnSimpleFunctionContext ctx) {
+      Set<DAGStatement.Identifier> datasetRef = visit(ctx.expr());
+      return aggregateResult(datasetRef, enterRestrictedContext(ctx));
+    }
+
+    @Override
+    public Set<DAGStatement.Identifier> visitLagOrLeadAn(VtlParser.LagOrLeadAnContext ctx) {
+      Set<DAGStatement.Identifier> datasetRef = visit(ctx.expr());
+      return aggregateResult(datasetRef, enterRestrictedContext(ctx));
+    }
+
+    @Override
+    public Set<DAGStatement.Identifier> visitRatioToReportAn(VtlParser.RatioToReportAnContext ctx) {
+      Set<DAGStatement.Identifier> datasetRef = visit(ctx.expr());
+      return aggregateResult(datasetRef, enterRestrictedContext(ctx));
+    }
+
+    @Override
+    public Set<DAGStatement.Identifier> visitRankAn(VtlParser.RankAnContext ctx) {
+
+      return enterRestrictedContext(ctx);
+    }
+
+    @Override
+    public Set<DAGStatement.Identifier> visitMembershipExpr(VtlParser.MembershipExprContext ctx) {
+      // Only visit the dataset (left side), ignore the component (right side)
+      return visit(ctx.expr());
     }
 
     @Override
     public Set<DAGStatement.Identifier> visitValidateDPruleset(
         VtlParser.ValidateDPrulesetContext node) {
-      final Set<DAGStatement.Identifier> thisResult =
+      Set<DAGStatement.Identifier> rulesetRef =
           Set.of(
               new DAGStatement.Identifier(
                   DAGStatement.Identifier.Type.RULESET_DATAPOINT,
                   node.IDENTIFIER().getSymbol().getText()));
-      final Set<DAGStatement.Identifier> subResult = this.visitChildren(node);
-      return aggregateResult(thisResult, subResult);
+      return aggregateResult(rulesetRef, visit(node.expr()));
     }
 
     @Override
     public Set<DAGStatement.Identifier> visitValidateHRruleset(
         VtlParser.ValidateHRrulesetContext node) {
-      final Set<DAGStatement.Identifier> thisResult =
+      Set<DAGStatement.Identifier> rulesetRef =
           Set.of(
               new DAGStatement.Identifier(
                   DAGStatement.Identifier.Type.RULESET_HIERARCHICAL,
                   node.IDENTIFIER().getSymbol().getText()));
-      final Set<DAGStatement.Identifier> subResult = this.visitChildren(node);
-      return aggregateResult(thisResult, subResult);
+      return aggregateResult(rulesetRef, visit(node.expr()));
+    }
+
+    @Override
+    public Set<DAGStatement.Identifier> visitOperatorID(VtlParser.OperatorIDContext node) {
+      return Set.of(
+          new DAGStatement.Identifier(
+              DAGStatement.Identifier.Type.OPERATOR, node.IDENTIFIER().getSymbol().getText()));
     }
 
     @Override

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

@@ -6,10 +6,12 @@
 import fr.insee.vtl.model.exceptions.VtlScriptException;
 import fr.insee.vtl.parser.VtlLexer;
 import fr.insee.vtl.parser.VtlParser;
-import java.util.*;
+import java.util.Set;
 import java.util.stream.Stream;
 import javax.script.ScriptException;
-import org.antlr.v4.runtime.*;
+import org.antlr.v4.runtime.CharStreams;
+import org.antlr.v4.runtime.CodePointCharStream;
+import org.antlr.v4.runtime.CommonTokenStream;
 import org.antlr.v4.runtime.tree.ParseTree;
 import org.antlr.v4.runtime.tree.RuleNode;
 import org.antlr.v4.runtime.tree.TerminalNode;
@@ -21,15 +23,19 @@ public class DagDefineStatementsTest {
 
   private static String performDagReordering(final String script, final Set<String> bindingVars)
       throws VtlScriptException {
-    final CodePointCharStream stream = CharStreams.fromString(script);
-    VtlLexer lexer = new VtlLexer(stream);
-    VtlParser parser = new VtlParser(new CommonTokenStream(lexer));
-    var start = parser.start();
+    VtlParser.StartContext start = parseScript(script);
     VtlSyntaxPreprocessor syntaxPreprocessor = new VtlSyntaxPreprocessor(start, bindingVars);
     VtlParser.StartContext res = syntaxPreprocessor.checkForMultipleAssignmentsAndReorderScript();
     return parseTreeToText(res);
   }
 
+  public static VtlParser.StartContext parseScript(String script) {
+    final CodePointCharStream stream = CharStreams.fromString(script);
+    VtlLexer lexer = new VtlLexer(stream);
+    VtlParser parser = new VtlParser(new CommonTokenStream(lexer));
+    return parser.start();
+  }
+
   private static String parseTreeToText(ParseTree child) {
     StringBuilder result = new StringBuilder();
     if (child instanceof TerminalNode) {

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

@@ -297,8 +297,6 @@ void testMultipleCycles() {
   void testDagDoubleAssignment() {
     ScriptContext context = engine.getContext();
     context.setAttribute("a", 1L, ScriptContext.ENGINE_SCOPE);
-    // Note that the double assignment is not detected while building the DAG but later during
-    // execution
     assertThatThrownBy(() -> engine.eval("b := a; b := 1;"))
         .isInstanceOf(VtlScriptException.class)
         .hasMessage("Dataset b has already been assigned");