Skip to content

Refine ignored cases for unused params check #20973

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
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke

override def prepareForDefDef(tree: tpd.DefDef)(using Context): Context =
unusedDataApply{ ud =>
if !tree.symbol.is(Private) then
if !tree.symbol.owner.isTopLevelDefinitionsObject && tree.symbol.owner.isClass && !tree.symbol.is(Private) then
tree.termParamss.flatten.foreach { p =>
ud.addIgnoredParam(p.symbol)
}
Expand Down
8 changes: 7 additions & 1 deletion tests/semanticdb/metac.expect
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ Text => empty
Language => Scala
Symbols => 108 entries
Occurrences => 127 entries
Diagnostics => 6 entries
Diagnostics => 7 entries
Synthetics => 2 entries

Symbols:
Expand Down Expand Up @@ -830,6 +830,7 @@ See: https://docs.scala-lang.org/scala3/reference/dropped-features/this-qualifie
This construct can be rewritten automatically under -rewrite -source 3.4-migration.
[22:27..22:28): [warning] unused explicit parameter
[24:10..24:11): [warning] unused explicit parameter
[51:30..51:31): [warning] unused explicit parameter

Synthetics:
[51:16..51:27):List(1).map => *[Int]
Expand Down Expand Up @@ -3535,6 +3536,7 @@ Text => empty
Language => Scala
Symbols => 52 entries
Occurrences => 137 entries
Diagnostics => 2 entries
Synthetics => 39 entries

Symbols:
Expand Down Expand Up @@ -3730,6 +3732,10 @@ Occurrences:
[57:12..57:15): Int -> scala/Int#
[58:6..58:9): foo -> example/Synthetic#Contexts.foo().

Diagnostics:
[19:21..19:22): [warning] unused explicit parameter
[41:4..41:5): [warning] unused explicit parameter

Synthetics:
[5:2..5:13):List(1).map => *[Int]
[5:2..5:6):List => *.apply[Int]
Expand Down
2 changes: 1 addition & 1 deletion tests/warn/i15503e.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ package scala3main:
package foo.test.lambda.param:
val default_val = 1
val a = (i: Int) => i // OK
val b = (i: Int) => default_val // OK
val b = (i: Int) => default_val // warn
val c = (_: Int) => default_val // OK

package foo.test.trivial:
Expand Down
2 changes: 1 addition & 1 deletion tests/warn/i15503i.scala
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ package foo.test.scala.annotation:
val default_int = 12

def a1(a: Int) = a // OK
def a2(a: Int) = default_int // ok
def a2(a: Int) = default_int // warn

def a3(@unused a: Int) = default_int //OK

Expand Down
12 changes: 12 additions & 0 deletions tests/warn/i20951.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- [E198] Unused Symbol Warning: tests/warn/i20951.scala:5:9 -----------------------------------------------------------
5 |def test(x: Int): Int = dummy // warn
| ^
| unused explicit parameter
-- [E198] Unused Symbol Warning: tests/warn/i20951.scala:8:32 ----------------------------------------------------------
8 | def f(): Unit = Option(1).map(x => dummy) // warn
| ^
| unused explicit parameter
-- [E198] Unused Symbol Warning: tests/warn/i20951.scala:10:16 ---------------------------------------------------------
10 | private def h(x: Int): Int = dummy // warn
| ^
| unused explicit parameter
20 changes: 20 additions & 0 deletions tests/warn/i20951.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//> using options -Wunused:params
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's the case that this subsumes the ticket for warning in a for comprehension (e.g. the case for x <- list yield y // warn for x, #18289) then we should add a test for that syntax as well since that ticket will no longer require any changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also would not consider it a problem that the for comprehension warning is unused parameter instead of local. The only way around that would run into the same issues as #18854 and may be subject to the for-comprehension desugaring changes.


// We need the dummy variable: with a constant the unused:params does not trigger a warning but unused:all does.
val dummy = 42
def test(x: Int): Int = dummy // warn

object Foo:
def f(): Unit = Option(1).map(x => dummy) // warn
def g(x: Int): Int = dummy // ok
private def h(x: Int): Int = dummy // warn
def main(args: Array[String]): Unit = {} // ok

trait Bar:
def f(x: Int): Int = dummy // ok

abstract class Baz:
def f(x: Int): Int = dummy // ok

class Qux:
def f(x: Int): Int = dummy // ok
4 changes: 2 additions & 2 deletions tests/warn/scala2-t11681.scala
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,13 @@ trait Proofs {
}

trait Anonymous {
def f = (i: Int) => answer // ok
def f = (i: Int) => answer // warn

def f1 = (_: Int) => answer // OK

def f2: Int => Int = _ + 1 // OK

def g = for (i <- List(1)) yield answer // ok
def g = for (i <- List(1)) yield answer // warn
}
trait Context[A]
trait Implicits {
Expand Down
Loading