Fix for 217918: CDT code formatter problems

core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/formatter/CodeFormatterVisitor.java

@@ -164,6 +164,7 @@ public class CodeFormatterVisitor extends CPPASTVisitor {
 		public boolean fSpaceAfterComma= true;
 		public boolean fSpaceAfterOpeningParen;
 		public boolean fSpaceBeforeClosingParen;
+		public boolean fSpaceBetweenEmptyParen;
 		public boolean fSpaceBeforeOpeningParen;
 		public int fContinuationIndentation= -1;
 		public ListAlignment(int mode) {
@@ -421,9 +422,26 @@ public class CodeFormatterVisitor extends CPPASTVisitor {
 	 * @see org.eclipse.cdt.core.dom.ast.ASTVisitor#visit(org.eclipse.cdt.core.dom.ast.IASTParameterDeclaration)
 	 */
 	@Override
-	public int visit(IASTParameterDeclaration parameterDeclaration) {
+	public int visit(IASTParameterDeclaration node) {
-		formatRaw(parameterDeclaration);
+		startNode(node);
-		endOfNode(parameterDeclaration);
+		try {
+			// decl-specifier
+			final IASTDeclSpecifier declSpec= node.getDeclSpecifier();
+			if (declSpec != null) {
+				declSpec.accept(this);
+			}
+			// declarator
+			final IASTDeclarator declarator= node.getDeclarator();
+			boolean needSpace= declarator.getPointerOperators().length > 0 && scribe.printComment();
+			if (needSpace) {
+				scribe.space();
+			}
+			if (declarator != null) {
+				declarator.accept(this);
+			}
+		} finally {
+			endOfNode(node);
+		}
 		return PROCESS_SKIP;
 	}
 
@@ -632,11 +650,17 @@ public class CodeFormatterVisitor extends CPPASTVisitor {
 			if (node instanceof IASTProblemHolder) {
 				throw new ASTProblemException(((IASTProblemHolder)node).getProblem());
 			}
-			IASTDeclSpecifier declSpec= node.getDeclSpecifier();
+			// decl-specifier
+			final IASTDeclSpecifier declSpec= node.getDeclSpecifier();
 			if (declSpec != null) {
 				declSpec.accept(this);
 			}
-			IASTDeclarator declarator= node.getAbstractDeclarator();
+			// declarator
+			final IASTDeclarator declarator= node.getAbstractDeclarator();
+			boolean needSpace= declarator.getPointerOperators().length > 0 && scribe.printComment();
+			if (needSpace) {
+				scribe.space();
+			}
 			if (declarator != null) {
 				declarator.accept(this);
 			}
@@ -869,7 +893,10 @@ public class CodeFormatterVisitor extends CPPASTVisitor {
 		node.getName().accept(this);
 		IASTTypeId defaultType= node.getDefaultType();
 		if (defaultType != null) {
-			scribe.printNextToken(Token.tASSIGN, scribe.printComment());
+			scribe.printNextToken(Token.tASSIGN, preferences.insert_space_before_assignment_operator);
+			if (preferences.insert_space_after_assignment_operator) {
+				scribe.space();
+			}
 			defaultType.accept(this);
 		}
 		return PROCESS_SKIP;
@@ -898,7 +925,10 @@ public class CodeFormatterVisitor extends CPPASTVisitor {
 		}
 		IASTExpression defaultValue= node.getDefaultValue();
 		if (defaultValue != null) {
-			scribe.printNextToken(Token.tASSIGN, scribe.printComment());
+			scribe.printNextToken(Token.tASSIGN, preferences.insert_space_before_assignment_operator);
+			if (preferences.insert_space_after_assignment_operator) {
+				scribe.space();
+			}
 			defaultValue.accept(this);
 		}
 		return PROCESS_SKIP;
@@ -907,12 +937,8 @@ public class CodeFormatterVisitor extends CPPASTVisitor {
 	private int visit(ICPPASTConstructorInitializer node) {
 		startNode(node);
 		try {
-			scribe.printNextToken(Token.tLPAREN, false);
+			// format like a function call
-			final IASTExpression value= node.getExpression();
+			formatFunctionCallArguments(node.getExpression());
-			if (value != null) {
-				value.accept(this);
-			}
-			scribe.printNextToken(Token.tRPAREN, false);
 		} finally {
 			endOfNode(node);
 		}
@@ -1078,12 +1104,11 @@ public class CodeFormatterVisitor extends CPPASTVisitor {
 
 	private int visit(IASTStandardFunctionDeclarator node) {
 		final List<IASTParameterDeclaration> parameters = Arrays.asList(node.getParameters());
-		final boolean forceSpaceInsideParen= parameters.isEmpty() && !node.takesVarArgs()
-				&& preferences.insert_space_between_empty_parens_in_method_declaration;
 		final ListAlignment align= new ListAlignment(preferences.alignment_for_parameters_in_method_declaration);
 		align.fSpaceBeforeOpeningParen= preferences.insert_space_before_opening_paren_in_method_declaration;
 		align.fSpaceAfterOpeningParen= preferences.insert_space_after_opening_paren_in_method_declaration;
-		align.fSpaceBeforeClosingParen= preferences.insert_space_before_closing_paren_in_method_declaration || forceSpaceInsideParen;
+		align.fSpaceBeforeClosingParen= preferences.insert_space_before_closing_paren_in_method_declaration;
+		align.fSpaceBetweenEmptyParen= preferences.insert_space_between_empty_parens_in_method_declaration;
 		align.fSpaceBeforeComma= preferences.insert_space_before_comma_in_method_declaration_parameters;
 		align.fSpaceAfterComma= preferences.insert_space_after_comma_in_method_declaration_parameters;
 		formatList(parameters, align, true, node.takesVarArgs());
@@ -1124,6 +1149,7 @@ public class CodeFormatterVisitor extends CPPASTVisitor {
 		ListAlignment align= new ListAlignment(preferences.alignment_for_parameters_in_method_declaration);
 		align.fSpaceAfterOpeningParen= preferences.insert_space_after_opening_paren_in_method_declaration;
 		align.fSpaceBeforeClosingParen= preferences.insert_space_before_closing_paren_in_method_declaration;
+		align.fSpaceBetweenEmptyParen= preferences.insert_space_between_empty_parens_in_method_declaration;
 		align.fSpaceBeforeComma= preferences.insert_space_before_comma_in_method_declaration_parameters;
 		align.fSpaceAfterComma= preferences.insert_space_after_comma_in_method_declaration_parameters;
 		formatList(parameters, align, true, false);
@@ -1607,7 +1633,11 @@ public class CodeFormatterVisitor extends CPPASTVisitor {
 			if (peekNextToken() == Token.tIDENTIFIER) {
 				scribe.skipToToken(Token.tRPAREN);
 			}
-			scribe.printNextToken(Token.tRPAREN, align.fSpaceBeforeClosingParen);
+			if (elementsLength == 0 && !addEllipsis) {
+				scribe.printNextToken(Token.tRPAREN, align.fSpaceBetweenEmptyParen);
+			} else {
+				scribe.printNextToken(Token.tRPAREN, align.fSpaceBeforeClosingParen);
+			}
 		}
 	}
 
@@ -1707,28 +1737,40 @@ public class CodeFormatterVisitor extends CPPASTVisitor {
 	private int visit(IASTFunctionCallExpression node) {
 		node.getFunctionNameExpression().accept(this);
 		IASTExpression paramExpr= node.getParameterExpression();
-		scribe.printNextToken(Token.tLPAREN, preferences.insert_space_before_opening_paren_in_method_invocation);
+		formatFunctionCallArguments(paramExpr);
-		if (preferences.insert_space_after_opening_paren_in_method_invocation) {
-			scribe.space();
-		}
-		if (paramExpr != null) {
-			if (paramExpr instanceof IASTExpressionList) {
-				final IASTExpressionList exprList= (IASTExpressionList)paramExpr;
-				final List<IASTExpression> expressions = Arrays.asList(exprList.getExpressions());
-				final ListAlignment align= new ListAlignment(preferences.alignment_for_arguments_in_method_invocation);
-				align.fSpaceBeforeComma= preferences.insert_space_before_comma_in_method_invocation_arguments;
-				align.fSpaceAfterComma= preferences.insert_space_after_comma_in_method_invocation_arguments;
-				formatList(expressions, align, false, false);
-			} else {
-				paramExpr.accept(this);
-			}
-		} else if (preferences.insert_space_between_empty_parens_in_method_invocation) {
-			scribe.space();
-		}
-		scribe.printNextToken(Token.tRPAREN, preferences.insert_space_before_closing_paren_in_method_invocation);
     	return PROCESS_SKIP;
 	}
 
+	/**
+	 * Formats given expression as a function call, ie. enclosed in parenthesis.
+	 * 
+	 * @param argumentExpr  the argument expression, may be <code>null</code>
+	 */
+	private void formatFunctionCallArguments(IASTExpression argumentExpr) {
+		final List<IASTExpression> expressions;
+		if (argumentExpr != null) {
+			if (argumentExpr instanceof IASTExpressionList) {
+				// argument list
+				final IASTExpressionList exprList= (IASTExpressionList)argumentExpr;
+				expressions= Arrays.asList(exprList.getExpressions());
+			} else {
+				// single argument
+				expressions= Collections.singletonList(argumentExpr);
+			}
+		} else {
+			// no arguments
+			expressions= Collections.emptyList();
+		}
+		final ListAlignment align= new ListAlignment(preferences.alignment_for_arguments_in_method_invocation);
+		align.fSpaceBeforeOpeningParen= preferences.insert_space_before_opening_paren_in_method_invocation;
+		align.fSpaceAfterOpeningParen= preferences.insert_space_after_opening_paren_in_method_invocation;
+		align.fSpaceBeforeClosingParen= preferences.insert_space_before_closing_paren_in_method_invocation;
+		align.fSpaceBetweenEmptyParen= preferences.insert_space_between_empty_parens_in_method_invocation;
+		align.fSpaceBeforeComma= preferences.insert_space_before_comma_in_method_invocation_arguments;
+		align.fSpaceAfterComma= preferences.insert_space_after_comma_in_method_invocation_arguments;
+		formatList(expressions, align, true, false);
+	}
+
 	private int visit(IASTExpressionList node) {
 		final List<IASTExpression> expressions = Arrays.asList(node.getExpressions());
 		final ListAlignment align= new ListAlignment(preferences.alignment_for_expression_list);

core/org.eclipse.cdt.ui.tests/resources/formatter/templates/After.cpp

@@ -11,7 +11,7 @@ class Foo {
 };
 
 // TEMPLATE_STRUCT
-template<class Key, class Value, class SortAlgorithm=DefaultSort>
+template<class Key, class Value, class SortAlgorithm = DefaultSort>
 struct Map {
 	Key* keys;
 	Value* values;
@@ -35,7 +35,7 @@ public:
 };
 
 // TEMPLATE_UNION
-template<class X, class Y, int size=16>
+template<class X, class Y, int size = 16>
 union ArrayOverlay {
 public:
 	X x[size];

core/org.eclipse.cdt.ui.tests/resources/formatter/templates/Before.cpp

@@ -6,9 +6,7 @@ class T;
 class X;
 class Y;
 class Bar;
-class Foo {
+class Foo { template<class Bar> void fum(int i); };
-	template<class Bar> void fum(int i);
-};
 
 // TEMPLATE_STRUCT
 template<class Key, class Value, class SortAlgorithm=DefaultSort>

core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/CodeFormatterTest.java

@@ -280,7 +280,7 @@ public class CodeFormatterTest extends BaseUITestCase {
 
 	//template<typename T> class B {
 	//};
-	//template<typename T1, typename T2=B<T1> > class A {
+	//template<typename T1, typename T2 = B<T1> > class A {
 	//};
 	public void testNestedTemplateParameters_Bug206801() throws Exception {
 		assertFormatterResult();
@@ -463,4 +463,32 @@ public class CodeFormatterTest extends BaseUITestCase {
 		assertFormatterResult();
 	}
 
+	//typedef signed int TInt;
+	//extern void Bar();  // should not have space between parens
+	//
+	//void Foo()    // should not have space between parens
+	//{
+	//  TInt a(3);  // should become TInt a( 3 );
+	//  Bar();   // should not have space between parens
+	//}
+
+	//typedef signed int TInt;
+	//extern void Bar(); // should not have space between parens
+	//
+	//void Foo() // should not have space between parens
+	//	{
+	//	TInt a( 3 ); // should become TInt a( 3 );
+	//	Bar(); // should not have space between parens
+	//	}
+	public void testSpaceBetweenParen_Bug217918() throws Exception {
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_BRACE_POSITION_FOR_METHOD_DECLARATION, DefaultCodeFormatterConstants.NEXT_LINE_SHIFTED);
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_INDENT_STATEMENTS_COMPARE_TO_BODY, DefaultCodeFormatterConstants.FALSE);
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_INSERT_SPACE_AFTER_OPENING_PAREN_IN_METHOD_DECLARATION, CCorePlugin.INSERT);
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_INSERT_SPACE_BEFORE_CLOSING_PAREN_IN_METHOD_DECLARATION, CCorePlugin.INSERT);
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_INSERT_SPACE_BETWEEN_EMPTY_PARENS_IN_METHOD_DECLARATION, CCorePlugin.DO_NOT_INSERT);
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_INSERT_SPACE_AFTER_OPENING_PAREN_IN_METHOD_INVOCATION, CCorePlugin.INSERT);
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_INSERT_SPACE_BEFORE_CLOSING_PAREN_IN_METHOD_INVOCATION, CCorePlugin.INSERT);
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_INSERT_SPACE_BETWEEN_EMPTY_PARENS_IN_METHOD_INVOCATION, CCorePlugin.DO_NOT_INSERT);
+		assertFormatterResult();
+	}
 }