Skip to content

Backport "Improve Unit ascription escape hatch" to 3.3 LTS #419

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

Open
wants to merge 2 commits into
base: backport-lts-3.3-22521
Choose a base branch
from
Open
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
13 changes: 13 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/TreeInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,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)
Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,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 _ =>
tree.withType(checkedType)
val tree2 = toNotNullTermRef(tree1, pt)
Expand Down Expand Up @@ -4236,7 +4236,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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ object CompletionItemResolver extends ItemResolver:
if companion == NoSymbol || gsym.is(JavaDefined) then
if gsymDoc.isEmpty() then
if gsym.isAliasType then
fullDocstring(gsym.info.deepDealias.typeSymbol, search)
fullDocstring(gsym.info.deepDealiasAndSimplify.typeSymbol, search)
else if gsym.is(Method) then
gsym.info.finalResultType match
case tr @ TermRef(_, sym) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ object HoverProvider:
if symbol.name == nme.selectDynamic || symbol.name == nme.applyDynamic =>
fallbackToDynamics(path, printer, contentType)
case symbolTpes @ ((symbol, tpe, _) :: _) =>
val exprTpw = tpe.widenTermRefExpr.deepDealias
val exprTpw = tpe.widenTermRefExpr.deepDealiasAndSimplify
val hoverString =
tpw match
// https://github.com/lampepfl/dotty/issues/8891
Expand All @@ -123,7 +123,7 @@ object HoverProvider:
if tpe != NoType then tpe
else tpw

printer.hoverSymbol(sym, finalTpe.deepDealias)
printer.hoverSymbol(sym, finalTpe.deepDealiasAndSimplify)
end match
end hoverString

Expand Down Expand Up @@ -177,7 +177,7 @@ object HoverProvider:
val resultType =
rest match
case Select(_, asInstanceOf) :: TypeApply(_, List(tpe)) :: _ if asInstanceOf == nme.asInstanceOfPM =>
tpe.tpe.widenTermRefExpr.deepDealias
tpe.tpe.widenTermRefExpr.deepDealiasAndSimplify
case _ if n == nme.selectDynamic => tpe.resultType
case _ => tpe

Expand All @@ -202,9 +202,9 @@ object HoverProvider:
findRefinement(parent)
case _ => None

val refTpe = sel.typeOpt.widen.deepDealias match
val refTpe = sel.typeOpt.widen.deepDealiasAndSimplify match
case r: RefinedType => Some(r)
case t: (TermRef | TypeProxy) => Some(t.termSymbol.info.deepDealias)
case t: (TermRef | TypeProxy) => Some(t.termSymbol.info.deepDealiasAndSimplify)
case _ => None

