Merge pull request #8584 from dotty-staging/fix-#8569

Fix #8569: Better error message for erroneous creator applications

Author: odersky

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

@@ -327,4 +327,7 @@ abstract class Reporter extends interfaces.ReporterResult {
   /** Issue all error messages in this reporter to next outer one, or make sure they are written. */
   def flush()(implicit ctx: Context): Unit =
     removeBufferedMessages.foreach(ctx.reporter.report)
+
+  /** If this reporter buffers messages, all buffered messages, otherwise Nil */
+  def pendingMessages(using Context): List[MessageContainer] = Nil
 }

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

@@ -38,5 +38,7 @@ class StoreReporter(outer: Reporter) extends Reporter {
     if (infos != null) try infos.toList finally infos = null
     else Nil
 
+  override def pendingMessages(using Context): List[MessageContainer] = infos.toList
+
   override def errorsReported: Boolean = hasErrors || (outer != null && outer.errorsReported)
 }

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

@@ -91,7 +91,7 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
     ExpectedTopLevelDefID,
     AnonymousFunctionMissingParamTypeID,
     SuperCallsNotAllowedInlineableID,
-    UNUSED1,  // not used anymore, but left so that error numbers stay the same
+    NotAPathID,
     WildcardOnTypeArgumentNotAllowedOnNewID,
     FunctionTypeNeedsNonEmptyParameterListID,
     WrongNumberOfParametersID,

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

@@ -279,7 +279,7 @@ object messages {
     }
   }
 
