MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-04-19 04:51:24 +07:00
Code Issues Packages Projects Releases Wiki Activity

SwitchStatementWithTooFewBranches: warn on switch expression; provide fix in simple case (single default)

Browse Source
This commit is contained in:
Tagir Valeev
2018-12-07 12:46:07 +07:00
parent b81af4a9d6
commit 53b426dfae
22 changed files with 104 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/UnwrapSwitchLabelFix.java
View File

@@ -10,6 +10,7 @@ import com.intellij.psi.*;
import com.intellij.psi.impl.PsiImplUtil;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.controlflow.SwitchStatementWithTooFewBranchesInspection.UnwrapSwitchStatementFix;
import com.siyeh.ig.psiutils.BreakConverter;
import com.siyeh.ig.psiutils.CommentTracker;
import org.jetbrains.annotations.Nls;
@@ -55,12 +56,7 @@ public class UnwrapSwitchLabelFix implements LocalQuickFix {
converter.process();
unwrapStatement(labelStatement, (PsiSwitchStatement)block);
} else {
if (labelStatement instanceof PsiSwitchLabeledRuleStatement) {
PsiSwitchLabeledRuleStatement ruleStatement = (PsiSwitchLabeledRuleStatement)labelStatement;
if (ruleStatement.getBody() instanceof PsiExpressionStatement) {
new CommentTracker().replaceAndRestoreComments(block, ((PsiExpressionStatement)ruleStatement.getBody()).getExpression());
}
}
UnwrapSwitchStatementFix.unwrapExpression((PsiSwitchExpression)block);
}
}

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/afterBreakInIf.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
if (c == 'a') {

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/afterBreakInIf2.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
if (c == 'a') {

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/afterBreakInIf3.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
if (c == 'a') {

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/afterBreakLast.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
if (c == 'a') {

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/afterBreakOuterLoop.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
for(int i=0; i<10; i++) {

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/afterConflict.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
{

6
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/afterExpression.java Normal file
View File

@@ -0,0 +1,6 @@
// "Unwrap 'switch'" "true"
class X {
String test(int i) {
return "foo";
}
}

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/afterSimple.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
return "foo";

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/beforeBreakInIf.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
s<caret>witch (c) {

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/beforeBreakInIf2.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
s<caret>witch (c) {

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/beforeBreakInIf3.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
s<caret>witch (c) {

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/beforeBreakLast.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
s<caret>witch (c) {

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/beforeBreakOuterLoop.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
for(int i=0; i<10; i++)

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/beforeBreakOuterLoopNotLast.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "false"
// "Unwrap 'switch'" "false"
class X {
String test(char c) {
for(int i=0; i<10; i++) {

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/beforeConflict.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
s<caret>witch (c) {

6
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/beforeExpression.java Normal file
View File

@@ -0,0 +1,6 @@
// "Unwrap 'switch'" "true"
class X {
String test(int i) {
return switch<caret>(i) { default -> "foo"; };
}
}

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/switchDefault/beforeSimple.java
View File

@@ -1,4 +1,4 @@
// "Unwrap 'switch' statement" "true"
// "Unwrap 'switch'" "true"
class X {
String test(char c) {
s<caret>witch (c) {

1
platform/platform-resources-en/src/messages/CommonQuickFixBundle.properties
View File

@@ -4,6 +4,7 @@
#
# Every fix name constant in this file should start with "fix.", then verb in infinitive which describes the action.
# Constants should be grouped by this verb.
fix.unwrap=Unwrap ''{0}''
fix.unwrap.statement=Unwrap ''{0}'' statement
fix.remove=Remove ''{0}''

2
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties
View File

@@ -1253,6 +1253,8 @@ switch.statement.density.problem.descriptor=<code>#ref</code> has too low of a b
switch.statement.with.too.few.branches.min.option=Minimum number of branches:
switch.statement.with.too.few.branches.problem.descriptor=''switch'' statement has too few case labels ({0}), and should probably be replaced with an ''if'' statement #loc
switch.statement.with.single.default.message='switch' statement has only 'default' case
switch.expression.with.too.few.branches.problem.descriptor=''switch'' expression has too few case labels ({0}), and should probably be replaced with an ''if'' statement or conditional operator #loc
switch.expression.with.single.default.message='switch' expression has only 'default' case
switch.statement.without.default.ignore.option=Ignore if all cases of an enum type are covered
unnecessary.label.remove.quickfix=Remove label
unnecessary.return.problem.descriptor=<code>#ref</code> is unnecessary as the last statement in a 'void' method #loc

81
plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/SwitchStatementWithTooFewBranchesInspection.java
View File

@@ -26,6 +26,7 @@ import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.InspectionGadgetsFix;
import com.siyeh.ig.psiutils.CommentTracker;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -55,17 +56,22 @@ public class SwitchStatementWithTooFewBranchesInspection extends BaseInspection
@NotNull
protected String buildErrorString(Object... infos) {
final Integer branchCount = (Integer)infos[0];
if (branchCount == 0) {
return InspectionGadgetsBundle.message("switch.statement.with.single.default.message");
final PsiSwitchBlock block = (PsiSwitchBlock)infos[1];
if (block instanceof PsiSwitchExpression) {
return branchCount == 0
? InspectionGadgetsBundle.message("switch.expression.with.single.default.message")
: InspectionGadgetsBundle.message("switch.expression.with.too.few.branches.problem.descriptor", branchCount);
}
return InspectionGadgetsBundle.message("switch.statement.with.too.few.branches.problem.descriptor", branchCount);
return branchCount == 0
? InspectionGadgetsBundle.message("switch.statement.with.single.default.message")
: InspectionGadgetsBundle.message("switch.statement.with.too.few.branches.problem.descriptor", branchCount);
}
@Nullable
@Override
protected InspectionGadgetsFix buildFix(Object... infos) {
final Integer branchCount = (Integer)infos[0];
return (Boolean)infos[1] ? new UnwrapSwitchStatementFix(branchCount) : null;
return (Boolean)infos[2] ? new UnwrapSwitchStatementFix(branchCount) : null;
}
@Override
@@ -74,11 +80,24 @@ public class SwitchStatementWithTooFewBranchesInspection extends BaseInspection
}
private class SwitchStatementWithTooFewBranchesVisitor extends BaseInspectionVisitor {
@Override
public void visitSwitchExpression(PsiSwitchExpression expression) {
Object[] infos = processSwitch(expression);
if (infos == null) return;
registerError(expression.getFirstChild(), infos);
}
@Override
public void visitSwitchStatement(@NotNull PsiSwitchStatement statement) {
final PsiCodeBlock body = statement.getBody();
if (body == null) return;
Object[] infos = processSwitch(statement);
if (infos == null) return;
registerStatementError(statement, infos);
}
@Nullable
public Object[] processSwitch(@NotNull PsiSwitchBlock block) {
final PsiCodeBlock body = block.getBody();
if (body == null) return null;
int branches = 0;
boolean defaultFound = false;
for (final PsiSwitchLabelStatementBase child : PsiTreeUtil.getChildrenOfTypeAsList(body, PsiSwitchLabelStatementBase.class)) {
@@ -89,23 +108,37 @@ public class SwitchStatementWithTooFewBranchesInspection extends BaseInspection
PsiExpressionList values = child.getCaseValues();
if (values == null) {
// Erroneous switch: compilation error is reported instead
return;
return null;
}
branches += values.getExpressionCount();
if (branches >= m_limit) {
return;
return null;
}
}
}
if (branches == 0 && !defaultFound) {
// Empty switch is reported by another inspection
return;
return null;
}
registerStatementError(statement, Integer.valueOf(branches), ConvertSwitchToIfIntention.isAvailable(statement));
boolean fixIsAvailable;
if (block instanceof PsiSwitchStatement) {
fixIsAvailable = ConvertSwitchToIfIntention.isAvailable((PsiSwitchStatement)block);
}
else {
PsiStatement[] statements = body.getStatements();
if (statements.length == 1 && statements[0] instanceof PsiSwitchLabeledRuleStatement) {
PsiSwitchLabeledRuleStatement statement = (PsiSwitchLabeledRuleStatement)statements[0];
fixIsAvailable = statement.isDefaultCase() && statement.getBody() instanceof PsiExpressionStatement;
}
else {
fixIsAvailable = false;
}
}
return new Object[]{Integer.valueOf(branches), block, fixIsAvailable};
}
}
private static class UnwrapSwitchStatementFix extends InspectionGadgetsFix {
public static class UnwrapSwitchStatementFix extends InspectionGadgetsFix {
int myBranchCount;
private UnwrapSwitchStatementFix(int branchCount) {
@@ -123,14 +156,32 @@ public class SwitchStatementWithTooFewBranchesInspection extends BaseInspection
@NotNull
@Override
public String getFamilyName() {
return CommonQuickFixBundle.message("fix.unwrap.statement", PsiKeyword.SWITCH);
return CommonQuickFixBundle.message("fix.unwrap", PsiKeyword.SWITCH);
}
@Override
public void doFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
PsiSwitchStatement statement = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiSwitchStatement.class);
if (statement == null) return;
ConvertSwitchToIfIntention.doProcessIntention(statement);
PsiSwitchBlock block = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiSwitchBlock.class);
if (block instanceof PsiSwitchStatement) {
ConvertSwitchToIfIntention.doProcessIntention((PsiSwitchStatement)block);
} else if (block instanceof PsiSwitchExpression) {
unwrapExpression((PsiSwitchExpression)block);
}
}
/**
* Unwraps switch expression if it consists of single expression-branch; does nothing otherwise
* @param switchExpression expression to unwrap
*/
public static void unwrapExpression(PsiSwitchExpression switchExpression) {
PsiCodeBlock body = switchExpression.getBody();
if (body == null) return;
PsiStatement[] statements = body.getStatements();
if (statements.length != 1 || !(statements[0] instanceof PsiSwitchLabeledRuleStatement)) return;
PsiSwitchLabeledRuleStatement rule = (PsiSwitchLabeledRuleStatement)statements[0];
PsiStatement ruleBody = rule.getBody();
if (!(ruleBody instanceof PsiExpressionStatement)) return;
new CommentTracker().replaceAndRestoreComments(switchExpression, ((PsiExpressionStatement)ruleBody).getExpression());
}
}
}

6
plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/switch_statement_with_too_few_branches/SwitchStatementWithTooFewBranches.java
View File

@@ -52,6 +52,12 @@ class SwitchStatementWithTooFewBranches {
<warning descr="'switch' statement has only 'default' case">switch</warning> (i) {
default -> {}
}
System.out.println(<warning descr="'switch' expression has only 'default' case">switch</warning>(i) {default -> "foo";});
System.out.println(<warning descr="'switch' expression has too few case labels (1), and should probably be replaced with an 'if' statement or conditional operator">switch</warning>(i) {case 1 -> "bar"; default -> "foo";});
System.out.println(switch(i) {case 1,2 -> "bar"; default -> "foo";});
System.out.println(switch(i) {case 1 -> "bar"; case 2 -> "baz"; default -> "foo";});
switch (i)<EOLError descr="'{' expected"></EOLError>
}
}