fix(localize): relax error to warning for missing target (#41944)

Some localization workflows want to use the extracted source translation
files directy back in the project as a target translation file.

The extraction process generates files that only contain "source" messages
and not "target" messages. This is actually valid for most translation formats
but currently the Angular localization process expects target translation files
to always contain target messages and will stop with an error in this case.

Now, instead of an error, the translation file loader will log a warning,
and then try to falback to a source message, only erroring if this is also
missing.

Fixes #21690

PR Close #41944
This commit is contained in:
Pete Bacon Darwin 2021-05-04 15:28:41 +01:00 committed by Misko Hevery
parent fdf2e02bff
commit 992c70df59
4 changed files with 1491 additions and 1409 deletions

View File

@ -140,14 +140,24 @@ class XliffTranslationVisitor extends BaseVisitor {
return; return;
} }
// Error if there is no `<target>` child element let targetMessage = element.children.find(isNamedElement('target'));
const targetMessage = element.children.find(isNamedElement('target'));
if (targetMessage === undefined) { if (targetMessage === undefined) {
// Warn if there is no `<target>` child element
addParseDiagnostic( addParseDiagnostic(
bundle.diagnostics, element.sourceSpan, 'Missing required <target> element', bundle.diagnostics, element.sourceSpan, 'Missing <target> element',
ParseErrorLevel.WARNING);
// Fallback to the `<source>` element if available.
targetMessage = element.children.find(isNamedElement('source'));
if (targetMessage === undefined) {
// Error if there is neither `<target>` nor `<source>`.
addParseDiagnostic(
bundle.diagnostics, element.sourceSpan,
'Missing required element: one of <target> or <source> is required',
ParseErrorLevel.ERROR); ParseErrorLevel.ERROR);
return; return;
} }
}
const {translation, parseErrors, serializeErrors} = serializeTranslationMessage(targetMessage, { const {translation, parseErrors, serializeErrors} = serializeTranslationMessage(targetMessage, {
inlineElements: ['g', 'bx', 'ex', 'bpt', 'ept', 'ph', 'it', 'mrk'], inlineElements: ['g', 'bx', 'ex', 'bpt', 'ept', 'ph', 'it', 'mrk'],

View File

@ -132,13 +132,24 @@ class Xliff2TranslationVisitor extends BaseVisitor {
return; return;
} }
const targetMessage = element.children.find(isNamedElement('target')); let targetMessage = element.children.find(isNamedElement('target'));
if (targetMessage === undefined) { if (targetMessage === undefined) {
// Warn if there is no `<target>` child element
addParseDiagnostic( addParseDiagnostic(
bundle.diagnostics, element.sourceSpan, 'Missing required <target> element', bundle.diagnostics, element.sourceSpan, 'Missing <target> element',
ParseErrorLevel.WARNING);
// Fallback to the `<source>` element if available.
targetMessage = element.children.find(isNamedElement('source'));
if (targetMessage === undefined) {
// Error if there is neither `<target>` nor `<source>`.
addParseDiagnostic(
bundle.diagnostics, element.sourceSpan,
'Missing required element: one of <target> or <source> is required',
ParseErrorLevel.ERROR); ParseErrorLevel.ERROR);
return; return;
} }
}
const {translation, parseErrors, serializeErrors} = serializeTranslationMessage(targetMessage, { const {translation, parseErrors, serializeErrors} = serializeTranslationMessage(targetMessage, {
inlineElements: ['cp', 'sc', 'ec', 'mrk', 'sm', 'em'], inlineElements: ['cp', 'sc', 'ec', 'mrk', 'sm', 'em'],

View File

@ -9,7 +9,8 @@ import {ɵcomputeMsgId, ɵmakeParsedTranslation} from '@angular/localize';
import {ParseAnalysis, ParsedTranslationBundle} from '../../../../src/translate/translation_files/translation_parsers/translation_parser'; import {ParseAnalysis, ParsedTranslationBundle} from '../../../../src/translate/translation_files/translation_parsers/translation_parser';
import {Xliff1TranslationParser} from '../../../../src/translate/translation_files/translation_parsers/xliff1_translation_parser'; import {Xliff1TranslationParser} from '../../../../src/translate/translation_files/translation_parsers/xliff1_translation_parser';
describe('Xliff1TranslationParser', () => { describe(
'Xliff1TranslationParser', () => {
describe('canParse()', () => { describe('canParse()', () => {
it('should return true only if the file contains an <xliff> element with version="1.2" attribute', it('should return true only if the file contains an <xliff> element with version="1.2" attribute',
() => { () => {
@ -86,7 +87,8 @@ describe('Xliff1TranslationParser', () => {
}); });
for (const withHint of [true, false]) { for (const withHint of [true, false]) {
describe(`parse() [${withHint ? 'with' : 'without'} hint]`, () => { describe(
`parse() [${withHint ? 'with' : 'without'} hint]`, () => {
const doParse: (fileName: string, XLIFF: string) => ParsedTranslationBundle = const doParse: (fileName: string, XLIFF: string) => ParsedTranslationBundle =
withHint ? (fileName, XLIFF) => { withHint ? (fileName, XLIFF) => {
const parser = new Xliff1TranslationParser(); const parser = new Xliff1TranslationParser();
@ -100,12 +102,13 @@ describe('Xliff1TranslationParser', () => {
return parser.parse(fileName, XLIFF); return parser.parse(fileName, XLIFF);
}; };
const expectToFail: const expectToFail: (
(fileName: string, XLIFF: string, errorMatcher: RegExp, diagnosticMessage: string) => fileName: string, XLIFF: string, errorMatcher: RegExp,
void = withHint ? (fileName, XLIFF, _errorMatcher, diagnosticMessage) => { diagnosticMessage: string) => void =
withHint ? (fileName, XLIFF, _errorMatcher, diagnosticMessage) => {
const result = doParse(fileName, XLIFF); const result = doParse(fileName, XLIFF);
expect(result.diagnostics.messages.length).toEqual(1); expect(result.diagnostics.messages.length).toBeGreaterThan(0);
expect(result.diagnostics.messages[0].message).toEqual(diagnosticMessage); expect(result.diagnostics.messages.pop()!.message).toEqual(diagnosticMessage);
} : (fileName, XLIFF, errorMatcher, _diagnosticMessage) => { } : (fileName, XLIFF, errorMatcher, _diagnosticMessage) => {
expect(() => doParse(fileName, XLIFF)).toThrowError(errorMatcher); expect(() => doParse(fileName, XLIFF)).toThrowError(errorMatcher);
}; };
@ -239,7 +242,8 @@ describe('Xliff1TranslationParser', () => {
`</xliff>`, `</xliff>`,
].join('\n'); ].join('\n');
const result = doParse('/some/file.xlf', XLIFF); const result = doParse('/some/file.xlf', XLIFF);
expect(result.translations[ɵcomputeMsgId( expect(
result.translations[ɵcomputeMsgId(
'translatable {$START_TAG_SPAN}element {$START_BOLD_TEXT}with placeholders' + 'translatable {$START_TAG_SPAN}element {$START_BOLD_TEXT}with placeholders' +
'{$CLOSE_BOLD_TEXT}{$CLOSE_TAG_SPAN} {$INTERPOLATION}')]) '{$CLOSE_BOLD_TEXT}{$CLOSE_TAG_SPAN} {$INTERPOLATION}')])
.toEqual(ɵmakeParsedTranslation( .toEqual(ɵmakeParsedTranslation(
@ -277,11 +281,12 @@ describe('Xliff1TranslationParser', () => {
`</xliff>`, `</xliff>`,
].join('\n'); ].join('\n');
const result = doParse('/some/file.xlf', XLIFF); const result = doParse('/some/file.xlf', XLIFF);
const id = const id = ɵcomputeMsgId(
ɵcomputeMsgId('{$START_TAG_APP_MY_COMPONENT}{$CLOSE_TAG_APP_MY_COMPONENT} Welcome'); '{$START_TAG_APP_MY_COMPONENT}{$CLOSE_TAG_APP_MY_COMPONENT} Welcome');
expect(result.translations[id]).toEqual(ɵmakeParsedTranslation(['', '', ' Translate'], [ expect(result.translations[id])
'START_TAG_APP_MY_COMPONENT', 'CLOSE_TAG_APP_MY_COMPONENT' .toEqual(ɵmakeParsedTranslation(
])); ['', '', ' Translate'],
['START_TAG_APP_MY_COMPONENT', 'CLOSE_TAG_APP_MY_COMPONENT']));
}); });
it('should extract translations with simple ICU expressions', () => { it('should extract translations with simple ICU expressions', () => {
@ -360,7 +365,8 @@ describe('Xliff1TranslationParser', () => {
].join('\n'); ].join('\n');
const result = doParse('/some/file.xlf', XLIFF); const result = doParse('/some/file.xlf', XLIFF);
expect(result.translations[ɵcomputeMsgId('foo')]).toEqual(ɵmakeParsedTranslation(['oof'])); expect(result.translations[ɵcomputeMsgId('foo')])
.toEqual(ɵmakeParsedTranslation(['oof']));
expect(result.translations['i']).toEqual(ɵmakeParsedTranslation(['toto'])); expect(result.translations['i']).toEqual(ɵmakeParsedTranslation(['toto']));
expect(result.translations['bar']).toEqual(ɵmakeParsedTranslation(['tata'])); expect(result.translations['bar']).toEqual(ɵmakeParsedTranslation(['tata']));
}); });
@ -394,8 +400,8 @@ describe('Xliff1TranslationParser', () => {
const result = doParse('/some/file.xlf', XLIFF); const result = doParse('/some/file.xlf', XLIFF);
expect(result.translations[ɵcomputeMsgId('{$LINE_BREAK}{$TAG_IMG}{$TAG_IMG_1}')]) expect(result.translations[ɵcomputeMsgId('{$LINE_BREAK}{$TAG_IMG}{$TAG_IMG_1}')])
.toEqual( .toEqual(ɵmakeParsedTranslation(
ɵmakeParsedTranslation(['', '', '', ''], ['TAG_IMG_1', 'TAG_IMG', 'LINE_BREAK'])); ['', '', '', ''], ['TAG_IMG_1', 'TAG_IMG', 'LINE_BREAK']));
}); });
it('should extract translations with empty target', () => { it('should extract translations with empty target', () => {
@ -425,7 +431,8 @@ describe('Xliff1TranslationParser', () => {
].join('\n'); ].join('\n');
const result = doParse('/some/file.xlf', XLIFF); const result = doParse('/some/file.xlf', XLIFF);
expect(result.translations[ɵcomputeMsgId('hello {$START_TAG_SPAN}{$CLOSE_TAG_SPAN}')]) expect(
result.translations[ɵcomputeMsgId('hello {$START_TAG_SPAN}{$CLOSE_TAG_SPAN}')])
.toEqual(ɵmakeParsedTranslation([''])); .toEqual(ɵmakeParsedTranslation(['']));
}); });
@ -434,8 +441,8 @@ describe('Xliff1TranslationParser', () => {
* Source HTML: * Source HTML:
* *
* ``` * ```
* Test: { count, plural, =0 { { sex, select, other {<p>deeply nested</p>}} } =other {a * Test: { count, plural, =0 { { sex, select, other {<p>deeply nested</p>}} }
* lot}} * =other {a lot}}
* ``` * ```
* *
* Note that the message gets split into two translation units: * Note that the message gets split into two translation units:
@ -443,7 +450,8 @@ describe('Xliff1TranslationParser', () => {
* * The second one is the ICU expansion itself * * The second one is the ICU expansion itself
* *
* Note that special markers `VAR_PLURAL` and `VAR_SELECT` are added, which are then * Note that special markers `VAR_PLURAL` and `VAR_SELECT` are added, which are then
* replaced by IVY at runtime with the actual values being rendered by the ICU expansion. * replaced by IVY at runtime with the actual values being rendered by the ICU
* expansion.
*/ */
const XLIFF = [ const XLIFF = [
`<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`, `<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`,
@ -621,7 +629,8 @@ describe('Xliff1TranslationParser', () => {
}); });
describe('[structure errors]', () => { describe('[structure errors]', () => {
it('should fail when a trans-unit has no translation', () => { it('should warn when a trans-unit has no translation target but does have a source',
() => {
const XLIFF = [ const XLIFF = [
`<?xml version="1.0" encoding="UTF-8" ?>`, `<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`, `<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`,
@ -635,16 +644,42 @@ describe('Xliff1TranslationParser', () => {
`</xliff>`, `</xliff>`,
].join('\n'); ].join('\n');
expectToFail('/some/file.xlf', XLIFF, /Missing required <target> element/, [ const result = doParse('/some/file.xlf', XLIFF);
`Missing required <target> element ("e-language="en" target-language="fr" datatype="plaintext" original="ng2.template">`, expect(result.diagnostics.messages.length).toEqual(1);
expect(result.diagnostics.messages[0].message).toEqual([
`Missing <target> element ("e-language="en" target-language="fr" datatype="plaintext" original="ng2.template">`,
` <body>`, ` <body>`,
` [ERROR ->]<trans-unit id="missingtarget">`, ` [WARNING ->]<trans-unit id="missingtarget">`,
` <source/>`, ` <source/>`,
` </trans-unit>`, ` </trans-unit>`,
`"): /some/file.xlf@4:6`, `"): /some/file.xlf@4:6`,
].join('\n')); ].join('\n'));
}); });
it('should fail when a trans-unit has no translation target nor source', () => {
const XLIFF = [
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`,
` <file source-language="en" target-language="fr" datatype="plaintext" original="ng2.template">`,
` <body>`,
` <trans-unit id="missingtarget">`,
` </trans-unit>`,
` </body>`,
` </file>`,
`</xliff>`,
].join('\n');
expectToFail(
'/some/file.xlf', XLIFF,
/Missing required element: one of <target> or <source> is required/, [
`Missing required element: one of <target> or <source> is required ("e-language="en" target-language="fr" datatype="plaintext" original="ng2.template">`,
` <body>`,
` [ERROR ->]<trans-unit id="missingtarget">`,
` </trans-unit>`,
` </body>`,
`"): /some/file.xlf@4:6`,
].join('\n'));
});
it('should fail when a trans-unit has no id attribute', () => { it('should fail when a trans-unit has no id attribute', () => {
const XLIFF = [ const XLIFF = [
@ -690,7 +725,8 @@ describe('Xliff1TranslationParser', () => {
`</xliff>`, `</xliff>`,
].join('\n'); ].join('\n');
expectToFail('/some/file.xlf', XLIFF, /Duplicated translations for message "deadbeef"/, [ expectToFail(
'/some/file.xlf', XLIFF, /Duplicated translations for message "deadbeef"/, [
`Duplicated translations for message "deadbeef" ("`, `Duplicated translations for message "deadbeef" ("`,
` <target/>`, ` <target/>`,
` </trans-unit>`, ` </trans-unit>`,
@ -759,4 +795,4 @@ describe('Xliff1TranslationParser', () => {
}); });
}); });
} }
}); });

View File

@ -9,8 +9,7 @@ import {ɵcomputeMsgId, ɵmakeParsedTranslation} from '@angular/localize';
import {ParseAnalysis, ParsedTranslationBundle} from '../../../../src/translate/translation_files/translation_parsers/translation_parser'; import {ParseAnalysis, ParsedTranslationBundle} from '../../../../src/translate/translation_files/translation_parsers/translation_parser';
import {Xliff2TranslationParser} from '../../../../src/translate/translation_files/translation_parsers/xliff2_translation_parser'; import {Xliff2TranslationParser} from '../../../../src/translate/translation_files/translation_parsers/xliff2_translation_parser';
describe( describe('Xliff2TranslationParser', () => {
'Xliff2TranslationParser', () => {
describe('canParse()', () => { describe('canParse()', () => {
it('should return true if the file contains an <xliff> element with version="2.0" attribute', it('should return true if the file contains an <xliff> element with version="2.0" attribute',
() => { () => {
@ -107,12 +106,11 @@ describe(
}; };
const expectToFail: const expectToFail:
(fileName: string, XLIFF: string, errorMatcher: RegExp, (fileName: string, XLIFF: string, errorMatcher: RegExp, diagnosticMessage: string) =>
diagnosticMessage: string) => void = void = withHint ? (fileName, XLIFF, _errorMatcher, diagnosticMessage) => {
withHint ? (fileName, XLIFF, _errorMatcher, diagnosticMessage) => {
const result = doParse(fileName, XLIFF); const result = doParse(fileName, XLIFF);
expect(result.diagnostics.messages.length).toEqual(1); expect(result.diagnostics.messages.length).toBeGreaterThan(0);
expect(result.diagnostics.messages[0].message).toEqual(diagnosticMessage); expect(result.diagnostics.messages.pop()!.message).toEqual(diagnosticMessage);
} : (fileName, XLIFF, errorMatcher, _diagnosticMessage) => { } : (fileName, XLIFF, errorMatcher, _diagnosticMessage) => {
expect(() => doParse(fileName, XLIFF)).toThrowError(errorMatcher); expect(() => doParse(fileName, XLIFF)).toThrowError(errorMatcher);
}; };
@ -231,8 +229,7 @@ describe(
`</xliff>`, `</xliff>`,
].join('\n'); ].join('\n');
const result = doParse('/some/file.xlf', XLIFF); const result = doParse('/some/file.xlf', XLIFF);
expect( expect(result.translations[ɵcomputeMsgId(
result.translations[ɵcomputeMsgId(
'translatable {$START_TAG_SPAN}element {$START_BOLD_TEXT}with placeholders' + 'translatable {$START_TAG_SPAN}element {$START_BOLD_TEXT}with placeholders' +
'{$CLOSE_BOLD_TEXT}{$CLOSE_TAG_SPAN} {$INTERPOLATION}')]) '{$CLOSE_BOLD_TEXT}{$CLOSE_TAG_SPAN} {$INTERPOLATION}')])
.toEqual(ɵmakeParsedTranslation( .toEqual(ɵmakeParsedTranslation(
@ -386,8 +383,7 @@ describe(
`</xliff>`, `</xliff>`,
].join('\n'); ].join('\n');
const result = doParse('/some/file.xlf', XLIFF); const result = doParse('/some/file.xlf', XLIFF);
expect( expect(result.translations[ɵcomputeMsgId('hello {$START_TAG_SPAN}{$CLOSE_TAG_SPAN}')])
result.translations[ɵcomputeMsgId('hello {$START_TAG_SPAN}{$CLOSE_TAG_SPAN}')])
.toEqual(ɵmakeParsedTranslation([''])); .toEqual(ɵmakeParsedTranslation(['']));
}); });
@ -548,7 +544,8 @@ describe(
}); });
describe('[structure errors]', () => { describe('[structure errors]', () => {
it('should provide a diagnostic error when a trans-unit has no translation', () => { it('should provide a diagnostic warning when a trans-unit has no translation target but does have a source',
() => {
const XLIFF = [ const XLIFF = [
`<?xml version="1.0" encoding="UTF-8" ?>`, `<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="2.0" xmlns="urn:oasis:names:tc:xliff:document:2.0" srcLang="en" trgLang="fr">`, `<xliff version="2.0" xmlns="urn:oasis:names:tc:xliff:document:2.0" srcLang="en" trgLang="fr">`,
@ -562,13 +559,42 @@ describe(
`</xliff>`, `</xliff>`,
].join('\n'); ].join('\n');
expectToFail('/some/file.xlf', XLIFF, /Missing required <target> element/, [ const result = doParse('/some/file.xlf', XLIFF);
`Missing required <target> element ("`, expect(result.diagnostics.messages.length).toEqual(1);
expect(result.diagnostics.messages[0].message).toEqual([
`Missing <target> element ("`,
` <file original="ng.template" id="ngi18n">`,
` <unit id="missingtarget">`,
` [WARNING ->]<segment>`,
` <source/>`,
` </segment>`,
`"): /some/file.xlf@4:6`,
].join('\n'));
});
it('should provide a diagnostic error when a trans-unit has no translation target nor source',
() => {
const XLIFF = [
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="2.0" xmlns="urn:oasis:names:tc:xliff:document:2.0" srcLang="en" trgLang="fr">`,
` <file original="ng.template" id="ngi18n">`,
` <unit id="missingtarget">`,
` <segment>`,
` </segment>`,
` </unit>`,
` </file>`,
`</xliff>`,
].join('\n');
expectToFail(
'/some/file.xlf', XLIFF,
/Missing required element: one of <target> or <source> is required/, [
`Missing required element: one of <target> or <source> is required ("`,
` <file original="ng.template" id="ngi18n">`, ` <file original="ng.template" id="ngi18n">`,
` <unit id="missingtarget">`, ` <unit id="missingtarget">`,
` [ERROR ->]<segment>`, ` [ERROR ->]<segment>`,
` <source/>`,
` </segment>`, ` </segment>`,
` </unit>`,
`"): /some/file.xlf@4:6`, `"): /some/file.xlf@4:6`,
].join('\n')); ].join('\n'));
}); });
@ -661,8 +687,7 @@ describe(
].join('\n')); ].join('\n'));
}); });
it('should provide a diagnostic error when a placeholder misses an id attribute', it('should provide a diagnostic error when a placeholder misses an id attribute', () => {
() => {
const XLIFF = [ const XLIFF = [
`<?xml version="1.0" encoding="UTF-8" ?>`, `<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="2.0" xmlns="urn:oasis:names:tc:xliff:document:2.0" srcLang="en" trgLang="fr">`, `<xliff version="2.0" xmlns="urn:oasis:names:tc:xliff:document:2.0" srcLang="en" trgLang="fr">`,
@ -691,4 +716,4 @@ describe(
}); });
}); });
} }
}); });