Bug 534420 - Add checker for [[nodiscard]] attribute

Change-Id: I5e40a2d50281b669c62ed48ee1a3a399ca71981c

codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties

@@ -213,3 +213,7 @@ problem.name.MagicNumberProblem = Avoid magic numbers
 problem.messagePattern.MagicNumberProblem = Avoid constant literals
 problem.description.MagicNumberProblem = This rule will flag all constant literals numbers
 
+checker.name.NoDiscard = NoDiscardChecker
+problem.description.NoDiscard = This rule will flag the use of functions marked as 'no discard' without taking into account return value
+problem.messagePattern.NoDiscard = Return value from ''{0}'' not evaluated
+problem.name.NoDiscard = Return value not evaluated

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

@@ -672,5 +672,17 @@
                name="%problem.name.MagicNumberProblem">
          </problem>
       </checker>
+      <checker
+            class="org.eclipse.cdt.codan.internal.checkers.NoDiscardChecker"
+            id="org.eclipse.cdt.codan.internal.checkers.NoDiscardChecker"
+            name="%checker.name.NoDiscard">
+         <problem
+               category="org.eclipse.cdt.codan.core.categories.ProgrammingProblems"
+               defaultSeverity="Warning"
+               description="%problem.description.NoDiscard"
+               id="org.eclipse.cdt.codan.internal.checkers.NoDiscardProblem"
+               messagePattern="%problem.messagePattern.NoDiscard"
+               name="%problem.name.NoDiscard"/>
+      </checker>
    </extension>
 </plugin>

codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.java

