Bug 321471 Return statement style patch from Marc-Andre Laperle

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

@@ -48,4 +48,8 @@ problem.name.NoReturn = No return
 checker.name.AssignmentToItself = Assignment to itself
 problem.messagePattern.AssignmentToItself = Assignment to itself ''{0}''
 problem.name.AssignmentToItself = Assignment to itself
-problem.description.AssignmentToItself = Finds expression where left and right side of the assignment is the same, i.e. 'var = var'
+problem.description.AssignmentToItself = Finds expression where left and right side of the assignment is the same, i.e. 'var = var'
+checker.name.ReturnStyle = Return with parenthesis
+problem.name.ReturnStyle = Return with parenthesis
+problem.messagePattern.ReturnStyle = Return statement has invalid style. Return value should be surrounded by parenthesis
+problem.description.ReturnStyle = Checks for return statements that do no return the value in parenthesis. For example 'return 0;'  

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

@@ -278,5 +278,19 @@
               name="%problem.name.AssignmentToItself">
 		      	</problem>
 		     </checker>
+             <checker
+                class="org.eclipse.cdt.codan.internal.checkers.ReturnStyleChecker"
+                id="org.eclipse.cdt.codan.internal.checkers.ReturnStyle"
+	            name="%checker.name.ReturnStyle">
+             <problem
+                   category="org.eclipse.cdt.codan.core.categories.CodeStyle"
+                   defaultEnabled="false"
+                   defaultSeverity="Warning"
+                   description="%problem.description.ReturnStyle"
+                   id="org.eclipse.cdt.codan.internal.checkers.ReturnStyleProblem"
+                   messagePattern="%problem.messagePattern.ReturnStyle"
+                   name="%problem.name.ReturnStyle">
+             </problem>
+      </checker>
    </extension>
 </plugin>

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

@@ -0,0 +1,61 @@
+/*******************************************************************************
+ * Copyright (c) 2010 Marc-Andre Laperle and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Marc-Andre Laperle - Initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.codan.internal.checkers;
+
+import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker;
+import org.eclipse.cdt.core.dom.ast.ASTVisitor;
+import org.eclipse.cdt.core.dom.ast.IASTNode;
+import org.eclipse.cdt.core.dom.ast.IASTReturnStatement;
+import org.eclipse.cdt.core.dom.ast.IASTStatement;
+import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
+import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression;
+
+public class ReturnStyleChecker extends AbstractIndexAstChecker {
+	public final String ERR_ID = "org.eclipse.cdt.codan.internal.checkers.ReturnStyleProblem"; //$NON-NLS-1$
+	
+	@Override
+	public boolean runInEditor() {
+		return true;
+	}
+
+	public void processAst(IASTTranslationUnit ast) {
+		ast.accept(new ASTVisitor() {
+			{
+				shouldVisitStatements = true;
+			}
+
+			@Override
+			public int visit(IASTStatement statement) {
+				if (statement instanceof IASTReturnStatement) {
+					
+					boolean isValidStyle = false;
+					
+					IASTNode[] children = statement.getChildren();
+					
+					if (children.length == 0) {
+						isValidStyle = true;
+					} else if (children.length == 1
+							&& children[0] instanceof IASTUnaryExpression) {
+						IASTUnaryExpression unaryExpression = (IASTUnaryExpression) children[0];
+						if (unaryExpression.getOperator() == IASTUnaryExpression.op_bracketedPrimary) {
+							isValidStyle = true;
+						}
+					}
+					if(!isValidStyle) {
+						reportProblem(ERR_ID, statement);
+					}
+				}
+				return PROCESS_CONTINUE;
+			}
+		});
+	}
+
+}

codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnStyleCheckerTest.java

@@ -0,0 +1,115 @@
+/*******************************************************************************
+ * Copyright (c) 2010 Marc-Andre Laperle and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Marc-Andre Laperle - Initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.codan.core.internal.checkers;
+
+import org.eclipse.cdt.codan.core.test.CheckerTestCase;
+
+public class ReturnStyleCheckerTest extends CheckerTestCase {
+
+	@Override
+	public void setUp() throws Exception {
+		super.setUp();
+		enableProblems("org.eclipse.cdt.codan.internal.checkers.ReturnStyleProblem"); //$NON-NLS-1$
+	}
+
+	// void foo() {
+	// return; // no error
+	// }
+	public void testSimpleReturn() {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrors();
+	}
+
+	// void foo() {
+	// return
+	// ; // no error
+	// }
+	public void testSimpleReturnMultiLine() {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrors();
+	}
+
+	// int foo() {
+	// return(0); // no error
+	// }
+	public void testSimpleReturnValue() {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrors();
+	}
+
+	// int foo() {
+	// return 0; // error line 2
+	// }
+	public void testSimpleReturnValueError() {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(2);
+	}
+
+	// int foo() {
+	// return // no error
+	// (
+	// 0
+	// );
+	// }
+	public void testReturnValueMultiline() {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrors();
+	}
+
+	// int foo() {
+	// return // error line 2
+	// 0
+	// ;
+	// }
+	public void testReturnValueMultilineError() {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(2);
+	}
+
+	// int foo() {
+	// return ((0));// no error
+	// }
+	public void testReturnValueMultipleBrackets() {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrors();
+	}
+
+	// int foo() {
+	// return // no error
+	// (
+	// (0)
+	// );
+	// }
+	public void testReturnValueMultilineMultipleBrackets() {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrors();
+	}
+
+	// #define MY_RETURN return(0);
+	//
+	// int foo() {
+	// MY_RETURN // no error
+	// }
+	public void testReturnDefine() {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrors();
+	}
+
+	// #define MY_RETURN return 0;
+	//
+	// int foo() {
+	// MY_RETURN // error line 4
+	// }
+	public void testReturnDefineError() {
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(4);
+	}
+}

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

@@ -18,6 +18,7 @@ import org.eclipse.cdt.codan.core.internal.checkers.AssignmentInConditionChecker
 import org.eclipse.cdt.codan.core.internal.checkers.AssignmentToItselfCheckerTest;
 import org.eclipse.cdt.codan.core.internal.checkers.CatchByReferenceTest;
 import org.eclipse.cdt.codan.core.internal.checkers.ReturnCheckerTest;
+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.internal.checkers.ui.quickfix.SuggestedParenthesisQuickFixTest;
@@ -47,6 +48,7 @@ public class AutomatedIntegrationSuite extends TestSuite {
 		suite.addTestSuite(CatchByReferenceTest.class);
 		suite.addTestSuite(AssignmentInConditionCheckerTest.class);
 		suite.addTestSuite(AssignmentToItselfCheckerTest.class);
+		suite.addTestSuite(ReturnStyleCheckerTest.class);
 		// framework
 		suite.addTest(CodanFastTestSuite.suite());
 		// quick fixes