From 9b66cef2374adc10218d7d4db1a3de3754774952 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 25 Jan 2022 11:27:24 -0800 Subject: [PATCH] Pretty print type on f-interpolator, improve caret Tweak vulpix to show difference in expectations. --- .../transform/localopt/FormatChecker.scala | 18 +- .../dotty/tools/vulpix/ParallelTesting.scala | 88 ++++---- tests/neg/f-interpolator-neg.check | 200 ++++++++++++++++++ tests/neg/f-interpolator-neg.scala | 4 +- tests/neg/f-interpolator-tests.check | 8 + 5 files changed, 269 insertions(+), 49 deletions(-) create mode 100644 tests/neg/f-interpolator-neg.check create mode 100644 tests/neg/f-interpolator-tests.check diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala index 94b10a5b538e..0ba7bd14a9b6 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -15,6 +15,7 @@ import dotty.tools.dotc.core.Contexts._ import dotty.tools.dotc.core.Symbols._ import dotty.tools.dotc.core.Types._ import dotty.tools.dotc.core.Phases.typerPhase +import dotty.tools.dotc.util.Spans.Span /** Formatter string checker. */ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List[Tree])(using Context): @@ -33,7 +34,7 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List types.find(t => argConformsTo(argi, tpe, t)) .orElse(types.find(t => argConvertsTo(argi, tpe, t))) .getOrElse { - report.argError(s"Found: ${tpe.show}, Required: ${types.mkString(", ")}", argi) + report.argError(s"Found: ${tpe.show}, Required: ${types.map(_.show).mkString(", ")}", argi) actuals += args(argi) types.head } @@ -118,6 +119,7 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List extension (descriptor: Match) def at(g: SpecGroup): Int = descriptor.start(g.ordinal) + def end(g: SpecGroup): Int = descriptor.end(g.ordinal) def offset(g: SpecGroup, i: Int = 0): Int = at(g) + i def group(g: SpecGroup): Option[String] = Option(descriptor.group(g.ordinal)) def stringOf(g: SpecGroup): String = group(g).getOrElse("") @@ -243,8 +245,8 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List val i = flags.indexOf(f) match { case -1 => 0 case j => j } errorAt(Flags, i)(msg) - def errorAt(g: SpecGroup, i: Int = 0)(msg: String) = report.partError(msg, argi, descriptor.offset(g, i)) - def warningAt(g: SpecGroup, i: Int = 0)(msg: String) = report.partWarning(msg, argi, descriptor.offset(g, i)) + def errorAt(g: SpecGroup, i: Int = 0)(msg: String) = report.partError(msg, argi, descriptor.offset(g, i), descriptor.end(g)) + def warningAt(g: SpecGroup, i: Int = 0)(msg: String) = report.partWarning(msg, argi, descriptor.offset(g, i), descriptor.end(g)) object Conversion: def apply(m: Match, i: Int): Conversion = @@ -272,12 +274,14 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List var reported = false - private def partPosAt(index: Int, offset: Int) = + private def partPosAt(index: Int, offset: Int, end: Int) = val pos = partsElems(index).sourcePos - pos.withSpan(pos.span.shift(offset)) + val bgn = pos.span.start + offset + val fin = if end < 0 then pos.span.end else pos.span.start + end + pos.withSpan(Span(bgn, fin, bgn)) extension (r: report.type) def argError(message: String, index: Int): Unit = r.error(message, args(index).srcPos).tap(_ => reported = true) - def partError(message: String, index: Int, offset: Int): Unit = r.error(message, partPosAt(index, offset)).tap(_ => reported = true) - def partWarning(message: String, index: Int, offset: Int): Unit = r.warning(message, partPosAt(index, offset)).tap(_ => reported = true) + def partError(message: String, index: Int, offset: Int, end: Int = -1): Unit = r.error(message, partPosAt(index, offset, end)).tap(_ => reported = true) + def partWarning(message: String, index: Int, offset: Int, end: Int = -1): Unit = r.warning(message, partPosAt(index, offset, end)).tap(_ => reported = true) end TypedFormatChecker diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index a497eed7072e..b2b4942ccb1b 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -14,13 +14,14 @@ import java.util.concurrent.{TimeUnit, TimeoutException, Executors => JExecutors import scala.collection.mutable import scala.io.{Codec, Source} +import scala.jdk.CollectionConverters.* import scala.util.{Random, Try, Failure => TryFailure, Success => TrySuccess, Using} import scala.util.control.NonFatal import scala.util.matching.Regex import scala.collection.mutable.ListBuffer import dotc.{Compiler, Driver} -import dotc.core.Contexts._ +import dotc.core.Contexts.* import dotc.decompiler import dotc.report import dotc.interfaces.Diagnostic.ERROR @@ -750,17 +751,26 @@ trait ParallelTesting extends RunnerOrchestration { self => def compilerCrashed = reporters.exists(_.compilerCrashed) lazy val (errorMap, expectedErrors) = getErrorMapAndExpectedCount(testSource.sourceFiles.toIndexedSeq) lazy val actualErrors = reporters.foldLeft(0)(_ + _.errorCount) - def hasMissingAnnotations = getMissingExpectedErrors(errorMap, reporters.iterator.flatMap(_.errors)) + lazy val (expected, unexpected) = getMissingExpectedErrors(errorMap, reporters.iterator.flatMap(_.errors)) + def hasMissingAnnotations = expected.nonEmpty || unexpected.nonEmpty def showErrors = "-> following the errors:\n" + - reporters.flatMap(_.allErrors.map(e => (e.pos.line + 1).toString + ": " + e.message)).mkString(start = "at ", sep = "\n at ", end = "") - - if (compilerCrashed) Some(s"Compiler crashed when compiling: ${testSource.title}") - else if (actualErrors == 0) Some(s"\nNo errors found when compiling neg test $testSource") - else if (expectedErrors == 0) Some(s"\nNo errors expected/defined in $testSource -- use // error or // nopos-error") - else if (expectedErrors != actualErrors) Some(s"\nWrong number of errors encountered when compiling $testSource\nexpected: $expectedErrors, actual: $actualErrors " + showErrors) - else if (hasMissingAnnotations) Some(s"\nErrors found on incorrect row numbers when compiling $testSource\n$showErrors") - else if (!errorMap.isEmpty) Some(s"\nExpected error(s) have {=}: $errorMap") - else None + reporters.flatMap(_.allErrors.sortBy(_.pos.line).map(e => s"${e.pos.line + 1}: ${e.message}")).mkString(" at ", "\n at ", "") + + Option { + if compilerCrashed then s"Compiler crashed when compiling: ${testSource.title}" + else if actualErrors == 0 then s"\nNo errors found when compiling neg test $testSource" + else if expectedErrors == 0 then s"\nNo errors expected/defined in $testSource -- use // error or // nopos-error" + else if expectedErrors != actualErrors then + s"""|Wrong number of errors encountered when compiling $testSource + |expected: $expectedErrors, actual: $actualErrors + |${expected.mkString("Unfulfilled expectations:\n", "\n", "")} + |${unexpected.mkString("Unexpected errors:\n", "\n", "")} + |$showErrors + |""".stripMargin.trim.linesIterator.mkString("\n", "\n", "") + else if hasMissingAnnotations then s"\nErrors found on incorrect row numbers when compiling $testSource\n$showErrors" + else if !errorMap.isEmpty then s"\nExpected error(s) have {=}: $errorMap" + else null + } } override def onSuccess(testSource: TestSource, reporters: Seq[TestReporter], logger: LoggedRunnable): Unit = @@ -783,7 +793,7 @@ trait ParallelTesting extends RunnerOrchestration { self => source.getLines.zipWithIndex.foreach { case (line, lineNbr) => val errors = line.toSeq.sliding("// error".length).count(_.unwrap == "// error") if (errors > 0) - errorMap.put(s"${file.getPath}:$lineNbr", errors) + errorMap.put(s"${file.getPath}:${lineNbr+1}", errors) val noposErrors = line.toSeq.sliding("// nopos-error".length).count(_.unwrap == "// nopos-error") if (noposErrors > 0) { @@ -813,34 +823,32 @@ trait ParallelTesting extends RunnerOrchestration { self => (errorMap, expectedErrors) } - def getMissingExpectedErrors(errorMap: HashMap[String, Integer], reporterErrors: Iterator[Diagnostic]) = !reporterErrors.forall { error => - val pos1 = error.pos.nonInlined - val key = if (pos1.exists) { - def toRelative(path: String): String = // For some reason, absolute paths leak from the compiler itself... - path.split(JFile.separatorChar).dropWhile(_ != "tests").mkString(JFile.separator) - val fileName = toRelative(pos1.source.file.toString) - s"$fileName:${pos1.line}" - - } else "nopos" - - val errors = errorMap.get(key) - - def missing = { echo(s"Error reported in ${pos1.source}, but no annotation found") ; false } - - if (errors ne null) { - if (errors == 1) errorMap.remove(key) - else errorMap.put(key, errors - 1) - true - } - else if key == "nopos" then - missing - else - errorMap.get("anypos") match - case null => missing - case 1 => errorMap.remove("anypos") ; true - case slack => if slack < 1 then missing - else errorMap.put("anypos", slack - 1) ; true - } + // return unfulfilled expected errors and unexpected diagnostics + def getMissingExpectedErrors(errorMap: HashMap[String, Integer], reporterErrors: Iterator[Diagnostic]): (List[String], List[String]) = + val unexpected, unpositioned = ListBuffer.empty[String] + // For some reason, absolute paths leak from the compiler itself... + def relativize(path: String): String = path.split(JFile.separatorChar).dropWhile(_ != "tests").mkString(JFile.separator) + def seenAt(key: String): Boolean = + errorMap.get(key) match + case null => false + case 1 => errorMap.remove(key) ; true + case n => errorMap.put(key, n - 1) ; true + def sawDiagnostic(d: Diagnostic): Unit = + d.pos.nonInlined match + case srcpos if srcpos.exists => + val key = s"${relativize(srcpos.source.file.toString)}:${srcpos.line + 1}" + if !seenAt(key) then unexpected += key + case srcpos => + if !seenAt("nopos") then unpositioned += relativize(srcpos.source.file.toString) + + reporterErrors.foreach(sawDiagnostic) + + errorMap.get("anypos") match + case n if n == unexpected.size => errorMap.remove("anypos") ; unexpected.clear() + case _ => + + (errorMap.asScala.keys.toList, (unexpected ++ unpositioned).toList) + end getMissingExpectedErrors } private final class NoCrashTest(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit summaryReport: SummaryReporting) diff --git a/tests/neg/f-interpolator-neg.check b/tests/neg/f-interpolator-neg.check new file mode 100644 index 000000000000..ea8df052589e --- /dev/null +++ b/tests/neg/f-interpolator-neg.check @@ -0,0 +1,200 @@ +-- Error: tests/neg/f-interpolator-neg.scala:4:4 ----------------------------------------------------------------------- +4 | new StringContext().f() // error + | ^^^^^^^^^^^^^^^^^^^^^ + | there are no parts +-- Error: tests/neg/f-interpolator-neg.scala:5:4 ----------------------------------------------------------------------- +5 | new StringContext("", " is ", "%2d years old").f(s) // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | too few arguments for interpolated string +-- Error: tests/neg/f-interpolator-neg.scala:6:4 ----------------------------------------------------------------------- +6 | new StringContext("", " is ", "%2d years old").f(s, d, d) // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | too many arguments for interpolated string +-- Error: tests/neg/f-interpolator-neg.scala:7:4 ----------------------------------------------------------------------- +7 | new StringContext("", "").f() // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | too few arguments for interpolated string +-- Error: tests/neg/f-interpolator-neg.scala:11:7 ---------------------------------------------------------------------- +11 | f"$s%b" // error + | ^ + | Found: (s : String), Required: Boolean, Null +-- Error: tests/neg/f-interpolator-neg.scala:12:7 ---------------------------------------------------------------------- +12 | f"$s%c" // error + | ^ + | Found: (s : String), Required: Char, Byte, Short, Int +-- Error: tests/neg/f-interpolator-neg.scala:13:7 ---------------------------------------------------------------------- +13 | f"$f%c" // error + | ^ + | Found: (f : Double), Required: Char, Byte, Short, Int +-- Error: tests/neg/f-interpolator-neg.scala:14:7 ---------------------------------------------------------------------- +14 | f"$s%x" // error + | ^ + | Found: (s : String), Required: Int, Long, Byte, Short, BigInt +-- Error: tests/neg/f-interpolator-neg.scala:15:7 ---------------------------------------------------------------------- +15 | f"$b%d" // error + | ^ + | Found: (b : Boolean), Required: Int, Long, Byte, Short, BigInt +-- Error: tests/neg/f-interpolator-neg.scala:16:7 ---------------------------------------------------------------------- +16 | f"$s%d" // error + | ^ + | Found: (s : String), Required: Int, Long, Byte, Short, BigInt +-- Error: tests/neg/f-interpolator-neg.scala:17:7 ---------------------------------------------------------------------- +17 | f"$f%o" // error + | ^ + | Found: (f : Double), Required: Int, Long, Byte, Short, BigInt +-- Error: tests/neg/f-interpolator-neg.scala:18:7 ---------------------------------------------------------------------- +18 | f"$s%e" // error + | ^ + | Found: (s : String), Required: Double, Float, BigDecimal +-- Error: tests/neg/f-interpolator-neg.scala:19:7 ---------------------------------------------------------------------- +19 | f"$b%f" // error + | ^ + | Found: (b : Boolean), Required: Double, Float, BigDecimal +-- Error: tests/neg/f-interpolator-neg.scala:20:9 ---------------------------------------------------------------------- +20 | f"$s%i" // error + | ^ + | illegal conversion character 'i' +-- Error: tests/neg/f-interpolator-neg.scala:24:9 ---------------------------------------------------------------------- +24 | f"$s%+ 0,(s" // error + | ^^^^^ + | Illegal flag '+' +-- Error: tests/neg/f-interpolator-neg.scala:25:9 ---------------------------------------------------------------------- +25 | f"$c%#+ 0,(c" // error + | ^^^^^^ + | Only '-' allowed for c conversion +-- Error: tests/neg/f-interpolator-neg.scala:26:9 ---------------------------------------------------------------------- +26 | f"$d%#d" // error + | ^ + | # not allowed for d conversion +-- Error: tests/neg/f-interpolator-neg.scala:27:9 ---------------------------------------------------------------------- +27 | f"$d%,x" // error + | ^ + | ',' only allowed for d conversion of integral types +-- Error: tests/neg/f-interpolator-neg.scala:28:9 ---------------------------------------------------------------------- +28 | f"$d%+ (x" // error + | ^^^ + | only use '+' for BigInt conversions to o, x, X +-- Error: tests/neg/f-interpolator-neg.scala:29:9 ---------------------------------------------------------------------- +29 | f"$f%,(a" // error + | ^^ + | ',' not allowed for a, A +-- Error: tests/neg/f-interpolator-neg.scala:30:9 ---------------------------------------------------------------------- +30 | f"$t%#+ 0,(tT" // error + | ^^^^^^ + | Only '-' allowed for date/time conversions +-- Error: tests/neg/f-interpolator-neg.scala:31:7 ---------------------------------------------------------------------- +31 | f"%-#+ 0,(n" // error + | ^^^^^^^ + | flags not allowed +-- Error: tests/neg/f-interpolator-neg.scala:32:7 ---------------------------------------------------------------------- +32 | f"%#+ 0,(%" // error + | ^^^^^^ + | Illegal flag '#' +-- Error: tests/neg/f-interpolator-neg.scala:36:9 ---------------------------------------------------------------------- +36 | f"$c%.2c" // error + | ^^ + | precision not allowed +-- Error: tests/neg/f-interpolator-neg.scala:37:9 ---------------------------------------------------------------------- +37 | f"$d%.2d" // error + | ^^ + | precision not allowed +-- Error: tests/neg/f-interpolator-neg.scala:38:7 ---------------------------------------------------------------------- +38 | f"%.2%" // error + | ^^ + | precision not allowed +-- Error: tests/neg/f-interpolator-neg.scala:39:7 ---------------------------------------------------------------------- +39 | f"%.2n" // error + | ^^ + | precision not allowed +-- Error: tests/neg/f-interpolator-neg.scala:40:9 ---------------------------------------------------------------------- +40 | f"$f%.2a" // error + | ^^ + | precision not allowed +-- Error: tests/neg/f-interpolator-neg.scala:41:9 ---------------------------------------------------------------------- +41 | f"$t%.2tT" // error + | ^^ + | precision not allowed +-- Error: tests/neg/f-interpolator-neg.scala:45:7 ---------------------------------------------------------------------- +45 | f"%