-  case class MissingIdent(tree: untpd.Ident, treeKind: String, name: String)(implicit ctx: Context)
+  case class MissingIdent(tree: untpd.Ident, treeKind: String, name: Name)(implicit ctx: Context)
   extends Message(MissingIdentID) {
     val kind: String = "Unbound Identifier"
     val msg: String = em"Not found: $treeKind$name"
@@ -1701,10 +1701,19 @@ object messages {
   case class SuperCallsNotAllowedInlineable(symbol: Symbol)(implicit ctx: Context)
     extends Message(SuperCallsNotAllowedInlineableID) {
     val kind: String = "Syntax"
-    val msg: String = s"Super call not allowed in inlineable $symbol"
+    val msg: String = em"Super call not allowed in inlineable $symbol"
     val explanation: String = "Method inlining prohibits calling superclass methods, as it may lead to confusion about which super is being called."
   }
 
+  case class NotAPath(tp: Type, usage: String)(using Context) extends Message(NotAPathID):
+    val kind: String = "Type"
+    val msg: String = em"$tp is not a valid $usage, since it is not an immutable path"
+    val explanation: String =
+      i"""An immutable path is
+         | - a reference to an immutable value, or
+         | - a reference to `this`, or
+         | - a selection of an immutable path with an immutable value."""
+
   case class WrongNumberOfParameters(expected: Int)(implicit ctx: Context)
     extends Message(WrongNumberOfParametersID) {
     val kind: String = "Syntax"

compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala

@@ -239,7 +239,7 @@ class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages(
 
     tag.tpe match
       case tp: TermRef =>
-        checkStable(tp, pos)
+        checkStable(tp, pos, "type witness")
         Some(ref(getQuoteTypeTags.getTagRef(tp)))
       case _: SearchFailureType =>
         levelError(sym, tp, pos,

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

@@ -29,7 +29,7 @@ import config.Printers.{overload, typr, unapp}
 import TypeApplications._
 
 import reporting.diagnostic.Message
-import reporting.diagnostic.messages.{UnexpectedPatternForSummonFrom, NotAMember}
+import reporting.diagnostic.messages.{UnexpectedPatternForSummonFrom, NotAMember, MissingIdent}
 import reporting.trace
 import Constants.{Constant, IntTag, LongTag}
 import dotty.tools.dotc.reporting.diagnostic.messages.{UnapplyInvalidReturnType, NotAnExtractor, UnapplyInvalidNumberOfArguments}
@@ -819,12 +819,13 @@ trait Applications extends Compatibility {
     tryEither {
       typedExpr(fn, pt)
     } { (result, tstate) =>
-      def fallBack = {
-        tstate.commit()
+      def fallBack(nuState: TyperState) =
+        if (nuState ne ctx.typerState) && !saysNotFound(nuState, EmptyTypeName)
+        then nuState.commit() // nuState messages are more interesting that tstate's "not found"
+        else tstate.commit()  // it's "not found" both ways; keep original message
         result
-      }
-      if (untpd.isPath(fn)) tryNew(untpd)(fn, pt, fallBack)
-      else fallBack
+      if untpd.isPath(fn) then tryNew(untpd)(fn, pt, fallBack)
+      else fallBack(ctx.typerState)
     }
 
   /** Typecheck application. Result could be an `Apply` node,
@@ -1056,6 +1057,21 @@ trait Applications extends Compatibility {
       tree
   }
 
+  /** Does `state` contain a single "NotAMember" or "MissingIdent" message as
+   *  pending error message that says `$memberName is not a member of ...` or
+   *  `Not found: $memberName`? If memberName is empty, any name will do.
+   */
+  def saysNotFound(state: TyperState, memberName: Name)(using Context): Boolean =
+    state.reporter.pendingMessages match
+      case msg :: Nil =>
+        msg.contained match
+          case NotAMember(_, name, _, _) =>
+            memberName.isEmpty || name == memberName
+          case MissingIdent(_, _, name) =>
+            memberName.isEmpty || name == memberName
+          case _ => false
+      case _ => false
+
   def typedUnApply(tree: untpd.Apply, selType: Type)(implicit ctx: Context): Tree = {
     record("typedUnApply")
     val Apply(qual, args) = tree
@@ -1073,15 +1089,10 @@ trait Applications extends Compatibility {
      */
     def reportErrors(tree: Tree, state: TyperState): Tree =
       assert(state.reporter.hasErrors)
-      val msgs = state.reporter.removeBufferedMessages
-      msgs match
-        case msg :: Nil =>
-          msg.contained match
-            case NotAMember(_, nme.unapply, _, _) => return notAnExtractor(tree)
-            case _ =>
-        case _ =>
-      msgs.foreach(ctx.reporter.report)
-      tree
+      if saysNotFound(state, nme.unapply) then notAnExtractor(tree)
+      else
+        state.reporter.flush()
+        tree
 
     /** If this is a term ref tree, try to typecheck with its type name.
      *  If this refers to a type alias, follow the alias, and if
@@ -1145,7 +1156,12 @@ trait Applications extends Compatibility {
       tryWithName(nme.unapply) {
         (sel, state) =>
           tryWithName(nme.unapplySeq) {
-            (_, _) => fallBack(sel, state)
+            (sel2, state2) =>
+              // if both fail, return unapply error, unless that is simply a
+              // "not a member", and the unapplySeq error is more refined.
+              if saysNotFound(state, nme.unapply) && !saysNotFound(state2, nme.unapplySeq)
+              then fallBack(sel2, state2)
+              else fallBack(sel, state)
           }
       }
     }

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

@@ -652,8 +652,8 @@ trait Checking {
     Checking.checkNonCyclicInherited(joint, parents, decls, posd)
 
   /** Check that type `tp` is stable. */
-  def checkStable(tp: Type, pos: SourcePosition)(implicit ctx: Context): Unit =
-    if (!tp.isStable) ctx.error(ex"$tp is not stable", pos)
+  def checkStable(tp: Type, pos: SourcePosition, kind: String)(implicit ctx: Context): Unit =
+    if !tp.isStable then ctx.error(NotAPath(tp, kind), pos)
 
   /** Check that all type members of `tp` have realizable bounds */
   def checkRealizableBounds(cls: Symbol, pos: SourcePosition)(implicit ctx: Context): Unit = {
@@ -712,7 +712,7 @@ trait Checking {
 
   /** Check that `path` is a legal prefix for an import or export clause */
   def checkLegalImportPath(path: Tree)(implicit ctx: Context): Unit = {
-    checkStable(path.tpe, path.sourcePos)
+    checkStable(path.tpe, path.sourcePos, "import prefix")
     if (!ctx.isAfterTyper) Checking.checkRealizable(path.tpe, path.posd)
   }
 
@@ -726,7 +726,7 @@ trait Checking {
     tp.underlyingClassRef(refinementOK = false) match {
       case tref: TypeRef =>
         if (traitReq && !tref.symbol.is(Trait)) ctx.error(TraitIsExpected(tref.symbol), pos)
-        if (stablePrefixReq && ctx.phase <= ctx.refchecksPhase) checkStable(tref.prefix, pos)
+        if (stablePrefixReq && ctx.phase <= ctx.refchecksPhase) checkStable(tref.prefix, pos, "class prefix")
         tp
       case _ =>
         ctx.error(ex"$tp is not a class type", pos)
@@ -1194,7 +1194,7 @@ trait NoChecking extends ReChecking {
   import tpd._
   override def checkNonCyclic(sym: Symbol, info: TypeBounds, reportErrors: Boolean)(implicit ctx: Context): Type = info
   override def checkNonCyclicInherited(joint: Type, parents: List[Type], decls: Scope, posd: Positioned)(implicit ctx: Context): Unit = ()
-  override def checkStable(tp: Type, pos: SourcePosition)(implicit ctx: Context): Unit = ()
+  override def checkStable(tp: Type, pos: SourcePosition, kind: String)(implicit ctx: Context): Unit = ()
   override def checkClassType(tp: Type, pos: SourcePosition, traitReq: Boolean, stablePrefixReq: Boolean)(implicit ctx: Context): Type = tp
   override def checkImplicitConversionDefOK(sym: Symbol)(implicit ctx: Context): Unit = ()
   override def checkImplicitConversionUseOK(sym: Symbol, posd: Positioned)(implicit ctx: Context): Unit = ()

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

@@ -104,7 +104,8 @@ class ReTyper extends Typer with ReChecking {
     fallBack
 
   override def tryNew[T >: Untyped <: Type]
-    (treesInst: Instance[T])(tree: Trees.Tree[T], pt: Type, fallBack: => Tree)(implicit ctx: Context): Tree = fallBack
+    (treesInst: Instance[T])(tree: Trees.Tree[T], pt: Type, fallBack: TyperState => Tree)(implicit ctx: Context): Tree =
+      fallBack(ctx.typerState)
 
   override def completeAnnotations(mdef: untpd.MemberDef, sym: Symbol)(implicit ctx: Context): Unit = ()
 

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

@@ -451,7 +451,7 @@ class Typer extends Namer
         // of a this(...) constructor call
         errorType(ex"$tree is not accessible from constructor arguments", tree.sourcePos)
       else
-        errorType(new MissingIdent(tree, kind, name.show), tree.sourcePos)
+        errorType(new MissingIdent(tree, kind, name), tree.sourcePos)
 
     val tree1 = ownType match {
       case ownType: NamedType =>
@@ -491,7 +491,7 @@ class Typer extends Namer
         case _ => app
       }
     case qual =>
-      if (tree.name.isTypeName) checkStable(qual.tpe, qual.sourcePos)
+      if (tree.name.isTypeName) checkStable(qual.tpe, qual.sourcePos, "type prefix")
       val select = assignType(cpy.Select(tree)(qual, tree.name), qual)
 
       val select1 = toNotNullTermRef(select, pt)
@@ -1429,7 +1429,7 @@ class Typer extends Namer
 
   def typedSingletonTypeTree(tree: untpd.SingletonTypeTree)(implicit ctx: Context): SingletonTypeTree = {
     val ref1 = typedExpr(tree.ref)
-    checkStable(ref1.tpe, tree.sourcePos)
+    checkStable(ref1.tpe, tree.sourcePos, "singleton type")
     assignType(cpy.SingletonTypeTree(tree)(ref1), ref1)
   }
 
@@ -2466,7 +2466,7 @@ class Typer extends Namer
    *  is more efficient since it re-uses the prefix `p` in typed form.
    */
   def tryNew[T >: Untyped <: Type]
-    (treesInst: Instance[T])(tree: Trees.Tree[T], pt: Type, fallBack: => Tree)(implicit ctx: Context): Tree = {
+    (treesInst: Instance[T])(tree: Trees.Tree[T], pt: Type, fallBack: TyperState => Tree)(implicit ctx: Context): Tree = {
 
     def tryWithType(tpt: untpd.Tree): Tree =
       tryEither {
@@ -2486,7 +2486,7 @@ class Typer extends Namer
             .reporting(i"try new $tree -> $result", typr)
         }
       } { (nu, nuState) =>
-        if (nu.isEmpty) fallBack
+        if (nu.isEmpty) fallBack(nuState)
         else {
           // we found a type constructor, signal the error in its application instead of the original one
           nuState.commit()
@@ -2504,7 +2504,7 @@ class Typer extends Namer
         }
         tryWithType(cpy.Select(tree)(qual1, name.toTypeName))
       case _ =>
-        fallBack
+        fallBack(ctx.typerState)
     }
   }
 
@@ -2550,7 +2550,7 @@ class Typer extends Namer
 
     def tryImplicit(fallBack: => Tree) =
       tryInsertImplicitOnQualifier(tree, pt.withContext(ctx), locked)
-        .getOrElse(tryNew(tpd)(tree, pt, fallBack))
+        .getOrElse(tryNew(tpd)(tree, pt, _ => fallBack))
 
     if (ctx.mode.is(Mode.SynthesizeExtMethodReceiver))
       // Suppress insertion of apply or implicit conversion on extension method receiver

tests/neg-custom-args/explicit-nulls/i7883.check

@@ -0,0 +1,20 @@
+-- [E134] Type Mismatch Error: tests/neg-custom-args/explicit-nulls/i7883.scala:6:11 -----------------------------------
+6 |      case r(hd, tl) => Some((hd, tl))  // error // error // error
+  |           ^
+  |           None of the overloaded alternatives of method unapplySeq in class Regex with types
+  |            (m: scala.util.matching.Regex.Match): Option[List[String]]
+  |            (c: Char): Option[List[Char]]
+  |            (s: CharSequence): Option[List[String]]
+  |           match arguments (String | UncheckedNull)
+-- [E006] Unbound Identifier Error: tests/neg-custom-args/explicit-nulls/i7883.scala:6:30 ------------------------------
+6 |      case r(hd, tl) => Some((hd, tl))  // error // error // error
+  |                              ^^
+  |                              Not found: hd
+
+longer explanation available when compiling with `-explain`
+-- [E006] Unbound Identifier Error: tests/neg-custom-args/explicit-nulls/i7883.scala:6:34 ------------------------------
+6 |      case r(hd, tl) => Some((hd, tl))  // error // error // error
+  |                                  ^^
+  |                                  Not found: tl
+
+longer explanation available when compiling with `-explain`

tests/neg-custom-args/explicit-nulls/i7883.scala

@@ -0,0 +1,9 @@
+import scala.util.matching.Regex
+
+object Test extends App {
+  def head(s: String, r: Regex): Option[(String, String)] =
+    s.trim match {
+      case r(hd, tl) => Some((hd, tl))  // error // error // error
+      case _ => None
+    }
+}

tests/neg/i8569.check

@@ -0,0 +1,12 @@
+-- [E083] Type Error: tests/neg/i8569.scala:8:2 ------------------------------------------------------------------------
+8 |  outer.Inner(2) // error
+  |  ^^^^^
+  |  (Test.outer : => Outer) is not a valid type prefix, since it is not an immutable path
+
+longer explanation available when compiling with `-explain`
+-- [E083] Type Error: tests/neg/i8569.scala:9:6 ------------------------------------------------------------------------
+9 |  new outer.Inner(2) // error
+  |      ^^^^^
+  |      (Test.outer : => Outer) is not a valid type prefix, since it is not an immutable path
+
+longer explanation available when compiling with `-explain`

tests/neg/i8569.scala

@@ -0,0 +1,10 @@
+class Outer(x: Int) {
+  class Inner(y: Int) {
+  }
+}
+object Test {
+  def outer = Outer(1)
+
+  outer.Inner(2) // error
+  new outer.Inner(2) // error
+}