@@ -46,6 +46,7 @@ public class CheckersMessages extends NLS {
 	public static String Copyright_regex;
 	public static String ShallowCopyChecker_OnlyNew;
 	public static String CStyleCastCheck_checkInMacro;
+	public static String NoDiscardChecker_ParameterMacro;
 
 	static {
 		NLS.initializeMessages(CheckersMessages.class.getName(), CheckersMessages.class);

codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.properties

@@ -29,7 +29,7 @@ SuspiciousSemicolonChecker_ParamAfterElse=Report an error if semicolon is right
 SuspiciousSemicolonChecker_ParamElse=Do not report an error after 'if' when 'else' exists
 MagicNumberChecker_ParameterArray=Check literals in array expressions
 MagicNumberChecker_ParameterOperatorParen=Check literals in calls of class operator()
-
+NoDiscardChecker_ParameterMacro=Report problem in statements that come from macro expansion
 SwitchCaseChecker_ParameterDefaultAllEnums=Mark even switches with complete enum items
 
 BlacklistChecker_list=List of methods/functions to be checked

codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/NoDiscardChecker.java

@@ -0,0 +1,179 @@
+/*******************************************************************************
+ * Copyright (c) 2020 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.Optional;
+
+import org.eclipse.cdt.codan.core.cxx.model.AbstractAstFunctionChecker;
+import org.eclipse.cdt.codan.core.model.IProblemWorkingCopy;
+import org.eclipse.cdt.core.dom.ast.ASTVisitor;
+import org.eclipse.cdt.core.dom.ast.IASTExpression;
+import org.eclipse.cdt.core.dom.ast.IASTExpressionList;
+import org.eclipse.cdt.core.dom.ast.IASTExpressionStatement;
+import org.eclipse.cdt.core.dom.ast.IASTFieldReference;
+import org.eclipse.cdt.core.dom.ast.IASTFunctionCallExpression;
+import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition;
+import org.eclipse.cdt.core.dom.ast.IASTIdExpression;
+import org.eclipse.cdt.core.dom.ast.IASTName;
+import org.eclipse.cdt.core.dom.ast.IASTNode;
+import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression;
+import org.eclipse.cdt.core.dom.ast.IBinding;
+import org.eclipse.cdt.core.dom.ast.IFunction;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCastExpression;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTExpression;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPClassType;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPFunction;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPVariable;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPEvaluation;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.EvalFunctionCall;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.EvalTypeId;
+
+@SuppressWarnings("restriction")
+public class NoDiscardChecker extends AbstractAstFunctionChecker {
+	public static final String ER_ID = "org.eclipse.cdt.codan.internal.checkers.NoDiscardProblem"; //$NON-NLS-1$
+	public static final String PARAM_MACRO_ID = "macro"; //$NON-NLS-1$
+
+	@Override
+	protected void processFunction(IASTFunctionDefinition func) {
+		func.accept(new FunctionCallVisitor());
+	}
+
+	class FunctionCallVisitor extends ASTVisitor {
+
+		FunctionCallVisitor() {
+			shouldVisitExpressions = true;
+		}
+
+		@Override
+		public int visit(IASTExpression expr) {
+			if (!shouldReportInMacro() && enclosedInMacroExpansion(expr))
+				return PROCESS_SKIP;
+			if (isDiscardedValueExpression(expr)) {
+				Optional<IASTNode> res = getNodiscardFunction(expr);
+				if (res.isPresent()) {
+					reportProblem(ER_ID, expr, res.get());
+				}
+			}
+			return PROCESS_CONTINUE;
+		}
+
+		private boolean isDiscardedValueExpression(IASTExpression expr) {
+			IASTNode parent = expr.getParent();
+			IASTNode child = expr;
+
+			while (parent instanceof IASTUnaryExpression) {
+				int operator = ((IASTUnaryExpression) parent).getOperator();
+				if (operator != IASTUnaryExpression.op_bracketedPrimary) {
+					return false;
+				}
+				child = parent;
+				parent = parent.getParent();
+			}
+			if (parent instanceof IASTExpressionStatement) {
+				return true;
+			}
+
+			if (child instanceof IASTExpression && checkNestedLeftSide((IASTExpression) child)) {
+				return true;
+			}
+
+			return false;
+		}
+
+		private Optional<IASTNode> getNodiscardFunction(IASTExpression expr) {
+			if (expr instanceof IASTFunctionCallExpression) {
+				IASTFunctionCallExpression func = (IASTFunctionCallExpression) expr;
+				IASTExpression functionNameExpression = func.getFunctionNameExpression();
+
+				IASTName name;
+				if (functionNameExpression instanceof IASTIdExpression) {
+					name = ((IASTIdExpression) functionNameExpression).getName();
+				} else if (functionNameExpression instanceof IASTFieldReference) {
+					name = ((IASTFieldReference) functionNameExpression).getFieldName();
+				} else {
+					return Optional.empty();
+				}
+				IBinding binding = name.resolveBinding();
+
+				if (binding instanceof IFunction && ((IFunction) binding).isNoDiscard()) {
+					return Optional.of(name);
+				} else if ((binding instanceof ICPPClassType || binding instanceof ICPPVariable)
+						&& expr instanceof ICPPASTExpression && checkEvaluation((ICPPASTExpression) expr)) {
+					return Optional.of(name);
+				}
+
+				return Optional.empty();
+			} else if (expr instanceof ICPPASTCastExpression) {
+				ICPPASTCastExpression cast = (ICPPASTCastExpression) expr;
+				if (cast.getOperator() != ICPPASTCastExpression.op_static_cast) {
+					return Optional.empty();
+				}
+				if (checkEvaluation(cast)) {
+					return Optional.of(cast.getTypeId());
+				}
+				return Optional.empty();
+			} else
+				return Optional.empty();
+		}
+
+		/**
+		 * Check if the expression is 'nodiscard' looking for its evaluation
+		 * @param expr A cpp expression
+		 * @return True if nodiscard, false otherwise
+		 */
+		private boolean checkEvaluation(ICPPASTExpression expr) {
+			ICPPEvaluation eval = expr.getEvaluation();
+			if (eval instanceof EvalTypeId) {
+				ICPPFunction evalFunc = ((EvalTypeId) eval).getConstructor();
+				if (evalFunc != null && evalFunc.isNoDiscard()) {
+					return true;
+				}
+			} else if (eval instanceof EvalFunctionCall) {
+				ICPPFunction evalFunc = ((EvalFunctionCall) eval).resolveFunctionBinding();
+				if (evalFunc != null && evalFunc.isNoDiscard()) {
+					return true;
+				}
+			}
+			return false;
+		}
+
+		/**
+		 * Check if the function call is a left side of a nested expression. If it's a
+		 * left side we need to go ahead and we need to evaluate the expression since it
+		 * can be a discarded expression.
+		 * @param func The expression list
+		 * @return True if it's a nested expression and it's the left side, false otherwise
+		 */
+		private boolean checkNestedLeftSide(IASTExpression expr) {
+			if (expr.getPropertyInParent() == IASTExpressionList.NESTED_EXPRESSION) {
+				IASTExpressionList list = (IASTExpressionList) expr.getParent();
+				IASTExpression[] allExpr = list.getExpressions();
+				if (expr != allExpr[allExpr.length - 1])
+					return true;
+			}
+			return false;
+		}
+	}
+
+	@Override
+	public void initPreferences(IProblemWorkingCopy problem) {
+		super.initPreferences(problem);
+		addPreference(problem, PARAM_MACRO_ID, CheckersMessages.NoDiscardChecker_ParameterMacro, Boolean.TRUE);
+	}
+
+	/**
+	 * @return
+	 */
+	private boolean shouldReportInMacro() {
+		return (Boolean) getPreference(getProblemById(ER_ID, getFile()), PARAM_MACRO_ID);
+	}
+}

codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/NoDiscardCheckerTest.java

