Skip to content

Commit

Permalink
Fix -Wunused:import registering constructor ("<init>") instead of its…
Browse files Browse the repository at this point in the history
… owner (also fix false positive for enum) (#16661)

This solves the issue #16650.

The issue is that `member(sym.name)` failed to resolve when a type
contains multiple identical name. Here the `dotty.tools.dotc.util`
package contains multiple "<init>" (constructor), so `member` fail to
resolve the name

The solution is to register the owner of the contructor instead of the
constructor itself.

:warning: I couldn't create a test where this appends, I tested it by
`publishLocal` and testing the code in a different project with this
code from #16650 (also didn't check on the metals build):
```scala
//> using lib "org.scala-lang::scala3-compiler:3.3.0-RC1-bin-20230109-f56089b-NIGHTLY"
//> using scala "3.3.0-RC1-bin-20230109-f56089b-NIGHTLY"
//> using option "-Wunused:all"

import dotty.tools.dotc.util.LinearSet

@main
def run = 
  val a = 123
  println("Hello!")
```
```bash
sbt> run
[info] compiling 1 Scala source to .../target/scala-3.3.0-RC1-bin-SNAPSHOT/classes ...
[warn] -- Warning: ..../src/main/scala/Main.scala:5:29 
[warn] 5 |import dotty.tools.dotc.util.LinearSet
[warn]   |                             ^^^^^^^^^
[warn]   |                             unused import
[warn] -- Warning:.../src/main/scala/Main.scala:10:6 
[warn] 10 |  val a = 123
[warn]    |      ^
[warn]    |      unused local definition
[warn] two warnings found
[info] running run 
Hello!
```
### EDIT
Also add a related fix for import generated by enum (false positive)
  • Loading branch information
szymon-rd authored Jan 12, 2023
2 parents afeaf6e + fa1d2bb commit 483b1e8
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
12 changes: 10 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import dotty.tools.dotc.core.Definitions
import dotty.tools.dotc.core.Types.ConstantType
import dotty.tools.dotc.core.NameKinds.WildcardParamName
import dotty.tools.dotc.core.Types.TermRef
import dotty.tools.dotc.core.Types.NameFilter



Expand Down Expand Up @@ -326,9 +327,12 @@ object CheckUnused:
*/
def registerUsed(sym: Symbol, name: Option[Name])(using Context): Unit =
if !isConstructorOfSynth(sym) && !doNotRegister(sym) then
usedInScope.top += ((sym, sym.isAccessibleAsIdent, name))
if sym.isConstructor && sym.exists then
registerUsed(sym.owner, None) // constructor are "implicitly" imported with the class
else
usedInScope.top += ((sym, sym.isAccessibleAsIdent, name))
usedInScope.top += ((sym.companionModule, sym.isAccessibleAsIdent, name))
usedInScope.top += ((sym.companionClass, sym.isAccessibleAsIdent, name))

/** Register a symbol that should be ignored */
def addIgnoredUsage(sym: Symbol)(using Context): Unit =
Expand All @@ -345,7 +349,7 @@ object CheckUnused:

/** Register an import */
def registerImport(imp: tpd.Import)(using Context): Unit =
if !tpd.languageImport(imp.expr).nonEmpty then
if !tpd.languageImport(imp.expr).nonEmpty && !imp.isGeneratedByEnum then
impInScope.top += imp
unusedImport ++= imp.selectors.filter { s =>
!shouldSelectorBeReported(imp, s) && !isImportExclusion(s)
Expand Down Expand Up @@ -589,6 +593,10 @@ object CheckUnused:
!isSyntheticMainParam(sym) &&
!sym.shouldNotReportParamOwner

extension (imp: tpd.Import)
/** Enum generate an import for its cases (but outside them), which should be ignored */
def isGeneratedByEnum(using Context): Boolean =
imp.symbol.exists && imp.symbol.owner.is(Flags.Enum, butNot = Flags.Case)

extension (thisName: Name)
private def isWildcard: Boolean =
Expand Down
7 changes: 6 additions & 1 deletion tests/neg-custom-args/fatal-warnings/i15503a.scala
Original file line number Diff line number Diff line change
Expand Up @@ -250,4 +250,9 @@ package foo.testing.rename.imports:
package foo.testing.imports.precedence:
import scala.collection.immutable.{BitSet => _, _} // error
import scala.collection.immutable.BitSet // OK
def t = BitSet.empty
def t = BitSet.empty

package foo.test.enums:
enum A: // OK
case B extends A // OK
case C extends A // OK
9 changes: 9 additions & 0 deletions tests/neg-custom-args/fatal-warnings/i15503i.scala
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,12 @@ package foo.test.scala.annotation:
@unused private def c = 3 // OK

def other = b

package foo.test.companionprivate:
class A:
import A.b // OK
def a = b // OK

object A:
private def b = c // OK
def c = List(1,2,3) // OK

0 comments on commit 483b1e8

Please sign in to comment.