From 7c1a32db088ccbaab2aa29aed279e2bfdead45f0 Mon Sep 17 00:00:00 2001 From: jxnu-liguobin Date: Thu, 29 Feb 2024 14:47:06 +0800 Subject: [PATCH] https://github.com/lightbend-labs/scala-logging/issues/354 After wrapping varargs, the user code fails to compile. In Scala 2, there were no inline parameters, and the subtype of args was obtained during compilation. However, this approach may not always be accurate. Regarding https://github.com/lightbend-labs/scala-logging/issues/191 There is an issue with obtaining inline parameters in Scala 3. To address this, we recursively obtain the actual value of inline parameters. This necessitates the ongoing use of inlining in the parameters of the wrapper function. For instance: ```scala class LogWrapper(val underlying: Logger) { inline def info(message: String, inline args: AnyRef*): Unit = underlying.info(message, args: _*) } ``` This ensures compatibility and accurate handling of inline parameters in both Scala 2 and Scala 3. --- .../typesafe/scalalogging/LoggerMacro.scala | 10 ++++- .../typesafe/scalalogging/LoggerMacro.scala | 19 ++++---- .../scalalogging/Scala2LoggerSpec.scala | 45 +++++++++++++++++++ .../scalalogging/Scala3LoggerSpec.scala | 44 ++++++++++++++++++ .../typesafe/scalalogging/LoggerSpec.scala | 8 ++++ 5 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 src/test/scala-2/com/typesafe/scalalogging/Scala2LoggerSpec.scala create mode 100644 src/test/scala-3/com/typesafe/scalalogging/Scala3LoggerSpec.scala diff --git a/src/main/scala-2/com/typesafe/scalalogging/LoggerMacro.scala b/src/main/scala-2/com/typesafe/scalalogging/LoggerMacro.scala index dbef6b2..0a7cb81 100644 --- a/src/main/scala-2/com/typesafe/scalalogging/LoggerMacro.scala +++ b/src/main/scala-2/com/typesafe/scalalogging/LoggerMacro.scala @@ -295,7 +295,15 @@ private[scalalogging] object LoggerMacro { private def formatArgs(c: LoggerContext)(args: c.Expr[Any]*) = { import c.universe._ args.map { arg => - c.Expr[AnyRef](if (arg.tree.tpe <:< weakTypeOf[AnyRef]) q"$arg: _root_.scala.AnyRef" else q"$arg.asInstanceOf[_root_.scala.AnyRef]") + // If arg is a varargs, it is also a AnyRef and we need to check the subtree. + val argument = arg.tree.children.map(_.tpe) match { + case head :: next :: Nil if head <:< weakTypeOf[scala.collection.Seq[_]] && next <:< weakTypeOf[AnyRef] => + q"$arg" + case _ => + if (arg.tree.tpe <:< weakTypeOf[AnyRef]) q"$arg: _root_.scala.AnyRef" + else q"$arg.asInstanceOf[_root_.scala.AnyRef]" + } + c.Expr[AnyRef](argument) } } } diff --git a/src/main/scala-3/com/typesafe/scalalogging/LoggerMacro.scala b/src/main/scala-3/com/typesafe/scalalogging/LoggerMacro.scala index a52ff05..368c42e 100644 --- a/src/main/scala-3/com/typesafe/scalalogging/LoggerMacro.scala +++ b/src/main/scala-3/com/typesafe/scalalogging/LoggerMacro.scala @@ -264,19 +264,18 @@ private[scalalogging] object LoggerMacro { def formatArgs(args: Expr[Seq[Any]])(using q: Quotes): Seq[Expr[AnyRef]] = { import quotes.reflect.* import util.* - - args.asTerm match { - case p@Inlined(_, _, Typed(Repeated(v, _),_)) => - v.map{ - case t if t.tpe <:< TypeRepr.of[AnyRef] => t.asExprOf[AnyRef] - case t => '{${t.asExpr}.asInstanceOf[AnyRef]} - } - case Repeated(v, _) => - v.map{ + // we recursively obtain the actual value of inline parameters + def rec(tree: Term): Option[Seq[Expr[AnyRef]]] = tree match { + case Repeated(elems, _) => Some( + elems.map { case t if t.tpe <:< TypeRepr.of[AnyRef] => t.asExprOf[AnyRef] case t => '{${t.asExpr}.asInstanceOf[AnyRef]} } - case _ => Seq.empty + ) + case Typed(e, _) => rec(e) + case Inlined(_, Nil, e) => rec(e) + case _ => None } + rec(args.asTerm).getOrElse(Seq.empty) } } diff --git a/src/test/scala-2/com/typesafe/scalalogging/Scala2LoggerSpec.scala b/src/test/scala-2/com/typesafe/scalalogging/Scala2LoggerSpec.scala new file mode 100644 index 0000000..3486f5d --- /dev/null +++ b/src/test/scala-2/com/typesafe/scalalogging/Scala2LoggerSpec.scala @@ -0,0 +1,45 @@ +package com.typesafe.scalalogging + +import org.mockito.Mockito._ +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec +import org.scalatestplus.mockito.MockitoSugar +import org.slf4j.{ Logger => Underlying } + +class Scala2LoggerSpec extends AnyWordSpec with Matchers with Varargs with MockitoSugar { + + // Info + "Calling info with an interpolated message, only scala 2" should { + "fix Varargs compilation error issue 354 only scala 2" in { + val f = fixture(_.isInfoEnabled, isEnabled = true) + import f._ + class LogWrapper(val underlying: Logger) { + def info(message: String, args: AnyRef*): Unit = underlying.info(message, args: _*) + } + val logWrapper = new LogWrapper(logger) + logWrapper.info("""Hello {}""", arg5ref) + verify(underlying).info("""Hello {}""", forceVarargs(arg5ref): _*) + } + } + + private def fixture(p: Underlying => Boolean, isEnabled: Boolean) = new LoggerF(p, isEnabled) + + private class LoggerF(p: Underlying => Boolean, isEnabled: Boolean) { + val msg = "msg" + val cause = new RuntimeException("cause") + val arg1 = "arg1" + val arg2: Integer = Integer.valueOf(1) + val arg3 = "arg3" + val arg4 = 4 + val arg4ref: AnyRef = arg4.asInstanceOf[AnyRef] + val arg5 = true + val arg5ref: AnyRef = arg5.asInstanceOf[AnyRef] + val arg6 = 6L + val arg6ref: AnyRef = arg6.asInstanceOf[AnyRef] + val arg7 = new Throwable + val arg7ref: AnyRef = arg7.asInstanceOf[AnyRef] + val underlying: Underlying = mock[org.slf4j.Logger] + when(p(underlying)).thenReturn(isEnabled) + val logger: Logger = Logger(underlying) + } +} diff --git a/src/test/scala-3/com/typesafe/scalalogging/Scala3LoggerSpec.scala b/src/test/scala-3/com/typesafe/scalalogging/Scala3LoggerSpec.scala new file mode 100644 index 0000000..81f55b6 --- /dev/null +++ b/src/test/scala-3/com/typesafe/scalalogging/Scala3LoggerSpec.scala @@ -0,0 +1,44 @@ +package com.typesafe.scalalogging + +import org.mockito.Mockito._ +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec +import org.scalatestplus.mockito.MockitoSugar +import org.slf4j.{Logger => Underlying} + +class Scala3LoggerSpec extends AnyWordSpec with Matchers with Varargs with MockitoSugar { + + // Info + "Calling info with an interpolated message, only scala 3" should { + "fix Varargs compilation error issue 354 only scala 3" in { + val f = fixture(_.isInfoEnabled, isEnabled = true) + import f._ + class LogWrapper(val underlying: Logger) { + inline def info(message: String, inline args: AnyRef*): Unit = underlying.info(message, args: _*) + } + val logWrapper = new LogWrapper(logger) + logWrapper.info("""Hello {}""", arg5ref) + verify(underlying).info("""Hello {}""", arg5ref) + } + } + + private def fixture(p: Underlying => Boolean, isEnabled: Boolean) = new LoggerF(p, isEnabled) + private class LoggerF(p: Underlying => Boolean, isEnabled: Boolean) { + val msg = "msg" + val cause = new RuntimeException("cause") + val arg1 = "arg1" + val arg2: Integer = Integer.valueOf(1) + val arg3 = "arg3" + val arg4 = 4 + val arg4ref: AnyRef = arg4.asInstanceOf[AnyRef] + val arg5 = true + val arg5ref: AnyRef = arg5.asInstanceOf[AnyRef] + val arg6 = 6L + val arg6ref: AnyRef = arg6.asInstanceOf[AnyRef] + val arg7 = new Throwable + val arg7ref: AnyRef = arg7.asInstanceOf[AnyRef] + val underlying: Underlying = mock[org.slf4j.Logger] + when(p(underlying)).thenReturn(isEnabled) + val logger: Logger = Logger(underlying) + } +} diff --git a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala index 0082c6d..ca8695a 100644 --- a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala +++ b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala @@ -207,6 +207,14 @@ class LoggerSpec extends AnyWordSpec with Matchers with Varargs with MockitoSuga logger.info(s"msg $arg1 $arg2") verify(underlying).info("msg {} {}", forceVarargs(arg1, arg2): _*) } + + "fix Varargs compilation error, issue 191" in { + val f = fixture(_.isInfoEnabled, isEnabled = true) + import f._ + val message = "Hello" + logger.info(s"Message with id=$message, submittedAt=$message will be dropped.") + verify(underlying).info("""Message with id={}, submittedAt={} will be dropped.""", forceVarargs(message, message): _*) + } } "Calling info with a message and cause" should {