Skip to content

Commit cb99f7c

Browse files
cb372cb372
committed
Add special-case logic for function calls returning this.type
I copied all the relevant code over from `scala/scala:2.13.x`, so the behaviour should be equivalent to Scala 2.13.
1 parent 5e05468 commit cb99f7c

File tree

4 files changed

+202
-5
lines changed

4 files changed

+202
-5
lines changed

‎compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,160 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] =>
308308
case Block(_, expr) => forallResults(expr, p)
309309
case _ => p(tree)
310310
}
311+
312+
/** Applications in Scala can have one of the following shapes:
313+
*
314+
* 1) naked core: Ident(_) or Select(_, _) or basically anything else
315+
* 2) naked core with targs: TypeApply(core, targs) or AppliedTypeTree(core, targs)
316+
* 3) apply or several applies wrapping a core: Apply(core, _), or Apply(Apply(core, _), _), etc
317+
*
318+
* This class provides different ways to decompose applications and simplifies their analysis.
319+
*
320+
* ***Examples***
321+
* (TypeApply in the examples can be replaced with AppliedTypeTree)
322+
*
323+
* Ident(foo):
324+
* * callee = Ident(foo)
325+
* * core = Ident(foo)
326+
* * targs = Nil
327+
* * argss = Nil
328+
*
329+
* TypeApply(foo, List(targ1, targ2...))
330+
* * callee = TypeApply(foo, List(targ1, targ2...))
331+
* * core = foo
332+
* * targs = List(targ1, targ2...)
333+
* * argss = Nil
334+
*
335+
* Apply(foo, List(arg1, arg2...))
336+
* * callee = foo
337+
* * core = foo
338+
* * targs = Nil
339+
* * argss = List(List(arg1, arg2...))
340+
*
341+
* Apply(Apply(foo, List(arg21, arg22, ...)), List(arg11, arg12...))
342+
* * callee = foo
343+
* * core = foo
344+
* * targs = Nil
345+
* * argss = List(List(arg11, arg12...), List(arg21, arg22, ...))
346+
*
347+
* Apply(Apply(TypeApply(foo, List(targs1, targs2, ...)), List(arg21, arg22, ...)), List(arg11, arg12...))
348+
* * callee = TypeApply(foo, List(targs1, targs2, ...))
349+
* * core = foo
350+
* * targs = Nil
351+
* * argss = List(List(arg11, arg12...), List(arg21, arg22, ...))
352+
*/
353+
final class Applied(val tree: Tree) {
354+
/** The tree stripped of the possibly nested applications.
355+
* The original tree if it's not an application.
356+
*/
357+
def callee: Tree = {
358+
@tailrec
359+
def loop(tree: Tree): Tree = tree match {
360+
case Apply(fn, _) => loop(fn)
361+
case tree => tree
362+
}
363+
loop(tree)
364+
}
365+
366+
/** The `callee` unwrapped from type applications.
367+
* The original `callee` if it's not a type application.
368+
*/
369+
def core: Tree = callee match {
370+
case TypeApply(fn, _) => fn
371+
case AppliedTypeTree(fn, _) => fn
372+
case tree => tree
373+
}
374+
375+
/** The type arguments of the `callee`.
376+
* `Nil` if the `callee` is not a type application.
377+
*/
378+
def targs: List[Tree] = callee match {
379+
case TypeApply(_, args) => args
380+
case AppliedTypeTree(_, args) => args
381+
case _ => Nil
382+
}
383+
384+
/** (Possibly multiple lists of) value arguments of an application.
385+
* `Nil` if the `callee` is not an application.
386+
*/
387+
def argss: List[List[Tree]] = {
388+
def loop(tree: Tree): List[List[Tree]] = tree match {
389+
case Apply(fn, args) => loop(fn) :+ args
390+
case _ => Nil
391+
}
392+
loop(tree)
393+
}
394+
}
395+
396+
/** Returns a wrapper that knows how to destructure and analyze applications.
397+
*/
398+
final def dissectApplied(tree: Tree) = new Applied(tree)
399+
400+
/** Equivalent ot disectApplied(tree).core, but more efficient */
401+
@scala.annotation.tailrec
402+
final def dissectCore(tree: Tree): Tree = tree match {
403+
case TypeApply(fun, _) =>
404+
dissectCore(fun)
405+
case Apply(fun, _) =>
406+
dissectCore(fun)
407+
case t =>
408+
t
409+
}
410+
411+
/** Destructures applications into important subparts described in `Applied` class,
412+
* namely into: core, targs and argss (in the specified order).
413+
*
414+
* Trees which are not applications are also accepted. Their callee and core will
415+
* be equal to the input, while targs and argss will be Nil.
416+
*
417+
* The provided extractors don't expose all the API of the `Applied` class.
418+
* For advanced use, call `dissectApplied` explicitly and use its methods instead of pattern matching.
419+
*/
420+
object Applied {
421+
def apply(tree: Tree): Applied = new Applied(tree)
422+
423+
def unapply(applied: Applied): Some[(Tree, List[Tree], List[List[Tree]])] =
424+
Some((applied.core, applied.targs, applied.argss))
425+
426+
def unapply(tree: Tree): Some[(Tree, List[Tree], List[List[Tree]])] =
427+
unapply(dissectApplied(tree))
428+
}
429+
430+
/** Is tree an application with result `this.type`?
431+
* Accept `b.addOne(x)` and also `xs(i) += x`
432+
* where the op is an assignment operator.
433+
*/
434+
def isThisTypeResult(tree: Tree)(using Context): Boolean = tree match {
435+
case Applied(fun @ Select(receiver, op), _, argss) =>
436+
tree.tpe match {
437+
case ThisType(tref) =>
438+
tref.symbol == receiver.symbol
439+
case tref: TermRef =>
440+
tref.symbol == receiver.symbol || argss.exists(_.exists(tref.symbol == _.symbol))
441+
case _ =>
442+
def checkSingle(sym: Symbol): Boolean =
443+
(sym == receiver.symbol) || {
444+
receiver match {
445+
case Apply(_, _) => op.isOpAssignmentName // xs(i) += x
446+
case _ => receiver.symbol != null &&
447+
(receiver.symbol.isGetter || receiver.symbol.isField) // xs.addOne(x) for var xs
448+
}
449+
}
450+
@tailrec def loop(mt: Type): Boolean = mt match {
451+
case m: MethodType =>
452+
m.resType match {
453+
case ThisType(tref) => checkSingle(tref.symbol)
454+
case tref: TermRef => checkSingle(tref.symbol)
455+
case restpe => loop(restpe)
456+
}
457+
case PolyType(_, restpe) => loop(restpe)
458+
case _ => false
459+
}
460+
fun.symbol != null && loop(fun.symbol.info)
461+
}
462+
case _ =>
463+
tree.tpe.isInstanceOf[ThisType]
464+
}
311465
}
312466

