Fix #373: addLeft's implementation doesn't match its specification

Return `headOption` instead of `lastOption` in addLeft's implementation. Additionally, add a unit test, and refactor the syntax tests. TESTED=unit tests pass
scalacenter · Sep 28, 2017 · 53df5c0 · 53df5c0
1 parent 6b8cccd
commit 53df5c0
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 7 deletions.
diff --git a/scalafix-core/shared/src/main/scala/scalafix/internal/rule/RuleCtxImpl.scala b/scalafix-core/shared/src/main/scala/scalafix/internal/rule/RuleCtxImpl.scala
@@ -85,7 +85,7 @@ case class RuleCtxImpl(tree: Tree, config: ScalafixConfig) extends RuleCtx {
     toks(tree).lastOption.fold(Patch.empty)(addRight(_, toAdd))
   def addLeft(tok: Token, toAdd: String): Patch = Add(tok, toAdd, "")
   def addLeft(tree: Tree, toAdd: String): Patch =
-    toks(tree).lastOption.fold(Patch.empty)(addLeft(_, toAdd))
+    toks(tree).headOption.fold(Patch.empty)(addLeft(_, toAdd))
 
   // Semantic patch ops.
   def removeGlobalImport(symbol: Symbol)(

diff --git a/scalafix-testkit/src/main/scala/scalafix/testkit/SyntacticRuleSuite.scala b/scalafix-testkit/src/main/scala/scalafix/testkit/SyntacticRuleSuite.scala
@@ -8,8 +8,18 @@ import scalafix.syntax._
 
 import org.scalatest.FunSuiteLike
 
-class SyntacticRuleSuite(rule: Rule) extends FunSuiteLike with DiffAssertions {
+class SyntacticRuleSuite(defaultRule: Rule = Rule.empty)
+    extends FunSuiteLike
+    with DiffAssertions {
   def check(name: String, original: String, expected: String): Unit = {
+    check(defaultRule, name, original, expected)
+  }
+
+  def check(
+      rule: Rule,
+      name: String,
+      original: String,
+      expected: String): Unit = {
     test(name) {
       import scala.meta._
       val obtained = rule.apply(Input.String(original))
@@ -18,6 +28,10 @@ class SyntacticRuleSuite(rule: Rule) extends FunSuiteLike with DiffAssertions {
   }
 
   def checkDiff(original: Input, expected: String): Unit = {
+    checkDiff(defaultRule, original, expected)
+  }
+
+  def checkDiff(rule: Rule, original: Input, expected: String): Unit = {
     test(original.label) {
       val ctx = RuleCtx(original.parse[Source].get, ScalafixConfig.default)
       val obtained = rule.diff(ctx)

diff --git a/...est/scala/scalafix/tests/ErrorSuite.scala → ...scala/scalafix/tests/SyntacticTests.scala b/...est/scala/scalafix/tests/ErrorSuite.scala → ...scala/scalafix/tests/SyntacticTests.scala
@@ -9,24 +9,27 @@ import java.nio.file.Files
 import java.nio.file.Paths
 import scalafix.internal.rule.ProcedureSyntax
 
-class ErrorSuite extends SyntacticRuleSuite(ProcedureSyntax) {
+class ErrorSuite extends SyntacticRuleSuite {
   test("on parse error") {
     intercept[ParseException] {
       ProcedureSyntax.apply(Input.String("object A {"))
     }
   }
 }
-class PatchSuite
-    extends SyntacticRuleSuite(Rule.syntactic("PatchSuite")(ctx =>
-      ctx.addRight(ctx.tree.tokens.find(_.is[Ident]).get, "bba"))) {
+class PatchSuite extends SyntacticRuleSuite {
 
-  val original =
+  val original: String =
     """// Foobar
       |object a {
       |  val x = 2
       |}""".stripMargin
 
+  val addRightRule: Rule = Rule.syntactic("addRight") { (ctx) =>
+    ctx.addRight(ctx.tree.tokens.find(_.is[Ident]).get, "bba")
+  }
+
   checkDiff(
+    addRightRule,
     Input.String(original),
     """--- Input.String('<// Foobar...>')
       |+++ Input.String('<// Foobar...>')
@@ -39,6 +42,7 @@ class PatchSuite
   )
 
   checkDiff(
+    addRightRule,
     Input.VirtualFile("/label", original),
     """--- /label
       |+++ /label
@@ -53,6 +57,7 @@ class PatchSuite
   val file = File.createTempFile("foo", ".scala")
   Files.write(Paths.get(file.toURI), original.getBytes)
   checkDiff(
+    addRightRule,
     Input.File(file),
     s"""--- ${file.getAbsolutePath}
        |+++ ${file.getAbsolutePath}
@@ -63,4 +68,19 @@ class PatchSuite
        |   val x = 2
        | }""".stripMargin
   )
+
+  val addLeftRule: Rule = Rule.syntactic("addLeft") { (ctx) =>
+    ctx.addLeft(ctx.tree, "object Foo {}\n")
+  }
+
+  check(
+    addLeftRule,
+    "addLeft adds the string before the first tree token",
+    original,
+    """object Foo {}
+      |// Foobar
+      |object a {
+      |  val x = 2
+      |}""".stripMargin
+  )
 }