Skip to content

Commit

Permalink
Enable more param warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
som-snytt committed Nov 5, 2024
1 parent e8ca0cc commit 1907775
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 37 deletions.
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ class Compiler {
List(new sbt.ExtractAPI) :: // Sends a representation of the API of classes to sbt via callbacks
List(new Inlining) :: // Inline and execute macros
List(new PostInlining) :: // Add mirror support for inlined code
List(new CheckUnused.PostInlining) :: // Check for unused elements
List(new Staging) :: // Check staging levels and heal staged types
List(new Splicing) :: // Replace level 1 splices with holes
List(new PickleQuotes) :: // Turn quoted trees into explicit run-time data structures
List(new CheckUnused.PostInlining) :: // Check for unused elements
Nil

/** Phases dealing with the transformation from pickled trees to backend trees */
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,8 @@ class Definitions {
@tu lazy val Predef_undefined: Symbol = ScalaPredefModule.requiredMethod(nme.???)
@tu lazy val ScalaPredefModuleClass: ClassSymbol = ScalaPredefModule.moduleClass.asClass

@tu lazy val SameTypeClass: ClassSymbol = requiredClass("scala.=:=")
@tu lazy val SameType_refl: Symbol = SameTypeClass.companionModule.requiredMethod(nme.refl)
@tu lazy val SubTypeClass: ClassSymbol = requiredClass("scala.<:<")
@tu lazy val SubType_refl: Symbol = SubTypeClass.companionModule.requiredMethod(nme.refl)

Expand Down
30 changes: 22 additions & 8 deletions compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import dotty.tools.dotc.core.Types.{AnnotatedType, ClassInfo, ConstantType, Name
import dotty.tools.dotc.core.Flags
import dotty.tools.dotc.core.Names.{Name, TermName, termName}
import dotty.tools.dotc.core.NameOps.isReplWrapperName
import dotty.tools.dotc.core.NameKinds.WildcardParamName
import dotty.tools.dotc.core.NameKinds.{ContextFunctionParamName, WildcardParamName}
import dotty.tools.dotc.core.Symbols.{NoSymbol, Symbol, defn, isDeprecated}
import dotty.tools.dotc.report
import dotty.tools.dotc.reporting.{Message, UnusedSymbol as UnusedSymbolMessage}
Expand Down Expand Up @@ -517,13 +517,25 @@ object CheckUnused:
warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.LocalDefs))

if ctx.settings.WunusedHas.explicits then
def forgiven(sym: Symbol) =
containsSyntheticSuffix(sym)
|| sym.owner.hasAnnotation(defn.UnusedAnnot)
|| sym.info.isSingleton
for d <- explicitParamInScope do
if !d.symbol.usedDefContains && !isUsedInPosition(d.symbol.name, d.span) && !containsSyntheticSuffix(d.symbol) then
if !d.symbol.usedDefContains && !isUsedInPosition(d.symbol.name, d.span) && !forgiven(d.symbol) then
warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.ExplicitParams))

if ctx.settings.WunusedHas.implicits then
def forgiven(sym: Symbol) =
val dd = defn
sym.name.is(ContextFunctionParamName)
|| sym.owner.hasAnnotation(defn.UnusedAnnot)
|| sym.info.typeSymbol.match
case dd.DummyImplicitClass | dd.SubTypeClass | dd.SameTypeClass => true
case _ => false
|| sym.info.isSingleton
for d <- implicitParamInScope do
if !d.symbol.usedDefContains && !containsSyntheticSuffix(d.symbol) then
if !d.symbol.usedDefContains && !forgiven(d.symbol) then
warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.ImplicitParams))

// Partition to extract unset private variables from usedPrivates
Expand Down Expand Up @@ -556,7 +568,9 @@ object CheckUnused:
/** Heuristic to detect synthetic suffixes in names of symbols.
*/
private def containsSyntheticSuffix(symbol: Symbol)(using Context): Boolean =
symbol.name.mangledString.contains("$")
val mangled = symbol.name.mangledString
val index = mangled.indexOf('$')
index >= 0 && !(index == mangled.length - 1 && symbol.is(Module))

