[463430] Consider 'For' ModelOperation as a context change

Bug: 463430
Change-Id: Ia774aa8d6d5b5252ff9b9b6adac8da7ba3c18218
Signed-off-by: Cedric Brun <cedric.brun@obeo.fr>

3 files changed, 57 insertions, 13 deletions

diff --git a/plugins/org.eclipse.sirius.tests.junit/data/unit/vsm/validateVariableTypes.odesign b/plugins/org.eclipse.sirius.tests.junit/data/unit/vsm/validateVariableTypes.odesign
index 40610ac87a..514205dafd 100644
--- a/plugins/org.eclipse.sirius.tests.junit/data/unit/vsm/validateVariableTypes.odesign
+++ b/plugins/org.eclipse.sirius.tests.junit/data/unit/vsm/validateVariableTypes.odesign
@@ -187,6 +187,22 @@
                 </subModelOperations>
                 <subModelOperations xsi:type="tool:CreateInstance" typeName="ecore.EClass" referenceName="nonExistent">
                   <subModelOperations xsi:type="tool:ChangeContext" browseExpression="feature:abstract"/>
+                  <subModelOperations xsi:type="tool:For" expression="feature:eOperations" iteratorName="op">
+                    <subModelOperations xsi:type="tool:ChangeContext" browseExpression="var:op">
+                      <subModelOperations xsi:type="tool:ChangeContext" browseExpression="feature:eParameters"/>
+                    </subModelOperations>
+                    <subModelOperations xsi:type="tool:ChangeContext" browseExpression="var:self">
+                      <subModelOperations xsi:type="tool:ChangeContext" browseExpression="feature:eParameters"/>
+                    </subModelOperations>
+                    <subModelOperations xsi:type="tool:For" expression="feature:eTypeParameters" iteratorName="op">
+                      <subModelOperations xsi:type="tool:ChangeContext" browseExpression="var:op">
+                        <subModelOperations xsi:type="tool:ChangeContext" browseExpression="feature:eBounds"/>
+                      </subModelOperations>
+                      <subModelOperations xsi:type="tool:ChangeContext" browseExpression="var:self">
+                        <subModelOperations xsi:type="tool:ChangeContext" browseExpression="feature:eBounds"/>
+                      </subModelOperations>
+                    </subModelOperations>
+                  </subModelOperations>
                 </subModelOperations>
               </firstModelOperations>
             </initialOperation>
