Java inspection: In "Add Braces" and "Remove Braces" inspections don't offer the fix on an if/loop body if it contains another if/loop to avoid ambiguity. Tests added. (IDEA-157727)

This commit is contained in:
Pavel Dolgov
2016-07-15 19:39:28 +03:00
parent aa1f56a945
commit a310b65b13
24 changed files with 506 additions and 32 deletions
@@ -1334,7 +1334,7 @@ chained.method.call.ignore.this.super.option=Ignore chained method calls in 'thi
introduce.variable.quickfix=Introduce variable
introduce.variable.may.change.semantics.quickfix=Introduce variable (may change semantics)
flip.comparison.quickfix=Flip comparison
control.flow.statement.without.braces.add.quickfix=Add braces
control.flow.statement.without.braces.add.quickfix=Add braces to statement
control.flow.statement.without.braces.message=Add braces to ''{0}'' statement
extends.object.remove.quickfix=Remove redundant 'extends Object'
implicit.call.to.super.ignore.option=Ignore for direct subclasses of 'java.lang.Object'
@@ -2204,4 +2204,6 @@ replace.equality.with.equals.descriptor=Replace ''{0}'' with ''{1}equals()''
replace.equality.with.safe.equals.name=Replace Equality with Safe Equals
replace.equality.with.safe.equals.descriptor=Replace ''{0}'' with safe ''{1}equals()''
single.statement.in.block.name=Code Block Contains Single Statement
single.statement.in.block.descriptor=Remove braces from ''{0}'' statement
single.statement.in.block.descriptor=''{0}'' contains single statement
single.statement.in.block.quickfix=Remove braces from ''{0}'' statement
single.statement.in.block.family.quickfix=Remove Braces From Statement
@@ -19,11 +19,14 @@ import com.intellij.codeHighlighting.HighlightDisplayLevel;
import com.intellij.codeInsight.daemon.HighlightDisplayKey;
import com.intellij.codeInspection.InspectionProfile;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Pair;
import com.intellij.profile.codeInspection.InspectionProjectProfileManager;
import com.intellij.psi.*;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
public abstract class ControlFlowStatementVisitorBase extends BaseInspectionVisitor {
private final HighlightDisplayKey myKey;
@@ -36,33 +39,37 @@ public abstract class ControlFlowStatementVisitorBase extends BaseInspectionVisi
@Override
public void visitForeachStatement(PsiForeachStatement statement) {
super.visitForeachStatement(statement);
if (isApplicable(statement.getBody())) {
registerKeywordOrStatementError(statement, PsiKeyword.FOR);
final PsiStatement body = statement.getBody();
if (isApplicable(body)) {
registerLoopStatementErrors(statement, body, PsiKeyword.FOR);
}
}
@Override
public void visitForStatement(PsiForStatement statement) {
super.visitForStatement(statement);
if (isApplicable(statement.getBody())) {
registerKeywordOrStatementError(statement, PsiKeyword.FOR);
final PsiStatement body = statement.getBody();
if (isApplicable(body)) {
registerLoopStatementErrors(statement, body, PsiKeyword.FOR);
}
}
@Override
public void visitWhileStatement(PsiWhileStatement statement) {
super.visitWhileStatement(statement);
if (isApplicable(statement.getBody())) {
registerKeywordOrStatementError(statement, PsiKeyword.WHILE);
final PsiStatement body = statement.getBody();
if (isApplicable(body)) {
registerLoopStatementErrors(statement, body, PsiKeyword.WHILE);
}
}
@Override
public void visitDoWhileStatement(PsiDoWhileStatement statement) {
super.visitDoWhileStatement(statement);
if (isApplicable(statement.getBody())) {
registerKeywordOrStatementError(statement, PsiKeyword.DO);
final PsiStatement body = statement.getBody();
if (isApplicable(body)) {
registerLoopStatementErrors(statement, body, PsiKeyword.DO);
}
}
@@ -71,34 +78,65 @@ public abstract class ControlFlowStatementVisitorBase extends BaseInspectionVisi
super.visitIfStatement(statement);
final PsiStatement thenBranch = statement.getThenBranch();
if (isApplicable(thenBranch)) {
registerKeywordOrStatementError(statement.getFirstChild(), thenBranch, PsiKeyword.IF);
registerControlFlowStatementErrors(statement.getFirstChild(), thenBranch.getLastChild(), thenBranch, PsiKeyword.IF);
}
final PsiStatement elseBranch = statement.getElseBranch();
if (isApplicable(elseBranch)) {
registerKeywordOrStatementError(statement.getElseElement(), elseBranch, PsiKeyword.ELSE);
registerControlFlowStatementErrors(statement.getElseElement(), elseBranch.getLastChild(), elseBranch, PsiKeyword.ELSE);
}
}
@Contract("null->false")
protected abstract boolean isApplicable(PsiStatement body);
private void registerKeywordOrStatementError(PsiStatement statement, String text) {
boolean highlightOnlyKeyword = isHighlightOnlyKeyword(statement);
if (highlightOnlyKeyword) {
registerStatementError(statement, text);
}
else {
registerError(statement, text);
}
@Nullable
protected abstract Pair<PsiElement, PsiElement> getOmittedBodyBounds(PsiStatement body);
private void registerLoopStatementErrors(@NotNull PsiLoopStatement statement, @NotNull PsiStatement body, @NotNull String keywordText) {
registerControlFlowStatementErrors(statement.getFirstChild(), statement.getLastChild(), body, keywordText);
}
private void registerKeywordOrStatementError(PsiElement keyword, PsiStatement body, String text) {
private void registerControlFlowStatementErrors(@Nullable PsiElement rangeStart,
@Nullable PsiElement rangeEnd,
@NotNull PsiStatement body,
@NotNull String keywordText) {
boolean highlightOnlyKeyword = isHighlightOnlyKeyword(body);
if (highlightOnlyKeyword) {
registerError(keyword != null ? keyword : body, text);
if (rangeStart != null) {
registerError(rangeStart, keywordText);
}
return;
}
else {
registerErrorAtRange(keyword != null ? keyword : body, body, text);
final Pair<PsiElement, PsiElement> omittedBodyBounds = getOmittedBodyBounds(body);
if (omittedBodyBounds == null) {
if (rangeStart != null && rangeEnd != null) {
registerErrorAtRange(rangeStart, rangeEnd, keywordText);
}
return;
}
if (rangeStart != null) {
final PsiElement beforeOmitted = omittedBodyBounds.getFirst();
final PsiElement endOfHighlight = beforeOmitted != null ? beforeOmitted : rangeStart;
registerErrorAtRange(rangeStart, endOfHighlight, keywordText);
}
final PsiElement afterOmitted = omittedBodyBounds.getSecond();
if (afterOmitted != null) {
PsiElement endOfHighlight = afterOmitted;
if (rangeEnd != null && rangeEnd != afterOmitted) {
if (afterOmitted.getParent() == rangeEnd) {
final PsiElement rangeEndLastChild = rangeEnd.getLastChild();
if (rangeEndLastChild != null) {
endOfHighlight = rangeEndLastChild;
}
}
else {
endOfHighlight = rangeEnd;
}
}
registerErrorAtRange(afterOmitted, endOfHighlight, keywordText);
}
}
@@ -17,7 +17,9 @@ package com.siyeh.ig.style;
import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Pair;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.IncorrectOperationException;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
@@ -26,6 +28,7 @@ import com.siyeh.ig.InspectionGadgetsFix;
import com.siyeh.ig.PsiReplacementUtil;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
public class ControlFlowStatementWithoutBracesInspection
extends BaseInspection {
@@ -139,5 +142,18 @@ public class ControlFlowStatementWithoutBracesInspection
protected boolean isApplicable(PsiStatement body) {
return body != null && !(body instanceof PsiBlockStatement);
}
@Nullable
@Override
protected Pair<PsiElement, PsiElement> getOmittedBodyBounds(PsiStatement body) {
if (body instanceof PsiLoopStatement || body instanceof PsiIfStatement) {
final PsiElement lastChild = body.getLastChild();
return Pair.create(PsiTreeUtil.skipSiblingsBackward(body, PsiWhiteSpace.class, PsiComment.class),
lastChild instanceof PsiJavaToken && ((PsiJavaToken)lastChild).getTokenType() == JavaTokenType.SEMICOLON
? lastChild
: null);
}
return null;
}
}
}
@@ -17,6 +17,7 @@ package com.siyeh.ig.style;
import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Pair;
import com.intellij.psi.*;
import com.intellij.psi.util.FileTypeUtils;
import com.siyeh.InspectionGadgetsBundle;
@@ -98,10 +99,9 @@ public class SingleStatementInBlockInspection extends BaseInspection {
@Override
protected boolean isApplicable(PsiStatement body) {
if (body instanceof PsiBlockStatement) {
final PsiBlockStatement statement = (PsiBlockStatement)body;
final PsiStatement[] statements = statement.getCodeBlock().getStatements();
final PsiStatement[] statements = ((PsiBlockStatement)body).getCodeBlock().getStatements();
if (statements.length == 1 && !(statements[0] instanceof PsiDeclarationStatement)) {
final PsiFile file = statement.getContainingFile();
final PsiFile file = body.getContainingFile();
//this inspection doesn't work in JSP files, as it can't tell about tags
// inside the braces
if (!FileTypeUtils.isInServerPageFile(file)) {
@@ -111,6 +111,22 @@ public class SingleStatementInBlockInspection extends BaseInspection {
}
return false;
}
@Nullable
@Override
protected Pair<PsiElement, PsiElement> getOmittedBodyBounds(PsiStatement body) {
if (body instanceof PsiBlockStatement) {
final PsiCodeBlock codeBlock = ((PsiBlockStatement)body).getCodeBlock();
final PsiStatement[] statements = codeBlock.getStatements();
if (statements.length == 1) {
final PsiStatement statement = statements[0];
if (statement instanceof PsiLoopStatement || statement instanceof PsiIfStatement) {
return Pair.create(codeBlock.getLBrace(), codeBlock.getRBrace());
}
}
}
return null;
}
}
private static class SingleStatementInBlockFix extends InspectionGadgetsFix {
@@ -124,14 +140,14 @@ public class SingleStatementInBlockInspection extends BaseInspection {
@NotNull
@Override
public String getName() {
return InspectionGadgetsBundle.message("single.statement.in.block.descriptor", myKeywordText);
return InspectionGadgetsBundle.message("single.statement.in.block.quickfix", myKeywordText);
}
@Nls
@NotNull
@Override
public String getFamilyName() {
return InspectionGadgetsBundle.message("single.statement.in.block.name");
return InspectionGadgetsBundle.message("single.statement.in.block.family.quickfix");
}
@Override
@@ -0,0 +1,13 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else {
System.out.println(0);
}
else System.out.println("no");
}
}
@@ -0,0 +1,11 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else System.out.println(0);<caret>
else System.out.println("no");
}
}
@@ -0,0 +1,12 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++) {
System.out.println(arg.charAt(i));
}
else System.out.println(0);
else System.out.println("no");
}
}
@@ -0,0 +1,11 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)<caret>
System.out.println(arg.charAt(i));
else System.out.println(0);
else System.out.println("no");
}
}
@@ -0,0 +1,11 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for (String arg : a)
if (arg.length() > 1) {
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
} else System.out.println(0);
else System.out.println("no");
}
}
@@ -0,0 +1,11 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for (String arg : a)
if (arg.length() > 1)<caret>
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else System.out.println(0);
else System.out.println("no");
}
}
@@ -0,0 +1,13 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else System.out.println(0);
else {
System.out.println("no");
}
}
}
@@ -0,0 +1,11 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else System.out.println(0);
else<caret> System.out.println("no");
}
}
@@ -0,0 +1,12 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for (String arg : a) {
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else System.out.println(0);
}
else System.out.println("no");
}
}
@@ -0,0 +1,11 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for<caret> (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else System.out.println(0);
else System.out.println("no");
}
}
@@ -0,0 +1,11 @@
class X {
void ff(String[] a) {
if (a.length != 0) {
for (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else System.out.println(0);
} else System.out.println("no");
}
}
@@ -0,0 +1,11 @@
class X {
void ff(String[] a) {
if<caret> (a.length != 0)
for (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else System.out.println(0);
else System.out.println("no");
}
}
@@ -0,0 +1,12 @@
class X {
void ff(String[] a) {
if (a.length != 0)
for (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else System.out.println(0);
else System.out.println("no");
<caret>
}
}
@@ -0,0 +1,63 @@
class T {
void f(String[] a) {
for (String s : a) {
System.out.println(s);
}
if (a.length == 0) {
System.out.println("no");
} else {
System.out.println(a.length);
}
if (a.length == 0) {
System.out.println("no");
}
if (a.length == 0) {
} else {
System.out.println(a.length);
}
for (int i = 0; i < a.length; i++) {
System.out.println(a[i]);
}
int j = 0;
do {
System.out.println(a[j++]);
}
while (j < a.length);
int k = 0;
while (k < a.length) {
System.out.println(a[k++]);
}
}
void ff(String[] a) {
if (a.length != 0) {
for (String arg : a) {
if (arg.length() > 1) {
for (int i = 0; i < arg.length(); i++) {
System.out.println(arg.charAt(i));
}
} else {
System.out.println(0);
}
}
} else {
System.out.println("no");
}
}
void decl(String[] a) {
if (a.length == 1) {
String t = a[0];
}
for (int i = 0; i < a.length; i++) {
String t = a[i];
}
}
}
@@ -0,0 +1,102 @@
<?xml version="1.0" encoding="UTF-8"?>
<problems>
<problem>
<file>SingleStatement.java</file>
<line>4</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'for' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>8</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'if' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>10</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'else' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>14</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'if' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>19</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'else' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>23</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'for' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>28</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'do' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>34</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'while' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>40</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'if' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>41</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'for' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>42</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'if' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>43</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'for' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>46</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'else' contains single statement</description>
</problem>
<problem>
<file>SingleStatement.java</file>
<line>50</line>
<problem_class>Code Block Contains Single Statement</problem_class>
<description>'else' contains single statement</description>
</problem>
</problems>
@@ -26,4 +26,14 @@ class T {
} else
System.out.println(a.length);
}
void ff(String[] a) {
if (a.length != 0)
for (String arg : a)
if (arg.length() > 1)
for (int i = 0; i < arg.length(); i++)
System.out.println(arg.charAt(i));
else System.out.println(0);
else System.out.println("no");
}
}
@@ -57,4 +57,46 @@
<description>&lt;code&gt;else&lt;/code&gt; without braces #loc</description>
</problem>
<problem>
<file>ControlFlowStatements.java</file>
<line>31</line>
<problem_class>Control flow statement without braces</problem_class>
<description>&lt;code&gt;if&lt;/code&gt; without braces #loc</description>
</problem>
<problem>
<file>ControlFlowStatements.java</file>
<line>32</line>
<problem_class>Control flow statement without braces</problem_class>
<description>&lt;code&gt;for&lt;/code&gt; without braces #loc</description>
</problem>
<problem>
<file>ControlFlowStatements.java</file>
<line>33</line>
<problem_class>Control flow statement without braces</problem_class>
<description>&lt;code&gt;if&lt;/code&gt; without braces #loc</description>
</problem>
<problem>
<file>ControlFlowStatements.java</file>
<line>34</line>
<problem_class>Control flow statement without braces</problem_class>
<description>&lt;code&gt;for&lt;/code&gt; without braces #loc</description>
</problem>
<problem>
<file>ControlFlowStatements.java</file>
<line>36</line>
<problem_class>Control flow statement without braces</problem_class>
<description>&lt;code&gt;else&lt;/code&gt; without braces #loc</description>
</problem>
<problem>
<file>ControlFlowStatements.java</file>
<line>37</line>
<problem_class>Control flow statement without braces</problem_class>
<description>&lt;code&gt;else&lt;/code&gt; without braces #loc</description>
</problem>
</problems>
@@ -47,6 +47,14 @@ public class ControlFlowStatementWithoutBracesFixTest extends IGQuickFixesTestCa
public void testWhile() { doTest("while"); }
public void testWhileOutside() { assertQuickfixNotAvailable(getMessagePrefix()); }
public void testLadderInnerElse() { doTest("else"); }
public void testLadderInnerFor() { doTest("for"); }
public void testLadderInnerIf() { doTest("if"); }
public void testLadderOuterElse() { doTest("else"); }
public void testLadderOuterFor() { doTest("for"); }
public void testLadderOuterIf() { doTest("if"); }
public void testLadderOutside() { assertQuickfixNotAvailable(getMessagePrefix()); }
@Override
protected void setUp() throws Exception {
super.setUp();
@@ -50,11 +50,11 @@ public class SingleStatementInBlockFixTest extends IGQuickFixesTestCase {
}
private static String getMessage(String keyword) {
return InspectionGadgetsBundle.message("single.statement.in.block.descriptor", keyword);
return InspectionGadgetsBundle.message("single.statement.in.block.quickfix", keyword);
}
private static String getMessagePrefix() {
final String message = InspectionGadgetsBundle.message("single.statement.in.block.descriptor", "@");
final String message = InspectionGadgetsBundle.message("single.statement.in.block.quickfix", "@");
final int index = message.indexOf("@");
if (index >= 0) return message.substring(0, index);
return message;
@@ -0,0 +1,27 @@
/*
* Copyright 2000-2016 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.style;
import com.siyeh.ig.IGInspectionTestCase;
/**
* @author Pavel.Dolgov
*/
public class SingleStatementInBlockInspectionTest extends IGInspectionTestCase {
public void test() {
doTest("com/siyeh/igtest/style/single_statement_block", new SingleStatementInBlockInspection());
}
}