Skip to content

Top-level private definitions are not reported as unused #20520

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
rochala opened this issue Jun 4, 2024 · 1 comment · Fixed by #20894
Closed

Top-level private definitions are not reported as unused #20520

rochala opened this issue Jun 4, 2024 · 1 comment · Fixed by #20894
Labels
area:linting Linting warnings enabled with -W or -Xlint itype:bug

Comments

@rochala
Copy link
Contributor

rochala commented Jun 4, 2024

Compiler version

3.5.1-RC1-bin-20240602-c6fbe6f-NIGHTLY

Minimized code

Top level private definitions are not reported as unused.

//> using scala 3.nightly
//> using option -Wunused:all

@main def run =
  val veryUnusedVariable: Int = ???

private def veryUnusedMethod(x: Int): Unit = ???
private val veryUnusedVariableToplevel: Unit = ???


Output

//> using scala 3.nightly
//> using option -Wunused:all

@main def run =
  val veryUnusedVariable: Int = ??? // unused local definition

private def veryUnusedMethod(x: Int): Unit = ??? // no unused definition reported
private val veryUnusedVariableToplevel: Unit = ??? // no unused definition reported

Expectation

//> using scala 3.nightly
//> using option -Wunused:all

@main def run =
  val veryUnusedVariable: Int = ??? // unused local definition

private def veryUnusedMethod(x: Int): Unit = ??? // unused local definition
private val veryUnusedVariableToplevel: Unit = ??? // unused local definition
@som-snytt
Copy link
Contributor

This is as expected, because the example is

private[<empty>] def veryUnusedMethod(x: Int): Unit = println()

and if it's in a named package, then it is private to the package. The members can be accessed under separate compilation.

Possibly, the expectation was that private means private to the file or the package object, but that is not the case.

Also members that are unimplemented with ??? do not warn for unused, and their params also do not warn.

Closing here as not a bug, but there is a test for status quo.

@som-snytt som-snytt closed this as not planned Won't fix, can't repro, duplicate, stale Jan 13, 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: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