From 3a167912a5f72aefd3bdf9de7bddb6f7c3751932 Mon Sep 17 00:00:00 2001 From: James Cracknell Date: Wed, 28 Jun 2017 18:48:04 -0600 Subject: [PATCH 1/8] Convert interpolated AnyVals to strings --- .../com/typesafe/scalalogging/LoggerMacro.scala | 5 ++++- .../com/typesafe/scalalogging/LoggerSpec.scala | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala index 0334124..355c559 100644 --- a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala +++ b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala @@ -243,7 +243,10 @@ private object LoggerMacro { 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](_))) + (c.Expr(q"$messageFormat"), args map { arg => + // Box interpolated AnyVals by explicitly getting the string representation + c.Expr[Any](if (arg.tpe <:< weakTypeOf[AnyVal]) q"$arg.toString" else arg) + }) case _ => (message, Seq.empty) } diff --git a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala index c8d443b..9179e64 100644 --- a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala +++ b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala @@ -57,6 +57,13 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { logger.error(s"msg $arg1 $arg2") verify(underlying).error("msg {} {}", List(arg1, arg2): _*) } + + "call the underlying logger's error method with the string representation of interpolated AnyVal arguments" in { + val f = fixture(_.isErrorEnabled, true) + import f._ + logger.error(s"msg ${1}") + verify(underlying).error("msg {}", 1.toString) + } } "Calling error with a message and cause" should { @@ -261,6 +268,13 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { logger.debug(s"msg $arg1 $arg2 $arg3") verify(underlying).debug("msg {} {} {}", arg1, arg2, arg3) } + + "call the underlying logger's debug method with the string representation of interpolated AnyVal arguments" in { + val f = fixture(_.isDebugEnabled, true) + import f._ + logger.debug(s"msg ${1}") + verify(underlying).debug("msg {}", 1.toString) + } } "Calling debug with a message and cause" should { From 28b0942c16743fbc0f1af941d8a89fd7e30066c2 Mon Sep 17 00:00:00 2001 From: James Cracknell Date: Wed, 28 Jun 2017 19:01:30 -0600 Subject: [PATCH 2/8] Escape slf4j format anchors occurring in interpolated messages --- src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala | 6 ++---- src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala | 7 +++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala index 355c559..72212fb 100644 --- a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala +++ b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala @@ -21,8 +21,6 @@ import scala.reflect.macros.blackbox.Context private object LoggerMacro { - private final val ArgumentMarker = "{}" - type LoggerContext = Context { type PrefixType = Logger } // Error @@ -241,8 +239,8 @@ 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) + // Escape slf4j format anchors + val messageFormat = parts.map({ case Literal(Constant(s: String)) => s.replace("{}", "\\{}") }).mkString("{}") (c.Expr(q"$messageFormat"), args map { arg => // Box interpolated AnyVals by explicitly getting the string representation c.Expr[Any](if (arg.tpe <:< weakTypeOf[AnyVal]) q"$arg.toString" else arg) diff --git a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala index 9179e64..b1c279a 100644 --- a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala +++ b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala @@ -64,6 +64,13 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { logger.error(s"msg ${1}") verify(underlying).error("msg {}", 1.toString) } + + "call the underlying logger's error method preserving literal format anchors when the message is interpolated" in { + val f = fixture(_.isErrorEnabled, true) + import f._ + logger.error(s"foo {} bar $arg1") + verify(underlying).error("foo \\{} bar {}", arg1) + } } "Calling error with a message and cause" should { From f917dbd83e48f91b20d1e3cadb1cc00a08ee14de Mon Sep 17 00:00:00 2001 From: James Cracknell Date: Wed, 28 Jun 2017 22:31:36 -0600 Subject: [PATCH 3/8] Generate arrays instead of lists for varargs Generate arrays instead of lists for varargs as they would have to be converted anyways. --- .../typesafe/scalalogging/LoggerMacro.scala | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala index 72212fb..575b855 100644 --- a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala +++ b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala @@ -40,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)" } @@ -60,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)" } @@ -82,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)" } @@ -102,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)" } @@ -124,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)" } @@ -144,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)" } @@ -166,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)" } @@ -186,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)" } @@ -208,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)" } @@ -228,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)" } From 0679a2bd4dbd25d936117c1f437a2afef06375e7 Mon Sep 17 00:00:00 2001 From: James Cracknell Date: Wed, 28 Jun 2017 22:39:08 -0600 Subject: [PATCH 4/8] Dedicated test scopes for interpolated messages --- .../typesafe/scalalogging/LoggerSpec.scala | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala index b1c279a..8dcb114 100644 --- a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala +++ b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala @@ -43,22 +43,25 @@ 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): _*) } - "call the underlying logger's error method with the string representation of interpolated AnyVal arguments" in { + "call the underlying logger's error method with the string representation AnyVal arguments" in { val f = fixture(_.isErrorEnabled, true) import f._ logger.error(s"msg ${1}") @@ -132,8 +135,10 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { logger.warn(msg) verify(underlying, never).warn(anyString) } + } + "Calling warn with an interpolated message" should { - "call the underlying logger's warn method if the warn level is enabled and string is interpolated" in { + "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") @@ -200,8 +205,10 @@ 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") @@ -268,15 +275,17 @@ 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 the string representation of interpolated AnyVal arguments" in { + "call the underlying logger's debug method with the string representation of AnyVal arguments" in { val f = fixture(_.isDebugEnabled, true) import f._ logger.debug(s"msg ${1}") @@ -343,8 +352,10 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { logger.trace(msg) verify(underlying, never).trace(anyString) } + } + "Calling trace with an interpolated message" should { - "call the underlying logger's trace method if the trace level is enabled and string is interpolated" in { + "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") From 11623e4c1e7df8bed01460d4273c6fe3716c50cc Mon Sep 17 00:00:00 2001 From: James Cracknell Date: Wed, 28 Jun 2017 23:24:21 -0600 Subject: [PATCH 5/8] Emulate standard interpolator escape treatment --- .../com/typesafe/scalalogging/LoggerMacro.scala | 4 ++-- .../com/typesafe/scalalogging/LoggerSpec.scala | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala index 575b855..f1155e3 100644 --- a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala +++ b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala @@ -239,8 +239,8 @@ private object LoggerMacro { message.tree match { case q"scala.StringContext.apply(..$parts).s(..$args)" => - // Escape slf4j format anchors - val messageFormat = parts.map({ case Literal(Constant(s: String)) => s.replace("{}", "\\{}") }).mkString("{}") + // Emulate standard interpolator escaping and escape literal slf4j format anchors + val messageFormat = parts.map({ case Literal(Constant(s: String)) => StringContext.treatEscapes(s).replace("{}", "\\{}") }).mkString("{}") (c.Expr(q"$messageFormat"), args map { arg => // Box interpolated AnyVals by explicitly getting the string representation c.Expr[Any](if (arg.tpe <:< weakTypeOf[AnyVal]) q"$arg.toString" else arg) diff --git a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala index 8dcb114..8e10a56 100644 --- a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala +++ b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala @@ -74,6 +74,19 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { logger.error(s"foo {} bar $arg1") verify(underlying).error("foo \\{} bar {}", arg1) } + + "call the underlying logger's error 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 logger's error 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) + } } "Calling error with a message and cause" should { From 6426184be0575eedeaef374d207f9cddb058be74 Mon Sep 17 00:00:00 2001 From: James Cracknell Date: Thu, 29 Jun 2017 00:22:20 -0600 Subject: [PATCH 6/8] Don't escape format anchors with no interpolations --- .../scala/com/typesafe/scalalogging/LoggerMacro.scala | 9 +++++++-- .../scala/com/typesafe/scalalogging/LoggerSpec.scala | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala index f1155e3..9b7f4a3 100644 --- a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala +++ b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala @@ -239,8 +239,13 @@ private object LoggerMacro { message.tree match { case q"scala.StringContext.apply(..$parts).s(..$args)" => - // Emulate standard interpolator escaping and escape literal slf4j format anchors - val messageFormat = parts.map({ case Literal(Constant(s: String)) => StringContext.treatEscapes(s).replace("{}", "\\{}") }).mkString("{}") + val messageFormat = 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("{}") + (c.Expr(q"$messageFormat"), args map { arg => // Box interpolated AnyVals by explicitly getting the string representation c.Expr[Any](if (arg.tpe <:< weakTypeOf[AnyVal]) q"$arg.toString" else arg) diff --git a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala index 8e10a56..827360f 100644 --- a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala +++ b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala @@ -68,13 +68,18 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { verify(underlying).error("msg {}", 1.toString) } - "call the underlying logger's error method preserving literal format anchors when the message is interpolated" in { + "call the underlying logger's error 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 logger's error 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 logger's error method when the interpolated string contains escape sequences" in { val f = fixture(_.isErrorEnabled, true) import f._ From cf35016fac6f760a136bf43bba1dbcc22861c447 Mon Sep 17 00:00:00 2001 From: James Cracknell Date: Thu, 29 Jun 2017 08:46:35 -0600 Subject: [PATCH 7/8] Clean up test cases --- .../typesafe/scalalogging/LoggerSpec.scala | 101 ++++++++++++------ 1 file changed, 67 insertions(+), 34 deletions(-) diff --git a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala index 827360f..53c2e3d 100644 --- a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala +++ b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala @@ -61,37 +61,6 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { verify(underlying).error("msg {} {}", List(arg1, arg2): _*) } - "call the underlying logger's error method with the string representation AnyVal arguments" in { - val f = fixture(_.isErrorEnabled, true) - import f._ - logger.error(s"msg ${1}") - verify(underlying).error("msg {}", 1.toString) - } - - "call the underlying logger's error 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 logger's error 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 logger's error 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 logger's error 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) - } } "Calling error with a message and cause" should { @@ -154,6 +123,7 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { verify(underlying, never).warn(anyString) } } + "Calling warn with an interpolated message" should { "call the underlying logger's warn method if the warn level is enabled" in { @@ -162,6 +132,13 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { 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 { @@ -224,6 +201,7 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { 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" in { @@ -232,6 +210,13 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { 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 { @@ -303,11 +288,11 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { verify(underlying).debug("msg {} {} {}", arg1, arg2, arg3) } - "call the underlying logger's debug method with the string representation of AnyVal arguments" in { + "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 ${1}") - verify(underlying).debug("msg {}", 1.toString) + logger.debug(s"msg $arg1 $arg2") + verify(underlying).debug("msg {} {}", List(arg1, arg2): _*) } } @@ -371,6 +356,7 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { verify(underlying, never).trace(anyString) } } + "Calling trace with an interpolated message" should { "call the underlying logger's trace method if the trace level is enabled" in { @@ -379,6 +365,13 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { 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 { @@ -423,6 +416,46 @@ 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 the string representation of AnyVal arguments" in { + val f = fixture(_.isErrorEnabled, true) + import f._ + logger.error(s"msg ${1}") + verify(underlying).error("msg {}", 1.toString) + } + + "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] = { From 0f59f972dcf1f67e27dbaed1aa15f18407f6c561 Mon Sep 17 00:00:00 2001 From: James Cracknell Date: Thu, 29 Jun 2017 10:11:35 -0600 Subject: [PATCH 8/8] Box interpolated values as necessary Box interpolated values for consistency in testing and to avoid runtime checks for expressions of type Any. --- .../scala/com/typesafe/scalalogging/LoggerMacro.scala | 11 ++++++----- .../scala/com/typesafe/scalalogging/LoggerSpec.scala | 11 +++++++++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala index 9b7f4a3..7d4ff70 100644 --- a/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala +++ b/src/main/scala/com/typesafe/scalalogging/LoggerMacro.scala @@ -239,17 +239,18 @@ private object LoggerMacro { message.tree match { case q"scala.StringContext.apply(..$parts).s(..$args)" => - val messageFormat = parts.iterator.map({ case Literal(Constant(str: String)) => str }) + 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("{}") - (c.Expr(q"$messageFormat"), args map { arg => - // Box interpolated AnyVals by explicitly getting the string representation - c.Expr[Any](if (arg.tpe <:< weakTypeOf[AnyVal]) q"$arg.toString" else arg) - }) + 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) } diff --git a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala index 53c2e3d..68ce466 100644 --- a/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala +++ b/src/test/scala/com/typesafe/scalalogging/LoggerSpec.scala @@ -420,11 +420,18 @@ class LoggerSpec extends WordSpec with Matchers with MockitoSugar { "Logging a message using the standard string interpolator" should { - "call the underlying format method with the string representation of AnyVal arguments" in { + "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.toString) + 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 {