From 004cfc5ed76ea34245ca30c9cc3872e86f9e6d5e Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Wed, 11 Dec 2024 01:32:40 -0500 Subject: [PATCH] refactor: improve Given search preference warning **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. --- .../tools/dotc/reporting/ErrorMessageID.scala | 1 + .../dotty/tools/dotc/reporting/messages.scala | 38 +++++++++++++++++++ .../dotty/tools/dotc/typer/Implicits.scala | 22 ++--------- tests/neg/given-triangle.check | 6 +-- tests/warn/i21036a.check | 11 ++++-- tests/warn/i21036b.check | 9 +++-- tests/warn/i21036c.scala | 7 ++++ 7 files changed, 66 insertions(+), 28 deletions(-) create mode 100644 tests/warn/i21036c.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index 2c3774b59a9a..d3467fe70c52 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index b396aa62f599..75aa553827f2 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -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) = "" diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 228206d8fb1e..193cc443b4ae 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -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) @@ -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` @@ -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 diff --git a/tests/neg/given-triangle.check b/tests/neg/given-triangle.check index f366c18e78f0..8a05ed4b3129 100644 --- a/tests/neg/given-triangle.check +++ b/tests/neg/given-triangle.check @@ -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 diff --git a/tests/warn/i21036a.check b/tests/warn/i21036a.check index 63d611a6e246..6ce5b94d123f 100644 --- a/tests/warn/i21036a.check +++ b/tests/warn/i21036a.check @@ -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") diff --git a/tests/warn/i21036b.check b/tests/warn/i21036b.check index dfa19a0e9bb1..da0639438c86 100644 --- a/tests/warn/i21036b.check +++ b/tests/warn/i21036b.check @@ -1,4 +1,4 @@ --- 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 @@ -6,5 +6,8 @@ | 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") diff --git a/tests/warn/i21036c.scala b/tests/warn/i21036c.scala new file mode 100644 index 000000000000..4015cc8a84bb --- /dev/null +++ b/tests/warn/i21036c.scala @@ -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 \ No newline at end of file