From 9dc140460a4f341b9d09acc0db2a25d592c7cc18 Mon Sep 17 00:00:00 2001 From: Chris Birchall Date: Mon, 5 Sep 2022 19:42:44 +0100 Subject: [PATCH 1/7] Implement -Wvalue-discard warning Warn when an expression `e` with non-Unit type is adapted by embedding it into a block `{ e; () }` because the expected type is Unit. --- .../tools/dotc/config/ScalaSettings.scala | 1 + .../tools/dotc/reporting/ErrorMessageID.scala | 1 + .../dotty/tools/dotc/reporting/messages.scala | 6 ++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 3 +++ tests/neg/warn-value-discard.scala | 18 ++++++++++++++++++ 5 files changed, 29 insertions(+) create mode 100644 tests/neg/warn-value-discard.scala diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 9e34f8d726b5..003228dc2bdf 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -156,6 +156,7 @@ private sealed trait WarningSettings: self: SettingGroup => val Whelp: Setting[Boolean] = BooleanSetting("-W", "Print a synopsis of warning options.") val XfatalWarnings: Setting[Boolean] = BooleanSetting("-Werror", "Fail the compilation if there are any warnings.", aliases = List("-Xfatal-warnings")) + val YwarnValueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.", aliases = List("-Ywarn-value-discard")) val Wunused: Setting[List[String]] = MultiChoiceSetting( name = "-Wunused", diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index 7b22eb77e90e..4181e7a071a0 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -184,6 +184,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe case ImplicitSearchTooLargeID // errorNumber: 168 case TargetNameOnTopLevelClassID // errorNumber: 169 case NotClassTypeID // errorNumber 170 + case ValueDiscardingID // errorNumber 171 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 a3af4c1b2582..1e02c0edb91f 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2535,3 +2535,9 @@ import cc.CaptureSet.IdentityCaptRefMap extends TypeMsg(NotClassTypeID), ShowMatchTrace(tp): def msg = ex"$tp is not a class type" def explain = "" + + class ValueDiscarding(tp: Type)(using Context) + extends Message(ValueDiscardingID): + def kind = MessageKind.PotentialIssue + def msg = em"discarded non-Unit value of type $tp" + def explain = "" diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index b05ba9d1ca43..a53e724a1542 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3905,6 +3905,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // so will take the code path that decides on inlining val tree1 = adapt(tree, WildcardType, locked) checkStatementPurity(tree1)(tree, ctx.owner) + if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.YwarnValueDiscard.value) { + report.warning(ValueDiscarding(tree.tpe), tree.srcPos) + } return tpd.Block(tree1 :: Nil, Literal(Constant(()))) } diff --git a/tests/neg/warn-value-discard.scala b/tests/neg/warn-value-discard.scala new file mode 100644 index 000000000000..c82cc13d5062 --- /dev/null +++ b/tests/neg/warn-value-discard.scala @@ -0,0 +1,18 @@ +// scalac: -Wvalue-discard -Werror + +import scala.util.{Either, Right, Left} + +case class Failed(msg: String) + +def doSomething(): Either[Failed, Unit] = + Right(()) + +def log(): Either[Failed, Unit] = + Left(Failed("whoops you should have flatMapped me")) + +def singleExpr(): Either[Failed, Unit] = + doSomething().map(_ => log()) // error + +def block(): Either[Failed, Unit] = { + doSomething().map(_ => log()) // error +} From fae048cb97e13400fddd3be8bca3ba85190cbdee Mon Sep 17 00:00:00 2001 From: Chris Birchall Date: Mon, 5 Sep 2022 19:58:59 +0100 Subject: [PATCH 2/7] Add a .check file --- tests/neg/warn-value-discard.check | 8 ++++++++ tests/neg/warn-value-discard.scala | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 tests/neg/warn-value-discard.check diff --git a/tests/neg/warn-value-discard.check b/tests/neg/warn-value-discard.check new file mode 100644 index 000000000000..a9b39f187b32 --- /dev/null +++ b/tests/neg/warn-value-discard.check @@ -0,0 +1,8 @@ +-- [E171] Potential Issue Error: tests/neg/warn-value-discard.scala:14:35 ---------------------------------------------- +14 | firstThing().map(_ => secondThing()) // error + | ^^^^^^^^^^^^^ + | discarded non-Unit value of type Either[Failed, Unit] +-- [E171] Potential Issue Error: tests/neg/warn-value-discard.scala:17:35 ---------------------------------------------- +17 | firstThing().map(_ => secondThing()) // error + | ^^^^^^^^^^^^^ + | discarded non-Unit value of type Either[Failed, Unit] diff --git a/tests/neg/warn-value-discard.scala b/tests/neg/warn-value-discard.scala index c82cc13d5062..7e08bbf69086 100644 --- a/tests/neg/warn-value-discard.scala +++ b/tests/neg/warn-value-discard.scala @@ -4,15 +4,15 @@ import scala.util.{Either, Right, Left} case class Failed(msg: String) -def doSomething(): Either[Failed, Unit] = +def firstThing(): Either[Failed, Unit] = Right(()) -def log(): Either[Failed, Unit] = +def secondThing(): Either[Failed, Unit] = Left(Failed("whoops you should have flatMapped me")) def singleExpr(): Either[Failed, Unit] = - doSomething().map(_ => log()) // error + firstThing().map(_ => secondThing()) // error def block(): Either[Failed, Unit] = { - doSomething().map(_ => log()) // error + firstThing().map(_ => secondThing()) // error } From 5e05468dea19efcc257c7a2430defb5f77c811b0 Mon Sep 17 00:00:00 2001 From: Chris Birchall Date: Tue, 22 Nov 2022 20:24:55 +0000 Subject: [PATCH 3/7] Remove -Ywarn-value-discard alias --- compiler/src/dotty/tools/dotc/config/ScalaSettings.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 09fbba050bf9..ade5388cb80e 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -157,7 +157,7 @@ private sealed trait WarningSettings: self: SettingGroup => val Whelp: Setting[Boolean] = BooleanSetting("-W", "Print a synopsis of warning options.") val XfatalWarnings: Setting[Boolean] = BooleanSetting("-Werror", "Fail the compilation if there are any warnings.", aliases = List("-Xfatal-warnings")) - val YwarnValueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.", aliases = List("-Ywarn-value-discard")) + val WvalueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.") val Wunused: Setting[List[String]] = MultiChoiceSetting( name = "-Wunused", diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 357714649ef4..5e5f9b324e27 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3928,7 +3928,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // so will take the code path that decides on inlining val tree1 = adapt(tree, WildcardType, locked) checkStatementPurity(tree1)(tree, ctx.owner) - if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.YwarnValueDiscard.value) { + if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value) { report.warning(ValueDiscarding(tree.tpe), tree.srcPos) } return tpd.Block(tree1 :: Nil, Literal(Constant(()))) From cb99f7c41bc2822d9175b53d5c472d58ae5effab Mon Sep 17 00:00:00 2001 From: Chris Birchall Date: Wed, 23 Nov 2022 11:42:02 +0000 Subject: [PATCH 4/7] 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. --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 154 ++++++++++++++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- tests/neg/warn-value-discard.check | 16 +- tests/neg/warn-value-discard.scala | 35 ++++ 4 files changed, 202 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index d17bfd0f7564..1b00209448c0 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -308,6 +308,160 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => case Block(_, expr) => forallResults(expr, p) case _ => p(tree) } + + /** Applications in Scala can have one of the following shapes: + * + * 1) naked core: Ident(_) or Select(_, _) or basically anything else + * 2) naked core with targs: TypeApply(core, targs) or AppliedTypeTree(core, targs) + * 3) apply or several applies wrapping a core: Apply(core, _), or Apply(Apply(core, _), _), etc + * + * This class provides different ways to decompose applications and simplifies their analysis. + * + * ***Examples*** + * (TypeApply in the examples can be replaced with AppliedTypeTree) + * + * Ident(foo): + * * callee = Ident(foo) + * * core = Ident(foo) + * * targs = Nil + * * argss = Nil + * + * TypeApply(foo, List(targ1, targ2...)) + * * callee = TypeApply(foo, List(targ1, targ2...)) + * * core = foo + * * targs = List(targ1, targ2...) + * * argss = Nil + * + * Apply(foo, List(arg1, arg2...)) + * * callee = foo + * * core = foo + * * targs = Nil + * * argss = List(List(arg1, arg2...)) + * + * Apply(Apply(foo, List(arg21, arg22, ...)), List(arg11, arg12...)) + * * callee = foo + * * core = foo + * * targs = Nil + * * argss = List(List(arg11, arg12...), List(arg21, arg22, ...)) + * + * Apply(Apply(TypeApply(foo, List(targs1, targs2, ...)), List(arg21, arg22, ...)), List(arg11, arg12...)) + * * callee = TypeApply(foo, List(targs1, targs2, ...)) + * * core = foo + * * targs = Nil + * * argss = List(List(arg11, arg12...), List(arg21, arg22, ...)) + */ + final class Applied(val tree: Tree) { + /** The tree stripped of the possibly nested applications. + * The original tree if it's not an application. + */ + def callee: Tree = { + @tailrec + def loop(tree: Tree): Tree = tree match { + case Apply(fn, _) => loop(fn) + case tree => tree + } + loop(tree) + } + + /** The `callee` unwrapped from type applications. + * The original `callee` if it's not a type application. + */ + def core: Tree = callee match { + case TypeApply(fn, _) => fn + case AppliedTypeTree(fn, _) => fn + case tree => tree + } + + /** The type arguments of the `callee`. + * `Nil` if the `callee` is not a type application. + */ + def targs: List[Tree] = callee match { + case TypeApply(_, args) => args + case AppliedTypeTree(_, args) => args + case _ => Nil + } + + /** (Possibly multiple lists of) value arguments of an application. + * `Nil` if the `callee` is not an application. + */ + def argss: List[List[Tree]] = { + def loop(tree: Tree): List[List[Tree]] = tree match { + case Apply(fn, args) => loop(fn) :+ args + case _ => Nil + } + loop(tree) + } + } + + /** Returns a wrapper that knows how to destructure and analyze applications. + */ + final def dissectApplied(tree: Tree) = new Applied(tree) + + /** Equivalent ot disectApplied(tree).core, but more efficient */ + @scala.annotation.tailrec + final def dissectCore(tree: Tree): Tree = tree match { + case TypeApply(fun, _) => + dissectCore(fun) + case Apply(fun, _) => + dissectCore(fun) + case t => + t + } + + /** Destructures applications into important subparts described in `Applied` class, + * namely into: core, targs and argss (in the specified order). + * + * Trees which are not applications are also accepted. Their callee and core will + * be equal to the input, while targs and argss will be Nil. + * + * The provided extractors don't expose all the API of the `Applied` class. + * For advanced use, call `dissectApplied` explicitly and use its methods instead of pattern matching. + */ + object Applied { + def apply(tree: Tree): Applied = new Applied(tree) + + def unapply(applied: Applied): Some[(Tree, List[Tree], List[List[Tree]])] = + Some((applied.core, applied.targs, applied.argss)) + + def unapply(tree: Tree): Some[(Tree, List[Tree], List[List[Tree]])] = + unapply(dissectApplied(tree)) + } + + /** Is tree an application with result `this.type`? + * Accept `b.addOne(x)` and also `xs(i) += x` + * where the op is an assignment operator. + */ + def isThisTypeResult(tree: Tree)(using Context): Boolean = tree match { + case Applied(fun @ Select(receiver, op), _, argss) => + tree.tpe match { + case ThisType(tref) => + tref.symbol == receiver.symbol + case tref: TermRef => + tref.symbol == receiver.symbol || argss.exists(_.exists(tref.symbol == _.symbol)) + case _ => + def checkSingle(sym: Symbol): Boolean = + (sym == receiver.symbol) || { + receiver match { + case Apply(_, _) => op.isOpAssignmentName // xs(i) += x + case _ => receiver.symbol != null && + (receiver.symbol.isGetter || receiver.symbol.isField) // xs.addOne(x) for var xs + } + } + @tailrec def loop(mt: Type): Boolean = mt match { + case m: MethodType => + m.resType match { + case ThisType(tref) => checkSingle(tref.symbol) + case tref: TermRef => checkSingle(tref.symbol) + case restpe => loop(restpe) + } + case PolyType(_, restpe) => loop(restpe) + case _ => false + } + fun.symbol != null && loop(fun.symbol.info) + } + case _ => + tree.tpe.isInstanceOf[ThisType] + } } trait UntypedTreeInfo extends TreeInfo[Untyped] { self: Trees.Instance[Untyped] => diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 5e5f9b324e27..90350d8447d2 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3928,7 +3928,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // so will take the code path that decides on inlining val tree1 = adapt(tree, WildcardType, locked) checkStatementPurity(tree1)(tree, ctx.owner) - if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value) { + if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value && !isThisTypeResult(tree)) { report.warning(ValueDiscarding(tree.tpe), tree.srcPos) } return tpd.Block(tree1 :: Nil, Literal(Constant(()))) diff --git a/tests/neg/warn-value-discard.check b/tests/neg/warn-value-discard.check index dd40e701caff..5e85f9e1742d 100644 --- a/tests/neg/warn-value-discard.check +++ b/tests/neg/warn-value-discard.check @@ -1,8 +1,16 @@ --- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:14:35 ---------------------------------------------- -14 | firstThing().map(_ => secondThing()) // error +-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:15:35 ---------------------------------------------- +15 | firstThing().map(_ => secondThing()) // error | ^^^^^^^^^^^^^ | discarded non-Unit value of type Either[Failed, Unit] --- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:17:35 ---------------------------------------------- -17 | firstThing().map(_ => secondThing()) // error +-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:18:35 ---------------------------------------------- +18 | firstThing().map(_ => secondThing()) // error | ^^^^^^^^^^^^^ | discarded non-Unit value of type Either[Failed, Unit] +-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:27:36 ---------------------------------------------- +27 | mutable.Set.empty[String].remove("") // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | discarded non-Unit value of type Boolean +-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:39:41 ---------------------------------------------- +39 | mutable.Set.empty[String].subtractOne("") // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | discarded non-Unit value of type scala.collection.mutable.Set[String] diff --git a/tests/neg/warn-value-discard.scala b/tests/neg/warn-value-discard.scala index 7e08bbf69086..45b82a6a8e72 100644 --- a/tests/neg/warn-value-discard.scala +++ b/tests/neg/warn-value-discard.scala @@ -1,6 +1,7 @@ // scalac: -Wvalue-discard -Werror import scala.util.{Either, Right, Left} +import scala.collection.mutable case class Failed(msg: String) @@ -16,3 +17,37 @@ def singleExpr(): Either[Failed, Unit] = def block(): Either[Failed, Unit] = { firstThing().map(_ => secondThing()) // error } + +class ValueDiscardTest: + val field = mutable.Set.empty[String] + + def remove(): Unit = + // Set#remove returns a Boolean, not this.type + // --> Warning + mutable.Set.empty[String].remove("") // error + + // TODO IMHO we don't need to support this, + // as it's just as easy to add a @nowarn annotation as a Unit ascription + //def removeAscribed(): Unit = { + // mutable.Set.empty[String].remove(""): Unit // nowarn + //} + + def subtract(): Unit = + // - Set#subtractOne returns this.type + // - receiver is not a field or a local variable (not quite sure what you'd call it) + // --> Warning + mutable.Set.empty[String].subtractOne("") // error + + def mutateLocalVariable(): Unit = { + // - Set#subtractOne returns this.type + // - receiver is a local variable + // --> No warning + val s: mutable.Set[String] = mutable.Set.empty[String] + s.subtractOne("") + } + + def mutateField(): Unit = + // - Set#subtractOne returns this.type + // - receiver is a local variable + // --> No warning + field.subtractOne("") From 702c485d6d8c6a24191b3cfaa51158a11a4ddd98 Mon Sep 17 00:00:00 2001 From: Chris Birchall Date: Wed, 23 Nov 2022 11:50:19 +0000 Subject: [PATCH 5/7] Add tests for assignment operator --- tests/neg/warn-value-discard.check | 4 ++++ tests/neg/warn-value-discard.scala | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/tests/neg/warn-value-discard.check b/tests/neg/warn-value-discard.check index 5e85f9e1742d..db935481efaa 100644 --- a/tests/neg/warn-value-discard.check +++ b/tests/neg/warn-value-discard.check @@ -14,3 +14,7 @@ 39 | mutable.Set.empty[String].subtractOne("") // error | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | discarded non-Unit value of type scala.collection.mutable.Set[String] +-- [E172] Potential Issue Error: tests/neg/warn-value-discard.scala:59:4 ----------------------------------------------- +59 | mutable.Set.empty[String] += "" // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | discarded non-Unit value of type scala.collection.mutable.Set[String] diff --git a/tests/neg/warn-value-discard.scala b/tests/neg/warn-value-discard.scala index 45b82a6a8e72..149433395cc5 100644 --- a/tests/neg/warn-value-discard.scala +++ b/tests/neg/warn-value-discard.scala @@ -51,3 +51,16 @@ class ValueDiscardTest: // - receiver is a local variable // --> No warning field.subtractOne("") + + def assignmentOperator(): Unit = + // - += returns this.type + // - receiver is not a field or a local variable + // --> Warning + mutable.Set.empty[String] += "" // error + + def assignmentOperatorLocalVariable(): Unit = + // - += returns this.type + // - receiver is a local variable + // --> No warning + val s: mutable.Set[String] = mutable.Set.empty[String] + s += "" From 83a01b8725d228c7611774193a53be2487f00715 Mon Sep 17 00:00:00 2001 From: Chris Birchall Date: Wed, 23 Nov 2022 12:14:43 +0000 Subject: [PATCH 6/7] Remove null checks --- compiler/src/dotty/tools/dotc/ast/TreeInfo.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 1b00209448c0..380733fbccc8 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -443,8 +443,7 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => (sym == receiver.symbol) || { receiver match { case Apply(_, _) => op.isOpAssignmentName // xs(i) += x - case _ => receiver.symbol != null && - (receiver.symbol.isGetter || receiver.symbol.isField) // xs.addOne(x) for var xs + case _ => receiver.symbol.isGetter || receiver.symbol.isField // xs.addOne(x) for var xs } } @tailrec def loop(mt: Type): Boolean = mt match { @@ -457,7 +456,7 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => case PolyType(_, restpe) => loop(restpe) case _ => false } - fun.symbol != null && loop(fun.symbol.info) + loop(fun.symbol.info) } case _ => tree.tpe.isInstanceOf[ThisType] From 973829b09c6253654498b0b9be192cb307b80af3 Mon Sep 17 00:00:00 2001 From: Chris Birchall Date: Wed, 21 Dec 2022 15:41:03 +0000 Subject: [PATCH 7/7] Refactor Applied to reuse existing helper methods TreeInfo already provided helpers that were almost identical to some of the methods in `Applied` that I copied over from Scala 2. Refactored `Applied` to make use of the existing methods. Moved `termArgss` from `TypedTreeInfo` up to `TreeInfo` so I could make use of it in `Applied`. Also moved `typeArgss`, just for consistency. --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 81 +++++++------------ 1 file changed, 27 insertions(+), 54 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 4465ce71f300..7c987b7dc887 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -110,6 +110,24 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => case _ => 0 } + /** The type arguments of a possibly curried call */ + def typeArgss(tree: Tree): List[List[Tree]] = + @tailrec + def loop(tree: Tree, argss: List[List[Tree]]): List[List[Tree]] = tree match + case TypeApply(fn, args) => loop(fn, args :: argss) + case Apply(fn, args) => loop(fn, argss) + case _ => argss + loop(tree, Nil) + + /** The term arguments of a possibly curried call */ + def termArgss(tree: Tree): List[List[Tree]] = + @tailrec + def loop(tree: Tree, argss: List[List[Tree]]): List[List[Tree]] = tree match + case Apply(fn, args) => loop(fn, args :: argss) + case TypeApply(fn, args) => loop(fn, argss) + case _ => argss + loop(tree, Nil) + /** All term arguments of an application in a single flattened list */ def allArguments(tree: Tree): List[Tree] = unsplice(tree) match { case Apply(fn, args) => allArguments(fn) ::: args @@ -342,26 +360,19 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => * * callee = foo * * core = foo * * targs = Nil - * * argss = List(List(arg11, arg12...), List(arg21, arg22, ...)) + * * argss = List(List(arg21, arg22, ...), List(arg11, arg12, ...)) * * Apply(Apply(TypeApply(foo, List(targs1, targs2, ...)), List(arg21, arg22, ...)), List(arg11, arg12...)) * * callee = TypeApply(foo, List(targs1, targs2, ...)) * * core = foo * * targs = Nil - * * argss = List(List(arg11, arg12...), List(arg21, arg22, ...)) + * * argss = List(List(arg21, arg22, ...), List(arg11, arg12, ...)) */ final class Applied(val tree: Tree) { /** The tree stripped of the possibly nested applications. * The original tree if it's not an application. */ - def callee: Tree = { - @tailrec - def loop(tree: Tree): Tree = tree match { - case Apply(fn, _) => loop(fn) - case tree => tree - } - loop(tree) - } + def callee: Tree = stripApply(tree) /** The `callee` unwrapped from type applications. * The original `callee` if it's not a type application. @@ -384,28 +395,7 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => /** (Possibly multiple lists of) value arguments of an application. * `Nil` if the `callee` is not an application. */ - def argss: List[List[Tree]] = { - def loop(tree: Tree): List[List[Tree]] = tree match { - case Apply(fn, args) => loop(fn) :+ args - case _ => Nil - } - loop(tree) - } - } - - /** Returns a wrapper that knows how to destructure and analyze applications. - */ - final def dissectApplied(tree: Tree) = new Applied(tree) - - /** Equivalent ot disectApplied(tree).core, but more efficient */ - @scala.annotation.tailrec - final def dissectCore(tree: Tree): Tree = tree match { - case TypeApply(fun, _) => - dissectCore(fun) - case Apply(fun, _) => - dissectCore(fun) - case t => - t + def argss: List[List[Tree]] = termArgss(tree) } /** Destructures applications into important subparts described in `Applied` class, @@ -424,7 +414,7 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => Some((applied.core, applied.targs, applied.argss)) def unapply(tree: Tree): Some[(Tree, List[Tree], List[List[Tree]])] = - unapply(dissectApplied(tree)) + unapply(new Applied(tree)) } /** Is tree an application with result `this.type`? @@ -442,8 +432,9 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => def checkSingle(sym: Symbol): Boolean = (sym == receiver.symbol) || { receiver match { - case Apply(_, _) => op.isOpAssignmentName // xs(i) += x - case _ => receiver.symbol.isGetter || receiver.symbol.isField // xs.addOne(x) for var xs + case Apply(_, _) => op.isOpAssignmentName // xs(i) += x + case _ => receiver.symbol != NoSymbol && + (receiver.symbol.isGetter || receiver.symbol.isField) // xs.addOne(x) for var xs } } @tailrec def loop(mt: Type): Boolean = mt match { @@ -456,7 +447,7 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => case PolyType(_, restpe) => loop(restpe) case _ => false } - loop(fun.symbol.info) + fun.symbol != NoSymbol && loop(fun.symbol.info) } case _ => tree.tpe.isInstanceOf[ThisType] @@ -836,24 +827,6 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => } } - /** The type arguments of a possibly curried call */ - def typeArgss(tree: Tree): List[List[Tree]] = - @tailrec - def loop(tree: Tree, argss: List[List[Tree]]): List[List[Tree]] = tree match - case TypeApply(fn, args) => loop(fn, args :: argss) - case Apply(fn, args) => loop(fn, argss) - case _ => argss - loop(tree, Nil) - - /** The term arguments of a possibly curried call */ - def termArgss(tree: Tree): List[List[Tree]] = - @tailrec - def loop(tree: Tree, argss: List[List[Tree]]): List[List[Tree]] = tree match - case Apply(fn, args) => loop(fn, args :: argss) - case TypeApply(fn, args) => loop(fn, argss) - case _ => argss - loop(tree, Nil) - /** The type and term arguments of a possibly curried call, in the order they are given */ def allArgss(tree: Tree): List[List[Tree]] = @tailrec