Skip to content

Backport "Warn if implicit default shadows given" to 3.3 LTS #550

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

Merged
merged 4 commits into from
Aug 13, 2025
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
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ private sealed trait WarningSettings:
private val WNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
private val WenumCommentDiscard = BooleanSetting("-Wenum-comment-discard", "Warn when a comment ambiguously assigned to multiple enum cases is discarded.")
private val WtoStringInterpolated = BooleanSetting("-Wtostring-interpolated", "Warn a standard interpolator used toString on a reference type.")
private val WrecurseWithDefault = BooleanSetting("-Wrecurse-with-default", "Warn when a method calls itself with a default argument.")
private val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
name = "-Wunused",
helpArg = "warning",
Expand Down Expand Up @@ -292,6 +293,7 @@ private sealed trait WarningSettings:
def nonUnitStatement(using Context): Boolean = allOr(WNonUnitStatement)
def enumCommentDiscard(using Context): Boolean = allOr(WenumCommentDiscard)
def toStringInterpolated(using Context): Boolean = allOr(WtoStringInterpolated)
def recurseWithDefault(using Context): Boolean = allOr(WrecurseWithDefault)
def checkInit(using Context): Boolean = allOr(YcheckInit)

/** -X "Extended" or "Advanced" settings */
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,8 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case ErasedNotPureID // errorNumber: 217
case IllegalErasedDefID // errorNumber: 218
case CannotInstantiateQuotedTypeVarID // errorNumber: 219
case DefaultShadowsGivenID // errorNumber: 220
case RecurseWithDefaultID // errorNumber: 221

def errorNumber = ordinal - 1

Expand Down
12 changes: 12 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3307,3 +3307,15 @@ final class NamedPatternNotApplicable(selectorType: Type)(using Context) extends
i"Named patterns cannot be used with $selectorType, because it is not a named tuple or case class"

override protected def explain(using Context): String = ""

final class DefaultShadowsGiven(name: Name)(using Context) extends TypeMsg(DefaultShadowsGivenID):
override protected def msg(using Context): String =
i"Argument for implicit parameter $name was supplied using a default argument."
override protected def explain(using Context): String =
"Usually the given in scope is intended, but you must specify it after explicit `using`."