diff --git a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/api/dialect/description/AbstractInterpretedExpressionQuery.java b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/api/dialect/description/AbstractInterpretedExpressionQuery.java
index 51493a3cea..154726df6e 100644
--- a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/api/dialect/description/AbstractInterpretedExpressionQuery.java
+++ b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/api/dialect/description/AbstractInterpretedExpressionQuery.java
@@ -366,13 +366,29 @@ public abstract class AbstractInterpretedExpressionQuery implements IInterpreted
 
         while (context != null && context != top.eContainer()) {
             appendAllLocalVariableDefinitions(definitions, context);
-            if (context instanceof ChangeContext && context != bottom) {
-                ChangeContext f = (ChangeContext) context;
-                IInterpreterContext iContext = SiriusInterpreterContextFactory.createInterpreterContext(f, ToolPackage.Literals.CHANGE_CONTEXT__BROWSE_EXPRESSION);
-                ValidationResult res = MultiLanguagesValidator.getInstance().validateExpression(iContext, f.getBrowseExpression());
-                VariableType returnTypes = res.getReturnTypes();
-                changeSelfType(definitions, returnTypes);
+            /*
+             * Any ModelOperation which require to call IInterpreter to get more
+             * typing information should not create a new context at the
+             * ModelOperation leaf or we'll end up with an inifite loop.
+             */
+            if (context != bottom) {
+                if (context instanceof ChangeContext) {
+                    ChangeContext f = (ChangeContext) context;
+                    IInterpreterContext iContext = SiriusInterpreterContextFactory.createInterpreterContext(f, ToolPackage.Literals.CHANGE_CONTEXT__BROWSE_EXPRESSION);
+                    ValidationResult res = MultiLanguagesValidator.getInstance().validateExpression(iContext, f.getBrowseExpression());
+                    VariableType returnTypes = res.getReturnTypes();
+                    changeSelfType(definitions, returnTypes);
+
+                }
+                if (context instanceof For) {
+                    For f = (For) context;
+                    IInterpreterContext iContext = SiriusInterpreterContextFactory.createInterpreterContext(f, ToolPackage.Literals.FOR__EXPRESSION);
+                    ValidationResult res = MultiLanguagesValidator.getInstance().validateExpression(iContext, f.getExpression());
+                    VariableType returnTypes = res.getReturnTypes();
+                    changeSelfType(definitions, returnTypes);
+                    addDefinition(definitions, f.getIteratorName(), returnTypes);
 
+                }
             }
             if (context instanceof CreateInstance) {
                 CreateInstance f = (CreateInstance) context;
@@ -496,12 +512,6 @@ public abstract class AbstractInterpretedExpressionQuery implements IInterpreted
             CreateInstance ci = (CreateInstance) context;
             addDefinition(definitions, ci.getVariableName(), ci.getTypeName());
         }
-        // The CreateInstance model operation implicitly defines a variable to
-        // reference the newly created instance.
-        if (context instanceof For) {
-            For f = (For) context;
-            addDefinition(definitions, f.getIteratorName(), VariableType.ANY_EOBJECT);
-        }
 
     }
 
diff --git a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/dialect/description/ToolInterpretedExpressionTargetSwitch.java b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/dialect/description/ToolInterpretedExpressionTargetSwitch.java
index 378926e975..bf6240fd1a 100644
--- a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/dialect/description/ToolInterpretedExpressionTargetSwitch.java
+++ b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/dialect/description/ToolInterpretedExpressionTargetSwitch.java
@@ -33,6 +33,7 @@ import org.eclipse.sirius.viewpoint.description.tool.CreateInstance;
 import org.eclipse.sirius.viewpoint.description.tool.ExternalJavaAction;
 import org.eclipse.sirius.viewpoint.description.tool.ExternalJavaActionCall;
 import org.eclipse.sirius.viewpoint.description.tool.ExternalJavaActionParameter;
+import org.eclipse.sirius.viewpoint.description.tool.For;
 import org.eclipse.sirius.viewpoint.description.tool.MappingBasedToolDescription;
 import org.eclipse.sirius.viewpoint.description.tool.ModelOperation;
 import org.eclipse.sirius.viewpoint.description.tool.OperationAction;
@@ -178,7 +179,7 @@ public class ToolInterpretedExpressionTargetSwitch extends ToolSwitch<Option<Col
 
     private boolean isChangingContextElement(EObject element) {
         boolean descCanChange = element instanceof RepresentationDescription || element instanceof RepresentationElementMapping;
-        boolean operationCanChange = element instanceof CreateInstance || element instanceof ChangeContext;
+        boolean operationCanChange = element instanceof CreateInstance || element instanceof ChangeContext || element instanceof For;
         boolean toolCanChange = element instanceof AbstractToolDescription;
         return descCanChange || operationCanChange || toolCanChange;
     }
@@ -470,6 +471,23 @@ public class ToolInterpretedExpressionTargetSwitch extends ToolSwitch<Option<Col
         }
         return result;
     }
+    
+    @Override
+    public Option<Collection<String>> caseFor(For object) {
+        Option<Collection<String>> result = null;
+        switch (getFeatureId(object.eClass())) {
+        case ToolPackage.FOR__EXPRESSION:
+            return globalSwitch.doSwitch(getFirstContextChangingContainer(object), false);
+        case DO_NOT_CONSIDER_FEATURE:
+            // Compile expression and ask for return type.
+            // EObject other wise.
+            result = Options.newNone();
+            break;
+        default:
+            break;
+        }
+        return result;
+    }
 
     @Override
     public Option<Collection<String>> caseCreateInstance(CreateInstance object) {