Repository: scala-steward-org/scala-steward
Pull request #1801 (2020-12-11 21:39:00)

Do not run post-update hooks if version could not be bumped
This prevents updates like https://github.com/ghostdogpr/caliban/pull/684 where
the version could not be bumped and the only commit is from the
post-update hook.

Follow-up to #1800.

codecov[bot]: # [Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801?src=pr&el=h1) Report
> Merging [#1801](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801?src=pr&el=desc) (41da978) into [master](https://codecov.io/gh/scala-steward-org/scala-steward/commit/81e64ea62e44b7ff7cd343050b33142a1fcc84c6?el=desc) (81e64ea) will **decrease** coverage by `0.21%`.
> The diff coverage is `33.33%`.

[![Impacted file tree graph](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801/graphs/tree.svg?width=650&height=150&src=pr&token=udzMsc1Dne)](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1801      +/-   ##
==========================================
- Coverage   74.89%   74.67%   -0.22%     
==========================================
  Files         123      122       -1     
  Lines        2079     2085       +6     
  Branches       53       61       +8     
==========================================
  Hits         1557     1557              
- Misses        522      528       +6     
```


| [Impacted Files](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ala/org/scalasteward/core/nurture/NurtureAlg.scala](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801/diff?src=pr&el=tree#diff-bW9kdWxlcy9jb3JlL3NyYy9tYWluL3NjYWxhL29yZy9zY2FsYXN0ZXdhcmQvY29yZS9udXJ0dXJlL051cnR1cmVBbGcuc2NhbGE=) | `4.71% <0.00%> (-0.10%)` | :arrow_down: |
| [...ain/scala/org/scalasteward/core/edit/EditAlg.scala](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801/diff?src=pr&el=tree#diff-bW9kdWxlcy9jb3JlL3NyYy9tYWluL3NjYWxhL29yZy9zY2FsYXN0ZXdhcmQvY29yZS9lZGl0L0VkaXRBbGcuc2NhbGE=) | `57.69% <45.45%> (-13.74%)` | :arrow_down: |
| [...la-2.12/org/scalasteward/mill/plugin/package.scala](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801/diff?src=pr&el=tree#diff-bW9kdWxlcy9taWxsLXBsdWdpbi9zcmMvbWFpbi9zY2FsYS0yLjEyL29yZy9zY2FsYXN0ZXdhcmQvbWlsbC9wbHVnaW4vcGFja2FnZS5zY2FsYQ==) | | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801?src=pr&el=footer). Last update [81e64ea...41da978](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1801?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/build.sbt b/build.sbt
index a1d67e22..b4357a82 100644
--- a/build.sbt
+++ b/build.sbt
@@ -113,9 +113,6 @@ lazy val core = myCrossProject("core")
       {
         case PathList(ps @ _*) if nativeSuffix.findFirstMatchIn(ps.last).isDefined =>
           MergeStrategy.first
-        case PathList(ps @ _*) if ps.last == "io.netty.versions.properties" =>
-          // This is included in Netty JARs which are pulled in by http4s-async-http-client.
-          MergeStrategy.first
         case PathList("org", "fusesource", _*) =>
           // (core / assembly) deduplicate: different file contents found in the following:
           // https/repo1.maven.org/maven2/jline/jline/2.14.6/jline-2.14.6.jar:org/fusesource/hawtjni/runtime/Callback.class
@@ -156,13 +153,11 @@ lazy val core = myCrossProject("core")
       import _root_.io.chrisdavenport.log4cats.Logger
       import _root_.io.chrisdavenport.log4cats.slf4j.Slf4jLogger
       import org.http4s.client.Client
-      import org.http4s.client.asynchttpclient.AsyncHttpClient
       import scala.concurrent.ExecutionContext
 
       implicit val ioContextShift: ContextShift[IO] = IO.contextShift(ExecutionContext.global)
       implicit val ioTimer: Timer[IO] = IO.timer(ExecutionContext.global)
       implicit val logger: Logger[IO] = Slf4jLogger.getLogger[IO]
-      implicit val client: Client[IO] = AsyncHttpClient.allocate[IO]().map(_._1).unsafeRunSync()
     """,
     fork in run := true,
     fork in Test := true,
diff --git a/modules/core/src/main/scala/org/scalasteward/core/data/Version.scala b/modules/core/src/main/scala/org/scalasteward/core/data/Version.scala
index 32d8266d..24efe46f 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/data/Version.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/data/Version.scala
@@ -21,7 +21,6 @@ import cats.implicits._
 import eu.timepit.refined.types.numeric.NonNegInt
 import io.circe.Codec
 import io.circe.generic.extras.semiauto.deriveUnwrappedCodec
-import scala.annotation.tailrec
 
 final case class Version(value: String) {
   private val components: List[Version.Component] =
@@ -58,7 +57,7 @@ final case class Version(value: String) {
           // prevents updates from 2.1.4.0-RC17 to 2.1.4.0-RC17+1-307f2f6c-SNAPSHOT.
           ((minAlphaOrder < 0) && (v.minAlphaOrder < minAlphaOrder)) ||
           // Do not select versions that are identical up to the hashes.
-          v.alnumComponentsWithoutHash === alnumComponentsWithoutHash ||
+          v.alnumComponents === alnumComponents ||
           // Do not select a version with hash if this version contains no hash.
           (v.hashIndex.nonEmpty && hashIndex.isEmpty) ||
           // Don't select "versions" like %5BWARNING%5D.
@@ -79,19 +78,12 @@ final case class Version(value: String) {
     preReleaseIndex.isDefined
 
   private val hashIndex: Option[NonNegInt] =
-    """[-+]g?\p{XDigit}{6,}""".r
-      .findAllMatchIn(value)
-      .to(Iterable)
-      .find(m => !Version.startsWithDate(m.matched.drop(1)))
-      .flatMap(m => NonNegInt.unapply(m.start))
-
-  private val alnumComponentsWithoutHash: List[Version.Component] =
-    hashIndex
-      .map(i => alnumComponents.takeWhile(_.startIndex < i.value))
-      .getOrElse(alnumComponents)
+    components.collectFirst { case h: Version.Component.Hash =>
+      NonNegInt.unsafeFrom(h.startIndex)
+    }
 
   private[this] val preReleaseIndex: Option[NonNegInt] = {
-    val preReleaseIdentIndex = alnumComponentsWithoutHash.collectFirst {
+    val preReleaseIdentIndex = alnumComponents.collectFirst {
       case a: Version.Component.Alpha if a.isPreReleaseIdent => NonNegInt.unsafeFrom(a.startIndex)
     }
     preReleaseIdentIndex.orElse(hashIndex)
@@ -157,61 +149,34 @@ object Version {
           case _                               => 0
         }
     }
+    final case class Hash(value: String, startIndex: Int) extends Component
     final case class Separator(c: Char, startIndex: Int) extends Component
     case object Empty extends Component { override def startIndex: Int = -1 }
 
-    def parse(str: String): List[Component] = {
-      @tailrec
-      def loop(
-          rest: List[Char],
-          accN: List[Char],
-          accA: List[Char],
-          acc: List[Component],
-          index: Int
-      ): List[Component] =
-        rest match {
-          case h :: t =>
-            h match {
-              case '.' | '-' | '_' | '+' =>
-                val (newAcc, newIndex) = materialize(accN, accA, acc, index)
-                loop(t, List.empty, List.empty, Separator(h, newIndex) :: newAcc, newIndex + 1)
-
-              case _ if h.isDigit && accA.nonEmpty =>
-                val (newAcc, newIndex) = materialize(List.empty, accA, acc, index)
-                loop(t, h :: accN, List.empty, newAcc, newIndex)
-
-              case _ if h.isDigit && accA.isEmpty =>
-                loop(t, h :: accN, List.empty, acc, index)
-
-              case _ if accN.nonEmpty =>
-                val (newAcc, newIndex) = materialize(accN, List.empty, acc, index)
-                loop(t, List.empty, h :: accA, newAcc, newIndex)
-
-              case _ if accN.isEmpty =>
-                loop(t, List.empty, h :: accA, acc, index)
-            }
-          case Nil =>
-            val (newAcc, _) = materialize(accN, accA, acc, index)
-            newAcc
-        }
-
-      def materialize(
-          accN: List[Char],
-          accA: List[Char],
-          acc: List[Component],
-          index: Int
-      ): (List[Component], Int) =
-        if (accN.nonEmpty) (Numeric(accN.reverse.mkString, index) :: acc, index + accN.length)
-        else if (accA.nonEmpty) (Alpha(accA.reverse.mkString, index) :: acc, index + accA.length)
-        else (acc, index)
-
-      loop(str.toList, List.empty, List.empty, List.empty, 0).reverse
-    }
+    private val numeric = """^(\d+)(.*)$""".r
+    private val separator = """^([.\-_+])(.*)$""".r
+    private val alpha = """^([^.\-_+\d]+)(.*)$""".r
+    private val hash = """^([-+])(g?\p{XDigit}{6,})(.*)$""".r
+
+    def parse(str: String, index: Int = 0): List[Component] =
+      str match {
+        case "" => List.empty
+        case hash(sep, value, rest) if !startsWithDate(value) =>
+          Separator(sep.head, index) +: Hash(value, index + 1) +:
+            parse(rest, index + value.length + 1)
+        case numeric(value, rest) =>
+          Numeric(value, index) +: parse(rest, index + value.length)
+        case alpha(value, rest) =>
+          Alpha(value, index) +: parse(rest, index + value.length)
+        case separator(value, rest) =>
+          Separator(value.head, index) +: parse(rest, index + value.length)
+      }
 
     def render(components: List[Component]): String =
       components.map {
         case n: Numeric   => n.value
         case a: Alpha     => a.value
+        case h: Hash      => h.value
         case s: Separator => s.c.toString
         case Empty        => ""
       }.mkString
diff --git a/modules/core/src/main/scala/org/scalasteward/core/edit/EditAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/edit/EditAlg.scala
index ad2d7690..663b4ead 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/edit/EditAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/edit/EditAlg.scala
@@ -45,13 +45,15 @@ final class EditAlg[F[_]](implicit
   def applyUpdate(repo: Repo, repoConfig: RepoConfig, update: Update): F[List[Commit]] =
     findFilesContainingCurrentVersion(repo, repoConfig, update).flatMap {
       case None =>
-        logger.warn("No files found that contain the current version").as(List.empty[Commit])
+        logger.warn("No files found that contain the current version").as(Nil)
       case Some(files) =>
-        List(
-          runScalafixMigrations(repo, update),
-          bumpVersion(repo, repoConfig, update, files),
-          hookExecutor.execPostUpdateHooks(repo, repoConfig, update)
-        ).flatSequence
+        for {
+          cs1 <- runScalafixMigrations(repo, update)
+          cs2 <- bumpVersion(repo, repoConfig, update, files)
+          cs3 <-
+            if (cs2.isEmpty) F.pure(Nil)
+            else hookExecutor.execPostUpdateHooks(repo, repoConfig, update)
+        } yield cs1 ++ cs2 ++ cs3
     }
 
   private def findFilesContainingCurrentVersion(
@@ -72,7 +74,7 @@ final class EditAlg[F[_]](implicit
             val msg = s"Run Scalafix rule(s) ${migration.rewriteRules.mkString_(", ")}"
             gitAlg.commitAllIfDirty(repo, msg)
           case Left(throwable) =>
-            logger.error(throwable)("Scalafix migration failed").as(Option.empty[Commit])
+            logger.error(throwable)("Scalafix migration failed").as(None)
         }
     }
 
@@ -88,7 +90,7 @@ final class EditAlg[F[_]](implicit
     }
     bindUntilTrue(actions).ifM(
       gitAlg.commitAllIfDirty(repo, git.commitMsgFor(update, repoConfig.commits)).map(_.toList),
-      logger.warn("Unable to bump version").as(List.empty[Commit])
+      logger.warn("Unable to bump version").as(Nil)
     )
   }
 }
diff --git a/project/Dependencies.scala b/project/Dependencies.scala
index 6886d80b..8d67ec99 100644
--- a/project/Dependencies.scala
+++ b/project/Dependencies.scala
@@ -25,7 +25,7 @@ object Dependencies {
     .excludeAll(ExclusionRule().withOrganization("org.scalatest"))
   val fs2Core = "co.fs2" %% "fs2-core" % "2.4.6"
   val fs2Io = "co.fs2" %% "fs2-io" % fs2Core.revision
-  val http4sCore = "org.http4s" %% "http4s-core" % "0.21.13"
+  val http4sCore = "org.http4s" %% "http4s-core" % "0.21.14"
   val http4sCirce = "org.http4s" %% "http4s-circe" % http4sCore.revision
   val http4sClient = "org.http4s" %% "http4s-client" % http4sCore.revision
   val http4sDsl = "org.http4s" %% "http4s-dsl" % http4sCore.revision


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/TestInstances.scala b/modules/core/src/test/scala/org/scalasteward/core/TestInstances.scala
index 021f5fff..47f01743 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/TestInstances.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/TestInstances.scala
@@ -32,6 +32,14 @@ object TestInstances {
   implicit def scopeCogen[T](implicit cogenT: Cogen[T]): Cogen[Scope[T]] =
     cogenT.contramap(_.value)
 
+  private val hashGen: Gen[String] =
+    for {
+      sep <- Gen.oneOf('-', '+')
+      maybeG <- Gen.option(Gen.const('g'))
+      length <- Gen.choose(6, 8)
+      rest <- Gen.listOfN(length, Gen.hexChar)
+    } yield sep.toString + maybeG.getOrElse("") + rest.mkString
+
   implicit val versionArbitrary: Arbitrary[Version] = {
     val commonStrings =
       Gen.oneOf(
@@ -54,7 +62,8 @@ object TestInstances {
       (3, Gen.alphaChar.map(_.toString)),
       (2, Gen.const('-').map(_.toString)),
       (1, Gen.const('+').map(_.toString)),
-      (1, commonStrings)
+      (1, commonStrings),
+      (1, hashGen)
     )
     Arbitrary(Gen.listOf(versionComponent).map(_.mkString).map(Version.apply))
   }
diff --git a/modules/core/src/test/scala/org/scalasteward/core/data/VersionTest.scala b/modules/core/src/test/scala/org/scalasteward/core/data/VersionTest.scala
index 5a981cae..a8302019 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/data/VersionTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/data/VersionTest.scala
@@ -233,6 +233,10 @@ class VersionTest
     forAll { str: String => Component.render(Component.parse(str)) shouldBe str }
   }
 
+  test("Component: round-trip using Version") {
+    forAll { v: Version => Component.render(Component.parse(v.value)) shouldBe v.value }
+  }
+
   test("Component: round-trip example") {
     val original = "1.0.0-rc.1+build.1"
     Component.render(Component.Empty :: Component.parse(original)) shouldBe original
diff --git a/modules/core/src/test/scala/org/scalasteward/core/edit/EditAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/edit/EditAlgTest.scala
index bfc9850c..23847486 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/edit/EditAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/edit/EditAlgTest.scala
@@ -6,7 +6,6 @@ import org.scalasteward.core.data.{GroupId, Update}
 import org.scalasteward.core.mock.MockContext.{config, editAlg}
 import org.scalasteward.core.mock.MockState
 import org.scalasteward.core.repoconfig.RepoConfig
-import org.scalasteward.core.scalafmt.scalafmtBinary
 import org.scalasteward.core.util.Nel
 import org.scalasteward.core.vcs.data.Repo
 import org.scalatest.funsuite.AnyFunSuite
@@ -55,21 +54,15 @@ class EditAlgTest extends AnyFunSuite with Matchers {
     val repoDir = config.workspace / repo.show
     val update = Update.Single("org.scalameta" % "scalafmt-core" % "2.0.0", Nel.of("2.1.0"))
     val scalafmtConf = repoDir / ".scalafmt.conf"
+    val scalafmtConfContent = """maxColumn = 100
+                                |version = 2.0.0
+                                |align.openParenCallSite = false
+                                |""".stripMargin
     val buildSbt = repoDir / "build.sbt"
 
     val state = editAlg
       .applyUpdate(repo, RepoConfig.empty, update)
-      .runS(
-        MockState.empty
-          .add(
-            scalafmtConf,
-            """maxColumn = 100
-              |version = 2.0.0
-              |align.openParenCallSite = false
-              |""".stripMargin
-          )
-          .add(buildSbt, "")
-      )
+      .runS(MockState.empty.add(scalafmtConf, scalafmtConfContent).add(buildSbt, ""))
       .unsafeRunSync()
 
     state shouldBe MockState.empty.copy(
@@ -86,8 +79,6 @@ class EditAlgTest extends AnyFunSuite with Matchers {
         List("read", scalafmtConf.pathAsString),
         List("read", scalafmtConf.pathAsString),
         List("write", scalafmtConf.pathAsString),
-        envVars ++ (repoDir.toString :: gitStatus),
-        List("VAR1=val1", "VAR2=val2", repoDir.toString, scalafmtBinary, "--non-interactive"),
         envVars ++ (repoDir.toString :: gitStatus)
       ),
       logs = Vector(
@@ -98,8 +89,7 @@ class EditAlgTest extends AnyFunSuite with Matchers {
         (None, "Trying heuristic 'sliding'"),
         (None, "Trying heuristic 'completeGroupId'"),
         (None, "Trying heuristic 'groupId'"),
-        (None, "Trying heuristic 'specific'"),
-        (None, "Executing post-update hook for org.scalameta:scalafmt-core")
+        (None, "Trying heuristic 'specific'")
       ),
       files = Map(
         scalafmtConf ->