Fix file deletions during a merge

Fixes https://github.com/src-d/hercules/issues/267

Signed-off-by: Vadim Markovtsev <vadim@sourced.tech>

internal/burndown/file.go

@@ -46,7 +46,7 @@ func (file *File) updateTime(currentTime, previousTime, delta int) {
 		panic("previousTime cannot be TreeMergeMark")
 	}
 	if currentTime&TreeMergeMark == TreeMergeMark {
-		// merge mode - `delta` is negative and we have already applied it in a branch
+		// merge mode - we have already updated in one of the branches
 		return
 	}
 	for _, update := range file.updaters {

leaves/burndown.go

@@ -94,6 +94,8 @@ type BurndownAnalysis struct {
 	mergedAuthor int
 	// renames is a quick and dirty solution for the "future branch renames" problem.
 	renames map[string]string
+	// deletions is a quick and dirty solution for the "real merge removals" problem.
+	deletions map[string]bool
 	// matrix is the mutual deletions and self insertions.
 	matrix []map[int]int64
 	// tick is the most recent tick index processed.
@@ -243,7 +245,7 @@ func (analyser *BurndownAnalysis) ListConfigurationOptions() []core.Configuratio
 		Type:    core.PathConfigurationOption,
 		Default: ""}, {
 		Name:        ConfigBurndownDebug,
-		Description: "Validate the trees on each step.",
+		Description: "Validate the trees at each step.",
 		Flag:        "burndown-debug",
 		Type:        core.BoolConfigurationOption,
 		Default:     false},
@@ -338,6 +340,7 @@ func (analyser *BurndownAnalysis) Initialize(repository *git.Repository) error {
 	analyser.mergedFiles = map[string]bool{}
 	analyser.mergedAuthor = identity.AuthorMissing
 	analyser.renames = map[string]string{}
+	analyser.deletions = map[string]bool{}
 	analyser.matrix = make([]map[int]int64, analyser.PeopleNumber)
 	analyser.tick = 0
 	analyser.previousTick = 0
@@ -1235,6 +1238,7 @@ func (analyser *BurndownAnalysis) handleInsertion(
 	}
 	file, err = analyser.newFile(hash, name, author, analyser.tick, lines)
 	analyser.files[name] = file
+	delete(analyser.deletions, name)
 	if analyser.tick == burndown.TreeMergeMark {
 		analyser.mergedFiles[name] = true
 	}
@@ -1260,7 +1264,17 @@ func (analyser *BurndownAnalysis) handleDeletion(
 	if !exists {
 		return nil
 	}
-	file.Update(analyser.packPersonWithTick(author, analyser.tick), 0, 0, lines)
+	// Parallel independent file removals are incorrectly handled. The solution seems to be quite
+	// complex, but feel free to suggest your ideas.
+	// These edge cases happen *very* rarely, so we don't bother for now.
+	tick := analyser.tick
+	// Are we merging and this file has never been actually deleted in any branch?
+	if analyser.tick == burndown.TreeMergeMark && !analyser.deletions[name] {
+		tick = 0
+		// Early removal in one branch with pre-merge changes in another is not handled correctly.
+	}
+	analyser.deletions[name] = true
+	file.Update(analyser.packPersonWithTick(author, tick), 0, 0, lines)
 	file.Delete()
 	delete(analyser.files, name)
 	delete(analyser.fileHistories, name)
@@ -1416,6 +1430,7 @@ func (analyser *BurndownAnalysis) handleRename(from, to string) error {
 	}
 	delete(analyser.files, from)
 	analyser.files[to] = file
+	delete(analyser.deletions, to)
 	if analyser.tick == burndown.TreeMergeMark {
 		analyser.mergedFiles[from] = false
 	}