refTpe.flatMap(findRefinement).asJava
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ final class InferredTypeProvider(
case _ => true
if isInScope(tpe)
then tpe
else tpe.deepDealias
else tpe.deepDealiasAndSimplify

val printer = ShortenedTypePrinter(
symbolSearch,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class PcInlayHintsProvider(
isInScope(tycon) && args.forall(isInScope)
case _ => true
if isInScope(tpe) then tpe
else tpe.deepDealias(using indexedCtx.ctx)
else tpe.deepDealiasAndSimplify(using indexedCtx.ctx)

val dealiased = optDealias(tpe)
val tpeStr = printer.tpe(dealiased)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import dotty.tools.dotc.core.Flags
import dotty.tools.dotc.core.Names.*
import dotty.tools.dotc.core.StdNames.nme
import dotty.tools.dotc.core.Symbols.*
import dotty.tools.pc.utils.InteractiveEnrichments.deepDealias
import dotty.tools.pc.utils.InteractiveEnrichments.deepDealiasAndSimplify
import dotty.tools.pc.SemanticdbSymbols
import dotty.tools.pc.utils.InteractiveEnrichments.allSymbols
import dotty.tools.pc.utils.InteractiveEnrichments.stripBackticks
Expand Down Expand Up @@ -51,7 +51,7 @@ class SymbolInformationProvider(using Context):
collect(classSym)
visited.toList.map(SemanticdbSymbols.symbolName)
val dealisedSymbol =
if sym.isAliasType then sym.info.deepDealias.typeSymbol else sym
if sym.isAliasType then sym.info.deepDealiasAndSimplify.typeSymbol else sym
val classOwner =
sym.ownersIterator.drop(1).find(s => s.isClass || s.is(Flags.Module))
val overridden = sym.denot.allOverriddenSymbols.toList
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ object CaseKeywordCompletion:
case (Ident(v), tpe) => v.decoded == value
case (Select(_, v), tpe) => v.decoded == value
case t => false
.map((_, id) => argPts(id).widen.deepDealias)
.map((_, id) => argPts(id).widen.deepDealiasAndSimplify)
/* Parent is a function expecting a case match expression */
case TreeApply(fun, _) if !fun.tpe.isErroneous =>
fun.tpe.paramInfoss match
Expand All @@ -103,12 +103,12 @@ object CaseKeywordCompletion:
) =>
val args = head.argTypes.init
if args.length > 1 then
Some(definitions.tupleType(args).widen.deepDealias)
else args.headOption.map(_.widen.deepDealias)
Some(definitions.tupleType(args).widen.deepDealiasAndSimplify)
else args.headOption.map(_.widen.deepDealiasAndSimplify)
case _ => None
case _ => None
case sel =>
Some(sel.tpe.widen.deepDealias)
Some(sel.tpe.widen.deepDealiasAndSimplify)

selTpe
.collect { case selTpe if selTpe != NoType =>
Expand Down Expand Up @@ -279,8 +279,8 @@ object CaseKeywordCompletion:
clientSupportsSnippets
)

val tpeStr = printer.tpe(selector.tpe.widen.deepDealias.bounds.hi)
val tpe = selector.typeOpt.widen.deepDealias.bounds.hi match
val tpeStr = printer.tpe(selector.tpe.widen.deepDealiasAndSimplify.bounds.hi)
val tpe = selector.typeOpt.widen.deepDealiasAndSimplify.bounds.hi match
case tr @ TypeRef(_, _) => tr.underlying
case t => t

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ object SingletonCompletions:
} yield value

private def collectSingletons(tpe: Type)(using Context): List[Constant] =
tpe.deepDealias match
tpe.deepDealiasAndSimplify match
case ConstantType(value) => List(value)
case OrType(tpe1, tpe2) =>
collectSingletons(tpe1) ++ collectSingletons(tpe2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ object InteractiveEnrichments extends CommonMtagsEnrichments:
if sym.is(Module) then sym.companionClass else sym.companionModule

def dealiasType: Symbol =
if sym.isType then sym.info.deepDealias.typeSymbol else sym
if sym.isType then sym.info.deepDealiasAndSimplify.typeSymbol else sym

def nameBackticked: String = nameBackticked(Set.empty[String])

Expand Down Expand Up @@ -402,16 +402,16 @@ object InteractiveEnrichments extends CommonMtagsEnrichments:
end extension

extension (tpe: Type)
def deepDealias(using Context): Type =
tpe.dealias match
def deepDealiasAndSimplify(using Context): Type =
val dealiased = tpe.dealias match
case app @ AppliedType(tycon, params) =>
AppliedType(tycon, params.map(_.deepDealias))
AppliedType(tycon, params.map(_.deepDealiasAndSimplify))
case aliasingBounds: AliasingBounds =>
aliasingBounds.derivedAlias(aliasingBounds.alias.deepDealiasAndSimplify)
case TypeBounds(lo, hi) =>
TypeBounds(lo.dealias, hi.dealias)
case RefinedType(parent, name, refinedInfo) =>
RefinedType(parent.dealias, name, refinedInfo.deepDealias)
RefinedType(parent.dealias, name, refinedInfo.deepDealiasAndSimplify)
case dealised => dealised
dealiased.simplified

Expand Down
7 changes: 0 additions & 7 deletions tests/pos/i20070.scala

This file was deleted.

8 changes: 4 additions & 4 deletions tests/warn/warn-value-discard.check
Original file line number Diff line number Diff line change
Expand Up @@ -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 -------------------------------------------
Expand Down
43 changes: 37 additions & 6 deletions tests/warn/warn-value-discard.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -63,4 +61,37 @@ class ValueDiscardTest:
// - receiver is a local variable
// --> No warning
val s: mutable.Set[String] = mutable.Set.empty[String]
s += ""
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