Enforce specifiers in messages (#1144)

* fix: require specifiers * chore: correct behavior in test
forcedotcom · Sep 26, 2024 · c59a6d3 · c59a6d3
1 parent 0bee3f5
commit c59a6d3
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 18 deletions.
diff --git a/src/messages.ts b/src/messages.ts
@@ -12,7 +12,7 @@ import * as util from 'node:util';
 import { fileURLToPath } from 'node:url';
 import { AnyJson, asString, ensureJsonMap, ensureString, isJsonMap, isObject } from '@salesforce/ts-types';
 import { ensureArray, upperFirst } from '@salesforce/kit';
-import { Lifecycle } from './lifecycleEvents';
+import { Logger } from './logger/logger';
 import { SfError } from './sfError';
 
 export type Tokens = Array<string | boolean | number | null | undefined>;
@@ -436,6 +436,7 @@ export class Messages<T extends string> {
    * @param tokens The values to substitute in the message.
    *
    * **See** https://nodejs.org/api/util.html#util_util_format_format_args
+   * **Note** Unlike util.format(), specifiers are required for a token to be rendered.
    */
   public getMessage(key: T, tokens: Tokens = []): string {
     return this.getMessageWithMap(key, tokens, this.messages).join(os.EOL);
@@ -462,6 +463,7 @@ export class Messages<T extends string> {
    * @param tokens The values to substitute in the message.
    *
    * **See** https://nodejs.org/api/util.html#util_util_format_format_args
+   * **Note** Unlike util.format(), specifiers are required for a token to be rendered.
    */
   public getMessages(key: T, tokens: Tokens = []): string[] {
     return this.getMessageWithMap(key, tokens, this.messages);
@@ -597,23 +599,16 @@ export class Messages<T extends string> {
       // https://nodejs.org/api/util.html#utilformatformat-args
       // https://regex101.com/r/8Hf8Z6/1
       const specifierRegex = new RegExp('%[sdifjoO]{1}', 'gm');
+      const specifierFound = specifierRegex.test(msgStr);
 
-      // NOTE: This is a temporary telemetry event to track down missing specifiers in messages.
-      //       Once we have enough data and correct missing specifiers, we can remove this.
-      //       The followup work is outlined in: W-16197665
-      if (!specifierRegex.test(msgStr) && tokens.length > 0) {
-        void Lifecycle.getInstance().emitTelemetry({
-          eventName: 'missing_message_specifier',
-          library: 'sfdx-core',
-          function: 'getMessageWithMap',
-          messagesLength: messages.length,
-          message: msgStr,
-          tokensLength: tokens.length,
-        });
+      if (!specifierFound && tokens.length > 0) {
+        const logger = Logger.childFromRoot('core:messages');
+        logger.warn(
+          `Unable to render tokens in message. Ensure a specifier (e.g. %s) exists in the message:\n${msgStr}`
+        );
       }
 
-      // return specifierRegex.test(msgStr) ? util.format(msgStr, ...tokens) : msgStr;
-      return util.format(msgStr, ...tokens);
+      return specifierFound ? util.format(msgStr, ...tokens) : msgStr;
     });
   }
 }

diff --git a/test/unit/messages.test.ts b/test/unit/messages.test.ts
@@ -67,14 +67,14 @@ describe('Messages', () => {
 
     it('should return single string from array of messages', () => {
       expect(messages.getMessage('manyMsgs', ['blah', 864])).to.equal(
-        `hello blah 864${EOL}world blah 864${EOL}test message 2 blah and 864`
+        `hello${EOL}world${EOL}test message 2 blah and 864`
       );
     });
 
     it('should return multiple string from array of messages', () => {
       expect(messages.getMessages('manyMsgs', ['blah', 864])).to.deep.equal([
-        'hello blah 864',
-        'world blah 864',
+        'hello',
+        'world',
         'test message 2 blah and 864',
       ]);
     });