False positive unused warnings in 3.7 nightly on method overrides
Closed this issue · 10 comments
Compiler version
3.7.0-RC1-bin-20250306-73ba485-NIGHTLY
Minimized code
With -Wunused:explicits enabled:
trait Foldable[F[_]] {
def foldLeft[A, B](fa: F[A], b: B)(f: (B, A) => B): B
}
type Id[A] = A
given foldableId: Foldable[Id] =
new Foldable[Id] {
def foldLeft[A, B](fa: Id[A], b: B)(f: (B, A) => B): B = b
}Output
-- [E198] Unused Symbol Warning: /Users/matt/scala3.7-nightly-unused/src/main/scala/example/Test.scala:9:23
9 | def foldLeft[A, B](fa: Id[A], b: B)(f: (B, A) => B): B = b
| ^^
| unused explicit parameter
-- [E198] Unused Symbol Warning: /Users/matt/scala3.7-nightly-unused/src/main/scala/example/Test.scala:9:40
9 | def foldLeft[A, B](fa: Id[A], b: B)(f: (B, A) => B): B = b
| ^
| unused explicit parameterExpectation
The parameters should not be reported unused since they're necessary to satisfy the method override
This change was intentional, as there was negative feedback about the heuristics in Scala 2.
I did not have a plan yet for gathering input, so I'll do that on this ticket. If the heuristic is desirable, should it be under a compiler option? Should all the heuristics (such as don't warn if the RHS is ???) be under a flag? and what is the default?
As a footnote, to the best of my memory, the heuristics preceded the introduction of @unused and @nowarn and -Wconf, when a lint for unused parameters was deemed to be impossibly noisy.
My experiment with this code base suggested just adding @unused to unused elements (rather than using brittle heuristics, which can be head-scratchers as to whether they apply).
re: the heuristics, I would love to see more consistency with them, or at least some documentation of them. For example, why doesn't this similar code produce a warning?
trait Functor[F[_]] {
def map[A, B](fa: F[A])(f: A => B): F[B]
}
type Never[A] = Unit
given functorNever: Functor[Never] =
new Functor[Never] {
def map[A, B](fa: Never[A])(f: A => B): Never[B] = ()
}I'm guessing it's because the RHS is considered "trivial" (related: #16640) but it doesn't seem any less trivial to me than the first example -- they're both a single expression
@mrdziuban Thanks, that is an example of the DX I'd like to avoid.
"Obviously" I did not remove all the heuristics, but I don't know yet how to offer "quiet" warnings.
A similar heuristic is that returning a single arg is "trivial".
The original ticket to restore the exclusion for overrides is #16865
There is an existing test.
Possible feature goal:
- don't warn because I'm in the middle of coding the implementation (which is why I wrote
???) - don't warn because this implementation is obviously a trivial default intended to be overridden in subclasses
- don't warn because I don't control the obsolete signature of the overridden method
I think 2&3 deserve annotations. For 1, I think the ideal is "gradual warnings": I need it to warn before integration testing, and hopefully before I create a PR, but not when I save the source file.
It's also desirable to avoid excessive "config". That is good for scalafix but not for the compiler. Maybe this ticket is really: "Lint should be finally moved out of the compiler into AbideScalafix."
Minimally, -Yunused:help could explain available heuristics (as a form of documentation).
Hypothetical -Yunused:-override,-unimplemented,-trivial to "turn off" certain heuristics.
Hypothetical
-Yunused:-override,-unimplemented,-trivialto "turn off" certain heuristics.
Something like that level of granularity sounds ideal to me.
Also just wanted to say thank you for all the work you're putting into linting! I'm testing the 3.7 nightly releases against my large codebase and there are a ton of new, valid unused warnings on imports and parameters that I'm very happy to get rid of.
OK I will learn from @lrytz who did this granularity with -Xsource:3. I will try to level up.
The PR is quite simple and not granular, in hopes that it can make the cut-off for 3.7.0.
The PR is quite simple and not granular, in hopes that it can make the cut-off for 3.7.0.
The cut-off is tomorrow, in case it gets delayed, I'll tag @WojciechMazur so that we remember to potentially backport this in RC.
The PR does not add an option, but just silences the warning in question.
A future version can acquire fancy knobs and levers.