git: support files that are renamed in working tree

EA-209061 - GFE: GitChangesCollector.throwGFE

GitOrigin-RevId: 037360774183da0426e4507511ff998bc776b938

plugins/git4idea/src/git4idea/status/GitChangesCollector.java

@@ -231,6 +231,18 @@ class GitChangesCollector {
 
       final FilePath filepath = GitContentRevision.createPath(myVcsRoot, path);
 
+      FilePath oldFilepath;
+      if (xStatus == 'R' || xStatus == 'C' ||
+          yStatus == 'R' || yStatus == 'C') {
+        // We treat "Copy" as "Added", but we still have to read the old path not to break the format parsing.
+        //noinspection AssignmentToForLoopParameter
+        pos += 1;  // read the "from" filepath which is separated also by NUL character.
+        oldFilepath = GitContentRevision.createPath(myVcsRoot, split[pos]);
+      }
+      else {
+        oldFilepath = null;
+      }
+
       switch (xStatus) {
         case ' ':
           if (yStatus == 'M') {
@@ -239,7 +251,7 @@ class GitChangesCollector {
           else if (yStatus == 'D') {
             reportDeleted(filepath, head);
           }
-          else if (yStatus == 'A') {
+          else if (yStatus == 'A' || yStatus == 'C') {
             reportAdded(filepath);
           }
           else if (yStatus == 'T') {
@@ -248,6 +260,9 @@ class GitChangesCollector {
           else if (yStatus == 'U') {
             reportConflict(filepath, head, Status.MODIFIED, Status.MODIFIED);
           }
+          else if (yStatus == 'R') {
+            reportRename(filepath, oldFilepath, head);
+          }
           else {
             throwYStatus(output, handler, line, xStatus, yStatus);
           }
@@ -266,10 +281,6 @@ class GitChangesCollector {
           break;
 
         case 'C':
-          //noinspection AssignmentToForLoopParameter
-          pos += 1;  // read the "from" filepath which is separated also by NUL character.
-          // NB: no "break" here!
-          // we treat "Copy" as "Added", but we still have to read the old path not to break the format parsing.
         case 'A':
           if (yStatus == 'M' || yStatus == ' ' || yStatus == 'T') {
             reportAdded(filepath);
@@ -298,6 +309,12 @@ class GitChangesCollector {
           else if (yStatus == 'D') { // DD - unmerged, both deleted
             reportConflict(filepath, head, Status.DELETED, Status.DELETED);
           }
+          else if (yStatus == 'C') {
+            reportModified(filepath, head);
+          }
+          else if (yStatus == 'R') {
+            reportRename(filepath, oldFilepath, head);
+          }
           else {
             throwYStatus(output, handler, line, xStatus, yStatus);
           }
@@ -319,10 +336,6 @@ class GitChangesCollector {
           break;
 
         case 'R':
-          //noinspection AssignmentToForLoopParameter
-          pos += 1;  // read the "from" filepath which is separated also by NUL character.
-          FilePath oldFilepath = GitContentRevision.createPath(myVcsRoot, split[pos]);
-
           if (yStatus == 'D') {
             reportDeleted(oldFilepath, head);
           }

plugins/git4idea/tests/git4idea/test/GitPlatformTest.kt

@@ -228,6 +228,14 @@ abstract class GitPlatformTest : VcsPlatformTest() {
     assertTrue("Commit dialog was not shown", vcsHelper.commitDialogWasShown())
   }
 
+  protected fun assertNoChanges() {
+    changeListManager.assertNoChanges()
+  }
+
+  protected fun assertChanges(changes: ChangesBuilder.() -> Unit): List<Change> {
+    return changeListManager.assertChanges(changes)
+  }
+
   protected data class ReposTrinity(val projectRepo: GitRepository, val parent: File, val bro: File)
 
 

plugins/git4idea/tests/git4idea/test/GitTestUtil.kt

@@ -55,7 +55,7 @@ fun createFileStructure(rootDir: VirtualFile, vararg paths: String) {
       mkdir(path)
     }
     else {
-      touch(path, "initial_content_" + Math.random())
+      touch(path, "initial_content_in_{$path}")
     }
   }
 }

plugins/git4idea/tests/git4idea/tests/GitChangeProviderConflictTest.kt

@@ -27,7 +27,7 @@ class GitChangeProviderConflictTest : GitChangeProviderTest() {
    */
   fun testConflictMM() {
     modifyFileInBranches("a.txt", FileAction.MODIFY, FileAction.MODIFY)
-    assertChanges(atxt, FileStatus.MERGED_WITH_CONFLICTS)
+    assertProviderChanges(atxt, FileStatus.MERGED_WITH_CONFLICTS)
     assertManagerConflicts(Conflict("a.txt", Status.MODIFIED, Status.MODIFIED))
   }
 
@@ -36,7 +36,7 @@ class GitChangeProviderConflictTest : GitChangeProviderTest() {
    */
   fun testConflictMD() {
     modifyFileInBranches("a.txt", FileAction.MODIFY, FileAction.DELETE)
-    assertChanges(atxt, FileStatus.MERGED_WITH_CONFLICTS)
+    assertProviderChanges(atxt, FileStatus.MERGED_WITH_CONFLICTS)
     assertManagerConflicts(Conflict("a.txt", Status.MODIFIED, Status.DELETED))
   }
 
@@ -45,7 +45,7 @@ class GitChangeProviderConflictTest : GitChangeProviderTest() {
    */
   fun testConflictDM() {
     modifyFileInBranches("a.txt", FileAction.DELETE, FileAction.MODIFY)
-    assertChanges(atxt, FileStatus.MERGED_WITH_CONFLICTS)
+    assertProviderChanges(atxt, FileStatus.MERGED_WITH_CONFLICTS)
     assertManagerConflicts(Conflict("a.txt", Status.DELETED, Status.MODIFIED))
   }
 
@@ -55,21 +55,21 @@ class GitChangeProviderConflictTest : GitChangeProviderTest() {
   fun testConflictCC() {
     modifyFileInBranches("z.txt", FileAction.CREATE, FileAction.CREATE)
     val zfile = projectRoot.findChild("z.txt")
-    assertChanges(zfile!!, FileStatus.MERGED_WITH_CONFLICTS)
+    assertProviderChanges(zfile!!, FileStatus.MERGED_WITH_CONFLICTS)
     assertManagerConflicts(Conflict("z.txt", Status.ADDED, Status.ADDED))
   }
 
   fun testConflictRD() {
     modifyFileInBranches("a.txt", FileAction.RENAME, FileAction.DELETE)
     val newfile = projectRoot.findChild("a.txt_master_new") // renamed in master
-    assertChanges(newfile!!, FileStatus.MERGED_WITH_CONFLICTS)
+    assertProviderChanges(newfile!!, FileStatus.MERGED_WITH_CONFLICTS)
     assertManagerConflicts(Conflict("a.txt_master_new", Status.ADDED, Status.MODIFIED))
   }
 
   fun testConflictDR() {
     modifyFileInBranches("a.txt", FileAction.DELETE, FileAction.RENAME)
     val newFile = projectRoot.findChild("a.txt_feature_new") // deleted in master, renamed in feature
-    assertChanges(newFile!!, FileStatus.MERGED_WITH_CONFLICTS)
+    assertProviderChanges(newFile!!, FileStatus.MERGED_WITH_CONFLICTS)
     assertManagerConflicts(Conflict("a.txt_feature_new", Status.MODIFIED, Status.ADDED))
   }
 
@@ -77,7 +77,7 @@ class GitChangeProviderConflictTest : GitChangeProviderTest() {
     modifyFileInBranches("a.txt", FileAction.RENAME, FileAction.RENAME)
     val newMasterFile = projectRoot.findChild("a.txt_master_new")!!
     val newFeatureFile = projectRoot.findChild("a.txt_feature_new")!!
-    assertChanges(listOf(newMasterFile, newFeatureFile), listOf(FileStatus.MERGED_WITH_CONFLICTS, FileStatus.MERGED_WITH_CONFLICTS))
+    assertProviderChanges(listOf(newMasterFile, newFeatureFile), listOf(FileStatus.MERGED_WITH_CONFLICTS, FileStatus.MERGED_WITH_CONFLICTS))
     assertManagerConflicts(Conflict("a.txt_master_new", Status.ADDED, Status.MODIFIED),
                            Conflict("a.txt_feature_new", Status.MODIFIED, Status.ADDED),
                            Conflict("a.txt", Status.DELETED, Status.DELETED, false))

plugins/git4idea/tests/git4idea/tests/GitChangeProviderTest.kt

@@ -7,20 +7,19 @@ import com.intellij.openapi.vcs.Executor.cd
 import com.intellij.openapi.vcs.FilePath
 import com.intellij.openapi.vcs.FileStatus
 import com.intellij.openapi.vcs.VcsTestUtil.*
-import com.intellij.openapi.vcs.changes.Change
-import com.intellij.openapi.vcs.changes.ChangeListManager
-import com.intellij.openapi.vcs.changes.ContentRevision
-import com.intellij.openapi.vcs.changes.VcsModifiableDirtyScope
+import com.intellij.openapi.vcs.changes.*
 import com.intellij.openapi.vfs.VfsUtil
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.testFramework.vcs.MockChangeListManagerGate
 import com.intellij.testFramework.vcs.MockChangelistBuilder
 import com.intellij.testFramework.vcs.MockDirtyScope
 import com.intellij.vcsUtil.VcsUtil
+import git4idea.config.GitVersion
 import git4idea.status.GitChangeProvider
 import git4idea.test.GitSingleRepoTest
 import git4idea.test.addCommit
 import git4idea.test.createFileStructure
+import org.junit.Assume
 import java.io.File
 import java.util.*
 
@@ -61,16 +60,21 @@ abstract class GitChangeProviderTest : GitSingleRepoTest() {
 
   override fun makeInitialCommit() = false
 
-  private fun getVirtualFile(relativePath: String) = VfsUtil.findFileByIoFile(File(projectPath, relativePath), true)!!
+  protected fun getVirtualFile(relativePath: String) = VfsUtil.findFileByIoFile(File(projectPath, relativePath), true)!!
 
   /**
    * Checks that the given files have respective statuses in the change list retrieved from myChangesProvider.
    * Pass null in the fileStatuses array to indicate that proper file has not changed.
    */
-  protected fun assertChanges(virtualFiles: List<VirtualFile>, fileStatuses: List<FileStatus?>) {
-    val result = getChanges(virtualFiles)
-    for (i in virtualFiles.indices) {
-      val fp = VcsUtil.getFilePath(virtualFiles[i])
+
+  protected fun assertProviderChanges(virtualFiles: List<VirtualFile>, fileStatuses: List<FileStatus?>) {
+    assertProviderChangesInPaths(virtualFiles.map { VcsUtil.getFilePath(it) }, fileStatuses)
+  }
+
+  protected fun assertProviderChangesInPaths(paths: List<FilePath>, fileStatuses: List<FileStatus?>) {
+    val result = getProviderChanges(paths)
+    for (i in paths.indices) {
+      val fp = paths[i]
       val status = fileStatuses[i]
       if (status == null) {
         assertFalse("File [" + tos(fp) + " shouldn't be in the changelist, but it was.", result.containsKey(fp))
@@ -81,17 +85,20 @@ abstract class GitChangeProviderTest : GitSingleRepoTest() {
     }
   }
 
-  protected fun assertChanges(virtualFile: VirtualFile, fileStatus: FileStatus) {
-    assertChanges(listOf(virtualFile), listOf(fileStatus))
+  protected fun assertProviderChanges(virtualFile: VirtualFile, fileStatus: FileStatus) {
+    assertProviderChanges(listOf(virtualFile), listOf(fileStatus))
+  }
+
+  protected fun assumeWorktreeRenamesSupported() {
+    Assume.assumeTrue("Worktree renames are not supported by git: ${vcs.version}",
+                      vcs.version.isLaterOrEqual(GitVersion(2, 17, 0, 0)))
   }
 
   /**
    * Marks the given files dirty in myDirtyScope, gets changes from myChangeProvider and groups the changes in the map.
    * Assumes that only one change for a file has happened.
    */
-  private fun getChanges(changedFiles: List<VirtualFile>): Map<FilePath, Change> {
-    val changedPaths = changedFiles.map { VcsUtil.getFilePath(it) }
-
+  private fun getProviderChanges(changedPaths: List<FilePath>): Map<FilePath, Change> {
     // get changes
     val builder = MockChangelistBuilder()
     changeProvider.getChanges(dirtyScope, builder, EmptyProgressIndicator(),
@@ -101,21 +108,7 @@ abstract class GitChangeProviderTest : GitSingleRepoTest() {
     // get changes for files
     val result = HashMap<FilePath, Change>()
     for (change in changes) {
-      val file = change.virtualFile
-      var filePath: FilePath? = null
-      if (file == null) { // if a file was deleted, just find the reference in the original list of files and use it.
-        val path = change.beforeRevision!!.file.path
-        for (fp in changedPaths) {
-          if (FileUtil.pathsEqual(fp.path, path)) {
-            filePath = fp
-            break
-          }
-        }
-      }
-      else {
-        filePath = VcsUtil.getFilePath(file)
-      }
-      result.put(filePath!!, change)
+      result.put(ChangesUtil.getFilePath(change), change)
     }
     return result
   }

plugins/git4idea/tests/git4idea/tests/GitChangeProviderVersionedTest.kt

@@ -3,20 +3,29 @@ package git4idea.tests
 
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.util.io.FileUtil
+import com.intellij.openapi.vcs.Executor.rm
+import com.intellij.openapi.vcs.Executor.touch
 import com.intellij.openapi.vcs.FileStatus.*
+import com.intellij.openapi.vfs.VfsUtil
 import com.intellij.openapi.vfs.VfsUtilCore
 import com.intellij.testFramework.VfsTestUtil.createDir
 import com.intellij.testFramework.runInEdtAndGet
 import com.intellij.ui.GuiUtils
 import com.intellij.vcsUtil.VcsUtil
 import git4idea.test.add
+import git4idea.test.addCommit
+import git4idea.test.git
 
 class GitChangeProviderVersionedTest : GitChangeProviderTest() {
 
   fun testCreateFile() {
     val file = create(projectRoot, "new.txt")
     repo.add(file.path)
-    assertChanges(file, ADDED)
+    assertProviderChanges(file, ADDED)
+
+    assertChanges {
+      added("new.txt")
+    }
   }
 
   fun testCreateFileInDir() {
@@ -24,17 +33,30 @@ class GitChangeProviderVersionedTest : GitChangeProviderTest() {
     dirty(dir)
     val bfile = create(dir, "new.txt")
     repo.add(bfile.path)
-    assertChanges(listOf(bfile, dir), listOf(ADDED, null))
+    assertProviderChanges(listOf(bfile, dir),
+                          listOf(ADDED, null))
+
+    assertChanges {
+      added("newdir/new.txt")
+    }
   }
 
   fun testEditFile() {
     edit(atxt, "new content")
-    assertChanges(atxt, MODIFIED)
+    assertProviderChanges(atxt, MODIFIED)
+
+    assertChanges {
+      modified("a.txt")
+    }
   }
 
   fun testDeleteFile() {
     deleteFile(atxt)
-    assertChanges(atxt, DELETED)
+    assertProviderChanges(atxt, DELETED)
+
+    assertChanges {
+      deleted("a.txt")
+    }
   }
 
   fun testDeleteDirRecursively() {
@@ -45,8 +67,13 @@ class GitChangeProviderVersionedTest : GitChangeProviderTest() {
         FileUtil.delete(VfsUtilCore.virtualToIoFile(dir))
       }
     }
-    assertChanges(listOf(dir_ctxt, subdir_dtxt),
-                  listOf(DELETED, DELETED))
+    assertProviderChanges(listOf(dir_ctxt, subdir_dtxt),
+                          listOf(DELETED, DELETED))
+
+    assertChanges {
+      deleted("dir/c.txt")
+      deleted("dir/subdir/d.txt")
+    }
   }
 
   fun testSimultaneousOperationsOnMultipleFiles() {
@@ -56,7 +83,61 @@ class GitChangeProviderVersionedTest : GitChangeProviderTest() {
     val newfile = create(projectRoot, "newfile.txt")
     repo.add()
 
-    assertChanges(listOf(atxt, dir_ctxt, subdir_dtxt, newfile), listOf(MODIFIED, MODIFIED, DELETED, ADDED))
+    assertProviderChanges(listOf(atxt, dir_ctxt, subdir_dtxt, newfile),
+                          listOf(MODIFIED, MODIFIED, DELETED, ADDED))
+
+    assertChanges {
+      modified("a.txt")
+      modified("dir/c.txt")
+      deleted("dir/subdir/d.txt")
+      added("newfile.txt")
+    }
   }
 
+  fun testRenamedInWorktree() {
+    assumeWorktreeRenamesSupported()
+
+    touch("rename.txt", "rename_file_content")
+    addCommit("init rename")
+
+    // do not trigger move via VcsVFSListener
+    rm("rename.txt")
+    touch("unstaged.txt", "rename_file_content")
+    repo.git("add -N unstaged.txt")
+
+    VfsUtil.markDirtyAndRefresh(false, true, true, projectRoot)
+    dirty(projectRoot)
+
+    assertProviderChangesInPaths(listOf("rename.txt", "staged.txt", "unstaged.txt").map { VcsUtil.getFilePath(projectRoot, it) },
+                                 listOf(null, null, MODIFIED))
+
+    assertChanges {
+      rename("rename.txt", "unstaged.txt")
+    }
+  }
+
+  fun testTwiceRenamed() {
+    assumeWorktreeRenamesSupported()
+
+    touch("rename.txt", "rename_file_content")
+    addCommit("init rename")
+
+    repo.git("mv rename.txt staged.txt")
+
+    // do not trigger move via VcsVFSListener
+    rm("staged.txt")
+    touch("unstaged.txt", "rename_file_content")
+    repo.git("add -N unstaged.txt")
+
+    VfsUtil.markDirtyAndRefresh(false, true, true, projectRoot)
+    dirty(projectRoot)
+
+    assertProviderChangesInPaths(listOf("rename.txt", "staged.txt", "unstaged.txt").map { VcsUtil.getFilePath(projectRoot, it) },
+                                 listOf(null, MODIFIED, MODIFIED))
+
+    assertChanges {
+      rename("rename.txt", "staged.txt")
+      rename("staged.txt", "unstaged.txt")
+    }
+  }
 }

plugins/git4idea/tests/git4idea/tests/GitCommitTest.kt

@@ -8,7 +8,6 @@ import com.intellij.openapi.util.io.FileUtil
 import com.intellij.openapi.util.registry.Registry
 import com.intellij.openapi.vcs.Executor.*
 import com.intellij.openapi.vcs.FilePath
-import com.intellij.openapi.vcs.changes.Change
 import git4idea.checkin.GitCheckinEnvironment
 import git4idea.checkin.GitCheckinExplicitMovementProvider
 import git4idea.checkin.isCommitRenamesSeparately
@@ -917,14 +916,6 @@ abstract class GitCommitTest(private val useStagingArea: Boolean) : GitSingleRep
     git("mv -f $from $to")
   }
 
-  private fun assertNoChanges() {
-    changeListManager.assertNoChanges()
-  }
-
-  private fun assertChanges(changes: ChangesBuilder.() -> Unit): List<Change> {
-    return changeListManager.assertChanges(changes)
-  }
-
   private class MyExplicitMovementProvider : GitCheckinExplicitMovementProvider() {
     override fun isEnabled(project: Project): Boolean = true
 

plugins/git4idea/tests/git4idea/tests/GitPartialCommitTest.kt

@@ -7,14 +7,16 @@ import com.intellij.openapi.application.runWriteAction
 import com.intellij.openapi.editor.Document
 import com.intellij.openapi.fileEditor.FileDocumentManager
 import com.intellij.openapi.vcs.Executor.child
-import com.intellij.openapi.vcs.changes.Change
 import com.intellij.openapi.vcs.changes.ChangesUtil
 import com.intellij.openapi.vcs.changes.LocalChangeList
 import com.intellij.openapi.vcs.ex.PartialLocalLineStatusTracker
 import com.intellij.openapi.vcs.impl.LineStatusTrackerManager
 import com.intellij.openapi.vfs.LocalFileSystem
 import com.intellij.util.ui.UIUtil
-import git4idea.test.*
+import git4idea.test.GitSingleRepoTest
+import git4idea.test.assertCommitted
+import git4idea.test.gitAsBytes
+import git4idea.test.tac
 
 class GitPartialCommitTest : GitSingleRepoTest() {
   fun `test partial commit with changelists`() {
@@ -190,14 +192,6 @@ class GitPartialCommitTest : GitSingleRepoTest() {
   }
 
 
-  private fun assertNoChanges() {
-    changeListManager.assertNoChanges()
-  }
-
-  private fun assertChanges(changes: ChangesBuilder.() -> Unit): List<Change> {
-    return changeListManager.assertChanges(changes)
-  }
-
   private fun assertCommittedContent(fileName: String, expectedContent: String, useFilters: Boolean = false) {
     val actualContent = repo.gitAsBytes("cat-file" +
                                         (if (useFilters) " --filters" else " -p") +