EnsureCodeBlockImpl: support try splitting; used in AddAssertStatementFix

Fixes IDEA-230023 Applying `Assert != null` in try-with-resources leads to uncompilable code

GitOrigin-RevId: 3dc141010420570c061849287adaeb0fccbcf1f4
This commit is contained in:
Tagir Valeev
2020-01-10 11:37:34 +00:00
committed by intellij-monorepo-bot
parent b6c8e10a79
commit 1bb6f68f9b
12 changed files with 149 additions and 30 deletions
@@ -20,7 +20,6 @@ import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.refactoring.util.RefactoringUtil;
import com.intellij.util.IncorrectOperationException;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
@@ -44,34 +43,22 @@ public class AddAssertStatementFix implements LocalQuickFix {
@Override
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
PsiElement element = descriptor.getPsiElement();
PsiExpression element = PsiTreeUtil.getParentOfType(descriptor.getPsiElement(), PsiExpression.class);
if (element == null) return;
element = RefactoringUtil.ensureCodeBlock(element);
if (element == null) return;
PsiElement anchorElement = RefactoringUtil.getParentStatement(element, false);
LOG.assertTrue(anchorElement != null);
final PsiElement tempParent = anchorElement.getParent();
if (tempParent instanceof PsiForStatement && !PsiTreeUtil.isAncestor(((PsiForStatement)tempParent).getBody(), anchorElement, false)) {
anchorElement = tempParent;
}
PsiElement prev = PsiTreeUtil.skipWhitespacesBackward(anchorElement);
if (prev instanceof PsiComment && JavaSuppressionUtil.getSuppressedInspectionIdsIn(prev) != null) {
anchorElement = prev;
}
try {
final PsiElementFactory factory = JavaPsiFacade.getElementFactory(element.getProject());
@NonNls String text = "assert " + myText + ";";
PsiAssertStatement assertStatement = (PsiAssertStatement)factory.createStatementFromText(text, element);
final PsiElementFactory factory = JavaPsiFacade.getElementFactory(element.getProject());
@NonNls String text = "assert " + myText + ";";
PsiAssertStatement assertStatement = (PsiAssertStatement)factory.createStatementFromText(text, element);
final PsiElement parent = anchorElement.getParent();
if (parent instanceof PsiCodeBlock) {
parent.addBefore(assertStatement, anchorElement);
}
else {
RefactoringUtil.putStatementInLoopBody(assertStatement, parent, anchorElement);
}
}
catch (IncorrectOperationException e) {
LOG.error(e);
}
anchorElement.getParent().addBefore(assertStatement, anchorElement);
}
@Override
@@ -29,12 +29,12 @@ import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiPrecedenceUtil;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.refactoring.util.RefactoringUtil;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.SmartList;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.ui.JBUI;
import com.siyeh.ig.fixes.IntroduceVariableFix;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.ParenthesesUtils;
import com.siyeh.ig.psiutils.SideEffectChecker;
@@ -144,7 +144,7 @@ public class DataFlowInspection extends DataFlowInspectionBase {
PsiExpression operand = castExpression.getOperand();
PsiTypeElement typeElement = castExpression.getCastType();
if (typeElement != null && operand != null) {
if (!alwaysFails && !SideEffectChecker.mayHaveSideEffects(operand)) {
if (!alwaysFails && !SideEffectChecker.mayHaveSideEffects(operand) && ControlFlowUtils.canExtractStatement(castExpression)) {
String suffix = " instanceof " + typeElement.getText();
fixes.add(new AddAssertStatementFix(ParenthesesUtils.getText(operand, PsiPrecedenceUtil.RELATIONAL_PRECEDENCE) + suffix));
if (onTheFly && SurroundWithIfFix.isAvailable(operand)) {
@@ -188,7 +188,7 @@ public class DataFlowInspection extends DataFlowInspectionBase {
else if (!ExpressionUtils.isNullLiteral(qualifier) && !SideEffectChecker.mayHaveSideEffects(qualifier)) {
String suffix = " != null";
if (PsiUtil.getLanguageLevel(qualifier).isAtLeast(LanguageLevel.JDK_1_4) &&
RefactoringUtil.getParentStatement(expression, false) != null) {
ControlFlowUtils.canExtractStatement(expression)) {
String replacement = ParenthesesUtils.getText(qualifier, ParenthesesUtils.EQUALITY_PRECEDENCE) + suffix;
fixes.add(new AddAssertStatementFix(replacement));
}
@@ -18,6 +18,7 @@ import one.util.streamex.StreamEx;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.List;
import java.util.Objects;
import java.util.function.BinaryOperator;
@@ -94,9 +95,52 @@ class EnsureCodeBlockImpl {
if (parent instanceof PsiReturnStatement && condition != null) {
return replace(expression, parent, (oldParent, copy) -> splitReturn((PsiReturnStatement)oldParent, condition, operand));
}
if (parent instanceof PsiTryStatement) {
PsiResourceVariable resourceVar = PsiTreeUtil.getParentOfType(expression, PsiResourceVariable.class);
if (resourceVar != null) {
PsiResourceList list = ObjectUtils.tryCast(resourceVar.getParent(), PsiResourceList.class);
if (list != null && list.getParent() == parent && list.iterator().next() != resourceVar) {
return replace(expression, parent, (oldParent, copy) -> splitTry((PsiTryStatement)oldParent, (PsiTryStatement)copy, resourceVar));
}
}
}
return expression;
}
private static PsiElement splitTry(PsiTryStatement statement, PsiTryStatement copy, PsiResourceVariable var) {
PsiResourceList list = statement.getResourceList();
if (list == null) return statement;
PsiResourceList copyList = copy.getResourceList();
if (copyList == null) return statement;
PsiCodeBlock tryBlock = statement.getTryBlock();
if (tryBlock == null) return statement;
List<PsiResourceListElement> elementsToMove = StreamEx.of(list.iterator()).dropWhile(e -> e != var).toList();
for (PsiResourceListElement element : elementsToMove) {
element.delete();
}
for (PsiResourceListElement element : StreamEx.of(copyList.iterator()).limit(copyList.getResourceVariablesCount()-elementsToMove.size()).toList()) {
element.delete();
}
PsiElement[] children = copyList.getChildren();
if (children[0].textMatches("(") && children[1] instanceof PsiWhiteSpace) {
children[1].delete();
}
for (PsiCatchSection section : copy.getCatchSections()) {
section.delete();
}
PsiCodeBlock copyFinally = copy.getFinallyBlock();
if (copyFinally != null) {
PsiElement element = PsiTreeUtil.skipWhitespacesAndCommentsBackward(copyFinally);
if (element != null && element.textMatches(PsiKeyword.FINALLY)) {
element.delete();
}
copyFinally.delete();
}
PsiElement codeBlock = tryBlock.replace(JavaPsiFacade.getElementFactory(statement.getProject()).createCodeBlock());
codeBlock.add(copy);
return statement;
}
private static <T extends PsiElement> T replace(@NotNull T element, @NotNull PsiElement parent, BinaryOperator<PsiElement> replacer) {
Object marker = new Object();
PsiTreeUtil.mark(element, marker);
@@ -0,0 +1,10 @@
// "Assert 'myFoo != null'" "true"
class A{
private final String myFoo = null;
String myBar;
{
assert myFoo != null;
myBar = myFoo.substring(0);
}
}
@@ -2,7 +2,8 @@
class A{
void test(){
Integer container = null;
int i = 0;
assert container != null;
for (int i = 0, limit = container.intValue(); i < limit; i++){}
for (int limit = container.intValue(); i < limit; i++){}
}
}
@@ -3,8 +3,12 @@ class A{
void test(){
Object container = null;
Runnable r = () -> {
assert container != null;
container == null ? container.toString() : "";
if (container == null) {
assert container != null;
container.toString();
} else {
"";
}
};
}
}
@@ -0,0 +1,31 @@
// "Assert 'foo != null'" "true"
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.NotNull;
class X {
class Foo implements AutoCloseable {
public void close() {}
}
class Bar implements AutoCloseable {
Bar(@NotNull Foo foo) {}
public void close() {}
}
native @Nullable Foo getFoo();
void test() {
try (Foo foo = getFoo()) {
assert foo != null;
try (Bar bar = new Bar(foo))
{
System.out.println(bar);
}
}
catch (Exception ex) {
ex.printStackTrace();
}
finally {
System.out.println("exiting");
}
}
}
@@ -1,5 +1,5 @@
// "Assert 'myFoo != null'" "false"
// "Assert 'myFoo != null'" "true"
class A{
private final String myFoo = null;
String myBar = my<caret>Foo.substring(0);
String myBar = myFoo.su<caret>bstring(0);
}
@@ -1,4 +1,4 @@
// "Assert 'container != null'" "true"
// "Assert 'container != null'" "false"
class A{
void test(){
Integer container = null;
@@ -0,0 +1,8 @@
// "Assert 'container != null'" "true"
class A{
void test(){
Integer container = null;
int i = 0;
for (int limit = container.int<caret>Value(); i < limit; i++){}
}
}
@@ -0,0 +1,29 @@
// "Assert 'foo != null'" "true"
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.NotNull;
class X {
class Foo implements AutoCloseable {
public void close() {}
}
class Bar implements AutoCloseable {
Bar(@NotNull Foo foo) {}
public void close() {}
}
native @Nullable Foo getFoo();
void test() {
try (Foo foo = getFoo();
Bar bar = new Bar(f<caret>oo))
{
System.out.println(bar);
}
catch (Exception ex) {
ex.printStackTrace();
}
finally {
System.out.println("exiting");
}
}
}
@@ -923,6 +923,7 @@ public class ControlFlowUtils {
if (parent instanceof PsiConditionalExpression && ((PsiConditionalExpression)parent).getCondition() != cur) {
PsiElement ternaryParent = PsiUtil.skipParenthesizedExprUp(parent.getParent());
return ternaryParent instanceof PsiReturnStatement ||
ternaryParent instanceof PsiLambdaExpression ||
(ternaryParent instanceof PsiLocalVariable &&
(!((PsiLocalVariable)ternaryParent).getTypeElement().isInferredType() ||
PsiTypesUtil.isDenotableType(((PsiLocalVariable)ternaryParent).getType(), ternaryParent))) ||
@@ -957,6 +958,10 @@ public class ControlFlowUtils {
}
}
}
if (parent instanceof PsiResourceVariable) {
PsiResourceList list = ObjectUtils.tryCast(parent.getParent(), PsiResourceList.class);
return list != null && list.getParent() instanceof PsiTryStatement;
}
if (parent instanceof PsiField) {
PsiElement prev = PsiTreeUtil.skipWhitespacesAndCommentsBackward(parent);
PsiElement next = PsiTreeUtil.skipWhitespacesAndCommentsForward(parent);