Backport "Warn about encoded pkg obj names" to 3.3 LTS

compiler/src/dotty/tools/dotc/Driver.scala

@@ -10,6 +10,7 @@ import dotty.tools.io.AbstractFile
 import reporting.*
 import core.Decorators.*
 import config.Feature
+import util.chaining.*
 
 import scala.util.control.NonFatal
 import fromtasty.{TASTYCompiler, TastyFileUtil}
@@ -78,15 +79,26 @@ class Driver {
     MacroClassLoader.init(ictx)
     Positioned.init(using ictx)
 
-    inContext(ictx) {
+    inContext(ictx):
       if !ctx.settings.YdropComments.value || ctx.settings.YreadComments.value then
         ictx.setProperty(ContextDoc, new ContextDocstrings)
       val fileNamesOrNone = command.checkUsage(summary, sourcesRequired)(using ctx.settings)(using ctx.settingsState)
-      fileNamesOrNone.map { fileNames =>
+      fileNamesOrNone.map: fileNames =>
         val files = fileNames.map(ctx.getFile)
         (files, fromTastySetup(files))
-      }
-    }
+      .tap: _ =>
+        if !ctx.settings.Yreporter.isDefault then
+          ctx.settings.Yreporter.value match
+          case "help" =>
+          case reporterClassName =>
+            try
+              Class.forName(reporterClassName).getDeclaredConstructor().newInstance() match
+              case userReporter: Reporter =>
+                ictx.setReporter(userReporter)
+              case badReporter => report.error:
+                em"Not a reporter: ${ctx.settings.Yreporter.value}"
+            catch case e: ReflectiveOperationException => report.error:
+              em"Could not create reporter ${ctx.settings.Yreporter.value}: ${e}"
   }
 
   /** Setup extra classpath of tasty and jar files */

compiler/src/dotty/tools/dotc/ast/Desugar.scala

@@ -1024,25 +1024,27 @@ object desugar {
     else tree
   }
 
-  def checkPackageName(mdef: ModuleDef | PackageDef)(using Context): Unit =
-
-    def check(name: Name, errSpan: Span): Unit = name match
-      case name: SimpleName if !errSpan.isSynthetic && name.exists(Chars.willBeEncoded) =>
-        report.warning(em"The package name `$name` will be encoded on the classpath, and can lead to undefined behaviour.", mdef.source.atSpan(errSpan))
-      case _ =>
-
-    def loop(part: RefTree): Unit = part match
-      case part @ Ident(name) => check(name, part.span)
-      case part @ Select(qual: RefTree, name) =>
-        check(name, part.nameSpan)
-        loop(qual)
+  def checkSimplePackageName(name: Name, errSpan: Span, source: SourceFile, isPackageObject: Boolean)(using Context) =
+    if !ctx.isAfterTyper then
+      name match
+      case name: SimpleName if (isPackageObject || !errSpan.isSynthetic) && name.exists(Chars.willBeEncoded) =>
+        report.warning(EncodedPackageName(name), source.atSpan(errSpan))
       case _ =>
 
+  def checkPackageName(mdef: ModuleDef | PackageDef)(using Context): Unit =
+    def check(name: Name, errSpan: Span) = checkSimplePackageName(name, errSpan, mdef.source, isPackageObject = false)
     mdef match
