From 61c9277c170eb906b07d466caa2121cbb4d42a60 Mon Sep 17 00:00:00 2001 From: Georgii Ustinov Date: Mon, 1 Dec 2025 13:29:19 +0100 Subject: [PATCH] [Java. Inspections] IDEA-381448 better type presentation when nullability conflict occurs GitOrigin-RevId: 353527faa14b194d45ee50a2a637ceba11b4e175 --- .../messages/JavaAnalysisBundle.properties | 3 +- .../nullable/NullableStuffInspectionBase.java | 2 +- .../nullable/NullableStuffInspectionUtil.java | 65 ++++++++++++------- .../util/JavaTypeNullabilityUtil.java | 35 ++++++---- .../ReturnIncompatibilitiesWithGeneric.java | 24 +++---- 5 files changed, 79 insertions(+), 50 deletions(-) diff --git a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties index b73d58745138..83fd64de4680 100644 --- a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties +++ b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties @@ -140,7 +140,8 @@ returning.a.class.with.notnull.arguments=\ \ Returning a class with not-null type arguments when a class with nullable type arguments is expected{0}\ -returning.a.type.nullability.conflict.message=Return type: +expected.type.nullability.conflict.message=Expected type: +actual.type.nullability.conflict.message=Actual type: conflicting.nullability.annotations=Conflicting nullability annotations nullable.stuff.error.overriding.nullable.with.notnull=Overriding a collection of nullable elements with a collection of non-null elements nullable.stuff.error.overriding.notnull.with.nullable=Overriding a collection of non-null elements with a collection of nullable elements diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java index 66e8a4b567c4..a9849c3cf496 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java @@ -542,7 +542,7 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection reportProblem(holder, returnValue, LocalQuickFix.EMPTY_ARRAY, messageKey, new Object[]{""}, - messageKey, new Object[]{NullableStuffInspectionUtil.getTypePresentationInNullabilityConflict(context)}); + messageKey, new Object[]{NullableStuffInspectionUtil.getNullabilityConflictPresentation(context)}); } private void checkCollectionNullityOnAssignment(@NotNull PsiElement errorElement, diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionUtil.java b/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionUtil.java index c1c3906c9a26..02e420fd57a3 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionUtil.java @@ -16,38 +16,47 @@ import java.util.Collections; import java.util.List; final class NullableStuffInspectionUtil { - static @NotNull @NlsSafe String getTypePresentationInNullabilityConflict(@NotNull JavaTypeNullabilityUtil.NullabilityConflictContext context) { - PsiElement place = context.getPlace(); - if (place == null) return ""; - int dimensionsInArray = (context.type() instanceof PsiArrayType type) ? type.getArrayDimensions() : 0; + static @NotNull @NlsSafe String getNullabilityConflictPresentation(@NotNull JavaTypeNullabilityUtil.NullabilityConflictContext context) { + HtmlChunk expectedChunk = getSideChunk(context, JavaTypeNullabilityUtil.Side.EXPECTED); + HtmlChunk actualChunk = getSideChunk(context, JavaTypeNullabilityUtil.Side.ACTUAL); + if (expectedChunk.isEmpty() || actualChunk.isEmpty()) return ""; + return HtmlChunk.tag("table") + .children(expectedChunk, actualChunk) + .toString(); + } + + private static @NotNull HtmlChunk getSideChunk(@NotNull JavaTypeNullabilityUtil.NullabilityConflictContext context, + @NotNull JavaTypeNullabilityUtil.Side side) { + PsiElement place = context.getPlace(side); + if (place == null) return HtmlChunk.empty(); + int dimensionsInArray = (context.getType(side) instanceof PsiArrayType type) ? type.getArrayDimensions() : 0; PsiTypeElement topmostType = PsiTreeUtil.getTopmostParentOfType(place, PsiTypeElement.class); - if (topmostType == null) return ""; + if (topmostType == null) return HtmlChunk.empty(); PsiTypeElement target = PsiTreeUtil.getParentOfType(place, PsiTypeElement.class, false); - if (target == null) return ""; + if (target == null) return HtmlChunk.empty(); PsiType outerType = topmostType.getType(); if (target == topmostType) { // Presentation is not handling with the whole type - return ""; + return HtmlChunk.empty(); } List path = computeTypeArgumentPath(topmostType, target, dimensionsInArray); - if (path == null) return ""; + if (path == null) return HtmlChunk.empty(); Context presentationContext = getPresentationContext(outerType, path, outerType instanceof PsiArrayType); String typeText = presentationContext.sb.toString(); - String annotationText = getAnnotationText(context); - if (presentationContext.position == null || annotationText == null) return ""; - HtmlChunk result = generateHtmlChunk(typeText, "@" + annotationText, presentationContext.position); - - return result.toString(); + String annotationText = getAnnotationText(context, side); + if (presentationContext.position == null || annotationText == null) return HtmlChunk.empty(); + HtmlChunk result = generateHtmlChunk(typeText, "@" + annotationText, side, presentationContext.position); + return result; } - - private static @Nullable String getAnnotationText(@NotNull JavaTypeNullabilityUtil.NullabilityConflictContext context) { - PsiAnnotation annotation = context.getAnnotation(); + + private static @Nullable String getAnnotationText(@NotNull JavaTypeNullabilityUtil.NullabilityConflictContext context, @NotNull JavaTypeNullabilityUtil.Side side) { + PsiAnnotation annotation = context.getAnnotation(side); if (annotation == null) return null; PsiJavaCodeReferenceElement ref = annotation.getNameReferenceElement(); if (ref == null) return null; @@ -56,18 +65,24 @@ final class NullableStuffInspectionUtil { private static @NotNull HtmlChunk generateHtmlChunk(@NlsSafe String text, @NlsSafe String annotationText, + @NotNull JavaTypeNullabilityUtil.Side side, int position) { - HtmlChunk result = HtmlChunk - .p() + return HtmlChunk.tag("tr") .children( - HtmlChunk.text(JavaAnalysisBundle.message("returning.a.type.nullability.conflict.message")), - HtmlChunk.text(" "), - HtmlChunk.text(text.substring(0, position)), - HtmlChunk.tag("b").addText(annotationText), - HtmlChunk.text(" "), - HtmlChunk.text(text.substring(position)) + HtmlChunk.tag("td") + .addText( + JavaAnalysisBundle.message( + side == JavaTypeNullabilityUtil.Side.EXPECTED ? "expected.type.nullability.conflict.message" + : "actual.type.nullability.conflict.message" + ) + ), + HtmlChunk.tag("td").children( + HtmlChunk.text(text.substring(0, position)), + HtmlChunk.tag("b").addText(annotationText), + HtmlChunk.text(" "), + HtmlChunk.text(text.substring(position)) + ) ); - return result; } private static @Nullable List<@NotNull Integer> computeTypeArgumentPath(@NotNull PsiTypeElement top, @NotNull PsiTypeElement target, int firstArrayDepth) { diff --git a/java/java-psi-api/src/com/intellij/util/JavaTypeNullabilityUtil.java b/java/java-psi-api/src/com/intellij/util/JavaTypeNullabilityUtil.java index efdf39f3a461..a33cb060f493 100644 --- a/java/java-psi-api/src/com/intellij/util/JavaTypeNullabilityUtil.java +++ b/java/java-psi-api/src/com/intellij/util/JavaTypeNullabilityUtil.java @@ -240,29 +240,30 @@ public final class JavaTypeNullabilityUtil { Nullability rightNullability = rightTypeNullability.nullability(); if (leftNullability == Nullability.NOT_NULL && rightNullability == Nullability.NULLABLE) { - return new NullabilityConflictContext(NullabilityConflict.NULL_TO_NOT_NULL, rightType); + return new NullabilityConflictContext(NullabilityConflict.NULL_TO_NOT_NULL, leftType, rightType); } // It is not possible to have NOT_NULL_TO_NULL conflict when left type is wildcard with upper bound, // e.g., this assignment is legal {@code List = List<@NotNull String>} else if (leftNullability == Nullability.NULLABLE && rightNullability == Nullability.NOT_NULL && !GenericsUtil.isWildcardWithExtendsBound(leftType)) { - return new NullabilityConflictContext(NullabilityConflict.NOT_NULL_TO_NULL, rightType); + return new NullabilityConflictContext(NullabilityConflict.NOT_NULL_TO_NULL, leftType, rightType); } return NullabilityConflictContext.UNKNOWN; } - /** * Holds information about the nullability conflict that might be used to provide more descriptive error messages. */ public static class NullabilityConflictContext { private final @NotNull NullabilityConflict nullabilityConflict; - private final @Nullable PsiType type; + private final @Nullable PsiType expectedType; + private final @Nullable PsiType actualType; - public static final NullabilityConflictContext UNKNOWN = new NullabilityConflictContext(NullabilityConflict.UNKNOWN, null); + public static final NullabilityConflictContext UNKNOWN = new NullabilityConflictContext(NullabilityConflict.UNKNOWN, null, null); - public NullabilityConflictContext(@NotNull NullabilityConflict nullabilityConflict, @Nullable PsiType type) { - this.type = type; + public NullabilityConflictContext(@NotNull NullabilityConflict nullabilityConflict, @Nullable PsiType expectedType, @Nullable PsiType actualType) { this.nullabilityConflict = nullabilityConflict; + this.expectedType = expectedType; + this.actualType = actualType; } /** @@ -276,14 +277,16 @@ public final class JavaTypeNullabilityUtil { /** * @return part of the actual {@code PsiType} in which the conflict is occurred. */ - public PsiType type() { - return type; + public PsiType getType(@NotNull Side side) { + if (side == Side.EXPECTED) return expectedType; + return actualType; } /** * @return type argument or array type in which the conflict is occurred. */ - public @Nullable PsiElement getPlace() { + public @Nullable PsiElement getPlace(@NotNull Side side) { + PsiType type = getType(side); return getPlace(type); } @@ -291,7 +294,8 @@ public final class JavaTypeNullabilityUtil { /** * @return nullability annotation that produces the conflict. */ - public @Nullable PsiAnnotation getAnnotation() { + public @Nullable PsiAnnotation getAnnotation(@NotNull Side side) { + PsiType type = getType(side); if (type == null) return null; TypeNullability nullability = type.getNullability(); NullabilityAnnotationInfo info = nullability.toNullabilityAnnotationInfo(); @@ -316,6 +320,15 @@ public final class JavaTypeNullabilityUtil { } } + /** + * Represents the side of nullability conflict. + * @see NullabilityConflictContext + */ + public enum Side { + EXPECTED, + ACTUAL, + } + /** * Represents a conflict in nullability between 2 types */ diff --git a/java/java-tests/testData/inspection/nullableProblems/ReturnIncompatibilitiesWithGeneric.java b/java/java-tests/testData/inspection/nullableProblems/ReturnIncompatibilitiesWithGeneric.java index d94919ecab6e..a3f5e8fea319 100644 --- a/java/java-tests/testData/inspection/nullableProblems/ReturnIncompatibilitiesWithGeneric.java +++ b/java/java-tests/testData/inspection/nullableProblems/ReturnIncompatibilitiesWithGeneric.java @@ -7,15 +7,15 @@ import java.util.Collection; class B { B<@NotNull String> simpleNullableToNotNull(B<@Nullable String> arg) { - return arg; + return arg; } B<@Nullable String> simpleNotNullToNullable(B<@NotNull String> arg) { - return arg; + return arg; } B> nested(B> arg) { - return arg; + return arg; } B extendsWildcardNullable(B<@NotNull Object> arg) { @@ -23,7 +23,7 @@ class B { } B extendsWildcardNotNull(B<@Nullable String> arg) { - return arg; + return arg; } B SupperWildcard(B<@Nullable Object> arg) { @@ -31,7 +31,7 @@ class B { } B extendsWildcardBothNotNull(B arg) { - return arg; + return arg; } B extendsWildcardBothNullable(B arg) { @@ -43,28 +43,28 @@ class B { } B> nestedWithWildcard(B> arg) { - return arg; + return arg; } Map, List<@Nullable String>> checkIsPerformedIfSecondTypeArgumentIsTheSame(Map, List<@NotNull String>> arg) { - return arg; + return arg; } B<@NotNull String>[] array(B<@Nullable String>[] arg) { - return arg; + return arg; } Object[] @NotNull [] nullabilityInNestedArray(Object[] @Nullable [] arg) { - return arg; + return arg; } B<@NotNull String>[][] multiDimensionalArray(B<@Nullable String>[][] arg) { - return arg; + return arg; } static class C { C secondArgument(C arg) { - return arg; + return arg; } } @@ -101,7 +101,7 @@ class B { @NullMarked static class ReturnWithNullMarked { static List<@Nullable String> f(List arg) { - return arg; + return arg; } } } \ No newline at end of file