From 333445d93e1c24495c33af5d951bea478e2b4db5 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Sat, 23 May 2015 15:20:57 +0300 Subject: [PATCH] Fixed Convert to foreach intention loosing comments --- .../kotlin/psi/psiUtil/jetPsiUtil.kt | 16 ++-- .../jetbrains/kotlin/psi/psiUtil/psiUtils.kt | 75 +++++++++++++++++++ .../ConvertToForEachFunctionCallIntention.kt | 13 +++- .../intentions/InvertIfConditionIntention.kt | 12 +-- .../commentsInBody.kt | 11 +++ .../commentsInBody.kt.after | 11 +++ .../commentsInBody2.kt | 8 ++ .../commentsInBody2.kt.after | 7 ++ .../intentions/IntentionTestGenerated.java | 12 +++ 9 files changed, 149 insertions(+), 16 deletions(-) create mode 100644 compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/psiUtils.kt create mode 100644 idea/testData/intentions/convertToForEachFunctionCall/commentsInBody.kt create mode 100644 idea/testData/intentions/convertToForEachFunctionCall/commentsInBody.kt.after create mode 100644 idea/testData/intentions/convertToForEachFunctionCall/commentsInBody2.kt create mode 100644 idea/testData/intentions/convertToForEachFunctionCall/commentsInBody2.kt.after diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/jetPsiUtil.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/jetPsiUtil.kt index 4120e13fc92..9bfafd63c17 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/jetPsiUtil.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/jetPsiUtil.kt @@ -646,11 +646,17 @@ public fun JetElement.getCalleeHighlightingRange(): TextRange { return TextRange(startOffset, annotationEntry.getCalleeExpression().endOffset) } -public val PsiElement.startOffset: Int - get() = getTextRange().getStartOffset() - -public val PsiElement.endOffset: Int - get() = getTextRange().getEndOffset() +public fun JetBlockExpression.contentRange(): PsiChildRange { + val first = (getLBrace()?.getNextSibling() ?: getFirstChild()) + ?.siblings(withItself = false) + ?.firstOrNull { it !is PsiWhiteSpace } + val rBrace = getRBrace() + if (first == rBrace) return PsiChildRange.EMPTY + val last = rBrace!! + .siblings(forward = false, withItself = false) + .first { it !is PsiWhiteSpace } + return PsiChildRange(first, last) +} // Annotations on labeled expression lies on it's base expression public fun JetExpression.getAnnotationEntries(): List { diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/psiUtils.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/psiUtils.kt new file mode 100644 index 00000000000..65a44521c89 --- /dev/null +++ b/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/psiUtils.kt @@ -0,0 +1,75 @@ +/* + * Copyright 2010-2015 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. + */ + +package org.jetbrains.kotlin.psi.psiUtil + +import com.intellij.openapi.util.TextRange +import com.intellij.psi.PsiElement + +//TODO: move here more functions from jetPsiUtil.kt + +public val PsiElement.startOffset: Int + get() = getTextRange().getStartOffset() + +public val PsiElement.endOffset: Int + get() = getTextRange().getEndOffset() + +public data class PsiChildRange(public val first: PsiElement?, public val last: PsiElement?) : Sequence { + init { + if (first == null) { + assert(last == null) + } + else { + assert(first.getParent() == last!!.getParent()) + } + } + + public val isEmpty: Boolean + get() = first == null + + override fun iterator(): Iterator { + val sequence = if (first == null) { + emptySequence() + } + else { + val afterLast = last!!.getNextSibling() + first.siblings().takeWhile { it != afterLast } + } + return sequence.iterator() + } + + companion object { + public val EMPTY: PsiChildRange = PsiChildRange(null, null) + } +} + +public val PsiElement.allChildren: PsiChildRange + get() { + val first = getFirstChild() + return if (first != null) PsiChildRange(first, getLastChild()) else PsiChildRange.EMPTY + } + +public val PsiChildRange.textRange: TextRange? + get() { + if (isEmpty) return null + return TextRange(first!!.startOffset, last!!.endOffset) + } + +public fun PsiChildRange.getText(): String { + if (isEmpty) return "" + return this.map { it.getText() }.joinToString("") +} + diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToForEachFunctionCallIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToForEachFunctionCallIntention.kt index eb75cce72a1..106a5a60bfd 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToForEachFunctionCallIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToForEachFunctionCallIntention.kt @@ -17,11 +17,14 @@ package org.jetbrains.kotlin.idea.intentions import com.intellij.openapi.editor.Editor +import org.jetbrains.kotlin.lexer.JetTokens import org.jetbrains.kotlin.psi.JetBlockExpression import org.jetbrains.kotlin.psi.JetForExpression import org.jetbrains.kotlin.psi.JetPsiFactory import org.jetbrains.kotlin.psi.createExpressionByPattern +import org.jetbrains.kotlin.psi.psiUtil.contentRange import org.jetbrains.kotlin.psi.psiUtil.endOffset +import org.jetbrains.kotlin.psi.psiUtil.getText public class ConvertToForEachFunctionCallIntention : JetSelfTargetingIntention(javaClass(), "Replace with a forEach function call") { override fun isApplicableTo(element: JetForExpression, caretOffset: Int): Boolean { @@ -35,7 +38,15 @@ public class ConvertToForEachFunctionCallIntention : JetSelfTargetingIntention body.getStatements().map { it.getText() }.joinToString("\n") + is JetBlockExpression -> { + val content = body.contentRange() + val text = content.getText() + if (content.last?.getNode()?.getElementType() == JetTokens.EOL_COMMENT) + text + "\n" + else + text + } + else -> body.getText() } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/InvertIfConditionIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/InvertIfConditionIntention.kt index f7e7efd2dc8..4ce90bf3f31 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/InvertIfConditionIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/InvertIfConditionIntention.kt @@ -143,8 +143,8 @@ public class InvertIfConditionIntention : JetSelfTargetingIntention? { - val first = getLBrace()?.siblings(withItself = false)?.firstOrNull { it !is PsiWhiteSpace } ?: return null - val rBrace = getRBrace() - if (first == rBrace) return null - val last = rBrace!!.siblings(forward = false, withItself = false).first { it !is PsiWhiteSpace } - return Pair(first, last) - } - private fun exitStatementExecutedAfter(expression: JetExpression): JetExpression? { val parent = expression.getParent() if (parent is JetBlockExpression) { diff --git a/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody.kt b/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody.kt new file mode 100644 index 00000000000..2d14c1dacb7 --- /dev/null +++ b/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody.kt @@ -0,0 +1,11 @@ +fun foo() { + val list = 1..4 + + for (x in list) { // start of loop + // comment 1 + var v = x + 1 + // comment 2 + v++ + // end of loop + } +} \ No newline at end of file diff --git a/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody.kt.after b/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody.kt.after new file mode 100644 index 00000000000..f3365518d72 --- /dev/null +++ b/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody.kt.after @@ -0,0 +1,11 @@ +fun foo() { + val list = 1..4 + + list.forEach { x ->// start of loop + // comment 1 + var v = x + 1 + // comment 2 + v++ + // end of loop + } +} \ No newline at end of file diff --git a/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody2.kt b/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody2.kt new file mode 100644 index 00000000000..126e7cc82a3 --- /dev/null +++ b/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody2.kt @@ -0,0 +1,8 @@ +fun foo() { + val list = 1..4 + + for (x in list) { + // comment + var v = x + 1 + } +} \ No newline at end of file diff --git a/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody2.kt.after b/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody2.kt.after new file mode 100644 index 00000000000..4d3b54f82dd --- /dev/null +++ b/idea/testData/intentions/convertToForEachFunctionCall/commentsInBody2.kt.after @@ -0,0 +1,7 @@ +fun foo() { + val list = 1..4 + + list.forEach { x ->// comment + var v = x + 1 + } +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index a5bab717592..2e5946f0eff 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -3898,6 +3898,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("commentsInBody.kt") + public void testCommentsInBody() throws Exception { + String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/convertToForEachFunctionCall/commentsInBody.kt"); + doTest(fileName); + } + + @TestMetadata("commentsInBody2.kt") + public void testCommentsInBody2() throws Exception { + String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/convertToForEachFunctionCall/commentsInBody2.kt"); + doTest(fileName); + } + @TestMetadata("iterativeElementTypeSpecified.kt") public void testIterativeElementTypeSpecified() throws Exception { String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/convertToForEachFunctionCall/iterativeElementTypeSpecified.kt");