Skip to content

False positive deprecation warnings when -Wunused:imports is used in 3.4.0-RCx #19675

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
kyri-petrou opened this issue Feb 12, 2024 · 11 comments · Fixed by #21159
Closed

False positive deprecation warnings when -Wunused:imports is used in 3.4.0-RCx #19675

kyri-petrou opened this issue Feb 12, 2024 · 11 comments · Fixed by #21159
Assignees
Labels
area:linting Linting warnings enabled with -W or -Xlint area:metaprogramming:quotes Issues related to quotes and splices itype:bug stat:needs minimization Needs a self contained minimization

Comments

@kyri-petrou
Copy link

Compiler version

All of 3.4.0-RCx (no issues with 3.3.x)

Minimized code

Unfortunately I wasn't able to minimize, but I hope I can explain the issue well enough to identify what's causing it

Context / Setup

One of our (large) service at $WORK uses Scala 3 for quite some time now. I wanted to try out 3.4.0-RC4, but came across a very weird regression when Wunused:imports is enabled.

The service uses Thrift clients that are generated via scrooge, and are defined as a dependency via the Scala 2.13 cross CrossVersion.for3Use2_13 flag. The client is initialized as shown below (Note that the reflectiveSelectable import is required so that we can close the client. I also tried removing the reflection, but the issue still persists)

  import reflect.Selectable.reflectiveSelectable

  type SvcType = {
    def asClosable: Closable
  } & ThriftService

  def createServiceClient[S <: SvcType](address: String)(using ClassTag[S]): S =
    Thrift.client.build[S](address)

Issue

Since 3.4.0-RC1, having the following code in the project causes ~3,500 deprecation warnings to be raised, many of them that are not even part of the project (i.e., 3rd-party libraries)

...
[warn] .../src/main/scala-3/io/scalaland/chimney/internal/compiletime/ExprPromisesPlatform.scala:  <none> is deprecated since 2019-10-03: Use thriftReusableBufferFactory instead
[warn] .../src/main/scala-3/zio/ZLayerCompanionVersionSpecific.scala:  <none> is deprecated since 2019-10-03: Use thriftReusableBufferFactory instead
[warn] 3387 warnings found

After searching for the 2019-10-03 string in the finagle codebase, I found this method which contains an argument that is annotated as deprecated.

The (very) weird thing here is that the service codebase does not call this method, at least not directly. Removing the -Wunused:imports compiler flag makes the issue go away. Similarly, using ??? as the body for the createServiceClient method above also makes the issue go away.

Hope this helps, please let me know if you need some more info to help track down what's causing this. Unfortunately, there's no workaround (unless linting is disabled which is not possible)

@kyri-petrou kyri-petrou added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Feb 12, 2024
@Gedochao Gedochao added stat:needs info stat:needs minimization Needs a self contained minimization 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 Feb 13, 2024
@Gedochao
Copy link
Contributor

The question here if this is indeed a valid bug, or if the deprecation warning should indeed have been raised, while it wasn't in earlier versions... Hard to say without minimising this.

Tried reproducing around code similar to

//> using scala 3.4.0-RC4
//> using dep com.twitter:finagle-thrift_2.13:23.11.0
//> using options -Wunused:imports
import com.twitter.finagle.thrift.RichClientParam.apply

But it behaves as expected (as it should).
I can't seem to get any weird behaviour with the following, either.

object ThriftMayBehaveFunny {
  def reproAttempt = com.twitter.finagle.thrift.RichClientParam(
    maxThriftBufferSize = 1
  )
}
import ThriftMayBehaveFunny.reproAttempt

The (very) weird thing here is that the service codebase does not call this method, at least not directly. Removing the -Wunused:imports compiler flag makes the issue go away. Similarly, using ??? as the body for the createServiceClient method above also makes the issue go away.

@kyri-petrou we may need something more to go on. Perhaps you'd be able to find the indirect way com.twitter.finagle.thrift.RichClientParam.apply may be called? Or maybe it's some other deprecation entirely?

Bringing in @szymon-rd to the conversation, perhaps there's something obvious here I can't see.

@szymon-rd
Copy link
Contributor

Do you maybe know why it reports warnings in directories like src/main/scala-3/io/scalaland/chimney/internal/? Are you building dependencies from sources?

@kyri-petrou
Copy link
Author

