Fix some varargs issues in scala 2 and scala 3

[jxnu-liguobin]

Regarding 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 , https://github.com/lightbend-labs/scala-logging/issues/436

These two are about correctly obtaining inline parameters in scala3

To fix this, we recursively obtain the actual value of inline parameters.

For #191, we need to continue using inlining in the parameters of the wrapper function. for example. For instance:

class LogWrapper(val underlying: Logger) {
   // must inline
    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.

For #436, now we can support this:

// work well
inline def argss = Seq("1")
logger.info("""Hello {}""", argss*)

// also work well
logger.info("""Hello {}""", Seq(arg5ref)*)
logger.info("""Hello {}""", forceVarargs(arg5ref):_*)

val argss = Seq("1")
logger.info("""Hello {}""", argss*)

Note that this is equivalent to using hard-coded match syntax trees. Another option is to add specific APIs that do not not use formatArgs

[lightbend-cla-validator]

Hi @jxnu-liguobin,

Thank you for your contribution! We really value the time you've taken to put this together.

We see that you have signed the Lightbend Contributors License Agreement before, however, the CLA has changed since you last signed it. Please review the new CLA and sign it before we proceed with reviewing this pull request:

http://www.lightbend.com/contribute/cla

[SethTisue]

I don't know Scala 3 macros or the code in this library, so I'm not able to review this.

[SakulK]

Unfortunately this change doesn't solve the Scala 3 issue I wrote about in https://github.com/lightbend-labs/scala-logging/issues/354#issuecomment-1969395962 here's a test I tried in Scala3LoggerSpec:

    "work when passing a Seq as repeated arguments" in {
      val f = fixture(_.isInfoEnabled, isEnabled = true)
      import f._
      val args = Seq(arg5ref)
      logger.info("""Hello {}""", args*)
      verify(underlying).info("""Hello {}""", arg5ref)
    }

Output:

info] - should work when passing a Seq as repeated arguments *** FAILED ***
[info]   org.mockito.exceptions.verification.ArgumentsAreDifferent: Argument(s) are different! Wanted:
[info] logger.info("Hello {}", true);
[info] -> at com.typesafe.scalalogging.Scala3LoggerSpec.f$proxy2$1(Scala3LoggerSpec.scala:29)
[info] Actual invocations have different arguments:
[info] logger.isInfoEnabled();
[info] -> at com.typesafe.scalalogging.Scala3LoggerSpec.f$proxy2$1(Scala3LoggerSpec.scala:28)
[info] logger.info("Hello {}");
[info] -> at com.typesafe.scalalogging.Scala3LoggerSpec.f$proxy2$1(Scala3LoggerSpec.scala:28)
[info]   at com.typesafe.scalalogging.Scala3LoggerSpec.f$proxy2$1(Scala3LoggerSpec.scala:29)
[info]   at com.typesafe.scalalogging.Scala3LoggerSpec.$init$$$anonfun$1$$anonfun$2(Scala3LoggerSpec.scala:24)

[jxnu-liguobin]

logger.info("""Hello {}""", args*)

Unfortunately this change doesn't solve the Scala 3 issue I wrote about in #354 (comment) here's a test I tried in Scala3LoggerSpec:

    "work when passing a Seq as repeated arguments" in {
      val f = fixture(_.isInfoEnabled, isEnabled = true)
      import f._
      val args = Seq(arg5ref)
      logger.info("""Hello {}""", args*)
      verify(underlying).info("""Hello {}""", arg5ref)
    }

Output:

