Bug 1452200 - 1b. Add template literal support to Log.jsm; r=markh
☠☠ backed out by e96685584bf7 ☠ ☠
authorJim Chen <nchen@mozilla.com>
Sun, 15 Apr 2018 00:15:26 -0400
changeset 466926 8ac249bdc7726ab42cb4e381aae09a573e454e81
parent 466925 225bb7ed5f712d457e7207360cf618cf44a66ed4
child 466927 e4cdad2cd3d2e21fd0994ba523f743a7b7f43896
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1452200
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
toolkit/modules/Log.jsm
toolkit/modules/tests/xpcshell/test_Log.js
--- a/toolkit/modules/Log.jsm
+++ b/toolkit/modules/Log.jsm
@@ -413,54 +413,85 @@ Logger.prototype = {
     } else {
       level = this.level;
     }
 
     params.action = action;
     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;
 
     // Hold off on creating the message object until we actually have
     // an appender that's responsible.
     let message;
     let appenders = this.appenders;
     for (let appender of appenders) {
       if (appender.level > level) {
         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);
   }
 };
 
 /*
  * LoggerRepository
  * Implements a hierarchy of Loggers
  */
@@ -543,17 +574,26 @@ LoggerRepository.prototype = {
    *        (string) The Logger to retrieve.
    * @param prefix
    *        (string) The string to prefix each logged message with.
    */
   getLoggerWithMessagePrefix(name, prefix) {
     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;
   },
 };
 
 /*
  * Formatters
  * These massage a LogMessage into whatever output is desired.
  * BasicFormatter and StructuredFormatter are implemented here.
@@ -592,17 +632,17 @@ BasicFormatter.prototype = {
     // We could add a special case for NSRESULT values here...
     let pIsObject = (typeof(params) == "object" || typeof(params) == "function");
 
     // if we have params, try and find substitutions.
     if (this.parameterFormatter) {
       // 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) => {
           // ${foo} means use the params['foo']
           if (sub) {
             if (pIsObject && sub in message.params) {
               subDone = true;
               return this.parameterFormatter.format(message.params[sub]);
--- a/toolkit/modules/tests/xpcshell/test_Log.js
+++ b/toolkit/modules/tests/xpcshell/test_Log.js
@@ -69,21 +69,23 @@ add_test(function test_LoggerWithMessage
   let appender = new MockAppender(new Log.MessageOnlyFormatter());
   log.addAppender(appender);
 
   let prefixed = Log.repository.getLoggerWithMessagePrefix(
     "test.logger.prefix", "prefix: ");
 
   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();
 });
 
 /*
  * A utility method for checking object equivalence.
  * Fields with a reqular expression value in expected will be tested
@@ -410,16 +412,20 @@ add_task(async function log_message_with
   Assert.equal(formatMessage("Null ${n} undefined ${u}", {n: null, u: undefined}),
                "Null null undefined undefined");
 
   // Format params with number, bool, and String type.
   Assert.equal(formatMessage("number ${n} boolean ${b} boxed Boolean ${bx} String ${s}",
                              {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
    * larger structure.
    */
   let err = Components.Exception("test exception", Cr.NS_ERROR_FAILURE);
   let str = formatMessage("Exception is ${}", err);
   Assert.ok(str.includes('Exception is [Exception... "test exception"'));
@@ -550,16 +556,37 @@ add_task(async function log_message_with
   Assert.equal(appender.messages.length, 7);
   for (let msg of appender.messages) {
     Assert.ok(msg.params === testParams);
     Assert.ok(msg.message.startsWith("Test "));
   }
 });
 
 /*
+ * 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.
  */
 add_task(function* format_errors() {
   let pFormat = new Log.ParameterFormatter();
 
   // Test that subclasses of Error are recognized as errors.
   let err = new ReferenceError("Ref Error", "ERROR_FILE", 28);