scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

[2.13.15] False negatives for unused pattern vars #13035

Closed bjaglin closed 2 months ago

bjaglin commented 2 months ago

It seems that 2.13.15 no longer reports unused pattern vars when they have the same name as the attributes.

Reproduction steps

//> using scala 2.13.15
//> using options -Wunused

case class CC(field1: Int, field2: Int)

object CC {
  CC(42, 42) match {
    case CC(field1, field2) => 1
  }
}

Problem

Actual

No warning

$ scala-cli compile PatternVars.scala 
$

Expected

Warnings like in 2.13.14

$ scala-cli compile -S 2.13.14 PatternVars.scala 
[warn] ./PatternVars.scala:8:21
[warn] pattern var field2 in value <local CC> is never used: use a wildcard `_` or suppress this warning with `field2@_`
[warn]     case CC(field1, field2) => 1
[warn]                     ^^^^^^
[warn] ./PatternVars.scala:8:13
[warn] pattern var field1 in value <local CC> is never used: use a wildcard `_` or suppress this warning with `field1@_`
[warn]     case CC(field1, field2) => 1
[warn]             ^^^^^^
$
som-snytt commented 2 months ago

Thanks for the report and for using -Wunused!

This is a feature, never to warn on the "canonical pattern".

It's mentioned in the comment at https://github.com/scala/scala/pull/10812

but I don't see where lrytz proposed it. Possibly, the idea was inherited from -Xlint:pattern-shadow.

The experience in the scala/scala code base was that warning on case Apply(fun, args) for these lints was too noisy.

The previous workaround was case Apply(fun @ _, args @ _) to mean that the var names are documentary, but that is too verbose to be useful. That was a workaround for lacking named args in patterns.

So for warning, the idiom works like case x: Apply => import x.* if the import did not warn as unused.

A -strict option was not proposed because there are too many knobs, and the goal was the happy medium that errs on the side of false negative. A 3rd party lint could provide fine-grained config.

bjaglin commented 2 months ago

Oh, thanks! Do you know by any chance if there is a plan to forward-port this change to Scala 3 for consistency? I guess not - I couldn't find any related ticket.