info] - should work when passing a Seq as repeated arguments *** FAILED ***
[info]   org.mockito.exceptions.verification.ArgumentsAreDifferent: Argument(s) are different! Wanted:
[info] logger.info("Hello {}", true);
[info] -> at com.typesafe.scalalogging.Scala3LoggerSpec.f$proxy2$1(Scala3LoggerSpec.scala:29)
[info] Actual invocations have different arguments:
[info] logger.isInfoEnabled();
[info] -> at com.typesafe.scalalogging.Scala3LoggerSpec.f$proxy2$1(Scala3LoggerSpec.scala:28)
[info] logger.info("Hello {}");
[info] -> at com.typesafe.scalalogging.Scala3LoggerSpec.f$proxy2$1(Scala3LoggerSpec.scala:28)
[info]   at com.typesafe.scalalogging.Scala3LoggerSpec.f$proxy2$1(Scala3LoggerSpec.scala:29)
[info]   at com.typesafe.scalalogging.Scala3LoggerSpec.$init$$$anonfun$1$$anonfun$2(Scala3LoggerSpec.scala:24)

Indeed, I just realized that it only solves the problem in #354 for scala3 and scala 2.

so logger.info("""Hello {} {}""", args:_*) is still unavailable

[jxnu-liguobin]

Hi@SakulK ,Since val can't be inlined, no workaround has been found, but functions are possible.

inline def argss = Seq("1")
  logger.info("""Hello {}""", argss*)

// also work well
logger.info("""Hello {}""", Seq(arg5ref)*)
logger.info("""Hello {}""", forceVarargs(arg5ref):_*)

[SakulK]

@jxnu-liguobin unfortunately the current version still doesn't help with my actual use case - calling a method that returns a Seq[net.logstash.logback.argument.StructuredArgument] which I want to pass to the logger with *. I feel like LoggerImpl methods that take inline args: Any* ( for example https://github.com/lightbend-labs/scala-logging/blob/main/src/main/scala-3/com/typesafe/scalalogging/LoggerImpl.scala#L32 ) could use a simpler macro without calling formatArgs, like

  def infoMessageArgsSimple(underlying: Expr[Underlying], message: Expr[String], args: Expr[Seq[Any]]) (using Quotes) = {
    '{ if ($underlying.isInfoEnabled) $underlying.info($message, ${args}*) }
  }

But maybe I'm missing some downside of this approach

[jxnu-liguobin]

@jxnu-liguobin unfortunately the current version still doesn't help with my actual use case - calling a method that returns a Seq[net.logstash.logback.argument.StructuredArgument] which I want to pass to the logger with *. I feel like LoggerImpl methods that take inline args: Any* ( for example https://github.com/lightbend-labs/scala-logging/blob/main/src/main/scala-3/com/typesafe/scalalogging/LoggerImpl.scala#L32 ) could use a simpler macro without calling formatArgs, like

  def infoMessageArgsSimple(underlying: Expr[Underlying], message: Expr[String], args: Expr[Seq[Any]]) (using Quotes) = {

    '{ if ($underlying.isInfoEnabled) $underlying.info($message, ${args}*) }

  }

But maybe I'm missing some downside of this approach

I think it's infeasible as SLF4j has three overloads, and we must know the size in order to choose whether to use Expr.ofSeq. But I'm not sure, I need to wait for others to see.

Also, there is no support for log.info("a {}", 11) if no format. So I created a new issue https://github.com/lightbend-labs/scala-logging/issues/436 which is a different issue from the other two.

[jxnu-liguobin]

@SakulK I have preliminarily solved the problem with val, please take a look. More matches may need to be added to handle the dotty ast.

[SakulK]

Looks like it works only for statically created Seq in the val, changing it to def or making the val get the value from another def breaks it again. I think it's not great that the macro defaults to omitting the args Expr when it couldn't find a match, simple refactoring can break the code (even though it still compiles).

[jxnu-liguobin]

Looks like it works only for statically created Seq in the val, changing it to def or making the val get the value from another def breaks it again. I think it's not great that the macro defaults to omitting the args Expr when it couldn't find a match, simple refactoring can break the code (even though it still compiles).

Indeed, I agree that once we if we need to match the entire syntax tree doesn't make much sense.

we can add a dedicated method without formatArgs and wait for others to vote.