/**
* Is the constructor of synthetic package object
Expand Down Expand Up @@ -659,8 +673,7 @@ object CheckUnused:
owner.isPrimaryConstructor ||
owner.isDeprecated ||
owner.isAllOf(Synthetic | PrivateLocal) ||
owner.is(Accessor) ||
owner.isOverridden
owner.is(Accessor)
}

private def usedDefContains(using Context): Boolean =
Expand All @@ -669,9 +682,10 @@ object CheckUnused:
private def everySymbol(using Context): List[Symbol] =
List(sym, sym.companionClass, sym.companionModule, sym.moduleClass).filter(_.exists)

/** A function is overridden. Either has `override flags` or parent has a matching member (type and name) */
/** A function is overridden. Either has `override flags` or parent has a matching member (type and name)
private def isOverridden(using Context): Boolean =
sym.is(Flags.Override) || (sym.exists && sym.owner.thisType.parents.exists(p => sym.matchingMember(p).exists))
*/

end extension

Expand All @@ -688,7 +702,7 @@ object CheckUnused:
case ConstantType(_) => true
case tp: TermRef =>
// Detect Scala 2 SingleType
tp.underlying.classSymbol.is(Flags.Module)
tp.underlying.classSymbol.is(Module)
case _ =>
false
def registerTrivial(using Context): Unit =
Expand Down
4 changes: 2 additions & 2 deletions tests/warn/i15503b.scala
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ package foo.scala2.tests:

