Java: Don't offer "Unwrap 'else' branch" intention if it might cause unreachable code (IDEA-165428)

This commit is contained in:
Pavel Dolgov
2017-01-17 15:46:27 +03:00
parent 09aa5b41f8
commit ce236e3666
50 changed files with 594 additions and 87 deletions

View File

@@ -1,69 +0,0 @@
/*
* 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.intellij.codeInsight.intention.impl;
import com.intellij.codeInsight.CodeInsightBundle;
import com.intellij.codeInsight.intention.PsiElementBaseIntentionAction;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.util.IncorrectOperationException;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
/**
* @author Pavel.Dolgov
*/
public class InlineElseBranchAction extends PsiElementBaseIntentionAction {
@Override
public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement element) throws IncorrectOperationException {
PsiElement parent = element.getParent();
if (parent instanceof PsiIfStatement) {
PsiIfStatement ifStatement = (PsiIfStatement)parent;
PsiStatement elseBranch = ifStatement.getElseBranch();
PsiKeyword elseKeyword = ifStatement.getElseElement();
if (elseBranch != null && elseKeyword != null) {
InvertIfConditionAction.addAfter(ifStatement, elseBranch);
elseBranch.delete();
elseKeyword.delete();
}
}
}
@Override
public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull PsiElement element) {
if (element instanceof PsiKeyword && ((PsiKeyword)element).getTokenType() == JavaTokenType.ELSE_KEYWORD) {
PsiElement parent = element.getParent();
return parent instanceof PsiIfStatement &&
((PsiIfStatement)parent).getElseBranch() != null &&
parent.getParent() instanceof PsiCodeBlock;
}
return false;
}
@NotNull
@Override
public String getText() {
return getFamilyName();
}
@Nls
@NotNull
@Override
public String getFamilyName() {
return CodeInsightBundle.message("intention.inline.else.branch");
}
}

View File

@@ -0,0 +1,131 @@
/*
* 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.intellij.codeInsight.intention.impl;
import com.intellij.codeInsight.CodeInsightBundle;
import com.intellij.codeInsight.intention.PsiElementBaseIntentionAction;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
/**
* @author Pavel.Dolgov
*/
public class UnwrapElseBranchAction extends PsiElementBaseIntentionAction {
private static final Logger LOG = Logger.getInstance(UnwrapElseBranchAction.class);
@Override
public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement element) throws IncorrectOperationException {
PsiElement parent = element.getParent();
if (parent instanceof PsiIfStatement) {
PsiIfStatement ifStatement = (PsiIfStatement)parent;
PsiStatement elseBranch = ifStatement.getElseBranch();
PsiElement grandParent = ifStatement.getParent();
if (elseBranch != null && grandParent != null) {
if (!(grandParent instanceof PsiCodeBlock)) {
PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
PsiCodeBlock codeBlock = factory.createCodeBlockFromText("{" + ifStatement.getText() + "}", ifStatement);
codeBlock = (PsiCodeBlock)ifStatement.replace(codeBlock);
ifStatement = (PsiIfStatement)codeBlock.getStatements()[0];
elseBranch = ifStatement.getElseBranch();
LOG.assertTrue(elseBranch != null);
}
InvertIfConditionAction.addAfter(ifStatement, elseBranch);
elseBranch.delete();
}
}
}
@Override
public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull PsiElement element) {
if (element instanceof PsiKeyword && ((PsiKeyword)element).getTokenType() == JavaTokenType.ELSE_KEYWORD) {
PsiElement parent = element.getParent();
if (parent instanceof PsiIfStatement) {
PsiIfStatement ifStatement = (PsiIfStatement)parent;
PsiStatement elseBranch = ifStatement.getElseBranch();
if (elseBranch != null) {
PsiStatement thenBranch = ifStatement.getThenBranch();
boolean thenCompletesNormally = ControlFlowUtils.statementMayCompleteNormally(thenBranch);
boolean elseCompletesNormally = ControlFlowUtils.statementMayCompleteNormally(elseBranch);
if (!thenCompletesNormally ||
elseCompletesNormally ||
!nextStatementMayBecomeUnreachable(ifStatement)) {
if (thenCompletesNormally) {
setText(CodeInsightBundle.message("intention.unwrap.else.branch.changes.semantics"));
}
else {
setText(CodeInsightBundle.message("intention.unwrap.else.branch"));
}
return true;
}
}
}
}
return false;
}
/**
* Check if there could be new unreachable code if the statement may not complete normally
*
* @param statement after refactoring it may not complete normally
* @return true if the refactoring may cause unreachable code
*/
private static boolean nextStatementMayBecomeUnreachable(PsiStatement statement) {
PsiStatement nextStatement = PsiTreeUtil.getNextSiblingOfType(statement, PsiStatement.class);
if (nextStatement != null) {
return !(nextStatement instanceof PsiSwitchLabelStatement);
}
PsiElement parent = statement.getParent();
if (parent instanceof PsiIfStatement) {
PsiIfStatement ifStatement = (PsiIfStatement)parent;
PsiStatement thenBranch = ifStatement.getThenBranch();
PsiStatement elseBranch = ifStatement.getElseBranch();
if (thenBranch == statement && ControlFlowUtils.statementMayCompleteNormally(elseBranch) ||
elseBranch == statement && ControlFlowUtils.statementMayCompleteNormally(thenBranch)) {
return false;
}
return nextStatementMayBecomeUnreachable(ifStatement);
}
if (parent instanceof PsiLabeledStatement) {
return nextStatementMayBecomeUnreachable((PsiLabeledStatement)parent);
}
if (parent instanceof PsiCodeBlock) {
PsiStatement parentStatement = ObjectUtils.tryCast(parent.getParent(), PsiStatement.class);
if (parentStatement instanceof PsiBlockStatement ||
parentStatement instanceof PsiSynchronizedStatement ||
parentStatement instanceof PsiTryStatement || // TODO handle try-catch more accurately
parentStatement instanceof PsiSwitchStatement) {
return nextStatementMayBecomeUnreachable(parentStatement);
}
}
return false;
}
@Nls
@NotNull
@Override
public String getFamilyName() {
return CodeInsightBundle.message("intention.unwrap.else.branch");
}
}

