Skip to content

Commit 4d98df9

Browse files
oderskyodersky
authored
Warn if implicit default shadows given (#23559)
Fixes #23541 When supplying a default arg for an implicit application, warn if there is an implicit value available. That is for `f(using x = v)` where a default is supplied in `f(using x = v, y = default)`. The user might think the implicit value is supplied, which is not the case for explicit `using`; this is especially confusing in a recursive call, where the user intends to "replace" an implicit parameter. Separately, behind a flag, warn if any recursive call uses a default arg instead of passing the current parameter value. `-Wrecurse-with-default`, `"Warn when a method calls itself with a default argument."`
2 parents 53bea78 + 8ba7cd3 commit 4d98df9

File tree

12 files changed

+115
-24
lines changed

12 files changed

+115
-24
lines changed

‎compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ private sealed trait WarningSettings:
167167
private val WimplausiblePatterns = BooleanSetting(WarningSetting, "Wimplausible-patterns", "Warn if comparison with a pattern value looks like it might always fail.")
168168
private val WunstableInlineAccessors = BooleanSetting(WarningSetting, "WunstableInlineAccessors", "Warn an inline methods has references to non-stable binary APIs.")
169169
private val WtoStringInterpolated = BooleanSetting(WarningSetting, "Wtostring-interpolated", "Warn a standard interpolator used toString on a reference type.")
170+
private val WrecurseWithDefault = BooleanSetting(WarningSetting, "Wrecurse-with-default", "Warn when a method calls itself with a default argument.")
170171
private val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
171172
WarningSetting,
172173
name = "Wunused",
@@ -309,6 +310,7 @@ private sealed trait WarningSettings:
309310
def implausiblePatterns(using Context): Boolean = allOr(WimplausiblePatterns)
310311
def unstableInlineAccessors(using Context): Boolean = allOr(WunstableInlineAccessors)
311312
def toStringInterpolated(using Context): Boolean = allOr(WtoStringInterpolated)
313+
def recurseWithDefault(using Context): Boolean = allOr(WrecurseWithDefault)
312314
def checkInit(using Context): Boolean = allOr(WcheckInit)
313315

314316
/** -X "Extended" or "Advanced" settings */

‎compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,8 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
233233
case ErasedNotPureID // errorNumber: 217
234234
case IllegalErasedDefID // errorNumber: 218
235235
case CannotInstantiateQuotedTypeVarID // errorNumber: 219
236+
case DefaultShadowsGivenID // errorNumber: 220
237+
case RecurseWithDefaultID // errorNumber: 221
236238

237239
def errorNumber = ordinal - 1
238240

‎compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3664,3 +3664,15 @@ final class IllegalErasedDef(sym: Symbol)(using Context) extends TypeMsg(Illegal
36643664
override protected def explain(using Context): String =
36653665
"Only non-lazy immutable values can be `erased`"
36663666
end IllegalErasedDef
3667+
3668+
final class DefaultShadowsGiven(name: Name)(using Context) extends TypeMsg(DefaultShadowsGivenID):
3669+
override protected def msg(using Context): String =
3670+
i"Argument for implicit parameter $name was supplied using a default argument."
3671+
override protected def explain(using Context): String =
3672+
"Usually the given in scope is intended, but you must specify it after explicit `using`."
3673+
3674+
final class RecurseWithDefault(name: Name)(using Context) extends TypeMsg(RecurseWithDefaultID):
3675+
override protected def msg(using Context): String =
3676+
i"Recursive call used a default argument for parameter $name."
3677+
override protected def explain(using Context): String =
3678+
"It's more explicit to pass current or modified arguments in a recursion."

‎compiler/src/dotty/tools/dotc/transform/TailRec.scala

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ package transform
44
import ast.{TreeTypeMap, tpd}
55
import config.Printers.tailrec
66
import core.*
7-
import Contexts.*, Flags.*, Symbols.*, Decorators.em
7+
import Contexts.*, Flags.*, Symbols.*, Decorators.*
88
import Constants.Constant
9-
import NameKinds.{TailLabelName, TailLocalName, TailTempName}
9+
import NameKinds.{DefaultGetterName, TailLabelName, TailLocalName, TailTempName}
1010
import StdNames.nme
1111
import reporting.*
1212
import transform.MegaPhase.MiniPhase
@@ -325,7 +325,14 @@ class TailRec extends MiniPhase {
325325
method.matches(calledMethod) &&
326326
enclosingClass.appliedRef.widen <:< prefix.tpe.widenDealias
327327

328-
if (isRecursiveCall)
328+
if isRecursiveCall then
329+
if ctx.settings.Whas.recurseWithDefault then
330+
tree.args.find(_.symbol.name.is(DefaultGetterName)) match
331+
case Some(arg) =>
332+
val DefaultGetterName(_, index) = arg.symbol.name: @unchecked
333+
report.warning(RecurseWithDefault(calledMethod.info.firstParamNames(index)), tree.srcPos)
334+
case _ =>
335+
329336
if (inTailPosition) {
330337
tailrec.println("Rewriting tail recursive call: " + tree.span)
331338
rewrote = true

‎compiler/src/dotty/tools/dotc/typer/Applications.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,11 @@ trait Applications extends Compatibility {
774774
methodType.isImplicitMethod && (applyKind == ApplyKind.Using || failEmptyArgs)
775775

776776
if !defaultArg.isEmpty then
777+
if methodType.isImplicitMethod && ctx.mode.is(Mode.ImplicitsEnabled)
778+
&& !inferImplicitArg(formal, appPos.span).tpe.isError
779+
then
780+
report.warning(DefaultShadowsGiven(methodType.paramNames(n)), appPos)
781+
777782
defaultArg.tpe.widen match
778783
case _: MethodOrPoly if testOnly => matchArgs(args1, formals1, n + 1)
779784
case _ => matchArgs(args1, addTyped(treeToArg(defaultArg)), n + 1)

‎scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package dotty.tools.scaladoc
22
package tasty
33

4-
import scala.jdk.CollectionConverters._
5-
6-
import scala.quoted._
4+
import scala.annotation.*
5+
import scala.jdk.CollectionConverters.*
6+
import scala.quoted.*
77
import scala.util.control.NonFatal
88

9-
import NameNormalizer._
10-
import SyntheticsSupport._
9+
import NameNormalizer.*
10+
import SyntheticsSupport.*
1111

1212
trait TypesSupport:
1313
self: TastyParser =>
@@ -157,24 +157,25 @@ trait TypesSupport:
157157
.reduceLeftOption((acc: SSignature, elem: SSignature) => acc ++ plain(", ").l ++ elem).getOrElse(List())
158158
++ plain(")").l
159159

160-
def parseRefinedElem(name: String, info: TypeRepr, polyTyped: SSignature = Nil): SSignature = ( info match {
160+
def parseRefinedElem(name: String, info: TypeRepr, polyTyped: SSignature = Nil): SSignature =
161+
val ssig = info match
161162
case m: MethodType => {
162163
val paramList = getParamList(m)
163164
keyword("def ").l ++ plain(name).l ++ polyTyped ++ paramList ++ plain(": ").l ++ inner(m.resType, skipThisTypePrefix)
164165
}
165-
case t: PolyType => {
166+
case t: PolyType =>
166167
val paramBounds = getParamBounds(t)
167-
val parsedMethod = parseRefinedElem(name, t.resType)
168-
if (!paramBounds.isEmpty){
168+
if !paramBounds.isEmpty then
169169
parseRefinedElem(name, t.resType, plain("[").l ++ paramBounds ++ plain("]").l)
170-
} else parseRefinedElem(name, t.resType)
171-
}
170+
else
171+
parseRefinedElem(name, t.resType, polyTyped = Nil)
172172
case ByNameType(tp) => keyword("def ").l ++ plain(s"$name: ").l ++ inner(tp, skipThisTypePrefix)
173173
case t: TypeBounds => keyword("type ").l ++ plain(name).l ++ inner(t, skipThisTypePrefix)
174174
case t: TypeRef => keyword("val ").l ++ plain(s"$name: ").l ++ inner(t, skipThisTypePrefix)
175175
case t: TermRef => keyword("val ").l ++ plain(s"$name: ").l ++ inner(t, skipThisTypePrefix)
176176
case other => noSupported(s"Not supported type in refinement $info")
177-
} ) ++ plain("; ").l
177+
178+
ssig ++ plain("; ").l
178179

179180
def parsePolyFunction(info: TypeRepr): SSignature = info match {
180181
case t: PolyType =>
@@ -255,6 +256,7 @@ trait TypesSupport:
255256
}) ++ plain("]").l
256257

257258
case tp @ TypeRef(qual, typeName) =>
259+
inline def wrapping = shouldWrapInParens(inner = qual, outer = tp, isLeft = true)
258260
qual match {
259261
case r: RecursiveThis => tpe(s"this.$typeName").l
260262
case ThisType(tr) =>
@@ -271,23 +273,28 @@ trait TypesSupport:
271273
if skipPrefix(qual, elideThis, originalOwner, skipThisTypePrefix) then
272274
tpe(tp.typeSymbol)
273275
else
274-
val sig = inParens(inner(qual, skipThisTypePrefix)(using skipTypeSuffix = true), shouldWrapInParens(qual, tp, true))
275-
sig ++ plain(".").l ++ tpe(tp.typeSymbol)
276+
val sig = inParens(
277+
inner(qual, skipThisTypePrefix)(using indent = indent, skipTypeSuffix = true), wrapping)
278+
sig
279+
++ plain(".").l
280+
++ tpe(tp.typeSymbol)
276281

277282
case t if skipPrefix(t, elideThis, originalOwner, skipThisTypePrefix) =>
278283
tpe(tp.typeSymbol)
279284
case _: TermRef | _: ParamRef =>
280285
val suffix = if tp.typeSymbol == Symbol.noSymbol then tpe(typeName).l else tpe(tp.typeSymbol)
281-
inner(qual, skipThisTypePrefix)(using skipTypeSuffix = true) ++ plain(".").l ++ suffix
286+
inner(qual, skipThisTypePrefix)(using indent = indent, skipTypeSuffix = true)
287+
++ plain(".").l
288+
++ suffix
282289
case _ =>
283-
val sig = inParens(inner(qual, skipThisTypePrefix), shouldWrapInParens(qual, tp, true))
290+
val sig = inParens(inner(qual, skipThisTypePrefix), wrapping)
284291
sig ++ keyword("#").l ++ tpe(tp.typeSymbol)
285292
}
286293

287294
case tr @ TermRef(qual, typeName) =>
288295
val prefix = qual match
289296
case t if skipPrefix(t, elideThis, originalOwner, skipThisTypePrefix) => Nil
290-
case tp => inner(tp, skipThisTypePrefix)(using skipTypeSuffix = true) ++ plain(".").l
297+
case tp => inner(tp, skipThisTypePrefix)(using indent = indent, skipTypeSuffix = true) ++ plain(".").l
291298
val suffix = if skipTypeSuffix then Nil else List(plain("."), keyword("type"))
292299
val typeSig = tr.termSymbol.tree match
293300
case vd: ValDef if tr.termSymbol.flags.is(Flags.Module) =>
@@ -306,9 +313,17 @@ trait TypesSupport:
306313
val spaces = " " * (indent)
307314
val casesTexts = cases.flatMap {
308315
case MatchCase(from, to) =>
309-
keyword(caseSpaces + "case ").l ++ inner(from, skipThisTypePrefix) ++ keyword(" => ").l ++ inner(to, skipThisTypePrefix)(using indent = indent + 2) ++ plain("\n").l
316+
keyword(caseSpaces + "case ").l
317+
++ inner(from, skipThisTypePrefix)
318+
++ keyword(" => ").l
319+
++ inner(to, skipThisTypePrefix)(using indent = indent + 2, skipTypeSuffix = skipTypeSuffix)
320+
++ plain("\n").l
310321
case TypeLambda(_, _, MatchCase(from, to)) =>
311-
keyword(caseSpaces + "case ").l ++ inner(from, skipThisTypePrefix) ++ keyword(" => ").l ++ inner(to, skipThisTypePrefix)(using indent = indent + 2) ++ plain("\n").l
322+
keyword(caseSpaces + "case ").l
323+
++ inner(from, skipThisTypePrefix)
324+
++ keyword(" => ").l
325+
++ inner(to, skipThisTypePrefix)(using indent = indent + 2, skipTypeSuffix = skipTypeSuffix)
326+
++ plain("\n").l
312327
}
313328
inner(sc, skipThisTypePrefix) ++ keyword(" match ").l ++ plain("{\n").l ++ casesTexts ++ plain(spaces + "}").l
314329

‎tests/neg/19414-desugared.check renamed to ‎tests/neg/i19414-desugared.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
-- [E172] Type Error: tests/neg/19414-desugared.scala:22:34 ------------------------------------------------------------
1+
-- [E172] Type Error: tests/neg/i19414-desugared.scala:22:34 -----------------------------------------------------------
22
22 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances
33
| ^
44
|No best given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.

‎tests/neg/19414-desugared.scala renamed to ‎tests/neg/i19414-desugared.scala

File renamed without changes.

‎tests/neg/19414.check renamed to ‎tests/neg/i19414.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
-- [E172] Type Error: tests/neg/19414.scala:15:34 ----------------------------------------------------------------------
1+
-- [E172] Type Error: tests/neg/i19414.scala:15:34 ---------------------------------------------------------------------
22
15 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances
33
| ^
44
|No best given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.

‎tests/neg/19414.scala renamed to ‎tests/neg/i19414.scala

File renamed without changes.

0 commit comments

Comments
 (0)