Skip to content

false-positive "unused import" warning when importing givens defined in common trait for several objects #19657

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
danielleontiev opened this issue Feb 9, 2024 · 5 comments · Fixed by #20894
Labels
area:implicits related to implicits area:linting Linting warnings enabled with -W or -Xlint itype:bug

Comments

@danielleontiev
Copy link

Compiler version

Scala 3.3.1, JVM 17

Minimized code

bug.sc
//> using scala 3.3.1
//> using options "-Wunused:imports"

trait Schema[A]

case class Foo()
case class Bar()

trait SchemaGenerator[A] {
  given Schema[A] = new Schema[A]{}
}

object FooCodec extends SchemaGenerator[Foo]
object BarCodec extends SchemaGenerator[Bar]

def summonSchemas(using Schema[Foo], Schema[Bar]) = ()

import FooCodec.given
import BarCodec.given

summonSchemas

Run code with

scala-cli run bug.sc

Output

[warn] ./bug.sc:18:17
[warn] unused import
[warn] import FooCodec.given
[warn]                 ^^^^^

Expectation

Warning should not be reported

Comment

It seems that some sort of names collision occurs due to the common SchemaGenerator trait because defining givens explicitly works as expected

//> using scala 3.3.1
//> using options "-Wunused:imports"

trait Schema[A]

case class Foo()
case class Bar()

object FooCodec {
  given Schema[Foo] = new Schema[Foo] {}
}
object BarCodec {
  given Schema[Bar] = new Schema[Bar] {}
}

def summonSchemas(using Schema[Foo], Schema[Bar]) = ()

import FooCodec.given
import BarCodec.given

summonSchemas

compiles without warning message

@danielleontiev danielleontiev added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Feb 9, 2024
@Gedochao
Copy link
Contributor

Gedochao commented Feb 9, 2024

Reproduced on the main branch, as well as with 3.4.0-RC4

Minimised code in non-script format (to be passed straight to the compiler)

trait Schema[A]

case class Foo()
case class Bar()

trait SchemaGenerator[A] {
  given Schema[A] = new Schema[A]{}
}

object FooCodec extends SchemaGenerator[Foo]
object BarCodec extends SchemaGenerator[Bar]

def summonSchemas(using Schema[Foo], Schema[Bar]) = ()

@main def main = {
  import FooCodec.given
  import BarCodec.given
  summonSchemas
}

output:

scalac -d . -Wunused:imports repro.scala
-- Warning: repro.scala:17:18 --------------------------------------------------
17 |  import BarCodec.given
   |                  ^^^^^
   |                  unused import
1 warning found

@Gedochao Gedochao added area:implicits related to implicits and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Feb 9, 2024
@Gedochao Gedochao added the area:linting Linting warnings enabled with -W or -Xlint label Mar 12, 2024
@grzegorz-bielski
Copy link
Contributor

grzegorz-bielski commented Apr 18, 2024

This error is very annoying. It basically prevents us from using given imports with -Wunused:imports so we have to rely on mixing in traits.

@som-snytt
Copy link
Contributor

@grzegorz-bielski -Wunused:strict-no-implicit-warn weakens the check to avoid false positives, as a workaround.

➜  snips ~/projects/dotty/bin/scalac -Wunused:all -d /tmp/sandbox i19657.scala
-- Warning: i19657.scala:17:18 -----------------------------------------------------------------------------------------
17 |  import BarCodec.given
   |                  ^^^^^
   |                  unused import
-- Warning: i19657.scala:18:34 -----------------------------------------------------------------------------------------
18 |  import scala.collection.mutable.*
   |                                  ^
   |                                  unused import
2 warnings found
➜  snips ~/projects/dotty/bin/scalac -Wunused:all -Wunused:strict-no-implicit-warn -d /tmp/sandbox i19657.scala
➜  snips

It seems to avoid warning about wildcard import selectors which may reference any implicit.

so we have to rely on mixing in traits

I don't think optional warnings (especially this one) are intended to invite architectural rewrites.

@grzegorz-bielski
Copy link
Contributor

@som-snytt I didn't know about -Wunused:strict-no-implicit-warn. It helped, thx!

I don't think optional warnings (especially this one) are intended to invite architectural rewrites.

My thoughts exactly! However, this was the only workaround I could think of at the time that did not involve the removal of -Wunused:imports.

@som-snytt som-snytt self-assigned this Apr 18, 2024
@som-snytt
Copy link
Contributor

Waiting on #20321 which obviates my working branch.

@som-snytt som-snytt removed their assignment Jan 21, 2025
sjrd added a commit that referenced this issue Jan 28, 2025
…20894)

This PR changes the `CheckUnused` phase to rely on the `MiniPhase` API
(instead of custom traversal). That improves fidelity to `Context`
(instead of approximate scoping).

The phase should work seamlessly with subsequent linting phases
(currently, `CheckShadowed`).

It is a goal of the PR to eliminate false reports. It is also a goal not
to regress previous work on efficiency.

A remaining limitation of the current approach is that contexts don't
provide a nesting level. Practically, this means that for a wildcard
import nested below a higher precedence named import, the wildcard is
deemed "unused". (A more general tool for "managing" or "formatting"
imports could do more to pick a preferred scope.)

This PR adds `-Wunused:patvars`, as forward-ported from Scala 2: it
relies on attachments for some details about desugaring, but otherwise
uses positions (where only the original patvar has a non-synthetic
position).

As in Scala 2, it does not warn about patvars with the "canonical" name
of a case class element (this is complicated by type tests and the
quotes API); other exclusions are to be ported, such as "name derived
from the match selector".

Support is added for `-Wconf:origin=full.path.selector`, as in Scala 2.
That allows, for example:
```
-Wconf:origin=scala.util.chaining.given:s
```
to exclude certain blessed imports from warnings, or to work around
false positives (should they arise).

Support is added to `-rewrite` unused imports. There are no options to
"format"; instead, textual deletions preserve existing formatting,
except that blank lines are removed and braces removed when there is
only one selector.

Notable fixes are to support `compiletime` and `inline`; there are more
fixes to pursue in this area.

Fixes #19657
Fixes #20520
Fixes #19998
Fixes #18313
Fixes #17371
Fixes #18708
Fixes #21917
Fixes #21420
Fixes #20951
Fixes #19252
Fixes #18289
Fixes #17667
Fixes #17252
Fixes #21807
Fixes #17753
Fixes #17318
Fixes #18564
Fixes #22376
Fixes #21525
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:implicits related to implicits area:linting Linting warnings enabled with -W or -Xlint itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants