From 7958b308d9df29a6c114d30074306c792450498c Mon Sep 17 00:00:00 2001
From: Jim Chen <nchen@mozilla.com>
Date: Sun, 15 Apr 2018 00:15:26 -0400
Subject: [PATCH] Bug 1452200 - 1b. Add template literal support to Log.jsm;
 r=markh

Make Log.jsm functions support tagged template literals. For example,
instead of |logger.debug("foo " + bar)| or |logger.debug(`foo ${bar}`)|,
you can now use |logger.debug `foo ${bar}`| (without parentheses).

Using tagged template literals has the benefit of less verbosity
compared to regular string concatenation, with the added benefit of
lazily-stringified parameters -- the parameters are only stringified
when logging is enabled, possibly saving from an expensive stringify
operation.

This patch also fixes a bug in BasicFormatter where consecutive tokens
are not formatted correctly (e.g. "${a}${b}").

MozReview-Commit-ID: 9kjLvpZF5ch

--HG--
extra : rebase_source : 41a4760f0f106ae9b7fb69342df3e28bc1cf1c20
---
 toolkit/modules/Log.jsm                    | 58 ++++++++++++++++++----
 toolkit/modules/tests/xpcshell/test_Log.js | 29 ++++++++++-
 2 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/toolkit/modules/Log.jsm b/toolkit/modules/Log.jsm
index 552841915b35..f07e71d66c9a 100644
--- a/toolkit/modules/Log.jsm
+++ b/toolkit/modules/Log.jsm
@@ -418,6 +418,36 @@ Logger.prototype = {
     this.log(level, params._message, params);
   },
 
+  _unpackTemplateLiteral(string, params) {
+    if (!Array.isArray(params)) {
+      // Regular log() call.
+      return [string, params];
+    }
+
+    if (!Array.isArray(string)) {
+      // Not using template literal. However params was packed into an array by
+      // the this.[level] call, so we need to unpack it here.
+      return [string, params[0]];
+    }
+
+    // We're using template literal format (logger.warn `foo ${bar}`). Turn the
+    // template strings into one string containing "${0}"..."${n}" tokens, and
+    // feed it to the basic formatter. The formatter will treat the numbers as
+    // indices into the params array, and convert the tokens to the params.
+
+    if (!params.length) {
+      // No params; we need to set params to undefined, so the formatter
+      // doesn't try to output the params array.
+      return [string[0], undefined];
+    }
+
+    let concat = string[0];
+    for (let i = 0; i < params.length; i++) {
+      concat += `\${${i}}${string[i + 1]}`;
+    }
+    return [concat, params];
+  },
+
   log(level, string, params) {
     if (this.level > level)
       return;
@@ -431,31 +461,32 @@ Logger.prototype = {
         continue;
       }
       if (!message) {
+        [string, params] = this._unpackTemplateLiteral(string, params);
         message = new LogMessage(this._name, level, string, params);
       }
       appender.append(message);
     }
   },
 
-  fatal(string, params) {
+  fatal(string, ...params) {
     this.log(Log.Level.Fatal, string, params);
   },
-  error(string, params) {
+  error(string, ...params) {
     this.log(Log.Level.Error, string, params);
   },
-  warn(string, params) {
+  warn(string, ...params) {
     this.log(Log.Level.Warn, string, params);
   },
-  info(string, params) {
+  info(string, ...params) {
     this.log(Log.Level.Info, string, params);
   },
-  config(string, params) {
+  config(string, ...params) {
     this.log(Log.Level.Config, string, params);
   },
-  debug(string, params) {
+  debug(string, ...params) {
     this.log(Log.Level.Debug, string, params);
   },
-  trace(string, params) {
+  trace(string, ...params) {
     this.log(Log.Level.Trace, string, params);
   }
 };
@@ -548,7 +579,16 @@ LoggerRepository.prototype = {
     let log = this.getLogger(name);
 
     let proxy = Object.create(log);
-    proxy.log = (level, string, params) => log.log(level, prefix + string, params);
+    proxy.log = (level, string, params) => {
+      if (Array.isArray(string) && Array.isArray(params)) {
+        // Template literal.
+        // We cannot change the original array, so create a new one.
+        string = [prefix + string[0]].concat(string.slice(1));
+      } else {
+        string = prefix + string; // Regular string.
+      }
+      return log.log(level, string, params);
+    };
     return proxy;
   },
 };
@@ -597,7 +637,7 @@ BasicFormatter.prototype = {
       // have we successfully substituted any parameters into the message?
       // in the log message
       let subDone = false;
-      let regex = /\$\{(\S*)\}/g;
+      let regex = /\$\{(\S*?)\}/g;
       let textParts = [];
       if (message.message) {
         textParts.push(message.message.replace(regex, (_, sub) => {
diff --git a/toolkit/modules/tests/xpcshell/test_Log.js b/toolkit/modules/tests/xpcshell/test_Log.js
index f49337a20f6d..60c84958020c 100644
--- a/toolkit/modules/tests/xpcshell/test_Log.js
+++ b/toolkit/modules/tests/xpcshell/test_Log.js
@@ -74,11 +74,13 @@ add_test(function test_LoggerWithMessagePrefix() {
 
   log.warn("no prefix");
   prefixed.warn("with prefix");
+  prefixed.warn `with prefix`;
 
-  Assert.equal(appender.messages.length, 2, "2 messages were logged.");
+  Assert.equal(appender.messages.length, 3, "3 messages were logged.");
   Assert.deepEqual(appender.messages, [
     "no prefix",
     "prefix: with prefix",
+    "prefix: with prefix",
   ], "Prefix logger works.");
 
   run_next_test();
@@ -415,6 +417,10 @@ add_task(async function log_message_with_params() {
                              {n: 45, b: false, bx: Boolean(true), s: String("whatevs")}),
                "number 45 boolean false boxed Boolean true String whatevs");
 
+  // Format params with consecutive tokens.
+  Assert.equal(formatMessage("${a}${b}${c}", {a: "foo", b: "bar", c: "baz"}),
+               "foobarbaz");
+
   /*
    * Check that errors get special formatting if they're formatted directly as
    * a named param or they're the only param, but not if they're a field in a
@@ -554,6 +560,27 @@ add_task(async function log_message_with_params() {
   }
 });
 
+/*
+ * Test that all the basic logger methods support tagged template literal format.
+ */
+add_task(async function log_template_literal_message() {
+  let log = Log.repository.getLogger("error.logger");
+  let appender = new MockAppender(new Log.BasicFormatter());
+  log.addAppender(appender);
+
+  log.fatal `Test ${"foo"} ${42}`;
+  log.error `Test ${"foo"} 42`;
+  log.warn `Test foo 42`;
+  log.info `Test ${"foo " + 42}`;
+  log.config `${"Test"} foo ${42}`;
+  log.debug `Test ${"f"}${"o"}${"o"} 42`;
+  log.trace `${"Test foo 42"}`;
+  Assert.equal(appender.messages.length, 7);
+  for (let msg of appender.messages) {
+    Assert.equal(msg.split("\t")[3], "Test foo 42");
+  }
+});
+
 /*
  * Check that we format JS Errors reasonably.
  * This needs to stay a generator to exercise Task.jsm's stack rewriting.