Merge pull request #15068 from griggt/disallow-targetname-on-toplevel-classes

Disallow `@targetName` on top-level `class`, `trait`, and `object`

Author: griggt

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

@@ -176,7 +176,8 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID]:
     MatchableWarningID,
     CannotExtendFunctionID,
     LossyWideningConstantConversionID,
-    ImplicitSearchTooLargeID
+    ImplicitSearchTooLargeID,
+    TargetNameOnTopLevelClassID
 
   def errorNumber = ordinal - 2
 

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

@@ -2540,3 +2540,24 @@ import transform.SymUtils._
           |
           |${openSearchPairs.reverse.map(showQuery)}%\n%
         """
+
+  class TargetNameOnTopLevelClass(symbol: Symbol)(using Context)
+  extends SyntaxMsg(TargetNameOnTopLevelClassID):
+    def msg = em"${hl("@targetName")} annotation not allowed on top-level $symbol"
+    def explain =
+      val annot = symbol.getAnnotation(defn.TargetNameAnnot).get
+      em"""The @targetName annotation may be applied to a top-level ${hl("val")} or ${hl("def")}, but not
+          |a top-level ${hl("class")}, ${hl("trait")}, or ${hl("object")}.
+          |
+          |This restriction is due to the naming convention of Java classfiles, whose filenames
+          |are based on the name of the class defined within. If @targetName were permitted
+          |here, the name of the classfile would be based on the target name, and the compiler
+          |could not associate that classfile with the Scala-visible defined name of the class.
+          |
+          |If your use case requires @targetName, consider wrapping $symbol in an ${hl("object")}
+          |(and possibly exporting it), as in the following example:
+          |
+          |${hl("object Wrapper:")}
+          |  $annot $symbol { ... }
+          |
+          |${hl("export")} Wrapper.${symbol.name}  ${hl("// optional")}"""

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

@@ -505,6 +505,8 @@ object Checking {
         fail(TailrecNotApplicable(sym))
       else if sym.is(Inline) then
         fail("Inline methods cannot be @tailrec")
+    if sym.hasAnnotation(defn.TargetNameAnnot) && sym.isClass && sym.isTopLevelClass then
+      fail(TargetNameOnTopLevelClass(sym))
     if (sym.hasAnnotation(defn.NativeAnnot)) {
       if (!sym.is(Deferred))
         fail(NativeMembersMayNotHaveImplementation(sym))

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -171,7 +171,7 @@ class CompilationTests {
         defaultOptions),
       compileFile("tests/neg-custom-args/i6300.scala", allowDeepSubtypes),
       compileFile("tests/neg-custom-args/infix.scala", defaultOptions.and("-source", "future", "-deprecation", "-Xfatal-warnings")),
-      compileFile("tests/neg-custom-args/missing-alpha.scala", defaultOptions.and("-Yrequire-targetName", "-Xfatal-warnings")),
+      compileFile("tests/neg-custom-args/missing-targetName.scala", defaultOptions.and("-Yrequire-targetName", "-Xfatal-warnings")),
       compileFile("tests/neg-custom-args/wildcards.scala", defaultOptions.and("-source", "future", "-deprecation", "-Xfatal-warnings")),
       compileFile("tests/neg-custom-args/indentRight.scala", defaultOptions.and("-no-indent", "-Xfatal-warnings")),
       compileDir("tests/neg-custom-args/adhoc-extension", defaultOptions.and("-source", "future", "-feature", "-Xfatal-warnings")),

docs/_docs/reference/other-new-features/targetName.md

@@ -29,7 +29,7 @@ The [`@targetName`](https://scala-lang.org/api/3.x/scala/annotation/targetName.h
     of type `String`. That string is called the _external name_ of the definition
     that's annotated.
 
- 2. A `@targetName` annotation can be given for all kinds of definitions.
+ 2. A `@targetName` annotation can be given for all kinds of definitions except a top-level `class`, `trait`, or `object`.
 
  3. The name given in a [`@targetName`](https://scala-lang.org/api/3.x/scala/annotation/targetName.html) annotation must be a legal name
     for the defined entities on the host platform.

tests/neg-custom-args/missing-alpha.scala renamed to tests/neg-custom-args/missing-targetName.scala

@@ -0,0 +1,18 @@
+import scala.annotation.targetName
+
+@targetName("B1") class A1  // error targetName on top-level class
+@targetName("B2") trait A2  // error targetName on top-level trait
+@targetName("B3") object A3 // error targetName on top-level object
+
+@targetName("bar") def foo = 42 // OK
+
+object Outer:
+  @targetName("B1") class A1  // OK
+  @targetName("B2") trait A2  // OK
+  @targetName("B3") object A3 // OK
+  @targetName("D1") class C1  // OK
+  @targetName("D2") trait C2  // OK
+  @targetName("D3") object C3 // OK
+
+export Outer.{A1, A2, A3}  // error // error // error already defined
+export Outer.{C1, C2, C3}  // OK

tests/neg/alpha-early.scala renamed to tests/neg/targetName-early.scala

@@ -4,7 +4,7 @@
 public class Test_2 {
 
   public static void main(String[] args) {
-    Alpha<String> a = new Bar();
+    Alpha<String> a = new Outer$Bar();
     assert a.foo() == 1;
     assert a.bar("a").equals("aa");
     Alpha<String> aa = a.append(a);

tests/neg/alpha-late.scala renamed to tests/neg/targetName-late.scala

@@ -0,0 +1,21 @@
+package alpha
+import annotation.targetName
+
+abstract class Alpha[T] {
+
+  def foo() = 1
+
+  @targetName("bar") def foo(x: T): T
+
+  @targetName("append") def ++ (xs: Alpha[T]): Alpha[T] = this
+
+}
+
+object Outer {
+  @targetName("Bar") class | extends Alpha[String] {
+
+    @targetName("bar") override def foo(x: String) = x ++ x
+
+    @targetName("append") override def ++ (xs: Alpha[String]) = this
+  }
+}

tests/neg/alpha-override/B_1.java renamed to tests/neg/targetName-override0/B_1.java

@@ -0,0 +1,7 @@
+package alpha
+
+object Outer {
+  @scala.annotation.targetName("A") object B {
+    def foo = 23
+  }
+}

tests/neg/alpha-override/C_2.scala renamed to tests/neg/targetName-override0/C_2.scala

@@ -3,7 +3,7 @@
 public class Test_2 {
 
   public static void main(String[] args) {
-    assert A.foo() == 23;
-    assert A$.MODULE$.foo() == 23;
+    assert Outer$.A.foo() == 23;
+    assert Outer$A$.MODULE$.foo() == 23;
   }
 }

tests/neg/alpha-override1/B_1.java renamed to tests/neg/targetName-override1/B_1.java

@@ -0,0 +1,4 @@
+package alpha
+
+object Outer:
+  @scala.annotation.targetName("A") class B(val i: Int = 1)

tests/neg/alpha-override1/D_2.scala renamed to tests/neg/targetName-override1/D_2.scala

@@ -0,0 +1,10 @@
+package alpha;
+
+public class Test_2 {
+
+  public static void main(String[] args) {
+    assert new Outer$A(101).i() == 101;
+    assert new Outer$A(Outer$A.$lessinit$greater$default$1()).i() == 101;
+    assert new Outer$A(Outer$A$.MODULE$.$lessinit$greater$default$1()).i() == 101;
+  }
+}

tests/neg/targetName-toplevel.scala

@@ -0,0 +1,5 @@
+object Outer:
+  @annotation.targetName("Bar") class Foo:
+    def it: Int = 42
+
+export Outer.Foo

tests/pos/alpha-override/A_1.scala renamed to tests/pos/targetName-override/A_1.scala

@@ -0,0 +1,3 @@
+@main def Test =
+  assert(new Foo().it == 42)
+  assert(Foo().it == 42)