From 1bc4cda08e5d5d4c00f6dbf3e29e2e6c1daf71a2 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Thu, 7 May 2020 22:46:09 +0200 Subject: [PATCH] java redundant cast: cleanup: visitor finally inverted GitOrigin-RevId: 8b4449e6d9163c24b046aad9065e5ec055d6d62c --- .../intellij/psi/util/RedundantCastUtil.java | 183 ++++++++++-------- .../RedundantCastInSwitchBranch.java | 8 + .../redundantCast/ConditionalNoType.java | 2 +- .../redundantCast/lambda/MiscStatements.java | 24 +++ .../codeInspection/RedundantCast18Test.java | 1 + 5 files changed, 131 insertions(+), 87 deletions(-) create mode 100644 java/java-tests/testData/inspection/redundantCast/lambda/MiscStatements.java diff --git a/java/java-analysis-impl/src/com/intellij/psi/util/RedundantCastUtil.java b/java/java-analysis-impl/src/com/intellij/psi/util/RedundantCastUtil.java index 40452ac7ce44..b10bda4b257d 100644 --- a/java/java-analysis-impl/src/com/intellij/psi/util/RedundantCastUtil.java +++ b/java/java-analysis-impl/src/com/intellij/psi/util/RedundantCastUtil.java @@ -34,11 +34,11 @@ public class RedundantCastUtil { @NotNull public static List getRedundantCastsInside(@NotNull PsiElement where) { MyCollectingVisitor visitor = new MyCollectingVisitor(); - if (where instanceof PsiField) { - where.accept(visitor); + if (where instanceof PsiClass) { + where.acceptChildren(visitor); } else { - where.acceptChildren(visitor); + where.accept(visitor); } return new ArrayList<>(visitor.myFoundCasts); } @@ -69,11 +69,6 @@ public class RedundantCastUtil { // avoid multiple visit } - @Override - public void visitMethod(PsiMethod method) { - // avoid multiple visit - } - @Override protected void registerCast(@NotNull PsiTypeCastExpression typeCast) { myFoundCasts.add(typeCast); @@ -98,6 +93,16 @@ public class RedundantCastUtil { } } + private void addIfNarrowing(PsiExpression expression, PsiType expectedTypeByParent) { + expression = deparenthesizeExpression(expression); + if (expression instanceof PsiTypeCastExpression) { + PsiExpression operand = getInnerMostOperand(expression); + if (operand != null) { + addIfNarrowing((PsiTypeCastExpression)expression, operand.getType(), expectedTypeByParent); + } + } + } + protected void registerCast(@NotNull PsiTypeCastExpression typeCast) { foundRedundantCast = typeCast; stopWalking(); @@ -385,27 +390,7 @@ public class RedundantCastUtil { !(newResult instanceof MethodCandidateInfo && ((MethodCandidateInfo)newResult).getInferenceErrorMessage() != null) && newResult.getSubstitutor().equals(oldResult.getSubstitutor())) { PsiExpression newArg = PsiUtil.deparenthesizeExpression(newArgs[i]); - if (newArg instanceof PsiConditionalExpression && PsiPolyExpressionUtil.isPolyExpression(newArg)) { - PsiType targetType = newArg.getType(); - LOG.assertTrue(targetType != null); - //target type is detected by method call - //check that both sides are fine with that - PsiExpression thenExpression = ((PsiConditionalExpression)newArg).getThenExpression(); - PsiType thenType = thenExpression != null ? thenExpression.getType() : null; - PsiExpression elseExpression = ((PsiConditionalExpression)newArg).getElseExpression(); - PsiType elseType = elseExpression != null ? elseExpression.getType() : null; - if (thenType != null && targetType.isAssignableFrom(thenType) && - elseType != null && targetType.isAssignableFrom(elseType)) { - addToResults(cast); - } - else { - newArg.replace(arg); - } - } - else if (!(newArg instanceof PsiFunctionalExpression)) { - addToResults(cast); - } - else { + if (newArg instanceof PsiFunctionalExpression) { final boolean varargs = newResult instanceof MethodCandidateInfo && ((MethodCandidateInfo)newResult).isVarargs(); final PsiType parameterType = PsiTypesUtil.getParameterType(parameters, i, varargs); PsiType newArgType = newResult.getSubstitutor().substitute(parameterType); @@ -421,6 +406,9 @@ public class RedundantCastUtil { newArg.replace(arg); } } + else { + addToResults(cast); + } } else { newArgs[i].replace(arg); @@ -553,9 +541,6 @@ public class RedundantCastUtil { addToResults(innerCast); } } - else { - processAlreadyHasTypeCast(typeCast); - } super.visitTypeCastExpression(typeCast); } @@ -694,17 +679,23 @@ public class RedundantCastUtil { @Override public void visitSwitchStatement(PsiSwitchStatement statement) { - visitSwitchBlock(statement); + visitSwitchBlockSelector(statement); super.visitSwitchStatement(statement); } @Override public void visitSwitchExpression(PsiSwitchExpression expression) { - visitSwitchBlock(expression); + visitSwitchBlockSelector(expression); + + PsiType expectedTypeByParent = PsiTypesUtil.getExpectedTypeByParent(expression); + for (PsiExpression resultExpression : PsiUtil.getSwitchResultExpressions(expression)) { + addIfNarrowing(resultExpression, expectedTypeByParent); + } + super.visitSwitchExpression(expression); } - private void visitSwitchBlock(PsiSwitchBlock expression) { + private void visitSwitchBlockSelector(PsiSwitchBlock expression) { PsiExpression switchVariable = deparenthesizeExpression(expression.getExpression()); if (switchVariable instanceof PsiTypeCastExpression) { PsiExpression operand = ((PsiTypeCastExpression)switchVariable).getOperand(); @@ -721,58 +712,84 @@ public class RedundantCastUtil { } } + @Override + public void visitAssertStatement(PsiAssertStatement statement) { + addIfNarrowing(statement.getAssertCondition(), PsiType.BOOLEAN); + addIfNarrowing(statement.getAssertDescription(), PsiType.getJavaLangString(statement.getManager(), statement.getResolveScope())); + super.visitAssertStatement(statement); + } + - private void processAlreadyHasTypeCast(PsiTypeCastExpression typeCast){ - PsiElement parent = PsiUtil.skipParenthesizedExprUp(typeCast.getParent()); - if (parent instanceof PsiExpressionList) return; // do not replace in arg lists - should be handled by parent - if (parent instanceof PsiReturnStatement) return; - if (parent instanceof PsiTypeCastExpression) return; - if (parent instanceof PsiPolyadicExpression) return; - if (parent instanceof PsiForeachStatement) return; - if (parent instanceof PsiInstanceOfExpression) return; - if (parent instanceof PsiThrowStatement) return; - if (parent instanceof PsiSynchronizedStatement) return; - if (parent instanceof PsiLambdaExpression) return; - if (parent instanceof PsiSwitchBlock) return; - if (parent instanceof PsiArrayAccessExpression) return; - if (parent instanceof PsiReferenceExpression) return; - if (parent instanceof PsiAssignmentExpression) return; - if (parent instanceof PsiVariable) return; - if (parent instanceof PsiArrayInitializerExpression) return; - if (parent instanceof PsiConditionalExpression) return; + @Override + public void visitYieldStatement(PsiYieldStatement statement) { + PsiSwitchExpression switchExpression = statement.findEnclosingExpression(); + PsiType expectedTypeByParent = switchExpression != null ? PsiTypesUtil.getExpectedTypeByParent(switchExpression) : null; + addIfNarrowing(statement.getExpression(), expectedTypeByParent); + super.visitYieldStatement(statement); + } + + @Override + public void visitDoWhileStatement(PsiDoWhileStatement statement) { + addIfNarrowing(statement.getCondition(), PsiType.BOOLEAN); + super.visitDoWhileStatement(statement); + } - if (isTypeCastSemantic(typeCast)) return; + @Override + public void visitIfStatement(PsiIfStatement statement) { + addIfNarrowing(statement.getCondition(), PsiType.BOOLEAN); + super.visitIfStatement(statement); + } - PsiTypeElement typeElement = typeCast.getCastType(); - if (typeElement == null) return; - final PsiType castTo = typeElement.getType(); - final PsiExpression operand = deparenthesizeExpression(typeCast.getOperand()); - if (operand == null) return; + @Override + public void visitWhileStatement(PsiWhileStatement statement) { + addIfNarrowing(statement.getCondition(), PsiType.BOOLEAN); + super.visitWhileStatement(statement); + } - final PsiType expectedTypeByParent = PsiTypesUtil.getExpectedTypeByParent(typeCast); - PsiType opType = getOpTypeWithExpected(operand, expectedTypeByParent); + @Override + public void visitResourceExpression(PsiResourceExpression expression) { + addIfNarrowing(expression.getExpression(), null); + super.visitResourceExpression(expression); + } - if (opType == null) return; - - if (operand instanceof PsiFunctionalExpression) { - if (expectedTypeByParent != null) { - if (expectedTypeByParent.equals(castTo)) { - addToResults(typeCast); - return; + @Override + public void visitExpressionStatement(PsiExpressionStatement statement) { + if (!(statement.getParent() instanceof PsiSwitchLabeledRuleStatement)) { + addIfNarrowing(statement.getExpression(), null); + } + super.visitExpressionStatement(statement); + } + + @Override + public void visitNameValuePair(PsiNameValuePair pair) { + PsiAnnotationMemberValue value = pair.getValue(); + if (value instanceof PsiExpression) { + addIfNarrowing((PsiExpression)value, null); + } + super.visitNameValuePair(pair); + } + + @Override + public void visitLambdaExpression(PsiLambdaExpression expression) { + if (!(PsiUtil.skipParenthesizedExprUp(expression.getParent()) instanceof PsiExpressionList)) { + PsiMethod method = LambdaUtil.getFunctionalInterfaceMethod(expression); + if (method != null) { + PsiType returnType = method.getReturnType(); + if (returnType != null) { + List returns = LambdaUtil.getReturnExpressions(expression); + for (PsiExpression aReturn : returns) { + PsiExpression returnInLambda = deparenthesizeExpression(aReturn); + if (returnInLambda instanceof PsiTypeCastExpression) { + PsiExpression operand = getInnerMostOperand(returnInLambda); + if (operand != null && returnType.equals(operand.getType())) { + addToResults((PsiTypeCastExpression)returnInLambda); + } + } + } } - else if (!TypeConversionUtil.isAssignable(castTo, expectedTypeByParent, false)) { - return; - } - } - - if (parent instanceof PsiExpressionStatement && - parent.getParent() instanceof PsiSwitchLabeledRuleStatement && - !castTo.equals(PsiTypesUtil.getExpectedTypeByParent(parent))) { - return; } } - - addIfNarrowing(typeCast, opType, expectedTypeByParent); + super.visitLambdaExpression(expression); } @Override @@ -791,13 +808,7 @@ public class RedundantCastUtil { visitConditional((PsiTypeCastExpression)elseExpression, conditionalExpression, getInnerMostOperand(thenExpression)); } - PsiExpression condition = deparenthesizeExpression(conditionalExpression.getCondition()); - if (condition instanceof PsiTypeCastExpression) { - PsiExpression operand = getInnerMostOperand(((PsiTypeCastExpression)condition).getOperand()); - if (operand != null) { - addIfNarrowing(((PsiTypeCastExpression)condition), operand.getType(), PsiType.BOOLEAN); - } - } + addIfNarrowing(conditionalExpression.getCondition(), PsiType.BOOLEAN); super.visitConditionalExpression(conditionalExpression); } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/RedundantCastInSwitchBranch.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/RedundantCastInSwitchBranch.java index 8300b6a97c0b..9d1b970b4650 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/RedundantCastInSwitchBranch.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/RedundantCastInSwitchBranch.java @@ -5,5 +5,13 @@ class RedundantCast { Object o = switch (matchType) { default -> (Predicate) target -> target == null; }; + Predicate o1 = switch (matchType) { + default -> (Predicate) target -> target == null; + }; + + Predicate o2 = switch (matchType) { + default: + yield (Predicate) target -> target == null; + }; } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantCast/ConditionalNoType.java b/java/java-tests/testData/inspection/redundantCast/ConditionalNoType.java index e74156e50b44..4ac4b68e4933 100644 --- a/java/java-tests/testData/inspection/redundantCast/ConditionalNoType.java +++ b/java/java-tests/testData/inspection/redundantCast/ConditionalNoType.java @@ -3,6 +3,6 @@ class X { void foo(String str) {} void test(Object obj, boolean b) { - foo((Object)(b ? obj : ())); + foo((Object)(b ? obj : ())); } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/redundantCast/lambda/MiscStatements.java b/java/java-tests/testData/inspection/redundantCast/lambda/MiscStatements.java new file mode 100644 index 000000000000..b364b1e6e748 --- /dev/null +++ b/java/java-tests/testData/inspection/redundantCast/lambda/MiscStatements.java @@ -0,0 +1,24 @@ + +import java.util.List; + +class MyTest { + + void m() { + assert (boolean)(boolean)(boolean) (new Object() != null) : (String)(CharSequence)(String)"message"; + if ((boolean)(boolean)(boolean) (1 != 2)) { + for(String string : ((String[])new String[] {"a", "b", (String)"c"})) { + do { + while ((boolean)(boolean)(boolean) (1 != 2)) { + (String)MyTest.class.toString(); + } + } while ((boolean)(boolean)(boolean) (1 != 2)); + } + } + CharSequence sequence = "null"; + I ii = () -> (String) sequence; + } + + interface I { + CharSequence foo(); + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast18Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast18Test.java index 1c2678405e25..17d4ea6081ef 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast18Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast18Test.java @@ -50,4 +50,5 @@ public class RedundantCast18Test extends LightDaemonAnalyzerTestCase { public void testInferenceIncompatibilityWithoutCast() { doTest();} public void testCastToPrimitive() { doTest();} public void testParenthesisAroundConditional() { doTest();} + public void testMiscStatements() { doTest();} } \ No newline at end of file