From 106e1b866119ccef85afdf6d3ece3d940e1af8e9 Mon Sep 17 00:00:00 2001 From: Dmitry Jemerov Date: Fri, 12 Jan 2018 15:48:01 +0100 Subject: [PATCH] Don't include unindented comments preceding a function into its text range #KT-10591 Fixed --- .../idea/formatter/KotlinCommonBlock.kt | 79 +++++++++++++++--- .../kotlin/idea/formatter/KotlinBlock.kt | 80 ++++++++++--------- ...lumnCommentsBeforeDeclaration.after.inv.kt | 6 +- ...stColumnCommentsBeforeDeclaration.after.kt | 6 +- ...matFirstColumnCommentsBeforeDeclaration.kt | 2 - .../indentationOnNewline/KT20783.after.kt | 6 ++ idea/testData/indentationOnNewline/KT20783.kt | 5 ++ .../TypingIndentationTestBaseGenerated.java | 6 ++ 8 files changed, 130 insertions(+), 60 deletions(-) create mode 100644 idea/testData/indentationOnNewline/KT20783.after.kt create mode 100644 idea/testData/indentationOnNewline/KT20783.kt diff --git a/idea/formatter/src/org/jetbrains/kotlin/idea/formatter/KotlinCommonBlock.kt b/idea/formatter/src/org/jetbrains/kotlin/idea/formatter/KotlinCommonBlock.kt index 495ffd23417..6fac3f847f0 100644 --- a/idea/formatter/src/org/jetbrains/kotlin/idea/formatter/KotlinCommonBlock.kt +++ b/idea/formatter/src/org/jetbrains/kotlin/idea/formatter/KotlinCommonBlock.kt @@ -7,6 +7,7 @@ package org.jetbrains.kotlin.idea.formatter import com.intellij.formatting.* import com.intellij.lang.ASTNode +import com.intellij.openapi.util.TextRange import com.intellij.psi.PsiComment import com.intellij.psi.PsiElement import com.intellij.psi.TokenType @@ -57,18 +58,27 @@ abstract class KotlinCommonBlock( private val node: ASTNode, private val settings: CodeStyleSettings, private val spacingBuilder: KotlinSpacingBuilder, - private val alignmentStrategy: CommonAlignmentStrategy + private val alignmentStrategy: CommonAlignmentStrategy, + private val overrideChildren: Sequence? = null ) { @Volatile private var mySubBlocks: List? = null + fun getTextRange(): TextRange { + if (overrideChildren != null) { + return TextRange(overrideChildren.first().startOffset, overrideChildren.last().textRange.endOffset) + } + return node.textRange + } + protected abstract fun createBlock( node: ASTNode, alignmentStrategy: CommonAlignmentStrategy, indent: Indent?, wrap: Wrap?, settings: CodeStyleSettings, - spacingBuilder: KotlinSpacingBuilder + spacingBuilder: KotlinSpacingBuilder, + overrideChildren: Sequence? = null ): ASTBlock protected abstract fun createSyntheticSpacingNodeBlock(node: ASTNode): ASTBlock @@ -355,7 +365,12 @@ abstract class KotlinCommonBlock( } - private fun buildSubBlock(child: ASTNode, alignmentStrategy: CommonAlignmentStrategy, wrappingStrategy: WrappingStrategy): ASTBlock { + private fun buildSubBlock( + child: ASTNode, + alignmentStrategy: CommonAlignmentStrategy, + wrappingStrategy: WrappingStrategy, + overrideChildren: Sequence? = null + ): ASTBlock { val childWrap = wrappingStrategy(child) // Skip one sub-level for operators, so type of block node is an element type of operator @@ -368,32 +383,60 @@ abstract class KotlinCommonBlock( createChildIndent(child), childWrap, settings, - spacingBuilder + spacingBuilder, + overrideChildren ) } } - return createBlock(child, alignmentStrategy, createChildIndent(child), childWrap, settings, spacingBuilder) + return createBlock(child, alignmentStrategy, createChildIndent(child), childWrap, settings, spacingBuilder, overrideChildren) } private fun buildSubBlocks(): List { val childrenAlignmentStrategy = getChildrenAlignmentStrategy() val wrappingStrategy = getWrappingStrategy() - val childNodes = if (node.elementType == KtNodeTypes.BINARY_EXPRESSION) { - val binaryExpressionChildren = mutableListOf() - collectBinaryExpressionChildren(node, binaryExpressionChildren) - binaryExpressionChildren.asSequence() - } else { - node.children() + val childNodes = when { + overrideChildren != null -> overrideChildren.asSequence() + node.elementType == KtNodeTypes.BINARY_EXPRESSION -> { + val binaryExpressionChildren = mutableListOf() + collectBinaryExpressionChildren(node, binaryExpressionChildren) + binaryExpressionChildren.asSequence() + } + else -> node.children() } return childNodes .filter { it.textRange.length > 0 && it.elementType != TokenType.WHITE_SPACE } - .map { buildSubBlock(it, childrenAlignmentStrategy, wrappingStrategy) } + .flatMap { buildSubBlocksForChildNode(it, childrenAlignmentStrategy, wrappingStrategy) } .toList() } + private fun buildSubBlocksForChildNode( + node: ASTNode, + childrenAlignmentStrategy: CommonAlignmentStrategy, + wrappingStrategy: WrappingStrategy + ): Sequence { + if (node.elementType == KtNodeTypes.FUN) { + val filteredChildren = node.children().filter { + it.textRange.length > 0 && it.elementType != TokenType.WHITE_SPACE + } + val significantChildren = filteredChildren.dropWhile { it.elementType == KtTokens.EOL_COMMENT } + val funIndent = extractIndent(significantChildren.first()) + val eolComments = filteredChildren.takeWhile { + it.elementType == KtTokens.EOL_COMMENT && extractIndent(it) != funIndent + }.toList() + val remainingChildren = filteredChildren.drop(eolComments.size) + + val blocks = eolComments.map { buildSubBlock(it, childrenAlignmentStrategy, wrappingStrategy) } + + sequenceOf(buildSubBlock(node, childrenAlignmentStrategy, wrappingStrategy, remainingChildren)) + val blockList = blocks.toList() + return blockList.asSequence() + } + + return sequenceOf(buildSubBlock(node, childrenAlignmentStrategy, wrappingStrategy)) + } + private fun collectBinaryExpressionChildren(node: ASTNode, result: MutableList) { for (child in node.children()) { if (child.elementType == KtNodeTypes.BINARY_EXPRESSION) { @@ -641,7 +684,10 @@ private val INDENT_RULES = arrayOf( strategy("Indent for parts") .within(KtNodeTypes.PROPERTY, KtNodeTypes.FUN, KtNodeTypes.DESTRUCTURING_DECLARATION, KtNodeTypes.SECONDARY_CONSTRUCTOR) - .notForType(KtNodeTypes.BLOCK, FUN_KEYWORD, VAL_KEYWORD, VAR_KEYWORD, CONSTRUCTOR_KEYWORD, KtTokens.RPAR) + .notForType( + KtNodeTypes.BLOCK, FUN_KEYWORD, VAL_KEYWORD, VAR_KEYWORD, CONSTRUCTOR_KEYWORD, KtTokens.RPAR, + KtTokens.EOL_COMMENT + ) .set(Indent.getContinuationWithoutFirstIndent()), strategy("Chained calls") @@ -814,3 +860,10 @@ private fun getWrappingStrategyForItemList(wrapType: Int, itemTypes: TokenSet, w private fun List.indexOfBlockWithType(tokenSet: TokenSet): Int { return indexOfFirst { block -> block.node?.elementType in tokenSet } } + +private fun extractIndent(node: ASTNode): String { + val prevNode = node.treePrev + if (prevNode?.elementType != TokenType.WHITE_SPACE) + return "" + return prevNode.text.substringAfterLast("\n", prevNode.text) +} diff --git a/idea/src/org/jetbrains/kotlin/idea/formatter/KotlinBlock.kt b/idea/src/org/jetbrains/kotlin/idea/formatter/KotlinBlock.kt index 93cbe682e71..b3e41b5951e 100644 --- a/idea/src/org/jetbrains/kotlin/idea/formatter/KotlinBlock.kt +++ b/idea/src/org/jetbrains/kotlin/idea/formatter/KotlinBlock.kt @@ -1,17 +1,6 @@ /* - * Copyright 2010-2016 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/LICENSE.txt file. */ package org.jetbrains.kotlin.idea.formatter @@ -31,14 +20,18 @@ import org.jetbrains.kotlin.lexer.KtTokens.ARROW * @see Block for good JavaDoc documentation */ class KotlinBlock( - node: ASTNode, - private val myAlignmentStrategy: CommonAlignmentStrategy, - private val myIndent: Indent?, - wrap: Wrap?, - mySettings: CodeStyleSettings, - private val mySpacingBuilder: KotlinSpacingBuilder) : AbstractBlock(node, wrap, myAlignmentStrategy.getAlignment(node)) { + node: ASTNode, + private val myAlignmentStrategy: CommonAlignmentStrategy, + private val myIndent: Indent?, + wrap: Wrap?, + mySettings: CodeStyleSettings, + private val mySpacingBuilder: KotlinSpacingBuilder, + overrideChildren: Sequence? = null +) : AbstractBlock(node, wrap, myAlignmentStrategy.getAlignment(node)) { - private val kotlinDelegationBlock = object : KotlinCommonBlock(node, mySettings, mySpacingBuilder, myAlignmentStrategy) { + private val kotlinDelegationBlock = object : KotlinCommonBlock( + node, mySettings, mySpacingBuilder, myAlignmentStrategy, overrideChildren + ) { override fun getNullAlignmentStrategy(): CommonAlignmentStrategy = NodeAlignmentStrategy.getNullStrategy() override fun createAlignmentStrategy(alignOption: Boolean, defaultAlignment: Alignment?): CommonAlignmentStrategy { @@ -48,9 +41,9 @@ class KotlinBlock( override fun getAlignmentForCaseBranch(shouldAlignInColumns: Boolean): CommonAlignmentStrategy { return if (shouldAlignInColumns) { NodeAlignmentStrategy.fromTypes( - AlignmentStrategy.createAlignmentPerTypeStrategy(listOf(ARROW as IElementType), WHEN_ENTRY, true)) - } - else { + AlignmentStrategy.createAlignmentPerTypeStrategy(listOf(ARROW as IElementType), WHEN_ENTRY, true) + ) + } else { NodeAlignmentStrategy.getNullStrategy() } } @@ -63,14 +56,24 @@ class KotlinBlock( override fun getSubBlocks(): List = subBlocks - override fun createBlock(node: ASTNode, alignmentStrategy: CommonAlignmentStrategy, indent: Indent?, wrap: Wrap?, settings: CodeStyleSettings, spacingBuilder: KotlinSpacingBuilder): ASTBlock { + override fun createBlock( + node: ASTNode, + alignmentStrategy: CommonAlignmentStrategy, + indent: Indent?, + wrap: Wrap?, + settings: CodeStyleSettings, + spacingBuilder: KotlinSpacingBuilder, + overrideChildren: Sequence? + ): ASTBlock { return KotlinBlock( - node, - alignmentStrategy, - indent, - wrap, - mySettings, - mySpacingBuilder) + node, + alignmentStrategy, + indent, + wrap, + mySettings, + mySpacingBuilder, + overrideChildren + ) } override fun createSyntheticSpacingNodeBlock(node: ASTNode): ASTBlock { @@ -91,6 +94,8 @@ class KotlinBlock( override fun getChildAttributes(newChildIndex: Int): ChildAttributes = kotlinDelegationBlock.getChildAttributes(newChildIndex) override fun isLeaf(): Boolean = kotlinDelegationBlock.isLeaf() + + override fun getTextRange() = kotlinDelegationBlock.getTextRange() } object KotlinSpacingBuilderUtilImpl : KotlinSpacingBuilderUtil { @@ -103,13 +108,14 @@ object KotlinSpacingBuilderUtilImpl : KotlinSpacingBuilderUtil { } override fun createLineFeedDependentSpacing( - minSpaces: Int, - maxSpaces: Int, - minimumLineFeeds: Int, - keepLineBreaks: Boolean, - keepBlankLines: Int, - dependency: TextRange, - rule: DependentSpacingRule): Spacing { + minSpaces: Int, + maxSpaces: Int, + minimumLineFeeds: Int, + keepLineBreaks: Boolean, + keepBlankLines: Int, + dependency: TextRange, + rule: DependentSpacingRule + ): Spacing { return object : DependantSpacingImpl(minSpaces, maxSpaces, dependency, keepLineBreaks, keepBlankLines, rule) { override fun getMinLineFeeds(): Int { val superMin = super.getMinLineFeeds() diff --git a/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.after.inv.kt b/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.after.inv.kt index 311308eba53..384f3a69f23 100644 --- a/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.after.inv.kt +++ b/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.after.inv.kt @@ -1,10 +1,8 @@ package format.test -// TODO: Comment on first column shouldn't be formatted, but now there's no way to adjust rule for the first comment in parent declaration. - class LineComments { - // Should not be formatted - // Format +// Should not be formatted +// Format // Format fun test() { } diff --git a/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.after.kt b/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.after.kt index 311308eba53..384f3a69f23 100644 --- a/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.after.kt +++ b/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.after.kt @@ -1,10 +1,8 @@ package format.test -// TODO: Comment on first column shouldn't be formatted, but now there's no way to adjust rule for the first comment in parent declaration. - class LineComments { - // Should not be formatted - // Format +// Should not be formatted +// Format // Format fun test() { } diff --git a/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.kt b/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.kt index ba0c1442524..c950e13522c 100644 --- a/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.kt +++ b/idea/testData/formatter/FormatFirstColumnCommentsBeforeDeclaration.kt @@ -1,7 +1,5 @@ package format.test -// TODO: Comment on first column shouldn't be formatted, but now there's no way to adjust rule for the first comment in parent declaration. - class LineComments { // Should not be formatted // Format diff --git a/idea/testData/indentationOnNewline/KT20783.after.kt b/idea/testData/indentationOnNewline/KT20783.after.kt new file mode 100644 index 00000000000..4c64b2e755e --- /dev/null +++ b/idea/testData/indentationOnNewline/KT20783.after.kt @@ -0,0 +1,6 @@ +object Test { +// some + fun test() { + + } +} \ No newline at end of file diff --git a/idea/testData/indentationOnNewline/KT20783.kt b/idea/testData/indentationOnNewline/KT20783.kt new file mode 100644 index 00000000000..d194e15c571 --- /dev/null +++ b/idea/testData/indentationOnNewline/KT20783.kt @@ -0,0 +1,5 @@ +object Test { +// some + fun test() { + } +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/formatter/TypingIndentationTestBaseGenerated.java b/idea/tests/org/jetbrains/kotlin/formatter/TypingIndentationTestBaseGenerated.java index dbb565e8b7a..ba85636373e 100644 --- a/idea/tests/org/jetbrains/kotlin/formatter/TypingIndentationTestBaseGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/formatter/TypingIndentationTestBaseGenerated.java @@ -261,6 +261,12 @@ public class TypingIndentationTestBaseGenerated extends AbstractTypingIndentatio doNewlineTest(fileName); } + @TestMetadata("KT20783.after.kt") + public void testKT20783() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/indentationOnNewline/KT20783.after.kt"); + doNewlineTest(fileName); + } + @TestMetadata("MultideclarationAfterEq.after.kt") public void testMultideclarationAfterEq() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/indentationOnNewline/MultideclarationAfterEq.after.kt");