Suppression comments sometimes suppress more than intended

[coreywoodfield]

I have discovered that sometimes, suppression comments suppress more than they are (ostensibly) intended to suppress. Consider the following simple scalafix rule that simply disallows the use of the name foo, and test file we will run it on:

Rule.scala

import scala.meta.*
import scalafix.v1.*

class Test extends SyntacticRule("Test") {
  override def fix(implicit doc: SyntacticDocument): Patch = {
    doc.tree.collect { case t @ Term.Name("foo") =>
      Patch.lint(Diagnostic("foundFoo", s"don't use foo", t.pos))
    }.asPatch
  }
}

Test.scala

class Test {
  List().foreach { value =>
    foo
    foo
    foo
  }
  () match {
    case "end comment works as expected" =>
      foo
      foo
    case "end comment works as expected" => {
      foo
      foo
    }
  }
  () match {
    case "end comment suppresses the rule in the whole case" =>
      foo
      foo
  }
}

We run the rule on the file, and it works as expected: we get one linting error for every use of foo. We don't want to clean these up right away, so we run again with --auto-suppress-linter-errors. We end up with

class Test {
  List().foreach { value =>
    foo/* scalafix:ok */
    foo/* scalafix:ok */
    foo/* scalafix:ok */
  }
  () match {
    case "end comment works as expected" =>
      foo/* scalafix:ok */
      foo/* scalafix:ok */
    case "end comment works as expected" => {
      foo/* scalafix:ok */
      foo/* scalafix:ok */
    }
  }
  () match {
    case "end comment suppresses the rule in the whole case" =>
      foo/* scalafix:ok */
      foo/* scalafix:ok */
  }
}

So far so good. The next time we run the linting, however, we get something unexpected: three warnings

/path/to/Test.scala:3:8: warning: [UnusedScalafixSuppression] Unused Scalafix suppression, this can be removed
    foo/* scalafix:ok */
       ^^^^^^^^^^^^^^^^^
/path/to/Test.scala:4:8: warning: [UnusedScalafixSuppression] Unused Scalafix suppression, this can be removed
    foo/* scalafix:ok */
       ^^^^^^^^^^^^^^^^^
/path/to/Test.scala:18:10: warning: [UnusedScalafixSuppression] Unused Scalafix suppression, this can be removed
      foo/* scalafix:ok */
         ^^^^^^^^^^^^^^^^^

... what? Those were just added.

It turns out that the comment after the last statement of some blocks is interpreted as applying to the whole block, rather than just to the last statement. If we appease the rule, we end up with something like

class Test {
  List().foreach { value =>
    foo
    foo
    foo/* scalafix:ok; applies to whole block */
  }
  () match {
    case "end comment works as expected" =>
      foo/* scalafix:ok */
      foo/* scalafix:ok; doesn't apply to whole block */
    case "end comment works as expected" => {
      foo/* scalafix:ok */
      foo/* scalafix:ok; doesn't apply to whole block */
    }
  }
  () match {
    case "end comment suppresses the rule in the whole case" =>
      foo
      foo/* scalafix:ok; applies to whole block */
  }
}

Adding new foo references inside the foreach or inside the last match does not cause the rule to emit a new error, whereas it arguably should. I think in general, a comment at the end of a block that is not delimited by braces should probably be treated as a comment on the last statement of that block.