Correctly handle end-of-line comment in "Add braces to if"

#KT-27408 Fixed
This commit is contained in:
Timo Obereder
2018-10-23 22:33:11 +03:00
committed by Mikhail Glukhikh
parent 61d3b6ee1f
commit 3c75d46328
9 changed files with 108 additions and 26 deletions
@@ -123,9 +123,9 @@ class CommentSaver(originalElements: PsiChildRange, private val saveLineBreaks:
private class LineBreakTreeElement() : TreeElement() private class LineBreakTreeElement() : TreeElement()
private class CommentTreeElement( private class CommentTreeElement(
val commentText: String, val commentText: String,
val spaceBefore: String, val spaceBefore: String,
val spaceAfter: String val spaceAfter: String
) : TreeElement() { ) : TreeElement() {
companion object { companion object {
fun create(comment: PsiComment): CommentTreeElement { fun create(comment: PsiComment): CommentTreeElement {
@@ -234,11 +234,15 @@ class CommentSaver(originalElements: PsiChildRange, private val saveLineBreaks:
putNewElementIntoMap(newPsiElement, treeElement) putNewElementIntoMap(newPsiElement, treeElement)
} }
fun restore(resultElement: PsiElement, isCommentBeneathSingleLine: Boolean, forceAdjustIndent: Boolean = false) {
restore(PsiChildRange.singleElement(resultElement), forceAdjustIndent, isCommentBeneathSingleLine)
}
fun restore(resultElement: PsiElement, forceAdjustIndent: Boolean = false) { fun restore(resultElement: PsiElement, forceAdjustIndent: Boolean = false) {
restore(PsiChildRange.singleElement(resultElement), forceAdjustIndent) restore(PsiChildRange.singleElement(resultElement), forceAdjustIndent)
} }
fun restore(resultElements: PsiChildRange, forceAdjustIndent: Boolean = false) { fun restore(resultElements: PsiChildRange, forceAdjustIndent: Boolean = false, isCommentBeneathSingleLine: Boolean = false) {
assert(!isFinished) assert(!isFinished)
assert(!resultElements.isEmpty) assert(!resultElements.isEmpty)
@@ -260,7 +264,7 @@ class CommentSaver(originalElements: PsiChildRange, private val saveLineBreaks:
}) })
} }
restoreComments(resultElements) restoreComments(resultElements, isCommentBeneathSingleLine)
restoreLineBreaks() restoreLineBreaks()
@@ -291,7 +295,7 @@ class CommentSaver(originalElements: PsiChildRange, private val saveLineBreaks:
isFinished = true isFinished = true
} }
private fun restoreComments(resultElements: PsiChildRange) { private fun restoreComments(resultElements: PsiChildRange, isCommentBeneathSingleLine: Boolean = false) {
var putAbandonedCommentsAfter = resultElements.last!! var putAbandonedCommentsAfter = resultElements.last!!
for (commentTreeElement in commentsToRestore) { for (commentTreeElement in commentsToRestore) {
@@ -316,25 +320,30 @@ class CommentSaver(originalElements: PsiChildRange, private val saveLineBreaks:
val whiteSpace = restored.nextLeaf(skipEmptyElements = true) as? PsiWhiteSpace val whiteSpace = restored.nextLeaf(skipEmptyElements = true) as? PsiWhiteSpace
if (whiteSpace == null) { if (whiteSpace == null) {
parent.addAfter(psiFactory.createWhiteSpace("\n"), restored) parent.addAfter(psiFactory.createWhiteSpace("\n"), restored)
} } else if (!whiteSpace.textContains('\n')) {
else if (!whiteSpace.textContains('\n')) {
val newWhiteSpace = psiFactory.createWhiteSpace("\n" + whiteSpace.text) val newWhiteSpace = psiFactory.createWhiteSpace("\n" + whiteSpace.text)
whiteSpace.replace(newWhiteSpace) whiteSpace.replace(newWhiteSpace)
} }
} }
} } else {
else {
restored = parent.addBefore(comment, anchorElement) as PsiComment restored = parent.addBefore(comment, anchorElement) as PsiComment
if (commentTreeElement.spaceAfter.isNotEmpty()) { if (commentTreeElement.spaceAfter.isNotEmpty()) {
parent.addBefore(psiFactory.createWhiteSpace(commentTreeElement.spaceAfter), anchorElement) parent.addBefore(psiFactory.createWhiteSpace(commentTreeElement.spaceAfter), anchorElement)
} }
} }
} } else {
else {
restored = putAbandonedCommentsAfter.parent.addBefore(comment, putAbandonedCommentsAfter) as PsiComment restored = putAbandonedCommentsAfter.parent.addBefore(comment, putAbandonedCommentsAfter) as PsiComment
putAbandonedCommentsAfter = restored putAbandonedCommentsAfter = restored
} }
// shift (possible contained) comment in expression underneath braces
if (isCommentBeneathSingleLine && resultElements.count() == 1) {
val element = resultElements.first
element?.add(psiFactory.createWhiteSpace("\n"))
element?.add(restored)
restored.delete()
}
bindNewElement(restored, commentTreeElement) // will be used when restoring line breaks bindNewElement(restored, commentTreeElement) // will be used when restoring line breaks
if (restored.tokenType == KtTokens.EOL_COMMENT) { if (restored.tokenType == KtTokens.EOL_COMMENT) {
@@ -352,9 +361,9 @@ class CommentSaver(originalElements: PsiChildRange, private val saveLineBreaks:
fun findRestored(leaf: TreeElement): PsiElement? { fun findRestored(leaf: TreeElement): PsiElement? {
if (leaf is LineBreakTreeElement) return null if (leaf is LineBreakTreeElement) return null
return leaf.parentsWithSelf return leaf.parentsWithSelf
.takeWhile { it != lineBreakParent } .takeWhile { it != lineBreakParent }
.mapNotNull { toNewPsiElementMap[it]?.first() } //TODO: what about multiple? .mapNotNull { toNewPsiElementMap[it]?.first() } //TODO: what about multiple?
.firstOrNull() .firstOrNull()
} }
@@ -365,8 +374,7 @@ class CommentSaver(originalElements: PsiChildRange, private val saveLineBreaks:
psiElement = skipTokensForward(psiElement, tokensToMatch.asReversed()) psiElement = skipTokensForward(psiElement, tokensToMatch.asReversed())
psiElement?.restoreLineBreakAfter() psiElement?.restoreLineBreakAfter()
break break
} } else {
else {
if (leaf !is TokenTreeElement) break if (leaf !is TokenTreeElement) break
tokensToMatch.add(leaf.tokenType) tokensToMatch.add(leaf.tokenType)
} }
@@ -391,8 +399,7 @@ class CommentSaver(originalElements: PsiChildRange, private val saveLineBreaks:
if (whitespace == null) { if (whitespace == null) {
addAfter.parent.addAfter(psiFactory.createNewLine(), addAfter) addAfter.parent.addAfter(psiFactory.createNewLine(), addAfter)
} } else {
else {
whitespace.replace(psiFactory.createWhiteSpace("\n" + whitespace.text)) whitespace.replace(psiFactory.createWhiteSpace("\n" + whitespace.text))
} }
@@ -419,8 +426,8 @@ class CommentSaver(originalElements: PsiChildRange, private val saveLineBreaks:
private fun PsiElement.anchorToAddCommentOrSpace(before: Boolean): PsiElement { private fun PsiElement.anchorToAddCommentOrSpace(before: Boolean): PsiElement {
return parentsWithSelf return parentsWithSelf
.dropWhile { it.parent !is PsiFile && (if (before) it.prevSibling else it.nextSibling) == null } .dropWhile { it.parent !is PsiFile && (if (before) it.prevSibling else it.nextSibling) == null }
.first() .first()
} }
private fun chooseAnchor(anchorBefore: Anchor?, anchorAfter: Anchor?): Anchor? { private fun chooseAnchor(anchorBefore: Anchor?, anchorAfter: Anchor?): Anchor? {
@@ -17,14 +17,15 @@
package org.jetbrains.kotlin.idea.intentions package org.jetbrains.kotlin.idea.intentions
import com.intellij.openapi.editor.Editor import com.intellij.openapi.editor.Editor
import com.intellij.psi.PsiComment import com.intellij.psi.*
import com.intellij.psi.PsiWhiteSpace import org.jetbrains.kotlin.idea.refactoring.getLineCount
import org.jetbrains.kotlin.idea.refactoring.getLineNumber import org.jetbrains.kotlin.idea.refactoring.getLineNumber
import org.jetbrains.kotlin.idea.util.CommentSaver import org.jetbrains.kotlin.idea.util.CommentSaver
import org.jetbrains.kotlin.j2k.isInSingleLine
import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.* import org.jetbrains.kotlin.psi.psiUtil.*
import java.lang.IllegalArgumentException import org.jetbrains.kotlin.resolve.calls.CallExpressionElement
class AddBracesIntention : SelfTargetingIntention<KtElement>(KtElement::class.java, "Add braces") { class AddBracesIntention : SelfTargetingIntention<KtElement>(KtElement::class.java, "Add braces") {
override fun isApplicableTo(element: KtElement, caretOffset: Int): Boolean { override fun isApplicableTo(element: KtElement, caretOffset: Int): Boolean {
@@ -51,7 +52,7 @@ class AddBracesIntention : SelfTargetingIntention<KtElement>(KtElement::class.ja
override fun applyTo(element: KtElement, editor: Editor?) { override fun applyTo(element: KtElement, editor: Editor?) {
if (editor == null) throw IllegalArgumentException("This intention requires an editor") if (editor == null) throw IllegalArgumentException("This intention requires an editor")
val expression = element.getTargetExpression(editor.caretModel.offset)!! val expression = element.getTargetExpression(editor.caretModel.offset)!!
var isCommentBeneath = false
val psiFactory = KtPsiFactory(element) val psiFactory = KtPsiFactory(element)
val semicolon = element.getNextSiblingIgnoringWhitespaceAndComments()?.takeIf { it.node.elementType == KtTokens.SEMICOLON } val semicolon = element.getNextSiblingIgnoringWhitespaceAndComments()?.takeIf { it.node.elementType == KtTokens.SEMICOLON }
@@ -63,6 +64,21 @@ class AddBracesIntention : SelfTargetingIntention<KtElement>(KtElement::class.ja
semicolon.delete() semicolon.delete()
} }
// Check for single line if expression
if (element is KtIfExpression && expression.isInSingleLine() && element.`else` == null) {
// Check if a comment is actually underneath (\n) the expression
val allElements = element.siblings(withItself = false).filterIsInstance<PsiElement>()
val sibling = allElements.firstOrNull { it is PsiComment }
if (sibling is PsiComment) {
// Check if \n before first received comment sibling
// if false, the normal procedure of adding braces occurs.
isCommentBeneath =
sibling.prevSibling is PsiWhiteSpace &&
sibling.prevSibling.textContains('\n') &&
(sibling.prevSibling.prevSibling is PsiComment || sibling.prevSibling.prevSibling is PsiElement)
}
}
val nextComment = when { val nextComment = when {
element is KtDoWhileExpression -> null // bound to the closing while element is KtDoWhileExpression -> null // bound to the closing while
element is KtIfExpression && expression === element.then && element.`else` != null -> null // bound to else element is KtIfExpression && expression === element.then && element.`else` != null -> null // bound to else
@@ -83,7 +99,9 @@ class AddBracesIntention : SelfTargetingIntention<KtElement>(KtElement::class.ja
is KtIfExpression -> is KtIfExpression ->
(result?.parent?.nextSibling as? PsiWhiteSpace)?.delete() (result?.parent?.nextSibling as? PsiWhiteSpace)?.delete()
} }
saver.restore(result)
// Check for single line expression with comment beneath.
saver.restore(result, isCommentBeneath, forceAdjustIndent = false)
} }
private fun KtElement.getTargetExpression(caretLocation: Int): KtExpression? { private fun KtElement.getTargetExpression(caretLocation: Int): KtExpression? {
@@ -0,0 +1,6 @@
fun foo() {}
fun test(b: Boolean) {
<caret>if (b) foo()
// ccc
}
@@ -0,0 +1,8 @@
fun foo() {}
fun test(b: Boolean) {
if (b) {
foo()
}
// ccc
}
@@ -0,0 +1,6 @@
fun foo() {}
fun test(b: Boolean) {
<caret>if (b) foo() // bbb
// ccc
}
@@ -0,0 +1,8 @@
fun foo() {}
fun test(b: Boolean) {
if (b) {
foo() // bbb
}
// ccc
}
@@ -0,0 +1,6 @@
fun foo() {}
fun test(b: Boolean) {
<caret>if (b) /* aaa */ foo() // bbb
// ccc
}
@@ -0,0 +1,8 @@
fun foo() {}
fun test(b: Boolean) {
if (b) {
/* aaa */ foo() // bbb
}
// ccc
}
@@ -696,6 +696,21 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
runTest("idea/testData/intentions/addBraces/addBracesForIfWithSemicolonAndExpression.kt"); runTest("idea/testData/intentions/addBraces/addBracesForIfWithSemicolonAndExpression.kt");
} }
@TestMetadata("addBracesForSingleLineIfWithCommentBeneath.kt")
public void testAddBracesForSingleLineIfWithCommentBeneath() throws Exception {
runTest("idea/testData/intentions/addBraces/addBracesForSingleLineIfWithCommentBeneath.kt");
}
@TestMetadata("addBracesForSingleLineIfWithCommentBeneath2.kt")
public void testAddBracesForSingleLineIfWithCommentBeneath2() throws Exception {
runTest("idea/testData/intentions/addBraces/addBracesForSingleLineIfWithCommentBeneath2.kt");
}
@TestMetadata("addBracesForSingleLineIfWithCommentBeneath3.kt")
public void testAddBracesForSingleLineIfWithCommentBeneath3() throws Exception {
runTest("idea/testData/intentions/addBraces/addBracesForSingleLineIfWithCommentBeneath3.kt");
}
@TestMetadata("addBracesForWhile.kt") @TestMetadata("addBracesForWhile.kt")
public void testAddBracesForWhile() throws Exception { public void testAddBracesForWhile() throws Exception {
runTest("idea/testData/intentions/addBraces/addBracesForWhile.kt"); runTest("idea/testData/intentions/addBraces/addBracesForWhile.kt");