object Types {
def l1() = {
object HiObject { def f = this } // OK
object HiObject { def f = this } // warn
class Hi { // warn
def f1: Hi = new Hi
def f2(x: Hi) = x
Expand All @@ -141,4 +141,4 @@ package test.foo.twisted.i16682:
}
isInt

def f = myPackage("42")
def f = myPackage("42")
4 changes: 2 additions & 2 deletions tests/warn/i15503e.scala
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ package foo.test.i16865:
trait Bar extends Foo

object Ex extends Bar:
def fn(a: Int, b: Int): Int = b + 3 // OK
def fn(a: Int, b: Int): Int = b + 3 // warn

object Ex2 extends Bar:
override def fn(a: Int, b: Int): Int = b + 3 // OK
override def fn(a: Int, b: Int): Int = b + 3 // warn
4 changes: 2 additions & 2 deletions tests/warn/i15503f.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ val default_int = 1
object Xd {
private def f1(a: Int) = a // OK
private def f2(a: Int) = 1 // OK
private def f3(a: Int)(using Int) = a // OK
private def f4(a: Int)(using Int) = default_int // OK
private def f3(a: Int)(using Int) = a // warn
private def f4(a: Int)(using Int) = default_int // warn
private def f6(a: Int)(using Int) = summon[Int] // OK
private def f7(a: Int)(using Int) = summon[Int] + a // OK
private def f8(a: Int)(using foo: Int) = a // warn
Expand Down
6 changes: 3 additions & 3 deletions tests/warn/i15503g.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ object Foo {

private def f1(a: Int) = a // OK
private def f2(a: Int) = default_int // warn
private def f3(a: Int)(using Int) = a // OK
private def f4(a: Int)(using Int) = default_int // warn
private def f3(a: Int)(using Int) = a // warn
private def f4(a: Int)(using Int) = default_int // warn // warn
private def f6(a: Int)(using Int) = summon[Int] // warn
private def f7(a: Int)(using Int) = summon[Int] + a // OK
/* --- Trivial method check --- */
Expand All @@ -20,4 +20,4 @@ package foo.test.i17101:
extension[A] (x: Test[A]) { // OK
def value: A = x
def causesIssue: Unit = println("oh no")
}
}
34 changes: 17 additions & 17 deletions tests/warn/unused-params.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//> using options -Wunused:params -Werror
//> using options -Wunused:params
//

import Answers._
Expand All @@ -23,7 +23,7 @@ trait BadAPI extends InterFace {
a
}
override def call(a: Int,
b: String, // no warn, required by superclass
b: String, // warn
c: Double): Int = {
println(c)
a
Expand All @@ -33,7 +33,7 @@ trait BadAPI extends InterFace {

override def equals(other: Any): Boolean = true // no warn

def i(implicit s: String) = answer // yes, warn
def i(implicit s: String) = answer // warn

/*
def future(x: Int): Int = {
Expand All @@ -60,7 +60,7 @@ class Revaluing(u: Int) { def f = u } // no warn

case class CaseyKasem(k: Int) // no warn

case class CaseyAtTheBat(k: Int)(s: String) // warn
case class CaseyAtTheBat(k: Int)(s: String) // NO warn

trait Ignorance {
def f(readResolve: Int) = answer // warn
Expand All @@ -78,13 +78,13 @@ trait Unimplementation {

trait DumbStuff {
def f(implicit dummy: DummyImplicit) = answer
def g(dummy: DummyImplicit) = answer
def g(dummy: DummyImplicit) = answer // warn
}
trait Proofs {
def f[A, B](implicit ev: A =:= B) = answer
def g[A, B](implicit ev: A <:< B) = answer
def f2[A, B](ev: A =:= B) = answer
def g2[A, B](ev: A <:< B) = answer
def f2[A, B](ev: A =:= B) = answer // warn
def g2[A, B](ev: A <:< B) = answer // warn
}

trait Anonymous {
Expand All @@ -94,19 +94,19 @@ trait Anonymous {

def f2: Int => Int = _ + 1 // no warn placeholder syntax (a fresh name and synthetic parameter)

def g = for (i <- List(1)) yield answer // no warn patvar elaborated as map.(i => 42)
def g = for (i <- List(1)) yield answer // warn // TODO no warn patvar elaborated as map.(i => 42)
}
trait Context[A] { def m(a: A): A = a }
trait Implicits {
def f[A](implicit ctx: Context[A]) = answer
def g[A: Context] = answer
def f[A](implicit ctx: Context[A]) = answer // warn
def g[A: Context] = answer // warn
}
class Bound[A: Context]
class Bound[A: Context] // warn
object Answers {
def answer: Int = 42
}

trait BadMix { _: InterFace =>
trait BadMix { self: InterFace =>
def f(a: Int,
b: String, // warn
c: Double): Int = {
Expand All @@ -121,7 +121,7 @@ trait BadMix { _: InterFace =>
a
}
override def call(a: Int,
b: String, // no warn, required by superclass
XXXX: String, // warn no longer excused because required by superclass
c: Double): Int = {
println(c)
a
Expand All @@ -131,23 +131,23 @@ trait BadMix { _: InterFace =>

override def equals(other: Any): Boolean = true // no warn

def i(implicit s: String) = answer // yes, warn
def i(implicit s: String) = answer // warn
}

class Unequal {
override def equals(other: Any) = toString.nonEmpty // no warn non-trivial RHS, required by universal method
override def equals(other: Any) = toString.nonEmpty // warn
}

class Seriously {
def f(s: Serializable) = toString.nonEmpty // warn explicit param of marker trait
}

class TryStart(start: String) {
def FINALLY(end: END.type) = start
def FINALLY(end: END.type) = start // no warn for DSL taking a singleton
}

object END

class Nested {
@annotation.unused private def actuallyNotUsed(fresh: Int, stale: Int) = fresh
@annotation.unused private def actuallyNotUsed(fresh: Int, stale: Int) = fresh // no warn if owner is unused
}
4 changes: 2 additions & 2 deletions tests/warn/unused-privates.scala
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ trait Locals {
}

object Types {
private object Dongo { def f = this } // NO warn
private object Dongo { def f = this } // warn
private class Bar1 // warn
private class Bar2 // no warn
private type Alias1 = String // warn
Expand All @@ -105,7 +105,7 @@ object Types {
def f(x: Alias2) = x.length

def l1() = {
object HiObject { def f = this } // NO warn
object HiObject { def f = this } // warn
class Hi { // warn
def f1: Hi = new Hi
def f2(x: Hi) = x
Expand Down

0 comments on commit 1907775

Please sign in to comment.