-      case pdef: PackageDef => loop(pdef.pid)
-      case mdef: ModuleDef if mdef.mods.is(Package) => check(mdef.name, mdef.nameSpan)
-      case _ =>
-  end checkPackageName
+    case pdef: PackageDef =>
+      def loop(part: RefTree): Unit = part match
+        case part @ Ident(name) => check(name, part.span)
+        case part @ Select(qual: RefTree, name) =>
+          check(name, part.nameSpan)
+          loop(qual)
+        case _ =>
+      loop(pdef.pid)
+    case mdef: ModuleDef if mdef.mods.is(Package) =>
+      check(mdef.name, mdef.nameSpan)
+    case _ =>
 
   /** The normalized name of `mdef`. This means
    *   1. Check that the name does not redefine a Scala core class.

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -377,6 +377,8 @@ private sealed trait YSettings:
   val YdisableFlatCpCaching: Setting[Boolean] = BooleanSetting("-YdisableFlatCpCaching", "Do not cache flat classpath representation of classpath elements from jars across compiler instances.")
 
   val Yscala2Unpickler: Setting[String] = StringSetting("-Yscala2-unpickler", "", "Control where we may get Scala 2 symbols from. This is either \"always\", \"never\", or a classpath.", "always")
+  val YnoReporter: Setting[Boolean] = BooleanSetting("-Yno-reporter", "Diagnostics are silently consumed")
+  val Yreporter: Setting[String] = StringSetting(name = "-Yreporter", helpArg = "<class>", descr = "Specify a dotty.tools.dotc.reporting.Reporter", default = "dotty.tools.dotc.reporting.ConsoleReporter")
 
   val YnoImports: Setting[Boolean] = BooleanSetting("-Yno-imports", "Compile without importing scala.*, java.lang.*, or Predef.")
   val Yimports: Setting[List[String]] = MultiStringSetting("-Yimports", helpArg="", "Custom root imports. If set, none of scala.*, java.lang.*, or Predef.* will be imported unless explicitly included.")

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -235,6 +235,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case CannotInstantiateQuotedTypeVarID // errorNumber: 219
   case DefaultShadowsGivenID // errorNumber: 220
   case RecurseWithDefaultID // errorNumber: 221
+  case EncodedPackageNameID // errorNumber: 222
 
   def errorNumber = ordinal - 1
 

compiler/src/dotty/tools/dotc/reporting/Reporter.scala

@@ -30,6 +30,10 @@ object Reporter {
     override def report(dia: Diagnostic)(using Context): Unit = ()
   }
 
+  /** A silent reporter for testing */
+  class SilentReporter extends Reporter:
+    def doReport(dia: Diagnostic)(using Context): Unit = ()
+
   type ErrorHandler = (Diagnostic, Context) => Unit
 
   private val defaultIncompleteHandler: ErrorHandler =

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -3391,4 +3391,16 @@ final class RecurseWithDefault(name: Name)(using Context) extends TypeMsg(Recurs
   override protected def msg(using Context): String =
     i"Recursive call used a default argument for parameter $name."
   override protected def explain(using Context): String =
-    "It's more explicit to pass current or modified arguments in a recursion."
+    "It's more explicit to pass current or modified arguments in a recursion."
+
+final class EncodedPackageName(name: Name)(using Context) extends SyntaxMsg(EncodedPackageNameID):
+  override protected def msg(using Context): String =
+    i"The package name `$name` will be encoded on the classpath, and can lead to undefined behaviour."
+  override protected def explain(using Context): String =
+    i"""Tools may not handle directories whose names differ from their corresponding package names.
+       |For example, `p-q` is encoded as `p$$minusq` when written to the file system.
+       |
+       |Package objects derive their names from the file names, so files such as `myfile.test.scala`
+       |or `myfile-test.scala` can produce encoded names for the generated package objects.
+       |
+       |In this case, the name `$name` is encoded as `${name.encode}`."""

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -31,6 +31,7 @@ import EtaExpansion.etaExpand
 import TypeComparer.CompareResult
 import inlines.{Inlines, PrepareInlineable}
 import util.Spans.*
+import util.chaining.*
 import util.common.*
 import util.{Property, SimpleIdentityMap, SrcPos}
 import Applications.{tupleComponentTypes, wrapDefs, defaultArgument}
@@ -2921,12 +2922,19 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         inContext(ctx.packageContext(tree, pkg)) {
           // If it exists, complete the class containing the top-level definitions
           // before typing any statement in the package to avoid cycles as in i13669.scala
-          val topLevelClassName = desugar.packageObjectName(ctx.source).moduleClassName
-          pkg.moduleClass.info.decls.lookup(topLevelClassName).ensureCompleted()
+          val packageObjectName = desugar.packageObjectName(ctx.source)
+          val topLevelClassSymbol = pkg.moduleClass.info.decls.lookup(packageObjectName.moduleClassName)
+          topLevelClassSymbol.ensureCompleted()
           var stats1 = typedStats(tree.stats, pkg.moduleClass)._1
           if (!ctx.isAfterTyper)
             stats1 = stats1 ++ typedBlockStats(MainProxies.proxies(stats1))._1
           cpy.PackageDef(tree)(pid1, stats1).withType(pkg.termRef)
+            .tap: _ =>
+              if !ctx.isAfterTyper
+              && pkg != defn.EmptyPackageVal
+              && !topLevelClassSymbol.info.decls.filter(sym => !sym.isConstructor && !sym.is(Synthetic)).isEmpty
+              then
+                desugar.checkSimplePackageName(packageObjectName, tree.span, ctx.source, isPackageObject = true)
         }
       case _ =>
         // Package will not exist if a duplicate type has already been entered, see `tests/neg/1708.scala`

compiler/test/dotty/tools/dotc/semanticdb/SemanticdbTests.scala

@@ -143,7 +143,8 @@ class SemanticdbTests:
       "-classpath", target.toString,
       "-Xignore-scala2-macros",
       "-usejavacp",
-      "-Wunused:all"
+      "-Wunused:all",
+      "-Yreporter:dotty.tools.dotc.reporting.Reporter$SilentReporter",
     ) ++ inputFiles().map(_.toString)
     val exit = Main.process(args)
     assertFalse(s"dotc errors: ${exit.errorCount}", exit.hasErrors)

