Fixing typeParameter resolve bugs and adding tests (also fixed reific…

…ation bug in bnf)
HaxeFoundation · Apr 15, 2024 · 8df6227 · 8df6227
1 parent ac850b2
commit 8df6227
Show file tree

Hide file tree

Showing 18 changed files with 326 additions and 121 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,8 @@
 # Changelog
 ## 1.5.2
 * Fixed: Methods and properties in module scope are now resolved correctly.
+* Fixed: Issue with reification parsing
+* Fixed: incorrect type parameter use in some cases where Method typeParameter and classParameter was the same name.
 
 ## 1.5.1
 * Fixed: Issue setting up Haxe SDK in intelliJ 2024.1

diff --git a/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/HaxeCallExpressionUtil.java b/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/HaxeCallExpressionUtil.java
diff --git a/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/TypeParameterTable.java b/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/TypeParameterTable.java
@@ -0,0 +1,40 @@
+package com.intellij.plugins.haxe.ide.annotator.semantics;
+
+import com.intellij.plugins.haxe.model.type.ResultHolder;
+import com.intellij.plugins.haxe.model.type.resolver.ResolveSource;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class TypeParameterTable {
+
+  private record typeAndSource(ResultHolder type, ResolveSource resolveSource) {}
+
+  Map<String, typeAndSource> data = new HashMap<>();
+
+  public void put(@NotNull String name, ResultHolder holder, ResolveSource scope) {
+    data.put(name, new typeAndSource(holder, scope));
+  }
+
+  public boolean contains(String name) {
+    return data.containsKey(name);
+  }
+  public boolean contains(@NotNull String name, @NotNull ResolveSource scope) {
+    return data.entrySet().stream().anyMatch(entry -> entry.getKey().equals(name) && entry.getValue().resolveSource().equals(scope));
+  }
+
+  @Nullable
+  public ResultHolder get(@NotNull String name) {
+    if(!contains(name)) return null;
+    return data.get(name).type();
+  }
+  @Nullable
+  public ResultHolder get(@NotNull String name, ResolveSource scope) {
+    return data.entrySet().stream().filter(entry -> entry.getKey().equals(name) && entry.getValue().resolveSource().equals(scope))
+      .findFirst()
+      .map(entry -> entry.getValue().type)
+      .orElse(null);
+  }
+}
diff --git a/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/TypeParameterUtil.java b/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/TypeParameterUtil.java
@@ -18,23 +18,24 @@
 public class TypeParameterUtil {
 
   @NotNull
-  public static Map<String, ResultHolder> createTypeParameterConstraintMap(HaxeMethod method, HaxeGenericResolver resolver) {
-    Map<String, ResultHolder> typeParamMap = new HashMap<>();
+  public static TypeParameterTable createTypeParameterConstraintTable(HaxeMethod method, HaxeGenericResolver resolver) {
+
+    TypeParameterTable typeParamTable = new TypeParameterTable();
 
     HaxeMethodModel methodModel = method.getModel();
     if (methodModel != null) {
       List<HaxeGenericParamModel> params = methodModel.getGenericParams();
       for (HaxeGenericParamModel model : params) {
         ResultHolder constraint = model.getConstraint(resolver);
-        typeParamMap.put(model.getName(), constraint);
+        typeParamTable.put(model.getName(), constraint, ResolveSource.METHOD_TYPE_PARAMETER);
       }
       if (method.isConstructor()) {
         HaxeClassModel declaringClass = method.getModel().getDeclaringClass();
         if (declaringClass != null) {
           params = declaringClass.getGenericParams();
           for (HaxeGenericParamModel model : params) {
             ResultHolder constraint = model.getConstraint(resolver);
-            typeParamMap.put(model.getName(), constraint);
+            typeParamTable.put(model.getName(), constraint, ResolveSource.CLASS_TYPE_PARAMETER);
           }
         }
       }
@@ -44,44 +45,49 @@ public static Map<String, ResultHolder> createTypeParameterConstraintMap(HaxeMet
         List<HaxeGenericParamModel> classParams = declaringClass.getGenericParams();
         for (HaxeGenericParamModel model : classParams) {
           ResultHolder constraint = model.getConstraint(resolver);
-          typeParamMap.put(model.getName(), constraint);
+          // make sure we do not add if method type parameter with the same name is present
+          if(!typeParamTable.contains(model.getName(), ResolveSource.METHOD_TYPE_PARAMETER)) {
+            typeParamTable.put(model.getName(), constraint, ResolveSource.CLASS_TYPE_PARAMETER);
+          }
         }
       }
     }
-    return typeParamMap;
+    return typeParamTable;
   }
 
-  public static void applyCallieConstraints(Map<String, ResultHolder> map, HaxeGenericResolver callieResolver) {
+  public static void applyCallieConstraints(TypeParameterTable table, HaxeGenericResolver callieResolver) {
     HaxeGenericResolver resolver = new HaxeGenericResolver();
     resolver.addAll(callieResolver, ResolveSource.CLASS_TYPE_PARAMETER);
 
     for (String name : resolver.names()) {
-      ResultHolder resolve = resolver.resolve(name);
-      map.put(name, resolve);
+      if(table.contains(name, ResolveSource.CLASS_TYPE_PARAMETER)) {
+        ResultHolder resolve = resolver.resolve(name);
+        table.put(name, resolve, ResolveSource.CLASS_TYPE_PARAMETER);
+      }
     }
   }
 
 
   @NotNull
-  static Map<String, ResultHolder> createTypeParameterConstraintMap(List<HaxeGenericParamModel> modelList,
+  static TypeParameterTable  createTypeParameterConstraintTable(List<HaxeGenericParamModel> modelList,
                                                                            HaxeGenericResolver resolver) {
-    Map<String, ResultHolder> typeParamMap = new HashMap<>();
+    TypeParameterTable typeParamTable = new TypeParameterTable();
     for (HaxeGenericParamModel model : modelList) {
       ResultHolder constraint = model.getConstraint(resolver);
       if (constraint != null && constraint.isUnknown()) {
-        typeParamMap.put(model.getName(), constraint);
+        typeParamTable.put(model.getName(), constraint, ResolveSource.CLASS_TYPE_PARAMETER);
       }
     }
-    return typeParamMap;
+    return typeParamTable;
   }
 
-  static boolean containsTypeParameter(@NotNull ResultHolder parameterType, @NotNull Map<String, ResultHolder> typeParamMap) {
+  static boolean containsTypeParameter(@NotNull ResultHolder parameterType, @NotNull TypeParameterTable typeParamTable) {
     if (parameterType.getClassType() != null) {
       if (parameterType.getClassType().isTypeParameter()) return true;
 
       ResultHolder[] specifics = parameterType.getClassType().getSpecifics();
       if (specifics.length == 0) {
-        return typeParamMap.containsKey(parameterType.getClassType().getClassName());
+        return typeParamTable.contains(parameterType.getClassType().getClassName());
       }
       List<ResultHolder> recursionGuard = new ArrayList<>();
 
@@ -91,7 +97,7 @@ static boolean containsTypeParameter(@NotNull ResultHolder parameterType, @NotNu
           List<ResultHolder> list = getSpecificsIfClass(specific, recursionGuard);
           if (list.stream()
             .map(holder -> holder.getClassType().getClassName())
-            .anyMatch(typeParamMap::containsKey)) {
+            .anyMatch(typeParamTable::contains)) {
             return true;
           }
         }
@@ -100,21 +106,21 @@ static boolean containsTypeParameter(@NotNull ResultHolder parameterType, @NotNu
       SpecificFunctionReference fn = parameterType.getFunctionType();
       List<SpecificFunctionReference.Argument> arguments = fn.getArguments();
       if (arguments != null) {
-        boolean anyMatch = arguments.stream().anyMatch(argument -> containsTypeParameter(argument.getType(), typeParamMap));
+        boolean anyMatch = arguments.stream().anyMatch(argument -> containsTypeParameter(argument.getType(), typeParamTable));
         if(anyMatch) return true;
       }
-      return containsTypeParameter(fn.getReturnType(), typeParamMap);
+      return containsTypeParameter(fn.getReturnType(), typeParamTable);
     }
 
     return false;
   }
-  static Optional<ResultHolder> findConstraintForTypeParameter(HaxeParameterModel parameter, @NotNull ResultHolder parameterType, @NotNull Map<String, ResultHolder> typeParamMap) {
+  static Optional<ResultHolder> findConstraintForTypeParameter(HaxeParameterModel parameter, @NotNull ResultHolder parameterType, @NotNull TypeParameterTable typeParamTable) {
     if (!parameterType.isClassType()) return Optional.empty();
 
     ResultHolder[] specifics = parameterType.getClassType().getSpecifics();
     if (specifics.length == 0){
       String className = parameterType.getClassType().getClassName();
-      return typeParamMap.containsKey(className) ? Optional.ofNullable(typeParamMap.get(className)) : Optional.empty();
+      return typeParamTable.contains(className) ? Optional.ofNullable(typeParamTable.get(className)) : Optional.empty();
     }
     List<ResultHolder> result = new ArrayList<>();
     List<ResultHolder> recursionGuard = new ArrayList<>();
@@ -129,9 +135,9 @@ static Optional<ResultHolder> findConstraintForTypeParameter(HaxeParameterModel
 
 
     result.stream().map(holder -> holder.getClassType().getClassName())
-      .filter(typeParamMap::containsKey)
-      .filter( s ->  typeParamMap.get(s) != null)
-      .forEach( s ->  resolver.addConstraint(s, typeParamMap.get(s), ResolveSource.CLASS_TYPE_PARAMETER));
+      .filter(typeParamTable::contains)
+      .filter( s ->  typeParamTable.get(s) != null)
+      .forEach( s ->  resolver.addConstraint(s, typeParamTable.get(s), ResolveSource.CLASS_TYPE_PARAMETER));
 
     ResultHolder type = HaxeTypeResolver.getTypeFromTypeTag(parameter.getTypeTagPsi(), parameter.getNamePsi());
     if (type.isTypeParameter()) {
@@ -154,14 +160,4 @@ private static List<ResultHolder> getSpecificsIfClass(@NotNull ResultHolder hold
     }
   return result;
   }
-  //private static Stream<ResultHolder> getSpecificsIfClass(@NotNull ResultHolder holder) {
-  //  List<ResultHolder> proccesed = new ArrayList<>();
-  //  @NotNull ResultHolder[] specifics = holder.getClassType().getSpecifics();
-  //  if (specifics.length == 0) return Stream.of(holder);
-  //  return Arrays.stream(specifics)
-  //    .filter(ResultHolder::isClassType)
-  //    .filter(h -> !proccesed.contains(h) )// avoid recursive loop (classes can have itself as type parameter)
-  //    .peek(proccesed::add)
-  //    .flatMap(TypeParameterUtil::getSpecificsIfClass);
-  //}
 }
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
@@ -238,7 +238,7 @@ suffixOperator ::= '!'                { extends=operator }
 // KFROM ('from') and KTO ('to') are only keywords for abstracts and can be used as identifiers elsewhere in the code (KNEVER ('never') is only used for getters/setters)
 private identifierWithKeywordName::= 'from' | 'to' | 'never'
 
-identifier ::= ID | identifierWithKeywordName  | MACRO_ID | (<<canBeReification>> macroIdentifierReification)
+identifier ::= ID | identifierWithKeywordName  |  (<<canBeReification>> macroIdentifierReification | MACRO_ID)
 {mixin="com.intellij.plugins.haxe.lang.psi.impl.HaxeIdentifierPsiMixinImpl" implements="com.intellij.plugins.haxe.lang.psi.HaxeIdentifierPsiMixin" name="identifier"}
 
 thisExpression ::= 'this'

diff --git a/src/main/java/com/intellij/plugins/haxe/model/HaxeParameterModel.java b/src/main/java/com/intellij/plugins/haxe/model/HaxeParameterModel.java
@@ -130,7 +130,7 @@ else if(type instanceof SpecificFunctionReference functionReference) {
 
       if(type instanceof SpecificHaxeClassReference classReference) {
         ResultHolder resolved = resolver.resolve(classReference.getClassName());
-        if (resolved != null) return resolved;
+        if (resolved != null && !resolved.isUnknown()) return resolved;
       }
 
     }

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
@@ -240,7 +240,12 @@ static ResultHolder handleReferenceExpression( HaxeExpressionEvaluatorContext co
 
             if (haxeClass.isGeneric()) {
               @NotNull ResultHolder[] specifics = resolver.getSpecificsFor(classReference);
-              typeHolder = SpecificHaxeClassReference.withGenerics(classReference, specifics).createHolder();
+              SpecificHaxeClassReference specificReference = SpecificHaxeClassReference.withGenerics(classReference, specifics);
+              // hackish way to ignore typeParameters for dynamic if not in expression
+              if (specificReference.isDynamic() && element.textMatches("Dynamic")) {
+                specificReference = SpecificHaxeClassReference.getDynamic(element);
+              }
+              typeHolder = specificReference.createHolder();
             }
             else {
               typeHolder = SpecificHaxeClassReference.withoutGenerics(classReference).createHolder();

diff --git a/src/main/java/com/intellij/plugins/haxe/model/type/HaxeGenericResolver.java b/src/main/java/com/intellij/plugins/haxe/model/type/HaxeGenericResolver.java
@@ -30,7 +30,7 @@
 import java.util.*;
 import java.util.stream.Collectors;
 
-import static com.intellij.plugins.haxe.model.type.resolver.ResolveSource.ARGUMENT_TYPE;
+import static com.intellij.plugins.haxe.model.type.resolver.ResolveSource.*;
 
 public class HaxeGenericResolver {
   // This must remain ordered, thus the LinkedHashMap.
@@ -453,6 +453,10 @@ public SpecificFunctionReference resolve(SpecificFunctionReference fnRef, boolea
   public String[] names() {
     return resolvers.stream().map(ResolverEntry::name).toArray(String[]::new);
   }
+  @NotNull
+  public ResolverEntry[] entries() {
+    return resolvers.toArray(ResolverEntry[]::new);
+  }
 
   /**
    * @return All specific generic types in this resolver in the order of their adding.
@@ -516,6 +520,12 @@ public HaxeGenericResolver without(String name) {
     }
     return resolver;
   }
+  public HaxeGenericResolver copy() {
+    HaxeGenericResolver resolver = new HaxeGenericResolver();
+    resolver.resolvers.addAll(resolvers);
+    resolver.constaints.addAll(constaints);
+    return resolver;
+  }
 
   private ResultHolder useAssignHintIfPossible(ResultHolder type) {
     Optional<ResolverEntry> assign = findAssignToType();
@@ -638,4 +648,21 @@ public void removeAll(String[] names) {
       constaints.removeIf(entry -> entry.name().equals(name));
     }
   }
+
+  public HaxeGenericResolver removeClassScopeIfMethodIsPresent() {
+    List<String> methodTypeParameters = new ArrayList<>();
+    methodTypeParameters.addAll(resolvers.stream().filter(entry -> entry.resolveSource() == METHOD_TYPE_PARAMETER).map(ResolverEntry::name).toList());
+    methodTypeParameters.addAll(constaints.stream().filter(entry -> entry.resolveSource() == METHOD_TYPE_PARAMETER).map(ResolverEntry::name).toList());
+    HaxeGenericResolver copy = copy();
+    copy.resolvers.removeIf(entry ->methodTypeParameters.contains(entry.name())  &&  entry.resolveSource() == CLASS_TYPE_PARAMETER);
+    copy.constaints.removeIf(entry ->methodTypeParameters.contains(entry.name())  &&  entry.resolveSource() == CLASS_TYPE_PARAMETER);
+    return copy;
+  }
+
+  public HaxeGenericResolver withoutMethodTypeParameters() {
+    HaxeGenericResolver copy = copy();
+    copy.resolvers.removeIf(entry -> entry.resolveSource() == METHOD_TYPE_PARAMETER);
+    copy.constaints.removeIf(entry -> entry.resolveSource() == METHOD_TYPE_PARAMETER);
+    return copy;
+  }
 }
diff --git a/src/main/java/com/intellij/plugins/haxe/model/type/HaxeTypeResolver.java b/src/main/java/com/intellij/plugins/haxe/model/type/HaxeTypeResolver.java
@@ -71,6 +71,7 @@ static public ResultHolder getMethodFunctionType(PsiElement comp, @Nullable Haxe
       resolver = resolver == null ? null : resolver.withoutUnknowns();
       HaxeGenericResolver methodResolver = method.getModel().getGenericResolver(null);
       methodResolver.addAll(resolver);
+      methodResolver = methodResolver.removeClassScopeIfMethodIsPresent().withoutUnknowns();
       return method.getModel().getFunctionType(methodResolver).createHolder();
     }
     // @TODO: error
@@ -151,6 +152,13 @@ static private ResultHolder getFieldType(HaxeNamedComponent comp, HaxeGenericRes
         return result;
       }
     }
+    if (comp instanceof  HaxeGenericListPart genericListPart) {
+      HaxeComponentName componentName = genericListPart.getComponentName();
+      if(componentName != null) {
+        HaxeClassReference reference = new HaxeClassReference(genericListPart.getName(), componentName, true);
+        return SpecificHaxeClassReference.withoutGenerics(reference).createHolder();
+      }
+    }
 
     return SpecificTypeReference.getUnknown(comp).createHolder();
   }
@@ -334,16 +342,9 @@ else if (comp instanceof HaxeMethod method) {
       List<ResultHolder> returnTypes = returnStatementList.stream().map(statement -> getPsiElementType(statement, resolver)).toList();
       if (returnTypes.isEmpty()) return SpecificHaxeClassReference.getVoid(psi).createHolder();
       ResultHolder holder = HaxeTypeUnifier.unifyHolders(returnTypes, psi);
-      return resolveParameterizedType(holder, resolver);
-
-      ////Performance tweak avoiding  full body evaluation if it does not contain any return statements (no returns = void)
-      //HaxeReturnStatement returnStatement = findReturnStatementForMethod(psi);
-      //if (returnStatement == null || isVoidReturn(returnStatement)) {
-      //  return SpecificHaxeClassReference.getVoid(psi).createHolder();
-      //}
-      //
-      //final HaxeExpressionEvaluatorContext context = getPsiElementType(psi, (AnnotationHolder)null, resolver);
-      //return resolveParameterizedType(context.getReturnType(), resolver);
+
+      // method typeParameters should have been used when resolving returnTypes, we want to avoid double resolve
+      return resolveParameterizedType(holder, resolver == null ? null : resolver.withoutMethodTypeParameters());
 
     }
     else if (comp instanceof HaxeFunctionLiteral) {

diff --git a/src/main/java/com/intellij/plugins/haxe/model/type/SpecificHaxeClassReference.java b/src/main/java/com/intellij/plugins/haxe/model/type/SpecificHaxeClassReference.java
@@ -808,7 +808,7 @@ public static ResultHolder propagateGenericsToType(@Nullable ResultHolder typeHo
                                    ? genericResolver.resolveReturnType(typeHolder)
                                    : genericResolver.resolve(typeParameterName);
 
-      if (possibleValue != null) {
+      if (possibleValue != null && !possibleValue.isUnknown()) {
         // TODO considder?
         //HaxeGenericResolver resolverWithoutCurrentTypeParam = genericResolver.without(typeParameterName);
         //ResultHolder holder = propagateGenericsToType(possibleValue, resolverWithoutCurrentTypeParam);

diff --git a/src/test/java/com/intellij/plugins/haxe/ide/inlay/HaxeForLoopInlayTest.java b/src/test/java/com/intellij/plugins/haxe/ide/inlay/HaxeForLoopInlayTest.java
@@ -26,6 +26,10 @@ public void setUp() throws Exception {
   public void testPreview() throws Exception {
     doTest(hintsProvider);
   }
+  @Test
+  public void testLoopGenerics() throws Exception {
+    doTest(hintsProvider);
+  }
 
 
 }
diff --git a/src/test/java/com/intellij/plugins/haxe/ide/inlay/HaxeInlayTestBase.java b/src/test/java/com/intellij/plugins/haxe/ide/inlay/HaxeInlayTestBase.java
@@ -1,6 +1,9 @@
 package com.intellij.plugins.haxe.ide.inlay;
 
 import com.intellij.codeInsight.hints.declarative.InlayHintsProvider;
+import com.intellij.openapi.diagnostic.DefaultLogger;
+import com.intellij.openapi.diagnostic.LogLevel;
+import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.roots.LanguageLevelProjectExtension;
 import com.intellij.openapi.util.RecursionManager;
@@ -25,6 +28,17 @@ public abstract class HaxeInlayTestBase extends DeclarativeInlayHintsProviderTes
   private final IdeaTestFixtureFactory testFixtureFactory = IdeaTestFixtureFactory.getFixtureFactory();
   private ModuleFixtureBuilder moduleFixtureBuilder;
 
+
+  protected HaxeInlayTestBase() {
+    super();
+    Logger.setUnitTestMode();
+    Logger.setFactory(category -> {
+      DefaultLogger logger = new DefaultLogger(category);
+      logger.setLevel(LogLevel.WARNING);
+      return logger;
+    });
+  }
+
   @Override
   protected abstract String getBasePath();
 

diff --git a/src/test/java/com/intellij/plugins/haxe/ide/inlay/HaxeReturnTypeInlayTest.java b/src/test/java/com/intellij/plugins/haxe/ide/inlay/HaxeReturnTypeInlayTest.java
@@ -27,5 +27,10 @@ public void testPreview() throws Exception {
     doTest(hintsProvider);
   }
 
+  @Test
+  public void testReturnTypeGenerics() throws Exception {
+    doTest(hintsProvider);
+  }
+
 
 }