Bug 355174 - Added cheker for missing case in a switch

Change-Id: I1ba1193186e275bed450eb7447eefd90dc09ebec
Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>

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

codan/org.eclipse.cdt.codan.checkers/plugin.xml

@@ -499,5 +499,30 @@
                name="%problem.name.CopyrightProblem">
          </problem>
       </checker>
+      <checker
+            class="org.eclipse.cdt.codan.internal.checkers.SwitchCaseChecker"
+            id="org.eclipse.cdt.codan.internal.checkers.SwitchCaseChecker"
+            name="%checker.name.SwitchCaseChecker">
+         <problem
+               category="org.eclipse.cdt.codan.core.categories.ProgrammingProblems"
+               defaultEnabled="false"
+               defaultSeverity="Warning"
+               description="%problem.description.MissCaseProblem"
+               id="org.eclipse.cdt.codan.internal.checkers.MissCaseProblem"
+               markerType="org.eclipse.cdt.codan.core.codanProblem"
+               messagePattern="%problem.messagePattern.MissCaseProblem"
+               name="%problem.name.MissCaseProblem">
+         </problem>
+         <problem
+               category="org.eclipse.cdt.codan.core.categories.ProgrammingProblems"
+               defaultEnabled="false"
+               defaultSeverity="Warning"
+               description="%problem.description.MissDefaultProblem"
+               id="org.eclipse.cdt.codan.internal.checkers.MissDefaultProblem"
+               markerType="org.eclipse.cdt.codan.core.codanProblem"
+               messagePattern="%problem.messagePattern.MissDefaultProblem"
+               name="%problem.name.MissDefaultProblem">
+         </problem>
+      </checker>
    </extension>
 </plugin>

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;
 

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)

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<Number> 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<IASTStatement> 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;
+			}
+		});
+	}
+}

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);
+	}
+}

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