compiler/test/dotty/tools/vulpix/TestConfiguration.scala

@@ -30,6 +30,10 @@ object TestConfiguration {
     Properties.dottyLibrary
   ))
 
+  val silenceOptions = Array(
+    "-Wconf:id=E222:s", // name=EncodedPackageName don't warn about file names with hyphens
+  )
+
   val withCompilerClasspath = mkClasspath(List(
     Properties.scalaLibrary,
     Properties.scalaAsm,
@@ -63,7 +67,7 @@ object TestConfiguration {
 
   val yCheckOptions = Array("-Ycheck:all")
 
-  val commonOptions = Array("-indent") ++ checkOptions ++ noCheckOptions ++ yCheckOptions
+  val commonOptions = Array("-indent") ++ checkOptions ++ noCheckOptions ++ yCheckOptions ++ silenceOptions
   val defaultOptions = TestFlags(basicClasspath, commonOptions)
   val unindentOptions = TestFlags(basicClasspath, Array("-no-indent") ++ checkOptions ++ noCheckOptions ++ yCheckOptions)
   val withCompilerOptions =

tests/neg/i22670.check

@@ -0,0 +1,20 @@
+-- [E222] Syntax Error: tests/neg/i22670/i22670-macro.scala:4:8 --------------------------------------------------------
+ 4 |package xy // error named package required for warning
+   |^
+   |The package name `i22670-macro$package` will be encoded on the classpath, and can lead to undefined behaviour.
+ 5 |import scala.quoted.*
+ 6 |transparent inline def foo =
+ 7 |  ${ fooImpl }
+ 8 |def fooImpl(using Quotes): Expr[Any] =
+ 9 |  Expr("hello")
+   |--------------------------------------------------------------------------------------------------------------------
+   | Explanation (enabled by `-explain`)
+   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+   | Tools may not handle directories whose names differ from their corresponding package names.
+   | For example, `p-q` is encoded as `p$minusq` when written to the file system.
+   |
+   | Package objects derive their names from the file names, so files such as `myfile.test.scala`
+   | or `myfile-test.scala` can produce encoded names for the generated package objects.
+   |
+   | In this case, the name `i22670-macro$package` is encoded as `i22670$minusmacro$package`.
+    --------------------------------------------------------------------------------------------------------------------

tests/neg/i22670/i22670-macro.scala

@@ -0,0 +1,12 @@
+//> using options -Werror -Wconf:id=E222:e -explain
+
+//package `X-Y` // explicit package name gets a diagnostic
+package xy // error named package required for warning
+
+import scala.quoted.*
+
+transparent inline def foo =
+  ${ fooImpl }
+
+def fooImpl(using Quotes): Expr[Any] =
+  Expr("hello")

tests/neg/i22670/i22670-usage.scala

@@ -0,0 +1,5 @@
+
+//import `X-Y`.*
+import xy.foo
+
+val x = foo // Cyclic macro dependencies (if compilation proceeds)

tests/semanticdb/metac.expect

@@ -4681,6 +4681,7 @@ Text => empty
 Language => Scala
 Symbols => 7 entries
 Occurrences => 5 entries
+Diagnostics => 1 entries
 
 Symbols:
 exports/`exports-package$package`. => final package object exports extends Object { self: exports.type => +4 decls }
@@ -4698,6 +4699,9 @@ Occurrences:
 [2:25..2:32): Encoder <- exports/`exports-package$package`.Encoder#
 [2:34..2:39): Codec <- exports/`exports-package$package`.Codec#
 
+Diagnostics:
+[0:0..2:40): [warning] The package name `exports-package$package` will be encoded on the classpath, and can lead to undefined behaviour.
+
 expect/filename with spaces.scala
 ---------------------------------
 

tests/warn/i22670-test.scala

@@ -0,0 +1,5 @@
+//> using options -Werror -Wconf:id=E222:e
+
+// note: ensure warning is not suppressed by -Wconf, see TestConfiguration
+// don't warn about file package object with special char in name when in empty package
+def f = 42

tests/warn/symbolic-packages.check

@@ -1,16 +1,24 @@
--- Warning: tests/warn/symbolic-packages.scala:3:8 ---------------------------------------------------------------------
+-- [E222] Syntax Warning: tests/warn/symbolic-packages.scala:3:8 -------------------------------------------------------
 3 |package `with spaces` { // warn
   |        ^^^^^^^^^^^^^
   |        The package name `with spaces` will be encoded on the classpath, and can lead to undefined behaviour.
--- Warning: tests/warn/symbolic-packages.scala:7:10 --------------------------------------------------------------------
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E222] Syntax Warning: tests/warn/symbolic-packages.scala:7:10 ------------------------------------------------------
 7 |package +.* { // warn // warn
   |          ^
   |          The package name `*` will be encoded on the classpath, and can lead to undefined behaviour.
--- Warning: tests/warn/symbolic-packages.scala:7:8 ---------------------------------------------------------------------
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E222] Syntax Warning: tests/warn/symbolic-packages.scala:7:8 -------------------------------------------------------
 7 |package +.* { // warn // warn
   |        ^
   |        The package name `+` will be encoded on the classpath, and can lead to undefined behaviour.
--- Warning: tests/warn/symbolic-packages.scala:11:16 -------------------------------------------------------------------
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E222] Syntax Warning: tests/warn/symbolic-packages.scala:11:16 -----------------------------------------------------
 11 |package object `mixed_*` { // warn
    |                ^^^^^^^
    |               The package name `mixed_*` will be encoded on the classpath, and can lead to undefined behaviour.
+   |
+   | longer explanation available when compiling with `-explain`

tests/warn/symbolic-packages.scala

@@ -1,4 +1,4 @@
-
+//> using options -Wconf:id=E222:w
 
 package `with spaces` { // warn
   class Foo
@@ -10,4 +10,4 @@ package +.* { // warn // warn
 
 package object `mixed_*` { // warn
   class Baz
-}
+}

tests/warn/unused-privates.scala

@@ -294,7 +294,7 @@ private val printed = false // TODO warn
 package locked:
   private[locked] def locker(): Unit = () // TODO warn as we cannot distinguish unqualified private at top level
   package basement:
-    private[locked] def shackle(): Unit = () // no warn as it is not top level at boundary
+    private[locked] def unlock(): Unit = () // no warn as it is not top level at boundary
 
 object `i19998 refinement`:
   trait Foo {