Warn about encoded pkg obj names

Author: som-snytt

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

@@ -19,6 +19,7 @@ import annotation.constructorOnly
 import printing.Formatting.hl
 import config.Printers
 import parsing.Parsers
+import dotty.tools.dotc.util.chaining.*
 
 import scala.annotation.internal.sharable
 import scala.annotation.threadUnsafe
@@ -1268,25 +1269,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.
@@ -1795,10 +1800,11 @@ 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
+      .tap(nm => checkSimplePackageName(nm, srcPos.span, src, isPackageObject = true))
 
   /** Group all definitions that can't be at the toplevel in
    *  an object named `<source>$package` where `<source>` is the name of the source file.
@@ -1826,7 +1832,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

@@ -3364,7 +3364,7 @@ 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
+          val topLevelClassName = desugar.packageObjectName(ctx.source, tree.srcPos).moduleClassName
           pkg.moduleClass.info.decls.lookup(topLevelClassName).ensureCompleted()
           var stats1 = typedStats(tree.stats, pkg.moduleClass)._1
           if (!ctx.isAfterTyper)
@@ -3745,8 +3745,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,18 @@
+-- Warning: tests/neg/i22670/i22670-macro.scala:4:0 --------------------------------------------------------------------
+ 4 |import scala.quoted.*
+   |^
+   |The package name `i22670-macro$package` will be encoded on the classpath, and can lead to undefined behaviour.
+ 5 |transparent inline def foo =
+ 6 |  ${ fooImpl }
+ 7 |def fooImpl(using Quotes): Expr[Any] =
+ 8 |  Expr("hello")
+-- Warning: tests/neg/i22670/i22670-usage.scala:4:0 --------------------------------------------------------------------
+4 |val x = foo // warn
+  |^^^^^^^^^^^
+  |The package name `i22670-usage$package` will be encoded on the classpath, and can lead to undefined behaviour.
+Cyclic macro dependencies in tests/neg/i22670/i22670-usage.scala.
+Compilation stopped since no further progress can be made.
+
+To fix this, place macros in one set of files and their callers in another.
+
+Compile with -Xprint-suspension for information.

tests/neg/i22670/i22670-macro.scala

@@ -0,0 +1,10 @@
+// nopos-error
+//package `X-Y` // explicit package name gets a diagnostic
+
+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,4 @@
+
+//import `X-Y`.*
+
+val x = foo // warn