313467
trait UntypedTreeInfo extends TreeInfo[Untyped] { self: Trees.Instance[Untyped] =>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3928,7 +3928,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
39283928
// so will take the code path that decides on inlining
39293929
val tree1 = adapt(tree, WildcardType, locked)
39303930
checkStatementPurity(tree1)(tree, ctx.owner)
3931-
if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value) {
3931+
if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value && !isThisTypeResult(tree)) {
39323932
report.warning(ValueDiscarding(tree.tpe), tree.srcPos)
39333933
}
39343934
return tpd.Block(tree1 :: Nil, Literal(Constant(())))

‎tests/neg/warn-value-discard.check

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
1-
-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:14:35 ----------------------------------------------
2-
14 | firstThing().map(_ => secondThing()) // error
1+
-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:15:35 ----------------------------------------------
2+
15 | firstThing().map(_ => secondThing()) // error
33
| ^^^^^^^^^^^^^
44
| discarded non-Unit value of type Either[Failed, Unit]
5-
-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:17:35 ----------------------------------------------
6-
17 | firstThing().map(_ => secondThing()) // error
5+
-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:18:35 ----------------------------------------------
6+
18 | firstThing().map(_ => secondThing()) // error
77
| ^^^^^^^^^^^^^
88
| discarded non-Unit value of type Either[Failed, Unit]
9+
-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:27:36 ----------------------------------------------
10+
27 | mutable.Set.empty[String].remove("") // error
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
| discarded non-Unit value of type Boolean
13+
-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:39:41 ----------------------------------------------
14+
39 | mutable.Set.empty[String].subtractOne("") // error
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
| discarded non-Unit value of type scala.collection.mutable.Set[String]

‎tests/neg/warn-value-discard.scala

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// scalac: -Wvalue-discard -Werror
22

33
import scala.util.{Either, Right, Left}
4+
import scala.collection.mutable
45

56
case class Failed(msg: String)
67

@@ -16,3 +17,37 @@ def singleExpr(): Either[Failed, Unit] =
1617
def block(): Either[Failed, Unit] = {
1718
firstThing().map(_ => secondThing()) // error
1819
}
20+
21+
class ValueDiscardTest:
22+
val field = mutable.Set.empty[String]
23+
24+
def remove(): Unit =
25+
// Set#remove returns a Boolean, not this.type
26+
// --> Warning
27+
mutable.Set.empty[String].remove("") // error
28+
29+
// TODO IMHO we don't need to support this,
30+
// as it's just as easy to add a @nowarn annotation as a Unit ascription
31+
//def removeAscribed(): Unit = {
32+
// mutable.Set.empty[String].remove(""): Unit // nowarn
33+
//}
34+
35+
def subtract(): Unit =
36+
// - Set#subtractOne returns this.type
37+
// - receiver is not a field or a local variable (not quite sure what you'd call it)
38+
// --> Warning
39+
mutable.Set.empty[String].subtractOne("") // error
40+
41+
def mutateLocalVariable(): Unit = {
42+
// - Set#subtractOne returns this.type
43+
// - receiver is a local variable
44+
// --> No warning
45+
val s: mutable.Set[String] = mutable.Set.empty[String]
46+
s.subtractOne("")
47+
}
48+
49+
def mutateField(): Unit =
50+
// - Set#subtractOne returns this.type
51+
// - receiver is a local variable
52+
// --> No warning
53+
field.subtractOne("")

0 commit comments

Comments
 (0)