fix(localize): serialize all the message locations to XLIFF (#39411)

Previously only the first message, for each id, was serialized
which meant that additional message location information
was lost.

Now all the message locations are included in the serialized
messages.

Fixes #39330

PR Close #39411
This commit is contained in:
Pete Bacon Darwin 2020-10-24 16:50:25 +01:00 committed by Alex Rickabaugh
parent 35fdc6d92f
commit 57ead7aa53
5 changed files with 237 additions and 21 deletions

View File

@ -0,0 +1,37 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
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.
*
* @param messages the messages to consolidate.
* @param getMessageId a function that will compute the message id of a message.
*/
export function consolidateMessages(
messages: ɵParsedMessage[],
getMessageId: (message: ɵParsedMessage) => string): Map<ɵMessageId, ɵParsedMessage[]> {
const consolidateMessages = new Map<ɵMessageId, ɵParsedMessage[]>();
for (const message of messages) {
const id = getMessageId(message);
if (!consolidateMessages.has(id)) {
consolidateMessages.set(id, [message]);
} else {
consolidateMessages.get(id)!.push(message);
}
}
return consolidateMessages;
}
/**
* Does the given message have a location property?
*/
export function hasLocation(message: ɵParsedMessage): message is ɵParsedMessage&
{location: ɵSourceLocation} {
return message.location !== undefined;
}

View File

@ -11,6 +11,7 @@ import {ɵParsedMessage, ɵSourceLocation} from '@angular/localize';
import {FormatOptions, validateOptions} from './format_options';
import {extractIcuPlaceholders} from './icu_parsing';
import {TranslationSerializer} from './translation_serializer';
import {consolidateMessages, hasLocation} from './utils';
import {XmlFile} from './xml_file';
/** This is the number of characters that a legacy Xliff 1.2 message id has. */
@ -33,7 +34,7 @@ export class Xliff1TranslationSerializer implements TranslationSerializer {
}
serialize(messages: ɵParsedMessage[]): string {
const ids = new Set<string>();
const messageMap = 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
@ -50,21 +51,19 @@ export class Xliff1TranslationSerializer implements TranslationSerializer {
...this.formatOptions,
});
xml.startTag('body');
for (const message of messages) {
const id = this.getMessageId(message);
if (ids.has(id)) {
// Do not render the same message more than once
continue;
}
ids.add(id);
for (const [id, duplicateMessages] of messageMap.entries()) {
const message = duplicateMessages[0];
xml.startTag('trans-unit', {id, datatype: 'html'});
xml.startTag('source', {}, {preserveWhitespace: true});
this.serializeMessage(xml, message);
xml.endTag('source', {preserveWhitespace: false});
if (message.location) {
this.serializeLocation(xml, message.location);
// Write all the locations
for (const {location} of duplicateMessages.filter(hasLocation)) {
this.serializeLocation(xml, location);
}
if (message.description) {
this.serializeNote(xml, 'description', message.description);
}

View File

@ -11,6 +11,7 @@ import {ɵParsedMessage, ɵSourceLocation} from '@angular/localize';
import {FormatOptions, validateOptions} from './format_options';
import {extractIcuPlaceholders} from './icu_parsing';
import {TranslationSerializer} from './translation_serializer';
import {consolidateMessages, hasLocation} from './utils';
import {XmlFile} from './xml_file';
/** This is the maximum number of characters that can appear in a legacy XLIFF 2.0 message id. */
@ -33,7 +34,7 @@ export class Xliff2TranslationSerializer implements TranslationSerializer {
}
serialize(messages: ɵParsedMessage[]): string {
const ids = new Set<string>();
const messageMap = consolidateMessages(messages, message => this.getMessageId(message));
const xml = new XmlFile();
xml.startTag('xliff', {
'version': '2.0',
@ -48,24 +49,23 @@ 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 message of messages) {
const id = this.getMessageId(message);
if (ids.has(id)) {
// Do not render the same message more than once
continue;
}
ids.add(id);
for (const [id, duplicateMessages] of messageMap.entries()) {
const message = duplicateMessages[0];
xml.startTag('unit', {id});
if (message.meaning || message.description || message.location) {
const messagesWithLocations = duplicateMessages.filter(hasLocation);
if (message.meaning || message.description || messagesWithLocations.length) {
xml.startTag('notes');
if (message.location) {
const {file, start, end} = message.location;
// Write all the locations
for (const {location: {file, start, end}} of messagesWithLocations) {
const endLineString =
end !== undefined && end.line !== start.line ? `,${end.line + 1}` : '';
this.serializeNote(
xml, 'location',
`${this.fs.relative(this.basePath, file)}:${start.line + 1}${endLineString}`);
}
if (message.description) {
this.serializeNote(xml, 'description', message.description);
}

View File

@ -135,6 +135,101 @@ runInEachFileSystem(() => {
`</xliff>\n`,
].join('\n'));
});
it('should convert a set of parsed messages into an XML string', () => {
const messageLocation1: ɵSourceLocation = {
start: {line: 0, column: 5},
end: {line: 0, column: 10},
file: absoluteFrom('/project/file-1.ts'),
text: 'message text'
};
const messageLocation2: ɵSourceLocation = {
start: {line: 3, column: 2},
end: {line: 4, column: 7},
file: absoluteFrom('/project/file-2.ts'),
text: 'message text'
};
const messageLocation3: ɵSourceLocation = {
start: {line: 0, column: 5},
end: {line: 0, column: 10},
file: absoluteFrom('/project/file-3.ts'),
text: 'message text'
};
const messageLocation4: ɵSourceLocation = {
start: {line: 3, column: 2},
end: {line: 4, column: 7},
file: absoluteFrom('/project/file-4.ts'),
text: 'message text'
};
const messages: ɵParsedMessage[] = [
mockMessage('1234', ['message text'], [], {location: messageLocation1}),
mockMessage('1234', ['message text'], [], {location: messageLocation2}),
mockMessage('1234', ['message text'], [], {
location: messageLocation3,
legacyIds: ['87654321FEDCBA0987654321FEDCBA0987654321', '563965274788097516']
}),
mockMessage(
'1234', ['message text'], [], {location: messageLocation4, customId: 'other'}),
];
const serializer = new Xliff1TranslationSerializer(
'xx', absoluteFrom('/project'), useLegacyIds, options);
const output = serializer.serialize(messages);
// Note that in this test the third message will match the first two if legacyIds is
// false. Otherwise it will be a separate message on its own.
expect(output).toEqual([
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`,
` <file source-language="xx" datatype="plaintext" original="ng2.template"${
toAttributes(options)}>`,
` <body>`,
` <trans-unit id="1234" datatype="html">`,
` <source>message text</source>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">file-1.ts</context>`,
` <context context-type="linenumber">1</context>`,
` </context-group>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">file-2.ts</context>`,
` <context context-type="linenumber">4,5</context>`,
` </context-group>`,
...useLegacyIds ?
[] :
[
` <context-group purpose="location">`,
` <context context-type="sourcefile">file-3.ts</context>`,
` <context context-type="linenumber">1</context>`,
` </context-group>`,
],
` </trans-unit>`,
...useLegacyIds ?
[
` <trans-unit id="87654321FEDCBA0987654321FEDCBA0987654321" datatype="html">`,
` <source>message text</source>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">file-3.ts</context>`,
` <context context-type="linenumber">1</context>`,
` </context-group>`,
` </trans-unit>`,
] :
[],
` <trans-unit id="other" datatype="html">`,
` <source>message text</source>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">file-4.ts</context>`,
` <context context-type="linenumber">4,5</context>`,
` </context-group>`,
` </trans-unit>`,
` </body>`,
` </file>`,
`</xliff>\n`,
].join('\n'));
});
});
});
});

View File

@ -158,6 +158,91 @@ runInEachFileSystem(() => {
`</xliff>\n`,
].join('\n'));
});
it('should convert a set of parsed messages into an XML string', () => {
const messageLocation1: ɵSourceLocation = {
start: {line: 0, column: 5},
end: {line: 0, column: 10},
file: absoluteFrom('/project/file-1.ts'),
text: 'message text'
};
const messageLocation2: ɵSourceLocation = {
start: {line: 3, column: 2},
end: {line: 4, column: 7},
file: absoluteFrom('/project/file-2.ts'),
text: 'message text'
};
const messageLocation3: ɵSourceLocation = {
start: {line: 0, column: 5},
end: {line: 0, column: 10},
file: absoluteFrom('/project/file-3.ts'),
text: 'message text'
};
const messageLocation4: ɵSourceLocation = {
start: {line: 3, column: 2},
end: {line: 4, column: 7},
file: absoluteFrom('/project/file-4.ts'),
text: 'message text'
};
const messages: ɵParsedMessage[] = [
mockMessage('1234', ['message text'], [], {location: messageLocation1}),
mockMessage('1234', ['message text'], [], {location: messageLocation2}),
mockMessage('1234', ['message text'], [], {
location: messageLocation3,
legacyIds: ['87654321FEDCBA0987654321FEDCBA0987654321', '563965274788097516']
}),
mockMessage(
'1234', ['message text'], [], {location: messageLocation4, customId: 'other'}),
];
const serializer = new Xliff2TranslationSerializer(
'xx', absoluteFrom('/project'), useLegacyIds, options);
const output = serializer.serialize(messages);
// Note that in this test the third message will match the first two if legacyIds is
// false. Otherwise it will be a separate message on its own.
expect(output).toEqual([
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="2.0" xmlns="urn:oasis:names:tc:xliff:document:2.0" srcLang="xx">`,
` <file id="ngi18n" original="ng.template"${toAttributes(options)}>`,
` <unit id="1234">`,
` <notes>`,
` <note category="location">file-1.ts:1</note>`,
` <note category="location">file-2.ts:4,5</note>`,
...useLegacyIds ? [] : [' <note category="location">file-3.ts:1</note>'],
` </notes>`,
` <segment>`,
` <source>message text</source>`,
` </segment>`,
` </unit>`,
...useLegacyIds ?
[
` <unit id="563965274788097516">`,
` <notes>`,
` <note category="location">file-3.ts:1</note>`,
` </notes>`,
` <segment>`,
` <source>message text</source>`,
` </segment>`,
` </unit>`,
] :
[],
` <unit id="other">`,
` <notes>`,
` <note category="location">file-4.ts:4,5</note>`,
` </notes>`,
` <segment>`,
` <source>message text</source>`,
` </segment>`,
` </unit>`,
` </file>`,
`</xliff>\n`,
].join('\n'));
});
});
});
});