Scala 2.13.15 marks pattern vars as unused in for comprehension if there is a filter

[dragisak]

Reproduction steps

Scala version: 2.13.15 Sbt: 1.10.2 Scalac options:

ThisBuild / scalacOptions ++= List(
  "-Xfatal-warnings",
  "-Ywarn-unused:patvars"
)

  val m = Map(
    "first" -> Map((true, 1), (false, 2), (true, 3)),
    "second" -> Map((true, 1), (false, 2), (true, 3))
  )
  m.map { case (a, m1) =>
    for {
      (status, lag) <- m1 if status
    } yield (a, status, lag)
  }

Problem

Getting compilation error:

[error] pattern var lag in value $anonfun is never used
[error]       (status, lag) <- m1 if status

If I remove if clause, it compiles:

  val m = Map(
    "first" -> Map((true, 1), (false, 2), (true, 3)),
    "second" -> Map((true, 1), (false, 2), (true, 3))
  )
  m.map { case (a, m1) =>
    for {
      (status, lag) <- m1
    } yield (a, status, lag)
  }

[dragisak]

If I write it like this, it's Ok:

  for {
    (a, m1) <- m
    (status, lag) <- m1 if status
  } yield (a, status, lag)

[drewfeelsblue]

  val m = Map(
    "first" -> Map((true, 1), (false, 2), (true, 3)),
    "second" -> Map((true, 1), (false, 2), (true, 3))
  )
  m.map { case (a, m1) =>
    for {
      (status, lag) <- m1
      c = 1
    } yield (a, status, lag, c)
  }

[error] pattern var status in value $anonfun is never used
[error]       (status, lag) <- m1
[error]        ^
[error] pattern var lag in value $anonfun is never used
[error]       (status, lag) <- m1
[error]                ^
[error] pattern var c in value $anonfun is never used
[error]       c = 1
[error]       ^
[error] three errors found

[SethTisue]

@som-snytt is unused-patvars part of the default -Wunused behavior, or does it have to be explicitly enabled?

[som-snytt]

It is a regular unused, but maybe you mean is it in -Xlint, and no not yet

  unused                  Enable -Wunused:imports,privates,locals,implicits,nowarn.

like params it is deemed too noisy for most code style.

The problem here is that the enclosing case breaks the mechanism for tracking when patvars are copied during parser -- tree copying during type checking doesn't update the links. I'll add a solution to the PR about the tree position.

Also not to neglect to say thanks @dragisak for reporting and for the follow-up comment.

[joroKr21]

Dunno, but it wasn't happening in 2.13.14 with the same compiler options

[som-snytt]

@joroKr21 it's a new feature; previously the lint did not cope at all with rewrites in parser.

[Christewart]

I have the same false positive error: https://github.com/bitcoin-s/bitcoin-s/actions/runs/11086656402/job/30804463250?pr=5686#step:5:197

the variable (hash) compiler claims is unused: https://github.com/scala-steward/bitcoin-s/blob/06a2c6c51b1da0822db94414bc03de705171d3d0/wallet/src/main/scala/org/bitcoins/wallet/internal/TransactionProcessing.scala#L136

The line using the variable hash

https://github.com/scala-steward/bitcoin-s/blob/06a2c6c51b1da0822db94414bc03de705171d3d0/wallet/src/main/scala/org/bitcoins/wallet/internal/TransactionProcessing.scala#L137

[som-snytt]

@Christewart thanks, the feature is brittle depending on context; I have a different approach to PR and will check out your examples.

Edit: thanks again, it was helpful to have a codebase to test against (because I had a typo...).

I didn't run tests, but I see the idiom for (...) yield () which is map instead of foreach. I did not refresh my memory about Future, but I wondered if there is something about Future semantics that makes the yield intentional. The resulting future is not used.

[hughsimpson]

Is there a better workaround for this issue, or do I just have to add -Wconf:cat=unused-pat-vars:s for now?

[som-snytt]

@hughsimpson It's not enabled under -Xlint, so I would not enable it, or -Wunused:-patvars,_, however you prefer to do it.

[hughsimpson]

I've been using -Wunused, but didn't know about the -Wunused:-disabledCheck,_ construction, thanks. That feels nicer.

[Christewart]

I didn't run tests, but I see the idiom for (...) yield () which is map instead of foreach. I did not refresh my memory about Future, but I wondered if there is something about Future semantics that makes the yield intentional. The resulting future is not used.

The resulting future (f) is used here to register various callbacks

https://github.com/scala-steward/bitcoin-s/blob/06a2c6c51b1da0822db94414bc03de705171d3d0/wallet/src/main/scala/org/bitcoins/wallet/internal/TransactionProcessing.scala#L143-157

[emanb29]

We've got another example here: https://github.com/thatdot/quine/blob/77e1040ef282f3db5581985709f60032a273a24c/quine-core/src/main/scala/com/thatdot/quine/graph/cypher/Expr.scala#L1023-L1029

Like @Christewart and @drewfeelsblue, our usage is just flatMaps and maps, and like @drewfeelsblue, the pattern variable is used to construct the yield-ed result. I have not been able to try the compilation on the https://github.com/scala/scala/pull/10870 branch

[som-snytt]

@emanb29 thanks, I'll give it a try, since I can do it without upgrading dependencies. I expect it to work.

@Christewart thanks for the reply. Obviously, I rely entirely on tooling to detect usages! I have no idea now what I was looking at.

Update: confirmed that it works. This was a good opportunity to remember to delete the test version of "2.13.15" from my local cache.