kyri-petrou commented Feb 13, 2024

This is the closest I can get to a repro / minimization. Note that the zio dependency / zlayer construction is added because the issue doesn't exist without it. It could have been be some other library / macro, but this is the first that came to mind. The issue seems to be present only whenever a macro is involved:

//> using scala 3.4.0-RC4
//> using dep com.twitter:finagle-thrift_2.13:23.11.0
//> using dep dev.zio::zio:2.0.21
//> using options -Wunused:imports -deprecation 

import com.twitter.finagle.*
import com.twitter.finagle.thrift.ThriftService
import scala.reflect.ClassTag
import zio.*

final class Unrelated()

object Unrelated {
    val layer = ZLayer.derive[Unrelated]
}

object Main {
    def foo[S <: ThriftService](using ClassTag[S]) = Thrift.client.build[S]("asd")
}

@kyri-petrou
Copy link
Author

Note that in the service I mentioned I also noticed the issue with some inline def methods, but I couldn't figure out how to reproduce it using one of those

@kyri-petrou
Copy link
Author

Do you maybe know why it reports warnings in directories like src/main/scala-3/io/scalaland/chimney/internal/? Are you building dependencies from sources?

Nope, doing nothing like this. I hope the reproduction above might shed some light on what's going on because it's really puzzling behaviour

@Gedochao
Copy link
Contributor

Okay... we'll still need to minimise it down to a snippet without the zio and finagle dependencies, but at the very least we have some hints now, thanks @kyri-petrou !
Do let us know if you manage to minimise it further.

@Gedochao Gedochao added the area:metaprogramming:quotes Issues related to quotes and splices label Feb 13, 2024
@kyri-petrou
Copy link
Author

Okay... we'll still need to minimise it down to a snippet without the zio and finagle dependencies, but at the very least we have some hints now, thanks @kyri-petrou ! Do let us know if you manage to minimise it further.

Managed to reproduce without ZIO using just an inline method:

//> using scala 3.4.0-RC4
//> using dep com.twitter:finagle-thrift_2.13:23.11.0
//> using options -Wunused:imports -deprecation

import com.twitter.finagle.*
import com.twitter.finagle.thrift.ThriftService
import scala.reflect.ClassTag

trait Foo[A]

object Foo {
    inline def make[A]: Foo[A] = new {}
}

final class Unrelated()

object Unrelated {
    val foo = Foo.make[Unrelated]
}

object Main {
    def foo[S <: ThriftService](using ClassTag[S]) = Thrift.client.build[S]("asd")
}

I'm afraid that's as far as I can minimize it for now

@Gedochao
Copy link
Contributor

@kyri-petrou I can't reproduce the problem with your last minimisation without ZIO.

