Don't include unindented comments preceding a function into its text range
#KT-10591 Fixed
This commit is contained in:
@@ -7,6 +7,7 @@ package org.jetbrains.kotlin.idea.formatter
|
|||||||
|
|
||||||
import com.intellij.formatting.*
|
import com.intellij.formatting.*
|
||||||
import com.intellij.lang.ASTNode
|
import com.intellij.lang.ASTNode
|
||||||
|
import com.intellij.openapi.util.TextRange
|
||||||
import com.intellij.psi.PsiComment
|
import com.intellij.psi.PsiComment
|
||||||
import com.intellij.psi.PsiElement
|
import com.intellij.psi.PsiElement
|
||||||
import com.intellij.psi.TokenType
|
import com.intellij.psi.TokenType
|
||||||
@@ -57,18 +58,27 @@ abstract class KotlinCommonBlock(
|
|||||||
private val node: ASTNode,
|
private val node: ASTNode,
|
||||||
private val settings: CodeStyleSettings,
|
private val settings: CodeStyleSettings,
|
||||||
private val spacingBuilder: KotlinSpacingBuilder,
|
private val spacingBuilder: KotlinSpacingBuilder,
|
||||||
private val alignmentStrategy: CommonAlignmentStrategy
|
private val alignmentStrategy: CommonAlignmentStrategy,
|
||||||
|
private val overrideChildren: Sequence<ASTNode>? = null
|
||||||
) {
|
) {
|
||||||
@Volatile
|
@Volatile
|
||||||
private var mySubBlocks: List<ASTBlock>? = null
|
private var mySubBlocks: List<ASTBlock>? = null
|
||||||
|
|
||||||
|
fun getTextRange(): TextRange {
|
||||||
|
if (overrideChildren != null) {
|
||||||
|
return TextRange(overrideChildren.first().startOffset, overrideChildren.last().textRange.endOffset)
|
||||||
|
}
|
||||||
|
return node.textRange
|
||||||
|
}
|
||||||
|
|
||||||
protected abstract fun createBlock(
|
protected abstract fun createBlock(
|
||||||
node: ASTNode,
|
node: ASTNode,
|
||||||
alignmentStrategy: CommonAlignmentStrategy,
|
alignmentStrategy: CommonAlignmentStrategy,
|
||||||
indent: Indent?,
|
indent: Indent?,
|
||||||
wrap: Wrap?,
|
wrap: Wrap?,
|
||||||
settings: CodeStyleSettings,
|
settings: CodeStyleSettings,
|
||||||
spacingBuilder: KotlinSpacingBuilder
|
spacingBuilder: KotlinSpacingBuilder,
|
||||||
|
overrideChildren: Sequence<ASTNode>? = null
|
||||||
): ASTBlock
|
): ASTBlock
|
||||||
|
|
||||||
protected abstract fun createSyntheticSpacingNodeBlock(node: ASTNode): 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<ASTNode>? = null
|
||||||
|
): ASTBlock {
|
||||||
val childWrap = wrappingStrategy(child)
|
val childWrap = wrappingStrategy(child)
|
||||||
|
|
||||||
// Skip one sub-level for operators, so type of block node is an element type of operator
|
// 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),
|
createChildIndent(child),
|
||||||
childWrap,
|
childWrap,
|
||||||
settings,
|
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<ASTBlock> {
|
private fun buildSubBlocks(): List<ASTBlock> {
|
||||||
val childrenAlignmentStrategy = getChildrenAlignmentStrategy()
|
val childrenAlignmentStrategy = getChildrenAlignmentStrategy()
|
||||||
val wrappingStrategy = getWrappingStrategy()
|
val wrappingStrategy = getWrappingStrategy()
|
||||||
|
|
||||||
val childNodes = if (node.elementType == KtNodeTypes.BINARY_EXPRESSION) {
|
val childNodes = when {
|
||||||
val binaryExpressionChildren = mutableListOf<ASTNode>()
|
overrideChildren != null -> overrideChildren.asSequence()
|
||||||
collectBinaryExpressionChildren(node, binaryExpressionChildren)
|
node.elementType == KtNodeTypes.BINARY_EXPRESSION -> {
|
||||||
binaryExpressionChildren.asSequence()
|
val binaryExpressionChildren = mutableListOf<ASTNode>()
|
||||||
} else {
|
collectBinaryExpressionChildren(node, binaryExpressionChildren)
|
||||||
node.children()
|
binaryExpressionChildren.asSequence()
|
||||||
|
}
|
||||||
|
else -> node.children()
|
||||||
}
|
}
|
||||||
|
|
||||||
return childNodes
|
return childNodes
|
||||||
.filter { it.textRange.length > 0 && it.elementType != TokenType.WHITE_SPACE }
|
.filter { it.textRange.length > 0 && it.elementType != TokenType.WHITE_SPACE }
|
||||||
.map { buildSubBlock(it, childrenAlignmentStrategy, wrappingStrategy) }
|
.flatMap { buildSubBlocksForChildNode(it, childrenAlignmentStrategy, wrappingStrategy) }
|
||||||
.toList()
|
.toList()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private fun buildSubBlocksForChildNode(
|
||||||
|
node: ASTNode,
|
||||||
|
childrenAlignmentStrategy: CommonAlignmentStrategy,
|
||||||
|
wrappingStrategy: WrappingStrategy
|
||||||
|
): Sequence<ASTBlock> {
|
||||||
|
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<ASTNode>) {
|
private fun collectBinaryExpressionChildren(node: ASTNode, result: MutableList<ASTNode>) {
|
||||||
for (child in node.children()) {
|
for (child in node.children()) {
|
||||||
if (child.elementType == KtNodeTypes.BINARY_EXPRESSION) {
|
if (child.elementType == KtNodeTypes.BINARY_EXPRESSION) {
|
||||||
@@ -641,7 +684,10 @@ private val INDENT_RULES = arrayOf(
|
|||||||
|
|
||||||
strategy("Indent for parts")
|
strategy("Indent for parts")
|
||||||
.within(KtNodeTypes.PROPERTY, KtNodeTypes.FUN, KtNodeTypes.DESTRUCTURING_DECLARATION, KtNodeTypes.SECONDARY_CONSTRUCTOR)
|
.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()),
|
.set(Indent.getContinuationWithoutFirstIndent()),
|
||||||
|
|
||||||
strategy("Chained calls")
|
strategy("Chained calls")
|
||||||
@@ -814,3 +860,10 @@ private fun getWrappingStrategyForItemList(wrapType: Int, itemTypes: TokenSet, w
|
|||||||
private fun List<ASTBlock>.indexOfBlockWithType(tokenSet: TokenSet): Int {
|
private fun List<ASTBlock>.indexOfBlockWithType(tokenSet: TokenSet): Int {
|
||||||
return indexOfFirst { block -> block.node?.elementType in tokenSet }
|
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)
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,17 +1,6 @@
|
|||||||
/*
|
/*
|
||||||
* Copyright 2010-2016 JetBrains s.r.o.
|
* 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.
|
||||||
* 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.
|
|
||||||
*/
|
*/
|
||||||
|
|
||||||
package org.jetbrains.kotlin.idea.formatter
|
package org.jetbrains.kotlin.idea.formatter
|
||||||
@@ -31,14 +20,18 @@ import org.jetbrains.kotlin.lexer.KtTokens.ARROW
|
|||||||
* @see Block for good JavaDoc documentation
|
* @see Block for good JavaDoc documentation
|
||||||
*/
|
*/
|
||||||
class KotlinBlock(
|
class KotlinBlock(
|
||||||
node: ASTNode,
|
node: ASTNode,
|
||||||
private val myAlignmentStrategy: CommonAlignmentStrategy,
|
private val myAlignmentStrategy: CommonAlignmentStrategy,
|
||||||
private val myIndent: Indent?,
|
private val myIndent: Indent?,
|
||||||
wrap: Wrap?,
|
wrap: Wrap?,
|
||||||
mySettings: CodeStyleSettings,
|
mySettings: CodeStyleSettings,
|
||||||
private val mySpacingBuilder: KotlinSpacingBuilder) : AbstractBlock(node, wrap, myAlignmentStrategy.getAlignment(node)) {
|
private val mySpacingBuilder: KotlinSpacingBuilder,
|
||||||
|
overrideChildren: Sequence<ASTNode>? = 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 getNullAlignmentStrategy(): CommonAlignmentStrategy = NodeAlignmentStrategy.getNullStrategy()
|
||||||
|
|
||||||
override fun createAlignmentStrategy(alignOption: Boolean, defaultAlignment: Alignment?): CommonAlignmentStrategy {
|
override fun createAlignmentStrategy(alignOption: Boolean, defaultAlignment: Alignment?): CommonAlignmentStrategy {
|
||||||
@@ -48,9 +41,9 @@ class KotlinBlock(
|
|||||||
override fun getAlignmentForCaseBranch(shouldAlignInColumns: Boolean): CommonAlignmentStrategy {
|
override fun getAlignmentForCaseBranch(shouldAlignInColumns: Boolean): CommonAlignmentStrategy {
|
||||||
return if (shouldAlignInColumns) {
|
return if (shouldAlignInColumns) {
|
||||||
NodeAlignmentStrategy.fromTypes(
|
NodeAlignmentStrategy.fromTypes(
|
||||||
AlignmentStrategy.createAlignmentPerTypeStrategy(listOf(ARROW as IElementType), WHEN_ENTRY, true))
|
AlignmentStrategy.createAlignmentPerTypeStrategy(listOf(ARROW as IElementType), WHEN_ENTRY, true)
|
||||||
}
|
)
|
||||||
else {
|
} else {
|
||||||
NodeAlignmentStrategy.getNullStrategy()
|
NodeAlignmentStrategy.getNullStrategy()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -63,14 +56,24 @@ class KotlinBlock(
|
|||||||
|
|
||||||
override fun getSubBlocks(): List<Block> = subBlocks
|
override fun getSubBlocks(): List<Block> = 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<ASTNode>?
|
||||||
|
): ASTBlock {
|
||||||
return KotlinBlock(
|
return KotlinBlock(
|
||||||
node,
|
node,
|
||||||
alignmentStrategy,
|
alignmentStrategy,
|
||||||
indent,
|
indent,
|
||||||
wrap,
|
wrap,
|
||||||
mySettings,
|
mySettings,
|
||||||
mySpacingBuilder)
|
mySpacingBuilder,
|
||||||
|
overrideChildren
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun createSyntheticSpacingNodeBlock(node: ASTNode): ASTBlock {
|
override fun createSyntheticSpacingNodeBlock(node: ASTNode): ASTBlock {
|
||||||
@@ -91,6 +94,8 @@ class KotlinBlock(
|
|||||||
override fun getChildAttributes(newChildIndex: Int): ChildAttributes = kotlinDelegationBlock.getChildAttributes(newChildIndex)
|
override fun getChildAttributes(newChildIndex: Int): ChildAttributes = kotlinDelegationBlock.getChildAttributes(newChildIndex)
|
||||||
|
|
||||||
override fun isLeaf(): Boolean = kotlinDelegationBlock.isLeaf()
|
override fun isLeaf(): Boolean = kotlinDelegationBlock.isLeaf()
|
||||||
|
|
||||||
|
override fun getTextRange() = kotlinDelegationBlock.getTextRange()
|
||||||
}
|
}
|
||||||
|
|
||||||
object KotlinSpacingBuilderUtilImpl : KotlinSpacingBuilderUtil {
|
object KotlinSpacingBuilderUtilImpl : KotlinSpacingBuilderUtil {
|
||||||
@@ -103,13 +108,14 @@ object KotlinSpacingBuilderUtilImpl : KotlinSpacingBuilderUtil {
|
|||||||
}
|
}
|
||||||
|
|
||||||
override fun createLineFeedDependentSpacing(
|
override fun createLineFeedDependentSpacing(
|
||||||
minSpaces: Int,
|
minSpaces: Int,
|
||||||
maxSpaces: Int,
|
maxSpaces: Int,
|
||||||
minimumLineFeeds: Int,
|
minimumLineFeeds: Int,
|
||||||
keepLineBreaks: Boolean,
|
keepLineBreaks: Boolean,
|
||||||
keepBlankLines: Int,
|
keepBlankLines: Int,
|
||||||
dependency: TextRange,
|
dependency: TextRange,
|
||||||
rule: DependentSpacingRule): Spacing {
|
rule: DependentSpacingRule
|
||||||
|
): Spacing {
|
||||||
return object : DependantSpacingImpl(minSpaces, maxSpaces, dependency, keepLineBreaks, keepBlankLines, rule) {
|
return object : DependantSpacingImpl(minSpaces, maxSpaces, dependency, keepLineBreaks, keepBlankLines, rule) {
|
||||||
override fun getMinLineFeeds(): Int {
|
override fun getMinLineFeeds(): Int {
|
||||||
val superMin = super.getMinLineFeeds()
|
val superMin = super.getMinLineFeeds()
|
||||||
|
|||||||
+2
-4
@@ -1,10 +1,8 @@
|
|||||||
package format.test
|
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 {
|
class LineComments {
|
||||||
// Should not be formatted
|
// Should not be formatted
|
||||||
// Format
|
// Format
|
||||||
// Format
|
// Format
|
||||||
fun test() {
|
fun test() {
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-4
@@ -1,10 +1,8 @@
|
|||||||
package format.test
|
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 {
|
class LineComments {
|
||||||
// Should not be formatted
|
// Should not be formatted
|
||||||
// Format
|
// Format
|
||||||
// Format
|
// Format
|
||||||
fun test() {
|
fun test() {
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,7 +1,5 @@
|
|||||||
package format.test
|
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 {
|
class LineComments {
|
||||||
// Should not be formatted
|
// Should not be formatted
|
||||||
// Format
|
// Format
|
||||||
|
|||||||
@@ -0,0 +1,6 @@
|
|||||||
|
object Test {
|
||||||
|
// some
|
||||||
|
fun test() {
|
||||||
|
<caret>
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,5 @@
|
|||||||
|
object Test {
|
||||||
|
// some
|
||||||
|
fun test() {<caret>
|
||||||
|
}
|
||||||
|
}
|
||||||
+6
@@ -261,6 +261,12 @@ public class TypingIndentationTestBaseGenerated extends AbstractTypingIndentatio
|
|||||||
doNewlineTest(fileName);
|
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")
|
@TestMetadata("MultideclarationAfterEq.after.kt")
|
||||||
public void testMultideclarationAfterEq() throws Exception {
|
public void testMultideclarationAfterEq() throws Exception {
|
||||||
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/indentationOnNewline/MultideclarationAfterEq.after.kt");
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/indentationOnNewline/MultideclarationAfterEq.after.kt");
|
||||||
|
|||||||
Reference in New Issue
Block a user