diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 69ec9f0d7b2b..bd521c8679d0 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -27,6 +27,7 @@ import dotty.tools.dotc.core.Symbols.Symbol import dotty.tools.dotc.core.StdNames.nme import scala.math.Ordering + /** * A compiler phase that checks for unused imports or definitions * @@ -146,6 +147,13 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke if !tree.isInstanceOf[tpd.InferredTypeTree] then typeTraverser(unusedDataApply).traverse(tree.tpe) ctx + override def prepareForAssign(tree: tpd.Assign)(using Context): Context = + unusedDataApply{ ud => + val sym = tree.lhs.symbol + if sym.exists then + ud.registerSetVar(sym) + } + // ========== MiniPhase Transform ========== override def transformBlock(tree: tpd.Block)(using Context): tpd.Tree = @@ -172,6 +180,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke unusedDataApply(_.removeIgnoredUsage(tree.symbol)) tree + // ---------- MiniPhase HELPERS ----------- private def pushInBlockTemplatePackageDef(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = @@ -215,11 +224,11 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke case sel: Select => prepareForSelect(sel) traverseChildren(tree)(using newCtx) - case _: (tpd.Block | tpd.Template | tpd.PackageDef) => + case tree: (tpd.Block | tpd.Template | tpd.PackageDef) => //! DIFFERS FROM MINIPHASE - unusedDataApply { ud => - ud.inNewScope(ScopeType.fromTree(tree))(traverseChildren(tree)(using newCtx)) - } + pushInBlockTemplatePackageDef(tree) + traverseChildren(tree)(using newCtx) + popOutBlockTemplatePackageDef() case t:tpd.ValDef => prepareForValDef(t) traverseChildren(tree)(using newCtx) @@ -235,6 +244,9 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke case t: tpd.Bind => prepareForBind(t) traverseChildren(tree)(using newCtx) + case t:tpd.Assign => + prepareForAssign(t) + traverseChildren(tree) case _: tpd.InferredTypeTree => case t@tpd.TypeTree() => //! DIFFERS FROM MINIPHASE @@ -278,6 +290,10 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke report.warning(s"unused private member", t) case UnusedSymbol(t, _, WarnTypes.PatVars) => report.warning(s"unused pattern variable", t) + case UnusedSymbol(t, _, WarnTypes.UnsetLocals) => + report.warning(s"unset local variable", t) + case UnusedSymbol(t, _, WarnTypes.UnsetPrivates) => + report.warning(s"unset private variable", t) } end CheckUnused @@ -297,6 +313,8 @@ object CheckUnused: case ImplicitParams case PrivateMembers case PatVars + case UnsetLocals + case UnsetPrivates /** * The key used to retrieve the "unused entity" analysis metadata, @@ -343,12 +361,8 @@ object CheckUnused: private val implicitParamInScope = MutSet[tpd.MemberDef]() private val patVarsInScope = MutSet[tpd.Bind]() - /* Unused collection collected at the end */ - private val unusedLocalDef = MutSet[tpd.MemberDef]() - private val unusedPrivateDef = MutSet[tpd.MemberDef]() - private val unusedExplicitParams = MutSet[tpd.MemberDef]() - private val unusedImplicitParams = MutSet[tpd.MemberDef]() - private val unusedPatVars = MutSet[tpd.Bind]() + /** All variables sets*/ + private val setVars = MutSet[Symbol]() /** All used symbols */ private val usedDef = MutSet[Symbol]() @@ -360,15 +374,6 @@ object CheckUnused: private val paramsToSkip = MutSet[Symbol]() - /** - * Push a new Scope of the given type, executes the given Unit and - * pop it back to the original type. - */ - def inNewScope(newScope: ScopeType)(execInNewScope: => Unit)(using Context): Unit = - val prev = currScopeType - pushScope(newScope) - execInNewScope - popScope() def finishAggregation(using Context)(): Unit = val unusedInThisStage = this.getUnused @@ -443,6 +448,9 @@ object CheckUnused: impInScope.push(MutSet()) usedInScope.push(MutSet()) + def registerSetVar(sym: Symbol): Unit = + setVars += sym + /** * leave the current scope and do : * @@ -501,15 +509,19 @@ object CheckUnused: unusedImport.map(d => UnusedSymbol(d.srcPos, d.name, WarnTypes.Imports)).toList else Nil - val sortedLocalDefs = + // Partition to extract unset local variables from usedLocalDefs + val (usedLocalDefs, unusedLocalDefs) = if ctx.settings.WunusedHas.locals then - localDefInScope - .filterNot(d => d.symbol.usedDefContains) - .filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name}) - .filterNot(d => containsSyntheticSuffix(d.symbol)) - .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.LocalDefs)).toList + localDefInScope.partition(d => d.symbol.usedDefContains) else - Nil + (Nil, Nil) + val sortedLocalDefs = + unusedLocalDefs + .filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name}) + .filterNot(d => containsSyntheticSuffix(d.symbol)) + .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.LocalDefs)).toList + val unsetLocalDefs = usedLocalDefs.filter(isUnsetVarDef).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.UnsetLocals)).toList + val sortedExplicitParams = if ctx.settings.WunusedHas.explicits then explicitParamInScope @@ -527,14 +539,14 @@ object CheckUnused: .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.ImplicitParams)).toList else Nil - val sortedPrivateDefs = + // Partition to extract unset private variables from usedPrivates + val (usedPrivates, unusedPrivates) = if ctx.settings.WunusedHas.privates then - privateDefInScope - .filterNot(d => d.symbol.usedDefContains) - .filterNot(d => containsSyntheticSuffix(d.symbol)) - .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PrivateMembers)).toList + privateDefInScope.partition(d => d.symbol.usedDefContains) else - Nil + (Nil, Nil) + val sortedPrivateDefs = unusedPrivates.filterNot(d => containsSyntheticSuffix(d.symbol)).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PrivateMembers)).toList + val unsetPrivateDefs = usedPrivates.filter(isUnsetVarDef).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.UnsetPrivates)).toList val sortedPatVars = if ctx.settings.WunusedHas.patvars then patVarsInScope @@ -544,7 +556,9 @@ object CheckUnused: .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PatVars)).toList else Nil - val warnings = List(sortedImp, sortedLocalDefs, sortedExplicitParams, sortedImplicitParams, sortedPrivateDefs, sortedPatVars).flatten.sortBy { s => + val warnings = + List(sortedImp, sortedLocalDefs, sortedExplicitParams, sortedImplicitParams, + sortedPrivateDefs, sortedPatVars, unsetLocalDefs, unsetPrivateDefs).flatten.sortBy { s => val pos = s.pos.sourcePos (pos.line, pos.column) } @@ -731,10 +745,13 @@ object CheckUnused: !isSyntheticMainParam(sym) && !sym.shouldNotReportParamOwner - private def shouldReportPrivateDef(using Context): Boolean = currScopeType.top == ScopeType.Template && !memDef.symbol.isConstructor && memDef.symbol.is(Private, butNot = SelfName | Synthetic | CaseAccessor) + private def isUnsetVarDef(using Context): Boolean = + val sym = memDef.symbol + sym.is(Mutable) && !setVars(sym) + extension (imp: tpd.Import) /** Enum generate an import for its cases (but outside them), which should be ignored */ def isGeneratedByEnum(using Context): Boolean = diff --git a/tests/neg-custom-args/fatal-warnings/i15503b.scala b/tests/neg-custom-args/fatal-warnings/i15503b.scala index 8a4a055150f9..c8a2d6bc2074 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503b.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503b.scala @@ -2,49 +2,91 @@ val a = 1 // OK +var cs = 3 // OK + val b = // OK + var e3 = 2 // error val e1 = 1 // error def e2 = 2 // error 1 val c = // OK - val e1 = 1 // OK + var e1 = 1 // error not set + def e2 = e1 // OK + val e3 = e2 // OK + e3 + +val g = // OK + var e1 = 1 // OK def e2 = e1 // OK - e2 + val e3 = e2 // OK + e1 = e3 // OK + e3 def d = 1 // OK def e = // OK val e1 = 1 // error def e2 = 2 // error + var e3 = 4 // error 1 def f = // OK val f1 = 1 // OK - def f2 = f1 // OK + var f2 = f1 // error not set + def f3 = f2 // OK + f3 + +def h = // OK + val f1 = 1 // OK + var f2 = f1 // OK + def f3 = f2 // OK + f2 = f3 // OK f2 class Foo { + val a = 1 // OK + + var cs = 3 // OK + val b = // OK + var e3 = 2 // error val e1 = 1 // error def e2 = 2 // error 1 val c = // OK - val e1 = 1 // OK + var e1 = 1 // error not set + def e2 = e1 // OK + val e3 = e2 // OK + e3 + + val g = // OK + var e1 = 1 // OK def e2 = e1 // OK - e2 + val e3 = e2 // OK + e1 = e3 // OK + e3 def d = 1 // OK def e = // OK val e1 = 1 // error def e2 = 2 // error + var e3 = 4 // error 1 def f = // OK val f1 = 1 // OK - def f2 = f1 // OK + var f2 = f1 // error not set + def f3 = f2 // OK + f3 + + def h = // OK + val f1 = 1 // OK + var f2 = f1 // OK + def f3 = f2 // OK + f2 = f3 // OK f2 } @@ -68,7 +110,7 @@ package foo.scala2.tests: new a.Inner } def f2 = { - var x = 100 + var x = 100 // error not set x } } @@ -89,7 +131,7 @@ package foo.scala2.tests: } package test.foo.twisted.i16682: - def myPackage = + def myPackage = object IntExtractor: // OK def unapply(s: String): Option[Int] = s.toIntOption diff --git a/tests/neg-custom-args/fatal-warnings/i15503c.scala b/tests/neg-custom-args/fatal-warnings/i15503c.scala index 630846df4e5d..e4e15116bf0d 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503c.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503c.scala @@ -12,12 +12,24 @@ class A: private[this] val f = e // OK private val g = f // OK + private[A] var h = 1 // OK + private[this] var i = h // error not set + private var j = i // error not set + + private[this] var k = 1 // OK + private var l = 2 // OK + private val m = // error + k = l + l = k + l + private def fac(x: Int): Int = // error if x == 0 then 1 else x * fac(x - 1) val x = 1 // OK def y = 2 // OK def z = g // OK + var w = 2 // OK package foo.test.contructors: case class A private (x:Int) // OK @@ -25,7 +37,12 @@ package foo.test.contructors: class C private (private val x: Int) // error class D private (private val x: Int): // OK def y = x - + class E private (private var x: Int): // error not set + def y = x + class F private (private var x: Int): // OK + def y = + x = 3 + x package test.foo.i16682: object myPackage: diff --git a/tests/neg-custom-args/fatal-warnings/i15503i.scala b/tests/neg-custom-args/fatal-warnings/i15503i.scala index fefead7f01a3..ea84c176eee4 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503i.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503i.scala @@ -142,8 +142,8 @@ package foo.test.possibleclasses.withvar: private var y: Int // OK )( s: Int, // OK - var t: Int, // OK - private var z: Int // OK + var t: Int, // OK global scope can be set somewhere else + private var z: Int // error not set ) { def a = k + y + s + t + z } @@ -159,11 +159,11 @@ package foo.test.possibleclasses.withvar: class AllUsed( k: Int, // OK - private var y: Int // OK + private var y: Int // error not set )( s: Int, // OK - var t: Int, // OK - private var z: Int // OK + var t: Int, // OK global scope can be set somewhere else + private var z: Int // error not set ) { def a = k + y + s + t + z } diff --git a/tests/neg-custom-args/fatal-warnings/i16639a.scala b/tests/neg-custom-args/fatal-warnings/i16639a.scala new file mode 100644 index 000000000000..c62910b7f566 --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i16639a.scala @@ -0,0 +1,207 @@ +// scalac: -Wunused:all +// +class Bippy(a: Int, b: Int) { + private def this(c: Int) = this(c, c) // warn /Dotty:NoWarn + private def boop(x: Int) = x+a+b // error + private def bippy(x: Int): Int = bippy(x) // error TODO: could warn + final private val MILLIS1 = 2000 // error no warn, /Dotty:Warn + final private val MILLIS2: Int = 1000 // error + final private val HI_COMPANION: Int = 500 // no warn, accessed from companion + def hi() = Bippy.HI_INSTANCE +} +object Bippy { + def hi(x: Bippy) = x.HI_COMPANION + private val HI_INSTANCE: Int = 500 // no warn, accessed from instance + private val HEY_INSTANCE: Int = 1000 // error warn + private lazy val BOOL: Boolean = true // error warn +} + +class A(val msg: String) +class B1(msg: String) extends A(msg) +class B2(msg0: String) extends A(msg0) +class B3(msg0: String) extends A("msg") // error /Dotty: unused explicit parameter + +trait Bing + +trait Accessors { + private var v1: Int = 0 // error warn + private var v2: Int = 0 // error warn, never set + private var v3: Int = 0 // warn, never got /Dotty: no warn even if not used + private var v4: Int = 0 // no warn + + private[this] var v5 = 0 // error warn, never set + private[this] var v6 = 0 // warn, never got /Dotty: no warn even if not used + private[this] var v7 = 0 // no warn + + def bippy(): Int = { + v3 = 3 + v4 = 4 + v6 = 6 + v7 = 7 + v2 + v4 + v5 + v7 + } +} + +class StableAccessors { + private var s1: Int = 0 // error warn + private var s2: Int = 0 // error warn, never set + private var s3: Int = 0 // warn, never got /Dotty: no warn even if not usued + private var s4: Int = 0 // no warn + + private[this] var s5 = 0 // error warn, never set + private[this] var s6 = 0 // no warn, limitation /Dotty: Why limitation ? + private[this] var s7 = 0 // no warn + + def bippy(): Int = { + s3 = 3 + s4 = 4 + s6 = 6 + s7 = 7 + s2 + s4 + s5 + s7 + } +} + +trait DefaultArgs { + // warn about default getters for x2 and x3 + private def bippy(x1: Int, x2: Int = 10, x3: Int = 15): Int = x1 + x2 + x3 // no more warn warn since #17061 + + def boppy() = bippy(5, 100, 200) +} + + +class Outer { + class Inner +} + +trait Locals { + def f0 = { + var x = 1 // error warn + var y = 2 + y = 3 + y + y + } + def f1 = { + val a = new Outer // no warn + val b = new Outer // error warn + new a.Inner + } + def f2 = { + var x = 100 // error warn about it being a var, var not set + x + } +} + +object Types { + private object Dongo { def f = this } // no more warn since #17061 + private class Bar1 // error warn + private class Bar2 // no warn + private type Alias1 = String // error warn + private type Alias2 = String // no warn + def bippo = (new Bar2).toString + + def f(x: Alias2) = x.length + + def l1() = { + object HiObject { def f = this } // no more warn since #17061 + class Hi { // error warn + def f1: Hi = new Hi + def f2(x: Hi) = x + } + class DingDongDoobie // error warn + class Bippy // no warn + type Something = Bippy // no warn + type OtherThing = String // error warn + (new Bippy): Something + } +} + +trait Underwarn { + def f(): Seq[Int] + + def g() = { + val Seq(_, _) = f() // no warn + true + } +} + +class OtherNames { + private def x_=(i: Int): Unit = () // no more warn since #17061 + private def x: Int = 42 // error Dotty triggers unused private member : To investigate + private def y_=(i: Int): Unit = () // // no more warn since #17061 + private def y: Int = 42 + + def f = y +} + + +trait Forever { + def f = { + val t = Option((17, 42)) + for { + ns <- t + (i, j) = ns // no warn + } yield (i + j) + } + def g = { + val t = Option((17, 42)) + for { + ns <- t + (i, j) = ns // no warn + } yield 42 // val emitted only if needed, hence nothing unused + } +} + +trait Ignorance { + private val readResolve = 42 // error ignore /dotty triggers unused private member/ why should we ignore ? +} + +trait CaseyKasem { + def f = 42 match { + case x if x < 25 => "no warn" + case y if toString.nonEmpty => "no warn" + y + case z => "warn" + } +} +trait CaseyAtTheBat { + def f = Option(42) match { + case Some(x) if x < 25 => "no warn" + case Some(y @ _) if toString.nonEmpty => "no warn" + case Some(z) => "warn" + case None => "no warn" + } +} + +class `not even using companion privates` + +object `not even using companion privates` { + private implicit class `for your eyes only`(i: Int) { // no more warn since #17061 + def f = i + } +} + +class `no warn in patmat anonfun isDefinedAt` { + def f(pf: PartialFunction[String, Int]) = pf("42") + def g = f { + case s => s.length // no warn (used to warn case s => true in isDefinedAt) + } +} + +// this is the ordinary case, as AnyRef is an alias of Object +class `nonprivate alias is enclosing` { + class C + type C2 = C + private class D extends C2 // error warn +} + +object `classof something` { + private class intrinsically + def f = classOf[intrinsically].toString() +} + +trait `short comings` { + def f: Int = { + val x = 42 // error /Dotty only triggers in dotty + 17 + } +} + diff --git a/tests/pos/i16639false-pos-on-trait.scala b/tests/pos/i16639false-pos-on-trait.scala new file mode 100644 index 000000000000..67e304f556e1 --- /dev/null +++ b/tests/pos/i16639false-pos-on-trait.scala @@ -0,0 +1,40 @@ +// scalac -Wunsued:all +//Avoid warning on setter in trait Regression test : issue10154 scala + +trait T { + private var x: String = _ + + def y: String = { + if (x eq null) x = "hello, world" + x + } +} + +/* +➜ skalac -version +Scala compiler version 2.13.10-20220920-001308-98972e5 -- Copyright 2002-2022, LAMP/EPFL and Lightbend, Inc. + +➜ skalac -d /tmp -Wunused -Vprint:typer t12646.scala +t12646.scala:3: warning: parameter value x_= in variable x is never used + private var x: String = _ + ^ +[[syntax trees at end of typer]] // t12646.scala +package { + abstract trait T extends scala.AnyRef { + def /*T*/$init$(): Unit = { + () + }; + private val x: String = _; + private def x_=(x$1: String): Unit; + def y: String = { + if (T.this.x.eq(null)) + T.this.x_=("hello, world") + else + (); + T.this.x + } + } +} + +1 warning +*/