Merge pull request #635 from scala/backport-lts-3.3-22707

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

Author: tgodzik

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`.
+    --------------------------------------------------------------------------------------------------------------------