Merge pull request #14954 from griggt/rewrite-fixes

scala · Apr 19, 2022 · c93a237 · c93a237
2 parents 7db4121 + c818e0a
commit c93a237
Show file tree

Hide file tree

Showing 7 changed files with 92 additions and 4 deletions.
diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -1603,14 +1603,32 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
             typedMatchFinish(tree, sel1, selType, tree.cases, pt)
         }
 
+        /** Are some form of brackets necessary to annotate the tree `sel` as `@unchecked`?
+         *  If so, return a Some(opening bracket, closing bracket), otherwise None.
+         */
+        def uncheckedBrackets(sel: untpd.Tree): Option[(String, String)] = sel match
+          case _: untpd.If
+             | _: untpd.Match
+             | _: untpd.ForYield
+             | _: untpd.ParsedTry
+             | _: untpd.Try => Some("(", ")")
+          case _: untpd.Block => Some("{", "}")
+          case _ => None
+
         result match {
           case result @ Match(sel, CaseDef(pat, _, _) :: _) =>
             tree.selector.removeAttachment(desugar.CheckIrrefutable) match {
               case Some(checkMode) if !sel.tpe.hasAnnotation(defn.UncheckedAnnot) =>
                 val isPatDef = checkMode == desugar.MatchCheck.IrrefutablePatDef
-                if (!checkIrrefutable(sel, pat, isPatDef) && sourceVersion == `future-migration`)
-                  if (isPatDef) patch(Span(tree.selector.span.end), ": @unchecked")
-                  else patch(Span(pat.span.start), "case ")
+                if !checkIrrefutable(sel, pat, isPatDef) && sourceVersion == `future-migration` then
+                  if isPatDef then uncheckedBrackets(tree.selector) match
+                    case None =>
+                      patch(Span(tree.selector.span.end), ": @unchecked")
+                    case Some(bl, br) =>
+                      patch(Span(tree.selector.span.start), s"$bl")
+                      patch(Span(tree.selector.span.end), s"$br: @unchecked")
+                  else
+                    patch(Span(tree.span.start), "case ")
 
                 // skip exhaustivity check in later phase
                 // TODO: move the check above to patternMatcher phase

diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala
@@ -73,6 +73,8 @@ class CompilationTests {
     aggregateTests(
       compileFile("tests/rewrites/rewrites.scala", scala2CompatMode.and("-rewrite", "-indent")),
       compileFile("tests/rewrites/rewrites3x.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
+      compileFile("tests/rewrites/filtering-fors.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
+      compileFile("tests/rewrites/refutable-pattern-bindings.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
       compileFile("tests/rewrites/i8982.scala", defaultOptions.and("-indent", "-rewrite")),
       compileFile("tests/rewrites/i9632.scala", defaultOptions.and("-indent", "-rewrite")),
       compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite")),

diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala
@@ -1064,7 +1064,13 @@ trait ParallelTesting extends RunnerOrchestration { self =>
           target.copy(dir = copyToDir(outDir, dir))
       }
 
-      new RewriteTest(copiedTargets, checkFileMap, times, threadLimit, shouldFail || shouldSuppressOutput).executeTestSuite()
+      val test = new RewriteTest(copiedTargets, checkFileMap, times, threadLimit, shouldFail || shouldSuppressOutput).executeTestSuite()
+
+      cleanup()
+
+      if test.didFail then
+        fail("Rewrite test failed")
+
       this
     }
 

diff --git a/tests/rewrites/filtering-fors.check b/tests/rewrites/filtering-fors.check
@@ -0,0 +1,6 @@
+val xs: List[Any] = ???
+val as = for case (x: String) <- xs yield x
+val bs =
+  for
+    case (x: String) <- xs
+  yield x
diff --git a/tests/rewrites/filtering-fors.scala b/tests/rewrites/filtering-fors.scala
@@ -0,0 +1,6 @@
+val xs: List[Any] = ???
+val as = for (x: String) <- xs yield x
+val bs =
+  for
+    (x: String) <- xs
+  yield x
diff --git a/tests/rewrites/refutable-pattern-bindings.check b/tests/rewrites/refutable-pattern-bindings.check
@@ -0,0 +1,25 @@
+val xs: List[Any] = ???
+
+val hd :: tl = (xs match
+  case Nil => null :: xs
+  case _   => xs): @unchecked
+
+val h :: t = xs: @unchecked
+
+val a :: b =
+  (if xs.isEmpty then null :: xs
+  else xs): @unchecked
+
+val c :: d =
+  (try xs.head :: xs
+  catch case _: NoSuchElementException => null :: xs): @unchecked
+
+val e :: f =
+  {val zero = null :: Nil
+  if xs.isEmpty then zero
+  else xs}: @unchecked
+
+val j :: k =
+  (for
+    case (x: String) <- xs
+  yield x): @unchecked
diff --git a/tests/rewrites/refutable-pattern-bindings.scala b/tests/rewrites/refutable-pattern-bindings.scala
@@ -0,0 +1,25 @@
+val xs: List[Any] = ???
+
+val hd :: tl = xs match
+  case Nil => null :: xs
+  case _   => xs
+
+val h :: t = xs
+
+val a :: b =
+  if xs.isEmpty then null :: xs
+  else xs
+
+val c :: d =
+  try xs.head :: xs
+  catch case _: NoSuchElementException => null :: xs
+
+val e :: f =
+  val zero = null :: Nil
+  if xs.isEmpty then zero
+  else xs
+
+val j :: k =
+  for
+    (x: String) <- xs
+  yield x