Skip to content

Commit

Permalink
refactor: improve Given search preference warning
Browse files Browse the repository at this point in the history
**Problem**
It wasn't clear what action users was suppose to take to suppress
the new-from-3.6 Given search preference warning.

**Solution**
1. This refactors the code to give the warning an error code E205.
2. In case of warnings, tell the user to choose -source 3.5 vs 3.7,
   or use nowarn annotation.
  • Loading branch information
eed3si9n committed Dec 11, 2024
1 parent 30b3eb9 commit 004cfc5
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case QuotedTypeMissingID // errorNumber: 202
case DeprecatedAssignmentSyntaxID // errorNumber: 203
case DeprecatedInfixNamedArgumentSyntaxID // errorNumber: 204
case GivenSearchPriorityID // errorNumber: 205

def errorNumber = ordinal - 1

Expand Down
38 changes: 38 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3361,3 +3361,41 @@ class DeprecatedInfixNamedArgumentSyntax()(using Context) extends SyntaxMsg(Depr
+ Message.rewriteNotice("This", version = SourceVersion.`3.6-migration`)

def explain(using Context) = ""

class GivenSearchPriorityWarning(
pt: Type,
cmp: Int,
prev: Int,
winner: TermRef,
loser: TermRef,
isLastOldVersion: Boolean
)(using Context) extends Message(GivenSearchPriorityID):
def kind = MessageKind.PotentialIssue
def choice(nth: String, c: Int) =
if c == 0 then "none - it's ambiguous"
else s"the $nth alternative"
val (change, whichChoice) =
if isLastOldVersion
then ("will change in the future release", "Current choice ")
else ("has changed", "Previous choice")
def warningMessage: String =
i"""Given search preference for $pt between alternatives
| ${loser}
|and
| ${winner}
|$change.
|$whichChoice : ${choice("first", prev)}
|Choice from Scala 3.7 : ${choice("second", cmp)}"""
def migrationHints: String =
i"""Suppress this warning by choosing -source 3.5, -source 3.7, or
|by using @annotation.nowarn("id=205")"""
def ambiguousNote: String =
i"""
|
|Note: $warningMessage"""
def msg(using Context) =
i"""$warningMessage
|
|$migrationHints"""

def explain(using Context) = ""
22 changes: 4 additions & 18 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -549,10 +549,10 @@ object Implicits:
/** An ambiguous implicits failure */
class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree, val nested: Boolean = false) extends SearchFailureType:

private[Implicits] var priorityChangeWarnings: List[Message] = Nil
private[Implicits] var priorityChangeWarnings: List[GivenSearchPriorityWarning] = Nil

def priorityChangeWarningNote(using Context): String =
priorityChangeWarnings.map(msg => s"\n\nNote: $msg").mkString
priorityChangeWarnings.map(_.ambiguousNote).mkString

def msg(using Context): Message =
var str1 = err.refStr(alt1.ref)
Expand Down Expand Up @@ -1312,7 +1312,7 @@ trait Implicits:
// A map that associates a priority change warning (between -source 3.6 and 3.7)
// with the candidate refs mentioned in the warning. We report the associated
// message if one of the critical candidates is part of the result of the implicit search.
val priorityChangeWarnings = mutable.ListBuffer[(/*critical:*/ List[TermRef], Message)]()
val priorityChangeWarnings = mutable.ListBuffer[(/*critical:*/ List[TermRef], GivenSearchPriorityWarning)]()

val sv = Feature.sourceVersion
val isLastOldVersion = sv.stable == SourceVersion.`3.6`
Expand Down Expand Up @@ -1353,21 +1353,7 @@ trait Implicits:
cmp match
case 1 => (alt2, alt1)
case -1 => (alt1, alt2)
def choice(nth: String, c: Int) =
if c == 0 then "none - it's ambiguous"
else s"the $nth alternative"
val (change, whichChoice) =
if isLastOldVersion
then ("will change", "Current choice ")
else ("has changed", "Previous choice")
val msg =
em"""Given search preference for $pt between alternatives
| ${loser.ref}
|and
| ${winner.ref}
|$change.
|$whichChoice : ${choice("first", prev)}
|New choice from Scala 3.7: ${choice("second", cmp)}"""
val msg = GivenSearchPriorityWarning(pt, cmp, prev, winner.ref, loser.ref, isLastOldVersion)
val critical = alt1.ref :: alt2.ref :: Nil
priorityChangeWarnings += ((critical, msg))
if isLastOldVersion then prev else cmp
Expand Down
6 changes: 3 additions & 3 deletions tests/neg/given-triangle.check
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
| (given_B : B)
|and
| (given_A : A)
|will change.
|Current choice : the first alternative
|New choice from Scala 3.7: the second alternative
|will change in the future release.
|Current choice : the first alternative
|Choice from Scala 3.7 : the second alternative
11 changes: 7 additions & 4 deletions tests/warn/i21036a.check
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
-- Warning: tests/warn/i21036a.scala:7:17 ------------------------------------------------------------------------------
-- [E205] Potential Issue Warning: tests/warn/i21036a.scala:7:17 -------------------------------------------------------
7 |val y = summon[A] // warn
| ^
| Given search preference for A between alternatives
| (b : B)
| and
| (a : A)
| will change.
| Current choice : the first alternative
| New choice from Scala 3.7: the second alternative
| will change in the future release.
| Current choice : the first alternative
| Choice from Scala 3.7 : the second alternative
|
| Suppress this warning by choosing -source 3.5, -source 3.7, or
| by using @annotation.nowarn("id=205")
9 changes: 6 additions & 3 deletions tests/warn/i21036b.check
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
-- Warning: tests/warn/i21036b.scala:7:17 ------------------------------------------------------------------------------
-- [E205] Potential Issue Warning: tests/warn/i21036b.scala:7:17 -------------------------------------------------------
7 |val y = summon[A] // warn
| ^
| Given search preference for A between alternatives
| (b : B)
| and
| (a : A)
| has changed.
| Previous choice : the first alternative
| New choice from Scala 3.7: the second alternative
| Previous choice : the first alternative
| Choice from Scala 3.7 : the second alternative
|
| Suppress this warning by choosing -source 3.5, -source 3.7, or
| by using @annotation.nowarn("id=205")
7 changes: 7 additions & 0 deletions tests/warn/i21036c.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
trait A
trait B extends A
given b: B = ???
given a: A = ???

@annotation.nowarn("id=205")
val y = summon[A] // don't warn

0 comments on commit 004cfc5

Please sign in to comment.