[cleanup] Clean variables initialized during direct edit

During analysis of issue #394 [1], it was observed that the variables
created during the execution of a direct edit tool ware not cleaned.
This commit fixes this problem and adds corresponding automatic tests.

[1]  #394

plugins/org.eclipse.sirius.diagram/src/org/eclipse/sirius/diagram/tools/internal/command/builders/DirectEditCommandBuilder.java

@@ -75,6 +75,7 @@ public Command buildCommand() {
             }
             addPostOperationTasks(result, interpreter);
             result.getTasks().add(new InitGlobalDirectEditVariablesTask(interpreter));
+            result.getTasks().add(new InitInterpreterFromParsedVariableTask(interpreter, messageFormat, newValue, true));
             return result;
         }
         return UnexecutableCommand.INSTANCE;

plugins/org.eclipse.sirius.table/src/org/eclipse/sirius/table/tools/internal/command/TableCommandFactory.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2023 THALES GLOBAL SERVICES and others.
+ * Copyright (c) 2007, 2024 THALES GLOBAL SERVICES and others.
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v2.0
  * which accompanies this distribution, and is available at
@@ -306,6 +306,8 @@ private SiriusCommand buildCommandFromCell(final DCell currentCell, final TableT
         addMaskInitTask(result, interpreterContext, mask, newValue);
         // Creation of the tasks to execute the tool
         addOperationTask(result, currentCell, interpreterContext, tool.getFirstModelOperation());
+        // Clear mask variables
+        addMaskResetTask(result, interpreterContext, mask, newValue);
 
         return result;
     }
@@ -338,6 +340,9 @@ private SiriusCommand buildCommandFromIntersection(final DLine currentLine, fina
         // Creation of the tasks to execute the tool
         addOperationTask(result, currentLine, interpreterContext, tool.getFirstModelOperation());
 
+        // Clear mask variables
+        addMaskResetTask(result, interpreterContext, tool.getMask(), newValue);
+
         return result;
     }
 
@@ -570,6 +575,13 @@ private void addMaskInitTask(SiriusCommand result, EObject context, EditMaskVari
         }
     }
 
