Merge pull request #241 from Dwolla/scalafixes

Narrow Scalafix selectors so they only operate on our code

README.md

@@ -7,28 +7,32 @@ A library for encrypting and decrypting fs2 `Stream[F, Byte]` using PGP.
 <tr>
 <th>Artifact</th>
 <th>Description</th>
-<th align="center">Cats Effect Version</th>
-<th align="center">fs2 Version</th>
 <th align="center">Scala 2.12</th>
 <th align="center">Scala 2.13</th>
+<th align="center">Scala 3</th>
 </tr>
 </thead>
 <tbody>
 <tr>
 <td><code>"fs2-pgp"</code></td>
 <td>Load PGP keys and use them to encrypt, decrypt, and armor byte streams.</td>
-<td align="center">Cats Effect 3</td>
-<td align="center">fs2 3.x</td>
+<td align="center"><g-emoji class="g-emoji" alias="white_check_mark" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/2705.png">✅</g-emoji></td>
 <td align="center"><g-emoji class="g-emoji" alias="white_check_mark" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/2705.png">✅</g-emoji></td>
 <td align="center"><g-emoji class="g-emoji" alias="white_check_mark" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/2705.png">✅</g-emoji></td>
 </tr>
 <tr>
 <td><code>"pgp-testkit"</code></td>
 <td>ScalaCheck Generators and Arbitrary instances for PGP classes</td>
-<td align="center">Cats Effect 3</td>
-<td align="center">fs2 3.x</td>
 <td align="center"><g-emoji class="g-emoji" alias="white_check_mark" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/2705.png">✅</g-emoji></td>
 <td align="center"><g-emoji class="g-emoji" alias="white_check_mark" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/2705.png">✅</g-emoji></td>
+<td align="center"><g-emoji class="g-emoji" alias="white_check_mark" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/2705.png">✅</g-emoji></td>
+</tr>
+<tr>
+<td><code>"fs2-pgp-scalafix"</code></td>
+<td>Scalafix rewrite rules to help update from `v0.4` to `v0.5`</td>
+<td align="center"><g-emoji class="g-emoji" alias="white_check_mark" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/2705.png">✅</g-emoji></td>
+<td align="center"><g-emoji class="g-emoji" alias="white_check_mark" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/2705.png">✅</g-emoji></td>
+<td align="center"><g-emoji class="g-emoji" alias="x" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/274c.png">❌</g-emoji></td>
 </tr>
 </tbody>
 </table>
@@ -124,3 +128,34 @@ hQEMAzY5h81qQKpXAQf/YTq6GtTkWlbg2DRu7r133FZaAudA149WB2BV/vsgyHkN
 ## Decryption
 
 Read a `PGPPrivateKey` using `PGPKeyAlg[F]`, then pipe the encrypted message bytes through `CryptoAlg[F].decrypt`. 
