Warn about encoded pkg obj names

Author: som-snytt

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

@@ -1270,25 +1270,29 @@ 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(
+          em"The package name `$name` will be encoded on the classpath, and can lead to undefined behaviour.",
+          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.
@@ -1859,7 +1863,7 @@ object desugar {
   /** Assuming `src` contains top-level definition, returns the name that should
    *  be using for the package object that will wrap them.
    */
-  def packageObjectName(src: SourceFile): TermName =
+  def packageObjectName(src: SourceFile, srcPos: SrcPos)(using Context): TermName =
     val fileName = src.file.name
     val sourceName = fileName.take(fileName.lastIndexOf('.'))
     (sourceName ++ str.TOPLEVEL_SUFFIX).toTermName
@@ -1890,7 +1894,7 @@ object desugar {
     val (nestedStats, topStats) = pdef.stats.partition(inPackageObject)
     if (nestedStats.isEmpty) pdef
     else {
-      val name = packageObjectName(ctx.source)
+      val name = packageObjectName(ctx.source, pdef.srcPos)
       val grouped =
         ModuleDef(name, Template(emptyConstructor, Nil, Nil, EmptyValDef, nestedStats))
           .withMods(Modifiers(Synthetic))

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

@@ -50,6 +50,7 @@ import cc.{CheckCaptures, isRetainsLike}
 import config.Config
 import config.MigrationVersion
 import transform.CheckUnused.OriginalName
+import dotty.tools.dotc.util.chaining.*
 
 import scala.annotation.{unchecked as _, *}
 import dotty.tools.dotc.util.chaining.*
@@ -3460,12 +3461,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, tree.srcPos)
+          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`
@@ -3850,8 +3858,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
   def typedStats(stats: List[untpd.Tree], exprOwner: Symbol)(using Context): (List[Tree], Context) = {
     val buf = new mutable.ListBuffer[Tree]
     var enumContexts: SimpleIdentityMap[Symbol, Context] = SimpleIdentityMap.empty
-    val initialNotNullInfos = ctx.notNullInfos
       // A map from `enum` symbols to the contexts enclosing their definitions
+    val initialNotNullInfos = ctx.notNullInfos
     @tailrec def traverse(stats: List[untpd.Tree])(using Context): (List[Tree], Context) = stats match {
       case (imp: untpd.Import) :: rest =>
         val imp1 = typed(imp)

tests/neg/i22670.check

@@ -0,0 +1,10 @@
+-- Warning: tests/neg/i22670/i22670-macro.scala:4:8 --------------------------------------------------------------------
+ 4 |package xy // 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")
+No warnings can be incurred under -Werror (or -Xfatal-warnings)

tests/neg/i22670/i22670-macro.scala

@@ -0,0 +1,12 @@
+//> using options -Werror
+// nopos-error
+//package `X-Y` // explicit package name gets a diagnostic
+package xy // 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/warn/i22670-test.scala

@@ -0,0 +1,4 @@
+//> using options -Werror
+
+// don't warn about file package object with special char in name when in empty package
+def f = 42