Skip to content

Commit

Permalink
https://github.com/lightbend-labs/scala-logging/issues/354
Browse files Browse the repository at this point in the history
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 #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.
  • Loading branch information
liguobin committed Feb 29, 2024
1 parent da10c6c commit ccf7f5b
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 11 deletions.
10 changes: 9 additions & 1 deletion src/main/scala-2/com/typesafe/scalalogging/LoggerMacro.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
19 changes: 9 additions & 10 deletions src/main/scala-3/com/typesafe/scalalogging/LoggerMacro.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
45 changes: 45 additions & 0 deletions src/test/scala-2/com/typesafe/scalalogging/Scala2LoggerSpec.scala
Original file line number Diff line number Diff line change
@@ -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)
}
}
44 changes: 44 additions & 0 deletions src/test/scala-3/com/typesafe/scalalogging/Scala3LoggerSpec.scala
Original file line number Diff line number Diff line change
@@ -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)
}
}
8 changes: 8 additions & 0 deletions src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit ccf7f5b

Please sign in to comment.