View File

@@ -1,4 +1,4 @@
// "Inline 'else' branch" "true"
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b) {

View File

@@ -1,4 +1,4 @@
// "Inline 'else' branch" "true"
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b) {

View File

@@ -0,0 +1,9 @@
// "Unwrap 'else' branch" "true"
class T {
void f(boolean b) {
if (b)
throw new RuntimeException("When true");
System.out.println("Otherwise");
}
}

View File

@@ -0,0 +1,12 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean a, boolean b) {
if (a) {
if (b) {
System.out.println("When true");
}
System.out.println("Otherwise");
}
}
}

View File

@@ -0,0 +1,13 @@
// "Unwrap 'else' branch" "true"
class T {
String f(boolean a, boolean b) {
if (a) {
if (b) {
return "When true";
}
System.out.println("Otherwise");
}
return "Default";
}
}

View File

@@ -0,0 +1,13 @@
// "Unwrap 'else' branch" "true"
class T {
String f(boolean a, boolean b) {
if (a) {
if (b) {
return "When true";
}
return "Otherwise";
}
return "Default";
}
}

View File

@@ -0,0 +1,15 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean a, boolean b) {
if (a) {
Label:
{
if (b)
System.out.println("When true");
throw new RuntimeException("Otherwise");
}
}
System.out.println("Done");
}
}

View File

@@ -0,0 +1,12 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean a, boolean b) {
if (a) {
if (b)
System.out.println("When true");
System.out.println("Otherwise");
return;
}
}
}

View File

@@ -0,0 +1,12 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
String f(boolean a, boolean b) {
if (a) {
if (b)
System.out.println("When true");
return "Otherwise";
}
return "Default";
}
}

View File

@@ -1,4 +1,4 @@
// "Inline 'else' branch" "true"
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b) {

View File

@@ -1,4 +1,4 @@
// "Inline 'else' branch" "true"
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b) {

View File

@@ -0,0 +1,11 @@
// "Unwrap 'else' branch" "true"
class T {
String f(boolean b) {
if (b)
return "When true";
// Before
return "Otherwise";
// After
}
}

View File

@@ -0,0 +1,11 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
String f(boolean b) {
if (b)
System.out.println("When true");
else
return "Otherwise";
return "Default";
}
}

