Skip to content

Fix #8569: Better error message for erroneous creator applications #8584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 11 additions & 2 deletions compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
48 changes: 32 additions & 16 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
Expand Down Expand Up @@ -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 = ()
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/ReTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ()

Expand Down
14 changes: 7 additions & 7 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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()
Expand All @@ -2504,7 +2504,7 @@ class Typer extends Namer
}
tryWithType(cpy.Select(tree)(qual1, name.toTypeName))
case _ =>
fallBack
fallBack(ctx.typerState)
}
}

Expand Down Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions tests/neg-custom-args/explicit-nulls/i7883.check
Original file line number Diff line number Diff line change
@@ -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`
9 changes: 9 additions & 0 deletions tests/neg-custom-args/explicit-nulls/i7883.scala
Original file line number Diff line number Diff line change
@@ -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
}
}
12 changes: 12 additions & 0 deletions tests/neg/i8569.check
Original file line number Diff line number Diff line change
@@ -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`
10 changes: 10 additions & 0 deletions tests/neg/i8569.scala
Original file line number Diff line number Diff line change
@@ -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
}