Merge pull request #14831 from dotty-staging/fix-14818

Detect clashes involving renamed exports

Author: anatoliykmetyuk

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

@@ -1072,17 +1072,34 @@ class Namer { typer: Typer =>
     /** The forwarders defined by export `exp` */
     private def exportForwarders(exp: Export)(using Context): List[tpd.MemberDef] =
       val buf = new mutable.ListBuffer[tpd.MemberDef]
-      val Export(expr, selectors) = exp
+      val Export(expr, selectors0) = exp
       if expr.isEmpty then
         report.error(em"Export selector must have prefix and `.`", exp.srcPos)
         return Nil
 
+      val renamed = mutable.Set[Name]()
+      val selectors = selectors0 map {
+        case sel @ ImportSelector(imported, id @ Ident(alias), bound) if alias != nme.WILDCARD =>
+          def noAliasSelector =
+            cpy.ImportSelector(sel)(imported, EmptyTree, bound).asInstanceOf[ImportSelector]
+          if renamed.contains(alias) then
+            report.error(i"duplicate rename target", id.srcPos)
+            noAliasSelector
+          else if alias == imported.name then
+            report.warning(i"redundant rename in export", id.srcPos)
+            noAliasSelector
+          else
+            renamed += alias
+            sel
+        case sel => sel
+      }
+
       val path = typedAheadExpr(expr, AnySelectionProto)
       checkLegalExportPath(path, selectors)
       lazy val wildcardBound = importBound(selectors, isGiven = false)
       lazy val givenBound = importBound(selectors, isGiven = true)
 
-      def canForward(mbr: SingleDenotation): CanForward = {
+      def canForward(mbr: SingleDenotation, alias: TermName): CanForward = {
         import CanForward.*
         val sym = mbr.symbol
         if !sym.isAccessibleFrom(path.tpe) then
@@ -1091,6 +1108,8 @@ class Namer { typer: Typer =>
           Skip
         else if cls.derivesFrom(sym.owner) && (sym.owner == cls || !sym.is(Deferred)) then
           No(i"is already a member of $cls")
+        else if alias == mbr.name.toTermName && renamed.contains(alias) then
+          No(i"clashes with a renamed export")
         else if sym.is(Override) then
           sym.allOverriddenSymbols.find(
             other => cls.derivesFrom(other.owner) && !other.is(Deferred)
@@ -1133,7 +1152,7 @@ class Namer { typer: Typer =>
             case _ =>
               acc.reverse ::: prefss
 
-        if canForward(mbr) == CanForward.Yes then
+        if canForward(mbr, alias) == CanForward.Yes then
           val sym = mbr.symbol
           val hasDefaults = sym.hasDefaultParams // compute here to ensure HasDefaultParams and NoDefaultParams flags are set
           val forwarder =
@@ -1190,7 +1209,7 @@ class Namer { typer: Typer =>
         val mbrs = List(name, name.toTypeName).flatMap(path.tpe.member(_).alternatives)
         mbrs.foreach(addForwarder(alias, _, span))
         if buf.size == size then
-          val reason = mbrs.map(canForward).collect {
+          val reason = mbrs.map(canForward(_, alias)).collect {
             case CanForward.No(whyNot) => i"\n$path.$name cannot be exported because it $whyNot"
           }.headOption.getOrElse("")
           report.error(i"""no eligible member $name at $path$reason""", ctx.source.atSpan(span))

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

@@ -100,6 +100,15 @@ def f: c.T = ...
 
  1. Export clauses can appear in classes or they can appear at the top-level. An   export clause cannot appear as a statement in a block.
  1. If an export clause contains a wildcard or given selector, it is forbidden for its qualifier path to refer to a package. This is because it is not yet known how to safely track wildcard dependencies to a package for the purposes of incremental compilation.
+ 1. An export renaming hides un-renamed exports matching the target name. For instance, the following
+    clause would be invalid since `B` is hidden by the renaming `A as B`.
+    ```scala
+    export {A as B, B}        // error: B is hidden
+    ```
+
+ 1. Renamings in an export clause must have pairwise different target names. For instance, the following clause would be invalid:
+    ```scala
+    export {A as C, B as C}   // error: duplicate renaming
 
  1. Simple renaming exports like
     ```scala

tests/neg/i14818.check

@@ -0,0 +1,15 @@
+-- Error: tests/neg/i14818.scala:9:12 ----------------------------------------------------------------------------------
+9 |  export M.{A, B as A}  // error
+  |            ^
+  |            no eligible member A at M
+  |            M.A cannot be exported because it clashes with a renamed export
+-- [E050] Type Error: tests/neg/i14818.scala:16:10 ---------------------------------------------------------------------
+16 |  val x = b(1)  // error
+   |          ^
+   |          method b in object T3 does not take parameters
+   |
+   | longer explanation available when compiling with `-explain`
+-- Error: tests/neg/i14818.scala:19:25 ---------------------------------------------------------------------------------
+19 |  export M.{A as C, B as C} // error
+   |                         ^
+   |                         duplicate rename target

tests/neg/i14818.scala

@@ -0,0 +1,23 @@
+object M {
+  type A
+  type B
+  def a = 1
+  def b(x: Int) = x
+}
+
+object T1:
+  export M.{A, B as A}  // error
+
+object T2:
+  export M.{A as B, *}
+
+object T3:
+  export M.{a as b, *}
+  val x = b(1)  // error
+
+object T4:
+  export M.{A as C, B as C} // error
+
+object T5:
+  export M.{A as B, B as A}  // OK
+