fix(localize): ensure extracted messages are serialized in a consiste…

…nt order (#40192)

The CLI integration can provide code files in a non-deterministic
order, which led to the extracted translation files having
messages in a non-consistent order between extractions.

This commit fixes this by ensuring that serialized messages
are ordered by their location.

Fixes #39262

PR Close #40192

packages/localize/src/tools/src/extract/translation_files/arb_translation_serializer.ts

@@ -48,12 +48,13 @@ export class ArbTranslationSerializer implements TranslationSerializer {
       private sourceLocale: string, private basePath: AbsoluteFsPath, private fs: FileSystem) {}
 
   serialize(messages: ɵParsedMessage[]): string {
-    const messageMap = consolidateMessages(messages, message => message.customId || message.id);
+    const messageGroups = consolidateMessages(messages, message => getMessageId(message));
 
     let output = `{\n  "@@locale": ${JSON.stringify(this.sourceLocale)}`;
 
-    for (const [id, duplicateMessages] of messageMap.entries()) {
+    for (const duplicateMessages of messageGroups) {
       const message = duplicateMessages[0];
+      const id = getMessageId(message);
       output += this.serializeMessage(id, message);
       output += this.serializeMeta(
           id, message.description, duplicateMessages.filter(hasLocation).map(m => m.location));
@@ -98,3 +99,7 @@ export class ArbTranslationSerializer implements TranslationSerializer {
     ].join('\n');
   }
 }
+
+function getMessageId(message: ɵParsedMessage): string {
+  return message.customId || message.id;
+}

packages/localize/src/tools/src/extract/translation_files/json_translation_serializer.ts

@@ -7,6 +7,7 @@
  */
 import {ɵMessageId, ɵParsedMessage, ɵSourceMessage} from '@angular/localize';
 import {TranslationSerializer} from './translation_serializer';
+import {consolidateMessages} from './utils';
 
 
 interface SimpleJsonTranslationFile {
@@ -24,7 +25,7 @@ export class SimpleJsonTranslationSerializer implements TranslationSerializer {
   constructor(private sourceLocale: string) {}
   serialize(messages: ɵParsedMessage[]): string {
     const fileObj: SimpleJsonTranslationFile = {locale: this.sourceLocale, translations: {}};
-    for (const message of messages) {
+    for (const [message] of consolidateMessages(messages, message => message.id)) {
       fileObj.translations[message.id] = message.text;
     }
     return JSON.stringify(fileObj, null, 2);

packages/localize/src/tools/src/extract/translation_files/utils.ts

@@ -8,24 +8,40 @@
 import {ɵMessageId, ɵParsedMessage, ɵSourceLocation} from '@angular/localize';
 
 /**
- * Consolidate an array of messages into a map from message id to an array of messages with that id.
+ * Consolidate messages into groups that have the same id.
+ *
+ * Messages with the same id are grouped together so that we can quickly deduplicate messages when
+ * rendering into translation files.
+ *
+ * To ensure that messages are rendered in a deterministic order:
+ *  - the messages within a group are sorted by location (file path, then start position)
+ *  - the groups are sorted by the location of the first message in the group
  *
  * @param messages the messages to consolidate.
  * @param getMessageId a function that will compute the message id of a message.
+ * @returns an array of message groups, where each group is an array of messages that have the same
+ *     id.
  */
 export function consolidateMessages(
     messages: ɵParsedMessage[],
-    getMessageId: (message: ɵParsedMessage) => string): Map<ɵMessageId, ɵParsedMessage[]> {
-  const consolidateMessages = new Map<ɵMessageId, ɵParsedMessage[]>();
+    getMessageId: (message: ɵParsedMessage) => string): ɵParsedMessage[][] {
+  const messageGroups = new Map<ɵMessageId, ɵParsedMessage[]>();
   for (const message of messages) {
     const id = getMessageId(message);
-    if (!consolidateMessages.has(id)) {
-      consolidateMessages.set(id, [message]);
+    if (!messageGroups.has(id)) {
+      messageGroups.set(id, [message]);
     } else {
-      consolidateMessages.get(id)!.push(message);
+      messageGroups.get(id)!.push(message);
     }
   }
-  return consolidateMessages;
+
+  // Here we sort the messages within a group into location order.
+  // Note that `Array.sort()` will mutate the array in-place.
+  for (const messages of messageGroups.values()) {
+    messages.sort(compareLocations);
+  }
+  // Now we sort the groups by location of the first message in the group.
+  return Array.from(messageGroups.values()).sort((a1, a2) => compareLocations(a1[0], a2[0]));
 }
 
 /**
@@ -35,3 +51,26 @@ export function hasLocation(message: ɵParsedMessage): message is ɵParsedMessag
     {location: ɵSourceLocation} {
   return message.location !== undefined;
 }
+
+export function compareLocations(
+    {location: location1}: ɵParsedMessage, {location: location2}: ɵParsedMessage): number {
+  if (location1 === location2) {
+    return 0;
+  }
+  if (location1 === undefined) {
+    return -1;
+  }
+  if (location2 === undefined) {
+    return 1;
+  }
+  if (location1.file !== location2.file) {
+    return location1.file < location2.file ? -1 : 1;
+  }
+  if (location1.start.line !== location2.start.line) {
+    return location1.start.line < location2.start.line ? -1 : 1;
+  }
+  if (location1.start.column !== location2.start.column) {
+    return location1.start.column < location2.start.column ? -1 : 1;
+  }
+  return 0;
+}

packages/localize/src/tools/src/extract/translation_files/xliff1_translation_serializer.ts

@@ -34,7 +34,7 @@ export class Xliff1TranslationSerializer implements TranslationSerializer {
   }
 
   serialize(messages: ɵParsedMessage[]): string {
-    const messageMap = consolidateMessages(messages, message => this.getMessageId(message));
+    const messageGroups = consolidateMessages(messages, message => this.getMessageId(message));
     const xml = new XmlFile();
     xml.startTag('xliff', {'version': '1.2', 'xmlns': 'urn:oasis:names:tc:xliff:document:1.2'});
     // NOTE: the `original` property is set to the legacy `ng2.template` value for backward
@@ -51,8 +51,9 @@ export class Xliff1TranslationSerializer implements TranslationSerializer {
       ...this.formatOptions,
     });
     xml.startTag('body');
-    for (const [id, duplicateMessages] of messageMap.entries()) {
+    for (const duplicateMessages of messageGroups) {
       const message = duplicateMessages[0];
+      const id = this.getMessageId(message);
 
       xml.startTag('trans-unit', {id, datatype: 'html'});
       xml.startTag('source', {}, {preserveWhitespace: true});

packages/localize/src/tools/src/extract/translation_files/xliff2_translation_serializer.ts

@@ -34,7 +34,7 @@ export class Xliff2TranslationSerializer implements TranslationSerializer {
   }
 
   serialize(messages: ɵParsedMessage[]): string {
-    const messageMap = consolidateMessages(messages, message => this.getMessageId(message));
+    const messageGroups = consolidateMessages(messages, message => this.getMessageId(message));
     const xml = new XmlFile();
     xml.startTag('xliff', {
       'version': '2.0',
@@ -49,8 +49,9 @@ export class Xliff2TranslationSerializer implements TranslationSerializer {
     // messages that come from a particular original file, and the translation file parsers may
     // not
     xml.startTag('file', {'id': 'ngi18n', 'original': 'ng.template', ...this.formatOptions});
-    for (const [id, duplicateMessages] of messageMap.entries()) {
+    for (const duplicateMessages of messageGroups) {
       const message = duplicateMessages[0];
+      const id = this.getMessageId(message);
 
       xml.startTag('unit', {id});
       const messagesWithLocations = duplicateMessages.filter(hasLocation);

packages/localize/src/tools/src/extract/translation_files/xmb_translation_serializer.ts

@@ -10,6 +10,7 @@ import {ɵParsedMessage, ɵSourceLocation} from '@angular/localize';
 
 import {extractIcuPlaceholders} from './icu_parsing';
 import {TranslationSerializer} from './translation_serializer';
+import {consolidateMessages} from './utils';
 import {XmlFile} from './xml_file';
 
 /**
@@ -26,7 +27,7 @@ export class XmbTranslationSerializer implements TranslationSerializer {
       private fs: FileSystem = getFileSystem()) {}
 
   serialize(messages: ɵParsedMessage[]): string {
-    const ids = new Set<string>();
+    const messageGroups = consolidateMessages(messages, message => this.getMessageId(message));
     const xml = new XmlFile();
     xml.rawText(
         `<!DOCTYPE messagebundle [\n` +
@@ -51,13 +52,9 @@ export class XmbTranslationSerializer implements TranslationSerializer {
         `<!ELEMENT ex (#PCDATA)>\n` +
         `]>\n`);
     xml.startTag('messagebundle');
-    for (const message of messages) {
+    for (const duplicateMessages of messageGroups) {
+      const message = duplicateMessages[0];
       const id = this.getMessageId(message);
-      if (ids.has(id)) {
-        // Do not render the same message more than once
-        continue;
-      }
-      ids.add(id);
       xml.startTag(
           'msg', {id, desc: message.description, meaning: message.meaning},
           {preserveWhitespace: true});

packages/localize/src/tools/test/extract/integration/main_spec.ts

@@ -464,7 +464,7 @@ runInEachFileSystem(() => {
           `  "locale": "en-GB",`,
           `  "translations": {`,
           `    "message-1": "message {$PH} contents",`,
-          `    "message-2": "different message contents"`,
+          `    "message-2": "message contents"`,
           `  }`,
           `}`,
         ].join('\n'));
@@ -489,7 +489,7 @@ runInEachFileSystem(() => {
           `  "locale": "en-GB",`,
           `  "translations": {`,
           `    "message-1": "message {$PH} contents",`,
-          `    "message-2": "different message contents"`,
+          `    "message-2": "message contents"`,
           `  }`,
           `}`,
         ].join('\n'));

packages/localize/src/tools/test/extract/translation_files/arb_translation_serializer_spec.ts

@@ -7,11 +7,11 @@
  */
 import {absoluteFrom, FileSystem, getFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {runInEachFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
-import {ɵParsedMessage, ɵSourceLocation} from '@angular/localize';
+import {ɵParsedMessage} from '@angular/localize';
 
 import {ArbTranslationSerializer} from '../../../src/extract/translation_files/arb_translation_serializer';
 
-import {mockMessage} from './mock_message';
+import {location, mockMessage} from './mock_message';
 
 runInEachFileSystem(() => {
   let fs: FileSystem;
@@ -25,30 +25,18 @@ runInEachFileSystem(() => {
         const messages: ɵParsedMessage[] = [
           mockMessage('12345', ['a', 'b', 'c'], ['PH', 'PH_1'], {
             meaning: 'some meaning',
-            location: {
-              file: absoluteFrom('/project/file.ts'),
-              start: {line: 5, column: 10},
-              end: {line: 5, column: 12}
-            },
+            location: location('/project/file.ts', 5, 10, 5, 12),
           }),
           mockMessage('54321', ['a', 'b', 'c'], ['PH', 'PH_1'], {
             customId: 'someId',
           }),
           mockMessage('67890', ['a', '', 'c'], ['START_TAG_SPAN', 'CLOSE_TAG_SPAN'], {
             description: 'some description',
-            location: {
-              file: absoluteFrom('/project/file.ts'),
-              start: {line: 5, column: 10},
-              end: {line: 5, column: 12}
-            },
+            location: location('/project/file.ts', 5, 10, 5, 12)
           }),
           mockMessage('67890', ['a', '', 'c'], ['START_TAG_SPAN', 'CLOSE_TAG_SPAN'], {
             description: 'some description',
-            location: {
-              file: absoluteFrom('/project/other.ts'),
-              start: {line: 2, column: 10},
-              end: {line: 3, column: 12}
-            },
+            location: location('/project/other.ts', 2, 10, 3, 12)
           }),
           mockMessage('13579', ['', 'b', ''], ['START_BOLD_TEXT', 'CLOSE_BOLD_TEXT'], {}),
           mockMessage('24680', ['a'], [], {meaning: 'meaning', description: 'and description'}),
@@ -72,6 +60,16 @@ runInEachFileSystem(() => {
         expect(output.split('\n')).toEqual([
           '{',
           '  "@@locale": "xx",',
+          '  "someId": "a{$PH}b{$PH_1}c",',
+          '  "13579": "{$START_BOLD_TEXT}b{$CLOSE_BOLD_TEXT}",',
+          '  "24680": "a",',
+          '  "@24680": {',
+          '    "description": "and description"',
+          '  },',
+          '  "80808": "multi\\nlines",',
+          '  "90000": "<escape{$double-quotes-\\"}me>",',
+          '  "100000": "pre-ICU {VAR_SELECT, select, a {a} b {{INTERPOLATION}} c {pre {INTERPOLATION_1} post}} post-ICU",',
+          '  "100001": "{VAR_PLURAL, plural, one {{START_BOLD_TEXT}something bold{CLOSE_BOLD_TEXT}} other {pre {START_TAG_SPAN}middle{CLOSE_TAG_SPAN} post}}",',
           '  "12345": "a{$PH}b{$PH_1}c",',
           '  "@12345": {',
           '    "x-locations": [',
@@ -82,7 +80,6 @@ runInEachFileSystem(() => {
           '      }',
           '    ]',
           '  },',
-          '  "someId": "a{$PH}b{$PH_1}c",',
           '  "67890": "a{$START_TAG_SPAN}{$CLOSE_TAG_SPAN}c",',
           '  "@67890": {',
           '    "description": "some description",',
@@ -98,16 +95,89 @@ runInEachFileSystem(() => {
           '        "end": { "line": "3", "column": "12" }',
           '      }',
           '    ]',
+          '  }',
+          '}',
+        ]);
+      });
+
+      it('should consistently order serialized messages by location', () => {
+        const messages: ɵParsedMessage[] = [
+          mockMessage('1', ['message-1'], [], {location: location('/root/c-1.ts', 5, 10, 5, 12)}),
+          mockMessage('2', ['message-1'], [], {location: location('/root/c-2.ts', 5, 10, 5, 12)}),
+          mockMessage('1', ['message-1'], [], {location: location('/root/b-1.ts', 8, 0, 10, 12)}),
+          mockMessage('2', ['message-1'], [], {location: location('/root/b-2.ts', 8, 0, 10, 12)}),
+          mockMessage('1', ['message-1'], [], {location: location('/root/a-1.ts', 5, 10, 5, 12)}),
+          mockMessage('2', ['message-1'], [], {location: location('/root/a-2.ts', 5, 10, 5, 12)}),
+          mockMessage('1', ['message-1'], [], {location: location('/root/b-1.ts', 5, 10, 5, 12)}),
+          mockMessage('2', ['message-1'], [], {location: location('/root/b-2.ts', 5, 10, 5, 12)}),
+          mockMessage('1', ['message-1'], [], {location: location('/root/b-1.ts', 5, 20, 5, 12)}),
+          mockMessage('2', ['message-1'], [], {location: location('/root/b-2.ts', 5, 20, 5, 12)}),
+        ];
+        const serializer = new ArbTranslationSerializer('xx', fs.resolve('/root'), fs);
+        const output = serializer.serialize(messages);
+        expect(output.split('\n')).toEqual([
+          '{',
+          '  "@@locale": "xx",',
+          '  "1": "message-1",',
+          '  "@1": {',
+          '    "x-locations": [',
+          '      {',
+          '        "file": "a-1.ts",',
+          '        "start": { "line": "5", "column": "10" },',
+          '        "end": { "line": "5", "column": "12" }',
+          '      },',
+          '      {',
+          '        "file": "b-1.ts",',
+          '        "start": { "line": "5", "column": "10" },',
+          '        "end": { "line": "5", "column": "12" }',
+          '      },',
+          '      {',
+          '        "file": "b-1.ts",',
+          '        "start": { "line": "5", "column": "20" },',
+          '        "end": { "line": "5", "column": "12" }',
+          '      },',
+          '      {',
+          '        "file": "b-1.ts",',
+          '        "start": { "line": "8", "column": "0" },',
+          '        "end": { "line": "10", "column": "12" }',
+          '      },',
+          '      {',
+          '        "file": "c-1.ts",',
+          '        "start": { "line": "5", "column": "10" },',
+          '        "end": { "line": "5", "column": "12" }',
+          '      }',
+          '    ]',
           '  },',
-          '  "13579": "{$START_BOLD_TEXT}b{$CLOSE_BOLD_TEXT}",',
-          '  "24680": "a",',
-          '  "@24680": {',
-          '    "description": "and description"',
-          '  },',
-          '  "80808": "multi\\nlines",',
-          '  "90000": "<escape{$double-quotes-\\"}me>",',
-          '  "100000": "pre-ICU {VAR_SELECT, select, a {a} b {{INTERPOLATION}} c {pre {INTERPOLATION_1} post}} post-ICU",',
-          '  "100001": "{VAR_PLURAL, plural, one {{START_BOLD_TEXT}something bold{CLOSE_BOLD_TEXT}} other {pre {START_TAG_SPAN}middle{CLOSE_TAG_SPAN} post}}"',
+          '  "2": "message-1",',
+          '  "@2": {',
+          '    "x-locations": [',
+          '      {',
+          '        "file": "a-2.ts",',
+          '        "start": { "line": "5", "column": "10" },',
+          '        "end": { "line": "5", "column": "12" }',
+          '      },',
+          '      {',
+          '        "file": "b-2.ts",',
+          '        "start": { "line": "5", "column": "10" },',
+          '        "end": { "line": "5", "column": "12" }',
+          '      },',
+          '      {',
+          '        "file": "b-2.ts",',
+          '        "start": { "line": "5", "column": "20" },',
+          '        "end": { "line": "5", "column": "12" }',
+          '      },',
+          '      {',
+          '        "file": "b-2.ts",',
+          '        "start": { "line": "8", "column": "0" },',
+          '        "end": { "line": "10", "column": "12" }',
+          '      },',
+          '      {',
+          '        "file": "c-2.ts",',
+          '        "start": { "line": "5", "column": "10" },',
+          '        "end": { "line": "5", "column": "12" }',
+          '      }',
+          '    ]',
+          '  }',
           '}',
         ]);
       });