KT-7291 Copy/paste of java declaration with preceding comment omitted does not convert it to Kotlin propertly

#KT-7291 Fixed
2015-04-09 15:21:56 +03:00
parent bfadd489a3
commit 498f24b98b
18 changed files with 304 additions and 21 deletions
@@ -367,16 +367,16 @@ public fun JetExpression.isFunctionLiteralOutsideParentheses(): Boolean {
    }
 }

-public fun PsiElement.siblings(forward: Boolean = true, withItself: Boolean = true): Stream<PsiElement> {
+public fun PsiElement.siblings(forward: Boolean = true, withItself: Boolean = true): Sequence<PsiElement> {
    val stepFun = if (forward) { e: PsiElement -> e.getNextSibling() } else { e: PsiElement -> e.getPrevSibling() }
-    val stream = stream(this, stepFun)
-    return if (withItself) stream else stream.drop(1)
+    val sequence = sequence(this, stepFun)
+    return if (withItself) sequence else sequence.drop(1)
 }

-public fun ASTNode.siblings(forward: Boolean = true, withItself: Boolean = true): Stream<ASTNode> {
+public fun ASTNode.siblings(forward: Boolean = true, withItself: Boolean = true): Sequence<ASTNode> {
    val stepFun = if (forward) { node: ASTNode -> node.getTreeNext() } else { e: ASTNode -> e.getTreeNext() }
-    val stream = stream(this, stepFun)
-    return if (withItself) stream else stream.drop(1)
+    val sequence = sequence(this, stepFun)
+    return if (withItself) sequence else sequence.drop(1)
 }

 public fun PsiElement.parents(withItself: Boolean = true): Sequence<PsiElement> {
@@ -384,9 +384,9 @@ public fun PsiElement.parents(withItself: Boolean = true): Sequence<PsiElement>
    return if (withItself) sequence else sequence.drop(1)
 }

-public fun ASTNode.parents(withItself: Boolean = true): Stream<ASTNode> {
-    val stream = stream(this) { it.getTreeParent() }
-    return if (withItself) stream else stream.drop(1)
+public fun ASTNode.parents(withItself: Boolean = true): Sequence<ASTNode> {
+    val sequence = sequence(this) { it.getTreeParent() }
+    return if (withItself) sequence else sequence.drop(1)
 }

 public fun JetExpression.getAssignmentByLHS(): JetBinaryExpression? {
@@ -19,11 +19,12 @@ package org.jetbrains.kotlin.idea.conversion.copy
 import com.intellij.lang.java.JavaLanguage
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.util.TextRange
-import com.intellij.psi.PsiFileFactory
-import com.intellij.psi.PsiJavaFile
+import com.intellij.psi.*
 import org.jetbrains.kotlin.name.FqNameUnsafe
 import org.jetbrains.kotlin.platform.JavaToKotlinClassMap
 import org.jetbrains.kotlin.psi.psiUtil.elementsInRange
+import org.jetbrains.kotlin.psi.psiUtil.parents
+import org.jetbrains.kotlin.psi.psiUtil.siblings
 import java.util.ArrayList

 data class DataForConversion private(
@@ -32,20 +33,179 @@ data class DataForConversion private(
 ) {
    companion object {
        fun prepare(copiedCode: CopiedJavaCode, project: Project): DataForConversion  {
-            assert(copiedCode.startOffsets.size() == copiedCode.endOffsets.size(), "Must have the same size")
+            val startOffsets = copiedCode.startOffsets.clone()
+            val endOffsets = copiedCode.endOffsets.clone()
+            assert(startOffsets.size() == endOffsets.size(), "Must have the same size")

-            val sourceFileText = copiedCode.fileText
-            val sourceFile = PsiFileFactory.getInstance(project).createFileFromText(JavaLanguage.INSTANCE, sourceFileText) as PsiJavaFile
-            val importsAndPackage = buildImportsAndPackage(sourceFile)
+            var fileText = copiedCode.fileText
+            var file = PsiFileFactory.getInstance(project).createFileFromText(JavaLanguage.INSTANCE, fileText) as PsiJavaFile
+
+            val importsAndPackage = buildImportsAndPackage(file)
+
+            val newFileText = clipTextIfNeeded(file, fileText, startOffsets, endOffsets)
+            if (newFileText != null) {
+                fileText = newFileText
+                file = PsiFileFactory.getInstance(project).createFileFromText(JavaLanguage.INSTANCE, newFileText) as PsiJavaFile
+            }

            val elementsAndTexts = ArrayList<Any>()
-            for (i in copiedCode.startOffsets.indices) {
-                elementsAndTexts.collectElementsToConvert(sourceFile, sourceFileText, TextRange(copiedCode.startOffsets[i], copiedCode.endOffsets[i]))
+            for (i in startOffsets.indices) {
+                elementsAndTexts.collectElementsToConvert(file, fileText, TextRange(startOffsets[i], endOffsets[i]))
            }

            return DataForConversion(elementsAndTexts, importsAndPackage)
        }

+        private fun clipTextIfNeeded(file: PsiJavaFile, fileText: String, startOffsets: IntArray, endOffsets: IntArray): String? {
+            fun canDropRange(range: TextRange)
+                    = startOffsets.indices.all { range !in TextRange(startOffsets[it], endOffsets[it]) }
+
+            val rangesToDrop = ArrayList<TextRange>()
+            for (i in startOffsets.indices) {
+                val start = startOffsets[i]
+                val end = endOffsets[i]
+                val range = TextRange(start, end)
+
+                val startToken = file.findElementAt(start)!!
+                val elementToClipLeft = startToken.maximalParentToClip(range)
+                if (elementToClipLeft != null) {
+                    val elementStart = elementToClipLeft.range.start
+                    if (elementStart < start) {
+                        val clipBound = tryClipLeftSide(elementToClipLeft, start)
+                        if (clipBound != null) {
+                            val rangeToDrop = TextRange(elementStart, clipBound)
+                            if (canDropRange(rangeToDrop)) {
+                                rangesToDrop.add(rangeToDrop)
+                            }
+                        }
+                    }
+                }
+
+                val endToken = file.findElementAt(end - 1)!!
+                val elementToClipRight = endToken.maximalParentToClip(range)
+                if (elementToClipRight != null) {
+                    val elementEnd = elementToClipRight.range.end
+                    if (elementEnd > end) {
+                        val clipBound = tryClipRightSide(elementToClipRight, end)
+                        if (clipBound != null) {
+                            val rangeToDrop = TextRange(clipBound, elementEnd)
+                            if (canDropRange(rangeToDrop)) {
+                                rangesToDrop.add(rangeToDrop)
+                            }
+                        }
+                    }
+                }
+            }
+
+            if (rangesToDrop.isEmpty()) return null
+
+            val newFileText = StringBuilder {
+                var offset = 0
+                for (range in rangesToDrop) {
+                    assert(range.start >= offset)
+                    append(fileText.substring(offset, range.start))
+                    offset = range.end
+                }
+                append(fileText.substring(offset, fileText.length()))
+            }.toString()
+
+            fun IntArray.update() {
+                for (range in rangesToDrop.reverse()) {
+                    for (i in indices) {
+                        val offset = this[i]
+                        if (offset >= range.end) {
+                            this[i] = offset - range.length
+                        }
+                        else {
+                            assert(offset <= range.start)
+                        }
+                    }
+                }
+            }
+
+            startOffsets.update()
+            endOffsets.update()
+
+            return newFileText
+        }
+
+        private fun PsiElement.maximalParentToClip(range: TextRange): PsiElement? {
+            val firstNotInRange = parents().takeWhile { it !is PsiDirectory }.firstOrNull { it.range !in range } ?: return null
+            return firstNotInRange.parents().lastOrNull { it.minimizedTextRange() in range }
+        }
+
+        private fun PsiElement.minimizedTextRange(): TextRange {
+            val firstChild = getFirstChild()?.siblings()?.firstOrNull { !canDropElementFromText(it) } ?: return range
+            val lastChild = getLastChild().siblings(forward = false).first { !canDropElementFromText(it) }
+            return TextRange(firstChild.minimizedTextRange().start, lastChild.minimizedTextRange().end)
+        }
+
+        // element's text can be removed from file's text keeping parsing the same
+        private fun canDropElementFromText(element: PsiElement): Boolean {
+            return when (element) {
+                is PsiWhiteSpace, is PsiComment, is PsiModifierList,
+                is PsiAnnotation, is PsiPackageStatement, is PsiImportList, is PsiImportStatementBase -> true
+
+                is PsiJavaToken -> {
+                    when (element.getTokenType()) {
+                        // modifiers
+                        JavaTokenType.PUBLIC_KEYWORD, JavaTokenType.PROTECTED_KEYWORD, JavaTokenType.PRIVATE_KEYWORD,
+                        JavaTokenType.STATIC_KEYWORD, JavaTokenType.ABSTRACT_KEYWORD, JavaTokenType.FINAL_KEYWORD,
+                        JavaTokenType.NATIVE_KEYWORD, JavaTokenType.SYNCHRONIZED_KEYWORD, JavaTokenType.STRICTFP_KEYWORD,
+                        JavaTokenType.TRANSIENT_KEYWORD, JavaTokenType.VOLATILE_KEYWORD, JavaTokenType.DEFAULT_KEYWORD -> element.getParent() is PsiModifierList
+
+                        JavaTokenType.SEMICOLON -> true
+
+                        else -> false
+                    }
+                }
+
+                is PsiCodeBlock -> element.getParent() is PsiMethod
+
+                else -> element.getFirstChild() == null
+            }
+        }
+
+        private fun tryClipLeftSide(element: PsiElement, leftBound: Int)
+                = tryClipSide(element, leftBound, { range }, { getFirstChild().siblings() })
+
+        private fun tryClipRightSide(element: PsiElement, rightBound: Int): Int? {
+            fun Int.transform() = Int.MAX_VALUE - this
+            fun TextRange.transform() = TextRange(end.transform(), start.transform())
+            return tryClipSide(element, rightBound.transform(), { range.transform() }, { getLastChild().siblings(forward = false) })?.transform()
+        }
+
+        private fun tryClipSide(
+                element: PsiElement,
+                rangeBound: Int,
+                rangeFunction: PsiElement.() -> TextRange,
+                childrenFunction: PsiElement.() -> Sequence<PsiElement>
+        ): Int? {
+            if (element.getFirstChild() == null) return null
+
+            val elementRange = element.rangeFunction()
+            assert(elementRange.start < rangeBound && rangeBound < elementRange.end)
+
+            var clipTo = elementRange.start
+            for (child in element.childrenFunction()) {
+                val childRange = child.rangeFunction()
+
+                if (childRange.start >= rangeBound) { // we have cut enough already
+                    break
+                }
+                else if (childRange.end <= rangeBound) { // need to drop the whole element
+                    if (!canDropElementFromText(child)) return null
+                    clipTo = childRange.end
+                }
+                else { // rangeBound is inside element's range
+                    if (child is PsiWhiteSpace) break // no need to cut whitespace - we can leave it as is
+                    return tryClipSide(child, rangeBound, rangeFunction, childrenFunction)
+                }
+            }
+
+            return clipTo
+        }
+
        private fun MutableList<Any>.collectElementsToConvert(
                file: PsiJavaFile,
                fileText: String,
@@ -53,12 +213,12 @@ data class DataForConversion private(
        ) {
            val elements = file.elementsInRange(range)
            if (elements.isEmpty()) {
-                add(fileText.substring(range.getStartOffset(), range.getEndOffset()))
+                add(fileText.substring(range.start, range.end))
            }
            else {
-                add(fileText.substring(range.getStartOffset(), elements.first().getTextRange().getStartOffset()))
+                add(fileText.substring(range.start, elements.first().range.start))
                addAll(elements)
-                add(fileText.substring(elements.last().getTextRange().getEndOffset(), range.getEndOffset()))
+                add(fileText.substring(elements.last().range.end, range.end))
            }
        }

@@ -28,6 +28,9 @@ public val TextRange.start: Int
 public val TextRange.end: Int
    get() = getEndOffset()

+public val TextRange.length: Int
+    get() = getLength()
+
 public val PsiElement.range: TextRange
    get() = getTextRange()!!

@@ -0,0 +1,6 @@
+package to
+
+public class JavaClass : Runnable {
+    override fun run() {
+    }
+}
@@ -0,0 +1,10 @@
+/**
+ * Doc comment for JavaClass
+ */
+<selection>public class JavaClass implements Runnable{
+    @Override
+    public void run() {
+
+    }
+}
+</selection>
@@ -0,0 +1,3 @@
+package to
+
+private val field = 1
@@ -0,0 +1,3 @@
+public class JavaClass {
+    <selection>private int field = 1;</selection> // comment for field
+}
@@ -0,0 +1,3 @@
+package to
+
+volatile var field = 1
@@ -0,0 +1,3 @@
+public class JavaClass {
+    public <selection>volatile int field = 1</selection>;
+}
@@ -0,0 +1,9 @@
+package to
+
+public object JavaClass {
+    public fun main(args: Array<String>) {
+        println("Hello, world!")
+    }
+}
+
+fun main(args: Array<String>) = JavaClass.main(args)
@@ -0,0 +1,10 @@
+// this test should cause the whole file to be converted at once (with non-selected text stripped) and the main function to be generated
+package helloWorld;
+
+// sample class
+<selection>public class JavaClass {
+    public static void main(String[] args) {
+        System.out.println("Hello, world!");
+    }
+}
+</selection>
@@ -0,0 +1,3 @@
+package to
+
+fun foo(p: Int)
@@ -0,0 +1,5 @@
+class A {
+    public <selection>void foo(int p)</selection> {
+        System.out.println("foo");
+    }
+}
@@ -0,0 +1,5 @@
+package to
+
+public fun foo(): String {
+    return ""
+}
@@ -0,0 +1,6 @@
+public class JavaClass {
+    @Deprecated
+<selection>    public String foo() {
+        return "";
+    }
+</selection>}
@@ -0,0 +1,6 @@
+package to
+
+SomeAnnotation
+public fun foo(): String {
+    return ""
+}
@@ -0,0 +1,7 @@
+public class JavaClass {
+    @Deprecated
+<selection>    @SomeAnnotation
+    public String foo() {
+        return "";
+    }
+</selection>}
@@ -17,7 +17,6 @@
 package org.jetbrains.kotlin.idea.conversion.copy;

 import com.intellij.testFramework.TestDataPath;
-import org.jetbrains.kotlin.test.InnerTestClasses;
 import org.jetbrains.kotlin.test.JUnit3RunnerWithInners;
 import org.jetbrains.kotlin.test.JetTestUtils;
 import org.jetbrains.kotlin.test.TestMetadata;
@@ -72,6 +71,12 @@ public class JavaToKotlinCopyPasteConversionTestGenerated extends AbstractJavaTo
        doTest(fileName);
    }

+    @TestMetadata("ClassWithNoDocComment.java")
+    public void testClassWithNoDocComment() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/ClassWithNoDocComment.java");
+        doTest(fileName);
+    }
+
    @TestMetadata("Constructor.java")
    public void testConstructor() throws Exception {
        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/Constructor.java");
@@ -84,6 +89,24 @@ public class JavaToKotlinCopyPasteConversionTestGenerated extends AbstractJavaTo
        doTest(fileName);
    }

+    @TestMetadata("FieldWithNoEndComment.java")
+    public void testFieldWithNoEndComment() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/FieldWithNoEndComment.java");
+        doTest(fileName);
+    }
+
+    @TestMetadata("FieldWithNoModifierAndNoSemicolon.java")
+    public void testFieldWithNoModifierAndNoSemicolon() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/FieldWithNoModifierAndNoSemicolon.java");
+        doTest(fileName);
+    }
+
+    @TestMetadata("FileWithNoPackageStatement.java")
+    public void testFileWithNoPackageStatement() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/FileWithNoPackageStatement.java");
+        doTest(fileName);
+    }
+
    @TestMetadata("HalfTheWhiteSpace.java")
    public void testHalfTheWhiteSpace() throws Exception {
        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/HalfTheWhiteSpace.java");
@@ -114,12 +137,30 @@ public class JavaToKotlinCopyPasteConversionTestGenerated extends AbstractJavaTo
        doTest(fileName);
    }

+    @TestMetadata("MethodDeclarationWithNoBody.java")
+    public void testMethodDeclarationWithNoBody() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/MethodDeclarationWithNoBody.java");
+        doTest(fileName);
+    }
+
    @TestMetadata("MethodReferenceWithoutQualifier.java")
    public void testMethodReferenceWithoutQualifier() throws Exception {
        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/MethodReferenceWithoutQualifier.java");
        doTest(fileName);
    }

+    @TestMetadata("MethodWithNoAnnotation.java")
+    public void testMethodWithNoAnnotation() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/MethodWithNoAnnotation.java");
+        doTest(fileName);
+    }
+
+    @TestMetadata("MethodWithOnlyOneAnnotation.java")
+    public void testMethodWithOnlyOneAnnotation() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/MethodWithOnlyOneAnnotation.java");
+        doTest(fileName);
+    }
+
    @TestMetadata("OnlyClosingBrace.java")
    public void testOnlyClosingBrace() throws Exception {
        String fileName = JetTestUtils.navigationMetadata("idea/testData/copyPaste/conversion/OnlyClosingBrace.java");