final class RecurseWithDefault(name: Name)(using Context) extends TypeMsg(RecurseWithDefaultID):
override protected def msg(using Context): String =
i"Recursive call used a default argument for parameter $name."
override protected def explain(using Context): String =
"It's more explicit to pass current or modified arguments in a recursion."
13 changes: 10 additions & 3 deletions compiler/src/dotty/tools/dotc/transform/TailRec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ package transform
import ast.{TreeTypeMap, tpd}
import config.Printers.tailrec
import core.*
import Contexts.*, Flags.*, Symbols.*, Decorators.em
import Contexts.*, Flags.*, Symbols.*, Decorators.*
import Constants.Constant
import NameKinds.{TailLabelName, TailLocalName, TailTempName}
import NameKinds.{DefaultGetterName, TailLabelName, TailLocalName, TailTempName}
import StdNames.nme
import reporting.*
import transform.MegaPhase.MiniPhase
Expand Down Expand Up @@ -323,7 +323,14 @@ class TailRec extends MiniPhase {
method.matches(calledMethod) &&
enclosingClass.appliedRef.widen <:< prefix.tpe.widenDealias

if (isRecursiveCall)
if isRecursiveCall then
if ctx.settings.Whas.recurseWithDefault then
tree.args.find(_.symbol.name.is(DefaultGetterName)) match
case Some(arg) =>
val DefaultGetterName(_, index) = arg.symbol.name: @unchecked
report.warning(RecurseWithDefault(calledMethod.info.firstParamNames(index)), tree.srcPos)
case _ =>

if (inTailPosition) {
tailrec.println("Rewriting tail recursive call: " + tree.span)
rewrote = true
Expand Down
5 changes: 5 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,11 @@ trait Applications extends Compatibility {
def implicitArg = implicitArgTree(formal, appPos.span)

if !defaultArg.isEmpty then
if methodType.isImplicitMethod && ctx.mode.is(Mode.ImplicitsEnabled)
&& !inferImplicitArg(formal, appPos.span).tpe.isError
then
report.warning(DefaultShadowsGiven(methodType.paramNames(n)), appPos)

defaultArg.tpe.widen match
case _: MethodOrPoly if testOnly => matchArgs(args1, formals1, n + 1)
case _ => matchArgs(args1, addTyped(treeToArg(defaultArg)), n + 1)
Expand Down
30 changes: 16 additions & 14 deletions scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ package tasty
import scala.jdk.CollectionConverters._

import scala.quoted._
import scala.annotation.*

import NameNormalizer._
import SyntheticsSupport._
import NameNormalizer.*
import SyntheticsSupport.*

trait TypesSupport:
self: TastyParser =>
Expand Down Expand Up @@ -141,24 +142,24 @@ trait TypesSupport:
.reduceLeftOption((acc: SSignature, elem: SSignature) => acc ++ plain(", ").l ++ elem).getOrElse(List())
++ plain(")").l

def parseRefinedElem(name: String, info: TypeRepr, polyTyped: SSignature = Nil): SSignature = ( info match {
def parseRefinedElem(name: String, info: TypeRepr, polyTyped: SSignature = Nil): SSignature =
val ssig = info match
case m: MethodType => {
val paramList = getParamList(m)
keyword("def ").l ++ plain(name).l ++ polyTyped ++ paramList ++ plain(": ").l ++ inner(m.resType)
}
case t: PolyType => {
case t: PolyType =>
val paramBounds = getParamBounds(t)
val parsedMethod = parseRefinedElem(name, t.resType)
if (!paramBounds.isEmpty){
if !paramBounds.isEmpty then
parseRefinedElem(name, t.resType, plain("[").l ++ paramBounds ++ plain("]").l)
} else parseRefinedElem(name, t.resType)
}
else parseRefinedElem(name, t.resType, polyTyped = Nil)
case ByNameType(tp) => keyword("def ").l ++ plain(s"$name: ").l ++ inner(tp)
case t: TypeBounds => keyword("type ").l ++ plain(name).l ++ inner(t)
case t: TypeRef => keyword("val ").l ++ plain(s"$name: ").l ++ inner(t)
case t: TermRef => keyword("val ").l ++ plain(s"$name: ").l ++ inner(t)
case other => noSupported(s"Not supported type in refinement $info")
} ) ++ plain("; ").l

ssig ++ plain("; ").l

def parsePolyFunction(info: TypeRepr): SSignature = info match {
case t: PolyType =>
Expand Down Expand Up @@ -225,6 +226,7 @@ trait TypesSupport:
}) ++ plain("]").l

case tp @ TypeRef(qual, typeName) =>
inline def wrapping = shouldWrapInParens(inner = qual, outer = tp, isLeft = true)
qual match {
case r: RecursiveThis => tpe(s"this.$typeName").l
case t if skipPrefix(t, elideThis) =>
Expand All @@ -245,17 +247,17 @@ trait TypesSupport:
case _ => tpe(tp.typeSymbol)
case Some(_) => tpe(tp.typeSymbol)
case None =>
val sig = inParens(inner(qual)(using skipTypeSuffix = true), shouldWrapInParens(qual, tp, true))
val sig = inParens(inner(qual)(using indent = indent, skipTypeSuffix = true), wrapping)
sig ++ plain(".").l ++ tpe(tp.typeSymbol)
case _ =>
val sig = inParens(inner(qual), shouldWrapInParens(qual, tp, true))
val sig = inParens(inner(qual, skipThisTypePrefix), wrapping)
sig ++ keyword("#").l ++ tpe(tp.typeSymbol)
}

case tr @ TermRef(qual, typeName) =>
val prefix = qual match
case t if skipPrefix(t, elideThis) => Nil
case tp => inner(tp)(using skipTypeSuffix = true) ++ plain(".").l
case tp => inner(tp)(using indent = indent, skipTypeSuffix = true) ++ plain(".").l
val suffix = if skipTypeSuffix then Nil else List(plain("."), keyword("type"))
val typeSig = tr.termSymbol.tree match
case vd: ValDef if tr.termSymbol.flags.is(Flags.Module) =>
Expand All @@ -274,9 +276,9 @@ trait TypesSupport:
val spaces = " " * (indent)
val casesTexts = cases.flatMap {
case MatchCase(from, to) =>
keyword(caseSpaces + "case ").l ++ inner(from) ++ keyword(" => ").l ++ inner(to)(using indent = indent + 2) ++ plain("\n").l
keyword(caseSpaces + "case ").l ++ inner(from) ++ keyword(" => ").l ++ inner(to)(using indent = indent + 2, skipTypeSuffix = skipTypeSuffix) ++ plain("\n").l
case TypeLambda(_, _, MatchCase(from, to)) =>
keyword(caseSpaces + "case ").l ++ inner(from) ++ keyword(" => ").l ++ inner(to)(using indent = indent + 2) ++ plain("\n").l
keyword(caseSpaces + "case ").l ++ inner(from) ++ keyword(" => ").l ++ inner(to)(using indent = indent + 2, skipTypeSuffix = skipTypeSuffix) ++ plain("\n").l
}
inner(sc) ++ keyword(" match ").l ++ plain("{\n").l ++ casesTexts ++ plain(spaces + "}").l

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
-- [E172] Type Error: tests/neg/19414-desugared.scala:22:34 ------------------------------------------------------------
-- [E172] Type Error: tests/neg/i19414-desugared.scala:22:34 -----------------------------------------------------------
22 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances
| ^
|No best given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion tests/neg/19414.check → tests/neg/i19414.check
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
-- [E172] Type Error: tests/neg/19414.scala:15:34 ----------------------------------------------------------------------
-- [E172] Type Error: tests/neg/i19414.scala:15:34 ---------------------------------------------------------------------
15 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances
| ^
|No best given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/19414.scala → tests/neg/i19414.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Printer
given Writer[JsValue] = ???
given Writer[JsObject] = ???

given [B: Writer](using printer: Printer = new Printer): BodySerializer[B] = ???
given [B: Writer] => (printer: Printer = new Printer) => BodySerializer[B] = ???

def f: Unit =
summon[BodySerializer[JsObject]] // error: Ambiguous given instances
18 changes: 18 additions & 0 deletions tests/warn/i23541.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- [E220] Type Warning: tests/warn/i23541.scala:29:13 ------------------------------------------------------------------
29 | println(f(using s = "ab")) // warn uses default instead of given // prints "ab"
| ^^^^^^^^^^^^^^^^^
| Argument for implicit parameter i was supplied using a default argument.
|
| longer explanation available when compiling with `-explain`
-- [E221] Type Warning: tests/warn/i23541.scala:5:17 -------------------------------------------------------------------
5 | else fun(x - 1)(using p = p + x) // warn recurse uses default (instead of given passed down the stack)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
| Recursive call used a default argument for parameter q.
|
| longer explanation available when compiling with `-explain`
-- [E221] Type Warning: tests/warn/i23541.scala:9:17 -------------------------------------------------------------------
9 | else gun(x - 1)(p = p + x) // warn recurse uses default (value not passed down the stack)
| ^^^^^^^^^^^^^^^^^^^^^
| Recursive call used a default argument for parameter q.
|
| longer explanation available when compiling with `-explain`
30 changes: 30 additions & 0 deletions tests/warn/i23541.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//> using options -Wrecurse-with-default

def fun(x: Int)(using p: Int, q: Int = 0): Int =
if x <= 0 then p * q
else fun(x - 1)(using p = p + x) // warn recurse uses default (instead of given passed down the stack)

def gun(x: Int)(p: Int, q: Int = 0): Int =
if x <= 0 then p * q
else gun(x - 1)(p = p + x) // warn recurse uses default (value not passed down the stack)

def nested(using x: Int, y: Int = 42): Int =
def f: Int = nested(using x) // nowarn only self-recursive tailrec is eligible for warning
f

def f(using s: String, i: Int = 1): String = s * i
def g(using s: String)(using i: Int = 1): String = s * i

@main def Test =
println(fun(3)(using p = 0, q = 1))
locally:
given String = "ab"
println(f) // prints "ab"
println(g) // prints "ab"
locally:
println(f(using s = "ab")) // prints "ab"
println(g(using s = "ab")) // prints "ab"
locally:
given Int = 2
println(f(using s = "ab")) // warn uses default instead of given // prints "ab"
println(g(using s = "ab")) // prints "abab"