Skip to content

Commit

Permalink
Better error message when a pattern match extractor is not found. (#1…
Browse files Browse the repository at this point in the history
  • Loading branch information
odersky committed Oct 19, 2023
2 parents 2fa54e8 + eed38ec commit 3e105f2
Show file tree
Hide file tree
Showing 13 changed files with 169 additions and 37 deletions.
14 changes: 9 additions & 5 deletions compiler/src/dotty/tools/dotc/core/TypeErrors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,18 @@ end handleRecursive
* so it requires knowing denot already.
* @param denot
*/
class CyclicReference private (val denot: SymDenotation)(using Context) extends TypeError:
class CyclicReference(val denot: SymDenotation)(using Context) extends TypeError:
var inImplicitSearch: Boolean = false

override def toMessage(using Context): Message =
val cycleSym = denot.symbol
val cycleSym = denot.symbol

// cycleSym.flags would try completing denot and would fail, but here we can use flagsUNSAFE to detect flags
// set by the parser.
def unsafeFlags = cycleSym.flagsUNSAFE
def isMethod = unsafeFlags.is(Method)
def isVal = !isMethod && cycleSym.isTerm

// cycleSym.flags would try completing denot and would fail, but here we can use flagsUNSAFE to detect flags
// set by the parser.
override def toMessage(using Context): Message =
val unsafeFlags = cycleSym.flagsUNSAFE
val isMethod = unsafeFlags.is(Method)
val isVal = !isMethod && cycleSym.isTerm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case ImplausiblePatternWarningID // erorNumber: 186
case SynchronizedCallOnBoxedClassID // errorNumber: 187
case VarArgsParamCannotBeGivenID // erorNumber: 188
case ExtractorNotFoundID // errorNumber: 189

def errorNumber = ordinal - 1

Expand Down
20 changes: 19 additions & 1 deletion compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2363,7 +2363,7 @@ class ClassCannotExtendEnum(cls: Symbol, parent: Symbol)(using Context) extends
def explain(using Context) = ""
}

class NotAnExtractor(tree: untpd.Tree)(using Context) extends SyntaxMsg(NotAnExtractorID) {
class NotAnExtractor(tree: untpd.Tree)(using Context) extends PatternMatchMsg(NotAnExtractorID) {
def msg(using Context) = i"$tree cannot be used as an extractor in a pattern because it lacks an unapply or unapplySeq method"
def explain(using Context) =
i"""|An ${hl("unapply")} method should be defined in an ${hl("object")} as follow:
Expand All @@ -2376,6 +2376,24 @@ class NotAnExtractor(tree: untpd.Tree)(using Context) extends SyntaxMsg(NotAnExt
|This mechanism is used for instance in pattern ${hl("case List(x1, ..., xn)")}"""
}

class ExtractorNotFound(val name: Name)(using Context) extends NotFoundMsg(ExtractorNotFoundID):
def msg(using Context) = i"no pattern match extractor named $name was found"
def explain(using Context) =
i"""An application $name(...) in a pattern can refer to an extractor
|which defines an unapply or unapplySeq method. Example:
|
| object split:
| def unapply(x: String) =
| val (leading, trailing) = x.splitAt(x.length / 2)
| Some((leading, trailing))
|
| val split(fst, snd) = "HiHo"
|
|The extractor pattern `split(fst, snd)` defines `fst` as the first half "Hi" and
|`snd` as the second half "Ho" of the right hand side "HiHo". Case classes and
|enum cases implicitly define extractors with the name of the class or enum case.
|Here, no extractor named $name was found, so the pattern could not be typed."""

class MemberWithSameNameAsStatic()(using Context)
extends SyntaxMsg(MemberWithSameNameAsStaticID) {
def msg(using Context) = i"Companion classes cannot define members with same name as a ${hl("@static")} member"
Expand Down
19 changes: 13 additions & 6 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1295,15 +1295,22 @@ trait Applications extends Compatibility {

/** Report errors buffered in state.
* @pre state has errors to report
* If there is a single error stating that "unapply" is not a member, print
* the more informative "notAnExtractor" message instead.
* If the last reported error states that "unapply" is not a member, report
* the more informative `NotAnExtractor` message instead.
* If the last reported error states that the qualifier was not found, report
* the more informative `ExtractorNotFound` message instead.
*/
def reportErrors(tree: Tree, state: TyperState): Tree =
assert(state.reporter.hasErrors)
if saysNotFound(state, nme.unapply) then notAnExtractor(tree)
else
state.reporter.flush()
tree
if saysNotFound(state, nme.unapply) then
notAnExtractor(tree)
else qual match
case qual: Ident if saysNotFound(state, qual.name) =>
report.error(ExtractorNotFound(qual.name), tree.srcPos)
tree
case _ =>
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
24 changes: 19 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3184,6 +3184,22 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
case _ => typedUnadapted(desugar(tree, pt), pt, locked)
}

def handleTypeError(ex: TypeError): Tree = ex match
case ex: CyclicReference
if ctx.reporter.errorsReported
&& xtree.span.isZeroExtent
&& ex.isVal =>
// Don't report a "recursive val ... needs type" if errors were reported
// previously and the span of the offending tree is empty. In this case,
// it's most likely that this is desugared code, and the error message would
// be redundant and confusing.
xtree.withType(ErrorType(ex.toMessage))
case _ =>
// Use focussed sourcePos since tree might be a large definition
// and a large error span would hide all errors in interior.
// TODO: Not clear that hiding is what we want, actually
errorTree(xtree, ex, xtree.srcPos.focus)

try
val ifpt = defn.asContextFunctionType(pt)
val result =
Expand All @@ -3206,11 +3222,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
result.tpe.stripTypeVar match
case e: ErrorType if !unsimplifiedType.isErroneous => errorTree(xtree, e.msg, xtree.srcPos)
case _ => result
catch case ex: TypeError => errorTree(xtree, ex, xtree.srcPos.focus)
// use focussed sourcePos since tree might be a large definition
// and a large error span would hide all errors in interior.
// TODO: Not clear that hiding is what we want, actually
}
catch case ex: TypeError =>
handleTypeError(ex)
}
}

