Java: Better handling of unreachable code (IDEA-172642)

This commit is contained in:
Pavel Dolgov
2017-05-22 17:15:07 +03:00
parent da8b1f4a00
commit 16b1765f7f
19 changed files with 280 additions and 7 deletions

View File

@@ -16,6 +16,7 @@
package com.intellij.codeInsight.daemon.impl.analysis;
import com.intellij.codeInsight.daemon.JavaErrorMessages;
import com.intellij.codeInsight.daemon.QuickFixBundle;
import com.intellij.codeInsight.daemon.impl.HighlightInfo;
import com.intellij.codeInsight.daemon.impl.HighlightInfoType;
import com.intellij.codeInsight.daemon.impl.quickfix.QuickFixAction;
@@ -99,7 +100,18 @@ public class HighlightControlFlowUtil {
final PsiElement unreachableStatement = ControlFlowUtil.getUnreachableStatement(controlFlow);
if (unreachableStatement != null) {
String description = JavaErrorMessages.message("unreachable.statement");
return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(unreachableStatement).descriptionAndTooltip(description).create();
PsiElement keyword = null;
if (unreachableStatement instanceof PsiIfStatement ||
unreachableStatement instanceof PsiSwitchStatement ||
unreachableStatement instanceof PsiLoopStatement) {
keyword = unreachableStatement.getFirstChild();
}
final PsiElement element = keyword != null ? keyword : unreachableStatement;
final HighlightInfo info =
HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(element).descriptionAndTooltip(description).create();
QuickFixAction.registerQuickFixAction(
info, QUICK_FIX_FACTORY.createDeleteFix(unreachableStatement, QuickFixBundle.message("delete.unreachable.statement.fix.text")));
return info;
}
}
catch (AnalysisCanceledException | IndexNotReadyException e) {

View File

@@ -21,10 +21,7 @@ import com.intellij.psi.*;
import com.intellij.psi.impl.source.DummyHolder;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ArrayUtil;
import com.intellij.util.Function;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.ReflectionUtil;
import com.intellij.util.*;
import com.intellij.util.containers.IntArrayList;
import com.intellij.util.containers.IntStack;
import gnu.trove.THashMap;
@@ -1152,6 +1149,10 @@ public class ControlFlowUtil {
for (int i = 0; i < processedInstructions.length; i++) {
if (!processedInstructions[i]) {
PsiElement element = myFlow.getElement(i);
final PsiElement unreachableParent = getUnreachableExpressionParent(element);
if (unreachableParent != null) return unreachableParent;
if (element == null || !PsiUtil.isStatement(element)) continue;
if (element.getParent() instanceof PsiExpression) continue;
@@ -1170,11 +1171,59 @@ public class ControlFlowUtil {
final int startOffset = myFlow.getStartOffset(element);
// this offset actually is a part of reachable statement
if (0 <= startOffset && startOffset < processedInstructions.length && processedInstructions[startOffset]) continue;
return element;
final PsiElement enclosingStatement = getEnclosingUnreachableStatement(element);
return enclosingStatement != null ? enclosingStatement : element;
}
}
return null;
}
@Nullable
private static PsiElement getUnreachableExpressionParent(@Nullable PsiElement element) {
if (element instanceof PsiExpression) {
final PsiElement expression = PsiTreeUtil.findFirstParent(element, e -> !(e.getParent() instanceof PsiParenthesizedExpression));
if (expression != null) {
final PsiElement parent = expression.getParent();
if (parent instanceof PsiExpressionStatement) {
return getUnreachableStatementParent(parent);
}
if (parent instanceof PsiIfStatement && ((PsiIfStatement)parent).getCondition() == expression ||
parent instanceof PsiSwitchStatement && ((PsiSwitchStatement)parent).getExpression() == expression ||
parent instanceof PsiWhileStatement && ((PsiWhileStatement)parent).getCondition() == expression ||
parent instanceof PsiForeachStatement && ((PsiForeachStatement)parent).getIteratedValue() == expression) {
return parent;
}
}
}
return null;
}
@Nullable
private static PsiElement getEnclosingUnreachableStatement(@NotNull PsiElement statement) {
final PsiElement parent = statement.getParent();
if (parent instanceof PsiDoWhileStatement && ((PsiDoWhileStatement)parent).getBody() == statement) {
return parent;
}
if (parent instanceof PsiCodeBlock && PsiTreeUtil.getNextSiblingOfType(parent.getFirstChild(), PsiStatement.class) == statement) {
final PsiBlockStatement blockStatement = ObjectUtils.tryCast(parent.getParent(), PsiBlockStatement.class);
if (blockStatement != null) {
final PsiElement blockParent = blockStatement.getParent();
if (blockParent instanceof PsiDoWhileStatement && ((PsiDoWhileStatement)blockParent).getBody() == blockStatement) {
return blockParent;
}
}
}
return getUnreachableStatementParent(statement);
}
@Nullable
private static PsiElement getUnreachableStatementParent(@NotNull PsiElement statement) {
final PsiElement parent = statement.getParent();
if (parent instanceof PsiForStatement && ((PsiForStatement)parent).getInitialization() == statement) {
return parent;
}
return null;
}
}
private static PsiReferenceExpression getEnclosingReferenceExpression(PsiElement element, PsiVariable variable) {

View File

@@ -0,0 +1,93 @@
class WithCondition {
void testIf1(boolean b) {
return;
<error descr="Unreachable statement">if</error> (b) {
System.out.println("Never");
}
else {
System.out.println("Never ever");
}
}
void testIf2(boolean b) {
return;
<error descr="Unreachable statement">if</error> ((b)) {
System.out.println("Never");
}
}
void testSwitch(int b) {
return;
<error descr="Unreachable statement">switch</error> (b) {
case 1:
System.out.println("Never");
}
}
void testFor1(int b) {
return;
<error descr="Unreachable statement">for</error> (int i = 0; i < b; i++) {
System.out.println("Never");
}
}
void testFor2(int b) {
int i;
return;
<error descr="Unreachable statement">for</error> (i = 0; i < b; i++) {
System.out.println("Never");
}
}
void testFor3(int b) {
int i = 0;
return;
<error descr="Unreachable statement">for</error> (i++; i < b; i++) {
System.out.println("Never");
}
}
void testFor3(boolean b) {
return;
<error descr="Unreachable statement">for</error> (; b; ) {
System.out.println("Never");
}
}
void testFor4() {
return;
<error descr="Unreachable statement">for</error> (; ; ) {
System.out.println("Never");
}
}
void testFor5() {
int i = 0;
return;
<error descr="Unreachable statement">for</error> (; ; i++) {
System.out.println("Never");
}
}
void testWhile(boolean b) {
return;
<error descr="Unreachable statement">while</error> (b) {
System.out.println("Never");
}
}
void testDoWhile1(boolean b) {
return;
<error descr="Unreachable statement">do</error> System.out.println("Never");
while (b);
}
void testDoWhile2(boolean b) {
return;
<error descr="Unreachable statement">do</error> {
System.out.println("Never");
}
while (b);
}
}

View File

@@ -0,0 +1,10 @@
class WithCondition {
void testForEach() {
return;
<error descr="Unreachable statement">for</error> (int i: this.array()) {
System.out.println("Never");
}
}
int[] array() { return new int[0]; }
}

View File

@@ -0,0 +1,6 @@
// "Delete unreachable statement" "true"
class Never {
void foo(int n) {
return;
}
}

View File

@@ -0,0 +1,6 @@
// "Delete unreachable statement" "true"
class Never {
void foo(boolean b) {
return;
}
}

View File

@@ -0,0 +1,6 @@
// "Delete unreachable statement" "true"
class Never {
void foo() {
return;
}
}

View File

@@ -0,0 +1,6 @@
// "Delete unreachable statement" "true"
class Never {
void foo(boolean b) {
return;
}
}

View File

@@ -0,0 +1,6 @@
// "Delete unreachable statement" "true"
class Never {
void foo(int n) {
return;
}
}

View File

@@ -0,0 +1,7 @@
// "Delete unreachable statement" "true"
class Never {
void foo(int n) {
return;
<caret>System.out.println(n);
}
}

View File

@@ -0,0 +1,10 @@
// "Delete unreachable statement" "true"
class Never {
void foo(boolean b) {
return;
<caret>do {
System.out.println("Never");
}
while (b);
}
}

View File

@@ -0,0 +1,9 @@
// "Delete unreachable statement" "true"
class Never {
void foo() {
return;
<caret>for (int i = 0; i < 2; i++) {
System.out.println("Never");
}
}
}

View File

@@ -0,0 +1,11 @@
// "Delete unreachable statement" "true"
class Never {
void foo(boolean b) {
return;
<caret>if (b) {
System.out.println("Never");
} else {
System.out.println("Never ever");
}
}
}

View File

@@ -0,0 +1,10 @@
// "Delete unreachable statement" "true"
class Never {
void foo(int n) {
return;
<caret>switch (n) {
case 1:
System.out.println("Never");
}
}
}

View File

@@ -61,6 +61,7 @@ public class LightAdvHighlightingJdk6Test extends LightDaemonAnalyzerTestCase {
public void testUnreachableAssignments() { doTest(false, false); }
public void testCompileTypeConstantsAccessibleFromStaticFieldInitializers() { doTest(false, false);}
public void testInheritUnrelatedConcreteMethodsWithSameSignature() { doTest(false, false);}
public void testStatementWithExpression() { doTest(false, false); }
public void testAssignmentFromStringToObject() {
doTest(true, false);

View File

@@ -416,6 +416,7 @@ public class LightAdvHighlightingTest extends LightDaemonAnalyzerTestCase {
public void testPrivateFieldInSuperClass() { doTest(false); }
public void testNoEnclosingInstanceWhenStaticNestedInheritsFromContainingClass() { doTest(false); }
public void testIDEA168768() { doTest(false); }
public void testStatementWithExpression() { doTest(false); }
public void testStaticMethodCalls() {
doTestFile(BASE_PATH + "/" + getTestName(false) + ".java").checkSymbolNames().test();

View File

@@ -0,0 +1,29 @@
/*
* 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.daemon.quickFix
/**
* @author Pavel.Dolgov
*/
class DeleteUnreachableTest : LightQuickFixParameterizedTestCase() {
override fun getBasePath() = "/codeInsight/daemonCodeAnalyzer/quickFix/deleteUnreachable"
@Suppress("DEPRECATION")
fun test() {
doAllTests()
}
}

View File

@@ -55,7 +55,7 @@ public class LoopStatementsThatDontLoop
throw new Exception();
}
<warning descr="'for' statement does not loop">for</warning>(<error descr="Unreachable statement">;</error> ;)
<error descr="Unreachable statement"><warning descr="'for' statement does not loop">for</warning></error>(; ;)
{
return;
}

View File

@@ -297,6 +297,7 @@ move.file.to.source.root.text=Move file to a source root
delete.element.fix.text=Delete element
delete.reference.fix.text=Delete reference
delete.unreachable.statement.fix.text=Delete unreachable statement
module.info.add.requires.family.name=Add 'requires' statement to module-info.java
module.info.add.requires.name=Add ''requires {0}'' statement to module-info.java