fix(ChangeLog): Don't include changes from an earlier pre-release in the change log (#61)

Author: jmongard

src/main/kotlin/git/semver/plugin/semver/MutableSemVersion.kt

@@ -231,12 +231,13 @@ internal class MutableSemVersion(
         }
     }
 
-    internal fun mergeChanges(versions: List<MutableSemVersion>) {
+    internal fun mergeChanges(versions: List<MutableSemVersion>): MutableSemVersion {
         this.commitCount = versions.sumOf { it.commitCount }
         this.bumpPatch = versions.sumOf { it.bumpPatch }
         this.bumpMinor = versions.sumOf { it.bumpMinor }
         this.bumpMajor = versions.sumOf { it.bumpMajor }
         this.bumpPre = versions.sumOf { it.bumpPre }
+        return this
     }
 
     private val hasPendingChanges

src/main/kotlin/git/semver/plugin/semver/VersionFinder.kt

@@ -3,8 +3,7 @@ package git.semver.plugin.semver
 import git.semver.plugin.scm.Commit
 import git.semver.plugin.scm.IRefInfo
 import org.slf4j.LoggerFactory
-import java.util.ArrayDeque
-import java.util.PriorityQueue
+import java.util.*
 
 class VersionFinder(private val settings: SemverSettings, private val tags: Map<String, List<IRefInfo>>) {
     private val logger = LoggerFactory.getLogger(javaClass)
@@ -32,51 +31,74 @@ class VersionFinder(private val settings: SemverSettings, private val tags: Map<
     }
 
     fun getChangeLog(commit: Commit): List<Commit> {
-        val changeLog = mutableListOf<Commit>()
-
         val release = getReleaseSemVersionFromCommit(commit)
-        if (isRelease(release)) {
-            findVersion(commit.parents, changeLog)
+        return if (isRelease(release, true)) {
+            // If this is a release commit, find all changes from the parent commits
+            val changeLog = getChangeLog(getIncludedCommits(commit.parents, true).includedCommits)
             changeLog.add(commit)
+            changeLog
         } else {
-            findVersion(commit, changeLog)
+            getChangeLog(getIncludedCommits(sequenceOf(commit), true).includedCommits)
         }
-        return changeLog
     }
 
-    private fun findVersion(startCommit: Commit, changeLog: MutableList<Commit>? = null): MutableSemVersion {
+    private fun getChangeLog(commitData: List<CommitData>): MutableList<Commit> = commitData.asReversed()
+        .filter { it.parents.size <= 1 }
+        .map { it.commit }
+        .toMutableList()
+
+    private fun findVersion(startCommit: Commit): MutableSemVersion {
         if (startCommit.sha.isBlank()) {
             //This is a fake commit created when there exists no real commits
             return versionZero()
         }
-        return findVersion(sequenceOf(startCommit), changeLog)
+
+        val result = getIncludedCommits(sequenceOf(startCommit), false)
+        var lastFoundVersion = result.lastFoundVersion
+        for (commitData in result.includedCommits.asReversed()) {
+            // Second time we visit this commit after visiting parent commits
+
+            // Check if we found a preRelease version first time we visited this commit
+            val preReleaseVersion = result.visitedCommits[commitData.commit.sha]
+            // Get and clear the semVersions for the parents so that they are not counted twice
+            val parentSemVersions = commitData.parents
+                .mapNotNull { result.visitedCommits.remove(it) }
+                .toList()
+            val maxVersionFromParents = getCombinedParentVersion(parentSemVersions)
+            maxVersionFromParents.updateFromCommit(commitData.commit, settings, preReleaseVersion)
+            result.visitedCommits[commitData.commit.sha] = maxVersionFromParents
+            lastFoundVersion = maxVersionFromParents
+        }
+        return lastFoundVersion
     }
 
-    data class CommitData(
+    private class CommitData(
         val commit: Commit,
-        val parents: MutableList<String>,
-        val isParentOfReleaseCommit: Boolean
+        val parents: MutableList<String> = ArrayList(1),
+        val isParentOfReleaseCommit: Boolean = false
     ) : Comparable<CommitData> {
 
         override fun compareTo(other: CommitData): Int {
             return other.commit.commitTime.compareTo(commit.commitTime)
         }
     }
 
-    private fun findVersion(
-        commitsList: Sequence<Commit>,
-        changeLog: MutableList<Commit>?
-    ): MutableSemVersion {
+    private class IncludedCommits(
+        val lastFoundVersion: MutableSemVersion,
+        val visitedCommits: MutableMap<String, MutableSemVersion?>,
+        val includedCommits: List<CommitData>
+    )
 
-        var liveBranchCount = 1;
+    private fun getIncludedCommits(commitsList: Sequence<Commit>, stopAtPreRelease: Boolean): IncludedCommits {
+        var liveBranchCount = 1
         var lastFoundVersion = versionZero()
 
         // This code is a recursive algoritm rewritten as iterative to avoid stack overflow exception.
         // Unfortunately that makes it hard to understand.
 
-        val commits = PriorityQueue(commitsList.map { CommitData(it, ArrayList(1), false) }.toList())
+        val commits = PriorityQueue(commitsList.map { CommitData(it) }.toList())
         val visitedCommits = mutableMapOf<String, MutableSemVersion?>()
-        val includedCommits = ArrayDeque<CommitData>()
+        val includedCommits = mutableListOf<CommitData>()
 
         while (commits.isNotEmpty()) {
             val commitData = commits.remove()
@@ -92,56 +114,46 @@ class VersionFinder(private val settings: SemverSettings, private val tags: Map<
                 val releaseVersion = getReleaseSemVersionFromCommit(currentCommit)
                 visitedCommits[currentCommit.sha] = releaseVersion
 
-                if (isRelease(releaseVersion)) {
+                if (isRelease(releaseVersion, stopAtPreRelease)) {
                     logger.debug("Release version found: {}", releaseVersion)
                     // Release fond so no need to visit this commit again
                     lastFoundVersion = releaseVersion!!
-
                     liveBranchCount -= 1
 
                     markParentCommitsAsVisited(liveBranchCount, currentCommit, visitedCommits, commits)
                 } else {
                     // This is a normal commit or a pre-release. We will visit this again in the second phase.
-                    includedCommits.push(commitData)
+                    includedCommits.add(commitData)
 
                     currentCommit.parents.forEach {
                         commitData.parents.add(it.sha)
                         if (!visitedCommits.containsKey(it.sha)) {
                             // prepare to visit parent commit
-                            commits.add(CommitData(it, ArrayList(1), false))
+                            commits.add(CommitData(it))
                         }
                     }
                     liveBranchCount += commitData.parents.size - 1
                 }
             }
         }
+        return IncludedCommits(lastFoundVersion, visitedCommits, includedCommits)
+    }
 
-        while (includedCommits.isNotEmpty()) {
-
-            val commitData = includedCommits.pop()
-            val currentCommit = commitData.commit
-            val currentParentList = commitData.parents
-
-            // Second time we visit this commit after visiting parent commits
-            addToChangeLog(currentCommit, changeLog, currentParentList.size > 1)
-
-            // Check if we found a preRelease version first time we visited this commit
-            val preReleaseVersion = visitedCommits[currentCommit.sha]
-
-            // Get and clear the semVersions for the parents so that they are not counted twice
-            val parentSemVersions = currentParentList
-                .mapNotNull { visitedCommits.put(it, null) }
-                .toList()
-
-            val maxVersionFromParents = parentSemVersions.maxOrNull() ?: versionZero()
-            maxVersionFromParents.mergeChanges(parentSemVersions)
-            maxVersionFromParents.updateFromCommit(currentCommit, settings, preReleaseVersion)
-            visitedCommits[currentCommit.sha] = maxVersionFromParents
+    private fun getCombinedParentVersion(parentSemVersions: List<MutableSemVersion>): MutableSemVersion {
+        return when {
+            parentSemVersions.isEmpty() -> {
+                versionZero()
+            }
 
-            lastFoundVersion = maxVersionFromParents
+            parentSemVersions.size == 1 -> {
+                parentSemVersions[0]
+            }
 
+            else -> {
+                parentSemVersions.max()
+                    .mergeChanges(parentSemVersions)
+            }
         }
-        return lastFoundVersion
     }
 
     private fun markParentCommitsAsVisited(
@@ -159,20 +171,8 @@ class VersionFinder(private val settings: SemverSettings, private val tags: Map<
         }
     }
 
-    private fun addToChangeLog(
-        currentCommit: Commit,
-        changeLog: MutableList<Commit>?,
-        isMergeCommit: Boolean
-    ) {
-        if (isMergeCommit) {
-            //Ignore merge commits
-            return
-        }
-        changeLog?.add(currentCommit)
-    }
-
-    private fun isRelease(releaseVersion: MutableSemVersion?) =
-        releaseVersion != null && !releaseVersion.isPreRelease
+    private fun isRelease(releaseVersion: MutableSemVersion?, stopAtPreRelease: Boolean) =
+        releaseVersion != null && (stopAtPreRelease || !releaseVersion.isPreRelease)
 
     private fun versionZero() = MutableSemVersion()
 

src/test/kotlin/git/semver/plugin/scm/GitProviderTest.kt

@@ -124,6 +124,31 @@ class GitProviderTest {
         }
     }
 
+    @Test
+    fun changeLog_stops_at_prerelease() {
+        val gitDir = File("build/integrationTest33")
+        gitDir.mkdirs()
+
+        val gitProvider = GitProvider(SemverSettings())
+        Git.init().setDirectory(gitDir).call().use {
+            initOrReset(it, gitProvider)
+            commit(it, "fix: first change", gitProvider)
+            release(gitProvider, it, "alpha.1")
+            commit(it, "build: some changes", gitProvider)
+            commit(it, "feat: another feature", gitProvider)
+            commit(it, "fix: another change", gitProvider)
+            release(gitProvider, it, "-")
+
+            val actual = gitProvider().changeLog(it)
+
+            assertThat(actual.map(Commit::toString))
+                .contains("feat: another feature")
+                .contains("fix: another change")
+                .contains("release: v0.1.0")
+                .doesNotContain("fix: first change")
+        }
+    }
+
     private fun lotsOfCommits(it: Git, gitProvider: GitProvider) {
         initOrReset(it, gitProvider)
         commit(it, "build: some changes", gitProvider)