Repository: scala-steward-org/scala-steward (scala)

Issue:
Versioning scheme of dependencies is not taken into account
When Scala Steward makes a PR, it decorates it with a set of labels such as “semver-spec-patch”. I believe such labels might be misleading because the actual versioning scheme of the dependency is not taken into account.

For instance, a bump from version `1.2.3` to version `1.3.0` would be labeled as a minor update (`semver-spec-minor`), but it could really be a major update if the library actually uses “package-versioning-policy” (PVP).

That could be an issue if someone wants to setup a mergify rule that automatically merges patch updates, for instance, since there is no proper way to detect what is a patch update (the label `semver-spec-patch` would be set by Scala Steward for a minor update, if the dependency uses PVP).

I believe this issue could be mitigated by taking into account the underlying [versioning scheme](https://www.scala-sbt.org/1.x/docs/Publishing.html#Version+scheme) of the library. The versioning scheme is manually set by the library authors, and there is nothing that forces them to apply the versioning scheme they declared (except some third-party plugins such that sbt-version-policy), so this is why this solution is not perfect, but I do believe that it would be more accurate than the status quo.

A possible way forward would be to add a label “version-scheme-pvp”, “version-scheme-semver-spec”, or “unknown-version-scheme” to the PR’s body without changing the labels that are currently created. Another approach would be to change the existing labels to use the actual versioning scheme (e.g., “semver-spec-patch”, or “pvp-minor”).

Retrieving the underlying version scheme of an artifact is possible with Coursier. It requires querying the `properties` of a `coursier.core.Project` to look for a key `info.versionScheme`.

Pull request:
An additional label is added that reflect the `versionScheme` of the updated artifact. This is a new label. The existing labels have not been changed to ensure compatibility.

- `versionScheme`reference: https://www.scala-sbt.org/1.x/docs/Publishing.html#Version+scheme
- As suggested in https://github.com/scala-steward-org/scala-steward/pull/2863#issuecomment-1368220133 getting the data was trivial.

fixes #2541

Patch:
diff --git a/modules/core/src/main/scala/org/scalasteward/core/coursier/CoursierAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/coursier/CoursierAlg.scala
index 2d9cbd4003..6517a4f86c 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/coursier/CoursierAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/coursier/CoursierAlg.scala
@@ -152,7 +152,9 @@ object CoursierAlg {
       scmUrl = project.info.scm.flatMap(_.url).flatMap(uri.fromStringWithScheme),
       releaseNotesUrl = project.properties
         .collectFirst { case (key, value) if key.equalsIgnoreCase("info.releaseNotesUrl") => value }
-        .flatMap(uri.fromStringWithScheme)
+        .flatMap(uri.fromStringWithScheme),
+      versionScheme = project.properties
+        .collectFirst { case (key, value) if key.equalsIgnoreCase("info.versionScheme") => value }
     )
 
   private def parentOf(project: Project): Option[coursier.Dependency] =
diff --git a/modules/core/src/main/scala/org/scalasteward/core/coursier/DependencyMetadata.scala b/modules/core/src/main/scala/org/scalasteward/core/coursier/DependencyMetadata.scala
index 20d225c7f1..d49d14588d 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/coursier/DependencyMetadata.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/coursier/DependencyMetadata.scala
@@ -24,13 +24,15 @@ import org.scalasteward.core.util.uri
 final case class DependencyMetadata(
     homePage: Option[Uri],
     scmUrl: Option[Uri],
-    releaseNotesUrl: Option[Uri]
+    releaseNotesUrl: Option[Uri],
+    versionScheme: Option[String] = None
 ) {
   def enrichWith(other: DependencyMetadata): DependencyMetadata =
     DependencyMetadata(
       homePage = homePage.orElse(other.homePage),
       scmUrl = scmUrl.orElse(other.scmUrl),
-      releaseNotesUrl = releaseNotesUrl.orElse(other.releaseNotesUrl)
+      releaseNotesUrl = releaseNotesUrl.orElse(other.releaseNotesUrl),
+      versionScheme = versionScheme.orElse(other.versionScheme)
     )
 
   def filterUrls[F[_]](f: Uri => F[Boolean])(implicit F: Monad[F]): F[DependencyMetadata] =
@@ -38,7 +40,7 @@ final case class DependencyMetadata(
       homePage <- homePage.filterA(f)
       scmUrl <- scmUrl.filterA(f)
       releaseNotesUrl <- releaseNotesUrl.filterA(f)
-    } yield DependencyMetadata(homePage, scmUrl, releaseNotesUrl)
+    } yield DependencyMetadata(homePage, scmUrl, releaseNotesUrl, versionScheme)
 
   def repoUrl: Option[Uri] = {
     val urls = scmUrl.toList ++ homePage.toList
@@ -48,5 +50,5 @@ final case class DependencyMetadata(
 
 object DependencyMetadata {
   val empty: DependencyMetadata =
-    DependencyMetadata(None, None, None)
+    DependencyMetadata(None, None, None, None)
 }
diff --git a/modules/core/src/main/scala/org/scalasteward/core/forge/data/NewPullRequestData.scala b/modules/core/src/main/scala/org/scalasteward/core/forge/data/NewPullRequestData.scala
index 3403612b20..deaa37fe13 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/forge/data/NewPullRequestData.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/forge/data/NewPullRequestData.scala
@@ -28,6 +28,7 @@ import org.scalasteward.core.nurture.UpdateInfoUrl
 import org.scalasteward.core.nurture.UpdateInfoUrl._
 import org.scalasteward.core.repoconfig.{GroupRepoConfig, RepoConfigAlg}
 import org.scalasteward.core.util.{Details, Nel}
+
 import scala.util.matching.Regex
 
 final case class NewPullRequestData(
@@ -225,9 +226,8 @@ object NewPullRequestData {
       artifactIdToUrl: Map[String, Uri] = Map.empty,
       artifactIdToUpdateInfoUrls: Map[String, List[UpdateInfoUrl]] = Map.empty,
       filesWithOldVersion: List[String] = List.empty,
-      includeMatchedLabels: Option[Regex] = None
-  ): NewPullRequestData = {
-    val labels = labelsFor(data.update, edits, filesWithOldVersion, includeMatchedLabels)
+      labels: List[String] = List.empty
+  ): NewPullRequestData =
     NewPullRequestData(
       title = CommitMsg
         .replaceVariables(data.repoConfig.commits.messageOrDefault)(
@@ -248,9 +248,8 @@ object NewPullRequestData {
       base = data.baseBranch,
       labels = labels
     )
-  }
 
-  def updateType(anUpdate: Update): List[String] = {
+  def updateTypeLabels(anUpdate: Update): List[String] = {
     def forUpdate(update: Update.Single) = {
       val dependencies = update.dependencies
       if (dependencies.forall(_.configurations.contains("test")))
@@ -268,9 +267,9 @@ object NewPullRequestData {
 
   def labelsFor(
       update: Update,
-      edits: List[EditAttempt],
-      filesWithOldVersion: List[String],
-      includeMatchedLabels: Option[Regex]
+      edits: List[EditAttempt] = List.empty,
+      filesWithOldVersion: List[String] = List.empty,
+      artifactIdToVersionScheme: Map[String, String] = Map.empty
   ): List[String] = {
     val commitCount = edits.flatMap(_.commits).size
     val commitCountLabel = "commit-count:" + (commitCount match {
@@ -287,18 +286,22 @@ object NewPullRequestData {
       val semVerSpecLabel = semVerVersions.flatMap { case (curr, next) =>
         SemVer.getChangeSpec(curr, next).map(c => s"semver-spec-${c.render}")
       }
-      List(earlySemVerLabel, semVerSpecLabel).flatten
+      val versionSchemeLabel =
+        artifactIdToVersionScheme.get(u.mainArtifactId).map(vs => s"version-scheme:$vs")
+      List(earlySemVerLabel, semVerSpecLabel, versionSchemeLabel).flatten
     }
-    val semver: List[String] =
+    val semverLabels =
       update.on(u => semverForUpdate(u), _.updates.flatMap(semverForUpdate(_)).distinct)
 
     val scalafixLabel = edits.collectFirst { case _: ScalafixEdit => "scalafix-migrations" }
     val oldVersionLabel = Option.when(filesWithOldVersion.nonEmpty)("old-version-remains")
 
-    val allLabels = updateType(update) ++
-      semver ++ List(scalafixLabel, oldVersionLabel).flatten ++
+    updateTypeLabels(update) ++
+      semverLabels ++ List(scalafixLabel, oldVersionLabel).flatten ++
       List(commitCountLabel)
-
-    allLabels.filter(label => includeMatchedLabels.fold(true)(_.matches(label)))
   }
+
+  def filterLabels(labels: List[String], includeMatchedLabels: Option[Regex]): List[String] =
+    labels.filter(label => includeMatchedLabels.fold(true)(_.matches(label)))
+
 }
diff --git a/modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala
index e7d8cb1a79..408a61da85 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala
@@ -24,6 +24,7 @@ import org.scalasteward.core.coursier.CoursierAlg
 import org.scalasteward.core.data.ProcessResult.{Created, Ignored, Updated}
 import org.scalasteward.core.data._
 import org.scalasteward.core.edit.{EditAlg, EditAttempt}
+import org.scalasteward.core.forge.data.NewPullRequestData.{filterLabels, labelsFor}
 import org.scalasteward.core.forge.data._
 import org.scalasteward.core.forge.{ForgeApiAlg, ForgeRepoAlg}
 import org.scalasteward.core.git.{Branch, Commit, GitAlg}
@@ -220,12 +221,18 @@ final class NurtureAlg[F[_]](config: ForgeCfg)(implicit
               .tupleLeft(dependency.artifactId.name)
           }
       }
+      artifactIdToVersionScheme = dependencyToMetadata.toList.mapFilter {
+        case (dependency, metadata) =>
+          metadata.versionScheme.tupleLeft(dependency.artifactId.name)
+      }.toMap
       filesWithOldVersion <-
         data.update
           .on(u => List(u.currentVersion.value), _.updates.map(_.currentVersion.value))
           .flatTraverse(gitAlg.findFilesContaining(data.repo, _))
           .map(_.distinct)
       branchName = forge.createBranch(config.tpe, data.fork, data.updateBranch)
+      allLabels = labelsFor(data.update, edits, filesWithOldVersion, artifactIdToVersionScheme)
+      labels = filterLabels(allLabels, data.repoData.config.pullRequests.includeMatchedLabels)
       requestData = NewPullRequestData.from(
         data,
         branchName,
@@ -233,7 +240,7 @@ final class NurtureAlg[F[_]](config: ForgeCfg)(implicit
         artifactIdToUrl,
         artifactIdToUpdateInfoUrls.toMap,
         filesWithOldVersion,
-        data.repoData.config.pullRequests.includeMatchedLabels
+        labels
       )
     } yield requestData
 


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/coursier/CoursierAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/coursier/CoursierAlgTest.scala
index 17105375a4..b6e516f069 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/coursier/CoursierAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/coursier/CoursierAlgTest.scala
@@ -87,7 +87,8 @@ class CoursierAlgTest extends CatsEffectSuite {
     val obtained = coursierAlg.getMetadata(dep, resolvers).runA(MockState.empty)
     val expected = emptyMetadata.copy(
       homePage = Some(uri"https://github.com/japgolly/scalajs-react"),
-      scmUrl = Some(uri"github.com:japgolly/scalajs-react.git")
+      scmUrl = Some(uri"github.com:japgolly/scalajs-react.git"),
+      versionScheme = Some("early-semver")
     )
     assertIO(obtained, expected)
   }
diff --git a/modules/core/src/test/scala/org/scalasteward/core/coursier/DependencyMetadataTest.scala b/modules/core/src/test/scala/org/scalasteward/core/coursier/DependencyMetadataTest.scala
index 06ee49f09c..137dad7a3b 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/coursier/DependencyMetadataTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/coursier/DependencyMetadataTest.scala
@@ -10,7 +10,8 @@ class DependencyMetadataTest extends FunSuite {
     val metadata = DependencyMetadata(
       homePage = Some(uri"https://github.com/japgolly/scalajs-react"),
       scmUrl = Some(uri"github.com:japgolly/scalajs-react.git"),
-      releaseNotesUrl = None
+      releaseNotesUrl = None,
+      versionScheme = Some("early-semver")
     )
     val obtained = metadata.filterUrls(_.renderString.startsWith("http").pure[Id])
     assertEquals(obtained, metadata.copy(scmUrl = None))
diff --git a/modules/core/src/test/scala/org/scalasteward/core/forge/data/NewPullRequestDataTest.scala b/modules/core/src/test/scala/org/scalasteward/core/forge/data/NewPullRequestDataTest.scala
index 0a19c2c41c..b5aba3f4cb 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/forge/data/NewPullRequestDataTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/forge/data/NewPullRequestDataTest.scala
@@ -26,7 +26,11 @@ class NewPullRequestDataTest extends FunSuite {
       dummySha1,
       Branch("update/logback-classic-1.2.3")
     )
-    val obtained = from(data, "scala-steward:update/logback-classic-1.2.3").asJson.spaces2
+    val obtained = from(
+      data,
+      "scala-steward:update/logback-classic-1.2.3",
+      labels = labelsFor(data.update)
+    ).asJson.spaces2
     val expected =
       raw"""|{
             |  "title" : "Update logback-classic to 1.2.3",
@@ -57,7 +61,11 @@ class NewPullRequestDataTest extends FunSuite {
       dummySha1,
       Branch("update/logback-classic-1.2.3")
     )
-    val obtained = from(data, "scala-steward:update/logback-classic-1.2.3").asJson.spaces2
+    val obtained = from(
+      data,
+      "scala-steward:update/logback-classic-1.2.3",
+      labels = labelsFor(data.update)
+    ).asJson.spaces2
     val expected =
       raw"""|{
             |  "title" : "Update logback-classic to 1.2.3",
@@ -148,7 +156,7 @@ class NewPullRequestDataTest extends FunSuite {
     val edits = List(scalafixEdit)
     val appliedMigrations = migrationNote(edits)
     val update = ("a".g % "b".a % "1" -> "2").single
-    val labels = labelsFor(update, edits, List.empty, None)
+    val labels = labelsFor(update, edits, List.empty)
 
     assert(labels.contains("scalafix-migrations"))
     assertEquals(
@@ -178,7 +186,7 @@ class NewPullRequestDataTest extends FunSuite {
     val edits = List(scalafixEdit)
     val detail = migrationNote(edits)
     val update = ("a".g % "b".a % "1" -> "2").single
-    val labels = labelsFor(update, edits, List.empty, None)
+    val labels = labelsFor(update, edits, List.empty)
 
     assert(labels.contains("scalafix-migrations"))
     assertEquals(
@@ -220,7 +228,7 @@ class NewPullRequestDataTest extends FunSuite {
     val edits = List(scalafixEdit1, scalafixEdit2)
     val detail = migrationNote(edits)
     val update = ("a".g % "b".a % "1" -> "2").single
-    val labels = labelsFor(update, edits, List.empty, None)
+    val labels = labelsFor(update, edits, List.empty)
 
     assert(labels.contains("scalafix-migrations"))
     assertEquals(
@@ -242,19 +250,19 @@ class NewPullRequestDataTest extends FunSuite {
   test("updateType") {
     val dependency = "com.example".g % "foo".a % "0.1"
     val single = (dependency %> "0.2").single
-    assertEquals(updateType(single), List("library-update"))
+    assertEquals(updateTypeLabels(single), List("library-update"))
 
     val group = ("com.example".g % Nel.of("foo".a, "bar".a) % "0.1" %> "0.2").group
-    assertEquals(updateType(group), List("library-update"))
+    assertEquals(updateTypeLabels(group), List("library-update"))
 
     val testUpdate = (dependency % "test" %> "0.2").single
-    assertEquals(updateType(testUpdate), List("test-library-update"))
+    assertEquals(updateTypeLabels(testUpdate), List("test-library-update"))
 
     val sbtPluginUpdate = (dependency.copy(sbtVersion = Some(SbtVersion("1.0"))) %> "0.2").single
-    assertEquals(updateType(sbtPluginUpdate), List("sbt-plugin-update"))
+    assertEquals(updateTypeLabels(sbtPluginUpdate), List("sbt-plugin-update"))
 
     val scalafixRuleUpdate = (dependency % "scalafix-rule" %> "0.2").single
-    assertEquals(updateType(scalafixRuleUpdate), List("scalafix-rule-update"))
+    assertEquals(updateTypeLabels(scalafixRuleUpdate), List("scalafix-rule-update"))
   }
 
   test("oldVersionNote without files") {
@@ -267,7 +275,7 @@ class NewPullRequestDataTest extends FunSuite {
     val files = List("Readme.md", "travis.yml")
     val update = ("com.example".g % "foo".a % "0.1" %> "0.2").single
     val note = oldVersionNote(files, update)
-    val labels = labelsFor(update, List.empty, files, None)
+    val labels = labelsFor(update, List.empty, files)
 
     assert(labels.contains("old-version-remains"))
     assertEquals(
@@ -301,21 +309,22 @@ class NewPullRequestDataTest extends FunSuite {
       Some(Commit(dummySha1))
     )
 
-    val oneEdit = labelsFor(update, List(updateEdit), List.empty, None)
+    val oneEdit = labelsFor(update, List(updateEdit), List.empty)
     assert(clue(oneEdit).contains("commit-count:1"))
 
-    val twoEdits = labelsFor(update, List(updateEdit, scalafixEdit), List.empty, None)
+    val twoEdits = labelsFor(update, List(updateEdit, scalafixEdit), List.empty)
     assert(clue(twoEdits).contains("commit-count:n:2"))
   }
 
   test("regex label filtering") {
     val update = ("a".g % "b".a % "1" -> "2").single
     val updateEdit = UpdateEdit(update, Commit(dummySha1))
+    val allLabels = labelsFor(update, List(updateEdit), List.empty)
 
-    val first = labelsFor(update, List(updateEdit), List.empty, Some("library-.+".r))
+    val first = filterLabels(allLabels, Some("library-.+".r))
     assertEquals(clue(first), List("library-update"))
 
-    val second = labelsFor(update, List(updateEdit), List.empty, Some("(.*update.*)|(.*count.*)".r))
+    val second = filterLabels(allLabels, Some("(.*update.*)|(.*count.*)".r))
     assertEquals(clue(second), List("library-update", "commit-count:1"))
   }
 
@@ -324,7 +333,7 @@ class NewPullRequestDataTest extends FunSuite {
     val update2 = ("c".g % "d".a % "1.1.0" % "test" %> "1.2.0").single
     val update = Update.Grouped("my-group", None, List(update1, update2))
 
-    val labels = labelsFor(update, Nil, Nil, None)
+    val labels = labelsFor(update, Nil, Nil)
 
     val expected = List(
       "library-update",
@@ -411,7 +420,11 @@ class NewPullRequestDataTest extends FunSuite {
       Branch("update/logback-classic-1.2.3")
     )
 
-    val obtained = from(data, "scala-steward:update/logback-classic-1.2.3").asJson.spaces2
+    val obtained = from(
+      data,
+      "scala-steward:update/logback-classic-1.2.3",
+      labels = labelsFor(update)
+    ).asJson.spaces2
 
     val body =
       raw"""Updates:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/nurture/NurtureAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/nurture/NurtureAlgTest.scala
index 3980c1a52c..71b2284190 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/nurture/NurtureAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/nurture/NurtureAlgTest.scala
@@ -65,11 +65,17 @@ class NurtureAlgTest extends CatsEffectSuite with Http4sDsl[MockEff] {
              |```
              |</details>
              |
-             |labels: library-update, early-semver-minor, semver-spec-minor, commit-count:1
+             |labels: library-update, early-semver-minor, semver-spec-minor, version-scheme:early-semver, commit-count:1
              |""".stripMargin.trim,
       head = "scala-steward:update/cats-effect-3.4.0",
       base = baseBranch,
-      labels = List("library-update", "early-semver-minor", "semver-spec-minor", "commit-count:1")
+      labels = List(
+        "library-update",
+        "early-semver-minor",
+        "semver-spec-minor",
+        "version-scheme:early-semver",
+        "commit-count:1"
+      )
     )
     assertIO(obtained, expected)
   }