Fix wunused false positive on CanEqual #18641
Conversation
szymon-rd
force-pushed
the
can-equal-wunused
branch
from
64b55e7
to
f8cc335
Compare
| * Ignore CanEqual imports | ||
| */ | ||
| private def isImportIgnored(imp: tpd.Import, sel: ImportSelector)(using Context): Boolean = | ||
| (sel.isWildcard && imp.expr.tpe.allMembers.exists(p => p.symbol.typeRef.baseClasses.exists(_.derivesFrom(defn.CanEqualClass)))) || |
There was a problem hiding this comment.
I think this is problematic because perhaps it only imports CanEqual's that are not even used. Perhaps it makes more sense to record the CanEqual usages in the typer?
There was a problem hiding this comment.
Also this should check if its also a "given" import, rather than "*"
There was a problem hiding this comment.
Unfortunately, after a lengthy discussion with Martin and the team and making some PoCs, we decided that extracting that information from Typer is unfeasible without making the Typer and Implicit resolution slower / more complex. So we want to emulate it or fix false positives. But I will address the second comment 👍
There was a problem hiding this comment.
Emulating the feature will require summoning the exact same implicit again wont it? seems like even more work
There was a problem hiding this comment.
Yes, and this logic could get even more complex I suppose given that it is a type param, and the resolution is pre typer vs the emulation would be post typer. Not a good idea
There was a problem hiding this comment.
Im not sure how it works, but might it produce more accurate warnings if you only ignore warnings generated by the import selector when the specific unused symbol derives from CanEqual?
There was a problem hiding this comment.
It's pretty much what we are doing here
szymon-rd
force-pushed
the
can-equal-wunused
branch
from
1d7a25b
to
88eed71
Compare
Backports #18641 to the LTS branch. PR submitted by the release tooling. [skip ci]
Backports #18641 to the LTS branch. PR submitted by the release tooling. [skip ci]
Fixes #17762