Merge pull request #812 from olafurpg/perf

Performance improvements

Author: olafurpg

scalafix-cli/src/main/scala/scalafix/internal/v1/MainOps.scala

@@ -23,6 +23,7 @@ import scala.collection.mutable.ListBuffer
 import scala.meta.Tree
 import scala.meta.inputs.Input
 import scala.meta.internal.semanticdb.TextDocument
+import scala.meta.internal.tokenizers.PlatformTokenizerCache
 import scala.meta.io.AbsolutePath
 import scala.meta.parsers.ParseException
 import scala.util.control.NoStackTrace
@@ -229,8 +230,10 @@ object MainOps {
   }
 
   def handleFile(args: ValidatedArgs, file: AbsolutePath): ExitStatus = {
-    try unsafeHandleFile(args, file)
-    catch {
+    try {
+      PlatformTokenizerCache.megaCache.clear()
+      unsafeHandleFile(args, file)
+    } catch {
       case e: ParseException =>
         args.config.reporter.error(e.shortMessage, e.pos)
         ExitStatus.ParseError
@@ -251,10 +254,15 @@ object MainOps {
 
   def run(args: ValidatedArgs): ExitStatus = {
     val files = this.files(args)
+    var i = 0
+    val N = files.length
+    val width = N.toString.length
     var exit = ExitStatus.Ok
     files.foreach { file =>
       if (args.args.verbose) {
-        args.config.reporter.info(s"Processing $file")
+        val message = s"Processing (%${width}s/%s) %s".format(i, N, file)
+        args.config.reporter.info(message)
+        i += 1
       }
       val next = handleFile(args, file)
       exit = ExitStatus.merge(exit, next)

scalafix-core/src/main/scala/scalafix/internal/config/Optimization.scala

@@ -10,7 +10,6 @@ case class ScalafixConfig(
     debug: DebugConfig = DebugConfig(),
     groupImportsByPrefix: Boolean = true,
     fatalWarnings: Boolean = true,
-    optimization: Optimization = Optimization.default,
     reporter: ScalafixReporter = ScalafixReporter.default,
     patches: ConfigRulePatches = ConfigRulePatches.default,
     dialect: Dialect = ScalafixConfig.DefaultDialect,

scalafix-core/src/main/scala/scalafix/internal/config/ScalafixConfig.scala

@@ -15,14 +15,17 @@ object DiffDisable {
 sealed trait DiffDisable {
   def isDisabled(position: Position): Boolean
   def isDisabled(file: Input): Boolean
+  def isEmpty: Boolean
 }
 
-private object EmptyDiff extends DiffDisable {
+object EmptyDiff extends DiffDisable {
+  override def isEmpty: Boolean = true
   def isDisabled(position: Position): Boolean = false
   def isDisabled(file: Input): Boolean = false
 }
 
-private class FullDiffDisable(diffs: List[GitDiff]) extends DiffDisable {
+class FullDiffDisable(diffs: List[GitDiff]) extends DiffDisable {
+  override def isEmpty: Boolean = diffs.isEmpty
   private val newFiles: Set[String] = diffs.collect {
     case NewFile(path) => path.toAbsolutePath.toString
   }.toSet

scalafix-core/src/main/scala/scalafix/internal/diff/DiffDisable.scala

@@ -10,8 +10,6 @@ import scala.meta.tokens.Token
 import scala.meta.tokens.Token.Comment
 import scalafix.v0._
 import scalafix.internal.config.FilterMatcher
-import scalafix.internal.config.ScalafixConfig
-import scalafix.internal.diff.DiffDisable
 import scalafix.internal.patch.EscapeHatch._
 import scalafix.lint.RuleDiagnostic
 import scalafix.patch.AtomicPatch
@@ -32,13 +30,33 @@ class EscapeHatch private (
     anchoredEscapes: AnchoredEscapes,
     annotatedEscapes: AnnotatedEscapes) {
 
+  def rawFilter(
+      patchesByName: Map[RuleName, Patch],
+      ctx: RuleCtx): (Patch, List[RuleDiagnostic]) = {
+    var patchBuilder = Patch.empty
+    val diagnostics = List.newBuilder[RuleDiagnostic]
+    patchesByName.foreach {
+      case (rule, rulePatch) =>
+        Patch.foreach(rulePatch) {
+          case LintPatch(message) =>
+            diagnostics += message.toDiagnostic(rule, ctx.config)
+          case rewritePatch =>
+            patchBuilder += rewritePatch
+        }
+    }
+    (patchBuilder, diagnostics.result())
+  }
+
   def filter(
       patchesByName: Map[RuleName, Patch],
       ctx: RuleCtx,
-      index: SemanticdbIndex,
-      diff: DiffDisable,
-      config: ScalafixConfig
+      index: SemanticdbIndex
   ): (Patch, List[RuleDiagnostic]) = {
+    if (!FastPatch.hasSuppression(ctx.input.text) && ctx.diffDisable.isEmpty) {
+      return rawFilter(patchesByName, ctx)
+    }
+    val diff = ctx.diffDisable
+    val config = ctx.config
     val usedEscapes = mutable.Set.empty[EscapeFilter]
     val lintMessages = List.newBuilder[RuleDiagnostic]
 

scalafix-core/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

@@ -0,0 +1,24 @@
+package scalafix.internal.patch
+
+import scalafix.patch.Patch
+import scalafix.rule.RuleName
+import scalafix.v0
+
+object FastPatch {
+
+  def hasSuppression(text: String): Boolean = {
+    text.indexOf("scalafix:") >= 0 ||
+    text.indexOf("SuppressWarnings") >= 0
+  }
+
+  def shortCircuit(
+      patchesByName: Map[RuleName, Patch],
+      ctx: v0.RuleCtx): Boolean = {
+    if (ctx.diffDisable.isEmpty &&
+      patchesByName.values.forall(_.isEmpty)) {
+      !hasSuppression(ctx.input.text)
+    } else {
+      false
+    }
+  }
+}

scalafix-core/src/main/scala/scalafix/internal/patch/FastPatch.scala

@@ -12,7 +12,7 @@ import scalafix.util.TokenList
 import scalafix.v0.RuleCtx
 import scalafix.v1.Doc
 
-class DeprecatedRuleCtx(doc: Doc) extends RuleCtx with DeprecatedPatchOps {
+class LegacyRuleCtx(doc: Doc) extends RuleCtx with DeprecatedPatchOps {
   override def tree: Tree = doc.tree
   override def input: Input = doc.input
   override def tokens: Tokens = doc.tokens

scalafix-core/src/main/scala/scalafix/internal/v0/DeprecatedRuleCtx.scala renamed to scalafix-core/src/main/scala/scalafix/internal/v0/LegacyRuleCtx.scala

@@ -21,7 +21,7 @@ class LegacySemanticRule(name: RuleName, fn: v0.SemanticdbIndex => v0.Rule)
     }
   }
   override def fix(implicit sdoc: SemanticDoc): Patch = {
-    val ctx = new DeprecatedRuleCtx(sdoc.internal.doc)
+    val ctx = new LegacyRuleCtx(sdoc.internal.doc)
     val rule = fn(new DocSemanticdbIndex(sdoc)).init(this.conf).get
     rule.fix(ctx) + LegacyRule.lints(ctx, rule)
   }

scalafix-core/src/main/scala/scalafix/internal/v0/LegacySemanticRule.scala

@@ -18,7 +18,7 @@ class LegacySyntacticRule(rule: v0.Rule) extends SyntacticRule(rule.name) {
     }
   }
   override def fix(implicit doc: Doc): Patch = {
-    val ctx = new DeprecatedRuleCtx(doc)
+    val ctx = new LegacyRuleCtx(doc)
     configuredRule.fix(ctx) + LegacyRule.lints(ctx, configuredRule)
   }
 }

scalafix-core/src/main/scala/scalafix/internal/v0/LegacySyntacticRule.scala

@@ -16,10 +16,11 @@ import scalafix.patch.TreePatch.ReplaceSymbol
 import org.scalameta.logger
 import scala.meta.Token
 import scala.meta.tokens.Tokens
+import scalafix.internal.patch.FastPatch
 import scalafix.internal.config.ScalafixMetaconfigReaders
 import scalafix.internal.util.SuppressOps
 import scalafix.internal.util.SymbolOps.Root
-import scalafix.internal.v0.DeprecatedRuleCtx
+import scalafix.internal.v0.LegacyRuleCtx
 import scalafix.internal.v0.DocSemanticdbIndex
 import scalafix.lint.RuleDiagnostic
 import scalafix.patch.TreePatch.AddGlobalImport
@@ -204,18 +205,11 @@ object Patch {
       index: Option[SemanticdbIndex],
       suppress: Boolean = false
   ): (String, List[RuleDiagnostic]) = {
-    if (ctx.config.optimization.skipSuppressionWhenPossible &&
-      patchesByName.values.forall(_.isEmpty)) {
+    if (FastPatch.shortCircuit(patchesByName, ctx)) {
       (ctx.input.text, Nil)
     } else {
       val idx = index.getOrElse(SemanticdbIndex.empty)
-      val (patch, lints) = ctx.escapeHatch.filter(
-        patchesByName,
-        ctx,
-        idx,
-        ctx.diffDisable,
-        ctx.config
-      )
+      val (patch, lints) = ctx.escapeHatch.filter(patchesByName, ctx, idx)
       val finalPatch =
         if (suppress) {
           patch + SuppressOps.addComments(ctx.tokens, lints.map(_.position))
@@ -232,7 +226,7 @@ object Patch {
       doc: Doc,
       suppress: Boolean
   ): (String, List[RuleDiagnostic]) = {
-    apply(patchesByName, new DeprecatedRuleCtx(doc), None, suppress)
+    apply(patchesByName, new LegacyRuleCtx(doc), None, suppress)
   }
 
   private[scalafix] def semantic(
@@ -242,7 +236,7 @@ object Patch {
   ): (String, List[RuleDiagnostic]) = {
     apply(
       patchesByName,
-      new DeprecatedRuleCtx(doc.internal.doc),
+      new LegacyRuleCtx(doc.internal.doc),
       Some(new DocSemanticdbIndex(doc)),
       suppress)
   }
@@ -311,7 +305,7 @@ object Patch {
     patch.isEmpty || hasLintMessage && onlyLint
   }
 
-  private def foreach(patch: Patch)(f: Patch => Unit): Unit = {
+  private[scalafix] def foreach(patch: Patch)(f: Patch => Unit): Unit = {
     def loop(patch: Patch): Unit = patch match {
       case Concat(a, b) =>
         loop(a)

scalafix-core/src/main/scala/scalafix/patch/Patch.scala

@@ -23,14 +23,13 @@ object DiffAssertions {
     else
       difflib.DiffUtils
         .generateUnifiedDiff(
-          "original",
-          "revised",
+          "expected",
+          "obtained",
           original.asJava,
           diff,
           1
         )
         .asScala
-        .drop(3)
         .mkString("\n")
   }
 }

scalafix-testkit/src/main/scala/scalafix/testkit/DiffAssertions.scala

@@ -3,7 +3,6 @@ rules = [
   "class:scalafix.test.NoDummy"
   "class:scalafix.test.NoNull"
 ]
-optimization.skipSuppressionWhenPossible = false
 */
 package test.escapeHatch
 

scalafix-tests/input/src/main/scala/test/escapeHatch/AnnotationUnused.scala

@@ -1,3 +1,4 @@
 banana.rule.SemanticRuleV1
 banana.rule.SyntacticRuleV1
+scalafix.tests.cli.NoOpRule
 scalafix.tests.cli.DeprecatedName

scalafix-tests/unit/src/main/resources/META-INF/services/scalafix.v1.Rule

@@ -304,23 +304,38 @@ class CliSyntacticSuite extends BaseCliSuite {
   )
 
   check(
-    name = "--settings.optimization.skipSuppressionWhenPossible",
+    name = "skip parser when it's not needed",
     originalLayout = """
                        |/src/shared/a.scala
                        |object a {
                        |""".stripMargin,
     args = Array(
       "-r",
-      "class:scalafix.tests.cli.NoOpRule",
-      "--settings.optimization.skipSuppressionWhenPossible",
-      "true"
+      "NoOpRule"
     ),
     expectedLayout = """
                        |/src/shared/a.scala
                        |object a {
                        |""".stripMargin,
     expectedExit = ExitStatus.Ok
   )
+
+  check(
+    name = "don't skip parser when there is a suppression",
+    originalLayout = """
+                       |/src/shared/a.scala
+                       |object a { // scalafix:
+                       |""".stripMargin,
+    args = Array(
+      "-r",
+      "NoOpRule"
+    ),
+    expectedLayout = """
+                       |/src/shared/a.scala
+                       |object a { // scalafix:
+                       |""".stripMargin,
+    expectedExit = ExitStatus.ParseError
+  )
 }
 
 class NoOpRule extends SyntacticRule("NoOpRule") {