Skip to content

Commit

Permalink
Merge pull request #898 from marcelocenerine/optimized_escape_hatch
Browse files Browse the repository at this point in the history
Optimizations in escape hatch
  • Loading branch information
olafurpg authored Oct 19, 2018
2 parents 18650f5 + fdaaf3f commit 1f2cff7
Show file tree
Hide file tree
Showing 11 changed files with 316 additions and 147 deletions.
238 changes: 130 additions & 108 deletions scalafix-core/src/main/scala/scalafix/internal/patch/EscapeHatch.scala

Large diffs are not rendered by default.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ object PatchInternals {
index: Option[v0.SemanticdbIndex],
suppress: Boolean = false
): (String, List[RuleDiagnostic]) = {
if (FastPatch.shortCircuit(patchesByName, ctx)) {
if (patchesByName.values.forall(_.isEmpty) && ctx.escapeHatch.isEmpty) {
(ctx.input.text, Nil)
} else {
val idx = index.getOrElse(v0.SemanticdbIndex.empty)
val (patch, lints) = ctx.escapeHatch.filter(patchesByName, ctx, idx)
val (patch, lints) = ctx.escapeHatch.filter(patchesByName)(ctx, idx)
val finalPatch =
if (suppress) {
patch + SuppressOps.addComments(ctx.tokens, lints.map(_.position))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,24 @@ package scalafix.internal.rule

import org.scalameta.FileLine
import org.scalameta.logger

import scala.meta._
import scala.meta.contrib.AssociatedComments
import scala.meta.tokens.Tokens
import scalafix.internal.config.ScalafixConfig
import scalafix.internal.diff.DiffDisable
import scalafix.internal.patch.EscapeHatch
import scalafix.internal.patch.LegacyPatchOps
import scalafix.internal.v1.LazyValue
import scalafix.syntax._
import scalafix.util.MatchingParens
import scalafix.util.SemanticdbIndex
import scalafix.util.TokenList
import scalafix.v0._

case class RuleCtxImpl(
tree: Tree,
config: ScalafixConfig,
class RuleCtxImpl(
val tree: Tree,
val config: ScalafixConfig,
diffDisable: DiffDisable)
extends RuleCtx
with LegacyPatchOps { ctx =>
Expand All @@ -31,7 +33,12 @@ case class RuleCtxImpl(
lazy val matchingParens: MatchingParens = MatchingParens(tokens)
lazy val comments: AssociatedComments = AssociatedComments(tokens)
lazy val input: Input = tokens.head.input
lazy val escapeHatch: EscapeHatch = EscapeHatch(tree, comments)
lazy val escapeHatch: EscapeHatch =
EscapeHatch(
input,
LazyValue.now(tree),
LazyValue.later(() => comments),
diffDisable)

// Debug utilities
def index(implicit index: SemanticdbIndex): SemanticdbIndex =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,4 @@ class LegacyRuleCtx(doc: SyntacticDocument)
t.tokens(doc.internal.config.dialect)
override private[scalafix] def config = doc.internal.config
override private[scalafix] def escapeHatch = doc.internal.escapeHatch.value
override private[scalafix] def diffDisable = doc.internal.diffDisable
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import scala.meta.Tree
import scala.meta.contrib.AssociatedComments
import scala.meta.inputs.Input
import scalafix.internal.config.ScalafixConfig
import scalafix.internal.diff.DiffDisable
import scalafix.internal.patch.EscapeHatch
import scalafix.util.MatchingParens
import scalafix.util.TokenList
Expand All @@ -15,7 +14,6 @@ class InternalDoc(
val comments: LazyValue[AssociatedComments],
val config: ScalafixConfig,
val escapeHatch: LazyValue[EscapeHatch],
val diffDisable: DiffDisable,
val matchingParens: LazyValue[MatchingParens],
val tokenList: LazyValue[TokenList]
)
3 changes: 1 addition & 2 deletions scalafix-core/src/main/scala/scalafix/patch/Patch.scala
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ sealed abstract class Patch extends Product {
def nonEmpty: Boolean = !isEmpty

/** Skip this entire patch if a part of it is disabled with // scalafix:off */
def atomic: Patch = AtomicPatch(this)

def atomic: Patch = if (isEmpty) this else AtomicPatch(this)
}

object Patch {
Expand Down
3 changes: 1 addition & 2 deletions scalafix-core/src/main/scala/scalafix/v0/RuleCtx.scala
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ trait RuleCtx extends PatchOps {
private[scalafix] def toks(t: Tree): Tokens
private[scalafix] def config: ScalafixConfig
private[scalafix] def escapeHatch: EscapeHatch
private[scalafix] def diffDisable: DiffDisable
}

object RuleCtx {
Expand All @@ -64,5 +63,5 @@ object RuleCtx {
tree: Tree,
config: ScalafixConfig,
diffDisable: DiffDisable): RuleCtx =
RuleCtxImpl(tree, config, diffDisable)
new RuleCtxImpl(tree, config, diffDisable)
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ object SyntacticDocument {
AssociatedComments(tree.value)
}
val escape = LazyValue.later { () =>
EscapeHatch(tree.value, comments.value)
EscapeHatch(input, tree, comments, diffDisable)
}
val matchingParens = LazyValue.later { () =>
MatchingParens(tokens.value)
Expand All @@ -77,7 +77,6 @@ object SyntacticDocument {
comments = comments,
config = config,
escapeHatch = escape,
diffDisable = diffDisable,
matchingParens = matchingParens,
tokenList = tokenList
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
package scalafix.tests.core

import java.nio.file.Paths

import org.scalatest.FunSuite
import scalafix.internal.config.ScalafixConfig
import scalafix.internal.diff.{DiffDisable, EmptyDiff, NewFile}
import scalafix.internal.patch.EscapeHatch
import scalafix.internal.v0.LegacyRuleCtx
import scalafix.internal.v1.LazyValue
import scalafix.patch.Patch
import scalafix.rule.RuleName
import scalafix.v0.SemanticdbIndex
import scalafix.v1.SyntacticDocument

import scala.meta.{Source, Tree}
import scala.meta.contrib.AssociatedComments
import scala.meta.inputs.Input

class EscapeHatchSuite extends FunSuite {

private val noEscapes = Input.String(
"""object Foo"""
)
private val scalafixOk = Input.String(
"""
|object Foo // scalafix:ok
""".stripMargin
)
private val scalafixOnOff = Input.String(
"""
|// scalafix:off
|object Foo
|// scalafix:on
""".stripMargin
)
private val suppressWarnings = Input.String(
"""
|@SuppressWarnings(Array("all"))
|object Foo
""".stripMargin
)

private class DontTouchMe(who: String) extends RuntimeException
private lazy val untouchableTree: LazyValue[Tree] =
LazyValue.later(() => throw new DontTouchMe("tree"))
private lazy val untouchableComments: LazyValue[AssociatedComments] =
LazyValue.later(() => throw new DontTouchMe("associated comments"))

test(
"`apply` should not evaluate tree nor associated comments if escapes are not found") {
EscapeHatch(noEscapes, untouchableTree, untouchableComments, EmptyDiff)
}

test(
"`apply` should evaluate tree and associated comments if 'scalafix:ok' is found") {
val (input, tree, comments) = params(scalafixOk)

intercept[DontTouchMe] {
EscapeHatch(input, untouchableTree, comments, EmptyDiff)
}
intercept[DontTouchMe] {
EscapeHatch(input, tree, untouchableComments, EmptyDiff)
}
}

test("`apply` should evaluate tree if 'scalafix:on|off' is found") {
val (input, tree, comments) = params(scalafixOnOff)

intercept[DontTouchMe] {
EscapeHatch(input, untouchableTree, comments, EmptyDiff)
}
EscapeHatch(input, tree, untouchableComments, EmptyDiff) // should not touch comments
}

test("`apply` should evaluate tree if `@SuppressWarnings` is found") {
val (input, tree, comments) = params(suppressWarnings)

intercept[DontTouchMe] {
EscapeHatch(input, untouchableTree, comments, EmptyDiff)
}
EscapeHatch(input, tree, untouchableComments, EmptyDiff) // should not touch comments
}

test(
"`isEmpty` should not evaluate tree nor associated comments if escapes are not found") {
assert(EscapeHatch(
noEscapes,
untouchableTree,
untouchableComments,
EmptyDiff).isEmpty)
}

test(
"`filter` should not evaluate tree nor associated comments if no escapes are found") {
val input = noEscapes
val doc = SyntacticDocument(
input,
untouchableTree,
EmptyDiff,
ScalafixConfig.default)
implicit val ctx = new LegacyRuleCtx(doc)
implicit val idx = SemanticdbIndex.empty
val patches = Map(RuleName("foo") -> Patch.empty)
val hatch =
EscapeHatch(input, untouchableTree, untouchableComments, EmptyDiff)

assert(hatch.filter(patches) == (Patch.empty, Nil))
}

test("should be empty if file contains no escapes nor git diffs") {
val (input, tree, comments) = params(noEscapes)
val hatch = EscapeHatch(input, tree, comments, EmptyDiff)

assert(hatch.isEmpty)
}

test("should not be empty if file contains 'scalafix:ok'") {
val (input, tree, comments) = params(scalafixOk)
val hatch = EscapeHatch(input, tree, comments, EmptyDiff)

assert(!hatch.isEmpty)
}

test("should not be empty if file contains 'scalafix:on|off'") {
val (input, tree, comments) = params(scalafixOnOff)
val hatch = EscapeHatch(input, tree, comments, EmptyDiff)

assert(!hatch.isEmpty)
}

test("should not be empty if file contains `@SuppressWarnings`") {
val (input, tree, comments) = params(suppressWarnings)
val hatch = EscapeHatch(input, tree, comments, EmptyDiff)

assert(!hatch.isEmpty)
}

test("should not be empty if there are git diffs") {
val (input, tree, comments) = params(noEscapes)
val diff = DiffDisable(List(NewFile(Paths.get("foo"))))
val hatch = EscapeHatch(input, tree, comments, diff)

assert(!hatch.isEmpty)
}

private def params(
input: Input): (Input, LazyValue[Tree], LazyValue[AssociatedComments]) = {
val tree = input.parse[Source].get
val comments = AssociatedComments.apply(tree)
(input, LazyValue.now(tree), LazyValue.now(comments))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import scala.meta.tokens.Token.Ident
import scalafix.v0.Rule
import scalafix.testkit.SyntacticRuleSuite
import scalafix.internal.tests.utils.SkipWindows
import scalafix.patch.Patch

class PatchSuite extends SyntacticRuleSuite {

Expand Down Expand Up @@ -95,4 +96,20 @@ class PatchSuite extends SyntacticRuleSuite {
|object A
|""".stripMargin
)

val tree = Input.String(original).parse[Source].get

test("Patch.empty") {
val empty = Patch.empty
assert(empty.isEmpty)
assert(empty.atomic.isEmpty)
assert((empty + empty).isEmpty)
assert((empty + empty).atomic.isEmpty)

val nonEmpty = Patch.addLeft(tree, "?")
assert(nonEmpty.isEmpty == false)
assert(nonEmpty.atomic.isEmpty == false)
assert((nonEmpty + nonEmpty).isEmpty == false)
assert((nonEmpty + nonEmpty).atomic.isEmpty == false)
}
}

0 comments on commit 1f2cff7

Please sign in to comment.