diff --git a/platform/diff-impl/tests/BUILD.bazel b/platform/diff-impl/tests/BUILD.bazel index 4179419aee07..0656d6d3a79f 100644 --- a/platform/diff-impl/tests/BUILD.bazel +++ b/platform/diff-impl/tests/BUILD.bazel @@ -29,6 +29,7 @@ jvm_library( "//platform/platform-impl:ide-impl_test_lib", "//platform/util/diff", "//platform/vcs-api/vcs-api-core:vcs-core", + "//platform/vcs-api/vcs-api-core:vcs-core_test_lib", "@lib//:mockito", "//platform/analysis-api:analysis", "//platform/editor-ui-ex:editor-ex", diff --git a/platform/inspect/BUILD.bazel b/platform/inspect/BUILD.bazel index 9ca32144769b..beb84bb50280 100644 --- a/platform/inspect/BUILD.bazel +++ b/platform/inspect/BUILD.bazel @@ -67,6 +67,7 @@ jvm_library( "//platform/util/jdom", "//platform/analysis-impl", "//platform/vcs-api/vcs-api-core:vcs-core", + "//platform/vcs-api/vcs-api-core:vcs-core_test_lib", "//platform/platform-impl:ide-impl", "//platform/platform-impl:ide-impl_test_lib", "//platform/diff-api:diff", diff --git a/platform/vcs-api/vcs-api-core/BUILD.bazel b/platform/vcs-api/vcs-api-core/BUILD.bazel index 8ce896bfde29..1332c7808b7b 100644 --- a/platform/vcs-api/vcs-api-core/BUILD.bazel +++ b/platform/vcs-api/vcs-api-core/BUILD.bazel @@ -1,5 +1,5 @@ ### auto-generated section `build intellij.platform.vcs.core` start -load("@rules_jvm//:jvm.bzl", "jvm_library", "jvm_resources") +load("@rules_jvm//:jvm.bzl", "jvm_library", "jvm_resources", "jvm_test") jvm_resources( name = "vcs-core_resources", @@ -23,4 +23,29 @@ jvm_library( ], runtime_deps = [":vcs-core_resources"] ) + +jvm_library( + name = "vcs-core_test_lib", + visibility = ["//visibility:public"], + srcs = glob(["test/**/*.kt", "test/**/*.java"], allow_empty = True), + associates = [":vcs-core"], + deps = [ + "//platform/core-api:core", + "//platform/util", + "//platform/editor-ui-api:editor-ui", + "//platform/ide-core", + "//platform/diff-api:diff", + "//platform/util/diff", + "@lib//:kotlin-stdlib", + "//libraries/junit5", + "//libraries/junit5-params", + "//libraries/assertj-core", + ], + runtime_deps = [":vcs-core_resources"] +) + +jvm_test( + name = "vcs-core_test", + runtime_deps = [":vcs-core_test_lib"] +) ### auto-generated section `build intellij.platform.vcs.core` end \ No newline at end of file diff --git a/platform/vcs-api/vcs-api-core/intellij.platform.vcs.core.iml b/platform/vcs-api/vcs-api-core/intellij.platform.vcs.core.iml index 786aa9b1de5a..77407804bcbb 100644 --- a/platform/vcs-api/vcs-api-core/intellij.platform.vcs.core.iml +++ b/platform/vcs-api/vcs-api-core/intellij.platform.vcs.core.iml @@ -5,6 +5,7 @@ + @@ -15,5 +16,8 @@ + + + \ No newline at end of file diff --git a/platform/vcs-api/vcs-api-core/test/com/intellij/openapi/diff/impl/patch/PatchHunkUtilTest.kt b/platform/vcs-api/vcs-api-core/test/com/intellij/openapi/diff/impl/patch/PatchHunkUtilTest.kt new file mode 100644 index 000000000000..648b2c1a4186 --- /dev/null +++ b/platform/vcs-api/vcs-api-core/test/com/intellij/openapi/diff/impl/patch/PatchHunkUtilTest.kt @@ -0,0 +1,37 @@ +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.openapi.diff.impl.patch + +import com.intellij.diff.util.Range +import com.intellij.openapi.diff.impl.patch.PatchHunkUtil.getChangeOnlyRanges +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.MethodSource + +class PatchHunkUtilTest { + @ParameterizedTest(name = "{0}") + @MethodSource("simpleRanges") + fun `getChangeOnlyRanges for a simple one-range hunk calculates that one range`(range: Range) { + val hunk = PatchHunk(range.start1, range.end1, range.start2, range.end2).apply { + repeat(range.end1 - range.start1) { addLine(REMOVE_LINE) } + repeat(range.end2 - range.start2) { addLine(ADD_LINE) } + } + + assertThat(getChangeOnlyRanges(hunk)) + .hasSize(1) + .first().isEqualTo(range) + } + + companion object { + @JvmStatic + fun simpleRanges(): Array = arrayOf( + Range(0, 1, 0, 1), + Range(0, 0, 0, 10), + Range(0, 5, 0, 10), + Range(3, 5, 3, 3), + Range(5, 5, 3, 4), + ) + + val REMOVE_LINE = PatchLine(PatchLine.Type.REMOVE, "").apply { isSuppressNewLine = true } + val ADD_LINE = PatchLine(PatchLine.Type.ADD, "").apply { isSuppressNewLine = true } + } +} \ No newline at end of file diff --git a/platform/vcs-tests/BUILD.bazel b/platform/vcs-tests/BUILD.bazel index 178247537944..881e14fc02fd 100644 --- a/platform/vcs-tests/BUILD.bazel +++ b/platform/vcs-tests/BUILD.bazel @@ -52,6 +52,7 @@ jvm_library( "//platform/lang-impl", "//platform/vcs-impl/lang", "//platform/vcs-api/vcs-api-core:vcs-core", + "//platform/vcs-api/vcs-api-core:vcs-core_test_lib", "//platform/vcs-api:vcs", "//platform/core-api:core", "//platform/ide-core-impl", diff --git a/plugins/devkit/intellij.devkit.git/BUILD.bazel b/plugins/devkit/intellij.devkit.git/BUILD.bazel index 22b51de997ab..101704a01b1d 100644 --- a/plugins/devkit/intellij.devkit.git/BUILD.bazel +++ b/plugins/devkit/intellij.devkit.git/BUILD.bazel @@ -61,6 +61,7 @@ jvm_library( "//platform/util", "//platform/ide-core", "//platform/vcs-api/vcs-api-core:vcs-core", + "//platform/vcs-api/vcs-api-core:vcs-core_test_lib", "//platform/vcs-log/api:vcs-log", "//platform/dvcs-impl:vcs-dvcs-impl", "//platform/dvcs-impl:vcs-dvcs-impl_test_lib", diff --git a/plugins/git4idea/intellij.vcs.git.coverage/BUILD.bazel b/plugins/git4idea/intellij.vcs.git.coverage/BUILD.bazel index a57e00fe692b..88e93f777ae6 100644 --- a/plugins/git4idea/intellij.vcs.git.coverage/BUILD.bazel +++ b/plugins/git4idea/intellij.vcs.git.coverage/BUILD.bazel @@ -43,6 +43,7 @@ jvm_library( "//plugins/git4idea:vcs-git_test_lib", "//plugins/git4idea/shared", "//platform/vcs-api/vcs-api-core:vcs-core", + "//platform/vcs-api/vcs-api-core:vcs-core_test_lib", "//platform/vcs-log/api:vcs-log", "//platform/vcs-log/impl", "//platform/vcs-log/impl:impl_test_lib", diff --git a/plugins/git4idea/src/git4idea/changes/GitFileHistory.kt b/plugins/git4idea/src/git4idea/changes/GitFileHistory.kt index 8efa872b67ea..465e8dba8db3 100644 --- a/plugins/git4idea/src/git4idea/changes/GitFileHistory.kt +++ b/plugins/git4idea/src/git4idea/changes/GitFileHistory.kt @@ -36,6 +36,7 @@ interface GitFileHistory : Comparator { /** * Retrieve a chain of patches between commits [parent] and [child] + * Including [child], but excluding [parent] */ fun getPatchesBetween(parent: String, child: String): List } \ No newline at end of file diff --git a/plugins/git4idea/src/git4idea/changes/GitTextFilePatchWithHistory.kt b/plugins/git4idea/src/git4idea/changes/GitTextFilePatchWithHistory.kt index a28c1082e4c8..1fdfa2f9ff72 100644 --- a/plugins/git4idea/src/git4idea/changes/GitTextFilePatchWithHistory.kt +++ b/plugins/git4idea/src/git4idea/changes/GitTextFilePatchWithHistory.kt @@ -8,6 +8,7 @@ import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.diff.impl.patch.PatchHunkUtil import com.intellij.openapi.diff.impl.patch.TextFilePatch import org.jetbrains.annotations.ApiStatus +import kotlin.math.floor /** * Holds the [patch] between two commits in [fileHistory] @@ -24,6 +25,32 @@ class GitTextFilePatchWithHistory(val patch: TextFilePatch, val isCumulative: Bo */ fun contains(commitSha: String, filePath: String): Boolean = fileHistory.contains(commitSha, filePath) + /** + * @return `null` only if the commit cannot be found. + * Otherwise, this function returns the most accurate line number on the given side of the patch history we could derive. + * If the line is removed during any patch, its location information can no longer be retrieved exactly, so instead, we + * continue with the topmost line of the changed hunk. + */ + fun forcefullyMapLine(fromCommitSha: String, lineIndex: Int, side: Side): Int? { + // map to merge base, not left revision + val beforeSha = if (isCumulative) fileHistory.findStartCommit()!! else patch.beforeVersionId!! + val afterSha = patch.afterVersionId!! + + if (fromCommitSha == beforeSha && side == Side.LEFT) return lineIndex + if (fromCommitSha == afterSha && side == Side.RIGHT) return lineIndex + + return when (side) { + Side.LEFT -> { + val patches = fileHistory.getPatchesBetween(beforeSha, fromCommitSha) + transferLine(patches, lineIndex, rightToLeft = true).line + } + Side.RIGHT -> { + val patches = fileHistory.getPatchesBetween(fromCommitSha, afterSha) + transferLine(patches, lineIndex, rightToLeft = false).line + } + } + } + /** * Map the [lineIndex] in a file in a *commit* [fromCommitSha] to a location in the current [patch] * @@ -39,38 +66,14 @@ class GitTextFilePatchWithHistory(val patch: TextFilePatch, val isCumulative: Bo if (fromCommitSha == beforeSha) return DiffLineLocation(Side.LEFT, lineIndex) if (fromCommitSha == afterSha) return DiffLineLocation(Side.RIGHT, lineIndex) - fun tryTransferToParent(): DiffLineLocation? { - return if (fileHistory.compare(afterSha, fromCommitSha) < 0) { - val patches = fileHistory.getPatchesBetween(afterSha, fromCommitSha) - transferLine(patches, lineIndex, true) - } - else if (fileHistory.compare(beforeSha, fromCommitSha) < 0) { - val patches = fileHistory.getPatchesBetween(beforeSha, fromCommitSha) - transferLine(patches, lineIndex, true) - } - else { - null - } - } - - fun tryTransferToChild(): DiffLineLocation? { - return if (fileHistory.compare(fromCommitSha, beforeSha) < 0) { - val patches = fileHistory.getPatchesBetween(fromCommitSha, beforeSha) - transferLine(patches, lineIndex, false) - } - else if (fileHistory.compare(fromCommitSha, afterSha) < 0) { - val patches = fileHistory.getPatchesBetween(fromCommitSha, afterSha) - transferLine(patches, lineIndex, false) - } - else { - null - } - } + if (!fileHistory.contains(fromCommitSha, patch.filePath)) return null return try { when (bias) { - Side.LEFT -> tryTransferToParent() ?: tryTransferToChild() - Side.RIGHT -> tryTransferToChild() ?: tryTransferToParent() + Side.LEFT -> transferToParent(lineIndex, beforeSha, fromCommitSha, afterSha) + ?: transferToChild(lineIndex, beforeSha, fromCommitSha, afterSha) + Side.RIGHT -> transferToChild(lineIndex, beforeSha, fromCommitSha, afterSha) + ?: transferToParent(lineIndex, beforeSha, fromCommitSha, afterSha) } } catch (e: Exception) { @@ -79,9 +82,43 @@ class GitTextFilePatchWithHistory(val patch: TextFilePatch, val isCumulative: Bo } } - private fun transferLine(patchChain: List, lineIndex: Int, rightToLeft: Boolean): DiffLineLocation? { - val side: Side = if (rightToLeft) Side.RIGHT else Side.LEFT - var currentLine: Int = lineIndex + private fun transferToParent(lineIndex: Int, beforeSha: String, fromCommitSha: String, afterSha: String): DiffLineLocation? = + if (fileHistory.compare(afterSha, fromCommitSha) < 0) { + val patches = fileHistory.getPatchesBetween(afterSha, fromCommitSha) + transferLine(patches, lineIndex, true).exactLocation()?.let { + Side.RIGHT to it + } + } + else if (fileHistory.compare(beforeSha, fromCommitSha) < 0) { + val patches = fileHistory.getPatchesBetween(beforeSha, fromCommitSha) + transferLine(patches, lineIndex, true).exactLocation()?.let { + Side.LEFT to it + } + } + else { + error("Couldn't find commit ${fromCommitSha}") + } + + private fun transferToChild(lineIndex: Int, beforeSha: String, fromCommitSha: String, afterSha: String): DiffLineLocation? = + if (fileHistory.compare(fromCommitSha, beforeSha) < 0) { + val patches = fileHistory.getPatchesBetween(fromCommitSha, beforeSha) + transferLine(patches, lineIndex, false).exactLocation()?.let { + Side.LEFT to it + } + } + else if (fileHistory.compare(fromCommitSha, afterSha) < 0) { + val patches = fileHistory.getPatchesBetween(fromCommitSha, afterSha) + transferLine(patches, lineIndex, false).exactLocation()?.let { + Side.RIGHT to it + } + } + else { + error("Couldn't find commit ${fromCommitSha}") + } + + private fun transferLine(patchChain: List, lineIndex: Int, rightToLeft: Boolean): TransferResult { + var currentLine = lineIndex.toDouble() + var isEstimate = false val patches = if (rightToLeft) patchChain.asReversed() else patchChain @@ -91,24 +128,52 @@ class GitTextFilePatchWithHistory(val patch: TextFilePatch, val isCumulative: Bo if (rightToLeft) ranges.map { reverseRange(it) } else ranges }.flatten() - var offset = 0 + var offset = 0.0 loop@ for (range in changeOnlyRanges) { when { currentLine < range.start1 -> break@loop - currentLine in range.start1 until range.end1 -> - return null - currentLine >= range.end1 -> + range.start1 <= currentLine && currentLine < range.end1 -> { + isEstimate = true + // Careful when changing: we assume that range.end1 - range.start1 is non-zero due to the above check + // Estimated position algo: find the relative position of the line in the hunk, then map it to that relative position in the new hunk + // A choice can be made about how to map to 'relative position' though; we choose to find the relative position based on the 'center' of the line within a range: + // - in a half-open range [1, 2), line 1 has relative position 0.5 (the 'center' of the line is used rather than the start) + // - in a half-open range [1, 3), line 1 has relative position 0.333, line 2 has relative position 0.667 + val relativePosition = ((currentLine - range.start1) + 1) / ((range.end1 - range.start1) + 1).toDouble() + offset -= currentLine - range.start1 // move line to the start of the range in the old hunk + // We map relative positions back to the output line in the following way: + // - for any relPos and out-range [1, 1) - there no output line -, we map to line 1 + // - for any relPos and out-range [1, 2) - there is only 1 possible output line -, we map to line 1.5 (and round after all mapping is done) + // - for relPos = 0.5 and out-range [1, 3) - there are 2 possible output lines -, we map to line 2 + // - for relPos = 0.5 and out-range [1, 4) - there are 3 possible output lines -, we map to line 2.5 (and round after all mapping is done) + offset += relativePosition * (range.end2 - range.start2) // then to the relative position in the new hunk + } + range.end1 <= currentLine -> offset += (range.end2 - range.start2) - (range.end1 - range.start1) } } currentLine += offset } - return DiffLineLocation(side, currentLine) + + // Note that the only way for the line number to become floating point is when it IS an estimate + return when (isEstimate) { + false -> TransferResult.ExactTransfer(floor(currentLine).toInt()) + true -> TransferResult.EstimatedTransfer(floor(currentLine).toInt()) + } } private fun reverseRange(range: Range) = Range(range.start2, range.end2, range.start1, range.end1) + private sealed interface TransferResult { + data class ExactTransfer(override val line: Int) : TransferResult + data class EstimatedTransfer(override val line: Int) : TransferResult + + val line: Int? + + fun exactLocation(): Int? = (this as? ExactTransfer)?.line + } + companion object { private val LOG = logger() } diff --git a/plugins/git4idea/tests/git4idea/changes/GitTextFilePatchWithHistoryTest.kt b/plugins/git4idea/tests/git4idea/changes/GitTextFilePatchWithHistoryTest.kt new file mode 100644 index 000000000000..aec2601bcbe4 --- /dev/null +++ b/plugins/git4idea/tests/git4idea/changes/GitTextFilePatchWithHistoryTest.kt @@ -0,0 +1,273 @@ +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package git4idea.changes + +import com.intellij.diff.util.Side +import com.intellij.openapi.diff.impl.patch.PatchHunk +import com.intellij.openapi.diff.impl.patch.PatchLine +import com.intellij.openapi.diff.impl.patch.TextFilePatch +import com.intellij.openapi.vcs.FileStatus +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.MethodSource + +class GitTextFilePatchWithHistoryTest { + @ParameterizedTest + @MethodSource("testCases") + fun `forceful mapping works`(case: TestCase) { + val patch = createTestPatch(case) + assertThat(patch.forcefullyMapLine(case.fromPatch, case.line, case.toSide)) + .isEqualTo(case.expected) + } + + @ParameterizedTest + @MethodSource("testCases") + fun `regular mapping works`(case: TestCase) { + val patch = createTestPatch(case) + assertThat(patch.mapLine(case.fromPatch, case.line, case.toSide)) + .isEqualTo(if (case.isEstimate) case.toSide.other() to case.line // no mapping, just use the easy side TODO: fix with complex start-in-the-middle tests + else case.toSide to case.expected) + } + + private fun createTestPatch(case : TestCase) = GitTextFilePatchWithHistory( + TextFilePatch(Charsets.UTF_8, "\n").apply { + beforeVersionId = ROOT_COMMIT + afterVersionId = case.patches.last().name + + beforeName = if (case.patches.first().fileStatus == FileStatus.ADDED) null else FILE_NAME + afterName = if (case.patches.last().fileStatus == FileStatus.DELETED) null else FILE_NAME + }, + isCumulative = true, + fileHistory = object : GitFileHistory { + override fun findStartCommit(): String = ROOT_COMMIT + override fun findFirstParent(commitSha: String): String? = case.patches.zipWithNext().find { it.second.name == commitSha }?.first?.name + override fun contains(commitSha: String, filePath: String): Boolean = commitSha == ROOT_COMMIT || case.patches.any { it.name == commitSha } + override fun compare(commitSha1: String, commitSha2: String): Int { + val commits = listOf(ROOT_COMMIT) + case.patches.map { it.name } + return commits.indexOf(commitSha1) compareTo commits.indexOf(commitSha2) + } + + override fun getPatchesBetween(parent: String, child: String): List { + val parentIndex = case.patches.indexOfFirst { it.name == parent } + val childIndex = case.patches.indexOfFirst { it.name == child } + + val patches = if (childIndex == -1) case.patches.subList(parentIndex + 1, case.patches.size) + else case.patches.subList(parentIndex + 1, childIndex + 1) + + return patches.toTextFilePatches() + } + } + ) + + companion object { + //region: Defining patches to test on + // just a filler at the start of the patch-list, avoiding the conditions to quickly return the current position + val startPatch = TestPatch( + name = "start_patch", + changes = listOf() + ) + + val patch1 = TestPatch( + name = "patch1", + changes = listOf( + (0..<0) to (0..<10), // insert 10 lines at line 0 + ) + ) + + val patch2 = TestPatch( + name = "patch2", + changes = listOf( + (1..<4) to (1..<5), // replace lines 1-3 with lines 1-4 + ) + ) + + val patch3 = TestPatch( + name = "patch3", + changes = listOf( + (2..<3) to (2..<4), // replace line 2 with lines 2-3 + (4..<6) to (5..<5), // delete lines 4-5 (shifted by 1 in result because of the insert at 2-4) + ) + ) + + // just a filler at the end of the patch-list, avoiding the conditions to quickly return the current position + val endPatch = TestPatch( + name = "end_patch", + changes = listOf() + ) + //endregion + + //region: Defining test cases + @JvmStatic + fun testCases(): Array = cases { + // The first commit pushes all lines forward by 10, other patches shift the line +1, then -1 + withPatches(patch1, patch2, patch3) { + 0 `maps to` /* -> 10 -> 11 -> */ 10 + 1 `maps to` /* -> 11 -> 12 -> */ 11 + 2 `maps to` /* -> 12 -> 13 -> */ 12 + 10 `reverse maps to` 0 + 11 `reverse maps to` 1 + 12 `reverse maps to` 2 + // in reverse, lines 0-10 collapse to 0-0 + 9 `reverse maps to` 0.approximately + 5 `reverse maps to` 0.approximately + } + // Single patch that: + // - replaces 2-2 with 2-3 + // - deletes 4-5 + withPatches(patch3) { + 0 `maps to` 0 + 1 `maps to` 1 + 2 `maps to` 3.approximately + // insert at line 2 causes shift by 1 + 3 `maps to` 4 + 4 `maps to` 5.approximately // deleted + 5 `maps to` 5.approximately // also deleted, mapped to the same as 4 + 6 `maps to` 5 + 7 `maps to` 6 + 6 `reverse maps to` 7 + 5 `reverse maps to` 6 + // gap happens because 4-5 was deleted + 4 `reverse maps to` 3 + // approximation happens because 2-2 is replaced by 2-3 + 3 `reverse maps to` 2.approximately + 2 `reverse maps to` 2.approximately + 1 `reverse maps to` 1 + } + // Illustrations of approximate mapping: + // relative position inside range 0-1 of line 0 = 0.5 + withPatches(TestPatch("estimation [0,1)->[0,0)", listOf((0..<1) to (0..<0)))) { + 0 `maps to` 0.approximately + } + withPatches(TestPatch("estimation [0,1)->[0,1)", listOf((0..<1) to (0..<1)))) { + 0 `maps to` 0.approximately + } + withPatches(TestPatch("estimation [0,1)->[0,2)", listOf((0..<1) to (0..<2)))) { + 0 `maps to` 1.approximately + } + withPatches(TestPatch("estimation [0,1)->[0,3)", listOf((0..<1) to (0..<3)))) { + 0 `maps to` 1.approximately + } + // relative position inside range 0-2 of line 0 = 0.333, 1 = 0.667 + withPatches(TestPatch("estimation [0,2)->[0,0)", listOf((0..<2) to (0..<0)))) { + 0 `maps to` 0.approximately + 1 `maps to` 0.approximately + } + withPatches(TestPatch("estimation [0,2)->[0,1)", listOf((0..<2) to (0..<1)))) { + 0 `maps to` 0.approximately + 1 `maps to` 0.approximately + } + withPatches(TestPatch("estimation [0,2)->[0,2)", listOf((0..<2) to (0..<2)))) { + 0 `maps to` 0.approximately + 1 `maps to` 1.approximately + } + withPatches(TestPatch("estimation [0,2)->[0,3)", listOf((0..<2) to (0..<3)))) { + 0 `maps to` 1.approximately // 0.333 -> line 1 in [0,1,2] + 1 `maps to` 2.approximately // 0.667 -> line 2 in [0,1,2] + } + // delete content is just always mapped to the first line + withPatches(TestPatch("estimation [0,100)->[0,0)", listOf((0..<100) to (0..<0)))) { + 0 `maps to` 0.approximately + 10 `maps to` 0.approximately + 55 `maps to` 0.approximately + 99 `maps to` 0.approximately + } + } + //endregion + + //region: Utilities + data class TestPatch( + val name: String, + val changes: List>, + val fileStatus: FileStatus = FileStatus.MODIFIED, + ) + + fun List.toTextFilePatches(): List { + val parentName = ROOT_COMMIT + + val patches = mutableListOf() + for (patch in this) { + patches.add(TextFilePatch(Charsets.UTF_8, "\n").apply { + if (patch.fileStatus != FileStatus.ADDED) { + beforeVersionId = parentName + beforeName = FILE_NAME + } + + if (patch.fileStatus != FileStatus.DELETED) { + afterVersionId = patch.name + afterName = FILE_NAME + } + + for (change in patch.changes) { + addHunk(PatchHunk(change.first.first, change.first.last, change.second.first, change.second.last).apply { + // our patch hunks are simple: no context lines, just changes + repeat((change.first.last - change.first.first) + 1) { addLine(REMOVE_LINE) } + repeat((change.second.last - change.second.first) + 1) { addLine(ADD_LINE) } + }) + } + }) + } + return patches.toList() + } + + data class TestCase( + val patches: List, + val fromPatch: String, + val line: Int, + val expected: Int, + val isEstimate: Boolean, + val toSide: Side, + ) { + private val filteredPatches = patches.filter { it.name != startPatch.name && it.name != endPatch.name } + + override fun toString(): String = + "[${filteredPatches.joinToString { it.name }}]: " + + "$line${if (isEstimate) " approximately" else ""}${if (Side.LEFT == toSide) " reverse" else ""} maps to ${expected}" + } + + @JvmInline + value class Approximately(val line: Int) + + class TestCaseBuilder( + private val actualPatches: List = listOf(), + private val fromPatch: String? = null, + ) { + private val firstCommit get() = startPatch.name + private val lastCommit get() = actualPatches.last().name + + // surrounded with a start and end so that we trick the algo into actually executing + // without a start and end patch the algo will just shortcut to return the current line index + private val surroundedPatches = listOf(startPatch) + actualPatches + listOf(endPatch) + var cases = mutableListOf() + + private fun register(case: TestCase) { + cases.add(case) + } + + fun withPatches(vararg patches: TestPatch, block: TestCaseBuilder.() -> Unit) { + cases += TestCaseBuilder(patches.toList()).apply { block() }.cases + } + + fun fromPatch(patch: String, block: TestCaseBuilder.() -> Unit) { + cases += TestCaseBuilder(surroundedPatches, patch).apply { block() }.cases + } + + //@formatter:off + infix fun Int.`maps to`(line: Int) = register(TestCase(surroundedPatches, fromPatch ?: firstCommit, this, line, false, Side.RIGHT)) + infix fun Int.`reverse maps to`(line: Int) = register(TestCase(surroundedPatches, fromPatch ?: lastCommit, this, line, false, Side.LEFT)) + + val Int.approximately get() = Approximately(this) + infix fun Int.`maps to`(approximately: Approximately) = register(TestCase(surroundedPatches, fromPatch ?: firstCommit, this, approximately.line, true, Side.RIGHT)) + infix fun Int.`reverse maps to`(approximately: Approximately) = register(TestCase(surroundedPatches, fromPatch ?: lastCommit, this, approximately.line, true, Side.LEFT)) + //@formatter:on + } + + fun cases(block: TestCaseBuilder.() -> Unit): Array = + TestCaseBuilder().apply { block() }.cases.toTypedArray() + + val REMOVE_LINE = PatchLine(PatchLine.Type.REMOVE, "").apply { isSuppressNewLine = true } + val ADD_LINE = PatchLine(PatchLine.Type.ADD, "").apply { isSuppressNewLine = true } + + const val ROOT_COMMIT = "root" + const val FILE_NAME = "file" + //endregion + } +} \ No newline at end of file diff --git a/plugins/github/github-core/src/org/jetbrains/plugins/github/api/data/pullrequest/GHPullRequestReviewThread.kt b/plugins/github/github-core/src/org/jetbrains/plugins/github/api/data/pullrequest/GHPullRequestReviewThread.kt index 7b9dd62baefd..35281bc4ec0a 100644 --- a/plugins/github/github-core/src/org/jetbrains/plugins/github/api/data/pullrequest/GHPullRequestReviewThread.kt +++ b/plugins/github/github-core/src/org/jetbrains/plugins/github/api/data/pullrequest/GHPullRequestReviewThread.kt @@ -15,22 +15,23 @@ import org.jetbrains.plugins.github.api.data.GHNode import java.util.* @GraphQLFragment("/graphql/fragment/pullRequestReviewThread.graphql") -data class GHPullRequestReviewThread(override val id: String, - val isResolved: Boolean, - val isOutdated: Boolean, - val path: String, - @JsonProperty("diffSide") val side: Side, - val line: Int?, - val originalLine: Int?, - @JsonProperty("startDiffSide") val startSide: Side?, - val startLine: Int?, - val originalStartLine: Int?, - // To be precise: the elements of this list can be null, but should practically never be... - @JsonProperty("comments") private val commentsNodes: GraphQLNodesDTO, - val viewerCanReply: Boolean, - val viewerCanResolve: Boolean, - val viewerCanUnresolve: Boolean) - : GHNode(id) { +data class GHPullRequestReviewThread( + override val id: String, + val isResolved: Boolean, + val isOutdated: Boolean, + val path: String, + @JsonProperty("diffSide") val side: Side, + val line: Int?, + val originalLine: Int?, + @JsonProperty("startDiffSide") val startSide: Side?, + val startLine: Int?, + val originalStartLine: Int?, + // To be precise: the elements of this list can be null, but should practically never be... + @JsonProperty("comments") private val commentsNodes: GraphQLNodesDTO, + val viewerCanReply: Boolean, + val viewerCanResolve: Boolean, + val viewerCanUnresolve: Boolean, +) : GHNode(id) { @JsonIgnore val comments: List = commentsNodes.nodes @JsonIgnore @@ -59,39 +60,48 @@ fun GHPullRequestReviewThread.isVisible(viewOption: DiscussionsViewOption): Bool DiscussionsViewOption.DONT_SHOW -> false } +fun GHPullRequestReviewThread.mapToLeftSideLine(diffData: GitTextFilePatchWithHistory): Int? = + mapToSidedLine(diffData, Side.LEFT) + +fun GHPullRequestReviewThread.mapToRightSideLine(diffData: GitTextFilePatchWithHistory): Int? = + mapToSidedLine(diffData, Side.RIGHT) + +private fun GHPullRequestReviewThread.mapToSidedLine(diffData: GitTextFilePatchWithHistory, side: Side): Int? { + val threadData = this + if (threadData.line == null && threadData.originalLine == null) return null + + val lineIndex = threadData.line ?: threadData.originalLine ?: return null + val fromCommitSha = fromCommitSha(diffData) ?: return null + + return diffData.forcefullyMapLine(fromCommitSha, lineIndex - 1, side) +} + +// TODO: Write tests to illustrate and check the working of location mapping :'( fun GHPullRequestReviewThread.mapToLocation(diffData: GitTextFilePatchWithHistory, sideBias: Side? = null): DiffLineLocation? { val threadData = this if (threadData.line == null && threadData.originalLine == null) return null - return if (threadData.line != null) { - val commitSha = threadData.commit?.oid ?: return null - if (!diffData.contains(commitSha, threadData.path)) return null - when (threadData.side) { - Side.RIGHT -> { - diffData.mapLine(commitSha, threadData.line - 1, sideBias ?: Side.RIGHT) - } - Side.LEFT -> { - diffData.fileHistory.findStartCommit()?.let { baseSha -> - diffData.mapLine(baseSha, threadData.line - 1, sideBias ?: Side.LEFT) - } - } - } + val lineIndex = threadData.line ?: threadData.originalLine ?: return null + val fromCommitSha = fromCommitSha(diffData) ?: return null + + val sideBias = sideBias ?: threadData.side + + return diffData.mapLine(fromCommitSha, lineIndex - 1, sideBias) +} + +private fun GHPullRequestReviewThread.fromCommitSha(diffData: GitTextFilePatchWithHistory): String? { + val threadData = this + + return if (threadData.line != null) when (threadData.side) { + Side.RIGHT -> threadData.commit?.oid + Side.LEFT -> diffData.fileHistory.findStartCommit() } else if (threadData.originalLine != null) { val originalCommitSha = threadData.originalCommit?.oid ?: return null - if (!diffData.contains(originalCommitSha, threadData.path)) return null when (threadData.side) { - Side.RIGHT -> { - diffData.mapLine(originalCommitSha, threadData.originalLine - 1, sideBias ?: Side.RIGHT) - } - Side.LEFT -> { - diffData.fileHistory.findFirstParent(originalCommitSha)?.let { parentSha -> - diffData.mapLine(parentSha, threadData.originalLine - 1, sideBias ?: Side.LEFT) - } - } + Side.RIGHT -> originalCommitSha + Side.LEFT -> diffData.fileHistory.findFirstParent(originalCommitSha) } } - else { - null - } + else null }