Introduce EditAttempt enum (#2242)

Author: fthomas

modules/core/src/main/scala/org/scalasteward/core/edit/EditAlg.scala

@@ -21,10 +21,11 @@ import cats.effect.Concurrent
 import cats.syntax.all._
 import org.scalasteward.core.buildtool.BuildToolDispatcher
 import org.scalasteward.core.data.{RepoData, Update}
+import org.scalasteward.core.edit.EditAttempt.{ScalafixEdit, UpdateEdit}
 import org.scalasteward.core.edit.hooks.HookExecutor
 import org.scalasteward.core.edit.scalafix.{ScalafixMigration, ScalafixMigrationsFinder}
 import org.scalasteward.core.git
-import org.scalasteward.core.git.{Commit, GitAlg}
+import org.scalasteward.core.git.GitAlg
 import org.scalasteward.core.io.{isSourceFile, FileAlg, WorkspaceAlg}
 import org.scalasteward.core.repoconfig.RepoConfig
 import org.scalasteward.core.util._
@@ -42,7 +43,11 @@ final class EditAlg[F[_]](implicit
     workspaceAlg: WorkspaceAlg[F],
     F: Concurrent[F]
 ) {
-  def applyUpdate(data: RepoData, update: Update, preCommit: F[Unit] = F.unit): F[List[Commit]] =
+  def applyUpdate(
+      data: RepoData,
+      update: Update,
+      preCommit: F[Unit] = F.unit
+  ): F[List[EditAttempt]] =
     findFilesContainingCurrentVersion(data.repo, data.config, update).flatMap {
       case None =>
         logger.warn("No files found that contain the current version").as(Nil)
@@ -54,15 +59,17 @@ final class EditAlg[F[_]](implicit
               _ <- preCommit
               repo = data.repo
               migrations = scalafixMigrationsFinder.findMigrations(update)
-              cs1 <-
+              scalafixEdits <-
                 if (migrations.isEmpty) F.pure(Nil)
                 else
                   gitAlg.discardChanges(repo) *>
                     runScalafixMigrations(repo, data.config, migrations) <*
                     bumpVersion(update, files)
-              cs2 <- gitAlg.commitAllIfDirty(repo, git.commitMsgFor(update, data.config.commits))
-              cs3 <- hookExecutor.execPostUpdateHooks(data, update)
-            } yield cs1 ++ cs2 ++ cs3
+              updateEdit <- gitAlg
+                .commitAllIfDirty(repo, git.commitMsgFor(update, data.config.commits))
+                .map(_.map(commit => UpdateEdit(update, commit)))
+              hooksEdits <- hookExecutor.execPostUpdateHooks(data, update)
+            } yield scalafixEdits ++ updateEdit ++ hooksEdits
         }
     }
 