/** Interpolate and simplify the type of the given tree. */
Expand Down
2 changes: 1 addition & 1 deletion tests/neg-macros/i6997b.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import scala.quoted.*
inline def mcr(x: => Any): Any = ${mcrImpl('x)}

def mcrImpl(body: Expr[Any])(using ctx: Quotes): Expr[Any] = {
val '{$x: $t} = body // error // error
val '{$x: $t} = body // error
'{
val tmp: $t = $x.asInstanceOf[$t] // error // error
println(tmp)
Expand Down
8 changes: 4 additions & 4 deletions tests/neg/bad-unapplies.check
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
| both match arguments (C)
|
| longer explanation available when compiling with `-explain`
-- [E127] Syntax Error: tests/neg/bad-unapplies.scala:23:9 -------------------------------------------------------------
-- [E127] Pattern Match Error: tests/neg/bad-unapplies.scala:23:9 ------------------------------------------------------
23 | case B("2") => // error (cannot be used as an extractor)
| ^
| B cannot be used as an extractor in a pattern because it lacks an unapply or unapplySeq method
|
| longer explanation available when compiling with `-explain`
-- [E127] Syntax Error: tests/neg/bad-unapplies.scala:24:9 -------------------------------------------------------------
-- [E127] Pattern Match Error: tests/neg/bad-unapplies.scala:24:9 ------------------------------------------------------
24 | case D("2") => // error (cannot be used as an extractor)
| ^
| D cannot be used as an extractor in a pattern because it lacks an unapply or unapplySeq method
Expand All @@ -31,9 +31,9 @@
| Wrong number of argument patterns for F; expected: ()
|
| longer explanation available when compiling with `-explain`
-- [E006] Not Found Error: tests/neg/bad-unapplies.scala:27:9 ----------------------------------------------------------
-- [E189] Not Found Error: tests/neg/bad-unapplies.scala:27:9 ----------------------------------------------------------
27 | case G("2") => // error (Not found: G)
| ^
| Not found: G
| no pattern match extractor named G was found
|
| longer explanation available when compiling with `-explain`
2 changes: 1 addition & 1 deletion tests/neg/i18020.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def foo1: Unit =
// then Typer rejects "String" as an infix extractor (like ::)
// which is the second error

def foo2: Unit = // error
def foo2: Unit = // was: error, recursive value _root_ needs type
val _root_ : String = "abc" // error

// i17757
Expand Down
82 changes: 82 additions & 0 deletions tests/neg/i18684.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
-- [E189] Not Found Error: tests/neg/i18684.scala:3:6 ------------------------------------------------------------------
3 | val s(): String = "hello, world" // error
| ^
| no pattern match extractor named s was found
|---------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| An application s(...) in a pattern can refer to an extractor
| which defines an unapply or unapplySeq method. Example:
|
| object split:
| def unapply(x: String) =
| val (leading, trailing) = x.splitAt(x.length / 2)
| Some((leading, trailing))
|
| val split(fst, snd) = "HiHo"
|
| The extractor pattern `split(fst, snd)` defines `fst` as the first half "Hi" and
| `snd` as the second half "Ho" of the right hand side "HiHo". Case classes and
| enum cases implicitly define extractors with the name of the class or enum case.
| Here, no extractor named s was found, so the pattern could not be typed.
---------------------------------------------------------------------------------------------------------------------
-- [E189] Not Found Error: tests/neg/i18684.scala:5:6 ------------------------------------------------------------------
5 | val i() = 22 // error
| ^
| no pattern match extractor named i was found
|---------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| An application i(...) in a pattern can refer to an extractor
| which defines an unapply or unapplySeq method. Example:
|
| object split:
| def unapply(x: String) =
| val (leading, trailing) = x.splitAt(x.length / 2)
| Some((leading, trailing))
|
| val split(fst, snd) = "HiHo"
|
| The extractor pattern `split(fst, snd)` defines `fst` as the first half "Hi" and
| `snd` as the second half "Ho" of the right hand side "HiHo". Case classes and
| enum cases implicitly define extractors with the name of the class or enum case.
| Here, no extractor named i was found, so the pattern could not be typed.
---------------------------------------------------------------------------------------------------------------------
-- [E189] Not Found Error: tests/neg/i18684.scala:10:8 -----------------------------------------------------------------
10 | val foo() = "33" // error
| ^^^
| no pattern match extractor named foo was found
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| An application foo(...) in a pattern can refer to an extractor
| which defines an unapply or unapplySeq method. Example:
|
| object split:
| def unapply(x: String) =
| val (leading, trailing) = x.splitAt(x.length / 2)
| Some((leading, trailing))
|
| val split(fst, snd) = "HiHo"
|
| The extractor pattern `split(fst, snd)` defines `fst` as the first half "Hi" and
| `snd` as the second half "Ho" of the right hand side "HiHo". Case classes and
| enum cases implicitly define extractors with the name of the class or enum case.
| Here, no extractor named foo was found, so the pattern could not be typed.
--------------------------------------------------------------------------------------------------------------------
-- [E127] Pattern Match Error: tests/neg/i18684.scala:12:6 -------------------------------------------------------------
12 | val inner(x) = 3 // error
| ^^^^^
| Test.inner cannot be used as an extractor in a pattern because it lacks an unapply or unapplySeq method
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| An unapply method should be defined in an object as follow:
| - If it is just a test, return a Boolean. For example case even()
| - If it returns a single sub-value of type T, return an Option[T]
| - If it returns several sub-values T1,...,Tn, group them in an optional tuple Option[(T1,...,Tn)]
|
| Sometimes, the number of sub-values isn't fixed and we would like to return a sequence.
| For this reason, you can also define patterns through unapplySeq which returns Option[Seq[T]].
| This mechanism is used for instance in pattern case List(x1, ..., xn)
--------------------------------------------------------------------------------------------------------------------
12 changes: 12 additions & 0 deletions tests/neg/i18684.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//> using options -explain
object Test:
val s(): String = "hello, world" // error

val i() = 22 // error

def foo(): String = "22"

object inner:
val foo() = "33" // error

val inner(x) = 3 // error
4 changes: 2 additions & 2 deletions tests/neg/i5101.check
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-- [E006] Not Found Error: tests/neg/i5101.scala:11:11 -----------------------------------------------------------------
-- [E189] Not Found Error: tests/neg/i5101.scala:11:11 -----------------------------------------------------------------
11 | case A0(_) => // error
| ^^
| Not found: A0
| no pattern match extractor named A0 was found
|
| longer explanation available when compiling with `-explain`
16 changes: 5 additions & 11 deletions tests/neg/t5702-neg-bad-and-wild.check
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
|
| longer explanation available when compiling with `-explain`
-- [E032] Syntax Error: tests/neg/t5702-neg-bad-and-wild.scala:23:17 ---------------------------------------------------
23 | val K(ns @ _*, xx) = k // error: pattern expected // error
23 | val K(ns @ _*, xx) = k // error: pattern expected
| ^
| pattern expected
|
Expand All @@ -46,22 +46,16 @@
| x is already defined as value x
|
| Note that overloaded methods must all be defined in the same group of toplevel definitions
-- [E006] Not Found Error: tests/neg/t5702-neg-bad-and-wild.scala:12:20 ------------------------------------------------
-- [E189] Not Found Error: tests/neg/t5702-neg-bad-and-wild.scala:12:20 ------------------------------------------------
12 | case List(1, _*3,) => // error: pattern expected // error
| ^
| Not found: *
| no pattern match extractor named * was found
|
| longer explanation available when compiling with `-explain`
-- [E006] Not Found Error: tests/neg/t5702-neg-bad-and-wild.scala:13:20 ------------------------------------------------
-- [E189] Not Found Error: tests/neg/t5702-neg-bad-and-wild.scala:13:20 ------------------------------------------------
13 | case List(1, _*3:) => // error // error
| ^
| Not found: *
|
| longer explanation available when compiling with `-explain`
-- [E045] Cyclic Error: tests/neg/t5702-neg-bad-and-wild.scala:23:19 ---------------------------------------------------
23 | val K(ns @ _*, xx) = k // error: pattern expected // error
| ^
| Recursive value $1$ needs type
| no pattern match extractor named * was found
|
| longer explanation available when compiling with `-explain`
-- Warning: tests/neg/t5702-neg-bad-and-wild.scala:13:22 ---------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/t5702-neg-bad-and-wild.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ object Test {
// good syntax, bad semantics, detected by typer
//gowild.scala:14: error: star patterns must correspond with varargs parameters
val K(x @ _*) = k
val K(ns @ _*, xx) = k // error: pattern expected // error
val K(ns @ _*, xx) = k // error: pattern expected
val K(x) = k // error: x is already defined as value x
val (b, _ * ) = (5,6) // error: bad use of `*`
// no longer complains
Expand Down

0 comments on commit 3e105f2

Please sign in to comment.