Skip to content

Commit

Permalink
better checks and error messages for struct member types (+ fix stack…
Browse files Browse the repository at this point in the history
…overflow issue after typeParameter hierarchy fix)
  • Loading branch information
m0rkeulv committed Sep 3, 2024
1 parent 0352de4 commit 3b999cd
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<PsiElement, String> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -839,25 +839,34 @@ private static HaxeGenericResolver findTypeParametersToInherit(SpecificTypeRefer
}


private static ErrorRecord annotateTypeMismatch(ResultHolder expected, ResultHolder got, HaxeExpression expression,
private static List<ErrorRecord> 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<PsiElement, String> 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ public HaxeAssignContext(@Nullable PsiElement toOrigin, @Nullable PsiElement fro

final List<String> missingMembers = new ArrayList<>();
final Map<String, String> wrongTypeMembers = new HashMap<>();
final Map<PsiElement, String> 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() {
Expand All @@ -43,12 +45,18 @@ public boolean hasWrongTypeMembers() {
return !wrongTypeMembers.isEmpty();
}

// TODO use map to generate errors
public Map<PsiElement, String> 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(", "));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -273,6 +274,9 @@ public static HaxeGenericResolver createInheritedClassResolver(HaxeClass inherit
}

private static boolean findClassHierarchy(HaxeClass from, HaxeClass to, List<SpecificHaxeClassReference> 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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<HaxeBaseMemberModel> 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;
Expand Down Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ class Test {
typeA = <error descr="Incompatible type: missing member(s) b:Float">{a:"1"}</error>;

//WRONG: incorrect type for "b"
typeA = <error descr="Incompatible type: Incompatible member type(s) (have 'b:String' wants 'b:Float')">{a:"1", b:"Str"}</error>;
typeA = {a:"1", <error descr="have 'b:String' wants 'b:Float'">b:"Str"</error>};
//WRONG missing members
parameterTestB(<error descr="Type mismatch, missing member(s) (x:Int, y:Float)">typeA</error>);
//WRONG: incorrect types
parameterTestC(<error descr="Type mismatch, Incompatible member type(s) (have 'x:String' wants 'x:Int', have 'y:String' wants 'y:Float')">typeZ</error>);
parameterTestC(<error descr="Type mismatch, Incompatible member type(s) (Incompatible type: have 'x:String' wants 'x:Int', Incompatible type: have 'y:String' wants 'y:Float')">typeZ</error>);
//WRONG: incorrect types
parameterTestC(<error descr="Type mismatch, Incompatible member type(s) (have 'x:String' wants 'x:Int', have 'y:String' wants 'y:Float')">{x:"i", y:"j"}</error>);
parameterTestC({<error descr="have 'x:String' wants 'x:Int'">x:"i"</error>, <error descr="have 'y:String' wants 'y:Float'">y:"j"</error>});

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ class Test {
// CORRECT (TypeParameter)
var typeParamA:MyTypeParamStruct<String> = {valueA:"name", valueB:30};
// WRONG (TypeParameter)
var <error descr="Incompatible type: Incompatible member type(s) (have 'valueA:String' wants 'valueA:Int')">typeParamB:MyTypeParamStruct<Int> = {valueA:"name", valueB:30}</error>;
var typeParamB:MyTypeParamStruct<Int> = {<error descr="have 'valueA:String' wants 'valueA:Int'">valueA:"name"</error>, valueB:30};

//CORRECT (constructor)
var NormalConstructor:ConstrcutorStruct<String> = {name:"str", value:1.0, type: ValueA};
var diffrentOrder:ConstrcutorStruct<String> = {value:1.0, type: ValueA, name:"str"};
var usingDefaults:ConstrcutorStruct<String> = {value:1.0, name:"str"};

// WRONG (constructor)
var <error descr="Incompatible type: Incompatible member type(s) (have 'value:String' wants 'value:Float')">wrongType:ConstrcutorStruct<String> = {value:"1.0", name:"str"}</error>;
var <error descr="Incompatible type: Incompatible member type(s) (have 'name:Int' wants 'name:String')">wrongTypeTypeParameter:ConstrcutorStruct<String> = {value:1.0, name:1}</error>;
var wrongType:ConstrcutorStruct<String> = {<error descr="have 'value:String' wants 'value:Float'"><error descr="have 'value:String' wants 'value:Float'">value</error>:"1.0"</error>, name:"str"};
var wrongTypeTypeParameter:ConstrcutorStruct<String> = {value:1.0, <error descr="have 'name:Int' wants 'name:String'"><error descr="have 'name:Int' wants 'name:String'">name</error>:1</error>};
var <error descr="Incompatible type: missing member(s) value">missingField:ConstrcutorStruct<String> = { name:"str"}</error>;

}
Expand Down

0 comments on commit 3b999cd

Please sign in to comment.