Skip to content
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

Backport "Better error message when a pattern match extractor is not found." to LTS #20726

Merged
merged 4 commits into from
Jun 22, 2024
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
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 @@ -131,14 +131,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
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 @@ -2340,7 +2340,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 @@ -2353,6 +2353,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 @@ -1278,15 +1278,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
22 changes: 18 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3098,6 +3098,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 @@ -3115,10 +3131,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
case xtree => typedUnnamed(xtree)

simplify(result, pt, locked)
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)
}
}

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`
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 @@ -27,7 +27,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 @@ -38,22 +38,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) // ok
// no longer complains
Expand Down
Loading