No warn implicit param of overriding method (#22901)

Fixes #22895 
Fixes #22896 
Follow-up to #22729 
Follow-up to #22757 

Since Scala 2, the check for unused parameters does not warn for an
overriding method, since the signature is dictated by the overridden
method. That behavior was intentionally omitted in 3.7 because it was
introduced in Scala 2 before `@nowarn` and `@unused` were available.
(The warning would be too noisy without mitigation.) A compiler option
for these heuristics was tried in a previous commit, but reverted.
(Compiler options are difficult to evolve once added.) So these commits
restore this heuristic without further ado. An option for "strict mode"
without allowances is future work.

The other issue was that detecting an override in a Java parent did not
work. The commit always checks `is(Override)` before searching for an
overridden member, since it is not necessary to verify the modifier
(which happens in RefChecks). Incorrect overriding would result in false
negatives, which is acceptable since it will error later.

Further asymmetry between checking implicit params and explicit class
param accessors may or may not be dubious.

Author: sjrd

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -1953,7 +1953,7 @@ object SymDenotations {
       case _ => NoSymbol
 
     /** The explicitly given self type (self types of modules are assumed to be
-     *  explcitly given here).
+     *  explicitly given here).
      */
     def givenSelfType(using Context): Type = classInfo.selfInfo match {
       case tp: Type => tp

compiler/src/dotty/tools/dotc/transform/CheckUnused.scala

@@ -590,7 +590,7 @@ object CheckUnused:
       end checkExplicit
       // begin
       if !infos.skip(m)
-        && !m.nextOverriddenSymbol.exists
+        && !m.isEffectivelyOverride
         && !allowed
       then
         checkExplicit()
@@ -614,6 +614,7 @@ object CheckUnused:
         || sym.info.isInstanceOf[RefinedType] // can't be expressed as a context bound
       if ctx.settings.WunusedHas.implicits
         && !infos.skip(m)
+        && !m.isEffectivelyOverride
         && !allowed
       then
         if m.isPrimaryConstructor then
@@ -891,41 +892,49 @@ object CheckUnused:
     inline def exists(p: Name => Boolean): Boolean = nm.ne(nme.NO_NAME) && p(nm)
     inline def isWildcard: Boolean = nm == nme.WILDCARD || nm.is(WildcardParamName)
 
-  extension (tp: Type)
-    def importPrefix(using Context): Type = tp match
+  extension (tp: Type)(using Context)
+    def importPrefix: Type = tp match
       case tp: NamedType => tp.prefix
       case tp: ClassInfo => tp.prefix
       case tp: TypeProxy => tp.superType.normalizedPrefix
       case _ => NoType
-    def underlyingPrefix(using Context): Type = tp match
+    def underlyingPrefix: Type = tp match
       case tp: NamedType => tp.prefix
       case tp: ClassInfo => tp.prefix
       case tp: TypeProxy => tp.underlying.underlyingPrefix
       case _ => NoType
-    def skipPackageObject(using Context): Type =
+    def skipPackageObject: Type =
       if tp.typeSymbol.isPackageObject then tp.underlyingPrefix else tp
-    def underlying(using Context): Type = tp match
+    def underlying: Type = tp match
       case tp: TypeProxy => tp.underlying
       case _ => tp
 
   private val serializationNames: Set[TermName] =
     Set("readResolve", "readObject", "readObjectNoData", "writeObject", "writeReplace").map(termName(_))
 
-  extension (sym: Symbol)
-    def isSerializationSupport(using Context): Boolean =
+  extension (sym: Symbol)(using Context)
+    def isSerializationSupport: Boolean =
       sym.is(Method) && serializationNames(sym.name.toTermName) && sym.owner.isClass
         && sym.owner.derivesFrom(defn.JavaSerializableClass)
-    def isCanEqual(using Context): Boolean =
+    def isCanEqual: Boolean =
       sym.isOneOf(GivenOrImplicit) && sym.info.finalResultType.baseClasses.exists(_.derivesFrom(defn.CanEqualClass))
-    def isMarkerTrait(using Context): Boolean =
+    def isMarkerTrait: Boolean =
       sym.isClass && sym.info.allMembers.forall: d =>
         val m = d.symbol
         !m.isTerm || m.isSelfSym || m.is(Method) && (m.owner == defn.AnyClass || m.owner == defn.ObjectClass)
-    def isEffectivelyPrivate(using Context): Boolean =
+    def isEffectivelyPrivate: Boolean =
       sym.is(Private, butNot = ParamAccessor)
-      || sym.owner.isAnonymousClass && !sym.nextOverriddenSymbol.exists
+      || sym.owner.isAnonymousClass && !sym.isEffectivelyOverride
+    def isEffectivelyOverride: Boolean =
+      sym.is(Override)
+      ||
+      sym.canMatchInheritedSymbols && { // inline allOverriddenSymbols using owner.info or thisType
+        val owner = sym.owner.asClass
+        val base = if owner.classInfo.selfInfo != NoType then owner.thisType else owner.info
+        base.baseClasses.drop(1).iterator.exists(sym.overriddenSymbol(_).exists)
+      }
     // pick the symbol the user wrote for purposes of tracking
-    inline def userSymbol(using Context): Symbol=
+    inline def userSymbol: Symbol=
       if sym.denot.is(ModuleClass) then sym.denot.companionModule else sym
 
   extension (sel: ImportSelector)
@@ -939,21 +948,21 @@ object CheckUnused:
       case untpd.Ident(nme.WILDCARD) => true
       case _ => false
 
-  extension (imp: Import)
+  extension (imp: Import)(using Context)
     /** Is it the first import clause in a statement? `a.x` in `import a.x, b.{y, z}` */
-    def isPrimaryClause(using Context): Boolean =
+    def isPrimaryClause: Boolean =
       imp.srcPos.span.pointDelta > 0 // primary clause starts at `import` keyword with point at clause proper
 
     /** Generated import of cases from enum companion. */
-    def isGeneratedByEnum(using Context): Boolean =
+    def isGeneratedByEnum: Boolean =
       imp.symbol.exists && imp.symbol.owner.is(Enum, butNot = Case)
 
     /** Under -Wunused:strict-no-implicit-warn, avoid false positives
      *  if this selector is a wildcard that might import implicits or
      *  specifically does import an implicit.
      *  Similarly, import of CanEqual must not warn, as it is always witness.
      */
-    def isLoose(sel: ImportSelector)(using Context): Boolean =
+    def isLoose(sel: ImportSelector): Boolean =
       if ctx.settings.WunusedHas.strictNoImplicitWarn then
         if sel.isWildcard
           || imp.expr.tpe.member(sel.name.toTermName).hasAltWith(_.symbol.isOneOf(GivenOrImplicit))

tests/warn/i15503f.scala

@@ -42,7 +42,7 @@ object ExampleWithoutWith:
       case '{ ${Expr(opt)} : Some[T] } => Some(opt)
       case _ => None
 
-//absolving names on matches of quote trees requires consulting non-abstract types in QuotesImpl
+//nowarning names on matches of quote trees requires consulting non-abstract types in QuotesImpl
 object Unmatched:
   import scala.quoted.*
   def transform[T](e: Expr[T])(using Quotes): Expr[T] =
@@ -84,3 +84,18 @@ package givens:
   given namely: (x: X) => Y: // warn protected param to given class
     def doY = "8"
 end givens
+
+object i22895:
+  trait Test[F[_], Ev] {
+    def apply[A, B](fa: F[A])(f: A => B)(using ev: Ev): F[B]
+  }
+  given testId: Test[[a] =>> a, Unit] =
+    new Test[[a] =>> a, Unit] {
+      def apply[A, B](fa: A)(f: A => B)(using ev: Unit): B = f(fa) // nowarn override
+    }
+  class C:
+    def f(using s: String) = s.toInt
+  class D(i: Int) extends C:
+    override def f(using String) = compute(i) // nowarn override
+    def g(using sss: String) = compute(i) // warn
+    def compute(i: Int) = i * 42 // returning a class param is deemed trivial, make it non-trivial

tests/warn/i22896/J.java

@@ -0,0 +1,6 @@
+
+public class J {
+    private int i = 42;
+    public int i() { return 27; }
+    public String i(int j) { return "hello, world"; }
+}

tests/warn/i22896/s.scala

@@ -0,0 +1,13 @@
+//> using options -Werror -Wunused:privates
+
+def f =
+  new J:
+    override val i = -1 // nowarn, trust override
+
+def g =
+  new J:
+    override def i = -1 // nowarn, trust override
+
+def h =
+  new J:
+    override def i() = -1 // nowarn correctly matches signature

tests/warn/unused-params.scala

@@ -122,7 +122,7 @@ trait BadMix { self: InterFace =>
     a
   }
   override def call(a: Int,
-                    XXXX: String,               // warn no longer excused because required by superclass
+                    XXXX: String, // no warn still excused because override (required by self type)
                     c: Double): Int = {
     println(c)
     a
@@ -135,6 +135,14 @@ trait BadMix { self: InterFace =>
   def i(implicit s: String) = answer           // warn
 }
 
+trait ImplFace:
+  self: InterFace =>
+  def call(a: Int,
+           b: String, // no warn required by self type
+           c: Double): Int =
+    println(c)
+    a
+
 class Unequal {
   override def equals(other: Any) = toString.nonEmpty   // no warn (override)
 }