From 9e18bfeb60521d59328098f1dc9a7ddc838b3070 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 13 May 2025 11:45:06 -0700 Subject: [PATCH] Improve Unit ascription escape hatch --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 13 ++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 5 ++- tests/warn/warn-value-discard.check | 8 ++-- tests/warn/warn-value-discard.scala | 43 ++++++++++++++++--- 4 files changed, 57 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 45e17794ec96..b0e1b1117e0b 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -264,6 +264,19 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] => case _ => false } + /** Expression was written `e: Unit` to quell warnings. Looks into adapted tree. */ + def isAscribedToUnit(tree: Tree): Boolean = + import typer.Typer.AscribedToUnit + tree.hasAttachment(AscribedToUnit) + || { + def loop(tree: Tree): Boolean = tree match + case Apply(fn, _) => fn.hasAttachment(AscribedToUnit) || loop(fn) + case TypeApply(fn, _) => fn.hasAttachment(AscribedToUnit) || loop(fn) + case Block(_, expr) => expr.hasAttachment(AscribedToUnit) || loop(expr) + case _ => false + loop(tree) + } + /** Does this CaseDef catch Throwable? */ def catchesThrowable(cdef: CaseDef)(using Context): Boolean = catchesAllOf(cdef, defn.ThrowableType) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 935f90519b7f..c8478f6f095b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -670,7 +670,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer val checkedType = checkNotShadowed(ownType) val tree1 = checkedType match case checkedType: NamedType if !prefixIsElidable(checkedType) => - ref(checkedType).withSpan(tree.span) + ref(checkedType).withSpan(tree.span).withAttachmentsFrom(tree) case _ => def isScalaModuleRef = checkedType match case moduleRef: TypeRef if moduleRef.symbol.is(ModuleClass, butNot = JavaDefined) => true @@ -4695,7 +4695,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer && !ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && !isThisTypeResult(tree) - && !tree.hasAttachment(AscribedToUnit) then + && !isAscribedToUnit(tree) + then report.warning(ValueDiscarding(tree.tpe), tree.srcPos) return tpd.Block(tree1 :: Nil, unitLiteral) diff --git a/tests/warn/warn-value-discard.check b/tests/warn/warn-value-discard.check index dcd6d62c00e0..03ada5c68621 100644 --- a/tests/warn/warn-value-discard.check +++ b/tests/warn/warn-value-discard.check @@ -2,12 +2,12 @@ 27 | mutable.Set.empty[String].remove("") // warn | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | discarded non-Unit value of type Boolean. Add `: Unit` to discard silently. --- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:39:41 ------------------------------------------- -39 | mutable.Set.empty[String].subtractOne("") // warn +-- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:37:41 ------------------------------------------- +37 | mutable.Set.empty[String].subtractOne("") // warn | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | discarded non-Unit value of type scala.collection.mutable.Set[String]. Add `: Unit` to discard silently. --- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:59:4 -------------------------------------------- -59 | mutable.Set.empty[String] += "" // warn +-- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:57:4 -------------------------------------------- +57 | mutable.Set.empty[String] += "" // warn | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | discarded non-Unit value of type scala.collection.mutable.Set[String]. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:15:35 ------------------------------------------- diff --git a/tests/warn/warn-value-discard.scala b/tests/warn/warn-value-discard.scala index 8bb9aebf515b..93c2c056b67c 100644 --- a/tests/warn/warn-value-discard.scala +++ b/tests/warn/warn-value-discard.scala @@ -26,11 +26,9 @@ class ValueDiscardTest: // --> Warning mutable.Set.empty[String].remove("") // warn - // 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 removeAscribed(): Unit = { + mutable.Set.empty[String].remove(""): Unit // nowarn + } def subtract(): Unit = // - Set#subtractOne returns this.type @@ -63,4 +61,37 @@ class ValueDiscardTest: // - receiver is a local variable // --> No warning val s: mutable.Set[String] = mutable.Set.empty[String] - s += "" \ No newline at end of file + s += "" + +// see also tests/warn/21557.scala +class UnitAscription: + import scala.concurrent.*, ExecutionContext.Implicits.given + + case class C(c: Int): + def f(i: Int, j: Int = c) = i + j + + def f(i: Int, j: Int = 27) = i + j + + def g[A]: List[A] = Nil + + def i: Int = 42 + + def `default arg is inline`: Unit = + f(i = 42): Unit // nowarn + + def `default arg requires block`: Unit = + C(27).f(i = 42): Unit // nowarn + + def `application requires implicit arg`: Unit = + Future(42): Unit // nowarn + + def `application requires inferred type arg`: Unit = + g: Unit // nowarn + + def `implicit selection from this`: Unit = + i: Unit // nowarn + +object UnitAscription: + def g[A]: List[A] = Nil + def `application requires inferred type arg`: Unit = + g: Unit // nowarn UnitAscription.g