IG: use some utility methods to reduce code in 'switch' inspections

This commit is contained in:
Bas Leijdekkers
2017-09-26 19:49:34 +02:00
parent 036ef00488
commit fc49f7a9b2
9 changed files with 135 additions and 137 deletions
@@ -126,7 +126,7 @@ null.argument.to.var.arg.method.problem.descriptor=Confusing argument <code>#ref
primitive.array.argument.to.var.arg.method.display.name=Confusing primitive array argument to varargs method
primitive.array.argument.to.var.arg.method.problem.descriptor=Confusing primitive array argument to varargs method #loc
object.comparison.display.name=Object comparison using '==', instead of 'equals()'
object.comparison.enumerated.ignore.option=Ignore '==' between enumerated types
object.comparison.enumerated.ignore.option=Ignore '==' between enum variables
object.comparison.klass.ignore.option=Ignore '==' between final class types without 'equals()' implementation
object.comparison.problem.description=Object values are compared using <code>#ref</code>, not 'equals()' #loc
equality.to.equals.quickfix=Replace '==' with 'equals()'
@@ -1141,7 +1141,7 @@ redundant.else.unwrap.quickfix=Remove redundant 'else'
constant.conditional.expression.problem.descriptor=<code>#ref</code> can be simplified to ''{0}'' #loc
constant.conditional.expression.simplify.quickfix=Simplify
constant.conditional.expression.simplify.quickfix.sideEffect=Extract side effects and simplify
enum.switch.statement.which.misses.cases.problem.descriptor=<code>#ref</code> statement on enumerated type ''{0}'' misses cases #loc
enum.switch.statement.which.misses.cases.problem.descriptor=<code>#ref</code> statement on enum type ''{0}'' misses cases #loc
for.loop.replaceable.by.while.ignore.option=Ignore 'infinite' for loops without conditions
for.loop.replaceable.by.while.replace.quickfix=Replace with 'while'
for.loop.with.missing.component.problem.descriptor1=<code>#ref</code> statement lacks initializer #loc
@@ -1285,7 +1285,7 @@ switch.statement.density.min.option=Minimum density of branches: %
switch.statement.density.problem.descriptor=<code>#ref</code> has too low of a branch density ({0}%) #loc
switch.statement.with.too.few.branches.min.option=Minimum number of branches:
switch.statement.with.too.few.branches.problem.descriptor=<code>#ref</code> has too few branches ({0}), and should probably be replaced with an ''if'' statement #loc
switch.statement.without.default.ignore.option=Ignore if all cases of an enumerated type are covered
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
unnecessary.return.constructor.problem.descriptor=<code>#ref</code> is unnecessary as the last statement in a constructor #loc
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2011 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2017 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -15,57 +15,45 @@
*/
package com.siyeh.ig.controlflow;
import com.intellij.psi.*;
import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiEnumConstant;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.PsiSwitchStatement;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.SwitchUtils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import javax.swing.*;
public class EnumSwitchStatementWhichMissesCasesInspection
extends BaseInspection {
public class EnumSwitchStatementWhichMissesCasesInspection extends BaseInspection {
@Override
@NotNull
public String getDisplayName() {
return InspectionGadgetsBundle.message(
"enum.switch.statement.which.misses.cases.display.name");
}
/**
* @noinspection PublicField
*/
@SuppressWarnings("PublicField")
public boolean ignoreSwitchStatementsWithDefault = false;
@Override
@NotNull
public String getDisplayName() {
return InspectionGadgetsBundle.message("enum.switch.statement.which.misses.cases.display.name");
}
@Override
@NotNull
public String buildErrorString(Object... infos) {
final PsiSwitchStatement switchStatement =
(PsiSwitchStatement)infos[0];
assert switchStatement != null;
final PsiExpression switchStatementExpression =
switchStatement.getExpression();
assert switchStatementExpression != null;
final PsiType switchStatementType =
switchStatementExpression.getType();
assert switchStatementType != null;
final String switchStatementTypeText =
switchStatementType.getPresentableText();
return InspectionGadgetsBundle.message(
"enum.switch.statement.which.misses.cases.problem.descriptor",
switchStatementTypeText);
final String enumName = (String)infos[0];
return InspectionGadgetsBundle.message("enum.switch.statement.which.misses.cases.problem.descriptor", enumName);
}
@Override
@Nullable
public JComponent createOptionsPanel() {
return new SingleCheckboxOptionsPanel(
InspectionGadgetsBundle.message(
"enum.switch.statement.which.misses.cases.option"),
this, "ignoreSwitchStatementsWithDefault");
return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("enum.switch.statement.which.misses.cases.option"),
this, "ignoreSwitchStatementsWithDefault");
}
@Override
@@ -73,61 +61,27 @@ public class EnumSwitchStatementWhichMissesCasesInspection
return new EnumSwitchStatementWhichMissesCasesVisitor();
}
private class EnumSwitchStatementWhichMissesCasesVisitor
extends BaseInspectionVisitor {
private class EnumSwitchStatementWhichMissesCasesVisitor extends BaseInspectionVisitor {
@Override
public void visitSwitchStatement(
@NotNull PsiSwitchStatement statement) {
public void visitSwitchStatement(@NotNull PsiSwitchStatement statement) {
super.visitSwitchStatement(statement);
if (!switchStatementMissingCases(statement)) {
return;
}
registerStatementError(statement, statement);
}
private boolean switchStatementMissingCases(
PsiSwitchStatement statement) {
final PsiExpression expression = statement.getExpression();
if (expression == null) {
return false;
return;
}
final PsiType type = expression.getType();
if (!(type instanceof PsiClassType)) {
return false;
}
final PsiClassType classType = (PsiClassType)type;
final PsiClass aClass = classType.resolve();
final PsiClass aClass = PsiUtil.resolveClassInClassTypeOnly(expression.getType());
if (aClass == null || !aClass.isEnum()) {
return false;
return;
}
final PsiCodeBlock body = statement.getBody();
if (body == null) {
return false;
final int count = SwitchUtils.calculateBranchCount(statement);
if (ignoreSwitchStatementsWithDefault && count < 0) {
return;
}
final PsiStatement[] statements = body.getStatements();
int numCases = 0;
for (final PsiStatement child : statements) {
if (child instanceof PsiSwitchLabelStatement) {
final PsiSwitchLabelStatement switchLabelStatement =
(PsiSwitchLabelStatement)child;
if (!switchLabelStatement.isDefaultCase()) {
numCases++;
}
else if (ignoreSwitchStatementsWithDefault) {
return false;
}
}
if (count == 0 || ControlFlowUtils.hasChildrenOfTypeCount(aClass, Math.abs(count), PsiEnumConstant.class)) {
return;
}
final PsiField[] fields = aClass.getFields();
int numEnums = 0;
for (final PsiField field : fields) {
if (!(field instanceof PsiEnumConstant)) {
continue;
}
numEnums++;
}
return numEnums != numCases;
registerStatementError(statement, aClass.getQualifiedName());
}
}
}
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2013 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2017 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -67,12 +67,12 @@ public class SwitchStatementDensityInspection extends BaseInspection {
if (branchCount == 0) {
return;
}
final double density = calculateDensity(body, branchCount);
final double density = calculateDensity(body, (branchCount < 0) ? -branchCount + 1 : branchCount);
final int intDensity = (int)(density * 100.0);
if (intDensity > m_limit) {
return;
}
registerStatementError(statement, intDensity);
registerStatementError(statement, Integer.valueOf(intDensity));
}
private double calculateDensity(@NotNull PsiCodeBlock body, int branchCount) {
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2013 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2017 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@
package com.siyeh.ig.controlflow;
import com.intellij.codeInspection.ui.SingleIntegerFieldOptionsPanel;
import com.intellij.psi.PsiCodeBlock;
import com.intellij.psi.PsiSwitchStatement;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
@@ -61,18 +60,15 @@ public class SwitchStatementWithTooFewBranchesInspection extends BaseInspection
@Override
public void visitSwitchStatement(@NotNull PsiSwitchStatement statement) {
final PsiCodeBlock body = statement.getBody();
if (body == null) {
return;
}
final int branchCount = SwitchUtils.calculateBranchCount(statement);
if (branchCount == 0) {
return; // // do not warn when no switch branches are present at all
return; // do not warn when no switch branches are present at all
}
if (branchCount >= m_limit) {
final int branchCountIncludingDefault = (branchCount < 0) ? -branchCount + 1 : branchCount;
if (branchCountIncludingDefault >= m_limit) {
return;
}
registerStatementError(statement, Integer.valueOf(branchCount));
registerStatementError(statement, Integer.valueOf(branchCountIncludingDefault));
}
}
}
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2007 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2017 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@
package com.siyeh.ig.controlflow;
import com.intellij.codeInspection.ui.SingleIntegerFieldOptionsPanel;
import com.intellij.psi.PsiCodeBlock;
import com.intellij.psi.PsiSwitchStatement;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
@@ -26,15 +25,11 @@ import org.jetbrains.annotations.NotNull;
import javax.swing.*;
public class SwitchStatementWithTooManyBranchesInspection
extends BaseInspection {
public class SwitchStatementWithTooManyBranchesInspection extends BaseInspection {
private static final int DEFAULT_BRANCH_LIMIT = 10;
/**
* this is public for the DefaultJDOMExternalizer thingy
*
* @noinspection PublicField
*/
@SuppressWarnings("PublicField")
public int m_limit = DEFAULT_BRANCH_LIMIT;
@Override
@@ -66,21 +61,16 @@ public class SwitchStatementWithTooManyBranchesInspection
return new SwitchStatementWithTooManyBranchesVisitor();
}
private class SwitchStatementWithTooManyBranchesVisitor
extends BaseInspectionVisitor {
private class SwitchStatementWithTooManyBranchesVisitor extends BaseInspectionVisitor {
@Override
public void visitSwitchStatement(
@NotNull PsiSwitchStatement statement) {
final PsiCodeBlock body = statement.getBody();
if (body == null) {
return;
}
public void visitSwitchStatement(@NotNull PsiSwitchStatement statement) {
final int branchCount = SwitchUtils.calculateBranchCount(statement);
if (branchCount <= m_limit) {
final int branchCountIncludingDefault = (branchCount < 0) ? -branchCount + 1 : branchCount;
if (branchCountIncludingDefault <= m_limit) {
return;
}
registerStatementError(statement, Integer.valueOf(branchCount));
registerStatementError(statement, Integer.valueOf(branchCountIncludingDefault));
}
}
}
@@ -17,16 +17,16 @@ package com.siyeh.ig.controlflow;
import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.SwitchUtils;
import org.intellij.lang.annotations.Pattern;
import org.jetbrains.annotations.NotNull;
import javax.swing.*;
import java.util.Collection;
public class SwitchStatementsWithoutDefaultInspection extends BaseInspection {
@@ -68,36 +68,22 @@ public class SwitchStatementsWithoutDefaultInspection extends BaseInspection {
@Override
public void visitSwitchStatement(@NotNull PsiSwitchStatement statement) {
super.visitSwitchStatement(statement);
if (switchStatementHasDefault(statement)) {
final int count = SwitchUtils.calculateBranchCount(statement);
if (count <= 0) {
return;
}
if (m_ignoreFullyCoveredEnums && switchStatementIsFullyCoveredEnum(statement, count)) {
return;
}
registerStatementError(statement);
}
private boolean switchStatementHasDefault(PsiSwitchStatement statement) {
final PsiCodeBlock body = statement.getBody();
if (body == null) {
return true; // do not warn about incomplete code
}
final Collection<PsiSwitchLabelStatement> labelStatements = PsiTreeUtil.findChildrenOfType(body, PsiSwitchLabelStatement.class);
// warn only when switch branches are present
if (labelStatements.isEmpty() || labelStatements.stream().anyMatch(PsiSwitchLabelStatement::isDefaultCase)) {
return true;
}
return m_ignoreFullyCoveredEnums && switchStatementIsFullyCoveredEnum(statement, labelStatements.size());
}
private boolean switchStatementIsFullyCoveredEnum(PsiSwitchStatement statement, int branchCount) {
final PsiExpression expression = statement.getExpression();
if (expression == null) {
return true; // don't warn on incomplete code
}
final PsiType type = expression.getType();
if (!(type instanceof PsiClassType)) {
return false;
}
final PsiClassType classType = (PsiClassType)type;
final PsiClass aClass = classType.resolve();
final PsiClass aClass = PsiUtil.resolveClassInClassTypeOnly(expression.getType());
return aClass != null && aClass.isEnum() && ControlFlowUtils.hasChildrenOfTypeCount(aClass, branchCount, PsiEnumConstant.class);
}
}
@@ -30,19 +30,28 @@ public class SwitchUtils {
private SwitchUtils() {}
/**
* Does not count the default statement, but returns a negative number when it is present.
* So for example if a switch statement contains 4 cases and a default case, it will return -4
* @param statement the statement to count the cases of.
* @return a negative number if a default case was encountered.
*/
public static int calculateBranchCount(@NotNull PsiSwitchStatement statement) {
final PsiCodeBlock body = statement.getBody();
if (body == null) {
return 0;
}
final PsiStatement[] statements = body.getStatements();
int branches = 0;
for (final PsiStatement child : statements) {
if (child instanceof PsiSwitchLabelStatement) {
boolean defaultFound = false;
for (final PsiSwitchLabelStatement child : PsiTreeUtil.getChildrenOfTypeAsList(body, PsiSwitchLabelStatement.class)) {
if (child.isDefaultCase()) {
defaultFound = true;
}
else {
branches++;
}
}
return branches;
return defaultFound ? -branches : branches;
}
@Nullable
@@ -81,4 +81,10 @@ public class SwitchStatementsWithoutDefault
break;
}
}
void empty(T t) {
switch (t) {
}
}
}
@@ -0,0 +1,57 @@
/*
* 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.siyeh.ig.controlflow;
import com.intellij.codeInspection.InspectionProfileEntry;
import com.siyeh.ig.LightInspectionTestCase;
import org.jetbrains.annotations.Nullable;
/**
* @author Bas Leijdekkers
*/
public class EnumSwitchStatementWhichMissesCasesInspectionTest extends LightInspectionTestCase {
public void testSimple() {
doTest("enum E { A, B, C }" +
"class X {" +
" void m(E e) {" +
" /*'switch' statement on enum type 'E' misses cases*/switch/**/ (e) {" +
" case A:" +
" case B:" +
" }" +
" }" +
"}");
}
public void testFullyCovered() {
doTest("enum E { A, B, C }" +
"class X {" +
" void m(E e) {" +
" switch(e) {" +
" case A:" +
" case B:" +
" case C:" +
" }" +
" }" +
"}");
}
@Nullable
@Override
protected InspectionProfileEntry getInspection() {
return new EnumSwitchStatementWhichMissesCasesInspection();
}
}