@@ -80,14 +87,14 @@ final class EditAlg[F[_]](implicit
       repo: Repo,
       config: RepoConfig,
       migrations: List[ScalafixMigration]
-  ): F[List[Commit]] =
-    migrations.traverseFilter(runScalafixMigration(repo, config, _))
+  ): F[List[EditAttempt]] =
+    migrations.traverse(runScalafixMigration(repo, config, _))
 
   private def runScalafixMigration(
       repo: Repo,
       config: RepoConfig,
       migration: ScalafixMigration
-  ): F[Option[Commit]] =
+  ): F[EditAttempt] =
     for {
       _ <- logger.info(s"Running migration $migration")
       result <- logger.attemptLogWarn("Scalafix migration failed")(
@@ -97,7 +104,7 @@ final class EditAlg[F[_]](implicit
       msg1 = s"$verb Scalafix rule(s) ${migration.rewriteRules.mkString_(", ")}"
       msg2 = migration.doc.map(url => s"See $url for details").toList
       maybeCommit <- gitAlg.commitAllIfDirty(repo, msg1, msg2: _*)
-    } yield maybeCommit
+    } yield ScalafixEdit(migration, result, maybeCommit)
 
   private def bumpVersion(update: Update, files: Nel[File]): F[Boolean] = {
     val actions = UpdateHeuristic.all.map { heuristic =>

modules/core/src/main/scala/org/scalasteward/core/edit/EditAttempt.scala

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2018-2021 Scala Steward contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.scalasteward.core.edit
+
+import org.scalasteward.core.data.Update
+import org.scalasteward.core.edit.hooks.PostUpdateHook
+import org.scalasteward.core.edit.scalafix.ScalafixMigration
+import org.scalasteward.core.git.Commit
+
+sealed trait EditAttempt extends Product with Serializable {
+  def maybeCommit: Option[Commit]
+}
+
+object EditAttempt {
+  final case class UpdateEdit(update: Update, commit: Commit) extends EditAttempt {
+    override def maybeCommit: Some[Commit] = Some(commit)
+  }
+
+  final case class ScalafixEdit(
+      migration: ScalafixMigration,
+      result: Either[Throwable, Unit],
+      maybeCommit: Option[Commit]
+  ) extends EditAttempt
+
+  final case class HookEdit(
+      hook: PostUpdateHook,
+      result: Either[Throwable, Unit],
+      maybeCommit: Option[Commit]
+  ) extends EditAttempt
+}

modules/core/src/main/scala/org/scalasteward/core/edit/hooks/HookExecutor.scala

@@ -20,7 +20,9 @@ import cats.MonadThrow
 import cats.syntax.all._
 import org.scalasteward.core.buildtool.sbt.{sbtArtifactId, sbtGroupId}
 import org.scalasteward.core.data._
-import org.scalasteward.core.git.{Commit, GitAlg}
+import org.scalasteward.core.edit.EditAttempt
+import org.scalasteward.core.edit.EditAttempt.HookEdit
+import org.scalasteward.core.git.GitAlg
 import org.scalasteward.core.io.{ProcessAlg, WorkspaceAlg}
 import org.scalasteward.core.repocache.RepoCache
 import org.scalasteward.core.scalafmt.{scalafmtArtifactId, scalafmtBinary, scalafmtGroupId}
@@ -36,7 +38,7 @@ final class HookExecutor[F[_]](implicit
     workspaceAlg: WorkspaceAlg[F],
     F: MonadThrow[F]
 ) {
-  def execPostUpdateHooks(data: RepoData, update: Update): F[List[Commit]] =
+  def execPostUpdateHooks(data: RepoData, update: Update): F[List[EditAttempt]] =
     HookExecutor.postUpdateHooks
       .filter { hook =>
         update.groupId === hook.groupId &&
@@ -45,21 +47,17 @@ final class HookExecutor[F[_]](implicit
         hook.enabledByConfig(data.config)
       }
       .distinctBy(_.command)
-      .flatTraverse(execPostUpdateHook(data.repo, update, _))
+      .traverse(execPostUpdateHook(data.repo, update, _))
 
-  private def execPostUpdateHook(
-      repo: Repo,
-      update: Update,
-      hook: PostUpdateHook
-  ): F[List[Commit]] =
+  private def execPostUpdateHook(repo: Repo, update: Update, hook: PostUpdateHook): F[EditAttempt] =
     for {
       _ <- logger.info(s"Executing post-update hook for ${hook.groupId}:${hook.artifactId.name}")
       repoDir <- workspaceAlg.repoDir(repo)
-      _ <- logger.attemptLogWarn_("Post-update hook failed") {
+      result <- logger.attemptLogWarn("Post-update hook failed") {
         processAlg.execMaybeSandboxed(hook.useSandbox)(hook.command, repoDir)
       }
       maybeCommit <- gitAlg.commitAllIfDirty(repo, hook.commitMessage(update))
-    } yield maybeCommit.toList
+    } yield HookEdit(hook, result.void, maybeCommit)
 }
 
 object HookExecutor {

modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala

@@ -25,8 +25,7 @@ import org.scalasteward.core.application.Config
 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
-import org.scalasteward.core.edit.scalafix.ScalafixMigrationsFinder
+import org.scalasteward.core.edit.{EditAlg, EditAttempt}
 import org.scalasteward.core.git.{Branch, Commit, GitAlg}
 import org.scalasteward.core.repoconfig.PullRequestUpdateStrategy
 import org.scalasteward.core.util.UrlChecker
@@ -42,7 +41,6 @@ final class NurtureAlg[F[_]](config: Config)(implicit
     gitAlg: GitAlg[F],
     logger: Logger[F],
     pullRequestRepository: PullRequestRepository[F],
-    scalafixMigrationsFinder: ScalafixMigrationsFinder,
     vcsApiAlg: VCSApiAlg[F],
     vcsExtraAlg: VCSExtraAlg[F],
     vcsRepoAlg: VCSRepoAlg[F],
@@ -148,9 +146,10 @@ final class NurtureAlg[F[_]](config: Config)(implicit
     gitAlg.returnToCurrentBranch(data.repo) {
       val createBranch = logger.info(s"Create branch ${data.updateBranch.name}") >>
         gitAlg.createBranch(data.repo, data.updateBranch)
-      editAlg.applyUpdate(data.repoData, data.update, createBranch).flatMap { editCommits =>
+      editAlg.applyUpdate(data.repoData, data.update, createBranch).flatMap { edits =>
+        val editCommits = edits.flatMap(_.maybeCommit)
         if (editCommits.isEmpty) logger.warn("No commits created").as(Ignored)
-        else pushCommits(data, editCommits) >> createPullRequest(data)
+        else pushCommits(data, editCommits) >> createPullRequest(data, edits)
       }
     }
 
@@ -162,7 +161,7 @@ final class NurtureAlg[F[_]](config: Config)(implicit
         _ <- gitAlg.push(data.repo, data.updateBranch)
       } yield Updated
 
-  def createPullRequest(data: UpdateData): F[ProcessResult] =
+  def createPullRequest(data: UpdateData, edits: List[EditAttempt]): F[ProcessResult] =
     for {
       _ <- logger.info(s"Create PR ${data.updateBranch.name}")
       dependenciesWithNextVersion =
@@ -178,13 +177,12 @@ final class NurtureAlg[F[_]](config: Config)(implicit
           .traverse(vcsExtraAlg.getReleaseRelatedUrls(_, data.update))
       filesWithOldVersion <- gitAlg.findFilesContaining(data.repo, data.update.currentVersion)
       branchName = vcs.createBranch(config.vcsType, data.fork, data.update)
-      migrations = scalafixMigrationsFinder.findMigrations(data.update)
       requestData = NewPullRequestData.from(
         data,
         branchName,
+        edits,
         existingArtifactUrlsMap,
         releaseRelatedUrls.getOrElse(List.empty),
-        migrations,
         filesWithOldVersion
       )
       pr <- vcsApiAlg.createPullRequest(data.repo, requestData)
@@ -237,7 +235,8 @@ final class NurtureAlg[F[_]](config: Config)(implicit
         s"Merge branch ${data.baseBranch.name} into ${data.updateBranch.name} and apply again"
       )
       maybeMergeCommit <- gitAlg.mergeTheirs(data.repo, data.baseBranch)
-      editCommits <- editAlg.applyUpdate(data.repoData, data.update)
+      edits <- editAlg.applyUpdate(data.repoData, data.update)
+      editCommits = edits.flatMap(_.maybeCommit)
       result <- pushCommits(data, maybeMergeCommit.toList ++ editCommits)
     } yield result
 }

modules/core/src/main/scala/org/scalasteward/core/vcs/data/NewPullRequestData.scala

@@ -21,6 +21,8 @@ import io.circe.Encoder
 import io.circe.generic.semiauto._
 import org.http4s.Uri
 import org.scalasteward.core.data._
+import org.scalasteward.core.edit.EditAttempt
+import org.scalasteward.core.edit.EditAttempt.ScalafixEdit
 import org.scalasteward.core.edit.scalafix.ScalafixMigration
 import org.scalasteward.core.git
 import org.scalasteward.core.git.Branch
@@ -48,12 +50,13 @@ object NewPullRequestData {
 
   def bodyFor(
       update: Update,
+      edits: List[EditAttempt],
       artifactIdToUrl: Map[String, Uri],
       releaseRelatedUrls: List[ReleaseRelatedUrl],
-      migrations: List[ScalafixMigration],
       filesWithOldVersion: List[String]
   ): String = {
     val artifacts = artifactsWithOptionalUrl(update, artifactIdToUrl)
+    val migrations = edits.collect { case ScalafixEdit(migration, _, _) => migration }
     val (migrationLabel, appliedMigrations) = migrationNote(migrations)
     val (oldVersionLabel, oldVersionDetails) = oldVersionNote(filesWithOldVersion, update)
     val details = appliedMigrations.toList ++
@@ -195,20 +198,14 @@ object NewPullRequestData {
   def from(
       data: UpdateData,
       branchName: String,
+      edits: List[EditAttempt] = List.empty,
       artifactIdToUrl: Map[String, Uri] = Map.empty,
       releaseRelatedUrls: List[ReleaseRelatedUrl] = List.empty,
-      migrations: List[ScalafixMigration] = List.empty,
       filesWithOldVersion: List[String] = List.empty
   ): NewPullRequestData =
     NewPullRequestData(
       title = git.commitMsgFor(data.update, data.repoConfig.commits),
-      body = bodyFor(
-        data.update,
-        artifactIdToUrl,
-        releaseRelatedUrls,
-        migrations,
-        filesWithOldVersion
-      ),
+      body = bodyFor(data.update, edits, artifactIdToUrl, releaseRelatedUrls, filesWithOldVersion),
       head = branchName,
       base = data.baseBranch
     )