diff --git a/java/java-impl/src/com/intellij/codeInspection/ObviousNullCheckInspection.java b/java/java-impl/src/com/intellij/codeInspection/ObviousNullCheckInspection.java index d5539c72425c..0970b6deaf47 100644 --- a/java/java-impl/src/com/intellij/codeInspection/ObviousNullCheckInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/ObviousNullCheckInspection.java @@ -24,10 +24,7 @@ import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.util.ObjectUtils; import com.siyeh.ig.callMatcher.CallMatcher; -import com.siyeh.ig.psiutils.BlockUtils; -import com.siyeh.ig.psiutils.CommentTracker; -import com.siyeh.ig.psiutils.ExpressionUtils; -import com.siyeh.ig.psiutils.SideEffectChecker; +import com.siyeh.ig.psiutils.*; import one.util.streamex.StreamEx; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; @@ -117,11 +114,11 @@ public class ObviousNullCheckInspection extends BaseJavaBatchLocalInspectionTool PsiElement parent = call.getParent(); CommentTracker ct = new CommentTracker(); if (parent instanceof PsiExpressionStatement) { - PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); - PsiStatement[] sideEffectStatements = StreamEx.of(call.getArgumentList().getExpressions()) + List expressions = StreamEx.of(call.getArgumentList().getExpressions()) .flatCollection(SideEffectChecker::extractSideEffectExpressions) - .map(expr -> factory.createStatementFromText(ct.text(expr) + ";", call)) - .toArray(PsiStatement[]::new); + .peek(ct::markUnchanged) + .toList(); + PsiStatement[] sideEffectStatements = StatementExtractor.generateStatements(expressions, call); if(sideEffectStatements.length > 0) { BlockUtils.addBefore((PsiStatement)parent, sideEffectStatements); } diff --git a/java/java-tests/testData/inspection/obviousNotNull/afterNonNullSideEffectComplex.java b/java/java-tests/testData/inspection/obviousNotNull/afterNonNullSideEffectComplex.java new file mode 100644 index 000000000000..5df8c85e0128 --- /dev/null +++ b/java/java-tests/testData/inspection/obviousNotNull/afterNonNullSideEffectComplex.java @@ -0,0 +1,68 @@ +// "Fix all 'Null-check method is called with obviously non-null argument' problems in file" "true" +import java.util.Objects; + +public class Test { + Test(int i) { + } + + public static void testTernaryLeft() { + if (args.length > 0) { + new Test(1); + new Test(2); + } + } + + public static void testTernaryRight() { + if (args.length <= 0) { + new Test(2); + new Test(3); + } + } + + public static void testTernaryBoth() { + if (args.length > 0) { + new Test(1); + } else { + new Test(2); + new Test(3); + } + } + + public static void testAndTernarySimply() { + new Test(1).hashCode(); + } + + public static void testAndTernaryBranch() { + if (new Test(1).hashCode() <= 0 || args.length <= 0) { + new Test(2); + } + } + + public static void testAndBoth() { + if (new Test(1).hashCode() > 0) { + new Test(2).hashCode(); + } + } + + public static void testAndTwoOfThree() { + if (new Test(1).hashCode() > 0) { + new Test(2).hashCode(); + } + } + + public static void testAndTwoOfThreePlusBranch() { + if (new Test(1).hashCode() > 0 && new Test(2).hashCode() > 0 + && args.length > 0) { + new Test(3).toString(); + } + } + + public static void testAndOrMixed() { + if (new Test(1).hashCode() <= 0 || new Test(2).hashCode() <= 0) { + if (new Test(3).hashCode() + new Test(4).hashCode() > 1) { + new Test(5).hashCode(); + new Test(6).hashCode(); + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/obviousNotNull/beforeNonNullSideEffectComplex.java b/java/java-tests/testData/inspection/obviousNotNull/beforeNonNullSideEffectComplex.java new file mode 100644 index 000000000000..abc1e8961911 --- /dev/null +++ b/java/java-tests/testData/inspection/obviousNotNull/beforeNonNullSideEffectComplex.java @@ -0,0 +1,47 @@ +// "Fix all 'Null-check method is called with obviously non-null argument' problems in file" "true" +import java.util.Objects; + +public class Test { + Test(int i) { + } + + public static void testTernaryLeft() { + Objects.requireNonNull("xyz" + (args.length > 0 ? new Test(1) + ":" + new Test(2) : "")); + } + + public static void testTernaryRight() { + Objects.requireNonNull("xyz" + (args.length > 0 ? "null" : new Test(2)+":"+new Test(3))); + } + + public static void testTernaryBoth() { + Objects.requireNonNull("xyz" + (args.length > 0 ? new Test(1) : new Test(2)+":"+new Test(3))); + } + + public static void testAndTernarySimply() { + Objects.requireNonNull("xyz" + (new Test(1).hashCode() > 0 && args.length > 0 ? "x" : "y")); + } + + public static void testAndTernaryBranch() { + Objects.requireNonNull("xyz" + (new Test(1).hashCode() > 0 && args.length > 0 ? "x" : new Test(2))); + } + + public static void testAndBoth() { + Objects.requireNonNull("xyz" + (new Test(1).hashCode() > 0 && new Test(2).hashCode() > 0 ? "x" : "y")); + } + + public static void testAndTwoOfThree() { + Objects.requireNonNull("xyz" + (new Test(1).hashCode() > 0 && new Test(2).hashCode() > 0 + && args.length > 0 ? "x" : "y")); + } + + public static void testAndTwoOfThreePlusBranch() { + Objects.requireNonNull("xyz" + (new Test(1).hashCode() > 0 && new Test(2).hashCode() > 0 + && args.length > 0 ? new Test(3).toString() : "y")); + } + + public static void testAndOrMixed() { + Objects.requireNonNull("xyz" + (new Test(1).hashCode() > 0 && new Test(2).hashCode() > 0 + || new Test(3).hashCode() + new Test(4).hashCode() > 1 && + new Test(5).hashCode() + new Test(6).hashCode() > 2)); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/StatementExtractor.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/StatementExtractor.java new file mode 100644 index 000000000000..d0afdc3dbeef --- /dev/null +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/StatementExtractor.java @@ -0,0 +1,218 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.siyeh.ig.psiutils; + +import com.intellij.psi.*; +import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.ArrayUtil; +import com.intellij.util.ObjectUtils; +import one.util.streamex.StreamEx; +import org.jetbrains.annotations.NotNull; + +import java.util.List; + +public class StatementExtractor { + private static final Node EMPTY = new Node(null) { + @Override + public Node prepend(Node node) { + return node; + } + + @Override + public String toString() { + return ""; + } + }; + + /** + * Generate statements from subexpressions of root expression which must be kept. + * + * @param expressionsToKeep list of expressions to keep. Each expression must be a descendant of root, they must be ordered inside list + * in program order and cannot be ancestors of each other. + * @param root a root expression + * @return an array of non-physical statements which represent the same logic as passed expressions + */ + @NotNull + public static PsiStatement[] generateStatements(List expressionsToKeep, PsiExpression root) { + String statementsCode = generateStatementsText(expressionsToKeep, root); + if(statementsCode.isEmpty()) return PsiStatement.EMPTY_ARRAY; + PsiElementFactory factory = JavaPsiFacade.getElementFactory(root.getProject()); + PsiCodeBlock codeBlock = factory.createCodeBlockFromText("{" + statementsCode + "}", root); + return codeBlock.getStatements(); + } + + public static String generateStatementsText(List expressionsToKeep, PsiExpression root) { + Node result = StreamEx.ofReversed(expressionsToKeep).map(expression -> createNode(expression, root)).foldLeft(EMPTY, Node::prepend); + return result.toString(); + } + + private static Node createNode(@NotNull PsiExpression expression, @NotNull PsiExpression root) { + Node result = new Expr(expression); + PsiExpression parent; + for (; expression != root; expression = parent) { + PsiElement parentElement = expression.getParent(); + if(parentElement instanceof PsiExpressionList) { + parentElement = parentElement.getParent(); + } + parent = ObjectUtils.tryCast(parentElement, PsiExpression.class); + if (parent == null) { + throw new IllegalStateException(expression.getText() + ": expected to have expression parent; root = " + root.getText()); + } + if (parent instanceof PsiPolyadicExpression) { + PsiPolyadicExpression polyadic = (PsiPolyadicExpression)parent; + IElementType type = polyadic.getOperationTokenType(); + boolean and; + if (type == JavaTokenType.ANDAND) { + and = true; + } + else if (type == JavaTokenType.OROR) { + and = false; + } + else { + continue; + } + PsiExpression[] operands = polyadic.getOperands(); + int index = ArrayUtil.indexOf(operands, expression); + if (index == 0) continue; + result = new Cond(parent, parent, index, and ? result : EMPTY, and ? EMPTY : result); + } + if (parent instanceof PsiConditionalExpression) { + PsiConditionalExpression ternary = (PsiConditionalExpression)parent; + if (expression == ternary.getThenExpression()) { + result = new Cond(ternary, ternary.getCondition(), -1, result, EMPTY); + } + else if (expression == ternary.getElseExpression()) { + result = new Cond(ternary, ternary.getCondition(), -1, EMPTY, result); + } + } + } + return result; + } + + private static abstract class Node { + final PsiExpression myAnchor; + + protected Node(PsiExpression anchor) { + myAnchor = anchor; + } + + public abstract Node prepend(Node node); + + public abstract String toString(); + } + + private static class Cond extends Node { + private final @NotNull PsiExpression myCondition; + private final @NotNull Node myThenBranch; + private final @NotNull Node myElseBranch; + private final int myLimit; + + private Cond(@NotNull PsiExpression anchor, + @NotNull PsiExpression condition, + int limit, + @NotNull Node thenBranch, + @NotNull Node elseBranch) { + super(anchor); + myCondition = condition; + myLimit = limit; + assert limit < 0 || condition instanceof PsiPolyadicExpression; + myThenBranch = thenBranch; + myElseBranch = elseBranch; + } + + private String getCondition(boolean invert) { + if (myLimit < 0) { + return invert ? BoolUtils.getNegatedExpressionText(myCondition) : myCondition.getText(); + } + PsiPolyadicExpression condition = (PsiPolyadicExpression)myCondition; + PsiExpression[] operands = condition.getOperands(); + String joiner = (condition.getOperationTokenType() == JavaTokenType.ANDAND) != invert ? "&&" : "||"; + return StreamEx.of(operands, 0, myLimit).map(invert ? BoolUtils::getNegatedExpressionText : PsiExpression::getText) + .joining(joiner); + } + + @Override + public String toString() { + if (myThenBranch == EMPTY) { + return "if(" + getCondition(true) + ") {" + myElseBranch + "}"; + } + return "if(" + getCondition(false) + ") {" + myThenBranch + "}" + (myElseBranch == EMPTY ? "" : "else {" + myElseBranch + "}"); + } + + @Override + public Node prepend(Node node) { + PsiExpression thatAnchor = node.myAnchor; + if(thatAnchor == null) return this; + if(thatAnchor == myAnchor) { + assert node instanceof Cond; + Cond cond = (Cond)node; + assert myCondition == cond.myCondition; + if(myLimit == cond.myLimit) { + return new Cond(myAnchor, myCondition, myLimit, myThenBranch.prepend(cond.myThenBranch), myElseBranch.prepend(cond.myElseBranch)); + } + assert myLimit > cond.myLimit; + return this; + } + if(PsiTreeUtil.isAncestor(myCondition, thatAnchor, false)) { + return this; + } + return new Cons(node, this); + } + } + + private static class Expr extends Node { + private Expr(@NotNull PsiExpression expression) { + super(expression); + } + + @Override + public Node prepend(Node node) { + return node.myAnchor == null ? this : new Cons(node, this); + } + + public String toString() { + return myAnchor.getText() + ";"; + } + } + + private static class Cons extends Node { + private final @NotNull Node myHead; + private final @NotNull Node myTail; + + private Cons(@NotNull Node head, @NotNull Node tail) { + super(head.myAnchor); + assert !(head instanceof Cons); + myHead = head; + myTail = tail; + } + + @Override + public Node prepend(Node node) { + if(node.myAnchor == null) return this; + if(PsiTreeUtil.isAncestor(myHead.myAnchor, node.myAnchor, false)) { + Node newHead = myHead.prepend(node); + return new Cons(newHead, myTail); + } + return new Cons(node, this); + } + + @Override + public String toString() { + return myHead.toString() + myTail; + } + } +}