From d4b1da5f5ec15894606263b8f74045066280bae2 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 26 Aug 2025 00:38:52 -0700 Subject: [PATCH 1/4] Improve message for nested package missing braces Move the caret closer to the real problem; clarify the error. --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 4 ++++ compiler/src/dotty/tools/dotc/reporting/messages.scala | 4 ++-- tests/neg/i23815.check | 9 +++++++++ tests/neg/i23815.scala | 9 +++++++++ tests/neg/parser-stability-11.scala | 4 ++-- 5 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 tests/neg/i23815.check create mode 100644 tests/neg/i23815.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index c4a77f17060c..16afbe9ad433 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -4692,6 +4692,10 @@ object Parsers { def packaging(start: Int): Tree = val pkg = qualId() possibleTemplateStart() + if in.token != INDENT && in.token != LBRACE then + val prefix = "':' or " + val suffix = "\nNested package statements that are not at the beginning of the file require braces or ':' with an indented body." + syntaxErrorOrIncomplete(ExpectedTokenButFound(LBRACE, in.token, prefix = prefix, suffix = suffix), in.lastOffset) val stats = inDefScopeBraces(topStatSeq(), rewriteWithColon = true) makePackaging(start, pkg, stats) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index d3f323fa3099..880c8add64cf 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -1231,7 +1231,7 @@ extends ReferenceMsg(ForwardReferenceExtendsOverDefinitionID) { |""" } -class ExpectedTokenButFound(expected: Token, found: Token, prefix: String = "")(using Context) +class ExpectedTokenButFound(expected: Token, found: Token, prefix: String = "", suffix: String = "")(using Context) extends SyntaxMsg(ExpectedTokenButFoundID) { private def foundText = Tokens.showToken(found) @@ -1240,7 +1240,7 @@ extends SyntaxMsg(ExpectedTokenButFoundID) { val expectedText = if (Tokens.isIdentifier(expected)) "an identifier" else Tokens.showToken(expected) - i"""$prefix$expectedText expected, but $foundText found""" + i"""$prefix$expectedText expected, but $foundText found$suffix""" def explain(using Context) = if (Tokens.isIdentifier(expected) && Tokens.isKeyword(found)) diff --git a/tests/neg/i23815.check b/tests/neg/i23815.check new file mode 100644 index 000000000000..6d77f30d3820 --- /dev/null +++ b/tests/neg/i23815.check @@ -0,0 +1,9 @@ +-- [E040] Syntax Error: tests/neg/i23815.scala:7:10 -------------------------------------------------------------------- +7 |package t2 // error + | ^ + | ':' or '{' expected, but 'end of statement' found + | Nested package statements that are not at the beginning of the file require braces or ':' with an indented body. +-- [E040] Syntax Error: tests/neg/i23815.scala:10:0 -------------------------------------------------------------------- +10 | + |^ + |'}' expected, but eof found diff --git a/tests/neg/i23815.scala b/tests/neg/i23815.scala new file mode 100644 index 000000000000..4be5f0bfc886 --- /dev/null +++ b/tests/neg/i23815.scala @@ -0,0 +1,9 @@ + +package test6 + +package t1 +trait T1 + +package t2 // error +trait T2 +// anypos-error diff --git a/tests/neg/parser-stability-11.scala b/tests/neg/parser-stability-11.scala index 582bcfb88117..8b13abc78ecf 100644 --- a/tests/neg/parser-stability-11.scala +++ b/tests/neg/parser-stability-11.scala @@ -1,6 +1,6 @@ package x0 { case class x0 // error // error } -package x0 +package x0 // error class x0 // error -// error \ No newline at end of file +// anypos-error From c5bd724796ae5ceb4960b123e1c0005f6dafe208 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 26 Aug 2025 06:25:45 -0700 Subject: [PATCH 2/4] Adjust rendering of error at EOF if NL --- .../dotc/reporting/MessageRendering.scala | 22 +++++++++++++++---- tests/neg/i23815.check | 8 +++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/MessageRendering.scala b/compiler/src/dotty/tools/dotc/reporting/MessageRendering.scala index 0414bdb3c58b..a83b73be3d6d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/MessageRendering.scala +++ b/compiler/src/dotty/tools/dotc/reporting/MessageRendering.scala @@ -243,14 +243,28 @@ trait MessageRendering { given Level = Level(level) given Offset = Offset(maxLineNumber.toString.length + 2) val sb = StringBuilder() - val posString = posStr(pos, msg, diagnosticLevel(dia)) + def adjust(pos: SourcePosition): SourcePosition = + if pos.span.isSynthetic + && pos.span.isZeroExtent + && pos.span.exists + && pos.span.start == pos.source.length + && pos.source(pos.span.start - 1) == '\n' + then + pos.withSpan(pos.span.shift(-1)) + else + pos + val adjusted = adjust(pos) + val posString = posStr(adjusted, msg, diagnosticLevel(dia)) if (posString.nonEmpty) sb.append(posString).append(EOL) if (pos.exists) { val pos1 = pos.nonInlined if (pos1.exists && pos1.source.file.exists) { - val (srcBefore, srcAfter, offset) = sourceLines(pos1) - val marker = positionMarker(pos1) - val err = errorMsg(pos1, msg.message) + val readjusted = + if pos1 == pos then adjusted + else adjust(pos1) + val (srcBefore, srcAfter, offset) = sourceLines(readjusted) + val marker = positionMarker(readjusted) + val err = errorMsg(readjusted, msg.message) sb.append((srcBefore ::: marker :: err :: srcAfter).mkString(EOL)) if inlineStack.nonEmpty then diff --git a/tests/neg/i23815.check b/tests/neg/i23815.check index 6d77f30d3820..d6389c40607b 100644 --- a/tests/neg/i23815.check +++ b/tests/neg/i23815.check @@ -3,7 +3,7 @@ | ^ | ':' or '{' expected, but 'end of statement' found | Nested package statements that are not at the beginning of the file require braces or ':' with an indented body. --- [E040] Syntax Error: tests/neg/i23815.scala:10:0 -------------------------------------------------------------------- -10 | - |^ - |'}' expected, but eof found +-- [E040] Syntax Error: tests/neg/i23815.scala:9:15 -------------------------------------------------------------------- + 9 |// anypos-error + | ^ + | '}' expected, but eof found From da68463a7da051ee22c28f0a7e8cd20b84ef5e30 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 26 Aug 2025 07:53:51 -0700 Subject: [PATCH 3/4] Fewer braces --- .../dotty/tools/vulpix/ParallelTesting.scala | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index 35fbb6e5fb14..e39021c2c52f 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -978,21 +978,17 @@ trait ParallelTesting extends RunnerOrchestration { self => case null => errorMap.put(key, 1) case n => errorMap.put(key, n+1) expectedErrors += 1 - files.filter(isSourceFile).foreach { file => - Using(Source.fromFile(file, StandardCharsets.UTF_8.name)) { source => - source.getLines().zipWithIndex.foreach { case (line, lineNbr) => - comment.findAllMatchIn(line).foreach { m => + for file <- files if isSourceFile(file) do + Using.resource(Source.fromFile(file, StandardCharsets.UTF_8.name)): source => + source.getLines().zipWithIndex.foreach: (line, lineNbr) => + comment.findAllMatchIn(line).foreach: m => m.group(2) match - case prefix if m.group(1).isEmpty => - val what = Option(prefix).getOrElse("") - echo(s"Warning: ${file.getCanonicalPath}:${lineNbr}: found `//${what}error` but expected `// ${what}error`, skipping comment") - case "nopos-" => bump("nopos") - case "anypos-" => bump("anypos") - case _ => bump(s"${file.getPath}:${lineNbr+1}") - } - } - }.get - } + case prefix if m.group(1).isEmpty => + val what = Option(prefix).getOrElse("") + echo(s"Warning: ${file.getCanonicalPath}:${lineNbr}: found `//${what}error` but expected `// ${what}error`, skipping comment") + case "nopos-" => bump("nopos") + case "anypos-" => bump("anypos") + case _ => bump(s"${file.getPath}:${lineNbr+1}") (errorMap, expectedErrors) end getErrorMapAndExpectedCount From dab4466cfe67d6fa706f6090f9cee8ff95ffabcf Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 26 Aug 2025 09:50:10 -0700 Subject: [PATCH 4/4] Test rig handles NL at EOF in neg --- .../dotty/tools/vulpix/ParallelTesting.scala | 49 ++++++++++++------- tests/neg/i23815.check | 8 +-- tests/neg/i23815.scala | 2 +- tests/neg/parser-stability-11.scala | 2 +- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index e39021c2c52f..1bb3c6ae5370 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -41,8 +41,8 @@ import dotty.tools.vulpix.TestConfiguration.defaultOptions * using this, you should be running your JUnit tests **sequentially**, as the * test suite itself runs with a high level of concurrency. */ -trait ParallelTesting extends RunnerOrchestration { self => - import ParallelTesting._ +trait ParallelTesting extends RunnerOrchestration: + import ParallelTesting.* /** If the running environment supports an interactive terminal, each `Test` * will be run with a progress bar and real time feedback @@ -992,29 +992,33 @@ trait ParallelTesting extends RunnerOrchestration { self => (errorMap, expectedErrors) end getErrorMapAndExpectedCount - // return unfulfilled expected errors and unexpected diagnostics + // return unfulfilled expected errors and unexpected diagnostics. + // the errorMap of expected errors is drained and returned as unfulfilled. + // a diagnostic at EOF after NL is recorded at the preceding line, + // to obviate `anypos-error` in that case. 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 + 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) + val srcpos = d.pos.nonInlined.adjustedAtEOF + val relatively = relativize(srcpos.source.file.toString) + if srcpos.exists then + val key = s"${relatively}:${srcpos.line + 1}" + if !seenAt(key) then unexpected += key + else + if !seenAt("nopos") then unpositioned += relatively reporterErrors.foreach(sawDiagnostic) - errorMap.get("anypos") match - case n if n == unexpected.size => errorMap.remove("anypos") ; unexpected.clear() - case _ => + if errorMap.get("anypos") == unexpected.size then + errorMap.remove("anypos") + unexpected.clear() (errorMap.asScala.keys.toList, (unexpected ++ unpositioned).toList) end getMissingExpectedErrors @@ -1834,9 +1838,8 @@ trait ParallelTesting extends RunnerOrchestration { self => def isUserDebugging: Boolean = val mxBean = ManagementFactory.getRuntimeMXBean mxBean.getInputArguments.asScala.exists(_.contains("jdwp")) -} -object ParallelTesting { +object ParallelTesting: def defaultOutputDir: String = "out"+JFile.separator @@ -1851,4 +1854,14 @@ object ParallelTesting { def isBestEffortTastyFile(f: JFile): Boolean = f.getName.endsWith(".betasty") -} + extension (pos: SourcePosition) + private def adjustedAtEOF: SourcePosition = + if pos.span.isSynthetic + && pos.span.isZeroExtent + && pos.span.exists + && pos.span.start == pos.source.length + && pos.source(pos.span.start - 1) == '\n' + then + pos.withSpan(pos.span.shift(-1)) + else + pos diff --git a/tests/neg/i23815.check b/tests/neg/i23815.check index d6389c40607b..342c1ce82e0c 100644 --- a/tests/neg/i23815.check +++ b/tests/neg/i23815.check @@ -3,7 +3,7 @@ | ^ | ':' or '{' expected, but 'end of statement' found | Nested package statements that are not at the beginning of the file require braces or ':' with an indented body. --- [E040] Syntax Error: tests/neg/i23815.scala:9:15 -------------------------------------------------------------------- - 9 |// anypos-error - | ^ - | '}' expected, but eof found +-- [E040] Syntax Error: tests/neg/i23815.scala:9:8 --------------------------------------------------------------------- + 9 |// error + | ^ + | '}' expected, but eof found diff --git a/tests/neg/i23815.scala b/tests/neg/i23815.scala index 4be5f0bfc886..526c407f936b 100644 --- a/tests/neg/i23815.scala +++ b/tests/neg/i23815.scala @@ -6,4 +6,4 @@ trait T1 package t2 // error trait T2 -// anypos-error +// error diff --git a/tests/neg/parser-stability-11.scala b/tests/neg/parser-stability-11.scala index 8b13abc78ecf..49e013784ec4 100644 --- a/tests/neg/parser-stability-11.scala +++ b/tests/neg/parser-stability-11.scala @@ -3,4 +3,4 @@ case class x0 // error // error } package x0 // error class x0 // error -// anypos-error +// error