@@ -0,0 +1,387 @@
+/*******************************************************************************
+ * Copyright (c) 2020 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.NoDiscardChecker;
+
+/**
+ * Test for {@link NoDiscardChecker} class
+ */
+public class NoDiscardCheckerTest extends CheckerTestCase {
+
+	public static final String ER_ID = NoDiscardChecker.ER_ID;
+
+	@Override
+	public void setUp() throws Exception {
+		super.setUp();
+		enableProblems(ER_ID);
+	}
+
+	@Override
+	public boolean isCpp() {
+		return true;
+	}
+
+	//[[nodiscard]] int test() {
+	//	return 2;
+	//}
+	//int main() {
+	//	test();
+	//	return 0;
+	//}
+	public void testCppSimpleCall() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(5, ER_ID);
+	}
+
+	//[[nodiscard]] int test() {
+	//	return 2;
+	//}
+	//int main() {
+	//	return test();
+	//}
+	public void testCppInReturn() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//[[nodiscard]] int test() {
+	//	return 2;
+	//}
+	//int main() {
+	//	return test() ? 0 : 1;
+	//}
+	public void testCppInTernaryReturn() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//[[nodiscard]] int test() {
+	//	return 2;
+	//}
+	//void process(int v) {}
+	//int main() {
+	//	process(test());
+	//	return 0;
+	//}
+	public void testCppInOtherCall() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//[[nodiscard]] int test() {
+	//	return 2;
+	//}
+	//void process(int v) {}
+	//int main() {
+	//	if (test()) {
+	//	}
+	//	return 0;
+	//}
+	public void testCppInCondition() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//class Test {
+	//public:
+	//	Test() {}
+	//	[[nodiscard]] int foo() {
+	//		return 2;
+	//	}
+	//};
+	//
+	//int main() {
+	//	Test t;
+	//	t.foo();
+	//	return 0;
+	//}
+	public void testCppNoDiscardMethod() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(11, ER_ID);
+	}
+
+	//int test() __attribute__((warn_unused_result)) {
+	//	return 2;
+	//}
+	//int main() {
+	//	test();
+	//	return 0;
+	//}
+	public void testCSimpleCall() throws Exception {
+		loadCodeAndRunC(getAboveComment());
+		checkErrorLine(5, ER_ID);
+	}
+
+	//int test() __attribute__((warn_unused_result))  {
+	//	return 2;
+	//}
+	//int main() {
+	//	return test();
+	//}
+	public void testCInReturn() throws Exception {
+		loadCodeAndRunC(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//int test() __attribute__((warn_unused_result)) {
+	//	return 2;
+	//}
+	//int main() {
+	//	return test() ? 0 : 1;
+	//}
+	public void testCInTernaryReturn() throws Exception {
+		loadCodeAndRunC(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//int test() __attribute__((warn_unused_result)) {
+	//	return 2;
+	//}
+	//void process(int v) {}
+	//int main() {
+	//	process(test());
+	//	return 0;
+	//}
+	public void testCInOtherCall() throws Exception {
+		loadCodeAndRunC(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//int test() __attribute__((warn_unused_result)) {
+	//	return 2;
+	//}
+	//void process(int v) {}
+	//int main() {
+	//	if (test()) {
+	//	}
+	//	return 0;
+	//}
+	public void testCInCondition() throws Exception {
+		loadCodeAndRunC(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//class Test {
+	//public:
+	//	Test() {}
+	//	int foo() __attribute__((warn_unused_result)) {
+	//		return 2;
+	//	}
+	//};
+	//
+	//int main() {
+	//	Test t;
+	//	t.foo();
+	//	return 0;
+	//}
+	public void testCppUnusedNoDiscardMethod() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(11, ER_ID);
+	}
+
+	//[[nodiscard]] int foo() {
+	//	return 2;
+	//}
+	//int main() {
+	//	((((foo()))));
+	//	return 0;
+	//}
+	public void testInParenNoDiscardMethod() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(5, ER_ID);
+	}
+
+	//[[nodiscard]] int foo() {
+	//	return 2;
+	//}
+	//#define MACRO(X) X()
+	//int main() {
+	//	MACRO(foo);
+	//	return 0;
+	//}
+	public void testInMacroEnabled() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(6, ER_ID);
+	}
+
+	//[[nodiscard]] int foo() {
+	//	return 2;
+	//}
+	//#define MACRO(X) X()
+	//int main() {
+	//	MACRO(foo);
+	//	return 0;
+	//}
+	public void testInMacroDisabled() throws Exception {
+		setPreferenceValue(ER_ID, NoDiscardChecker.PARAM_MACRO_ID, false);
+		loadCodeAndRun(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//[[nodiscard]] int foo() {
+	//	return 2;
+	//}
+	//int main() {
+	//	(void) foo();
+	//	return 0;
+	//}
+	public void testVoidCast() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//[[nodiscard]] int foo() {
+	//	return 2;
+	//}
+	//int main() {
+	//	static_cast<void>(foo());
+	//	return 0;
+	//}
+	public void testStaticVoidCast() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//[[nodiscard]] int foo() {
+	//	return 2;
+	//}
+	//int process() {
+	//	return foo(), 2;
+	//}
+	//int main() {
+	//	process();
+	//	return 0;
+	//}
+	public void testCommaLeftHandSide() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(5, ER_ID);
+	}
+
+	//[[nodiscard]] int foo() {
+	//	return 2;
+	//}
+	//int bar() {
+	//	return 4;
+	//}
+	//int process() {
+	//	return (foo()), bar();
+	//}
+	//int main() {
+	//	process();
+	//	return 0;
+	//}
+	public void testCommaLeftHandSide2() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(8, ER_ID);
+	}
+
+	//[[nodiscard]] int foo() {
+	//	return 2;
+	//}
+	//int bar() {
+	//	return 4;
+	//}
+	//int main() {
+	//	(foo()), bar();
+	//	return 0;
+	//}
+	public void testCommaLeftHandSide3() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(8, ER_ID);
+	}
+
+	//[[nodiscard]] int foo() {
+	//	return 2;
+	//}
+	//int process() {
+	//	return 2, foo();
+	//}
+	//int main() {
+	//	process();
+	//	return 0;
+	//}
+	public void testCommaRightHandSide() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//[[nodiscard]] int foo() {
+	//	return 2;
+	//}
+	//int main() {
+	//	int vv = 0;
+	//	vv = 2, foo();
+	//	return 0;
+	//}
+	public void testCommaRightHandSide2() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//class Test {
+	//public:
+	//	Test(double v) {}
+	//	[[nodiscard]] Test(int v) {}
+	//};
+	//int main() {
+	//	Test(42);
+	//	Test(42.0);
+	//	return 0;
+	//}
+	public void testCppNoDiscardCtor() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(7, ER_ID);
+	}
+
+	//class Test {
+	//public:
+	//	Test(double v) {}
+	//	[[nodiscard]] Test(int v) {}
+	//};
+	//int main() {
+	//	static_cast<Test>(42);
+	//	static_cast<Test>(42.0);
+	//	return 0;
+	//}
+	public void testCppNoDiscardCtorStaticCast() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(7, ER_ID);
+	}
+
+	//class Test {
+	//public:
+	//	Test(double v) {}
+	//	[[nodiscard]] Test(int v) {}
+	//};
+	//int main() {
+	//	reinterpret_cast<Test>(42);
+	//	(Test) 42;
+	//	return 0;
+	//}
+	public void testCppNoDiscardCtorOtherCast() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrorsOfKind(ER_ID);
+	}
+
+	//struct Foo {
+	//	[[nodiscard]] bool operator()() { return true; }
+	//};
+	//int main() {
+	//	Foo foo1;
+	//  foo1();
+	//}
+	public void testCppFunctor() throws Exception {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(6, ER_ID);
+	}
+}

codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java

@@ -30,6 +30,7 @@ import org.eclipse.cdt.codan.core.internal.checkers.FloatCompareCheckerTest;
 import org.eclipse.cdt.codan.core.internal.checkers.FormatStringCheckerTest;
 import org.eclipse.cdt.codan.core.internal.checkers.GotoStatementCheckerTest;
 import org.eclipse.cdt.codan.core.internal.checkers.MagicNumberCheckerTest;
+import org.eclipse.cdt.codan.core.internal.checkers.NoDiscardCheckerTest;
 import org.eclipse.cdt.codan.core.internal.checkers.NonVirtualDestructorCheckerTest;
 import org.eclipse.cdt.codan.core.internal.checkers.ProblemBindingCheckerTest;
 import org.eclipse.cdt.codan.core.internal.checkers.ReturnCheckerTest;
@@ -109,6 +110,7 @@ public class AutomatedIntegrationSuite extends TestSuite {
 		suite.addTestSuite(VariableShadowingCheckerTest.class);
 		suite.addTestSuite(ShallowCopyCheckerTest.class);
 		suite.addTestSuite(MagicNumberCheckerTest.class);
+		suite.addTestSuite(NoDiscardCheckerTest.class);
 		// framework
 		suite.addTest(CodanFastTestSuite.suite());
 		// quick fixes

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalFunctionCall.java

@@ -372,7 +372,7 @@ public final class EvalFunctionCall extends CPPDependentEvaluation {
 		return EvalFixed.INCOMPLETE;
 	}
 
-	private ICPPFunction resolveFunctionBinding() {
+	public ICPPFunction resolveFunctionBinding() {
 		ICPPFunction function = getOverload();
 		if (function == null) {
 			ICPPEvaluation funcEval = fArguments[0];