From 3b999cd9c5488ace51fb4c5e769a7937d45d0c36 Mon Sep 17 00:00:00 2001 From: m0rkeulv Date: Tue, 3 Sep 2024 21:56:26 +0200 Subject: [PATCH] better checks and error messages for struct member types (+ fix stackoverflow issue after typeParameter hierarchy fix) --- .../ide/annotator/HaxeStandardAnnotation.java | 21 ++++++++--- .../HaxeAssignExpressionAnnotator.java | 3 +- .../semantics/HaxeCallExpressionUtil.java | 35 +++++++++++------- .../semantics/HaxeSemanticsUtil.java | 2 +- .../haxe/model/type/HaxeAssignContext.java | 12 +++++- .../model/type/HaxeGenericResolverUtil.java | 4 ++ .../haxe/model/type/HaxeTypeCompatible.java | 37 ++++++++++++++++++- .../AssignAnonymousType.hx | 6 +-- .../annotation.semantic/AssignStructInit.hx | 6 +-- 9 files changed, 94 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/intellij/plugins/haxe/ide/annotator/HaxeStandardAnnotation.java b/src/main/java/com/intellij/plugins/haxe/ide/annotator/HaxeStandardAnnotation.java index c824fa94a..e3e356377 100644 --- a/src/main/java/com/intellij/plugins/haxe/ide/annotator/HaxeStandardAnnotation.java +++ b/src/main/java/com/intellij/plugins/haxe/ide/annotator/HaxeStandardAnnotation.java @@ -18,11 +18,14 @@ import com.intellij.lang.annotation.AnnotationBuilder; import com.intellij.lang.annotation.AnnotationHolder; import com.intellij.lang.annotation.HighlightSeverity; +import com.intellij.openapi.util.TextRange; import com.intellij.plugins.haxe.HaxeBundle; import com.intellij.plugins.haxe.model.type.HaxeAssignContext; import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; +import java.util.Map; + /** * A library of annotations that can be re-used. Place annotations that are used more than * once (or should be) in this class. @@ -49,13 +52,19 @@ private HaxeStandardAnnotation() { context.getMissingMembersString()); return holder.newAnnotation(HighlightSeverity.ERROR, message).range(incompatibleElement); } - public static @NotNull AnnotationBuilder typeMismatchWrongTypeMembers(@NotNull AnnotationHolder holder, - @NotNull PsiElement incompatibleElement, - HaxeAssignContext context) { + public static @NotNull void addtypeMismatchWrongTypeMembersAnnotations(@NotNull AnnotationHolder holder, + @NotNull PsiElement incompatibleElement, + HaxeAssignContext context) { - String message = HaxeBundle.message("haxe.semantic.incompatible.type.wrong.member.types.0", context.geWrongTypeMembersString()); - - return holder.newAnnotation(HighlightSeverity.ERROR, message).range(incompatibleElement); + TextRange expectedRange = incompatibleElement.getTextRange(); + Map wrongTypeMap = context.getWrongTypeMap(); + boolean allInRange = wrongTypeMap.keySet().stream().allMatch(psi -> expectedRange.contains(psi.getTextRange())); + if (allInRange) { + wrongTypeMap.forEach((key, value) -> holder.newAnnotation(HighlightSeverity.ERROR, value).range(key).create()); + }else { + String message = HaxeBundle.message("haxe.semantic.incompatible.type.wrong.member.types.0", context.geWrongTypeMembersString()); + holder.newAnnotation(HighlightSeverity.ERROR, message).range(incompatibleElement).create(); + } } public static @NotNull AnnotationBuilder returnTypeMismatch(@NotNull AnnotationHolder holder, diff --git a/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/HaxeAssignExpressionAnnotator.java b/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/HaxeAssignExpressionAnnotator.java index fdfccc943..414b91424 100644 --- a/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/HaxeAssignExpressionAnnotator.java +++ b/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/HaxeAssignExpressionAnnotator.java @@ -79,8 +79,7 @@ public static void check(HaxeAssignExpression psi, AnnotationHolder holder) { .create(); } if(context.hasWrongTypeMembers()) { - HaxeStandardAnnotation.typeMismatchWrongTypeMembers(holder, rhs, context) - .create(); + HaxeStandardAnnotation.addtypeMismatchWrongTypeMembersAnnotations(holder, rhs, context); } }else { AnnotationBuilder builder = typeMismatch(holder, rhs, rhsType.toPresentationString(), lhsType.toPresentationString()); 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 index 9af5822f4..e8a5cf891 100644 --- 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 @@ -165,7 +165,7 @@ public static CallExpressionValidation checkMethodCall(@NotNull HaxeCallExpressi else { // out of parameters and last is not var arg, must mean that ve have skipped optionals and still had arguments left if (parameterType != null && argumentType != null) { - validation.errors.add(annotateTypeMismatch(parameterType, argumentType, argument, null)); + validation.errors.addAll(annotateTypeMismatch(parameterType, argumentType, argument, null)); } break; } @@ -237,7 +237,7 @@ public static CallExpressionValidation checkMethodCall(@NotNull HaxeCallExpressi }else if (argumentType.isClassType() && argumentType.isMissingClassModel()){ validation.warnings.add(annotateUnableToCompare( argumentType, argument)); }else { - validation.errors.add(annotateTypeMismatch(constraint, argumentType, argument, assignContext)); + validation.errors.addAll(annotateTypeMismatch(constraint, argumentType, argument, assignContext)); } addToIndexMap(validation, argumentCounter, parameterCounter); } @@ -259,7 +259,7 @@ public static CallExpressionValidation checkMethodCall(@NotNull HaxeCallExpressi }else if (argumentType.isClassType() && argumentType.isMissingClassModel()){ validation.warnings.add(annotateUnableToCompare( argumentType, argument)); }else { - validation.errors.add(annotateTypeMismatch(resolvedParameterType, argumentType, argument, assignContext)); + validation.errors.addAll(annotateTypeMismatch(resolvedParameterType, argumentType, argument, assignContext)); } addToIndexMap(validation, argumentCounter, parameterCounter); } @@ -397,7 +397,7 @@ public static CallExpressionValidation checkFunctionCall(HaxeCallExpression call else { // out of parameters and last is not var arg, must mean that ve have skipped optionals and still had arguments left if (parameterType != null && argumentType != null) { - validation.errors.add(annotateTypeMismatch(parameterType, argumentType, argument, null)); + validation.errors.addAll(annotateTypeMismatch(parameterType, argumentType, argument, null)); } break; } @@ -429,7 +429,7 @@ public static CallExpressionValidation checkFunctionCall(HaxeCallExpression call if (parameter.isOptional()) { argumentCounter--; //retry argument with next parameter } else { - validation.errors.add(annotateTypeMismatch(parameterType, argumentType, argument, assignContext)); + validation.errors.addAll(annotateTypeMismatch(parameterType, argumentType, argument, assignContext)); addToIndexMap(validation, argumentCounter, parameterCounter); } } @@ -592,7 +592,7 @@ public static CallExpressionValidation checkConstructor(HaxeNewExpression newExp argumentCounter--; //retry argument with next parameter } else { - validation.errors.add(annotateTypeMismatch(constraint, argumentType, argument, assignContext)); + validation.errors.addAll(annotateTypeMismatch(constraint, argumentType, argument, assignContext)); addToIndexMap(validation, argumentCounter, parameterCounter); } } @@ -608,7 +608,7 @@ public static CallExpressionValidation checkConstructor(HaxeNewExpression newExp if (parameter.isOptional()) { argumentCounter--; //retry argument with next parameter }else { - validation.errors.add(annotateTypeMismatch(parameterType, argumentType, argument, assignContext)); + validation.errors.addAll(annotateTypeMismatch(parameterType, argumentType, argument, assignContext)); addToIndexMap(validation, argumentCounter, parameterCounter); } } @@ -839,25 +839,34 @@ private static HaxeGenericResolver findTypeParametersToInherit(SpecificTypeRefer } - private static ErrorRecord annotateTypeMismatch(ResultHolder expected, ResultHolder got, HaxeExpression expression, + private static List annotateTypeMismatch(ResultHolder expected, ResultHolder got, HaxeExpression expression, HaxeAssignContext context) { if (context != null) { if (context.hasMissingMembers()) { String message = HaxeBundle.message("haxe.semantic.method.parameter.mismatch.missing.members", context.getMissingMembersString()); - return new ErrorRecord(expression.getTextRange(), message); + return List.of(new ErrorRecord(expression.getTextRange(), message)); } else if (context.hasWrongTypeMembers()) { - String message = HaxeBundle.message("haxe.semantic.method.parameter.mismatch.wrong.type.members", - context.geWrongTypeMembersString()); - return new ErrorRecord(expression.getTextRange(), message); + TextRange expectedRange = expression.getTextRange(); + // we are not allowed to annotate outside the expression so we check if all are inside before attempting to make them + // if they are not inside, we fall back to just annotating the entire expression + Map wrongTypeMap = context.getWrongTypeMap(); + boolean allInRange = wrongTypeMap.keySet().stream().allMatch(psi -> expectedRange.contains(psi.getTextRange())); + if(allInRange) { + return wrongTypeMap.entrySet().stream().map(e -> new ErrorRecord(e.getKey().getTextRange(), e.getValue())).toList(); + }else { + String message = HaxeBundle.message("haxe.semantic.method.parameter.mismatch.wrong.type.members", + context.geWrongTypeMembersString()); + return List.of(new ErrorRecord(expression.getTextRange(), message)); + } } } String message = HaxeBundle.message("haxe.semantic.method.parameter.mismatch", expected.toPresentationString(), got.toPresentationString()); - return new ErrorRecord(expression.getTextRange(), message); + return List.of(new ErrorRecord(expression.getTextRange(), message)); } private static WarningRecord annotateUnableToCompare( ResultHolder problemType, HaxeExpression expression) { diff --git a/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/HaxeSemanticsUtil.java b/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/HaxeSemanticsUtil.java index b029a5bb3..025bddb98 100644 --- a/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/HaxeSemanticsUtil.java +++ b/src/main/java/com/intellij/plugins/haxe/ide/annotator/semantics/HaxeSemanticsUtil.java @@ -40,7 +40,7 @@ public static void check( if(context.hasMissingMembers()) { typeMismatchMissingMembers(holder, erroredElement, context).create(); }else if(context.hasWrongTypeMembers()) { - typeMismatchWrongTypeMembers(holder, erroredElement, context).create(); + addtypeMismatchWrongTypeMembersAnnotations(holder, erroredElement, context); }else { AnnotationBuilder builder = typeMismatch(holder, erroredElement, initType.toStringWithoutConstant(), varType.toStringWithoutConstant()); if (null != initType.getClassType()) { diff --git a/src/main/java/com/intellij/plugins/haxe/model/type/HaxeAssignContext.java b/src/main/java/com/intellij/plugins/haxe/model/type/HaxeAssignContext.java index 936582c62..1d6ec00f6 100644 --- a/src/main/java/com/intellij/plugins/haxe/model/type/HaxeAssignContext.java +++ b/src/main/java/com/intellij/plugins/haxe/model/type/HaxeAssignContext.java @@ -28,12 +28,14 @@ public HaxeAssignContext(@Nullable PsiElement toOrigin, @Nullable PsiElement fro final List missingMembers = new ArrayList<>(); final Map wrongTypeMembers = new HashMap<>(); + final Map wrongTypePsi = new HashMap<>(); public void addMissingMember(String name) { missingMembers.add(name); } - public void addWrongTypeMember(String have, String wants) { + public void addWrongTypeMember(String have, String wants, PsiElement psi) { wrongTypeMembers.put(have, wants); + wrongTypePsi.put(psi, "have '" + have + "' wants '" + wants + "'"); } public boolean hasMissingMembers() { @@ -43,12 +45,18 @@ public boolean hasWrongTypeMembers() { return !wrongTypeMembers.isEmpty(); } + // TODO use map to generate errors + public Map getWrongTypeMap(){ + return wrongTypePsi; + } + public String getMissingMembersString() { return Strings.join(getMissingMembers(), ", "); } public String geWrongTypeMembersString() { return getWrongTypeMembers().entrySet().stream() - .map(entry -> "have '" + entry.getKey() + "' wants '" + entry.getValue() + "'") + //TODO bundle + .map(entry -> "Incompatible type: have '" + entry.getKey() + "' wants '" + entry.getValue() + "'") .collect(Collectors.joining(", ")); } diff --git a/src/main/java/com/intellij/plugins/haxe/model/type/HaxeGenericResolverUtil.java b/src/main/java/com/intellij/plugins/haxe/model/type/HaxeGenericResolverUtil.java index 7b7a438ce..0f4e430e9 100644 --- a/src/main/java/com/intellij/plugins/haxe/model/type/HaxeGenericResolverUtil.java +++ b/src/main/java/com/intellij/plugins/haxe/model/type/HaxeGenericResolverUtil.java @@ -16,6 +16,7 @@ package com.intellij.plugins.haxe.model.type; import com.intellij.plugins.haxe.lang.psi.*; +import com.intellij.plugins.haxe.lang.psi.impl.HaxeClassWrapperForTypeParameter; import com.intellij.plugins.haxe.model.*; import com.intellij.plugins.haxe.model.type.resolver.ResolveSource; import com.intellij.plugins.haxe.util.HaxeResolveUtil; @@ -273,6 +274,9 @@ public static HaxeGenericResolver createInheritedClassResolver(HaxeClass inherit } private static boolean findClassHierarchy(HaxeClass from, HaxeClass to, List path) { + // stop if "from" is typeParameter + if(from instanceof HaxeClassWrapperForTypeParameter)return false; + HaxeClassModel fromModel = from.getModel(); if (fromModel.isTypedef()) { SpecificHaxeClassReference reference = fromModel.getUnderlyingClassReference(new HaxeGenericResolver()); diff --git a/src/main/java/com/intellij/plugins/haxe/model/type/HaxeTypeCompatible.java b/src/main/java/com/intellij/plugins/haxe/model/type/HaxeTypeCompatible.java index a993bcc57..f0154b4bb 100644 --- a/src/main/java/com/intellij/plugins/haxe/model/type/HaxeTypeCompatible.java +++ b/src/main/java/com/intellij/plugins/haxe/model/type/HaxeTypeCompatible.java @@ -577,7 +577,9 @@ private static boolean checkStructInitConstructor(SpecificHaxeClassReference to, if(!toType.canAssign(fromType)) { allMacteches = false; if (context != null) { - context.addWrongTypeMember(fromMemberModel.getPresentableText(null), parameter.getPresentableText(toResolver)); + context.addWrongTypeMember(fromMemberModel.getPresentableText(null), + parameter.getPresentableText(toResolver), + fromMemberModel.getNamePsi()); } } } else { @@ -640,10 +642,41 @@ private static boolean containsAllMembers(SpecificHaxeClassReference to, Specifi boolean ignored = false; if (toMember instanceof HaxeMethodModel methodModel){ if(!isStruct) { + // check for declarations in types memberExists = fromMembers.stream().filter(model -> model instanceof HaxeMethodModel) .map(model -> (HaxeMethodModel)model) .filter(mm -> methodModel.getParameters().size() == mm.getParameters().size()) .anyMatch(model -> model.getNamePsi().textMatches(name)); + if (checkAnonymousMemberTypes) { + if (!memberExists) { + Optional first = fromMembers.stream().filter(model -> model.getNamePsi().getIdentifier().textMatches(name)).findFirst(); + if (first.isPresent()) { + memberExists = true; + if (checkAnonymousMemberTypes) { + HaxeBaseMemberModel fromMember = first.get(); + if (context != null && context.getFromOrigin() != null) { + SpecificFunctionReference toType = methodModel.getFunctionType(toResolver); + + ResultHolder fromType = containsMembersRecursionGuard.computePreventingRecursion(context.getFromOrigin(), false, () -> + fromMember.getResultType(isObjectLiteral ? toResolver : fromResolver) + ); + + if(toType == null || fromType == null) { + log.warn("Unable to evaluate fieldType compatibility"); + return true; + } + + if (!toType.canAssign(fromType)) { + String fromText = fromMember.getPresentableText(null, isObjectLiteral ? toResolver : fromResolver); + String toText = toMember.getPresentableText(null, toResolver); + context.addWrongTypeMember(fromText, toText, fromMember.getBasePsi()); + allMembersMatches = false; + } + } + } + } + } + } }else { // ignore methods in @:structInit classes ignored = true; @@ -675,7 +708,7 @@ private static boolean containsAllMembers(SpecificHaxeClassReference to, Specifi if (!toType.canAssign(fromType)) { String fromText = fromMember.getPresentableText(null, isObjectLiteral ? toResolver : fromResolver); String toText = toMember.getPresentableText(null, toResolver); - context.addWrongTypeMember(fromText, toText); + context.addWrongTypeMember(fromText, toText, fromMember.getBasePsi()); allMembersMatches = false; } } diff --git a/src/test/resources/testData/annotation.semantic/AssignAnonymousType.hx b/src/test/resources/testData/annotation.semantic/AssignAnonymousType.hx index 435bc77b8..21d74ca57 100644 --- a/src/test/resources/testData/annotation.semantic/AssignAnonymousType.hx +++ b/src/test/resources/testData/annotation.semantic/AssignAnonymousType.hx @@ -29,13 +29,13 @@ class Test { typeA = {a:"1"}; //WRONG: incorrect type for "b" - typeA = {a:"1", b:"Str"}; + typeA = {a:"1", b:"Str"}; //WRONG missing members parameterTestB(typeA); //WRONG: incorrect types - parameterTestC(typeZ); + parameterTestC(typeZ); //WRONG: incorrect types - parameterTestC({x:"i", y:"j"}); + parameterTestC({x:"i", y:"j"}); } diff --git a/src/test/resources/testData/annotation.semantic/AssignStructInit.hx b/src/test/resources/testData/annotation.semantic/AssignStructInit.hx index 0a1d0375d..d9b7a44cf 100644 --- a/src/test/resources/testData/annotation.semantic/AssignStructInit.hx +++ b/src/test/resources/testData/annotation.semantic/AssignStructInit.hx @@ -14,7 +14,7 @@ class Test { // CORRECT (TypeParameter) var typeParamA:MyTypeParamStruct = {valueA:"name", valueB:30}; // WRONG (TypeParameter) - var typeParamB:MyTypeParamStruct = {valueA:"name", valueB:30}; + var typeParamB:MyTypeParamStruct = {valueA:"name", valueB:30}; //CORRECT (constructor) var NormalConstructor:ConstrcutorStruct = {name:"str", value:1.0, type: ValueA}; @@ -22,8 +22,8 @@ class Test { var usingDefaults:ConstrcutorStruct = {value:1.0, name:"str"}; // WRONG (constructor) - var wrongType:ConstrcutorStruct = {value:"1.0", name:"str"}; - var wrongTypeTypeParameter:ConstrcutorStruct = {value:1.0, name:1}; + var wrongType:ConstrcutorStruct = {value:"1.0", name:"str"}; + var wrongTypeTypeParameter:ConstrcutorStruct = {value:1.0, name:1}; var missingField:ConstrcutorStruct = { name:"str"}; }