Replacing "for-loop as namedComponent" logic with "iterator as namedC…

…omponent" for better separation between when to resolve iterator type and when to resolve for-loops expressions (array/map init).
HaxeFoundation · Apr 1, 2024 · 202497d · 202497d
1 parent 2556636
commit 202497d
Show file tree

Hide file tree

Showing 18 changed files with 136 additions and 93 deletions.
diff --git a/src/main/java/com/intellij/plugins/haxe/HaxeComponentType.java b/src/main/java/com/intellij/plugins/haxe/HaxeComponentType.java
@@ -180,7 +180,11 @@ public static HaxeComponentType typeOf(PsiElement element) {
     }
     if (element instanceof HaxeLocalVarDeclaration ||
         element instanceof HaxeForStatement ||
-        element instanceof HaxeEnumExtractedValue) {
+        element instanceof HaxeEnumExtractedValue ||
+        element instanceof HaxeValueIterator || // default iterator
+        element instanceof HaxeIteratorkey || // keyValueIterator
+        element instanceof HaxeIteratorValue // keyValueIterator
+    ) {
       return VARIABLE;
     }
     if (element instanceof HaxeParameter) {

diff --git a/src/main/java/com/intellij/plugins/haxe/ide/HaxeDocumentationProvider.java b/src/main/java/com/intellij/plugins/haxe/ide/HaxeDocumentationProvider.java
@@ -377,17 +377,17 @@ else if (component instanceof HaxeLocalVarDeclaration varDeclaration
 
       resolveTypeAndMakeHeader(builder, varDeclaration);
     }
-    else if (component instanceof HaxeForStatement forStatement) {
-      HaxeExpressionEvaluatorContext context = new HaxeExpressionEvaluatorContext(forStatement);
-      HaxeExpressionEvaluator.evaluate(forStatement, context, null);
-
-      makeHeader(builder, context.result);
+    else if (component instanceof HaxeValueIterator) {
+      resolveTypeAndMakeHeader(builder, component);
     }
-    else if (component instanceof HaxeEnumExtractedValue extractedValue) {
-      HaxeExpressionEvaluatorContext context = new HaxeExpressionEvaluatorContext(extractedValue);
-      HaxeExpressionEvaluator.evaluate(extractedValue, context, null);
-
-      makeHeader(builder, context.result);
+    else if (component instanceof HaxeIteratorkey) {
+      resolveTypeAndMakeHeader(builder, component);
+    }
+    else if (component instanceof HaxeIteratorValue) {
+      resolveTypeAndMakeHeader(builder, component);
+    }
+    else if (component instanceof HaxeEnumExtractedValue) {
+      resolveTypeAndMakeHeader(builder, component);
     }
   }
 

diff --git a/src/main/java/com/intellij/plugins/haxe/ide/completion/HaxeKeywordCompletionContributor.java b/src/main/java/com/intellij/plugins/haxe/ide/completion/HaxeKeywordCompletionContributor.java
@@ -154,6 +154,7 @@ private static void suggestKeywords(PsiElement position, @NotNull CompletionResu
 
           HaxeSwitchCase type = PsiTreeUtil.getPrevSiblingOfType(completionElementAsComment, HaxeSwitchCase.class);
           if (type!= null) {
+            // TODO, solve this using getVariants and walkTree
             addSwitchVars(type, lookupElements);
           }
         }

diff --git a/src/main/java/com/intellij/plugins/haxe/ide/hint/types/HaxeInlayForLoopHintsProvider.java b/src/main/java/com/intellij/plugins/haxe/ide/hint/types/HaxeInlayForLoopHintsProvider.java
@@ -40,16 +40,16 @@ private static void handleForEachHints(@NotNull PsiElement element,
       HaxeKeyValueIterator keyValueIterator = forStatement.getKeyValueIterator();
       HaxeGenericResolver resolver = HaxeGenericResolverUtil.generateResolverFromScopeParents(forStatement);
       if (iterable != null && keyValueIterator == null) {
-        HaxeComponentName componentName = forStatement.getComponentName();
+        HaxeValueIterator valueIterator = forStatement.getValueIterator();
         ResultHolder type = extractIteratorElementType(HaxeTypeResolver.getPsiElementType(iterable, element, resolver));
-        createInlayHint(componentName, sink, type);
+        if (valueIterator!= null) createInlayHint(valueIterator.getComponentName(), sink, type);
       }
       if (keyValueIterator != null) {
         HaxeIteratorkey iteratorKey = keyValueIterator.getIteratorkey();
         HaxeIteratorValue iteratorValue = keyValueIterator.getIteratorValue();
 
-        ResultHolder keyType = HaxeExpressionEvaluator.findIteratorType(element, iteratorKey);
-        ResultHolder valueType = HaxeExpressionEvaluator.findIteratorType(element, iteratorValue);
+        ResultHolder keyType = HaxeExpressionEvaluator.findIteratorType(iteratorKey);
+        ResultHolder valueType = HaxeExpressionEvaluator.findIteratorType(iteratorValue);
 
         if (keyType!= null) createInlayHint(iteratorKey.getComponentName(), sink, keyType);
         if (valueType!= null) createInlayHint(iteratorValue.getComponentName(), sink, valueType);

diff --git a/src/main/java/com/intellij/plugins/haxe/ide/intention/IteratorForLoopIntention.java b/src/main/java/com/intellij/plugins/haxe/ide/intention/IteratorForLoopIntention.java
@@ -8,10 +8,7 @@
 import com.intellij.plugins.haxe.HaxeBundle;
 import com.intellij.plugins.haxe.HaxeLanguage;
 import com.intellij.plugins.haxe.ide.refactoring.introduce.HaxeIntroduceHandler;
-import com.intellij.plugins.haxe.lang.psi.HaxeComponentName;
-import com.intellij.plugins.haxe.lang.psi.HaxeForStatement;
-import com.intellij.plugins.haxe.lang.psi.HaxePsiCompositeElement;
-import com.intellij.plugins.haxe.lang.psi.HaxeReference;
+import com.intellij.plugins.haxe.lang.psi.*;
 import com.intellij.plugins.haxe.model.type.HaxeExpressionEvaluator;
 import com.intellij.plugins.haxe.model.type.ResultHolder;
 import com.intellij.plugins.haxe.model.type.SpecificHaxeClassReference;
@@ -64,13 +61,16 @@ public void invoke(@NotNull final Project project, Editor editor, PsiFile file)
 
       if (!editor.isViewer()) {
         CodeInsightUtilCore.forcePsiPostprocessAndRestoreElement(itr);
-        HaxeComponentName name = itr.getComponentName();
-        final var introducer = new HaxeIntroduceHandler.HaxeInplaceVariableIntroducer(name, editor, List.of());
-        introducer.setElementToRename(name);
-        TextRange range = name.getTextRange();
-        editor.getSelectionModel().setSelection(range.getStartOffset(), range.getEndOffset());
-        editor.getCaretModel().moveToOffset(range.getEndOffset());
-        introducer.performInplaceRefactoring(new LinkedHashSet<>(List.of()));
+        HaxeValueIterator valueIterator = itr.getValueIterator();
+        if (valueIterator != null ) {
+          HaxeComponentName name = valueIterator.getComponentName();
+          final var introducer = new HaxeIntroduceHandler.HaxeInplaceVariableIntroducer(name, editor, List.of());
+          introducer.setElementToRename(name);
+          TextRange range = name.getTextRange();
+          editor.getSelectionModel().setSelection(range.getStartOffset(), range.getEndOffset());
+          editor.getCaretModel().moveToOffset(range.getEndOffset());
+          introducer.performInplaceRefactoring(new LinkedHashSet<>(List.of()));
+        }
       }
     }
   }

diff --git a/src/main/java/com/intellij/plugins/haxe/lang/parser/haxe.bnf b/src/main/java/com/intellij/plugins/haxe/lang/parser/haxe.bnf
@@ -603,13 +603,17 @@ forStatement ::= 'for' '(' iterableDeclaration ')' statement ';'?
 // XXX: Somehow or another, forStatement must derive from AbstractHaxeNamedComponent, or variable resolution will break.
 private iterableDeclaration ::= forDeclaration 'in' iterable {pin=1 recoverWhile="iterable_decl_recovery"}
 private iterable_decl_recovery ::= !(')')
-private forDeclaration ::= keyValueIterator | componentName
+private forDeclaration ::= keyValueIterator | valueIterator
+
+keyValueIterator ::= iteratorkey fatArrowOperator iteratorValue
+
 iteratorkey ::= componentName
 {mixin="com.intellij.plugins.haxe.lang.psi.impl.AbstractHaxeNamedComponent" implements="com.intellij.plugins.haxe.lang.psi.HaxeComponent"}
 iteratorValue ::= componentName
 {mixin="com.intellij.plugins.haxe.lang.psi.impl.AbstractHaxeNamedComponent" implements="com.intellij.plugins.haxe.lang.psi.HaxeComponent"}
-keyValueIterator ::= iteratorkey fatArrowOperator iteratorValue
 
+valueIterator ::= componentName
+{mixin="com.intellij.plugins.haxe.lang.psi.impl.AbstractHaxeNamedComponent" implements="com.intellij.plugins.haxe.lang.psi.HaxeComponent"}
 
 
 iterable ::= expression

diff --git a/src/main/java/com/intellij/plugins/haxe/lang/psi/HaxeForStatementPsiMixin.java b/src/main/java/com/intellij/plugins/haxe/lang/psi/HaxeForStatementPsiMixin.java
@@ -28,7 +28,8 @@
  * See HaxePsiCompositeElementImpl.getDeclarationToProcess() to see how
  * that works.
  */
-public interface HaxeForStatementPsiMixin extends HaxeStatementPsiMixin, PsiNameIdentifierOwner {
+//public interface HaxeForStatementPsiMixin extends HaxeStatementPsiMixin, PsiNameIdentifierOwner {
+public interface HaxeForStatementPsiMixin extends HaxeStatementPsiMixin{
 
   // The funny thing is that we don't need any interfaces at this level.
   // We just need to introduce a specific inheritance.

diff --git a/src/main/java/com/intellij/plugins/haxe/lang/psi/HaxeSpecificFunction.java b/src/main/java/com/intellij/plugins/haxe/lang/psi/HaxeSpecificFunction.java
@@ -133,6 +133,7 @@ public List<HaxeFunctionArgument> getFunctionArgumentList() {
   @Nullable
   @Override
   public HaxeFunctionReturnType getFunctionReturnType() {
+    //TODO consider cache ?
     return null != functionType ? functionType.getFunctionReturnType()
                                 : (HaxeFunctionReturnType)method.getReturnType();
   }

diff --git a/src/main/java/com/intellij/plugins/haxe/lang/psi/impl/HaxeForStatementPsiMixinImpl.java b/src/main/java/com/intellij/plugins/haxe/lang/psi/impl/HaxeForStatementPsiMixinImpl.java
@@ -26,17 +26,11 @@
 /**
  * Created by ebishton on 12/4/14.
  */
-public abstract class HaxeForStatementPsiMixinImpl extends AbstractHaxeNamedComponent implements HaxeForStatementPsiMixin {
+public abstract class HaxeForStatementPsiMixinImpl extends HaxePsiCompositeElementImpl implements HaxeForStatementPsiMixin {
 
   public HaxeForStatementPsiMixinImpl(ASTNode node) {
     super(node);
   }
 
-  @Override
-  @Nullable
-  public PsiElement getNameIdentifier() {
-    final HaxeComponentName componentName = getComponentName();
-    return componentName != null ? componentName.getNameIdentifier() : null;
-  }
 
 }
diff --git a/src/main/java/com/intellij/plugins/haxe/lang/psi/impl/HaxePsiCompositeElementImpl.java b/src/main/java/com/intellij/plugins/haxe/lang/psi/impl/HaxePsiCompositeElementImpl.java
@@ -159,16 +159,18 @@ private List<PsiElement> getDeclarationElementToProcess(PsiElement lastParent) {
       result.addAll(tygenericParameParam.getGenericListPartList());
     }
 
-    if (this instanceof HaxeForStatement) {
-      HaxeForStatement forStatement = (HaxeForStatement)this;
+    if (this instanceof HaxeForStatement forStatement) {
       HaxeKeyValueIterator keyValueIterator = forStatement.getKeyValueIterator();
-      HaxeIterable iterable = forStatement.getIterable();
-
-      if (keyValueIterator != null && forStatement.getKeyValueIterator() != lastParent) {
-        result.add(keyValueIterator.getIteratorkey());
-        result.add(keyValueIterator.getIteratorValue());
-      } else if (iterable != null && iterable != lastParent) {
-        result.add(this);
+      HaxeValueIterator valueIterator = forStatement.getValueIterator();
+      // any reference in HaxeIterable is always defined outside its current loop  (avoid problems like var x:Array<String>; for (x in x))
+      if (!(lastParent instanceof HaxeIterable)) {
+        if (keyValueIterator != null && keyValueIterator != lastParent) {
+          result.add(keyValueIterator.getIteratorkey());
+          result.add(keyValueIterator.getIteratorValue());
+        }
+        else if (valueIterator != null && valueIterator != lastParent) {
+          result.add(valueIterator);
+        }
       }
     }
     if (this instanceof  HaxeSwitchCase switchCase) {

diff --git a/src/main/java/com/intellij/plugins/haxe/model/type/HaxeExpressionEvaluator.java b/src/main/java/com/intellij/plugins/haxe/model/type/HaxeExpressionEvaluator.java
@@ -174,6 +174,13 @@ static private ResultHolder _handle(final PsiElement element,
         return functionDeclaration.getModel().getFunctionType(resolver).createHolder();
       }
 
+      if (element instanceof HaxeValueIterator valueIterator) {
+        return handleValueIterator(context, resolver, valueIterator);
+      }
+      if (element instanceof HaxeIteratorkey || element instanceof HaxeIteratorValue) {
+        return findIteratorType(element);
+      }
+
       if (element instanceof HaxeEnumExtractedValue extractedValue) {
         return handleEnumExtractedValue(extractedValue);
       }
@@ -333,12 +340,12 @@ static private ResultHolder _handle(final PsiElement element,
 
 
   @Nullable
-  public static ResultHolder findIteratorType(PsiElement reference, PsiElement iteratorElement) {
+  public static ResultHolder findIteratorType(PsiElement iteratorElement) {
     HaxeForStatement forStatement = PsiTreeUtil.getParentOfType(iteratorElement, HaxeForStatement.class);
     HaxeGenericResolver forResolver = HaxeGenericResolverUtil.generateResolverFromScopeParents(forStatement);
 
     HaxeIterable iterable = forStatement.getIterable();
-    var keyValueIteratorType = HaxeTypeResolver.getPsiElementType(iterable, reference, forResolver);
+    var keyValueIteratorType = HaxeTypeResolver.getPsiElementType(iterable, iteratorElement, forResolver);
 
     var iteratorType = keyValueIteratorType.getClassType();
     if (iteratorType.isTypeDef()) {

diff --git a/src/main/java/com/intellij/plugins/haxe/model/type/HaxeExpressionEvaluatorHandlers.java b/src/main/java/com/intellij/plugins/haxe/model/type/HaxeExpressionEvaluatorHandlers.java
@@ -275,27 +275,12 @@ else if (subelement instanceof HaxeMethodDeclaration methodDeclaration) {
             typeHolder = type.createHolder();
           }
 
-          else if (subelement instanceof HaxeForStatement forStatement) {
-            // key-value iterator is not relevant here as it will be resolved to HaxeIteratorkey  or HaxeIteratorValue
-            final HaxeComponentName name = forStatement.getComponentName();
-            // if element text matches  for loops  iterator  i guess we can consider it a match?
-            if (name != null && element.textMatches(name)) {
-              final HaxeIterable iterable = forStatement.getIterable();
-              if (iterable != null) {
-                ResultHolder iterator = handle(iterable, context, resolver);
-                if (iterator.isClassType()) {
-                  iterator = iterator.getClassType().fullyResolveTypeDefAndUnwrapNullTypeReference().createHolder();
-                }
-                // get specific from iterator as thats the type for our variable
-                ResultHolder[] specifics = iterator.getClassType().getSpecifics();
-                if (specifics.length > 0) {
-                  typeHolder = specifics[0];
-                }
-              }
-            }
+          else if (subelement instanceof HaxeValueIterator valueIterator) {
+            typeHolder = handleValueIterator(context, resolver, valueIterator);
           }
+
           else if (subelement instanceof HaxeIteratorkey || subelement instanceof HaxeIteratorValue) {
-            typeHolder = findIteratorType(element, subelement);
+            typeHolder = findIteratorType(subelement);
           }
 
           else if (subelement instanceof HaxeSwitchCaseCaptureVar caseCaptureVar) {
@@ -335,6 +320,28 @@ else if (subelement instanceof HaxeSwitchCaseExpr caseExpr) {
     }
   }
 
+  static ResultHolder handleValueIterator(HaxeExpressionEvaluatorContext context,
+                                        HaxeGenericResolver resolver,
+                                        HaxeValueIterator valueIterator) {
+    HaxeForStatement forStatement = PsiTreeUtil.getParentOfType(valueIterator, HaxeForStatement.class);
+    if (forStatement != null) {
+        final HaxeIterable iterable = forStatement.getIterable();
+        if (iterable != null) {
+          // NOTE do not forward resolver here, this is a different expression and might have its own typeParameters
+          ResultHolder iterator = handle(iterable, context, null);
+          if (iterator.isClassType()) {
+            iterator = iterator.getClassType().fullyResolveTypeDefAndUnwrapNullTypeReference().createHolder();
+          }
+          // get specific from iterator as thats the type for our variable
+          ResultHolder[] specifics = iterator.getClassType().getSpecifics();
+          if (specifics.length > 0) {
+            return specifics[0];
+          }
+      }
+    }
+    return createUnknown(valueIterator);
+  }
+
   static ResultHolder handleRegularExpressionLiteral(HaxeRegularExpressionLiteral regexLiteral) {
     HaxeClass regexClass = HaxeResolveUtil.findClassByQName(getLiteralClassName(HaxeTokenTypes.REG_EXP), regexLiteral);
     if (regexClass != null) {
@@ -542,7 +549,7 @@ static ResultHolder handleForStatement(
     HaxeForStatement forStatement) {
     final HaxeExpression forStatementExpression = forStatement.getExpression();
     final HaxeKeyValueIterator keyValueIterator = forStatement.getKeyValueIterator();
-    final HaxeComponentName name = forStatement.getComponentName();
+    final HaxeValueIterator valueIterator = forStatement.getValueIterator();
     final HaxeIterable iterable = forStatement.getIterable();
     final PsiElement body = forStatement.getLastChild();
     context.beginScope();
@@ -585,12 +592,13 @@ static ResultHolder handleForStatement(
             type = type.withRangeConstraint(constant);
           }
         }
-        if (name != null) {
-          context.setLocal(name.getText(), new ResultHolder(type));
-        } else if (keyValueIterator != null) {
-          context.setLocal(keyValueIterator.getIteratorkey().getComponentName().getText(), new ResultHolder(type));
-          context.setLocal(keyValueIterator.getIteratorValue().getComponentName().getText(), new ResultHolder(type));
-        }
+        if (valueIterator != null) {
+          HaxeComponentName name = valueIterator.getComponentName();
+            context.setLocal(name.getText(), new ResultHolder(type));
+          } else if (keyValueIterator != null) {
+            context.setLocal(keyValueIterator.getIteratorkey().getComponentName().getText(), new ResultHolder(type));
+            context.setLocal(keyValueIterator.getIteratorValue().getComponentName().getText(), new ResultHolder(type));
+          }
         return handle(body, context, resolver);
       }
     }

diff --git a/src/main/java/com/intellij/plugins/haxe/model/type/ResultHolder.java b/src/main/java/com/intellij/plugins/haxe/model/type/ResultHolder.java
@@ -64,11 +64,11 @@ public SpecificEnumValueReference getEnumValueType() {
     return (type instanceof SpecificEnumValueReference enumValueReference) ? enumValueReference : null;
   }
 
-  @Nullable
+
   public boolean isFunctionType() {
     return (type instanceof SpecificFunctionReference);
   }
-  @Nullable
+
   public boolean isClassType() {
     return (type instanceof SpecificHaxeClassReference);
   }

diff --git a/src/main/java/com/intellij/plugins/haxe/util/HaxeResolveUtil.java b/src/main/java/com/intellij/plugins/haxe/util/HaxeResolveUtil.java
@@ -704,6 +704,22 @@ private static HaxeResolveResult getHaxeClassResolveResultInternal(@Nullable Psi
     }
     if (element instanceof HaxeIteratorkey || element instanceof HaxeIteratorValue) {
         final HaxeForStatement forStatement = getParentForStatement(element);
+        final HaxeIterable iterable = forStatement.getIterable();
+        if (iterable == null) {
+          // iterable is @Nullable
+          // (sometimes when you're typing for statement it becames null for short time)
+          return HaxeResolveResult.EMPTY;
+        }
+        ResultHolder type = findIteratorType(element);
+        if (type!= null && !type.isUnknown() && type.isClassType()) {
+          return type.getClassType().asResolveResult();
+        }
+
+        return HaxeResolveResult.EMPTY;
+    }
+    if (element instanceof HaxeValueIterator valueIterator) {
+      final HaxeForStatement forStatement = PsiTreeUtil.getParentOfType(valueIterator, HaxeForStatement.class);
+      if (forStatement != null) {
         final HaxeIterable iterable = forStatement.getIterable();
         if (iterable == null) {
           // iterable is @Nullable
@@ -713,15 +729,16 @@ private static HaxeResolveResult getHaxeClassResolveResultInternal(@Nullable Psi
         final HaxeExpression expression = iterable.getExpression();
         if (expression instanceof HaxeReference reference) {
 
-          ResultHolder type = findIteratorType(reference, element);
-          if (type!= null && !type.isUnknown() && type.isClassType()) {
-            return type.getClassType().asResolveResult();
-          }
-
+          final HaxeResolveResult resolveResult = reference.resolveHaxeClass();
+          List<String> circularReferenceProtection = new LinkedList<>();
+          return searchForIterableTypeRecursively(resolveResult, circularReferenceProtection);
         }
-        return HaxeResolveResult.EMPTY;
+      }
+      return HaxeResolveResult.EMPTY;
     }
+
     if (element instanceof HaxeForStatement) {
+      //TODO remove?
       final HaxeIterable iterable = ((HaxeForStatement)element).getIterable();
       if (iterable == null) {
         // iterable is @Nullable

diff --git a/src/test/resources/testData/parsing/haxe/expressions/Haxe3.txt b/src/test/resources/testData/parsing/haxe/expressions/Haxe3.txt
@@ -1664,9 +1664,10 @@ Haxe File
             FOR_STATEMENT
               HaxePsiToken:for('for')
               HaxePsiToken:(('(')
-              COMPONENT_NAME
-                IDENTIFIER
-                  HaxePsiToken:ID('i')
+              VALUE_ITERATOR
+                COMPONENT_NAME
+                  IDENTIFIER
+                    HaxePsiToken:ID('i')
               HaxePsiToken:in('in')
               ITERABLE
                 ITERATOR_EXPRESSION

diff --git a/src/test/resources/testData/parsing/haxe/statements/for/Simple.txt b/src/test/resources/testData/parsing/haxe/statements/for/Simple.txt
@@ -28,9 +28,10 @@ Haxe File
             FOR_STATEMENT
               HaxePsiToken:for('for')
               HaxePsiToken:(('(')
-              COMPONENT_NAME
-                IDENTIFIER
-                  HaxePsiToken:ID('i')
+              VALUE_ITERATOR
+                COMPONENT_NAME
+                  IDENTIFIER
+                    HaxePsiToken:ID('i')
               HaxePsiToken:in('in')
               ITERABLE
                 ITERATOR_EXPRESSION