it works with the one with both ZIO and thrift (reposting from #19675 (comment))

//> using scala 3.4.0-RC4
//> using dep com.twitter:finagle-thrift_2.13:23.11.0
//> using dep dev.zio::zio:2.0.21
//> using options -Wunused:imports -deprecation 

import com.twitter.finagle.*
import com.twitter.finagle.thrift.ThriftService
import scala.reflect.ClassTag
import zio.*

final class Unrelated()

object Unrelated {
    val layer = ZLayer.derive[Unrelated]
}

object Main {
    def foo[S <: ThriftService](using ClassTag[S]) = Thrift.client.build[S]("asd")
}

gives me the output:

scala-cli compile repro.scala --server=false                              
# -- Deprecation Warning: core/shared/src/main/scala-3/zio/internal/macros/ZLayerDerivationMacros.scala:104:48 
#  <none> is deprecated since 2019-10-03: Use thriftReusableBufferFactory instead
# 1 warning found

removing --server=false makes Bloop go bonkers, posting here for context:

scala-cli compile repro.scala               
Compiling project (Scala 3.4.0-RC4, JVM (17))
[warn] /Users/pchabelski/Library/Caches/ScalaCli/bloop/core/shared/src/main/scala-3/zio/internal/macros/ZLayerDerivationMacros.scala:1:1
[warn]  <none> is deprecated since 2019-10-03: Use thriftReusableBufferFactory instead
Feb 13, 2024 2:43:22 PM org.eclipse.lsp4j.jsonrpc.RemoteEndpoint handleNotification
WARNING: Notification threw an exception: {
  "jsonrpc": "2.0",
  "method": "build/publishDiagnostics",
  "params": {
    "textDocument": {
      "uri": "file:///Users/pchabelski/Library/Caches/ScalaCli/bloop/core/shared/src/main/scala-3/zio/internal/macros/ZLayerDerivationMacros.scala"
    },
    "buildTarget": {
      "uri": "file:/Users/pchabelski/IdeaProjects/scala-cli-tests/finagle-deprecation/.scala-build/?id\u003dfinagle-deprecation_6fb237c6a8-4fa4765211"
    },
    "diagnostics": [
      {
        "range": {
          "start": {
            "line": 0,
            "character": 0
          },
          "end": {
            "line": 0,
            "character": 0
          }
        },
        "severity": 2,
        "message": "\u001b[33m\u001b[0m \u001b[35m\u003cnone\u003e\u001b[0m is deprecated since 2019-10-03: Use thriftReusableBufferFactory instead",
        "data": {
          "actions": []
        }
      }
    ],
    "reset": true
  }
}
java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
        at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.lambda$null$0(GenericEndpoint.java:67)
        at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.notify(GenericEndpoint.java:152)
        at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleNotification(RemoteEndpoint.java:220)
        at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:187)
        at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:194)
        at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:94)
        at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
        at [email protected]/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
        at [email protected]/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at [email protected]/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at [email protected]/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at [email protected]/java.lang.Thread.run(Thread.java:833)
        at com.oracle.svm.core.thread.PlatformThreads.threadStartRoutine(PlatformThreads.java:775)
        at com.oracle.svm.core.posix.thread.PosixPlatformThreads.pthreadStartRoutine(PosixPlatformThreads.java:203)
Caused by: java.lang.reflect.InvocationTargetException
        at [email protected]/java.lang.reflect.Method.invoke(Method.java:568)
        at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.lambda$null$0(GenericEndpoint.java:65)
        ... 13 more
Caused by: java.nio.file.NoSuchFileException: /Users/pchabelski/Library/Caches/ScalaCli/bloop/core/shared/src/main/scala-3/zio/internal/macros/ZLayerDerivationMacros.scala
        at [email protected]/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:218)
        at [email protected]/java.nio.file.Files.newByteChannel(Files.java:380)
        at [email protected]/java.nio.file.Files.newByteChannel(Files.java:432)
        at [email protected]/java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:422)
        at [email protected]/java.nio.file.Files.newInputStream(Files.java:160)
        at os.Path.getInputStream(Path.scala:540)
        at os.read$lines$stream$$anon$3.generate(ReadWriteOps.scala:361)
        at geny.Generator.foreach(Generator.scala:51)
        at geny.Generator.foreach$(Generator.scala:33)
        at os.read$lines$stream$$anon$3.foreach(ReadWriteOps.scala:359)
        at geny.Generator.toBuffer(Generator.scala:127)
        at geny.Generator.toBuffer$(Generator.scala:33)
        at os.read$lines$stream$$anon$3.toBuffer(ReadWriteOps.scala:359)
        at geny.Generator.toArray(Generator.scala:131)
        at geny.Generator.toArray$(Generator.scala:33)
        at os.read$lines$stream$$anon$3.toArray(ReadWriteOps.scala:359)
        at os.read$lines$.apply(ReadWriteOps.scala:346)
        at scala.build.ConsoleBloopBuildClient$.$anonfun$4$$anonfun$1(ConsoleBloopBuildClient.scala:192)
        at scala.Option.map(Option.scala:242)
        at scala.build.ConsoleBloopBuildClient$.$anonfun$4(ConsoleBloopBuildClient.scala:192)
        at scala.Option.flatMap(Option.scala:283)
        at scala.build.ConsoleBloopBuildClient$.printFileDiagnostic(ConsoleBloopBuildClient.scala:194)
        at scala.build.ConsoleBloopBuildClient.onBuildPublishDiagnostics$$anonfun$2(ConsoleBloopBuildClient.scala:86)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
        at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:576)
        at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:574)
        at scala.collection.AbstractIterable.foreach(Iterable.scala:933)
        at scala.build.ConsoleBloopBuildClient.onBuildPublishDiagnostics(ConsoleBloopBuildClient.scala:87)
        ... 15 more

Compiled project (Scala 3.4.0-RC4, JVM (17))

