Run scalafmt in CI and in pre-commit hook

[emdash-ie]

What's changed?

Similar to the recent changes to run prettier in CI and in a pre-commit hook, this PR configures scalafmt to do the same.

More information in the individual commit messages, reproduced here:

bc94ddd Run scalafmt in CI

Now that this repo is using prettier to check formatting in CI, why not also check scala formatting? This commit adds a check in CI that runs scalafmt via an sbt plugin, copying the approach from https://github.com/guardian/datawrapper-import/pull/12

I’ll also add a pre-commit hook shortly, and format the existing code. I’ll probably choose a formatting style that uses tabs for indentation, to be consistent with the frontend code (and given the accessibility motivations for using tabs for indentation).

6cf11c6 Add .scalafmt.conf

Scalafmt needs this file in order to run! These are the minimal config values required.

Unfortunately, it looks like scalafmt doesn’t support using tabs for indentation: we’ll go with spaces for now but I’ll raise an issue with them requesting a config for using tabs. I reckon there are strong accessibility arguments in favour of tabs, so I’m hopeful I can convince them.

96296b5 Run scalafmt on everything

241199e Add formatting commit to .git-blame-ignore-revs

cf7464c Update pre-commit hook to check scala formatting

This commit updates the pre-commit hook to run scalafmt the same way it already runs prettier. Apart from the exit code tweak, this basically involved copying the pre-commit hook from https://github.com/guardian/datawrapper-import/pull/12

As with the prettier part of the hook, this should only run the formatter on files with staged changes that don’t have unstaged changes, and for any with both that fail formatting it should just report an error and exit.

(I’ve also replaced the word “clobbering” with “overwriting”, because I think it’s a less obscure phrasing.)

Implementation notes

I’ve used scalafmt directly for the pre-commit hook rather than via the sbt plugin because sbt startup is quite slow, and it can be quite annoying to run it on every commit. Running scalafmt directly is a good bit faster.

Checklist

General

• 🤖 Relevant tests added
• ✅ CI checks / tests run locally
• 🔍 Checked on CODE

Client

• 🚫 No obvious console errors on the client (i.e. React dev mode errors)
• 🎛️ No regressions with existing user interactions (i.e. all existing buttons, inputs etc. work)
• 📷 Screenshots / GIFs of relevant UI changes included

[github-actions]

Deploy build 8619 of cms-fronts::facia-tool to CODE

All deployment options

• Deploy build 8619 of cms-fronts::facia-tool to CODE
• Deploy parts of build 8619 to CODE by previewing it first
• What's on CODE right now?

---

From guardian/actions-riff-raff.

[emdash-ie]

Here’re the results of testing the updated pre-commit hook:

git status

emily_bourke@31753_mac ~/c/facia-tool (run-scalafmt-in-CI)> git status
On branch run-scalafmt-in-CI
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
	modified:   app/config/Filters.scala
	modified:   app/util/Maps.scala
	modified:   app/util/Seqs.scala

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   app/Loader.scala
	modified:   app/util/Seqs.scala
	modified:   app/util/UserUtil.scala

• Maps.scala has staged changes which need formatting
• Seqs.scala has staged changes which need formatting and unstaged changes which need formatting
• Filters.scala has staged changes which don’t need formatting
• Loader.scala has unstaged changes which need formatting
• UserUtil.scala has unstaged changes which don’t need formatting

git diff (unstaged changes)

emily_bourke@31753_mac ~/c/facia-tool (run-scalafmt-in-CI)> git diff
diff --git a/app/Loader.scala b/app/Loader.scala
index 1e43d1b92b..8ae8785810 100644
--- a/app/Loader.scala
+++ b/app/Loader.scala
@@ -13,7 +13,7 @@ import conf.ApplicationConfiguration
 import scala.concurrent.ExecutionContext.Implicits.global
 import scala.concurrent.duration._
 
-class Loader extends ApplicationLoader {
+class   Loader extends ApplicationLoader {
   override def load(context: Context): Application = {
     LoggerConfigurator(context.environment.classLoader).foreach {
       _.configure(context.environment)
diff --git a/app/util/Seqs.scala b/app/util/Seqs.scala
index 77553a5448..ae551bd82c 100644
--- a/app/util/Seqs.scala
+++ b/app/util/Seqs.scala
@@ -1,7 +1,7 @@
 package util
 
 object Seqs {
-  implicit   class RichSeq[A](as: Seq[A]) {
+  implicit   class   RichSeq[A](as: Seq[A]) {
     def isDescending(implicit ordering: Ordering[A]) = as == reverseSorted
 
     def reverseSorted(implicit ordering: Ordering[A]) =
diff --git a/app/util/UserUtil.scala b/app/util/UserUtil.scala
index f4be17edd7..5fedb68436 100644
--- a/app/util/UserUtil.scala
+++ b/app/util/UserUtil.scala
@@ -2,6 +2,6 @@ package util
 
 import com.gu.pandomainauth.model.User
 
-object UserUtil {
+object UserUtil { // comment
   def getDisplayName(user: User) = s"${user.firstName} ${user.lastName}"
 }

git diff --staged (staged changes)

emily_bourke@31753_mac ~/c/facia-tool (run-scalafmt-in-CI)> git diff --staged
diff --git a/app/config/Filters.scala b/app/config/Filters.scala
index 77e93164e8..4b36bbe518 100644
--- a/app/config/Filters.scala
+++ b/app/config/Filters.scala
@@ -6,7 +6,7 @@ import play.filters.gzip.GzipFilter
 
 class CustomGzipFilter(implicit val materializer: Materializer)
     extends GzipFilter(shouldGzip =
-      (_, response) => !Responses.isImage(response.header)
+      (_, response) => !Responses.isImage(response.header) // comment
     )(materializer) {}
 
 object Responses {
diff --git a/app/util/Maps.scala b/app/util/Maps.scala
index 0544a27718..940537e6e8 100644
--- a/app/util/Maps.scala
+++ b/app/util/Maps.scala
@@ -1,8 +1,7 @@
 package util
 
 object Maps {
-
   /** Insert k -> v into map, resolving collisions with f */
-  def insertWith[A, B](map: Map[A, B], k: A, v: B)(f: (B, B) => B) =
+    def insertWith[A, B](map: Map[A, B], k: A, v: B)(f: (B, B) => B) =
     map + (k -> map.get(k).map(f.curried(v)).getOrElse(v))
 }
diff --git a/app/util/Seqs.scala b/app/util/Seqs.scala
index 8c065374a3..77553a5448 100644
--- a/app/util/Seqs.scala
+++ b/app/util/Seqs.scala
@@ -1,7 +1,7 @@
 package util
 
 object Seqs {
-  implicit class RichSeq[A](as: Seq[A]) {
+  implicit   class RichSeq[A](as: Seq[A]) {
     def isDescending(implicit ordering: Ordering[A]) = as == reverseSorted
 
     def reverseSorted(implicit ordering: Ordering[A]) =

Hook output

emily_bourke@31753_mac ~/c/facia-tool (run-scalafmt-in-CI)> git commit
Checking file formatting with prettier…

All good
Checking file formatting with scalafmt…

The following staged files needed formatting by scalafmt! Formatting has been
applied but the files have not been re-staged: please verify the changes, stage,
and commit again.

app/util/Maps.scala

The following staged files with additional unstaged changes were reported by
scalafmt as needing formatting: these files have not been formatted, to avoid
clobbering unstaged changes. Please format them yourself or bypass this hook with
--no-verify.

app/util/Seqs.scala