+    private void addMaskResetTask(SiriusCommand result, EObject context, EditMaskVariables mask, final Object newValue) {
+        if (mask != null && mask.getMask() != null) {
+            // Reset all mask variables initialized before
+            result.getTasks().add(new InitInterpreterFromParsedVariableTask2(InterpreterUtil.getInterpreter(context), mask.getMask(), newValue, true));
+        }
+    }
+
     private void addOperationTask(SiriusCommand result, DSemanticDecorator tableElement, EObject context, ModelOperation operation) {
         DTable table = TableHelper.getTable(tableElement);
         result.getTasks().add(commandTaskHelper.buildTaskFromModelOperation(table, context, operation));

plugins/org.eclipse.sirius.tests.junit/src/org/eclipse/sirius/tests/unit/api/tools/tasks/InitInterpreterFromParsedVariableTaskTest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010, 2014 THALES GLOBAL SERVICES.
+ * Copyright (c) 2010, 2024 THALES GLOBAL SERVICES.
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
  * which accompanies this distribution, and is available at
@@ -23,19 +23,17 @@
 import junit.framework.TestCase;
 
 /**
- * Test the parsing of mask variables and the variable duplication for integer
- * variables names.
+ * Test the parsing of mask variables and the variable duplication for integer variables names.
  * 
- * This is done by two tasks : InitInterpreterFromParsedVariableTask and
- * InitInterpreterFromParsedVariableTask2.
+ * This is done by two tasks : InitInterpreterFromParsedVariableTask and InitInterpreterFromParsedVariableTask2.
  * 
- * This test checks the parsing and the int name variable duplication for
- * different masks and values.
+ * This test checks the parsing and the int name variable duplication for different masks and values.
  * 
- * The duplication consists in creating an argX named variable for each X
- * variable: 0 and arg0 for example. It is required by some interpreters which
- * cannot have int variables like Acceleo3. Acceleo2 was supporting it thanks to
- * its prefix $.
+ * The duplication consists in creating an argX named variable for each X variable: 0 and arg0 for example. It is
+ * required by some interpreters which cannot have int variables like Acceleo3. Acceleo2 was supporting it thanks to its
+ * prefix $.
+ * 
+ * This test also checks that the dedicated task correctly cleans the interpreter.
  * 
  * @author mporhel
  * 
@@ -205,6 +203,10 @@ private void doTestParsedVariableTask(String mask, String message, Map<Integer,
         new InitInterpreterFromParsedVariableTask(interpreter, mask, message).execute();
 
         doTestVariables(expectedVariableValues);
+
+        // Call the task that should clean the interpreter
+        new InitInterpreterFromParsedVariableTask(interpreter, mask, message, true).execute();
+        doTestVariables(expectedVariableValues, true);
     }
 
     private void doTestParsedVariableTask2(String mask, String message, Map<Integer, String> expectedVariableValues) {
@@ -214,30 +216,41 @@ private void doTestParsedVariableTask2(String mask, String message, Map<Integer,
         new InitInterpreterFromParsedVariableTask2(interpreter, mask, message).execute();
 
         doTestVariables(expectedVariableValues);
+
+        // Call the task that should clean the interpreter
+        new InitInterpreterFromParsedVariableTask2(interpreter, mask, message, true).execute();
+        doTestVariables(expectedVariableValues, true);
     }
 
     private void doTestVariables(Map<Integer, String> expectedVariableValues) {
+        doTestVariables(expectedVariableValues, false);
+    }
+
+    private void doTestVariables(Map<Integer, String> expectedVariableValues, boolean clean) {
 
         // Check the global variable number, we should have duplication for all
         // int named variables.
-        assertEquals("The int named variables should be duplicated, example : 0 and arg0.", expectedVariableValues.size() * 2, interpreter.getVariables().size());
-
-        for (Integer varIntName : expectedVariableValues.keySet()) {
-            String intKey = varIntName.toString();
-            String argKey = "arg" + intKey;
-
-            assertTrue("The variable named " + intKey + " is not present in the variable list.", interpreter.getVariables().containsKey(intKey));
-            assertTrue("The variable named " + argKey + " is not present in the variable list.", interpreter.getVariables().containsKey(argKey));
-
-            // The test deals with String value.
-            String intKeyValue = (String) interpreter.getVariable(intKey);
-            String argKeyValue = (String) interpreter.getVariable(argKey);
-
-            String expectedValue = expectedVariableValues.get(varIntName);
-            assertEquals("The variable named " + intKey + " does not have the expected value.", intKeyValue, expectedValue);
-            assertEquals("The variable named " + argKeyValue + " does not have the expected value.", argKeyValue, expectedValue);
+        if (clean) {
+            assertEquals("No variable should be contains in the interpreter after direct edit.", 0, interpreter.getVariables().size());
+        } else {
+            assertEquals("The int named variables should be duplicated, example : 0 and arg0.", expectedVariableValues.size() * 2, interpreter.getVariables().size());
+
+            for (Integer varIntName : expectedVariableValues.keySet()) {
+                String intKey = varIntName.toString();
+                String argKey = "arg" + intKey;
+
+                assertTrue("The variable named " + intKey + " is not present in the variable list.", interpreter.getVariables().containsKey(intKey));
+                assertTrue("The variable named " + argKey + " is not present in the variable list.", interpreter.getVariables().containsKey(argKey));
+
+                // The test deals with String value.
+                String intKeyValue = (String) interpreter.getVariable(intKey);
+                String argKeyValue = (String) interpreter.getVariable(argKey);
+
+                String expectedValue = expectedVariableValues.get(varIntName);
+                assertEquals("The variable named " + intKey + " does not have the expected value.", intKeyValue, expectedValue);
+                assertEquals("The variable named " + argKeyValue + " does not have the expected value.", argKeyValue, expectedValue);
+            }
         }
-
     }
 
     @Override

plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/DirectEditWithInputLabelTest.java

@@ -15,6 +15,7 @@
 import java.util.List;
 
 import org.eclipse.gef.EditPart;
+import org.eclipse.sirius.common.tools.api.interpreter.IInterpreter;
 import org.eclipse.sirius.diagram.DDiagram;
 import org.eclipse.sirius.diagram.ui.edit.api.part.AbstractDiagramNameEditPart;
 import org.eclipse.sirius.diagram.ui.edit.api.part.AbstractDiagramNodeEditPart;
@@ -94,7 +95,12 @@ protected void onSetUpAfterOpeningDesignerPerspective() throws Exception {
      *             Test error.
      */
     public void testDirectEditWithInputLabelExpressionOnNode() throws Exception {
+        IInterpreter interpreter = localSession.getOpenedSession().getInterpreter();
+        assertNull("The variable named \"0\" should not be set before the direct edit.", interpreter.getVariable("0"));
+        assertNull("The variable named \"0\" should not be set before the direct edit.", interpreter.getVariable("arg0"));
         checkDirectEditWithInputLabelExpression("<<node>> ", AbstractDiagramNodeEditPart.class);
+        assertNull("The variable named \"0\" should not be set after the direct edit.", interpreter.getVariable("0"));
+        assertNull("The variable named \"0\" should not be set after the direct edit.", interpreter.getVariable("arg0"));
     }
 
     /**

plugins/org.eclipse.sirius.tree/src/org/eclipse/sirius/tree/tools/internal/command/TreeCommandFactory.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010, 2021 THALES GLOBAL SERVICES and others.
+ * Copyright (c) 2010, 2024 THALES GLOBAL SERVICES and others.
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
  * which accompanies this distribution, and is available at
@@ -359,6 +359,14 @@ public Command buildDirectEditLabelFromTool(final DTreeItem editedTreeItem, Tree
                     // on direct edit even in REFRESH_AUTO mode at false
                     ICommandTask refreshTreeElementTask = new RefreshTreeElementTask(editedTreeItem);
                     result.getTasks().add(refreshTreeElementTask);
+
+                    if (directEditTool.getMask() != null) {
+                        /*
+                         * Finally we need to clean the mask variables.
+                         */
+                        final String messageFormat = directEditTool.getMask().getMask();
+                        result.getTasks().add(new InitInterpreterFromParsedVariableTask2(InterpreterUtil.getInterpreter(interpreterContext), messageFormat, newValue, true));
+                    }
                 }
             }
         }

