[java-highlighting] IDEA-284864 Improve reporting of always false while/for loops

GitOrigin-RevId: 9e7403383296cfa3c9478d8e635e592d31ba0518
This commit is contained in:
Tagir Valeev
2021-12-14 13:26:42 +00:00
committed by intellij-monorepo-bot
parent ed94f6f07b
commit 7367405b46
16 changed files with 102 additions and 34 deletions
@@ -557,4 +557,7 @@ public abstract class QuickFixFactory {
@NotNull
public abstract IntentionAction createMoveSwitchBranchUpFix(@NotNull PsiCaseLabelElement moveBeforeLabel,
@NotNull PsiCaseLabelElement labelElement);
@NotNull
public abstract IntentionAction createSimplifyBooleanFix(@NotNull PsiExpression expression, boolean value);
}
@@ -25,6 +25,7 @@ import com.intellij.util.JavaPsiConstructorUtil;
import com.intellij.util.ObjectUtils;
import com.intellij.util.Processor;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.VariableAccessUtils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -85,8 +86,23 @@ public final class HighlightControlFlowUtil {
try {
AllVariablesControlFlowPolicy policy = AllVariablesControlFlowPolicy.getInstance();
final ControlFlow controlFlow = ControlFlowFactory.getControlFlow(codeBlock, policy, ControlFlowOptions.NO_CONST_EVALUATE);
final PsiElement unreachableStatement = ControlFlowUtil.getUnreachableStatement(controlFlow);
PsiElement unreachableStatement = ControlFlowUtil.getUnreachableStatement(controlFlow);
if (unreachableStatement != null) {
if (unreachableStatement instanceof PsiCodeBlock && unreachableStatement.getParent() instanceof PsiBlockStatement) {
unreachableStatement = unreachableStatement.getParent();
}
if (unreachableStatement instanceof PsiStatement) {
PsiElement parent = unreachableStatement.getParent();
if (parent instanceof PsiWhileStatement || parent instanceof PsiForStatement) {
PsiExpression condition = ((PsiConditionalLoopStatement)parent).getCondition();
if (Boolean.FALSE.equals(ExpressionUtils.computeConstantExpression(condition))) {
HighlightInfo info = HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(condition)
.descriptionAndTooltip(JavaErrorBundle.message("unreachable.statement.false.condition")).create();
QuickFixAction.registerQuickFixAction(info, QUICK_FIX_FACTORY.createSimplifyBooleanFix(condition, false));
return info;
}
}
}
String description = JavaErrorBundle.message("unreachable.statement");
PsiElement keyword = null;
if (unreachableStatement instanceof PsiIfStatement ||
@@ -135,7 +151,7 @@ public final class HighlightControlFlowUtil {
return false;
}
else {
// instance field should be initialized at the end of the each constructor
// instance field should be initialized at the end of each constructor
final PsiMethod[] constructors = aClass.getConstructors();
if (constructors.length == 0) return false;
@@ -301,7 +317,7 @@ public final class HighlightControlFlowUtil {
topBlock = FileTypeUtils.isInServerPageFile(scope) && scope instanceof PsiFile ? scope : PsiUtil.getTopLevelEnclosingCodeBlock(expression, scope);
if (variable instanceof PsiField) {
// non final field already initialized with default value
// non-final field already initialized with default value
if (!ignoreFinality && !variable.hasModifierProperty(PsiModifier.FINAL)) return null;
// final field may be initialized in ctor or class initializer only
// if we're inside non-ctr method, skip it
@@ -163,25 +163,12 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
}
}
@Override
public void visitWhileStatement(PsiWhileStatement statement) {
checkLoopCondition(statement.getCondition());
}
@Override
public void visitDoWhileStatement(PsiDoWhileStatement statement) {
checkLoopCondition(statement.getCondition());
}
@Override
public void visitForStatement(PsiForStatement statement) {
checkLoopCondition(statement.getCondition());
}
private void checkLoopCondition(PsiExpression condition) {
condition = PsiUtil.skipParenthesizedExprDown(condition);
PsiExpression condition = PsiUtil.skipParenthesizedExprDown(statement.getCondition());
if (condition != null && condition.textMatches(PsiKeyword.FALSE)) {
holder.registerProblem(condition, JavaAnalysisBundle.message("dataflow.message.constant.no.ref", 0), createSimplifyBooleanExpressionFix(condition, false));
holder.registerProblem(condition, JavaAnalysisBundle.message("dataflow.message.constant.no.ref", 0),
createSimplifyBooleanExpressionFix(condition, false));
}
}
};
@@ -8,13 +8,14 @@ import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.codeInsight.intention.impl.BaseIntentionAction;
import com.intellij.codeInsight.intention.impl.SplitConditionUtil;
import com.intellij.codeInspection.CommonQuickFixBundle;
import com.intellij.codeInspection.LocalQuickFixOnPsiElement;
import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement;
import com.intellij.codeInspection.dataFlow.NullabilityProblemKind;
import com.intellij.codeInspection.util.IntentionFamilyName;
import com.intellij.codeInspection.util.IntentionName;
import com.intellij.java.JavaBundle;
import com.intellij.openapi.diagnostic.Attachment;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.text.StringUtil;
@@ -37,7 +38,7 @@ import org.jetbrains.annotations.Nullable;
import java.util.*;
public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement {
public class SimplifyBooleanExpressionFix extends LocalQuickFixAndIntentionActionOnPsiElement {
private static final Logger LOG = Logger.getInstance(SimplifyBooleanExpressionFix.class);
private final boolean mySubExpressionValue;
@@ -144,7 +145,8 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement {
}
@Override
public void invoke(@NotNull final Project project, @NotNull PsiFile file, @NotNull PsiElement startElement, @NotNull PsiElement endElement) {
public void invoke(@NotNull final Project project, @NotNull PsiFile file, @Nullable Editor editor,
@NotNull PsiElement startElement, @NotNull PsiElement endElement) {
if (!isAvailable()) return;
PsiExpression subExpression = getSubExpression();
if (subExpression == null) return;
@@ -216,7 +218,7 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement {
if (ifStatement == null) return null;
PsiExpression lastOperand = ArrayUtil.getLastElement(orChain.getOperands());
if (!PsiTreeUtil.isAncestor(lastOperand, subExpression, false)) return null;
orChain.replace(SplitConditionUtil.getLOperands(orChain, orChain.getTokenBeforeOperand(lastOperand)));
orChain.replace(SplitConditionUtil.getLOperands(orChain, Objects.requireNonNull(orChain.getTokenBeforeOperand(lastOperand))));
ControlFlowUtils.ensureElseBranch(ifStatement);
PsiBlockStatement elseBranch = (PsiBlockStatement)Objects.requireNonNull(ifStatement.getElseBranch());
PsiCodeBlock codeBlock = elseBranch.getCodeBlock();
@@ -1129,4 +1129,9 @@ public final class QuickFixFactoryImpl extends QuickFixFactory {
@NotNull PsiCaseLabelElement labelElement) {
return new MoveSwitchBranchUpFix(moveBeforeLabel, labelElement);
}
@Override
public @NotNull IntentionAction createSimplifyBooleanFix(@NotNull PsiExpression expression, boolean value) {
return new SimplifyBooleanExpressionFix(expression, value);
}
}
@@ -125,6 +125,7 @@ cannot.be.referenced.from.static.context=''{0}'' cannot be referenced from a sta
no.default.constructor.available=There is no default constructor available in ''{0}''
missing.return.statement=Missing return statement
unreachable.statement=Unreachable statement
unreachable.statement.false.condition=Loop condition is always false making the loop body unreachable
variable.not.initialized=Variable ''{0}'' might not have been initialized
variable.already.assigned=Variable ''{0}'' might already have been assigned to
variable.assigned.in.loop=Variable ''{0}'' might be assigned in loop
@@ -0,0 +1,15 @@
public class AlwaysFalseForLoop {
void test() {
int i = 0;
for(i++; <error descr="Loop condition is always false making the loop body unreachable">fa<caret>lse</error>; System.out.println("oops")) {
System.out.println("oops");
}
}
void test2() {
for(int j=0; <error descr="Loop condition is always false making the loop body unreachable">false</error>; j++) {
}
}
}
@@ -0,0 +1,13 @@
public class AlwaysFalseForLoop {
void test() {
int i = 0;
i++;
}
void test2() {
for(int j=0; false; j++) {
}
}
}
@@ -10,15 +10,15 @@ class Bar {
}
void m() {
while (T == "a") <error descr="Unreachable statement">{
while (<error descr="Loop condition is always false making the loop body unreachable">T == "a"</error>) {
f();
}</error>
}
}
void m01() {
while (T != "") <error descr="Unreachable statement">{
while (<error descr="Loop condition is always false making the loop body unreachable">T != ""</error>) {
f();
}</error>
}
}
void m1() {
@@ -29,9 +29,9 @@ class Bar {
}
void m2() {
while (T != T) <error descr="Unreachable statement">{
while (<error descr="Loop condition is always false making the loop body unreachable">T != T</error>) {
f();
}</error>
}
}
void m3() {
@@ -1,8 +1,9 @@
class Test {
public static void test() {
for(int a = 1; <warning descr="Condition is always false">fal<caret>se</warning>; System.out.println("Just anything here: will not be executed anyways")) <error descr="Unreachable statement">{
boolean c = false;
for(int a = 1; <warning descr="Condition 'c' is always 'false'"><caret>c</warning>; System.out.println("Just anything here: will not be executed anyways")) {
System.out.println("Hello");
}</error>
}
}
}
@@ -1,5 +1,6 @@
class Test {
public static void test() {
boolean c = false;
}
}
@@ -1,7 +1,11 @@
class Test {
public static void test() {
for(int i = <error descr="Cannot resolve method 'launchMissiles' in 'Test'">launchMissiles</error>(); (<warning descr="Condition is always false">fa<caret>lse</warning>);) {
for(int i = <error descr="Cannot resolve method 'launchMissiles' in 'Test'">launchMissiles</error>(); (false);) {
System.out.println("Hello");
}
boolean c = false;
for(int i = <error descr="Cannot resolve method 'launchMissiles' in 'Test'">launchMissiles</error>(); (<warning descr="Condition 'c' is always 'false'"><caret>c</warning>);) {
System.out.println("Hello");
}
int i = 1;
@@ -1,6 +1,10 @@
class Test {
public static void test() {
for(int i = launchMissiles(); (false);) {
System.out.println("Hello");
}
boolean c = false;
{
int i = launchMissiles();
}
@@ -1,8 +1,12 @@
class Test {
public static void test() {
while ((<warning descr="Condition is always false">fa<caret>lse</warning>)) <error descr="Unreachable statement">{
while (<error descr="Loop condition is always false making the loop body unreachable">(false)</error>) {
System.out.println();
}</error>
}
boolean c = false;
while ((<warning descr="Condition 'c' is always 'false'"><caret>c</warning>)) {
System.out.println();
}
}
}
@@ -1,5 +1,9 @@
class Test {
public static void test() {
while ((false)) {
System.out.println();
}
boolean c = false;
}
}
@@ -4,6 +4,7 @@ package com.intellij.java.codeInsight.daemon;
import com.intellij.JavaTestUtil;
import com.intellij.codeInsight.daemon.DaemonAnalyzerTestCase;
import com.intellij.codeInsight.daemon.impl.HighlightInfo;
import com.intellij.codeInsight.intention.IntentionAction;
import com.intellij.codeInspection.redundantCast.RedundantCastInspection;
import com.intellij.lang.annotation.HighlightSeverity;
import com.intellij.lang.java.JavaDocumentationProvider;
@@ -246,6 +247,13 @@ public class LightAdvHighlightingFixtureTest extends LightJavaCodeInsightFixture
myFixture.checkHighlighting();
}
public void testAlwaysFalseForLoop() {
doTest();
IntentionAction action = myFixture.findSingleIntention("Remove 'for' statement");
myFixture.launchAction(action);
myFixture.checkResultByFile(getTestName(false) + "_after.java");
}
private void doTest() {
myFixture.configureByFile(getTestName(false) + ".java");
myFixture.checkHighlighting();