-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix implicitNotFound message for type aliases #19343
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2821,19 +2821,27 @@ class MissingImplicitArgument( | |
val idx = paramNames.indexOf(name) | ||
if (idx >= 0) Some(i"${args(idx)}") else None | ||
"""\$\{\s*([^}\s]+)\s*\}""".r.replaceAllIn(raw, (_: Regex.Match) match | ||
case Regex.Groups(v) => quoteReplacement(translate(v).getOrElse("")).nn | ||
case Regex.Groups(v) => quoteReplacement(translate(v).getOrElse("?" + v)).nn | ||
) | ||
|
||
/** @param rawMsg Message template with variables, e.g. "Variable A is ${A}" | ||
* @param sym Symbol of the annotated type or of the method whose parameter was annotated | ||
* @param paramNames Names of type parameters to substitute with `args` in the message template | ||
* @param args Resolved type arguments to substitute for `paramNames` in the message template | ||
* @param substituteType Function substituting specific types for abstract types associated with variables, e.g A -> Int | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The documentation should be updated to include the new parameters. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done 👍 |
||
*/ | ||
def formatAnnotationMessage(rawMsg: String, sym: Symbol, substituteType: Type => Type)(using Context): String = | ||
def formatAnnotationMessage( | ||
rawMsg: String, | ||
sym: Symbol, | ||
paramNames: List[Name], | ||
args: List[Type], | ||
substituteType: Type => Type, | ||
)(using Context): String = | ||
val substitutableTypesSymbols = substitutableTypeSymbolsInScope(sym) | ||
userDefinedErrorString( | ||
rawMsg, | ||
paramNames = substitutableTypesSymbols.map(_.name.unexpandedName.toString), | ||
args = substitutableTypesSymbols.map(_.typeRef).map(substituteType) | ||
paramNames = (paramNames ::: substitutableTypesSymbols.map(_.name)).map(_.unexpandedName.toString), | ||
args = args ::: substitutableTypesSymbols.map(_.typeRef).map(substituteType) | ||
) | ||
|
||
/** Extract a user defined error message from a symbol `sym` | ||
|
@@ -2845,14 +2853,17 @@ class MissingImplicitArgument( | |
msg <- ann.argumentConstantString(0) | ||
yield msg | ||
|
||
def userDefinedImplicitNotFoundTypeMessageFor(sym: Symbol)(using Context): Option[String] = | ||
for | ||
rawMsg <- userDefinedMsg(sym, defn.ImplicitNotFoundAnnot) | ||
if Feature.migrateTo3 || sym != defn.Function1 | ||
// Don't inherit "No implicit view available..." message if subtypes of Function1 are not treated as implicit conversions anymore | ||
yield | ||
val substituteType = (_: Type).asSeenFrom(pt, sym) | ||
formatAnnotationMessage(rawMsg, sym, substituteType) | ||
def userDefinedImplicitNotFoundTypeMessageFor( | ||
sym: Symbol, | ||
params: List[ParamInfo] = Nil, | ||
args: List[Type] = Nil | ||
)(using Context): Option[String] = for | ||
rawMsg <- userDefinedMsg(sym, defn.ImplicitNotFoundAnnot) | ||
if Feature.migrateTo3 || sym != defn.Function1 | ||
// Don't inherit "No implicit view available..." message if subtypes of Function1 are not treated as implicit conversions anymore | ||
yield | ||
val paramNames = params.map(_.paramName) | ||
formatAnnotationMessage(rawMsg, sym, paramNames, args, _.asSeenFrom(pt, sym)) | ||
|
||
/** Extracting the message from a method parameter, e.g. in | ||
* | ||
|
@@ -2867,19 +2878,22 @@ class MissingImplicitArgument( | |
val targs = tpd.typeArgss(applTree).flatten | ||
val methodOwner = fn.symbol.owner | ||
val methodOwnerType = tpd.qualifier(fn).tpe | ||
val methodTypeParams = fn.symbol.paramSymss.flatten.filter(_.isType) | ||
val methodTypeParams = fn.symbol.paramSymss.flatten.withFilter(_.isType).map(_.name) | ||
val methodTypeArgs = targs.map(_.tpe) | ||
val substituteType = (_: Type).asSeenFrom(methodOwnerType, methodOwner).subst(methodTypeParams, methodTypeArgs) | ||
formatAnnotationMessage(rawMsg, sym.owner, substituteType) | ||
formatAnnotationMessage(rawMsg, sym.owner, methodTypeParams, methodTypeArgs, _.asSeenFrom(methodOwnerType, methodOwner)) | ||
|
||
def userDefinedImplicitNotFoundTypeMessage(using Context): Option[String] = | ||
def recur(tp: Type): Option[String] = tp match | ||
def recur(tp: Type, params: List[ParamInfo] = Nil, args: List[Type] = Nil): Option[String] = tp match | ||
case tp: AppliedType => | ||
val tycon = tp.typeConstructor | ||
val typeParams = if tycon.isLambdaSub then tycon.hkTypeParams else tycon.typeParams | ||
recur(tycon, typeParams ::: params, tp.args ::: args) | ||
case tp: TypeRef => | ||
val sym = tp.symbol | ||
userDefinedImplicitNotFoundTypeMessageFor(sym).orElse(recur(tp.info)) | ||
userDefinedImplicitNotFoundTypeMessageFor(tp.symbol, params, args) | ||
.orElse(recur(tp.info)) | ||
case tp: ClassInfo => | ||
tp.baseClasses.iterator | ||
.map(userDefinedImplicitNotFoundTypeMessageFor) | ||
.map(userDefinedImplicitNotFoundTypeMessageFor(_)) | ||
.find(_.isDefined).flatten | ||
case tp: TypeProxy => | ||
recur(tp.superType) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -274,12 +274,11 @@ trait ParallelTesting extends RunnerOrchestration { self => | |
*/ | ||
final def diffTest(testSource: TestSource, checkFile: JFile, actual: List[String], reporters: Seq[TestReporter], logger: LoggedRunnable) = { | ||
for (msg <- FileDiff.check(testSource.title, actual, checkFile.getPath)) { | ||
onFailure(testSource, reporters, logger, Some(msg)) | ||
|
||
if (updateCheckFiles) { | ||
FileDiff.dump(checkFile.toPath.toString, actual) | ||
echo("Updated checkfile: " + checkFile.getPath) | ||
} else { | ||
onFailure(testSource, reporters, logger, Some(msg)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Originally I left this one out to force users to re-run the tests against the new checkfile. This can be useful if there is a source of non-determinism in the output. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can revert this. Was pretty confusing initially but I might be biased by the Scala 2 experience. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can keep your change. There is no right or wrong way to do this. Your way might be easier to understand. |
||
val outFile = checkFile.toPath.resolveSibling(s"${checkFile.toPath.getFileName}.out").toString | ||
FileDiff.dump(outFile, actual) | ||
echo(FileDiff.diffMessage(checkFile.getPath, outFile)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
-- [E172] Type Error: tests/neg/i7092.scala:24:19 ---------------------------------------------------------------------- | ||
24 | summon[F[String]] // error | ||
| ^ | ||
| Not found for String | ||
-- [E172] Type Error: tests/neg/i7092.scala:25:19 ---------------------------------------------------------------------- | ||
25 | summon[G[String]] // error | ||
| ^ | ||
| Not found for String | ||
-- [E172] Type Error: tests/neg/i7092.scala:26:16 ---------------------------------------------------------------------- | ||
26 | summon[H[Int]] // error | ||
| ^ | ||
| Not found for Int, ?B | ||
-- [E172] Type Error: tests/neg/i7092.scala:27:23 ---------------------------------------------------------------------- | ||
27 | summon[H[Int][Float]] // error | ||
| ^ | ||
| Not found for Int, Float | ||
-- [E172] Type Error: tests/neg/i7092.scala:28:18 ---------------------------------------------------------------------- | ||
28 | summon[AAA[Int]] // error | ||
| ^ | ||
| Not found for Int | ||
-- [E172] Type Error: tests/neg/i7092.scala:29:25 ---------------------------------------------------------------------- | ||
29 | summon[AAA[Int][Float]] // error | ||
| ^ | ||
| Not found for Int | ||
-- [E172] Type Error: tests/neg/i7092.scala:30:19 ---------------------------------------------------------------------- | ||
30 | summon[op.F[Int]] // error | ||
| ^ | ||
| Could not find Int | ||
-- [E172] Type Error: tests/neg/i7092.scala:31:28 ---------------------------------------------------------------------- | ||
31 | summon[String =!:= String] // error | ||
| ^ | ||
| Cannot proof type inequality because types are equal: String =:= String |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import scala.annotation.implicitNotFound | ||
import scala.util.NotGiven | ||
|
||
@implicitNotFound("Not found for ${A}") | ||
type F[A] | ||
|
||
@implicitNotFound("Not found for ${A}") | ||
trait G[A] | ||
|
||
@implicitNotFound("Not found for ${A}, ${B}") | ||
type H = [A] =>> [B] =>> (A, B) | ||
|
||
@implicitNotFound("Not found for ${A}") | ||
type AAA = [A] =>> [A] =>> A | ||
|
||
object op: | ||
@implicitNotFound("Could not find ${A}") | ||
opaque type F[A] = A | ||
|
||
@implicitNotFound("Cannot proof type inequality because types are equal: ${A} =:= ${B}") | ||
type =!:=[A, B] = NotGiven[A =:= B] | ||
|
||
object Test: | ||
summon[F[String]] // error | ||
summon[G[String]] // error | ||
summon[H[Int]] // error | ||
summon[H[Int][Float]] // error | ||
summon[AAA[Int]] // error | ||
summon[AAA[Int][Float]] // error | ||
summon[op.F[Int]] // error | ||
summon[String =!:= String] // error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should maybe have a separate test case for the "?" case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this test case:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I wrote a separate test case because I think it could deserve its own tests, but that's okay like that.