honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(localize): extract the correct message ids (#38498) 

Previously, if `useLegacyIds` was enabled, the message extractor
was always rendering the legacy message ids in translation
files even if an explicit "custom message id" had been provided
in the original message.

PR Close #38498
This commit is contained in:
Pete Bacon Darwin 2020-08-17 12:29:18 +01:00 committed by Misko Hevery
parent f245c6bb15
commit ac461e1efd
13 changed files with 273 additions and 161 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
packages/localize/src/tools/src/extract/translation_files/xliff1_translation_serializer.ts
View File

@ -107,6 +107,8 @@ export class Xliff1TranslationSerializer implements TranslationSerializer {
/**
* Get the id for the given `message`.
*
* If there was a custom id provided, use that.
*
* If we have requested legacy message ids, then try to return the appropriate id
* from the list of legacy ids that were extracted.
*
@ -116,7 +118,8 @@ export class Xliff1TranslationSerializer implements TranslationSerializer {
* https://csrc.nist.gov/csrc/media/publications/fips/180/4/final/documents/fips180-4-draft-aug2014.pdf
*/
private getMessageId(message: ɵParsedMessage): string {
return this.useLegacyIds && message.legacyIds !== undefined &&
return message.customId ||
this.useLegacyIds && message.legacyIds !== undefined &&
message.legacyIds.find(id => id.length === LEGACY_XLIFF_MESSAGE_LENGTH) ||
message.id;
}

5
packages/localize/src/tools/src/extract/translation_files/xliff2_translation_serializer.ts
View File

@ -120,6 +120,8 @@ export class Xliff2TranslationSerializer implements TranslationSerializer {
/**
* Get the id for the given `message`.
*
* If there was a custom id provided, use that.
*
* If we have requested legacy message ids, then try to return the appropriate id
* from the list of legacy ids that were extracted.
*
@ -130,7 +132,8 @@ export class Xliff2TranslationSerializer implements TranslationSerializer {
* https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/GoogleJsMessageIdGenerator.java
*/
private getMessageId(message: ɵParsedMessage): string {
return this.useLegacyIds && message.legacyIds !== undefined &&
return message.customId ||
this.useLegacyIds && message.legacyIds !== undefined &&
message.legacyIds.find(
id => id.length <= MAX_LEGACY_XLIFF_2_MESSAGE_LENGTH && !/[^0-9]/.test(id)) ||
message.id;

5
packages/localize/src/tools/src/extract/translation_files/xmb_translation_serializer.ts
View File

@ -99,6 +99,8 @@ export class XmbTranslationSerializer implements TranslationSerializer {
/**
* Get the id for the given `message`.
*
* If there was a custom id provided, use that.
*
* If we have requested legacy message ids, then try to return the appropriate id
* from the list of legacy ids that were extracted.
*
@ -109,7 +111,8 @@ export class XmbTranslationSerializer implements TranslationSerializer {
* https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/GoogleJsMessageIdGenerator.java
*/
private getMessageId(message: ɵParsedMessage): string {
return this.useLegacyIds && message.legacyIds !== undefined &&
return message.customId ||
this.useLegacyIds && message.legacyIds !== undefined &&
message.legacyIds.find(id => id.length <= 20 && !/[^0-9]/.test(id)) ||
message.id;
}

20
packages/localize/src/tools/test/extract/extractor_spec.ts
View File

@ -23,14 +23,16 @@ runInEachFileSystem(() => {
fs.ensureDir(absoluteFrom('/root/path/relative'));
fs.writeFile(file, [
'$localize`:meaning|description:a${1}b${2}c`;',
'$localize(__makeTemplateObject(["a", ":custom-placeholder:b", "c"], ["a", ":custom-placeholder:b", "c"]), 1, 2);'
'$localize(__makeTemplateObject(["a", ":custom-placeholder:b", "c"], ["a", ":custom-placeholder:b", "c"]), 1, 2);',
'$localize`:@@custom-id:a${1}b${2}c`;',
].join('\n'));
const messages = extractor.extractMessages(filename);
expect(messages.length).toEqual(2);
expect(messages.length).toEqual(3);
expect(messages[0]).toEqual({
id: '2714330828844000684',
customId: undefined,
description: 'description',
meaning: 'meaning',
messageParts: ['a', 'b', 'c'],
@ -43,6 +45,7 @@ runInEachFileSystem(() => {
expect(messages[1]).toEqual({
id: '5692770902395945649',
customId: undefined,
description: '',
meaning: '',
messageParts: ['a', 'b', 'c'],
@ -52,6 +55,19 @@ runInEachFileSystem(() => {
legacyIds: [],
location: {start: {line: 1, column: 0}, end: {line: 1, column: 111}, file},
});
expect(messages[2]).toEqual({
id: 'custom-id',
customId: 'custom-id',
description: '',
meaning: '',
messageParts: ['a', 'b', 'c'],
text: 'a{$PH}b{$PH_1}c',
placeholderNames: ['PH', 'PH_1'],
substitutions: jasmine.any(Object),
legacyIds: [],
location: {start: {line: 2, column: 9}, end: {line: 2, column: 35}, file},
});
});
});
});

61
packages/localize/src/tools/test/extract/integration/main_spec.ts
View File

@ -55,6 +55,8 @@ runInEachFileSystem(() => {
].join('\n'));
});
for (const useLegacyIds of [true, false]) {
describe(useLegacyIds ? '[using legacy ids]' : '', () => {
it('should extract translations from source code, and write as JSON format', () => {
extractTranslations({
rootPath,
@ -64,7 +66,7 @@ runInEachFileSystem(() => {
outputPath,
logger,
useSourceMaps: false,
useLegacyIds: false,
useLegacyIds,
duplicateMessageHandling: 'ignore',
});
expect(fs.readFile(outputPath)).toEqual([
@ -72,7 +74,10 @@ runInEachFileSystem(() => {
` "locale": "en-GB",`,
` "translations": {`,
` "3291030485717846467": "Hello, {$PH}!",`,
` "8669027859022295761": "try{$PH}me"`,
` "8669027859022295761": "try{$PH}me",`,
` "custom-id": "Custom id message",`,
` "273296103957933077": "Legacy id message",`,
` "custom-id-2": "Custom and legacy message"`,
` }`,
`}`,
].join('\n'));
@ -87,7 +92,7 @@ runInEachFileSystem(() => {
outputPath,
logger,
useSourceMaps: false,
useLegacyIds: false,
useLegacyIds,
duplicateMessageHandling: 'ignore',
});
expect(fs.readFile(outputPath)).toEqual([
@ -116,6 +121,12 @@ runInEachFileSystem(() => {
`<messagebundle>`,
` <msg id="3291030485717846467"><source>test_files/test.js:1</source>Hello, <ph name="PH"/>!</msg>`,
` <msg id="8669027859022295761"><source>test_files/test.js:2</source>try<ph name="PH"/>me</msg>`,
` <msg id="custom-id"><source>test_files/test.js:3</source>Custom id message</msg>`,
` <msg id="${
useLegacyIds ?
'12345678901234567890' :
'273296103957933077'}"><source>test_files/test.js:5</source>Legacy id message</msg>`,
` <msg id="custom-id-2"><source>test_files/test.js:7</source>Custom and legacy message</msg>`,
`</messagebundle>\n`,
].join('\n'));
});
@ -129,7 +140,7 @@ runInEachFileSystem(() => {
outputPath,
logger,
useSourceMaps: false,
useLegacyIds: false,
useLegacyIds,
duplicateMessageHandling: 'ignore',
});
expect(fs.readFile(outputPath)).toEqual([
@ -151,6 +162,29 @@ runInEachFileSystem(() => {
` <context context-type="linenumber">3</context>`,
` </context-group>`,
` </trans-unit>`,
` <trans-unit id="custom-id" datatype="html">`,
` <source>Custom id message</source>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">test_files/test.js</context>`,
` <context context-type="linenumber">4</context>`,
` </context-group>`,
` </trans-unit>`,
` <trans-unit id="${
useLegacyIds ? '1234567890123456789012345678901234567890' :
'273296103957933077'}" datatype="html">`,
` <source>Legacy id message</source>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">test_files/test.js</context>`,
` <context context-type="linenumber">6</context>`,
` </context-group>`,
` </trans-unit>`,
` <trans-unit id="custom-id-2" datatype="html">`,
` <source>Custom and legacy message</source>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">test_files/test.js</context>`,
` <context context-type="linenumber">8</context>`,
` </context-group>`,
` </trans-unit>`,
` </body>`,
` </file>`,
`</xliff>\n`,
@ -166,7 +200,7 @@ runInEachFileSystem(() => {
outputPath,
logger,
useSourceMaps: false,
useLegacyIds: false,
useLegacyIds,
duplicateMessageHandling: 'ignore',
});
expect(fs.readFile(outputPath)).toEqual([
@ -183,10 +217,27 @@ runInEachFileSystem(() => {
` <source>try<ph id="0" equiv="PH"/>me</source>`,
` </segment>`,
` </unit>`,
` <unit id="custom-id">`,
` <segment>`,
` <source>Custom id message</source>`,
` </segment>`,
` </unit>`,
` <unit id="${useLegacyIds ? '12345678901234567890' : '273296103957933077'}">`,
` <segment>`,
` <source>Legacy id message</source>`,
` </segment>`,
` </unit>`,
` <unit id="custom-id-2">`,
` <segment>`,
` <source>Custom and legacy message</source>`,
` </segment>`,
` </unit>`,
` </file>`,
`</xliff>\n`,
].join('\n'));
});
});
}
for (const target of ['es2015', 'es5']) {
it(`should render the original location of translations, when processing an ${

5
packages/localize/src/tools/test/extract/integration/test_files/test.js
View File

@ -1,3 +1,8 @@
var name = 'World';
var message = $localize`Hello, ${name}!`;
var other = $localize(__makeTemplateObject(['try', 'me'], ['try', 'me']), 40 + 2);
var customMessage = $localize`:@@custom-id:Custom id message`;
var legacyMessage =
$localize`:␟1234567890123456789012345678901234567890␟12345678901234567890:Legacy id message`;
var customAndLegacyMessage =
$localize`:@@custom-id-2␟1234567890123456789012345678901234567890␟12345678901234567890:Custom and legacy message`;

6
packages/localize/src/tools/test/extract/translation_files/json_translation_serializer_spec.ts
View File

@ -16,6 +16,9 @@ describe('JsonTranslationSerializer', () => {
it('should convert a set of parsed messages into a JSON string', () => {
const messages: ɵParsedMessage[] = [
mockMessage('12345', ['a', 'b', 'c'], ['PH', 'PH_1'], {meaning: 'some meaning'}),
mockMessage('54321', ['a', 'b', 'c'], ['PH', 'PH_1'], {
customId: 'someId',
}),
mockMessage(
'67890', ['a', '', 'c'], ['START_TAG_SPAN', 'CLOSE_TAG_SPAN'],
{description: 'some description'}),
@ -49,7 +52,8 @@ describe('JsonTranslationSerializer', () => {
` "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}}"`,
` "100001": "{VAR_PLURAL, plural, one {{START_BOLD_TEXT}something bold{CLOSE_BOLD_TEXT}} other {pre {START_TAG_SPAN}middle{CLOSE_TAG_SPAN} post}}",`,
` "someId": "a{$PH}b{$PH_1}c"`,
` }`,
`}`,
].join('\n'));

7
packages/localize/src/tools/test/extract/translation_files/mock_message.ts
View File

@ -9,28 +9,31 @@ import {ɵParsedMessage} from '@angular/localize';
import {SourceLocation} from '@angular/localize/src/utils';
export interface MockMessageOptions {
customId?: string;
meaning?: string;
description?: string;
location?: SourceLocation;
legacyIds?: string[];
}
/**
* This helper is used to create `ParsedMessage` objects to be rendered in the
* `TranslationSerializer` tests.
*/
export function mockMessage(
id: string, messageParts: string[], placeholderNames: string[],
{meaning = '', description = '', location, legacyIds = []}: MockMessageOptions):
{customId, meaning = '', description = '', location, legacyIds = []}: MockMessageOptions):
ɵParsedMessage {
let text = messageParts[0];
for (let i = 1; i < messageParts.length; i++) {
text += `{$${placeholderNames[i - 1]}}${messageParts[i]}`;
}
return {
id,
id: customId || id, // customId trumps id
text,
messageParts,
placeholderNames,
customId,
description,
meaning,
substitutions: [],

7
packages/localize/src/tools/test/extract/translation_files/xliff1_translation_serializer_spec.ts
View File

@ -28,6 +28,10 @@ runInEachFileSystem(() => {
},
legacyIds: ['1234567890ABCDEF1234567890ABCDEF12345678', '615790887472569365'],
}),
mockMessage('54321', ['a', 'b', 'c'], ['PH', 'PH_1'], {
customId: 'someId',
legacyIds: ['87654321FEDCBA0987654321FEDCBA0987654321', '563965274788097516'],
}),
mockMessage(
'67890', ['a', '', 'c'], ['START_TAG_SPAN', 'CLOSE_TAG_SPAN'],
{description: 'some description'}),
@ -66,6 +70,9 @@ runInEachFileSystem(() => {
` </context-group>`,
` <note priority="1" from="meaning">some meaning</note>`,
` </trans-unit>`,
` <trans-unit id="someId" datatype="html">`,
` <source>a<x id="PH"/>b<x id="PH_1"/>c</source>`,
` </trans-unit>`,
` <trans-unit id="67890" datatype="html">`,
` <source>a<x id="START_TAG_SPAN"/><x id="CLOSE_TAG_SPAN"/>c</source>`,
` <note priority="1" from="description">some description</note>`,

9
packages/localize/src/tools/test/extract/translation_files/xliff2_translation_serializer_spec.ts
View File

@ -29,6 +29,10 @@ runInEachFileSystem(() => {
},
legacyIds: ['1234567890ABCDEF1234567890ABCDEF12345678', '615790887472569365'],
}),
mockMessage('54321', ['a', 'b', 'c'], ['PH', 'PH_1'], {
customId: 'someId',
legacyIds: ['87654321FEDCBA0987654321FEDCBA0987654321', '563965274788097516'],
}),
mockMessage('67890', ['a', '', 'c'], ['START_TAG_SPAN', 'CLOSE_TAG_SPAN'], {
description: 'some description',
location: {
@ -70,6 +74,11 @@ runInEachFileSystem(() => {
` <source>a<ph id="0" equiv="PH"/>b<ph id="1" equiv="PH_1"/>c</source>`,
` </segment>`,
` </unit>`,
` <unit id="someId">`,
` <segment>`,
` <source>a<ph id="0" equiv="PH"/>b<ph id="1" equiv="PH_1"/>c</source>`,
` </segment>`,
` </unit>`,
` <unit id="67890">`,
` <notes>`,
` <note category="location">file.ts:3,4</note>`,

5
packages/localize/src/tools/test/extract/translation_files/xmb_translation_serializer_spec.ts
View File

@ -23,6 +23,10 @@ runInEachFileSystem(() => {
meaning: 'some meaning',
legacyIds: ['1234567890ABCDEF1234567890ABCDEF12345678', '615790887472569365'],
}),
mockMessage('54321', ['a', 'b', 'c'], ['PH', 'PH_1'], {
customId: 'someId',
legacyIds: ['87654321FEDCBA0987654321FEDCBA0987654321', '563965274788097516'],
}),
mockMessage(
'67890', ['a', '', 'c'], ['START_TAG_SPAN', 'CLOSE_TAG_SPAN'],
{description: 'some description'}),
@ -51,6 +55,7 @@ runInEachFileSystem(() => {
useLegacyIds ?
'615790887472569365' :
'12345'}" meaning="some meaning">a<ph name="PH"/>b<ph name="PH_1"/>c</msg>`,
` <msg id="someId">a<ph name="PH"/>b<ph name="PH_1"/>c</msg>`,
` <msg id="67890" desc="some description">a<ph name="START_TAG_SPAN"/><ph name="CLOSE_TAG_SPAN"/>c</msg>`,
` <msg id="13579"><ph name="START_BOLD_TEXT"/>b<ph name="CLOSE_BOLD_TEXT"/></msg>`,
` <msg id="24680" desc="and description" meaning="meaning">a</msg>`,

22
packages/localize/src/utils/src/messages.ts
View File

@ -58,10 +58,6 @@ export interface MessageMetadata {
* A human readable rendering of the message
*/
text: string;
/**
* A unique identifier for this message.
*/
id?: MessageId;
/**
* Legacy message ids, if provided.
*
@ -73,6 +69,12 @@ export interface MessageMetadata {
* of translation if the translations are encoded using the legacy message id.
*/
legacyIds?: string[];
/**
* The id of the `message` if a custom one was specified explicitly.
*
* This id overrides any computed or legacy ids.
*/
customId?: string;
/**
* The meaning of the `message`, used to distinguish identical `messageString`s.
*/
@ -110,8 +112,6 @@ export interface MessageMetadata {
export interface ParsedMessage extends MessageMetadata {
/**
* The key used to look up the appropriate translation target.
*
* In `ParsedMessage` this is a required field, whereas it is optional in `MessageMetadata`.
*/
id: MessageId;
/**
@ -129,7 +129,8 @@ export interface ParsedMessage extends MessageMetadata {
}
/**
* Parse a `$localize` tagged string into a structure that can be used for translation.
* Parse a `$localize` tagged string into a structure that can be used for translation or
* extraction.
*
* See `ParsedMessage` for an example.
*/
@ -151,13 +152,14 @@ export function parseMessage(
placeholderNames.push(placeholderName);
cleanedMessageParts.push(messagePart);
}
const messageId = metadata.id || computeMsgId(messageString, metadata.meaning || '');
const messageId = metadata.customId || computeMsgId(messageString, metadata.meaning || '');
const legacyIds = metadata.legacyIds ? metadata.legacyIds.filter(id => id !== messageId) : [];
return {
id: messageId,
legacyIds,
substitutions,
text: messageString,
customId: metadata.customId,
meaning: metadata.meaning || '',
description: metadata.description || '',
messageParts: cleanedMessageParts,
@ -198,7 +200,7 @@ export function parseMetadata(cooked: string, raw: string): MessageMetadata {
return {text: messageString};
} else {
const [meaningDescAndId, ...legacyIds] = block.split(LEGACY_ID_INDICATOR);
const [meaningAndDesc, id] = meaningDescAndId.split(ID_SEPARATOR, 2);
const [meaningAndDesc, customId] = meaningDescAndId.split(ID_SEPARATOR, 2);
let [meaning, description]: (string|undefined)[] = meaningAndDesc.split(MEANING_SEPARATOR, 2);
if (description === undefined) {
description = meaning;
@ -207,7 +209,7 @@ export function parseMetadata(cooked: string, raw: string): MessageMetadata {
if (description === '') {
description = undefined;
}
return {text: messageString, meaning, description, id, legacyIds};
return {text: messageString, meaning, description, customId, legacyIds};
}
}

29
packages/localize/src/utils/test/messages_spec.ts
View File

@ -9,13 +9,14 @@ import {findEndOfBlock, makeTemplateObject, parseMessage, parseMetadata, splitBl
describe('messages utils', () => {
describe('parseMessage', () => {
it('should use the custom id parsed from the metadata if available', () => {
it('should use the custom id parsed from the metadata for the message id, if available', () => {
const message = parseMessage(
makeTemplateObject(
[':@@custom-message-id:a', ':one:b', ':two:c'],
[':@@custom-message-id:a', ':one:b', ':two:c']),
[1, 2]);
expect(message.id).toEqual('custom-message-id');
expect(message.customId).toEqual('custom-message-id');
expect(message.id).toEqual(message.customId!);
});
it('should compute the translation key if no metadata', () => {
@ -24,7 +25,7 @@ describe('messages utils', () => {
expect(message.id).toEqual('8865273085679272414');
});
it('should compute the translation key if no id in the metadata', () => {
it('should compute the translation key if no custom id in the metadata', () => {
const message = parseMessage(
makeTemplateObject(
[':description:a', ':one:b', ':two:c'], [':description:a', ':one:b', ':two:c']),
@ -181,21 +182,21 @@ describe('messages utils', () => {
text: 'abc def',
description: 'description',
meaning: undefined,
id: undefined,
customId: undefined,
legacyIds: []
});
expect(parseMetadata(':meaning|:abc def', ':meaning|:abc def')).toEqual({
text: 'abc def',
description: undefined,
meaning: 'meaning',
id: undefined,
customId: undefined,
legacyIds: []
});
expect(parseMetadata(':@@message-id:abc def', ':@@message-id:abc def')).toEqual({
text: 'abc def',
description: undefined,
meaning: undefined,
id: 'message-id',
customId: 'message-id',
legacyIds: []
});
expect(parseMetadata(':meaning|description:abc def', ':meaning|description:abc def'))
@ -203,7 +204,7 @@ describe('messages utils', () => {
text: 'abc def',
description: 'description',
meaning: 'meaning',
id: undefined,
customId: undefined,
legacyIds: []
});
expect(parseMetadata(':description@@message-id:abc def', ':description@@message-id:abc def'))
@ -211,7 +212,7 @@ describe('messages utils', () => {
text: 'abc def',
description: 'description',
meaning: undefined,
id: 'message-id',
customId: 'message-id',
legacyIds: []
});
expect(parseMetadata(':meaning|@@message-id:abc def', ':meaning|@@message-id:abc def'))
@ -219,7 +220,7 @@ describe('messages utils', () => {
text: 'abc def',
description: undefined,
meaning: 'meaning',
id: 'message-id',
customId: 'message-id',
legacyIds: []
});
expect(parseMetadata(
@ -229,7 +230,7 @@ describe('messages utils', () => {
text: 'abc def',
description: 'description',
meaning: undefined,
id: 'message-id',
customId: 'message-id',
legacyIds: ['legacy-1', 'legacy-2', 'legacy-3']
});
expect(parseMetadata(
@ -239,7 +240,7 @@ describe('messages utils', () => {
text: 'abc def',
description: undefined,
meaning: 'meaning',
id: 'message-id',
customId: 'message-id',
legacyIds: ['legacy-message-id']
});
expect(parseMetadata(
@ -248,7 +249,7 @@ describe('messages utils', () => {
text: 'abc def',
description: undefined,
meaning: 'meaning',
id: undefined,
customId: undefined,
legacyIds: ['legacy-message-id']
});
@ -256,7 +257,7 @@ describe('messages utils', () => {
text: 'abc def',
description: undefined,
meaning: undefined,
id: undefined,
customId: undefined,
legacyIds: ['legacy-message-id']
});
});
@ -266,7 +267,7 @@ describe('messages utils', () => {
text: 'abc def',
meaning: undefined,
description: undefined,
id: undefined,
customId: undefined,
legacyIds: []
});
});