View File

@@ -0,0 +1,15 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b, int n) {
switch (n) {
case 1:
if (b)
System.out.println("When true");
// Before
System.out.println("Otherwise");
// After
case 2:
}
}
}

View File

@@ -0,0 +1,16 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b, int n) {
switch (n) {
case 1:
if (b)
System.out.println("When true");
// Before
System.out.println("Otherwise");
// After
break;
case 2:
}
}
}

View File

@@ -0,0 +1,17 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b, int n) {
switch (n) {
case 1:
if (b)
System.out.println("When true");
// Before
System.out.println("Otherwise");
// After
return;
case 2:
}
System.out.println("Done");
}
}

View File

@@ -0,0 +1,11 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(Object o, boolean b) {
synchronized (o) {
if (b)
System.out.println("When true");
throw new RuntimeException("Otherwise");
}
}
}

View File

@@ -0,0 +1,13 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b) {
try {
if (b)
System.out.println("When true");
throw new RuntimeException("Otherwise");
} finally {
System.out.println("Finally");
}
}
}

View File

@@ -1,4 +1,4 @@
// "Inline 'else' branch" "true"
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b) {

View File

@@ -1,4 +1,4 @@
// "Inline 'else' branch" "true"
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b) {

View File

@@ -0,0 +1,11 @@
// "Unwrap 'else' branch" "true"
class T {
void f(boolean b) {
if (b)
throw new RuntimeException("When true");
<caret>else {
System.out.println("Otherwise");
}
}
}

View File

@@ -1,4 +1,4 @@
// "Inline 'else' branch" "false"
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean a, boolean b) {

View File

@@ -0,0 +1,13 @@
// "Unwrap 'else' branch" "true"
class T {
String f(boolean a, boolean b) {
if (a)
if (b) {
return "When true";
} <caret>else {
System.out.println("Otherwise");
}
return "Default";
}
}

View File

@@ -0,0 +1,13 @@
// "Unwrap 'else' branch" "true"
class T {
String f(boolean a, boolean b) {
if (a)
if (b) {
return "When true";
} <caret>else {
return "Otherwise";
}
return "Default";
}
}

View File

@@ -0,0 +1,17 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean a, boolean b) {
if (a) {
Label:
{
if (b)
System.out.println("When true");
<caret>else {
throw new RuntimeException("Otherwise");
}
}
}
System.out.println("Done");
}
}

View File

@@ -0,0 +1,15 @@
// "Unwrap 'else' branch (changes semantics)" "false"
class T {
void f(boolean b) {
Label:
{
if (b)
System.out.println("When true");
<caret>else {
throw new RuntimeException("Otherwise");
}
}
System.out.println("Done");
}
}

View File

@@ -0,0 +1,14 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean a, boolean b) {
if (a) {
if (b)
System.out.println("When true");
<caret>else {
System.out.println("Otherwise");
return;
}
}
}
}

View File

@@ -0,0 +1,13 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
String f(boolean a, boolean b) {
if (a) {
if (b)
System.out.println("When true");
<caret>else
return "Otherwise";
}
return "Default";
}
}

View File

@@ -0,0 +1,14 @@
// "Unwrap 'else' branch (changes semantics)" "false"
class T {
void m(boolean b) {
if (b) {
System.out.println(1);
}
<caret>else {
System.out.println(2);
return;
}
System.out.println(3);
}
}

View File

@@ -0,0 +1,14 @@
// "Unwrap 'else' branch" "false"
class T {
void m(boolean b) {
if (b) {
System.out.println(1);
}
<caret>else {
System.out.println(2);
return;
}
System.out.println(3);
}
}

View File

