Skip to content

3.7 regression - false negative warning: unused imports #22690

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
OndrejSpanel opened this issue Mar 2, 2025 · 10 comments · Fixed by #22695
Closed

3.7 regression - false negative warning: unused imports #22690

OndrejSpanel opened this issue Mar 2, 2025 · 10 comments · Fixed by #22695
Assignees
Labels
area:linting Linting warnings enabled with -W or -Xlint itype:bug

Comments

@OndrejSpanel
Copy link
Member

OndrejSpanel commented Mar 2, 2025

I have tested 3.6.4 briefly against my project and to my dismay I have the sad duty to report a regression here. The code compiles without any warnings in 3.6.3.

I was not able to find a repro without external dependency yet, I guess some macros are needed for the issue to happen.

Compiler version

3.6.4-RC1, 3.6.4-RC2

Minimized code

import io.bullet.borer.*
import io.bullet.borer.derivation.MapBasedCodecs.*
import Settings.*

object Settings:
  given codec: Codec[Settings] = deriveCodec
  def x = 0

case class Settings(value: Double = x)

build.sbt:

scalaVersion := "3.6.4-RC1"

name := "Sandbox"

libraryDependencies += "io.bullet" %% "borer-core" % "1.15.0"
libraryDependencies += "io.bullet" %% "borer-derivation" % "1.15.0"

scalacOptions ++= Seq("-Xfatal-warnings", "-Wunused:imports")

Result

C:\Dev\Sandbox\src\main\scala\Settings.scala:3:17
unused import
import Settings.*

Image

Expectation

The import is not unused, in fact the code does not compile without the import.

Repro

Scastie link to repro: https://scastie.scala-lang.org/OndrejSpanel/bHJcv9AhSS2DODsdcQFvAw/5

@OndrejSpanel OndrejSpanel added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 2, 2025
@OndrejSpanel
Copy link
Member Author

OndrejSpanel commented Mar 2, 2025

The same warning is reported when using circe instead of borer:

import io.circe.*
import io.circe.generic.semiauto.*
import Settings.*

object Settings:
  given codec: Codec[Settings] = deriveCodec
  def x = 0

case class Settings(value: Double = x)
scalaVersion := "3.6.4-RC1"

name := "Sandbox"

libraryDependencies += "io.circe" %% "circe-core" % "0.14.10"
libraryDependencies += "io.circe" %% "circe-generic" % "0.14.10"

scalacOptions ++= Seq("-Xfatal-warnings", "-Wunused:imports")

@som-snytt som-snytt added area:linting Linting warnings enabled with -W or -Xlint and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 2, 2025
@som-snytt som-snytt self-assigned this Mar 2, 2025
@som-snytt
Copy link
Contributor

s/sad duty/solemn duty
also
s/-Xfatal-warnings/-Werror

Thanks for the report, the improved lint is more ambitious about helping with various compiletime constructs.

@som-snytt
Copy link
Contributor

som-snytt commented Mar 2, 2025

scalaVersion := "3.6.4-RC1"

name := "Sandbox"

libraryDependencies += "io.circe" %% "circe-core" % "0.14.10"
libraryDependencies += "io.circe" %% "circe-generic" % "0.14.10"

scalacOptions ++= Seq("-Werror", "-Wunused:imports")
//scalacOptions ++= Seq("-Wunused:imports")
scalacOptions ++= Seq("-Vprint:typer,inlining")
//scalacOptions ++= Seq("-Vprint:typer,inlining", "-Yplain-printer")
  i22690 git:(main)  cat src/main/scala/i22690.scala

import io.circe.*
import io.circe.generic.semiauto.*
//import Settings.*

object Settings:
  given codec: Codec[Settings] = deriveCodec
  def x = 0

case class Settings(value: Double = x)

The project compiles cleanly. More info about the use case, what the import is intended to do, etc, would be welcome.

[success] Total time: 4 s, completed Mar 2, 2025, 8:50:50 AM

Sadly, I must close for want of a reproduction; I will have no more interesting bug to pursue this afternoon.

@som-snytt som-snytt closed this as not planned Won't fix, can't repro, duplicate, stale Mar 2, 2025
@som-snytt
Copy link
Contributor

I'm not sure what the screen shot is a picture of. I just thought to retry with RC2, same result.

Don't neglect to clean before trying to build after a version change. Don't trust the IDE.

@som-snytt
Copy link
Contributor

Wait, I see I pasted wrong.

@som-snytt som-snytt reopened this Mar 2, 2025
@som-snytt
Copy link
Contributor

som-snytt commented Mar 2, 2025

Same result.

Tried the borer example, same result.

@som-snytt som-snytt closed this as not planned Won't fix, can't repro, duplicate, stale Mar 2, 2025
@som-snytt
Copy link
Contributor

som-snytt commented Mar 2, 2025

Actually, I don't see that the improved lint was backported to 3.6.

3.7 doesn't warn with or without the import.

The algorithm must accept the "outermost" matching element (since the code typechecks, and it doesn't have nesting levels, it can't compare all imports with fidelity). Worth checking if this example is a "false negative" in 3.7 that could be tightened up.

Certain redundant imports can't be declared "unused" for this reason. (A formatter or refactoring tool could do that.)

Edit: after deleting the ctor default arg (which are members of the companion), it seems to look up $asInstanceOf$ in the import. I'm not entirely enlightened on the fine points of that synthetic member, but it should not resolve here.

A not-null assertion for reference `x` has the form `x.$asInstanceOf$[x.type & T]`.

@OndrejSpanel
Copy link
Member Author

OndrejSpanel commented Mar 2, 2025

in fact the code does not compile without the import.

I was wrong here. The code like this did not compile in 2.12 or 2.13, but it compiles in all 3.x versions.

I see now the issue is very different in nature than I originally thought. It seems the symbol x is accessible in the case class constructor even without import, and it was accessible there ever since 3.0.0, only no version did report the import as unused so far.

The following code compiles fine with 3.0.0, but not with 2.13.16:

object Settings {
  def x = 0
}

case class Settings(value: Double = x)

I am not sure what the behaviour should be. To my naive understanding it seems symbol x should not be visible at that point without an import, but maybe there is some language rule changed between Scala 2.13 and Scala 3.

@som-snytt
Copy link
Contributor

som-snytt commented Mar 2, 2025

@OndrejSpanel ah right. I'm aware that default args (perhaps especially for constructors) is a bit unsettled in Scala 3. (There was some hint about changing how they are supported.) In any case, you're correct that an import is required for the default arg. This sort of "unhygienic" code movement is a bug.

(There are limitations on enums that are similarly based on "where the code winds up", but those are limits to visibility, as opposed to permitting visibility.)

@som-snytt som-snytt changed the title 3.6.4 regression - false positive warnings: unused imports 3.7 regression - false negative warning: unused imports Mar 3, 2025
@som-snytt
Copy link
Contributor

This was a false alarm on 3.6 but uncovered a false negative on forthcoming 3.7. Re-re-opening to track the progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:linting Linting warnings enabled with -W or -Xlint itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants