diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index 30e625126c0a..12f7c7f024e7 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -126,7 +126,7 @@ null.argument.to.var.arg.method.problem.descriptor=Confusing argument #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 #ref, 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=#ref 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=#ref statement on enumerated type ''{0}'' misses cases #loc +enum.switch.statement.which.misses.cases.problem.descriptor=#ref 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=#ref statement lacks initializer #loc @@ -1285,7 +1285,7 @@ switch.statement.density.min.option=Minimum density of branches: % switch.statement.density.problem.descriptor=#ref 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=#ref 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=#ref is unnecessary as the last statement in a 'void' method #loc unnecessary.return.constructor.problem.descriptor=#ref is unnecessary as the last statement in a constructor #loc diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/EnumSwitchStatementWhichMissesCasesInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/EnumSwitchStatementWhichMissesCasesInspection.java index 7af5ca477282..a2e274fc6cf9 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/EnumSwitchStatementWhichMissesCasesInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/EnumSwitchStatementWhichMissesCasesInspection.java @@ -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()); } } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementDensityInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementDensityInspection.java index e592f0943d46..cc9a973361d0 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementDensityInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementDensityInspection.java @@ -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) { diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementWithTooFewBranchesInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementWithTooFewBranchesInspection.java index 4e25987a34fc..dec6b5139898 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementWithTooFewBranchesInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementWithTooFewBranchesInspection.java @@ -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)); } } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementWithTooManyBranchesInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementWithTooManyBranchesInspection.java index 6dae5fa01006..4cddd3d08bbb 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementWithTooManyBranchesInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementWithTooManyBranchesInspection.java @@ -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)); } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementsWithoutDefaultInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementsWithoutDefaultInspection.java index 876a05bf5ef1..8b76e7013482 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementsWithoutDefaultInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SwitchStatementsWithoutDefaultInspection.java @@ -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 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); } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SwitchUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SwitchUtils.java index e403c7101915..00a7bda409f9 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SwitchUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SwitchUtils.java @@ -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 diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/switch_statements_without_default/SwitchStatementsWithoutDefault.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/switch_statements_without_default/SwitchStatementsWithoutDefault.java index fa5e6b85cb6d..080f88dac74a 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/switch_statements_without_default/SwitchStatementsWithoutDefault.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/switch_statements_without_default/SwitchStatementsWithoutDefault.java @@ -81,4 +81,10 @@ public class SwitchStatementsWithoutDefault break; } } + + void empty(T t) { + switch (t) { + + } + } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/EnumSwitchStatementWhichMissesCasesInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/EnumSwitchStatementWhichMissesCasesInspectionTest.java new file mode 100644 index 000000000000..36979e3dddba --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/EnumSwitchStatementWhichMissesCasesInspectionTest.java @@ -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(); + } +} \ No newline at end of file