diff --git a/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties b/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties index 7cd7677c36a..310446405e2 100644 --- a/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties +++ b/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties @@ -140,6 +140,7 @@ problem.name.UsingInHeaderProblem = Using directive in header problem.messagePattern.UsingInHeaderProblem = Using directive in header files can cause name clashes in other files problem.description.UsingInHeaderProblem = This rule will flag 'using' directive in header files + checker.name.CStyleCastChecker = C-Style cast problem.name.CStyleCastProblem = C-Style cast instead of C++ cast problem.messagePattern.CStyleCastProblem = C++ style casts express the intent of the programmer more clearly and they can be checked by compiler @@ -154,3 +155,11 @@ checker.name.CopyrightChecker = Copyright in source code checker problem.name.CopyrightProblem = Lack of copyright information problem.messagePattern.CopyrightProblem = Copyright information missing problem.description.CopyrightProblem = This rule will flag files without copyright information + +checker.name.SwitchCaseChecker = Missing cases in switch statements checker +problem.name.MissCaseProblem = Missing cases in switch +problem.messagePattern.MissCaseProblem = Not all enumeration values are present in the switch statement +problem.description.MissCaseProblem = This rule will flag switch statements with missing case where enumerations have been used +problem.name.MissDefaultProblem = Missing default in switch +problem.messagePattern.MissDefaultProblem = When default case is missing the switch is not complete for all values +problem.description.MissDefaultProblem = This rule will flag switch statements without a default case diff --git a/codan/org.eclipse.cdt.codan.checkers/plugin.xml b/codan/org.eclipse.cdt.codan.checkers/plugin.xml index 539f7a67ea3..45fbd7329c5 100644 --- a/codan/org.eclipse.cdt.codan.checkers/plugin.xml +++ b/codan/org.eclipse.cdt.codan.checkers/plugin.xml @@ -499,5 +499,30 @@ name="%problem.name.CopyrightProblem"> + + + + + + diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.java index 9f8b3afef34..844bd20b641 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.java +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.java @@ -35,7 +35,7 @@ public class CheckersMessages extends NLS { public static String SuspiciousSemicolonChecker_ParamAfterElse; public static String SuspiciousSemicolonChecker_ParamElse; public static String ProblemBindingChecker_Candidates; - + public static String SwitchCaseChecker_ParameterDefaultAllEnums; public static String UnusedSymbolInFileScopeChecker_CharacterSequence; public static String UnusedSymbolInFileScopeChecker_Exceptions; diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.properties b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.properties index 36347f40118..bdd44a3d8c3 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.properties +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.properties @@ -26,7 +26,7 @@ StatementHasNoEffectChecker_ParameterMacro=Report problem in statements that com SuggestedParenthesisChecker_SuggestParanthesesAroundNot=Suggest parenthesis around 'not' operator SuspiciousSemicolonChecker_ParamAfterElse=Report an error if semicolon is right after 'else' statement SuspiciousSemicolonChecker_ParamElse=Do not report an error after 'if' when 'else' exists - +SwitchCaseChecker_ParameterDefaultAllEnums=Mark even switches with complete enum items ProblemBindingChecker_Candidates=Candidates are: UnusedSymbolInFileScopeChecker_CharacterSequence=Identifier character sequence: UnusedSymbolInFileScopeChecker_Exceptions=Exceptions (identifier character sequence) diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/SwitchCaseChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/SwitchCaseChecker.java new file mode 100644 index 00000000000..8844a11085b --- /dev/null +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/SwitchCaseChecker.java @@ -0,0 +1,123 @@ +/******************************************************************************* + * Copyright (c) 2019 Marco Stornelli + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.cdt.codan.internal.checkers; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker; +import org.eclipse.cdt.codan.core.model.IProblem; +import org.eclipse.cdt.codan.core.model.IProblemWorkingCopy; +import org.eclipse.cdt.core.dom.ast.ASTVisitor; +import org.eclipse.cdt.core.dom.ast.IASTCaseStatement; +import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement; +import org.eclipse.cdt.core.dom.ast.IASTDefaultStatement; +import org.eclipse.cdt.core.dom.ast.IASTExpression; +import org.eclipse.cdt.core.dom.ast.IASTIdExpression; +import org.eclipse.cdt.core.dom.ast.IASTLiteralExpression; +import org.eclipse.cdt.core.dom.ast.IASTName; +import org.eclipse.cdt.core.dom.ast.IASTStatement; +import org.eclipse.cdt.core.dom.ast.IASTSwitchStatement; +import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; +import org.eclipse.cdt.core.dom.ast.IBinding; +import org.eclipse.cdt.core.dom.ast.IEnumeration; +import org.eclipse.cdt.core.dom.ast.IEnumerator; +import org.eclipse.cdt.core.dom.ast.IType; +import org.eclipse.cdt.internal.core.dom.parser.ValueFactory; +import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil; + +@SuppressWarnings("restriction") +public class SwitchCaseChecker extends AbstractIndexAstChecker { + public static final String MISS_CASE_ID = "org.eclipse.cdt.codan.internal.checkers.MissCaseProblem"; //$NON-NLS-1$ + public static final String MISS_DEFAULT_ID = "org.eclipse.cdt.codan.internal.checkers.MissDefaultProblem"; //$NON-NLS-1$ + public static final String PARAM_DEFAULT_ALL_ENUMS = "defaultWithAllEnums"; //$NON-NLS-1$ + private boolean defaultWithAllEnums = false; + + @Override + public void initPreferences(IProblemWorkingCopy problem) { + super.initPreferences(problem); + if (problem.getId().equals(MISS_DEFAULT_ID)) + addPreference(problem, PARAM_DEFAULT_ALL_ENUMS, CheckersMessages.SwitchCaseChecker_ParameterDefaultAllEnums, + Boolean.FALSE); + } + + @Override + public void processAst(IASTTranslationUnit ast) { + final IProblem pt = getProblemById(MISS_DEFAULT_ID, getFile()); + defaultWithAllEnums = (Boolean) getPreference(pt, PARAM_DEFAULT_ALL_ENUMS); + ast.accept(new ASTVisitor() { + { + shouldVisitStatements = true; + } + + @Override + public int visit(IASTStatement statement) { + if (statement instanceof IASTSwitchStatement) { + IASTExpression controller = ((IASTSwitchStatement) statement).getControllerExpression(); + IASTStatement bodyStmt = ((IASTSwitchStatement) statement).getBody(); + IType type = SemanticUtil.getUltimateType(controller.getExpressionType(), true); + Set enumValues = new HashSet<>(); + boolean defaultFound = false; + boolean isEnumSwitch = false; + if (type instanceof IEnumeration) { + IEnumerator[] enums = ((IEnumeration) type).getEnumerators(); + for (IEnumerator e : enums) { + enumValues.add(e.getValue().numberValue()); + } + isEnumSwitch = true; + } + final List statements; + if (bodyStmt instanceof IASTCompoundStatement) { + statements = Arrays.asList(((IASTCompoundStatement) bodyStmt).getStatements()); + } else { + statements = Collections.singletonList(bodyStmt); + } + for (IASTStatement s : statements) { + if (s instanceof IASTDefaultStatement) { + defaultFound = true; + } else if (s instanceof IASTCaseStatement + && ((IASTCaseStatement) s).getExpression() instanceof IASTIdExpression) { + IASTName name = ((IASTIdExpression) ((IASTCaseStatement) s).getExpression()).getName(); + IBinding binding = name.resolveBinding(); + if (binding instanceof IEnumerator) { + enumValues.remove(((IEnumerator) binding).getValue().numberValue()); + } + } else if (s instanceof IASTCaseStatement + && ((IASTCaseStatement) s).getExpression() instanceof IASTLiteralExpression) { + Number value = ValueFactory + .getConstantNumericalValue(((IASTCaseStatement) s).getExpression()); + if (value != null) + enumValues.remove(value); + } + } + if (!defaultFound) { + if (isEnumSwitch && enumValues.size() != 0) { + //switch not complete + reportProblem(MISS_CASE_ID, statement); + reportProblem(MISS_DEFAULT_ID, statement); + } else if (isEnumSwitch && enumValues.size() == 0) { + //switch complete but lack of default label + if (defaultWithAllEnums) + reportProblem(MISS_DEFAULT_ID, statement); + } else { + //switch is not an enum switch and lack of default label + reportProblem(MISS_DEFAULT_ID, statement); + } + } + } + return PROCESS_CONTINUE; + } + }); + } +} diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/SwitchCaseCheckerTest.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/SwitchCaseCheckerTest.java new file mode 100644 index 00000000000..934d6b1dbc5 --- /dev/null +++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/SwitchCaseCheckerTest.java @@ -0,0 +1,197 @@ +/******************************************************************************* + * Copyright (c) 2019 Marco Stornelli + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.cdt.codan.core.internal.checkers; + +import org.eclipse.cdt.codan.core.tests.CheckerTestCase; +import org.eclipse.cdt.codan.internal.checkers.SwitchCaseChecker; + +/** + * Test for {@link SwitchCaseChecker} class + */ +public class SwitchCaseCheckerTest extends CheckerTestCase { + + public static final String MISS_CASE_ID = SwitchCaseChecker.MISS_CASE_ID; + public static final String MISS_DEFAULT_ID = SwitchCaseChecker.MISS_DEFAULT_ID; + + @Override + public void setUp() throws Exception { + super.setUp(); + enableProblems(MISS_CASE_ID, MISS_DEFAULT_ID); + } + + @Override + public boolean isCpp() { + return true; + } + + //enum FRUIT { + // APPLE, PEAR, BANANA + //}; + //FRUIT getFruit() { + // return APPLE; + //} + //int main() { + // switch (FRUIT p = getFruit(); p) { + // case APPLE: + // case PEAR: + // case BANANA: + // break; + // } + // return 0; + //} + public void testSwitchWithInitClause() throws Exception { + setPreferenceValue(MISS_DEFAULT_ID, SwitchCaseChecker.PARAM_DEFAULT_ALL_ENUMS, true); + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MISS_CASE_ID); + checkErrorLine(8, MISS_DEFAULT_ID); + } + + //enum FRUIT { + // APPLE, PEAR, BANANA + //}; + //int main() { + // FRUIT p = APPLE; + // switch (p) { + // case APPLE: + // case PEAR: + // case BANANA: + // break; + // } + // return 0; + //} + public void testSwitchCompleteEnum() throws Exception { + setPreferenceValue(MISS_DEFAULT_ID, SwitchCaseChecker.PARAM_DEFAULT_ALL_ENUMS, true); + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MISS_CASE_ID); + checkErrorLine(6, MISS_DEFAULT_ID); + } + + //enum FRUIT { + // APPLE, PEAR, BANANA + //}; + //int main() { + // FRUIT p = APPLE; + // switch (p) { + // case APPLE: + // case PEAR: + // break; + // } + // return 0; + //} + public void testSwitchMissEnum() throws Exception { + loadCodeAndRun(getAboveComment()); + checkErrorLine(6, MISS_CASE_ID); + } + + //enum FRUIT { + // APPLE, PEAR, BANANA + //}; + //int main() { + // FRUIT p = APPLE; + // switch (p) { + // case APPLE: + // case PEAR: + // case 2: + // break; + // } + // return 0; + //} + public void testSwitchWithMixedValues() throws Exception { + setPreferenceValue(MISS_DEFAULT_ID, SwitchCaseChecker.PARAM_DEFAULT_ALL_ENUMS, true); + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MISS_CASE_ID); + checkErrorLine(6, MISS_DEFAULT_ID); + } + + //enum FRUIT { + // APPLE, PEAR, BANANA + //}; + //int main() { + // FRUIT p = APPLE; + // switch (p) { + // case APPLE: + // case PEAR: + // case BANANA: + // break; + // } + // return 0; + //} + public void testSwitchEnumComplete() throws Exception { + setPreferenceValue(MISS_DEFAULT_ID, SwitchCaseChecker.PARAM_DEFAULT_ALL_ENUMS, false); + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MISS_CASE_ID); + checkNoErrorsOfKind(MISS_DEFAULT_ID); + } + + //enum FRUIT { + // APPLE, PEAR, BANANA + //}; + //int main() { + // FRUIT p = APPLE; + // switch (p) { + // case APPLE: + // case PEAR: + // default: + // break; + // } + // return 0; + //} + public void testSwitchDefaultClausePresent() throws Exception { + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MISS_CASE_ID); + checkNoErrorsOfKind(MISS_DEFAULT_ID); + } + + //enum FRUIT { + // APPLE, PEAR, BANANA + //}; + //int main() { + // FRUIT p = APPLE; + // switch (p) + // break; + // return 0; + //} + public void testSwitchSingleStatement() throws Exception { + loadCodeAndRun(getAboveComment()); + checkErrorLine(6, MISS_CASE_ID); + checkErrorLine(6, MISS_DEFAULT_ID); + } + + //int main() { + // int p = 0; + // switch (p) { + // case 0: + // break; + // } + // return 0; + //} + public void testSwitchIntNoDefault() throws Exception { + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MISS_CASE_ID); + checkErrorLine(3, MISS_DEFAULT_ID); + } + + //#define MYSWITCH() \ + // switch (p) { \ + // case 0: \ + // break; \ + // } + //int main() { + // int p = 0; + // MYSWITCH(); + // return 0; + //} + public void testSwitchInMacro() throws Exception { + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MISS_CASE_ID); + checkErrorLine(8, MISS_DEFAULT_ID); + } +} diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java index 0d174e1ff87..51ec39307eb 100644 --- a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java +++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java @@ -34,6 +34,7 @@ import org.eclipse.cdt.codan.core.internal.checkers.ReturnStyleCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.StatementHasNoEffectCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.SuggestedParenthesisCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.SuspiciousSemicolonCheckerTest; +import org.eclipse.cdt.codan.core.internal.checkers.SwitchCaseCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.UnusedSymbolInFileScopeCheckerTest; import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.AssignmentInConditionQuickFixTest; import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CaseBreakQuickFixBreakTest; @@ -88,6 +89,7 @@ public class AutomatedIntegrationSuite extends TestSuite { suite.addTestSuite(CommentCheckerNestedTests.class); suite.addTestSuite(GotoStatementCheckerTest.class); suite.addTestSuite(CopyrightCheckerTest.class); + suite.addTestSuite(SwitchCaseCheckerTest.class); // framework suite.addTest(CodanFastTestSuite.suite()); // quick fixes