Refactor false positives fix (#183)

* small README fix * Refactor false positives fix with global * fix one comment
VirtusLab · Jul 7, 2022 · 5ee9abd · 5ee9abd
1 parent 657f29d
commit 5ee9abd
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 106 deletions.
diff --git a/README.md b/README.md
@@ -299,7 +299,7 @@ class ExampleSerializer(actorSystem: ExtendedActorSystem)
     // ...
     override lazy val codecs = Seq(Register[CommandOne]) // WHOOPS someone forgot to register CommandTwo...
     // ... but Codec Registration Checker will throw a compilation error here:
-    // `No codec for `CommandOne` is registered in class annotated with @org.virtuslab.ash.annotation.Serializer`
+    // `No codec for `CommandOne` is registered in a class annotated with @org.virtuslab.ash.annotation.Serializer`
 }
 ```
 

diff --git a/build.sbt b/build.sbt
@@ -56,8 +56,9 @@ lazy val commonSettings = Seq(
       if (sys.env.getOrElse("CI", "false") == "true") "-Xfatal-warnings" else ""),
   libraryDependencies ++= commonDeps)
 
-// As usage of https://github.com/pathikrit/better-files has been added to the runtime logic of codec-registration-checker-compiler-plugin
-// and dump-persistence-schema-compiler-plugin - this dependency has to be provided within a fat jar when ASH gets published.
+// As usage of https://github.com/pathikrit/better-files and https://github.com/spray/spray-json
+// has been added to the runtime logic of dump-persistence-schema-compiler-plugin -
+// this dependencies have to be provided within a fat jar when ASH gets published.
 // For reasons described in https://github.com/sbt/sbt/issues/2255 - without using fat-jar we would have java.lang.NoClassDefFoundErrors
 lazy val assemblySettings = Seq(
   packageBin / publishArtifact := false, //we want to publish fat jar
@@ -150,7 +151,6 @@ lazy val serializabilityCheckerCompilerPlugin = (projectMatrix in file("serializ
   .jvmPlatform(scalaVersions = supportedScalaVersions)
 
 lazy val codecRegistrationCheckerCompilerPlugin = (projectMatrix in file("codec-registration-checker-compiler-plugin"))
-  .enablePlugins(AssemblyPlugin)
   .settings(name := "codec-registration-checker-compiler-plugin")
   .settings(commonSettings)
   .settings(
@@ -163,8 +163,7 @@ lazy val codecRegistrationCheckerCompilerPlugin = (projectMatrix in file("codec-
         }
         .getOrElse(Seq.empty)
     },
-    libraryDependencies += betterFiles)
-  .settings(assemblySettings: _*)
+    libraryDependencies += betterFiles % Test)
   .dependsOn(annotation, circeAkkaSerializer % Test)
   .jvmPlatform(scalaVersions = supportedScalaVersions)
 

diff --git a/...n-checker-compiler-plugin/src/main/scala/org/virtuslab/ash/CacheFileInteractionMode.scala b/...n-checker-compiler-plugin/src/main/scala/org/virtuslab/ash/CacheFileInteractionMode.scala
@@ -0,0 +1,6 @@
+package org.virtuslab.ash
+
+sealed trait CacheFileInteractionMode
+
+case object DumpTypesIntoFile extends CacheFileInteractionMode
+case object RemoveOutdatedTypesFromFile extends CacheFileInteractionMode
diff --git a/...iler-plugin/src/main/scala/org/virtuslab/ash/CodecRegistrationCheckerCompilerPlugin.scala b/...iler-plugin/src/main/scala/org/virtuslab/ash/CodecRegistrationCheckerCompilerPlugin.scala
@@ -13,7 +13,6 @@ import scala.tools.nsc.plugins.PluginComponent
 
 import org.virtuslab.ash.CodecRegistrationCheckerCompilerPlugin.directClassDescendantsCacheFileName
 import org.virtuslab.ash.CodecRegistrationCheckerCompilerPlugin.disableFlag
-import org.virtuslab.ash.CodecRegistrationCheckerCompilerPlugin.sourceCodeDirectoryFlag
 
 class CodecRegistrationCheckerCompilerPlugin(override val global: Global) extends Plugin {
   override val name: String = "codec-registration-checker-plugin"
@@ -29,15 +28,6 @@ class CodecRegistrationCheckerCompilerPlugin(override val global: Global) extend
     if (options.contains(disableFlag))
       return false
 
-    options.find(flag => flag.contains(sourceCodeDirectoryFlag)) match {
-      case Some(directoryFlag) =>
-        pluginOptions.sourceCodeDirectoryToCheck = directoryFlag.replace(s"$sourceCodeDirectoryFlag=", "")
-      case None =>
-        error(
-          s"Required $sourceCodeDirectoryFlag option has not been set. Please, specify the $sourceCodeDirectoryFlag and retry compilation")
-        return false
-    }
-
     options.filterNot(_.startsWith("-")).headOption match {
       case Some(path) =>
         try {
@@ -77,7 +67,6 @@ class CodecRegistrationCheckerCompilerPlugin(override val global: Global) extend
   override val optionsHelp: Option[String] = Some(s"""
       |. - directory where cache file will be saved, required
       |$disableFlag - disables the plugin
-      |$sourceCodeDirectoryFlag - path of the source code directory, which has to be checked with this plugin
       |""".stripMargin)
 
 }
@@ -90,7 +79,6 @@ object CodecRegistrationCheckerCompilerPlugin {
   val serializerType = "org.virtuslab.ash.annotation.Serializer"
 
   val disableFlag = "--disable"
-  val sourceCodeDirectoryFlag = "--source-code-directory"
 
   def parseCacheFile(buffer: ByteBuffer): Seq[ParentChildFQCNPair] = {
     StandardCharsets.UTF_8.decode(buffer).toString.split("\n").toSeq.filterNot(_.isBlank).map(_.split(",")).map {

diff --git a/...er-compiler-plugin/src/main/scala/org/virtuslab/ash/CodecRegistrationCheckerOptions.scala b/...er-compiler-plugin/src/main/scala/org/virtuslab/ash/CodecRegistrationCheckerOptions.scala
@@ -21,15 +21,9 @@ import java.io.File
  *   (when `sbt compile` was incremental). And if we can't detect it - this would lead to runtime errors (see README
  *   for more details). `oldParentChildFQCNPairs` gets declared on the plugin's init
  *   by invoking the `CodecRegistrationCheckerCompilerPlugin.parseCacheFile` method.
- *
- * @param sourceCodeDirectoryToCheck - path of the source code directory that hold files with traits and classes
- *   checked by this plugin (i.e. types that are saved into `directClassDescendantsCacheFile`). This parameter is
- *   needed to fix possible false-positives in certain situations.
- *   Check https://github.com/VirtusLab/akka-serialization-helper/issues/141 for details about such false-positives.
  */
 case class CodecRegistrationCheckerOptions(
     var directClassDescendantsCacheFile: File = null,
-    var oldParentChildFQCNPairs: Seq[ParentChildFQCNPair] = null,
-    var sourceCodeDirectoryToCheck: String = null)
+    var oldParentChildFQCNPairs: Seq[ParentChildFQCNPair] = null)
 
 case class ParentChildFQCNPair(parentFQCN: String, childFQCN: String)
diff --git a/...iler-plugin/src/main/scala/org/virtuslab/ash/SerializerCheckCompilerPluginComponent.scala b/...iler-plugin/src/main/scala/org/virtuslab/ash/SerializerCheckCompilerPluginComponent.scala
@@ -13,8 +13,6 @@ import scala.tools.nsc.Global
 import scala.tools.nsc.Phase
 import scala.tools.nsc.plugins.PluginComponent
 
-import better.files._
-
 import org.virtuslab.ash.CodecRegistrationCheckerCompilerPlugin.classSweepPhaseName
 import org.virtuslab.ash.CodecRegistrationCheckerCompilerPlugin.serializabilityTraitType
 import org.virtuslab.ash.CodecRegistrationCheckerCompilerPlugin.serializerCheckPhaseName
@@ -56,7 +54,7 @@ class SerializerCheckCompilerPluginComponent(
         - it should be invoked only once - on the first `apply` call. That's why we use `typesNotDumped` flag.
          */
         if (typesNotDumped) {
-          dumpTypesIntoCacheFile()
+          interactWithTheCacheFile(DumpTypesIntoFile)
         }
 
         unit.body
@@ -158,7 +156,7 @@ class SerializerCheckCompilerPluginComponent(
 
         if (possibleMissingFullyQualifiedClassNames.nonEmpty) {
           // Due to the way how incremental compilation works - `possibleMissingFullyQualifiedClassNames` could contain
-          // "false-positives" - that's why a check against the source code in `collectMissingClassNames` is needed.
+          // "false-positives" - that's why additional check in `collectMissingClassNames` is needed.
           val actuallyMissingFullyQualifiedClassNames = collectMissingClassNames(
             possibleMissingFullyQualifiedClassNames)
           if (actuallyMissingFullyQualifiedClassNames.nonEmpty) {
@@ -169,7 +167,7 @@ class SerializerCheckCompilerPluginComponent(
                  |This will lead to a missing codec for Akka serialization in the runtime.
                  |Current type regex pattern: $typeRegexPattern""".stripMargin)
           } else {
-            removeOutdatedTypesFromCacheFile(possibleMissingFullyQualifiedClassNames)
+            interactWithTheCacheFile(RemoveOutdatedTypesFromFile, possibleMissingFullyQualifiedClassNames)
           }
         }
       }
@@ -198,24 +196,17 @@ class SerializerCheckCompilerPluginComponent(
       }
 
       private def collectMissingClassNames(fullyQualifiedClassNames: List[String]): List[String] = {
-        val sourceCodeDir = File(options.sourceCodeDirectoryToCheck)
-        val sourceCodeFilesAsStrings =
-          (for (file <- sourceCodeDir.collectChildren(_.name.endsWith(".scala"))) yield file.contentAsString).toList
-
-        def typeIsDefinedInScalaFiles(fqcn: String): Boolean = {
-          val indexOfLastDotInFQCN = fqcn.lastIndexOf('.')
-          val typeName = fqcn.substring(indexOfLastDotInFQCN + 1)
-          sourceCodeFilesAsStrings.exists(fileAsString => {
-            fileAsString.contains(s"class $typeName") ||
-            fileAsString.contains(s"trait $typeName") ||
-            fileAsString.contains(s"object $typeName")
-          })
-        }
-
-        fullyQualifiedClassNames.filter(typeIsDefinedInScalaFiles)
+        def typeIsDefinedInSourceCode(fqcn: String): Boolean =
+          global.findMemberFromRoot(TypeName(fqcn)) match {
+            case NoSymbol => false
+            case _        => true
+          }
+        fullyQualifiedClassNames.filter(typeIsDefinedInSourceCode)
       }
 
-      private def dumpTypesIntoCacheFile(): Unit = {
+      private def interactWithTheCacheFile(
+          mode: CacheFileInteractionMode,
+          typeNamesToRemove: List[String] = List.empty): Unit = {
         val raf = new RandomAccessFile(options.directClassDescendantsCacheFile, "rw")
         try {
           val channel = raf.getChannel
@@ -225,16 +216,25 @@ class SerializerCheckCompilerPluginComponent(
             channel.read(buffer)
             val parentChildFQCNPairsFromCacheFile =
               CodecRegistrationCheckerCompilerPlugin.parseCacheFile(buffer.rewind()).toSet
-            val outParentChildFQCNPairs =
-              ((parentChildFQCNPairsFromCacheFile -- classSweepFQCNPairsToUpdate) |
-              classSweepFoundFQCNPairs).toList
+
+            var outParentChildFQCNPairs: List[ParentChildFQCNPair] = List.empty // had to use var not to repeat too much
+            mode match {
+              case DumpTypesIntoFile =>
+                outParentChildFQCNPairs = ((parentChildFQCNPairsFromCacheFile -- classSweepFQCNPairsToUpdate) |
+                classSweepFoundFQCNPairs).toList
+                typeNamesToCheck ++= outParentChildFQCNPairs.groupBy(_.parentFQCN)
+                typesNotDumped = false
+              case RemoveOutdatedTypesFromFile =>
+                outParentChildFQCNPairs = parentChildFQCNPairsFromCacheFile
+                  .filterNot(pair =>
+                    typeNamesToRemove.contains(pair.parentFQCN) || typeNamesToRemove.contains(pair.childFQCN))
+                  .toList
+            }
+
             val outData: String =
               outParentChildFQCNPairs.map(pair => pair.parentFQCN + "," + pair.childFQCN).sorted.mkString("\n")
             channel.truncate(0)
             channel.write(ByteBuffer.wrap(outData.getBytes(StandardCharsets.UTF_8)))
-
-            typeNamesToCheck ++= outParentChildFQCNPairs.groupBy(_.parentFQCN)
-            typesNotDumped = false
           } finally {
             lock.close()
           }
@@ -243,19 +243,6 @@ class SerializerCheckCompilerPluginComponent(
         }
       }
 
-      private def removeOutdatedTypesFromCacheFile(typeNamesToRemove: List[String]): Unit = {
-        val cacheFile = options.directClassDescendantsCacheFile.toScala
-        val contentWithoutOutdatedTypes =
-          cacheFile.contentAsString
-            .split("\n")
-            .toList
-            .filterNot(line => typeNamesToRemove.exists(typeName => line.contains(typeName)))
-            .mkString("\n")
-            .stripMargin
-        cacheFile.clear()
-        cacheFile.write(contentWithoutOutdatedTypes)
-      }
-
     }
   }
 }
diff --git a/...-plugin/src/test/scala/org/virtuslab/ash/CodecRegistrationCheckerCompilerPluginSpec.scala b/...-plugin/src/test/scala/org/virtuslab/ash/CodecRegistrationCheckerCompilerPluginSpec.scala
@@ -12,8 +12,6 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
   private val dataSourceCode =
     (for (f <- File(getClass.getClassLoader.getResource("data")).children) yield f.contentAsString).toList
 
-  private val sourceCodeDirectory = File(getClass.getClassLoader.getResource(".")).path.toString
-
   private val CORRECT_SERIALIZER_CODE = getSerializerAsString("CorrectSerializer")
   private val EMPTY_SERIALIZER_CODE = getSerializerAsString("EmptySerializer")
   private val INCOMPLETE_SERIALIZER_ONE_CODE = getSerializerAsString("IncompleteSerializer")
@@ -28,7 +26,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
       File.usingTemporaryDirectory() { directory =>
         val out = CodecRegistrationCheckerCompiler.compileCode(
           CORRECT_SERIALIZER_CODE :: dataSourceCode,
-          List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+          List(s"${directory.toJava.getAbsolutePath}"))
         out should be("")
       }
     }
@@ -37,7 +35,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
       File.usingTemporaryDirectory() { directory =>
         val out = CodecRegistrationCheckerCompiler.compileCode(
           OBJECT_SERIALIZER_CODE :: dataSourceCode,
-          List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+          List(s"${directory.toJava.getAbsolutePath}"))
         out should include("error")
       }
     }
@@ -47,7 +45,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
         File.usingTemporaryDirectory() { directory =>
           val out = CodecRegistrationCheckerCompiler.compileCode(
             EMPTY_SERIALIZER_CODE :: dataSourceCode,
-            List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+            List(s"${directory.toJava.getAbsolutePath}"))
           out should include("error")
         }
       }
@@ -56,7 +54,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
         File.usingTemporaryDirectory() { directory =>
           val out = CodecRegistrationCheckerCompiler.compileCode(
             INCOMPLETE_SERIALIZER_ONE_CODE :: dataSourceCode,
-            List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+            List(s"${directory.toJava.getAbsolutePath}"))
           out should include("error")
           (out should not).include("literal")
         }
@@ -66,7 +64,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
         File.usingTemporaryDirectory() { directory =>
           val out = CodecRegistrationCheckerCompiler.compileCode(
             INVALID_ANNOTATION_SERIALIZER_CODE :: dataSourceCode,
-            List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+            List(s"${directory.toJava.getAbsolutePath}"))
           out should include("error")
         }
       }
@@ -75,7 +73,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
         File.usingTemporaryDirectory() { directory =>
           val out = CodecRegistrationCheckerCompiler.compileCode(
             INVALID_CLASS_SERIALIZER_CODE :: dataSourceCode,
-            List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+            List(s"${directory.toJava.getAbsolutePath}"))
           out should include("error")
         }
       }
@@ -84,7 +82,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
         File.usingTemporaryDirectory() { directory =>
           val out = CodecRegistrationCheckerCompiler.compileCode(
             INCOMPLETE_SERIALIZER_TWO_CODE :: dataSourceCode,
-            List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+            List(s"${directory.toJava.getAbsolutePath}"))
           out should include("error")
           (out should not).include("literal")
         }
@@ -94,19 +92,15 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
     "work with no serializer" in {
       File.usingTemporaryDirectory() { directory =>
         val out =
-          CodecRegistrationCheckerCompiler.compileCode(
-            dataSourceCode,
-            List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+          CodecRegistrationCheckerCompiler.compileCode(dataSourceCode, List(s"${directory.toJava.getAbsolutePath}"))
         out should be("")
       }
     }
 
     "create cache file when missing" in {
       File.usingTemporaryDirectory() { directory =>
         val out =
-          CodecRegistrationCheckerCompiler.compileCode(
-            dataSourceCode,
-            List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+          CodecRegistrationCheckerCompiler.compileCode(dataSourceCode, List(s"${directory.toJava.getAbsolutePath}"))
         out should be("")
         val cacheFile =
           (directory / CodecRegistrationCheckerCompilerPlugin.directClassDescendantsCacheFileName).contentAsString
@@ -122,9 +116,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
         val cacheFile = directory / CodecRegistrationCheckerCompilerPlugin.directClassDescendantsCacheFileName
         cacheFile < "org.random.project.SerializableTrait,org.random.project.MissingData"
         val out =
-          CodecRegistrationCheckerCompiler.compileCode(
-            dataSourceCode,
-            List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+          CodecRegistrationCheckerCompiler.compileCode(dataSourceCode, List(s"${directory.toJava.getAbsolutePath}"))
         out should be("")
         val cacheFileAfter = cacheFile.contentAsString
         cacheFileAfter should be("""org.random.project.SerializableTrait,org.random.project.GenericData
@@ -141,9 +133,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
           File.usingTemporaryDirectory() { directory =>
             val cacheFile = directory / CodecRegistrationCheckerCompilerPlugin.directClassDescendantsCacheFileName
             cacheFile < "org.random.project.SerializableTrait"
-            CodecRegistrationCheckerCompiler.compileCode(
-              dataSourceCode,
-              List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+            CodecRegistrationCheckerCompiler.compileCode(dataSourceCode, List(s"${directory.toJava.getAbsolutePath}"))
           }
         }
       }
@@ -153,7 +143,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
           File.usingTemporaryDirectory() { directory =>
             CodecRegistrationCheckerCompiler.compileCode(
               dataSourceCode,
-              List(s"${directory.toJava.getAbsolutePath}\u0000", s"--source-code-directory=$sourceCodeDirectory"))
+              List(s"${directory.toJava.getAbsolutePath}\u0000"))
           }
         }
       }
@@ -163,21 +153,13 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
           CodecRegistrationCheckerCompiler.compileCode(dataSourceCode, Nil)
         }
       }
-
-      "--source-code-directory option is not specified" in {
-        assertThrows[RuntimeException] {
-          File.usingTemporaryDirectory() { directory =>
-            CodecRegistrationCheckerCompiler.compileCode(dataSourceCode, List(s"${directory.toJava.getAbsolutePath}"))
-          }
-        }
-      }
     }
 
     "compile with REGISTRATION_REGEX macro" in {
       File.usingTemporaryDirectory() { directory =>
         val out = CodecRegistrationCheckerCompiler.compileCode(
           List(MACRO_REGEX_SERIALIZER_CODE, dataSourceCode.find(_.contains("@SerializabilityTrait")).get),
-          List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+          List(s"${directory.toJava.getAbsolutePath}"))
         out should be("")
       }
     }
@@ -191,7 +173,7 @@ class CodecRegistrationCheckerCompilerPluginSpec extends AnyWordSpecLike with sh
           "hydra.test.TestAkkaSerializable,hydra.test.ConcreteClasses")
         val out = CodecRegistrationCheckerCompiler.compileCode(
           CORRECT_SERIALIZER_CODE :: dataSourceCode,
-          List(s"${directory.toJava.getAbsolutePath}", s"--source-code-directory=$sourceCodeDirectory"))
+          List(s"${directory.toJava.getAbsolutePath}"))
         out should be("")
         cacheFile.contentAsString should be("""org.random.project.SerializableTrait,org.random.project.GenericData
                                               |org.random.project.SerializableTrait,org.random.project.IndirectData