From 3c9c050b461a9809fafee18048ba309087005e3a Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Fri, 9 Jun 2017 11:08:23 +0300 Subject: [PATCH] fixed yellow code in PersistentFSImpl where VEvent was casted to subclass for the method different not-nullness --- .../intellij/psi/util/RedundantCastUtil.java | 49 +++++++++++++------ .../generics/DifferentNullness/expected.xml | 9 ++++ .../generics/DifferentNullness/src/A.java | 32 ++++++++++++ .../codeInspection/RedundantCast15Test.java | 1 + 4 files changed, 76 insertions(+), 15 deletions(-) create mode 100644 java/java-tests/testData/inspection/redundantCast/generics/DifferentNullness/expected.xml create mode 100644 java/java-tests/testData/inspection/redundantCast/generics/DifferentNullness/src/A.java diff --git a/java/java-psi-api/src/com/intellij/psi/util/RedundantCastUtil.java b/java/java-psi-api/src/com/intellij/psi/util/RedundantCastUtil.java index 937dd8485cfc..3c3ed346ff3b 100644 --- a/java/java-psi-api/src/com/intellij/psi/util/RedundantCastUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/util/RedundantCastUtil.java @@ -16,6 +16,7 @@ package com.intellij.psi.util; import com.intellij.codeInsight.AnnotationUtil; +import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Comparing; @@ -44,11 +45,12 @@ public class RedundantCastUtil { private RedundantCastUtil() { } @NotNull - public static List getRedundantCastsInside(PsiElement where) { + public static List getRedundantCastsInside(@NotNull PsiElement where) { MyCollectingVisitor visitor = new MyCollectingVisitor(); if (where instanceof PsiEnumConstant) { where.accept(visitor); - } else { + } + else { where.acceptChildren(visitor); } return new ArrayList<>(visitor.myFoundCasts); @@ -242,7 +244,8 @@ public class RedundantCastUtil { } } - @Override public void visitMethodCallExpression(PsiMethodCallExpression expression) { + @Override + public void visitMethodCallExpression(PsiMethodCallExpression expression) { processCall(expression); checkForVirtual(expression); @@ -269,8 +272,8 @@ public class RedundantCastUtil { if (targetMethod.hasModifierProperty(PsiModifier.STATIC)) return; try { - PsiManager manager = methodExpr.getManager(); - PsiElementFactory factory = JavaPsiFacade.getInstance(manager.getProject()).getElementFactory(); + Project project = methodExpr.getProject(); + PsiElementFactory factory = JavaPsiFacade.getInstance(project).getElementFactory(); final PsiExpression expressionFromText = factory.createExpressionFromText(methodCall.getText(), methodCall); if (!(expressionFromText instanceof PsiMethodCallExpression)) return; @@ -282,23 +285,38 @@ public class RedundantCastUtil { final JavaResolveResult newResult = newCall.getMethodExpression().advancedResolve(false); if (!newResult.isValidResult()) return; final PsiMethod newTargetMethod = (PsiMethod)newResult.getElement(); - PsiType newReturnType = newCall.getType(), oldReturnType = methodCall.getType(); + PsiType newReturnType = newCall.getType(); + PsiType oldReturnType = methodCall.getType(); if (newReturnType instanceof PsiCapturedWildcardType && oldReturnType instanceof PsiCapturedWildcardType) { newReturnType = ((PsiCapturedWildcardType)newReturnType).getUpperBound(); oldReturnType = ((PsiCapturedWildcardType)oldReturnType).getUpperBound(); } - if (Comparing.equal(newReturnType, oldReturnType)) { - if (Comparing.equal(newTargetMethod, targetMethod) || - (newTargetMethod.getSignature(newResult.getSubstitutor()).equals(targetMethod.getSignature(resolveResult.getSubstitutor())) && - !(newTargetMethod.isDeprecated() && !targetMethod.isDeprecated()) && // see SCR11555, SCR14559 - areThrownExceptionsCompatible(targetMethod, newTargetMethod))) { - addToResults(typeCast); - } + if (Comparing.equal(newReturnType, oldReturnType) && + (Comparing.equal(newTargetMethod, targetMethod) || + newTargetMethod.getSignature(newResult.getSubstitutor()).equals(targetMethod.getSignature(resolveResult.getSubstitutor())) && + !(newTargetMethod.isDeprecated() && !targetMethod.isDeprecated()) && + // see SCR11555, SCR14559 + areThrownExceptionsCompatible(targetMethod, newTargetMethod) && + areNullnessCompatible(project, targetMethod, newTargetMethod))) { + addToResults(typeCast); } } catch (IncorrectOperationException ignore) { } } + private static boolean areNullnessCompatible(Project project, + final PsiMethod oldTargetMethod, + final PsiMethod newTargetMethod) { + // the cast may be for the @NotNull which newTargetMethod has whereas the oldTargetMethod doesn't + NullableNotNullManager nnm = NullableNotNullManager.getInstance(project); + boolean oldNotNull = nnm.isNotNull(oldTargetMethod, true); + boolean newNotNull = nnm.isNotNull(newTargetMethod, true); + if (oldNotNull != newNotNull) return false; + boolean oldNullable = nnm.isNullable(oldTargetMethod, true); + boolean newNullable = nnm.isNullable(newTargetMethod, true); + return oldNullable == newNullable; + } + private static boolean areThrownExceptionsCompatible(final PsiMethod targetMethod, final PsiMethod newTargetMethod) { final PsiClassType[] oldThrowsTypes = targetMethod.getThrowsList().getReferencedTypes(); final PsiClassType[] newThrowsTypes = newTargetMethod.getThrowsList().getReferencedTypes(); @@ -820,10 +838,11 @@ public class RedundantCastUtil { otherOperand = firstOperand; firstOperand = temp; } - if (firstOperand != null && otherOperand != null && wrapperCastChangeSemantics(firstOperand, otherOperand, operand)) { + if (otherOperand != null && wrapperCastChangeSemantics(firstOperand, otherOperand, operand)) { return true; } - } else if (parent instanceof PsiConditionalExpression) { + } + else if (parent instanceof PsiConditionalExpression) { if (opType instanceof PsiPrimitiveType && !(((PsiConditionalExpression)parent).getType() instanceof PsiPrimitiveType)) { if (PsiPrimitiveType.getUnboxedType(PsiTypesUtil.getExpectedTypeByParent(parent)) != null) { return true; diff --git a/java/java-tests/testData/inspection/redundantCast/generics/DifferentNullness/expected.xml b/java/java-tests/testData/inspection/redundantCast/generics/DifferentNullness/expected.xml new file mode 100644 index 000000000000..da9b38bb579c --- /dev/null +++ b/java/java-tests/testData/inspection/redundantCast/generics/DifferentNullness/expected.xml @@ -0,0 +1,9 @@ + + + + A.java + 5 + Redundant type cast + Casting <code>a</code> to <code>AA</code> is redundant + + diff --git a/java/java-tests/testData/inspection/redundantCast/generics/DifferentNullness/src/A.java b/java/java-tests/testData/inspection/redundantCast/generics/DifferentNullness/src/A.java new file mode 100644 index 000000000000..0a261671c358 --- /dev/null +++ b/java/java-tests/testData/inspection/redundantCast/generics/DifferentNullness/src/A.java @@ -0,0 +1,32 @@ +import org.jetbrains.annotations.*; + +class A { + static String doit(A a) { + String d = ((AA)a).danuna(); + String notNull = ((AA)a).doadd(); + return notNull + d; + } + + @Nullable + String doadd() { + return null; + } + + @NotNull + String danuna() { + return ""; + } +} + +class AA extends A { + @NotNull + String doadd() { + return ""; + } + + @Override + @NotNull + String danuna() { + return ""; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast15Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast15Test.java index 2fafe36465ac..7aaa465cc60d 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast15Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast15Test.java @@ -92,4 +92,5 @@ public class RedundantCast15Test extends InspectionTestCase { final LocalInspectionToolWrapper tool = new LocalInspectionToolWrapper(castInspection); doTest("redundantCast/generics/" + getTestName(false), tool, "java 1.5"); } + public void testDifferentNullness() throws Exception { doTest();} } \ No newline at end of file