@@ -1,4 +1,4 @@
// "Inline 'else' branch" "true"
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b) {

View File

@@ -1,4 +1,4 @@
// "Inline 'else' branch" "true"
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b) {

View File

@@ -0,0 +1,12 @@
// "Unwrap 'else' branch" "true"
class T {
String f(boolean b) {
if (b)
return "When true";
<caret>else
// Before
return "Otherwise";
// After
}
}

View File

@@ -0,0 +1,16 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b, int n) {
switch (n) {
case 1:
if (b)
System.out.println("When true");
<caret>else
// Before
System.out.println("Otherwise");
// After
case 2:
}
}
}

View File

@@ -0,0 +1,18 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b, int n) {
switch (n) {
case 1:
if (b)
System.out.println("When true");
<caret>else {
// Before
System.out.println("Otherwise");
// After
break;
}
case 2:
}
}
}

View File

@@ -0,0 +1,19 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b, int n) {
switch (n) {
case 1:
if (b)
System.out.println("When true");
<caret>else {
// Before
System.out.println("Otherwise");
// After
return;
}
case 2:
}
System.out.println("Done");
}
}

View File

@@ -0,0 +1,18 @@
// "Unwrap 'else' branch (changes semantics)" "false"
class T {
void f(boolean b, int n) {
switch (n) {
case 1:
if (b)
System.out.println("When true");
<caret>else {
// Before
System.out.println("Otherwise");
// After
return;
}
}
System.out.println("Done");
}
}

View File

@@ -0,0 +1,13 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(Object o, boolean b) {
synchronized (o) {
if (b)
System.out.println("When true");
<caret>else {
throw new RuntimeException("Otherwise");
}
}
}
}

View File

@@ -0,0 +1,14 @@
// "Unwrap 'else' branch (changes semantics)" "false"
class T {
void f(Object o, boolean b) {
synchronized (o) {
if (b)
System.out.println("When true");
<caret>else {
throw new RuntimeException("Otherwise");
}
}
System.out.println("Done");
}
}

View File

@@ -0,0 +1,15 @@
// "Unwrap 'else' branch (changes semantics)" "true"
class T {
void f(boolean b) {
try {
if (b)
System.out.println("When true");
<caret>else {
throw new RuntimeException("Otherwise");
}
} finally {
System.out.println("Finally");
}
}
}

View File

@@ -0,0 +1,15 @@
// "Unwrap 'else' branch (changes semantics)" "false"
class T {
void f(boolean b) {
try {
if (b)
System.out.println("When true");
<caret>else {
throw new RuntimeException("Otherwise");
}
} finally {
}
System.out.println("Done");
}
}

View File

@@ -20,11 +20,11 @@ import com.intellij.codeInsight.daemon.LightIntentionActionTestCase;
/**
* @author Pavel.Dolgov
*/
public class InlineElseBranchTest extends LightIntentionActionTestCase {
public class UnwrapElseBranchTest extends LightIntentionActionTestCase {
@Override
protected String getBasePath() {
return "/codeInsight/inlineElseBranch/";
return "/codeInsight/unwrapElseBranch/";
}
public void test() throws Exception {

View File

@@ -235,7 +235,8 @@ intention.replace.cast.with.var.family=Replace cast with variable
intention.convert.color.representation.text=Convert to ''new Color{0}''
intention.convert.color.representation.family=Convert Color representation
intention.break.string.on.line.breaks.text=Break string on '\\n'
intention.inline.else.branch=Inline 'else' branch
intention.unwrap.else.branch=Unwrap 'else' branch
intention.unwrap.else.branch.changes.semantics=Unwrap 'else' branch (changes semantics)
intention.create.test=Create Test

View File

@@ -1,5 +0,0 @@
<html>
<body>
This intention inlines the code from the <b>else</b> branch after the <b>if</b> statement.
</body>
</html>

View File

@@ -0,0 +1,5 @@
<html>
<body>
This intention moves the code of the <b>else</b> branch after the <b>if</b> statement.
</body>
</html>

View File

@@ -978,7 +978,7 @@
<category>Java/Control Flow</category>
</intentionAction>
<intentionAction>
<className>com.intellij.codeInsight.intention.impl.InlineElseBranchAction</className>
<className>com.intellij.codeInsight.intention.impl.UnwrapElseBranchAction</className>
<category>Java/Control Flow</category>
</intentionAction>