Skip to content

Commit

Permalink
fixing monomorph issue for objectLiterals
Browse files Browse the repository at this point in the history
  • Loading branch information
@m0rkeulv
m0rkeulv committed Jan 11, 2025
1 parent 0a4b408 commit 7bd11bf
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 32 deletions.
5 changes: 3 additions & 2 deletions src/main/java/com/intellij/plugins/haxe/ide/index/HaxeInheritanceDefinitionsUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ public static Collection<HaxeClass> getItemsByQNameFirstLevelChildrenOnly(final
if (dumbService.isDumb()) {
dumbService.waitForSmartMode();
}

return dumbService.runReadActionInSmartMode(() -> _getItemsByQNameFirstLevelChildrenOnly(haxeClass, project));
return dumbService.tryRunReadActionInSmartMode(
() -> _getItemsByQNameFirstLevelChildrenOnly(haxeClass, project),
"Collecting inheritance information");
}


Expand Down
19 changes: 16 additions & 3 deletions src/main/java/com/intellij/plugins/haxe/model/evaluator/HaxeExpressionEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ static ResultHolder handle(@NotNull final PsiElement element,
static ResultHolder _handle(final PsiElement element,
final HaxeExpressionEvaluatorContext context,
HaxeGenericResolver optionalResolver) {

ProgressIndicatorProvider.checkCanceled();

if (element == null) {
return createUnknown(context.root);
}
Expand Down Expand Up @@ -550,7 +553,10 @@ public static ResultHolder searchReferencesForType(final HaxeComponentName compo
for (int i = 0, size = references.size(); i < size; i++) {
PsiReference reference = references.get(i);
ResultHolder possibleType = checkSearchResult(context, resolver, reference, componentName, hint, i == 0);
if (possibleType != null) {
if (possibleType == null) {
// if we get "null" we might be hitting a recursion guard and should probably not return any types found past this reference
return createUnknown(componentName);
} else {
if (!possibleType.isUnknown()) {
if (lastValue == null) {
lastValue = possibleType;
Expand Down Expand Up @@ -735,10 +741,17 @@ private static ResultHolder checkSearchResult(HaxeExpressionEvaluatorContext con
if (callExpression.getExpression() instanceof HaxeReference callExpressionReference) {
PsiElement resolve = callExpressionReference.resolve();
ResultHolder holder = handleWithRecursionGuard(resolve, context, resolver);
SpecificFunctionReference functionType = null;

if (holder != null && holder.getFunctionType() != null) {
SpecificFunctionReference functionCall = holder.getFunctionType();
functionType = holder.getFunctionType();
}else if (resolve instanceof HaxeMethod method) {
functionType = method.getModel().getFunctionType(resolver);
}

if (functionType != null) {
HaxeCallExpressionList list = callExpression.getExpressionList();
objectLiteralType = findUsageAsParameterInFunctionCall(objectLiteral, callExpression, list, functionCall);
objectLiteralType = findUsageAsParameterInFunctionCall(objectLiteral, callExpression, list, functionType);
}
}
}
Expand Down
73 changes: 46 additions & 27 deletions src/main/java/com/intellij/plugins/haxe/model/evaluator/HaxeExpressionUsageUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public static ResultHolder searchReferencesForTypeParameters(final HaxeComponent
// AND stop any other logic picking up typeParameters from later reference when current reference is skipped by the recursion guard.
// This is a common problem when you got a variable that gets its typeParameters from method calls on that instance,
// and our code will try to find callie type
var newValues = searchReferencesForTypeParametersRecursionGuard.computePreventingRecursion(componentName, true, () -> {
var newValues = searchReferencesForTypeParametersRecursionGuard.computePreventingRecursion(componentName, false, () -> {
ResultHolder originalType = resultHolder.duplicate();
SpecificHaxeClassReference classType = originalType.getClassType();
// TODO mlo: should we add some kind of support for functions here ?
Expand All @@ -113,26 +113,39 @@ public static ResultHolder searchReferencesForTypeParameters(final HaxeComponent
final SearchScope useScope = searchHelper.getCodeUsageScope(componentName);

List<PsiReference> references = referenceSearch(componentName, useScope);
// search until all typeParams are found or we dont have any more references

// search until all typeParams are found or we dont have any more references or we stop due to recursion guard.
//
// a few notes on this loop and the method it calls
// - if information found they return a real type
// - if no information is found they return unknown
// - if stopped by recursion guard og result point it itself we return null
//
// when we get a null we break the loop as any reference occurring later can not guarantee that it wont
// provide other type parameters that are not compatible with our current reference that we are still
// trying to evaluate.
for (int i = continueFrom, size = references.size(); i < size; i++) {

PsiReference reference = references.get(i);

// if we are trying to resolve type for objectLiteral reference and have reach ourselves, we stop to prevent later references.
if (context.root.getParent() instanceof HaxeObjectLiteralElement) {
if (context.root == references.get(0)) return null;
}

if (reference instanceof HaxeExpression expression) {
PsiElement parent = expression.getParent();

if (reference instanceof HaxeReferenceExpression referenceExpression) {
ResultHolder result = tryFindTypeWhenUsedAsParameterInCallExpression(originalType, referenceExpression, parent);
if (result != null) originalType = mapTypeParameterIfAssignable(originalType, result);
if (result == null) return null;
if (!result.isUnknown()) originalType = mapTypeParameterIfAssignable(originalType, result);
if (!originalType.containsUnknownTypes()) return originalType;
}

if (parent instanceof HaxeAssignExpression assignExpression) {
ResultHolder assignType = tryTypeFromAssignExpression(context, resolver, originalType, assignExpression, componentName);


if (assignType != null) {
if (assignType == null) return null;
if (!assignType.isUnknown()) {
// we want to ignore assign to null value (flag to not change isFirst)
if (!(assignType.getConstant() instanceof HaxeNull)) {
boolean isRightExpresion = false;
Expand All @@ -143,7 +156,6 @@ public static ResultHolder searchReferencesForTypeParameters(final HaxeComponent
if (resolve instanceof HaxeNamedComponent namedComponent) {
if (namedComponent.getComponentName() == componentName) {
isRightExpresion = true;

}
}
}
Expand All @@ -157,33 +169,36 @@ public static ResultHolder searchReferencesForTypeParameters(final HaxeComponent
originalType = mapTypeParameter(originalType, assignType);
}
}

}
}
if (!originalType.containsUnknownTypes()) return originalType;
}

if (parent instanceof HaxeReferenceExpression referenceExpression) {
ResultHolder result = tryFindTypeFromMethodCallOnReference(originalType, referenceExpression);
if (result != null) originalType = mapTypeParameterIfAssignable(originalType, result);
if (result == null) return null;
if (!result.isUnknown()) originalType = mapTypeParameterIfAssignable(originalType, result);
if (!originalType.containsUnknownTypes()) return originalType;
}

if (parent instanceof HaxeObjectLiteralElement literalElement) {
ResultHolder result = tryTypeFromObjectLiteral(context, resolver, literalElement);
if (result != null) originalType = mapTypeParameterIfAssignable(originalType, result);
if (result == null) return null;
if (!result.isUnknown()) originalType = mapTypeParameterIfAssignable(originalType, result);
if (!originalType.containsUnknownTypes()) return originalType;
}

if (parent instanceof HaxeArrayAccessExpression arrayAccessExpression) {
ResultHolder result = tryUpdateTypeParamFromArrayAccess(context, resolver, arrayAccessExpression, classType, classResolver, classType);
if (result != null) originalType = mapTypeParameterIfAssignable(originalType, result);
if (result == null) return null;
if (!result.isUnknown()) originalType = mapTypeParameterIfAssignable(originalType, result);
if (!originalType.containsUnknownTypes()) return originalType;
}

if (parent instanceof HaxeObjectLiteralElement literalElement) {
ResultHolder result = tryUpdateTypeParamFromObjectLiteral(context, resolver, literalElement, classType);
if (result != null) originalType = mapTypeParameterIfAssignable(originalType, result);
if (result == null) return null;
if (!result.isUnknown()) originalType = mapTypeParameterIfAssignable(originalType, result);
if (!originalType.containsUnknownTypes()) return originalType;
}
}
Expand Down Expand Up @@ -240,7 +255,7 @@ private static ResultHolder tryUpdateTypeParamFromObjectLiteral(HaxeExpressionEv
HaxeObjectLiteralElement literalElement,
SpecificHaxeClassReference type) {
HaxeObjectLiteral objectLiteral = PsiTreeUtil.getParentOfType(literalElement, HaxeObjectLiteral.class);
if (objectLiteral == null) return null;
if (objectLiteral == null) return createUnknown(literalElement);

ResultHolder objectLiteralType = findObjectLiteralType(context, resolver, objectLiteral);

Expand All @@ -254,12 +269,13 @@ private static ResultHolder tryUpdateTypeParamFromObjectLiteral(HaxeExpressionEv
ResultHolder objectLiteralElementType = objectLiteralElementAsMember.getResultType(resolver);
if (objectLiteralElementType.getClassType() != null) {
HaxeGenericResolver genericResolver = objectLiteralElementType.getClassType().getGenericResolver();
return genericResolver.resolve(type.createHolder());
ResultHolder resolve = genericResolver.resolve(type.createHolder());
if (resolve != null) return resolve;
}
}
}
}
return null;
return createUnknown(literalElement);
}

private static ResultHolder tryUpdateTypeParamFromArrayAccess(HaxeExpressionEvaluatorContext context,
Expand Down Expand Up @@ -301,7 +317,8 @@ private static ResultHolder tryUpdateTypeParamFromArrayAccess(HaxeExpressionEval
}
}
HaxeGenericResolver resolverForClass = localResolver.translateFromTo(target, classType.getHaxeClass());
return resolverForClass.resolve(classType.createHolder());
ResultHolder resolve = resolverForClass.resolve(classType.createHolder());
if (resolve != null)return resolve;

} else {
HaxeNamedComponent arrayAccessGetter = classType.getHaxeClass().findArrayAccessGetter(resolver);
Expand All @@ -323,11 +340,12 @@ private static ResultHolder tryUpdateTypeParamFromArrayAccess(HaxeExpressionEval
}
}
HaxeGenericResolver resolverForClass = localResolver.translateFromTo(target, classType.getHaxeClass());
return resolverForClass.resolve(classType.createHolder());
ResultHolder resolve = resolverForClass.resolve(classType.createHolder());
if (resolve != null) return resolve;
}
}
}
return null;
return createUnknown(arrayAccessExpression);
}

private static @Nullable HaxeTypeParameterDeclaration tryGetTypeParameterFromTypeTag(HaxeTypeTag keyParamPsi) {
Expand All @@ -340,6 +358,7 @@ private static ResultHolder tryUpdateTypeParamFromArrayAccess(HaxeExpressionEval
return null;
}

// if returns null if stopped because of recursion, else real type or unknown
private static @Nullable ResultHolder tryTypeFromObjectLiteral(HaxeExpressionEvaluatorContext context,
HaxeGenericResolver resolver,
HaxeObjectLiteralElement literalElement) {
Expand All @@ -356,14 +375,14 @@ private static ResultHolder tryUpdateTypeParamFromArrayAccess(HaxeExpressionEval
HaxeBaseMemberModel member = classModel.getMember(literalElement.getName(), genericResolver);
if (member != null) {
ResultHolder resultType = member.getResultType(genericResolver);
if (resultType != null && !resultType.isUnknown()) {
if (resultType != null) {
return resultType;
}
}
}
}
}
return null;
return createUnknown(context.root);
});
if (result != null) return result;
}
Expand All @@ -387,13 +406,13 @@ private static ResultHolder tryUpdateTypeParamFromArrayAccess(HaxeExpressionEval
HaxeClass methodDeclaringClass = methodModel.getDeclaringClass().haxeClass;
HaxeGenericResolver translatedResolver = resolverFromCallExpression.translateFromTo(methodDeclaringClass, classType.getHaxeClass());
ResultHolder resolve = translatedResolver.resolve(classType.replaceUnknownsWithTypeParameter());
if (resolve != null && !resolve.isUnknown()) {
if (resolve != null) {
return resolve;
}
}
}
}
return null;
return createUnknown(referenceExpression);
}

private static @Nullable ResultHolder tryTypeFromAssignExpression(HaxeExpressionEvaluatorContext context,
Expand Down Expand Up @@ -440,9 +459,10 @@ private static ResultHolder tryUpdateTypeParamFromArrayAccess(HaxeExpressionEval
}
}

return null;
return createUnknown(assignExpression);
}


private static @Nullable ResultHolder tryFindTypeWhenUsedAsParameterInCallExpression(ResultHolder resultHolder,
HaxeReferenceExpression referenceExpression,
PsiElement parent) {
Expand All @@ -454,12 +474,11 @@ private static ResultHolder tryUpdateTypeParamFromArrayAccess(HaxeExpressionEval
ResultHolder paramType = findUsageAsParameterInFunctionCall(referenceExpression, callExpression, list, resolved);
if (paramType != null) {
// probably not the best solution, but the goal is to keep the original type and only update typeParameters
ResultHolder unified = HaxeTypeUnifier.unify(resultHolder, paramType);
if (!unified.isUnknown()) return unified;
return HaxeTypeUnifier.unify(resultHolder, paramType);
}
}
}
return null;
return createUnknown(referenceExpression);
}


Expand Down
5 changes: 5 additions & 0 deletions src/test/java/com/intellij/plugins/haxe/ide/inlay/HaxeLocalVarInlayTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,10 @@ public void testOptionalFieldsHints() throws Exception {
doTest(hintsProvider);
}

@Test
public void testComplexMonomorphHints() throws Exception {
doTest(hintsProvider);
}


}
45 changes: 45 additions & 0 deletions src/test/resources/testData/inlay/haxe.local.variable/ComplexMonomorphHints.hx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package testData.inlay.haxe.local.variable;

typedef ObjectDefinition<T> = {
array:Array<T>
}

class ComplexTypeParameterMonomorphTest {

public function testMethodCall() {

// this variable should get ":Array<String>" from BuildContext parameter type
// issues with recursion guard and incorrect caching will incorrectly cause this to become ":Array<Int>"
var testArray/*<# :Array<String> #>*/ = [];

morphByParameter({array: testArray });

// its important that the monomorph of "testArray" does not try to pick up its type from this usage
// it will then default to Array<Int> as the parameters are optional and it we would incorreclty
// use the first one

useMorphedValue(testArray);
}
public function testFunctionType(morphByFunctionCall:ObjectDefinition<Float>->Void) {

// this variable should get ":Array<Float>" from the parameter type BuildContext as part as the fuctionType
// issues with recursion guard and incorrect caching will incorrectly cause this to become ":Array<Int>"
var testArray/*<# :Array<Float> #>*/ = [];

morphByFunctionCall({array: testArray });

// its important that the monomorph of "testArray" does not try to pick up its type from this usage
// it will then default to Array<Int> as the parameters are optional and it we would incorreclty
// use the first one

useMorphedValue(testArray);
}

public function fromOptional() {

var testArray/*<# :Array<Int> #>*/ = [];
useMorphedValue(testArray);
}
public static function morphByParameter(value:ObjectDefinition<String>) {}
public static function useMorphedValue(?optional1:Array<Int>, ?optional2:Array<String>, ?optional3:Array<Float>) {}
}

0 comments on commit 7bd11bf

Please sign in to comment.