plugins/org.eclipse.sirius/plugin.properties

@@ -195,6 +195,7 @@ ISiriusMessages_notADecoratorErrorMsg = The element is not a DSemanticDecorator
 IfTask_label = Evaluate : {0}
 ImageManagerForWorkspaceResource_errorGettingTheOriginalPath=Failed to retrieve the original path from the html path {0} for the object {1}. The path will not be stored properly in the model.
 InitInterpreterFromParsedVariableTask_label = Init Acceleo interpreter with parsed variables
+InitInterpreterFromParsedVariableTask_unsetLabel = Init Acceleo interpreter by unsetting parsed variables
 InitInterpreterVariablesTask_label = Init Acceleo variables
 InitInterpreterVariablesTask_invalidModelErrorMsg = Invalid Model
 InitializeModelingProjectJob_invalidModelingProjectsErrorMsg = Several modeling projects are invalid.

plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/api/helper/task/label/InitInterpreterFromParsedVariableTask.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2021 THALES GLOBAL SERVICES.
+ * Copyright (c) 2007, 2024 THALES GLOBAL SERVICES.
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
  * which accompanies this distribution, and is available at
@@ -20,7 +20,7 @@
 import org.eclipse.sirius.tools.api.Messages;
 
 /**
- * Task to init interreter variables.
+ * Task to init interpreter variables.
  * 
  * @author mchauvin
  */
@@ -32,8 +32,10 @@ public class InitInterpreterFromParsedVariableTask extends AbstractCommandTask {
 
     private String message;
 
+    private boolean unset;
+
     /**
-     * Default constructor.
+     * Constructor to set variables according to the mask.
      * 
      * @param inter
      *            the interpreter
@@ -48,6 +50,25 @@ public InitInterpreterFromParsedVariableTask(final IInterpreter inter, final Str
         this.message = message;
     }
 
+    /**
+     * Constructor to unset variables according to the mask.
+     * 
+     * @param inter
+     *            the interpreter
+     * @param messageFormat
+     *            the message mask
+     * @param message
+     *            the message
+     * @param unset 
+     *            true to unset the variables
+     */
+    public InitInterpreterFromParsedVariableTask(final IInterpreter inter, final String messageFormat, final String message, final boolean unset) {
+        this.interpreter = inter;
+        this.messageMask = messageFormat;
+        this.message = message;
+        this.unset = unset;
+    }
+
     /**
      * {@inheritDoc}
      * 
@@ -56,13 +77,21 @@ public InitInterpreterFromParsedVariableTask(final IInterpreter inter, final Str
     @Override
     public void execute() {
         if (message != null && message.length() == 0) {
-            setVariable(Integer.valueOf(0), ""); //$NON-NLS-1$
+            if (unset) {
+                unSetVariable(Integer.valueOf(0));
+            } else {
+                setVariable(Integer.valueOf(0), ""); //$NON-NLS-1$
+            }
         } else {
             final MessageFormat parser = new MessageFormat(messageMask);
             try {
                 final Object[] values = parser.parse(message);
                 for (int i = 0; i < values.length; i++) {
-                    setVariable(Integer.valueOf(i), values[i]);
+                    if (unset) {
+                        unSetVariable(Integer.valueOf(i));
+                    } else {
+                        setVariable(Integer.valueOf(i), values[i]);
+                    }
                 }
             } catch (final ParseException e) {
                 /*
@@ -72,6 +101,14 @@ public void execute() {
         }
     }
 
+    /*
+     * Unset two variables, one with the X as key, the other with "argX" as key.
+     */
+    private void unSetVariable(Integer i) {
+        interpreter.unSetVariable(i.toString());
+        interpreter.unSetVariable("arg" + i.toString()); //$NON-NLS-1$
+    }
+
     /*
      * Set two variables, one with the X as key, the other with "argX" as key.
      */
@@ -108,6 +145,10 @@ public boolean canExecute() {
      */
     @Override
     public String getLabel() {
-        return Messages.InitInterpreterFromParsedVariableTask_label;
+        if (unset) {
+            return Messages.InitInterpreterFromParsedVariableTask_unsetLabel;
+        } else {
+            return Messages.InitInterpreterFromParsedVariableTask_label;
+        }
     }
 }