+
+## Scalafix Rule
+
+Add Scalafix to your project's build by [following the instructions](https://scalacenter.github.io/scalafix/docs/users/installation.html#sbt):
+
+1. Add the Scalafix plugin to the project by adding this to `project/plugins.sbt`:
+
+    ```scala
+    addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.11.1")
+    ```
+
+2. Enable SemanticDB by adding this to `build.sbt`:
+
+    ```scala
+    ThisBuild / semanticdbEnabled := true
+    ThisBuild / semanticdbVersion := scalafixSemanticdb.revision
+    ThisBuild / scalafixScalaBinaryVersion := CrossVersion.binaryScalaVersion(scalaVersion.value)
+    ThisBuild / scalafixDependencies += "com.dwolla" %% "fs2-pgp-scalafix" % "0.5.0"
+    ```
+
+3. Make sure everything compiles, and run the Scalafix rule on the sbt console:
+
+    ```
+    Test/compile
+    Test/scalafix com.dwolla.security.crypto.V04to05
+    scalafix com.dwolla.security.crypto.V04to05
+    ```
+
+   (Run the Scalafix rule on the leafs of your project graph, and then work back to the middle. Tests should be updated before production code, because if the production code is updated, the test project won't compile anymore, etc.)
+
+4. Update the fs2-pgp version from `v0.4` to `v0.5`. (At this point, you can remove the Scalafix and SemanticDB keys from your build if you want.)

scalafix/input/src/main/scala/example/SeveralDifferentUses.scala

@@ -18,6 +18,7 @@ object SeveralDifferentUses {
     // all arguments set
     alg.encrypt(key, chunkSize, Option("filename"), Encryption.Aes256, Compression.Bzip2, PgpLiteralDataPacketFormat.Utf8)
     alg.encrypt(key, tagChunkSize(42), Option("filename"), Encryption.Aes256, Compression.Bzip2, PgpLiteralDataPacketFormat.Utf8)
+    alg.encrypt(new PGPPublicKey(null, null), tagChunkSize(42), Option("filename"), Encryption.Aes256, Compression.Bzip2, PgpLiteralDataPacketFormat.Utf8)
 
     // all arguments set with named parameters
     alg.encrypt(key = key, chunkSize = chunkSize, fileName = Option("filename"), encryption = Encryption.Aes256, compression = Compression.Bzip2, packetFormat = PgpLiteralDataPacketFormat.Utf8)

scalafix/input/src/main/scala/example/ShouldBeIgnored.scala

@@ -0,0 +1,27 @@
+package example
+/*
+rule = com.dwolla.security.crypto.V04to05
+*/
+trait Foo {
+  def encrypt(key: String, request: Int): Unit
+  def armor(): Unit
+  def tagChunkSize(i: Int): Unit
+}
+
+class ShouldBeIgnored(foo: Foo) {
+  import foo.*
+  import Nested.*
+
+  foo.armor()
+  foo.encrypt("key", 42)
+  foo.encrypt(key = "key", request = 42)
+  tagChunkSize(42)
+
+  CryptoAlg[Option]
+}
+
+object Nested {
+  object CryptoAlg {
+    def apply[F[_]]: Unit = ???
+  }
+}

scalafix/output/src/main/scala/example/SeveralDifferentUses.scala

@@ -16,6 +16,7 @@ object SeveralDifferentUses {
     // all arguments set
     alg.encrypt(EncryptionConfig().withChunkSize(chunkSize).withFileName(Option("filename")).withEncryption(Encryption.Aes256).withCompression(Compression.Bzip2).withPacketFormat(PgpLiteralDataPacketFormat.Utf8), key)
     alg.encrypt(EncryptionConfig().withChunkSize(ChunkSize(42)).withFileName(Option("filename")).withEncryption(Encryption.Aes256).withCompression(Compression.Bzip2).withPacketFormat(PgpLiteralDataPacketFormat.Utf8), key)
+    alg.encrypt(EncryptionConfig().withChunkSize(ChunkSize(42)).withFileName(Option("filename")).withEncryption(Encryption.Aes256).withCompression(Compression.Bzip2).withPacketFormat(PgpLiteralDataPacketFormat.Utf8), new PGPPublicKey(null, null))
 
     // all arguments set with named parameters
     alg.encrypt(EncryptionConfig().withChunkSize(chunkSize).withFileName(Option("filename")).withEncryption(Encryption.Aes256).withCompression(Compression.Bzip2).withPacketFormat(PgpLiteralDataPacketFormat.Utf8), key)

scalafix/output/src/main/scala/example/ShouldBeIgnored.scala

@@ -0,0 +1,25 @@
+package example
+
+trait Foo {
+  def encrypt(key: String, request: Int): Unit
+  def armor(): Unit
+  def tagChunkSize(i: Int): Unit
+}
+
+class ShouldBeIgnored(foo: Foo) {
+  import foo.*
+  import Nested.*
+
+  foo.armor()
+  foo.encrypt("key", 42)
+  foo.encrypt(key = "key", request = 42)
+  tagChunkSize(42)
+
+  CryptoAlg[Option]
+}
+
+object Nested {
+  object CryptoAlg {
+    def apply[F[_]]: Unit = ???
+  }
+}

scalafix/rules/src/main/scala/com/dwolla/security/crypto/V04to05.scala

@@ -4,20 +4,47 @@ import scalafix.v1.*
 
 import scala.annotation.tailrec
 import scala.meta.*
+import scala.meta.inputs.Input.{File, VirtualFile}
 
 class V04to05 extends SemanticRule("com.dwolla.security.crypto.V04to05") {
 
   override def fix(implicit doc: SemanticDocument): Patch =
     doc.tree.collect {
-      case t@Term.ApplyType.After_4_6_0(Term.Name("CryptoAlg"), Type.ArgClause(List(Type.Name(name)))) =>
+      /*
+       * `Crypto[F]` -> `Crypto.resource[F]`
+       */
+      case t@Term.ApplyType.After_4_6_0(Term.Name("CryptoAlg"), Type.ArgClause(List(Type.Name(name)))) if t.symbol.normalized.value == "com.dwolla.security.crypto.CryptoAlg." =>
         Patch.replaceTree(t, s"CryptoAlg.resource[$name]").atomic
-      case t@Term.Apply.After_4_6_0(Term.Select(Term.Name(name), Term.Name("armor")), Term.ArgClause(List(), None)) =>
+
+      /*
+       * `cryptoAlg.armor()` -> `cryptoAlg.armor`
+       */
+      case t@Term.Apply.After_4_6_0(Term.Select(Term.Name(name), Term.Name("armor")), Term.ArgClause(List(), None)) if t.symbol.normalized.value == "com.dwolla.security.crypto.CryptoAlg.armor." =>
         Patch.replaceTree(t, s"$name.armor").atomic
-      case Term.Apply.After_4_6_0(Term.Select(Term.Name(_), fun@Term.Name("encrypt")), t@Term.ArgClause((keyName@Term.Name(_)) :: additionalArguments, None)) =>
-        migrateEncrypt(t, fun, additionalArguments, Some(keyName), offset = 1)
-      case Term.Apply.After_4_6_0(Term.Select(Term.Name(_), fun@Term.Name("encrypt")), t@Term.ArgClause(arguments, None)) =>
-        migrateEncrypt(t, fun, arguments, None, offset = 0)
-      case t@Term.Apply.After_4_6_0(Term.Name("tagChunkSize"), _) if !isEncryptAParent(t) =>
+
+      /*
+       * Rewrite the various named and optional parameters of the `encrypt` method to use
+       * the `EncryptionConfig` object.
+       */
+      case t@Term.Apply.After_4_6_0(Term.Select(Term.Name(_), fun@Term.Name("encrypt")), argClause@Term.ArgClause(_, _)) if t.symbol.normalized.value == "com.dwolla.security.crypto.CryptoAlg.encrypt." =>
+        try {
+          argClause match {
+            case Term.ArgClause(arguments@Term.Assign(_, _) :: _, None) => // encrypt(…) where all arguments are named parameters
+              migrateEncrypt(argClause, fun, arguments, None, offset = 0)
+            case Term.ArgClause(key :: additionalArguments, None) => // encrypt(key, …), i.e. where the unnamed first argument is the key
+              migrateEncrypt(argClause, fun, additionalArguments, Some(key), offset = 1)
+            case other =>
+              throw pathedException(other)(s => new RuntimeException(s"Unexpected argument clause for encrypt at $s"))
+          }
+        } catch {
+          case ex: NoSuchElementException if ex.getMessage == "key not found: key" =>
+            throw pathedException(fun)(s => new RuntimeException(s"Key not found at $s"))
+        }
+
+      /*
+       * `tagChunkSize(foo)` -> `ChunkSize(foo)`
+       */
+      case t@Term.Apply.After_4_6_0(Term.Name("tagChunkSize"), _) if !isEncryptAParent(t) && t.symbol.normalized.value == "com.dwolla.security.crypto.package.tagChunkSize." =>
         Patch.replaceToken(t.tokens.head, "ChunkSize")
     }.asPatch
 
@@ -32,11 +59,11 @@ class V04to05 extends SemanticRule("com.dwolla.security.crypto.V04to05") {
   private def migrateEncrypt(t: Term.ArgClause,
                              fun: Term.Name,
                              arguments: List[Term],
-                             keyName: Option[Term],
+                             keyValue: Option[Term],
                              offset: Int,
                             )
                             (implicit doc: SemanticDocument): Patch = {
-    val map = arguments.zipWithIndex.foldLeft(keyName.map("key" -> _).toList) {
+    val map = arguments.zipWithIndex.foldLeft(keyValue.map("key" -> _).toList) {
       case (s, (Term.Assign(Term.Name("key"), term), _)) => s :+ ("key" -> term)
       case (s, (Term.Assign(Term.Name("chunkSize"), Term.Apply.After_4_6_0(Term.Name("tagChunkSize"), Term.ArgClause(List(term), _))), _)) =>
         s :+ ("ChunkSize" -> term)
@@ -50,7 +77,7 @@ class V04to05 extends SemanticRule("com.dwolla.security.crypto.V04to05") {
                 val parameterName = parameter.displayName
 
                 s :+ (parameterName.capitalize -> value)
-              case _ => throw new RuntimeException(s"Unexpected method signature ${info.signature} (how did the code being modified ever compile?)")
+              case _ => throw pathedException(fun)(s => new RuntimeException(s"Unexpected signature ${info.signature} at $s"))
             }
           case _ => s
         }
@@ -102,4 +129,15 @@ class V04to05 extends SemanticRule("com.dwolla.security.crypto.V04to05") {
 
     remainder :+ ("key" -> key)
   }
+
+  private def pathedException(tree: Tree)
+                             (f: String => Exception): Exception =
+    tree.pos.input match {
+      case File(path, _) =>
+        f(s"$path:${tree.pos.startLine + 1}:${tree.pos.startColumn + 1}–${tree.pos.endLine + 1}:${tree.pos.endColumn + 1}")
+      case VirtualFile(path, _) =>
+        f(s"$path:${tree.pos.startLine + 1}:${tree.pos.startColumn + 1}–${tree.pos.endLine + 1}:${tree.pos.endColumn + 1}")
+      case _ =>
+        f(tree.pos.toString)
+    }
 }