This all goes away when compiled with 3.3.1

scala-cli compile repro.scala -S 3.3.1
# Compiling project (Scala 3.3.1, JVM (17))
# Compiled project (Scala 3.3.1, JVM (17))

However, I can't seem to get the same behaviour with the example without ZIO (#19675 (comment))

//> using scala 3.4.0-RC4
//> using dep com.twitter:finagle-thrift_2.13:23.11.0
//> using options -Wunused:imports -deprecation

import com.twitter.finagle.*
import com.twitter.finagle.thrift.ThriftService
import scala.reflect.ClassTag

trait Foo[A]

object Foo {
    inline def make[A]: Foo[A] = new {}
}

final class Unrelated()

object Unrelated {
    val foo = Foo.make[Unrelated]
}

object Main {
    def foo[S <: ThriftService](using ClassTag[S]) = Thrift.client.build[S]("asd")
}

It compiles just fine for me with 3.4.0-RC4, no unexpected warnings being logged.

scala-cli compile repro.scala
# Compiling project (Scala 3.4.0-RC4, JVM (17))
# Compiled project (Scala 3.4.0-RC4, JVM (17))

@kyri-petrou am I missing something with your last reproduction?

@kyri-petrou
Copy link
Author

@Gedochao this is bizarre. My last repro doesn't emit any warnings if the file has a .scala extension, but emits a warning if it has a .sc extension. Not sure if this is a red herring or not, but as I mentioned previously I noticed that inline def methods trigger the issue only in some cases (unclear when), whereas it's almost guaranteed to exist in the presence of macros

@kyri-petrou
Copy link
Author

kyri-petrou commented Feb 13, 2024

One last bit to note, the macro doesn't need to be in the same source file as Thrift.client.build[S]("asd"), it can be anywhere in the same SBT module (in the case of the service I mentioned) and it'll still trigger it

@Gedochao Gedochao assigned jchyb and unassigned szymon-rd May 10, 2024
@jchyb
Copy link
Contributor

jchyb commented Jul 10, 2024

Scala-cli wraps the contents of .sc files in an object (among other things), to allow running as scripts, easy addition of arguments etc., so the previous minimisation in a .scala file indeed showcases the problem when changed to this:

//> using dep com.twitter:finagle-thrift_2.13:24.2.0
//> using options -Wunused:imports -deprecation -Werror

import com.twitter.finagle.Thrift
import com.twitter.finagle.thrift.ThriftService
import scala.reflect.ClassTag

class Minim {
  trait Foo[A]
  
  object Foo {
    inline def make[A]: Foo[A] = ???
  }

  final class Unrelated()

  object Unrelated {
    val foo = Foo.make[Unrelated]
  }

  object Main {
    def foo[S <: ThriftService](using ClassTag[S]) = 
      Thrift.client.build[S]("asd")
  }
}

The issue actually appears only in 3.4.0 and 3.4.1, so it seems to have been fixed somewhere before 3.4.2. The bisect script points to #19926 as the fix, which seems to line up perfectly with what we see here. The false deprecation warning only appears here when using a scala-2 dependency, and the finagle deprecation itself comes from a case class accessor. It seems that before #19926, scala-2 case accessor symbols would be overzealously loaded (including our deprecation annotation), causing the unnecessary deprecation warnings.

I'll try to quickly prepare a regression test, but most likely it will scala-2-compat scripted sbt test, like in the previously mentioned PR.

WojciechMazur pushed a commit that referenced this issue Aug 28, 2024
Originally fixed by #19926
Closes #19675

Even though this is a slower sbt scripted test, I think it's worth
adding, since it showcases a different issue than what #19926 was
fixing, and I do not believe it is reproducible in any way without a
scala-2 dependency (so we cannot minimize it into regular compilation
test).
[Cherry-picked 53a40b4]
WojciechMazur pushed a commit that referenced this issue Dec 3, 2024
Originally fixed by #19926
Closes #19675

Even though this is a slower sbt scripted test, I think it's worth
adding, since it showcases a different issue than what #19926 was
fixing, and I do not believe it is reproducible in any way without a
scala-2 dependency (so we cannot minimize it into regular compilation
test).
[Cherry-picked 53a40b4]
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 area:metaprogramming:quotes Issues related to quotes and splices itype:bug stat:needs minimization Needs a self contained minimization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants