From ffe0db4c64857d70287c5fe5cf5418558d7b80a0 Mon Sep 17 00:00:00 2001 From: Daniil Ovchinnikov Date: Mon, 9 Jul 2018 18:46:53 +0300 Subject: [PATCH] [groovy] fix label formatting (IDEA-194906) - split #generateSubBlockForCodeBlocks into two methods: #generateCodeSubBlocks and #generateSubBlocks; - use former method for code block only, flatten children independently of label setting; - use latter method for members (don't need to flatten) and children of GrLabelBlock (already flattened); - given correct blocks from above changes, update GroovyIndentProcessor to use respective settings (i.e. go down to #visitLabeledStatement instead of this werid logic in #getChildIndent). --- .../formatter/blocks/ClosureBodyBlock.java | 18 +---- .../groovy/formatter/blocks/GrLabelBlock.java | 19 +---- .../blocks/GroovyBlockGenerator.java | 76 +++++++++---------- .../processors/GroovyIndentProcessor.java | 7 -- .../lang/formatter/FormatterTest.groovy | 59 -------------- .../GroovyCodeStyleFormatterTest.groovy | 23 +++--- .../groovy/codeStyle/labelIndentAbsolute.test | 50 ++++++++++++ .../groovy/codeStyle/labelIndentRelative.test | 50 ++++++++++++ .../codeStyle/labelIndentRelativeReverse.test | 50 ++++++++++++ 9 files changed, 202 insertions(+), 150 deletions(-) create mode 100644 plugins/groovy/testdata/groovy/codeStyle/labelIndentAbsolute.test create mode 100644 plugins/groovy/testdata/groovy/codeStyle/labelIndentRelative.test create mode 100644 plugins/groovy/testdata/groovy/codeStyle/labelIndentRelativeReverse.test diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/ClosureBodyBlock.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/ClosureBodyBlock.java index 9bafca4b4b33..0b3b48cf8932 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/ClosureBodyBlock.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/ClosureBodyBlock.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2013 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package org.jetbrains.plugins.groovy.formatter.blocks; import com.intellij.formatting.Block; @@ -52,7 +38,7 @@ public class ClosureBodyBlock extends GroovyBlock { GroovyBlockGenerator generator = new GroovyBlockGenerator(this); List children = GroovyBlockGenerator.getClosureBodyVisibleChildren(myNode.getTreeParent()); - mySubBlocks = generator.generateSubBlockForCodeBlocks(false, children, myContext.getGroovySettings().INDENT_LABEL_BLOCKS); + mySubBlocks = generator.generateCodeSubBlocks(children); //at least -> exists assert !mySubBlocks.isEmpty(); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/GrLabelBlock.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/GrLabelBlock.java index b2fc055f6cc5..304877290730 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/GrLabelBlock.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/GrLabelBlock.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2014 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package org.jetbrains.plugins.groovy.formatter.blocks; import com.intellij.formatting.Block; @@ -34,14 +20,13 @@ public class GrLabelBlock extends GroovyBlockWithRange { public GrLabelBlock(@NotNull ASTNode node, List subStatements, - boolean classLevel, @NotNull Indent indent, @Nullable Wrap wrap, @NotNull FormattingContext context) { super(node, indent, createTextRange(subStatements), wrap, context); final GroovyBlockGenerator generator = new GroovyBlockGenerator(this); - myBlocks = generator.generateSubBlockForCodeBlocks(classLevel, subStatements, false); + myBlocks = generator.generateSubBlocks(subStatements, false); } private static TextRange createTextRange(List subStatements) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/GroovyBlockGenerator.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/GroovyBlockGenerator.java index b72776b47d86..b04f5dbcdacb 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/GroovyBlockGenerator.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/blocks/GroovyBlockGenerator.java @@ -27,7 +27,6 @@ import org.jetbrains.plugins.groovy.formatter.processors.GroovyWrappingProcessor import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes; import org.jetbrains.plugins.groovy.lang.lexer.TokenSets; import org.jetbrains.plugins.groovy.lang.parser.GroovyElementTypes; -import org.jetbrains.plugins.groovy.lang.parser.GroovyParserDefinition; import org.jetbrains.plugins.groovy.lang.psi.GrQualifiedReference; import org.jetbrains.plugins.groovy.lang.psi.GroovyElementVisitor; import org.jetbrains.plugins.groovy.lang.psi.GroovyFile; @@ -247,8 +246,11 @@ public class GroovyBlockGenerator { return blocks; } - if (blockPsi instanceof GrCodeBlock || blockPsi instanceof GroovyFile || classLevel) { - return generateSubBlockForCodeBlocks(classLevel, visibleChildren(myNode), myContext.getGroovySettings().INDENT_LABEL_BLOCKS); + if (blockPsi instanceof GrCodeBlock || blockPsi instanceof GroovyFile) { + return generateCodeSubBlocks(visibleChildren(myNode)); + } + if (classLevel) { + return generateSubBlocks(visibleChildren(myNode), true); } if (blockPsi instanceof GrMethod) { @@ -374,54 +376,46 @@ public class GroovyBlockGenerator { } @NotNull - public List generateSubBlockForCodeBlocks(boolean classLevel, final List children, boolean indentLabelBlocks) { - + List generateCodeSubBlocks(final List children) { final ArrayList subBlocks = new ArrayList<>(); - if (indentLabelBlocks && isCodeBlock()) { - List flattenChildren = flattenChildren(children); - calculateAlignments(flattenChildren, classLevel); - for (int i = 0; i < flattenChildren.size(); i++) { - ASTNode childNode = flattenChildren.get(i); - if (childNode.getElementType() == GroovyElementTypes.LABELED_STATEMENT) { - int start = i; - do { - i++; - } - while (i < flattenChildren.size() && - flattenChildren.get(i).getElementType() != GroovyElementTypes.LABELED_STATEMENT && - flattenChildren.get(i).getElementType() != GroovyTokenTypes.mRCURLY); - subBlocks.add( - new GrLabelBlock( - childNode, - flattenChildren.subList(start + 1, i), - classLevel, getIndent(childNode), - getChildWrap(childNode), - myContext) - ); - i--; - } - else { - subBlocks.add(new GroovyBlock(childNode, getIndent(childNode), getChildWrap(childNode), myContext)); + List flattenChildren = flattenChildren(children); + calculateAlignments(flattenChildren, false); + for (int i = 0; i < flattenChildren.size(); i++) { + ASTNode childNode = flattenChildren.get(i); + if (childNode.getElementType() == GroovyElementTypes.LABELED_STATEMENT) { + int start = i; + do { + i++; } + while (i < flattenChildren.size() && + flattenChildren.get(i).getElementType() != GroovyElementTypes.LABELED_STATEMENT && + flattenChildren.get(i).getElementType() != GroovyTokenTypes.mRCURLY); + subBlocks.add( + new GrLabelBlock( + childNode, + flattenChildren.subList(start + 1, i), + getIndent(childNode), + getChildWrap(childNode), + myContext) + ); + i--; } - } - else { - calculateAlignments(children, classLevel); - - for (ASTNode childNode : children) { + else { subBlocks.add(new GroovyBlock(childNode, getIndent(childNode), getChildWrap(childNode), myContext)); } } + return subBlocks; } - private boolean isCodeBlock() { - IElementType type = myNode.getElementType(); - return type == GroovyElementTypes.OPEN_BLOCK || - type == GroovyElementTypes.CLOSABLE_BLOCK || - type == GroovyElementTypes.CONSTRUCTOR_BODY || - type == GroovyParserDefinition.GROOVY_FILE; + List generateSubBlocks(List children, boolean classLevel) { + final List subBlocks = new ArrayList<>(); + calculateAlignments(children, classLevel); + for (ASTNode childNode : children) { + subBlocks.add(new GroovyBlock(childNode, getIndent(childNode), getChildWrap(childNode), myContext)); + } + return subBlocks; } private static List flattenChildren(List children) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/processors/GroovyIndentProcessor.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/processors/GroovyIndentProcessor.java index 3d9163168dad..ba1a17714bea 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/processors/GroovyIndentProcessor.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/formatter/processors/GroovyIndentProcessor.java @@ -10,7 +10,6 @@ import com.intellij.psi.tree.IElementType; import com.intellij.psi.tree.TokenSet; import org.jetbrains.annotations.NotNull; import org.jetbrains.plugins.groovy.formatter.blocks.ClosureBodyBlock; -import org.jetbrains.plugins.groovy.formatter.blocks.GrLabelBlock; import org.jetbrains.plugins.groovy.formatter.blocks.GroovyBlock; import org.jetbrains.plugins.groovy.lang.groovydoc.lexer.GroovyDocTokenTypes; import org.jetbrains.plugins.groovy.lang.groovydoc.psi.api.GrDocComment; @@ -87,12 +86,6 @@ public class GroovyIndentProcessor extends GroovyElementVisitor { return getNormalIndent(); } } - if (parentBlock instanceof GrLabelBlock) { - ASTNode first = parentBlock.getNode().getFirstChildNode(); - return child == first - ? getNoneIndent() - : getLabelIndent(); - } if (GSTRING_TOKENS_INNER.contains(myChildType)) { return getAbsoluteNoneIndent(); diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/formatter/FormatterTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/formatter/FormatterTest.groovy index 50ff98f13e50..37d637514f19 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/formatter/FormatterTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/formatter/FormatterTest.groovy @@ -4,7 +4,6 @@ package org.jetbrains.plugins.groovy.lang.formatter import com.intellij.openapi.util.text.StringUtil import com.intellij.psi.codeStyle.CommonCodeStyleSettings -import org.jetbrains.plugins.groovy.GroovyLanguage import org.jetbrains.plugins.groovy.codeStyle.GroovyCodeStyleSettings import org.jetbrains.plugins.groovy.util.TestUtils @@ -205,12 +204,6 @@ class FormatterTest extends GroovyFormatterTestCase { doTest() } - void _testLabelIndentAbsolute() throws Throwable { - groovySettings.indentOptions.LABEL_INDENT_ABSOLUTE = true - groovySettings.indentOptions.LABEL_INDENT_SIZE = 1 - doTest() - } - void testClosureParametersAligned() throws Throwable { groovySettings.ALIGN_MULTILINE_PARAMETERS_IN_CALLS = true doTest() @@ -831,41 +824,6 @@ print abc ? cde : xyz''') } - void testLabelsInBasicMode() { - groovySettings.indentOptions.INDENT_SIZE = 4 - groovySettings.indentOptions.LABEL_INDENT_SIZE = 2 - groovyCustomSettings.INDENT_LABEL_BLOCKS = false - - checkFormatting('''\ -def bar() { - abc: - foo() - bar() -} -''', '''\ -def bar() { - abc: - foo() - bar() -} -''') - } - - void testLabels() { - groovyCustomSettings.INDENT_LABEL_BLOCKS = false - checkFormatting('''\ -def foo() { -abc:foo() -bar() -} -''', '''\ -def foo() { - abc: foo() - bar() -} -''') - } - void testGdocAsterisks() { checkFormatting('''\ /***** @@ -891,23 +849,6 @@ def foo() { void testExtraLines() { doTest() } - void testLabelWithDescription() { - GroovyCodeStyleSettings customSettings = myTempSettings.getCustomSettings(GroovyCodeStyleSettings.class) - CommonCodeStyleSettings commonSettings = myTempSettings.getCommonSettings(GroovyLanguage.INSTANCE) - - boolean indentLabelBlocks = customSettings.INDENT_LABEL_BLOCKS - int labelIndentSize = commonSettings.indentOptions.LABEL_INDENT_SIZE - try { - customSettings.INDENT_LABEL_BLOCKS = true - commonSettings.indentOptions.LABEL_INDENT_SIZE = 2 - doTest() - } - finally { - customSettings.INDENT_LABEL_BLOCKS = indentLabelBlocks - commonSettings.indentOptions.LABEL_INDENT_SIZE = labelIndentSize - } - } - void testNoLineFeedsInGString() { doTest() } private void doGeeseTest() { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/formatter/GroovyCodeStyleFormatterTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/formatter/GroovyCodeStyleFormatterTest.groovy index 3f0d1a84dda6..c0330a9091ce 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/formatter/GroovyCodeStyleFormatterTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/formatter/GroovyCodeStyleFormatterTest.groovy @@ -1,12 +1,9 @@ // Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package org.jetbrains.plugins.groovy.lang.formatter -import com.intellij.application.options.CodeStyle -import com.intellij.psi.codeStyle.CodeStyleSettings import com.intellij.psi.codeStyle.CommonCodeStyleSettings import org.jetbrains.annotations.NotNull import org.jetbrains.annotations.Nullable -import org.jetbrains.plugins.groovy.GroovyLanguage import org.jetbrains.plugins.groovy.codeStyle.GroovyCodeStyleSettings import org.jetbrains.plugins.groovy.util.TestUtils @@ -48,13 +45,13 @@ class GroovyCodeStyleFormatterTest extends GroovyFormatterTestCase { } private List findSettings(String name) { - CodeStyleSettings settings = CodeStyle.getSettings(project) - try { - return [CommonCodeStyleSettings.getField(name), settings.getCommonSettings(GroovyLanguage.INSTANCE)] - } - catch (NoSuchFieldException ignored) { - return [GroovyCodeStyleSettings.getField(name), settings.getCustomSettings(GroovyCodeStyleSettings)] - } + return findField(CommonCodeStyleSettings, name)?.with { [it, getGroovySettings()] } + ?: findField(GroovyCodeStyleSettings, name)?.with { [it, getGroovyCustomSettings()] } + ?: findField(CommonCodeStyleSettings.IndentOptions, name)?.with { [it, getGroovySettings().getIndentOptions()] } + } + + private static Field findField(Class clazz, String name) { + return clazz.fields.find { it.name == name } } @Nullable @@ -181,4 +178,10 @@ class GroovyCodeStyleFormatterTest extends GroovyFormatterTestCase { void testArrayInitializerWrapAlwaysAlign() { doTest() } void testArrayInitializerWrapAlwaysNl() { doTest() } + + void testLabelIndentAbsolute() { doTest() } + + void testLabelIndentRelative() { doTest() } + + void testLabelIndentRelativeReverse() { doTest() } } diff --git a/plugins/groovy/testdata/groovy/codeStyle/labelIndentAbsolute.test b/plugins/groovy/testdata/groovy/codeStyle/labelIndentAbsolute.test new file mode 100644 index 000000000000..0705a52492ed --- /dev/null +++ b/plugins/groovy/testdata/groovy/codeStyle/labelIndentAbsolute.test @@ -0,0 +1,50 @@ + + + +class Foo extends spock.lang.Specification { +void 'test table formatting'() { +given: "groovy" +13 +expect: +2*2 +where: +gradleVersion|_ +'long string expression'|_ +'3.5.1'|_ +'2.14.1'|_ +'1.12'|_ +} + +void foo() { +abcd: +efgh: +1+2 +aaa:42 +lll: +111 +} +} +----- +class Foo extends spock.lang.Specification { + void 'test table formatting'() { + given: "groovy" + 13 + expect: + 2 * 2 + where: + gradleVersion | _ + 'long string expression' | _ + '3.5.1' | _ + '2.14.1' | _ + '1.12' | _ + } + + void foo() { + abcd: + efgh: + 1 + 2 + aaa: 42 + lll: + 111 + } +} \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/codeStyle/labelIndentRelative.test b/plugins/groovy/testdata/groovy/codeStyle/labelIndentRelative.test new file mode 100644 index 000000000000..79752d6bd8f2 --- /dev/null +++ b/plugins/groovy/testdata/groovy/codeStyle/labelIndentRelative.test @@ -0,0 +1,50 @@ + + + +class Foo extends spock.lang.Specification { +void 'test table formatting'() { +given: "groovy" +13 +expect: +2*2 +where: +gradleVersion|_ +'long string expression'|_ +'3.5.1'|_ +'2.14.1'|_ +'1.12'|_ +} + +void foo() { +abcd: +efgh: +1+2 +aaa:42 +lll: +111 +} +} +----- +class Foo extends spock.lang.Specification { + void 'test table formatting'() { + given: "groovy" + 13 + expect: + 2 * 2 + where: + gradleVersion | _ + 'long string expression' | _ + '3.5.1' | _ + '2.14.1' | _ + '1.12' | _ + } + + void foo() { + abcd: + efgh: + 1 + 2 + aaa: 42 + lll: + 111 + } +} \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/codeStyle/labelIndentRelativeReverse.test b/plugins/groovy/testdata/groovy/codeStyle/labelIndentRelativeReverse.test new file mode 100644 index 000000000000..71a625a5c738 --- /dev/null +++ b/plugins/groovy/testdata/groovy/codeStyle/labelIndentRelativeReverse.test @@ -0,0 +1,50 @@ + + + +class Foo extends spock.lang.Specification { +void 'test table formatting'() { +given: "groovy" +13 +expect: +2*2 +where: +gradleVersion|_ +'long string expression'|_ +'3.5.1'|_ +'2.14.1'|_ +'1.12'|_ +} + +void foo() { +abcd: +efgh: +1+2 +aaa:42 +lll: +111 +} +} +----- +class Foo extends spock.lang.Specification { + void 'test table formatting'() { + given: "groovy" + 13 + expect: + 2 * 2 + where: + gradleVersion | _ + 'long string expression' | _ + '3.5.1' | _ + '2.14.1' | _ + '1.12' | _ + } + + void foo() { + abcd: + efgh: + 1 + 2 + aaa: 42 + lll: + 111 + } +} \ No newline at end of file