Skip to content

Fix compile errors for interpolated AnyVals #87

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Jul 4, 2017
Merged

Fix compile errors for interpolated AnyVals #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3a16791
Convert interpolated AnyVals to strings
jcracknell Jun 29, 2017
28b0942
Escape slf4j format anchors occurring in interpolated messages
jcracknell Jun 29, 2017
f917dbd
Generate arrays instead of lists for varargs
jcracknell Jun 29, 2017
0679a2b
Dedicated test scopes for interpolated messages
jcracknell Jun 29, 2017
11623e4
Emulate standard interpolator escape treatment
jcracknell Jun 29, 2017
6426184
Don't escape format anchors with no interpolations
jcracknell Jun 29, 2017
cf35016
Clean up test cases
jcracknell Jun 29, 2017
0f59f97
Box interpolated values as necessary
jcracknell Jun 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import scala.reflect.macros.blackbox.Context

private object LoggerMacro {

private final val ArgumentMarker = "{}"

type LoggerContext = Context { type PrefixType = Logger }

// Error
Expand All @@ -42,7 +40,7 @@ private object LoggerMacro {
import c.universe._
val underlying = q"${c.prefix}.underlying"
if (args.length == 2)
q"if ($underlying.isErrorEnabled) $underlying.error($message, _root_.scala.List(${args(0)}, ${args(1)}): _*)"
q"if ($underlying.isErrorEnabled) $underlying.error($message, _root_.scala.Array(${args(0)}, ${args(1)}): _*)"
else
q"if ($underlying.isErrorEnabled) $underlying.error($message, ..$args)"
}
Expand All @@ -62,7 +60,7 @@ private object LoggerMacro {
import c.universe._
val underlying = q"${c.prefix}.underlying"
if (args.length == 2)
q"if ($underlying.isErrorEnabled) $underlying.error($marker, $message, _root_.scala.List(${args(0)}, ${args(1)}): _*)"
q"if ($underlying.isErrorEnabled) $underlying.error($marker, $message, _root_.scala.Array(${args(0)}, ${args(1)}): _*)"
else
q"if ($underlying.isErrorEnabled) $underlying.error($marker, $message, ..$args)"
}
Expand All @@ -84,7 +82,7 @@ private object LoggerMacro {
import c.universe._
val underlying = q"${c.prefix}.underlying"
if (args.length == 2)
q"if ($underlying.isWarnEnabled) $underlying.warn($message, _root_.scala.List(${args(0)}, ${args(1)}): _*)"
q"if ($underlying.isWarnEnabled) $underlying.warn($message, _root_.scala.Array(${args(0)}, ${args(1)}): _*)"
else
q"if ($underlying.isWarnEnabled) $underlying.warn($message, ..$args)"
}
Expand All @@ -104,7 +102,7 @@ private object LoggerMacro {
import c.universe._
val underlying = q"${c.prefix}.underlying"
if (args.length == 2)
q"if ($underlying.isWarnEnabled) $underlying.warn($marker, $message, _root_.scala.List(${args(0)}, ${args(1)}): _*)"
q"if ($underlying.isWarnEnabled) $underlying.warn($marker, $message, _root_.scala.Array(${args(0)}, ${args(1)}): _*)"
else
q"if ($underlying.isWarnEnabled) $underlying.warn($marker, $message, ..$args)"
}
Expand All @@ -126,7 +124,7 @@ private object LoggerMacro {
import c.universe._
val underlying = q"${c.prefix}.underlying"
if (args.length == 2)
q"if ($underlying.isInfoEnabled) $underlying.info($message, _root_.scala.List(${args(0)}, ${args(1)}): _*)"
q"if ($underlying.isInfoEnabled) $underlying.info($message, _root_.scala.Array(${args(0)}, ${args(1)}): _*)"
else
q"if ($underlying.isInfoEnabled) $underlying.info($message, ..$args)"
}
Expand All @@ -146,7 +144,7 @@ private object LoggerMacro {
import c.universe._
val underlying = q"${c.prefix}.underlying"
if (args.length == 2)
q"if ($underlying.isInfoEnabled) $underlying.info($marker, $message, _root_.scala.List(${args(0)}, ${args(1)}): _*)"
q"if ($underlying.isInfoEnabled) $underlying.info($marker, $message, _root_.scala.Array(${args(0)}, ${args(1)}): _*)"
else
q"if ($underlying.isInfoEnabled) $underlying.info($marker, $message, ..$args)"
}
Expand All @@ -168,7 +166,7 @@ private object LoggerMacro {
import c.universe._
val underlying = q"${c.prefix}.underlying"
if (args.length == 2)
q"if ($underlying.isDebugEnabled) $underlying.debug($message, _root_.scala.List(${args(0)}, ${args(1)}): _*)"
q"if ($underlying.isDebugEnabled) $underlying.debug($message, _root_.scala.Array(${args(0)}, ${args(1)}): _*)"
else
q"if ($underlying.isDebugEnabled) $underlying.debug($message, ..$args)"
}
Expand All @@ -188,7 +186,7 @@ private object LoggerMacro {
import c.universe._
val underlying = q"${c.prefix}.underlying"
if (args.length == 2)
q"if ($underlying.isDebugEnabled) $underlying.debug($marker, $message, _root_.scala.List(${args(0)}, ${args(1)}): _*)"
q"if ($underlying.isDebugEnabled) $underlying.debug($marker, $message, _root_.scala.Array(${args(0)}, ${args(1)}): _*)"
else
q"if ($underlying.isDebugEnabled) $underlying.debug($marker, $message, ..$args)"
}
Expand All @@ -210,7 +208,7 @@ private object LoggerMacro {
import c.universe._
val underlying = q"${c.prefix}.underlying"
if (args.length == 2)
q"if ($underlying.isTraceEnabled) $underlying.trace($message, _root_.scala.List(${args(0)}, ${args(1)}): _*)"
q"if ($underlying.isTraceEnabled) $underlying.trace($message, _root_.scala.Array(${args(0)}, ${args(1)}): _*)"
else
q"if ($underlying.isTraceEnabled) $underlying.trace($message, ..$args)"
}
Expand All @@ -230,7 +228,7 @@ private object LoggerMacro {
import c.universe._
val underlying = q"${c.prefix}.underlying"
if (args.length == 2)
q"if ($underlying.isTraceEnabled) $underlying.trace($marker, $message, _root_.scala.List(${args(0)}, ${args(1)}): _*)"
q"if ($underlying.isTraceEnabled) $underlying.trace($marker, $message, _root_.scala.Array(${args(0)}, ${args(1)}): _*)"
else
q"if ($underlying.isTraceEnabled) $underlying.trace($marker, $message, ..$args)"
}
Expand All @@ -241,9 +239,18 @@ private object LoggerMacro {

message.tree match {
case q"scala.StringContext.apply(..$parts).s(..$args)" =>
val strings = parts.collect { case Literal(Constant(s: String)) => s }
val messageFormat = strings.mkString(ArgumentMarker)
(c.Expr(q"$messageFormat"), args.map(arg => q"$arg").map(c.Expr[Any](_)))
val format = parts.iterator.map({ case Literal(Constant(str: String)) => str })
// Emulate standard interpolator escaping
.map(StringContext.treatEscapes)
// Escape literal slf4j format anchors if the resulting call will require a format string
.map(str => if (args.nonEmpty) str.replace("{}", "\\{}") else str)
.mkString("{}")

val formatArgs = args map { arg =>
c.Expr[AnyRef](if (arg.tpe <:< weakTypeOf[AnyRef]) arg else q"$arg.asInstanceOf[_root_.scala.AnyRef]")
}

(c.Expr(q"$format"), formatArgs)

case _ => (message, Seq.empty)
}
Expand Down
102 changes: 96 additions & 6 deletions src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,24 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar {
logger.error(msg)
verify(underlying, never).error(anyString)
}
}

"Calling error with an interpolated message" should {

"call the underlying logger's error method with arguments if the error level is enabled and string is interpolated" in {
"call the underlying logger's error method with arguments if the error level is enabled" in {
val f = fixture(_.isErrorEnabled, true)
import f._
logger.error(s"msg $arg1 $arg2 $arg3")
verify(underlying).error("msg {} {} {}", arg1, arg2, arg3)
}

"call the underlying logger's error method with two arguments if the error level is enabled and string is interpolated" in {
"call the underlying logger's error method with two arguments if the error level is enabled" in {
val f = fixture(_.isErrorEnabled, true)
import f._
logger.error(s"msg $arg1 $arg2")
verify(underlying).error("msg {} {}", List(arg1, arg2): _*)
}

}

"Calling error with a message and cause" should {
Expand Down Expand Up @@ -118,13 +122,23 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar {
logger.warn(msg)
verify(underlying, never).warn(anyString)
}
}

"call the underlying logger's warn method if the warn level is enabled and string is interpolated" in {
"Calling warn with an interpolated message" should {

"call the underlying logger's warn method if the warn level is enabled" in {
val f = fixture(_.isWarnEnabled, true)
import f._
logger.warn(s"msg $arg1 $arg2 $arg3")
verify(underlying).warn("msg {} {} {}", arg1, arg2, arg3)
}

"call the underlying logger's warn method with two arguments if the warn level is enabled" in {
val f = fixture(_.isWarnEnabled, true)
import f._
logger.warn(s"msg $arg1 $arg2")
verify(underlying).warn("msg {} {}", List(arg1, arg2): _*)
}
}

"Calling warn with a message and cause" should {
Expand Down Expand Up @@ -186,13 +200,23 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar {
logger.info(msg)
verify(underlying, never).info(anyString)
}
}

"Calling info with an interpolated message" should {

"call the underlying logger's info method if the info level is enabled and string is interpolated" in {
"call the underlying logger's info method if the info level is enabled" in {
val f = fixture(_.isInfoEnabled, true)
import f._
logger.info(s"msg $arg1 $arg2 $arg3")
verify(underlying).info("msg {} {} {}", arg1, arg2, arg3)
}

"call the underlying logger's info method with two arguments if the info level is enabled" in {
val f = fixture(_.isInfoEnabled, true)
import f._
logger.info(s"msg $arg1 $arg2")
verify(underlying).info("msg {} {}", List(arg1, arg2): _*)
}
}

"Calling info with a message and cause" should {
Expand Down Expand Up @@ -254,13 +278,22 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar {
logger.debug(msg)
verify(underlying, never).debug(anyString)
}
}
"Calling debug with an interpolated message" should {

"call the underlying logger's debug method if the debug level is enabled and string is interpolated" in {
"call the underlying logger's debug method if the debug level is enabled" in {
val f = fixture(_.isDebugEnabled, true)
import f._
logger.debug(s"msg $arg1 $arg2 $arg3")
verify(underlying).debug("msg {} {} {}", arg1, arg2, arg3)
}

"call the underlying logger's debug method with two arguments if the debug level is enabled" in {
val f = fixture(_.isDebugEnabled, true)
import f._
logger.debug(s"msg $arg1 $arg2")
verify(underlying).debug("msg {} {}", List(arg1, arg2): _*)
}
}

"Calling debug with a message and cause" should {
Expand Down Expand Up @@ -322,13 +355,23 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar {
logger.trace(msg)
verify(underlying, never).trace(anyString)
}
}

"call the underlying logger's trace method if the trace level is enabled and string is interpolated" in {
"Calling trace with an interpolated message" should {

"call the underlying logger's trace method if the trace level is enabled" in {
val f = fixture(_.isTraceEnabled, true)
import f._
logger.trace(s"msg $arg1 $arg2 $arg3")
verify(underlying).trace("msg {} {} {}", arg1, arg2, arg3)
}

"call the underlying logger's trace method with two arguments if the trace level is enabled" in {
val f = fixture(_.isTraceEnabled, true)
import f._
logger.trace(s"msg $arg1 $arg2")
verify(underlying).trace("msg {} {}", List(arg1, arg2): _*)
}
}

"Calling trace with a message and cause" should {
Expand Down Expand Up @@ -373,6 +416,53 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar {
}
}

// Interpolator destructuring corner cases

"Logging a message using the standard string interpolator" should {

"call the underlying format method with boxed versions of value arguments" in {
val f = fixture(_.isErrorEnabled, true)
import f._
logger.error(s"msg ${1}")
verify(underlying).error("msg {}", 1.asInstanceOf[AnyRef])
}

"call the underlying format method with boxed versions of arguments of type Any" in {
val f = fixture(_.isErrorEnabled, true)
import f._
logger.error(s"msg ${1.asInstanceOf[Any]}")
verify(underlying).error("msg {}", 1.asInstanceOf[AnyRef])
}

"call the underlying format method escaping literal format anchors" in {
val f = fixture(_.isErrorEnabled, true)
import f._
logger.error(s"foo {} bar $arg1")
verify(underlying).error("foo \\{} bar {}", arg1)
}

"call the underlying method without escaping format anchors when the message has no interpolations" in {
val f = fixture(_.isErrorEnabled, true)
import f._
logger.error(s"foo {} bar")
verify(underlying).error("foo {} bar")
}

"call the underlying format method when the interpolated string contains escape sequences" in {
val f = fixture(_.isErrorEnabled, true)
import f._
logger.error(s"foo\nbar $arg1")
verify(underlying).error(s"foo\nbar {}", arg1)
}

"call the underlying format method when the interpolated string is triple quoted and contains escape sequences" in {
val f = fixture(_.isErrorEnabled, true)
import f._
logger.error(s"""foo\nbar $arg1""")
verify(underlying).error(s"""foo\nbar {}""", arg1)
}
}

"Serializing Logger" should {

def serialize(logger: Logger): Array[Byte] = {
Expand Down