[groovy] formatter: add all nodes between parentheses into ParameterListBlock (EA-125611) (IDEA-196300)

plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/GroovyBlockGenerator.java

@@ -56,6 +56,7 @@ import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil;
 
 import java.util.ArrayList;
 import java.util.Comparator;
+import java.util.Iterator;
 import java.util.List;
 
 import static org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes.mLCURLY;
@@ -254,21 +255,7 @@ public class GroovyBlockGenerator {
     }
 
     if (blockPsi instanceof GrMethod) {
-
-      final ArrayList<Block> subBlocks = new ArrayList<>();
-
-      for (ASTNode childNode : getGroovyChildren(myNode)) {
-        if (childNode.getElementType() == GroovyTokenTypes.mLPAREN) continue;
-        if (childNode.getElementType() == GroovyTokenTypes.mRPAREN) continue;
-
-        if (childNode.getElementType() == GroovyElementTypes.PARAMETERS_LIST) {
-          subBlocks.add(new ParameterListBlock(((GrMethod)blockPsi), Indent.getNoneIndent(), Wrap.createWrap(WrapType.NONE, false), myContext));
-        }
-        else if (canBeCorrectBlock(childNode)) {
-          subBlocks.add(new GroovyBlock(childNode, getIndent(childNode), getChildWrap(childNode), myContext));
-        }
-      }
-      return subBlocks;
+      return generateMethodSubBlocks();
     }
 
     else if (blockPsi instanceof GrTraditionalForClause) {
@@ -586,6 +573,70 @@ public class GroovyBlockGenerator {
     return list;
   }
 
+  @NotNull
+  private List<Block> generateMethodSubBlocks() {
+    final List<Block> result = new ArrayList<>();
+    final Iterator<ASTNode> children = ContainerUtil.iterate(getGroovyChildren(myNode));
+
+    ASTNode leftParen = null;
+    while (children.hasNext()) {
+      final ASTNode childNode = children.next();
+      final IElementType childElementType = childNode.getElementType();
+      if (childElementType == GroovyTokenTypes.mLPAREN) {
+        leftParen = childNode;
+        break;
+      }
+      else if (canBeCorrectBlock(childNode)) {
+        result.add(new GroovyBlock(childNode, getIndent(childNode), getChildWrap(childNode), myContext));
+      }
+    }
+    assert leftParen != null;
+
+    final List<ASTNode> nodes = new ArrayList<>();
+
+    ASTNode parameterList = null;
+    while (children.hasNext()) {
+      final ASTNode childNode = children.next();
+      final IElementType childElementType = childNode.getElementType();
+      if (childElementType == GroovyElementTypes.PARAMETERS_LIST) {
+        parameterList = childNode;
+        break;
+      }
+      if (canBeCorrectBlock(childNode)) {
+        nodes.add(childNode);
+      }
+    }
+    assert parameterList != null;
+
+    nodes.addAll(visibleChildren(parameterList));
+
+    ASTNode rightParen = null;
+    while (children.hasNext()) {
+      final ASTNode childNode = children.next();
+      final IElementType childElementType = childNode.getElementType();
+      if (childElementType == GroovyTokenTypes.mRPAREN) {
+        rightParen = childNode;
+        break;
+      }
+      if (canBeCorrectBlock(childNode)) {
+        nodes.add(childNode);
+      }
+    }
+
+    result.add(new ParameterListBlock(
+      Indent.getNoneIndent(), Wrap.createWrap(WrapType.NONE, false), myContext, leftParen, parameterList, rightParen, nodes
+    ));
+
+    while (children.hasNext()) {
+      final ASTNode childNode = children.next();
+      if (canBeCorrectBlock(childNode)) {
+        result.add(new GroovyBlock(childNode, getIndent(childNode), getChildWrap(childNode), myContext));
+      }
+    }
+
+    return result;
+  }
+
   private boolean mustAlign(PsiElement blockPsi, List<ASTNode> children) {
     // We don't want to align single call argument if it's a closure. The reason is that it looks better to have call like
     //

plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/ParameterListBlock.java

@@ -27,8 +27,6 @@ import org.jetbrains.annotations.Nullable;
 import org.jetbrains.plugins.groovy.formatter.AlignmentProvider;
 import org.jetbrains.plugins.groovy.formatter.FormattingContext;
 import org.jetbrains.plugins.groovy.formatter.processors.GroovyWrappingProcessor;
-import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes;
-import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -37,7 +35,7 @@ import java.util.List;
  * @author Max Medvedev
  */
 public class ParameterListBlock extends GroovyBlock {
-  private final List<Block> mySubBlocks;
+  private final List<Block> mySubBlocks = new ArrayList<>();
   private final TextRange myTextRange;
 
   @NotNull
@@ -46,19 +44,16 @@ public class ParameterListBlock extends GroovyBlock {
     return myTextRange;
   }
 
-  public ParameterListBlock(@NotNull GrMethod method, @NotNull Indent indent, @Nullable Wrap wrap, @NotNull FormattingContext context) {
-    super(method.getParameterList().getNode(), indent, wrap, context);
-    final ASTNode methodNode = method.getNode();
-    final ASTNode leftParenth = methodNode.findChildByType(GroovyTokenTypes.mLPAREN);
-    final ASTNode rightParenth = methodNode.findChildByType(GroovyTokenTypes.mRPAREN);
+  public ParameterListBlock(@NotNull Indent indent,
+                            @Nullable Wrap wrap,
+                            @NotNull FormattingContext context,
+                            @NotNull ASTNode leftParenth,
+                            @NotNull ASTNode parameterList,
+                            @Nullable ASTNode rightParenth,
+                            @NotNull List<ASTNode> astNodes) {
+    super(parameterList, indent, wrap, context);
 
-    final GroovyWrappingProcessor wrappingProcessor = new GroovyWrappingProcessor(this);
-    mySubBlocks = new ArrayList<>();
-    if (leftParenth != null) {
-      mySubBlocks.add(new GroovyBlock(leftParenth, Indent.getNoneIndent(), Wrap.createWrap(WrapType.NONE, false), myContext));
-    }
-
-    List<ASTNode> astNodes = GroovyBlockGenerator.visibleChildren(myNode);
+    mySubBlocks.add(new GroovyBlock(leftParenth, Indent.getNoneIndent(), Wrap.createWrap(WrapType.NONE, false), myContext));
 
     final boolean unfinished = isParameterListUnfinished(myNode);
 
@@ -74,11 +69,11 @@ public class ParameterListBlock extends GroovyBlock {
     }
 
 
+    final GroovyWrappingProcessor wrappingProcessor = new GroovyWrappingProcessor(this);
     for (ASTNode childNode : astNodes) {
       mySubBlocks.add(new GroovyBlock(childNode, Indent.getContinuationIndent(), wrappingProcessor.getChildWrap(childNode), myContext));
     }
 
-
     if (rightParenth != null) {
       mySubBlocks.add(new GroovyBlock(rightParenth,
                                       unfinished ? Indent.getContinuationIndent() : Indent.getNoneIndent(),
@@ -86,9 +81,7 @@ public class ParameterListBlock extends GroovyBlock {
                                       myContext));
 
       if (!unfinished && myContext.getSettings().ALIGN_MULTILINE_METHOD_BRACKETS) {
-        if (leftParenth != null) {
-          myContext.getAlignmentProvider().addPair(leftParenth, rightParenth, false);
-        }
+        myContext.getAlignmentProvider().addPair(leftParenth, rightParenth, false);
       }
     }
 

plugins/groovy/test/org/jetbrains/plugins/groovy/lang/formatter/GroovyCodeStyleFormatterTest.groovy

@@ -111,6 +111,8 @@ class GroovyCodeStyleFormatterTest extends GroovyFormatterTestCase {
 
   void testParametersWrapAlwaysNl() { doTest() }
 
+  void testParametersComments() { doTest() }
+
   void testMethod_decl1() throws Throwable { doTest() }
 
   void testMethod_decl2() throws Throwable { doTest() }

plugins/groovy/testdata/groovy/codeStyle/parametersComments.test

@@ -0,0 +1,3 @@
+def foo(/*foo*/ /*foo*/ a, b /*foo*/ /*foo*/) {}
+-----
+def foo(/*foo*/ /*foo*/ a, b /*foo*/ /*foo*/) {}