refactor(localize): add support for TranslationParser diagnostics (#35793)

This modifies the internal (but shared with CLI) API for loading/parsing
translation files. Now the parsers will return a new `Diagnostics` object
along with any translations and locale extracted from the file.

It is up to the caller to decide what to do about this, if there are errors
it is suggested that an error is thrown, which is what the `TranslationLoader`
class does.

PR Close #35793
This commit is contained in:
Pete Bacon Darwin 2020-02-28 14:45:31 +00:00 committed by Matias Niemelä
parent 99ce015f42
commit d88b237f1e
11 changed files with 115 additions and 27 deletions

View File

@ -15,4 +15,15 @@ export class Diagnostics {
get hasErrors() { return this.messages.some(m => m.type === 'error'); } get hasErrors() { return this.messages.some(m => m.type === 'error'); }
warn(message: string) { this.messages.push({type: 'warning', message}); } warn(message: string) { this.messages.push({type: 'warning', message}); }
error(message: string) { this.messages.push({type: 'error', message}); } error(message: string) { this.messages.push({type: 'error', message}); }
formatDiagnostics(message: string): string {
const errors = this.messages !.filter(d => d.type === 'error').map(d => ' - ' + d.message);
const warnings = this.messages !.filter(d => d.type === 'warning').map(d => ' - ' + d.message);
if (errors.length) {
message += '\nERRORS:\n' + errors.join('\n');
}
if (warnings.length) {
message += '\nWARNINGS:\n' + warnings.join('\n');
}
return message;
}
} }

View File

@ -142,7 +142,7 @@ export function translateFiles({sourceRootPath, sourceFilePaths, translationFile
[ [
new Xliff2TranslationParser(), new Xliff2TranslationParser(),
new Xliff1TranslationParser(), new Xliff1TranslationParser(),
new XtbTranslationParser(diagnostics), new XtbTranslationParser(),
new SimpleJsonTranslationParser(), new SimpleJsonTranslationParser(),
], ],
diagnostics); diagnostics);

View File

@ -14,7 +14,9 @@ import {TranslationParser} from './translation_parsers/translation_parser';
* Use this class to load a collection of translation files from disk. * Use this class to load a collection of translation files from disk.
*/ */
export class TranslationLoader { export class TranslationLoader {
constructor(private translationParsers: TranslationParser[], private diagnostics: Diagnostics) {} constructor(
private translationParsers: TranslationParser[],
/** @deprecated */ private diagnostics?: Diagnostics) {}
/** /**
* Load and parse the translation files into a collection of `TranslationBundles`. * Load and parse the translation files into a collection of `TranslationBundles`.
@ -34,22 +36,37 @@ export class TranslationLoader {
return translationFilePaths.map((filePath, index) => { return translationFilePaths.map((filePath, index) => {
const fileContents = FileUtils.readFile(filePath); const fileContents = FileUtils.readFile(filePath);
for (const translationParser of this.translationParsers) { for (const translationParser of this.translationParsers) {
if (translationParser.canParse(filePath, fileContents)) { const result = translationParser.canParse(filePath, fileContents);
const providedLocale = translationFileLocales[index]; if (!result) {
const {locale: parsedLocale, translations} = continue;
translationParser.parse(filePath, fileContents);
const locale = providedLocale || parsedLocale;
if (locale === undefined) {
throw new Error(
`The translation file "${filePath}" does not contain a target locale and no explicit locale was provided for this file.`);
}
if (parsedLocale !== undefined && providedLocale !== undefined &&
parsedLocale !== providedLocale) {
this.diagnostics.warn(
`The provided locale "${providedLocale}" does not match the target locale "${parsedLocale}" found in the translation file "${filePath}".`);
}
return {locale, translations};
} }
const {locale: parsedLocale, translations, diagnostics} =
translationParser.parse(filePath, fileContents);
if (diagnostics.hasErrors) {
throw new Error(diagnostics.formatDiagnostics(
`The translation file "${filePath}" could not be parsed.`));
}
const providedLocale = translationFileLocales[index];
const locale = providedLocale || parsedLocale;
if (locale === undefined) {
throw new Error(
`The translation file "${filePath}" does not contain a target locale and no explicit locale was provided for this file.`);
}
if (parsedLocale !== undefined && providedLocale !== undefined &&
parsedLocale !== providedLocale) {
diagnostics.warn(
`The provided locale "${providedLocale}" does not match the target locale "${parsedLocale}" found in the translation file "${filePath}".`);
}
// If we were passed a diagnostics object then copy the messages over to it.
if (this.diagnostics) {
this.diagnostics.messages.push(...diagnostics.messages);
}
return {locale, translations, diagnostics};
} }
throw new Error( throw new Error(
`There is no "TranslationParser" that can parse this translation file: ${filePath}.`); `There is no "TranslationParser" that can parse this translation file: ${filePath}.`);

View File

@ -7,6 +7,7 @@
*/ */
import {ɵMessageId, ɵParsedTranslation, ɵparseTranslation} from '@angular/localize'; import {ɵMessageId, ɵParsedTranslation, ɵparseTranslation} from '@angular/localize';
import {extname} from 'path'; import {extname} from 'path';
import {Diagnostics} from '../../../diagnostics';
import {ParsedTranslationBundle, TranslationParser} from './translation_parser'; import {ParsedTranslationBundle, TranslationParser} from './translation_parser';
/** /**
@ -32,6 +33,6 @@ export class SimpleJsonTranslationParser implements TranslationParser {
const targetMessage = translations[messageId]; const targetMessage = translations[messageId];
parsedTranslations[messageId] = ɵparseTranslation(targetMessage); parsedTranslations[messageId] = ɵparseTranslation(targetMessage);
} }
return {locale: parsedLocale, translations: parsedTranslations}; return {locale: parsedLocale, translations: parsedTranslations, diagnostics: new Diagnostics()};
} }
} }

View File

@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {ɵMessageId, ɵParsedTranslation} from '@angular/localize/private'; import {ɵMessageId, ɵParsedTranslation} from '@angular/localize/private';
import {Diagnostics} from '../../../diagnostics';
/** /**
* An object that holds translations that have been parsed from a translation file. * An object that holds translations that have been parsed from a translation file.
@ -13,6 +14,7 @@ import {ɵMessageId, ɵParsedTranslation} from '@angular/localize/private';
export interface ParsedTranslationBundle { export interface ParsedTranslationBundle {
locale: string|undefined; locale: string|undefined;
translations: Record<ɵMessageId, ɵParsedTranslation>; translations: Record<ɵMessageId, ɵParsedTranslation>;
diagnostics: Diagnostics;
} }
/** /**

View File

@ -9,6 +9,7 @@ import {Element, Node, XmlParser, visitAll} from '@angular/compiler';
import {ɵMessageId, ɵParsedTranslation} from '@angular/localize'; import {ɵMessageId, ɵParsedTranslation} from '@angular/localize';
import {extname} from 'path'; import {extname} from 'path';
import {Diagnostics} from '../../../diagnostics';
import {BaseVisitor} from '../base_visitor'; import {BaseVisitor} from '../base_visitor';
import {MessageSerializer} from '../message_serialization/message_serializer'; import {MessageSerializer} from '../message_serialization/message_serializer';
import {TargetMessageRenderer} from '../message_serialization/target_message_renderer'; import {TargetMessageRenderer} from '../message_serialization/target_message_renderer';
@ -55,7 +56,8 @@ class XliffFileElementVisitor extends BaseVisitor {
if (element.name === 'file') { if (element.name === 'file') {
this.bundle = { this.bundle = {
locale: getAttribute(element, 'target-language'), locale: getAttribute(element, 'target-language'),
translations: XliffTranslationVisitor.extractTranslations(element) translations: XliffTranslationVisitor.extractTranslations(element),
diagnostics: new Diagnostics(),
}; };
} else { } else {
return visitAll(this, element.children); return visitAll(this, element.children);

View File

@ -9,6 +9,7 @@ import {Element, Node, XmlParser, visitAll} from '@angular/compiler';
import {ɵMessageId, ɵParsedTranslation} from '@angular/localize'; import {ɵMessageId, ɵParsedTranslation} from '@angular/localize';
import {extname} from 'path'; import {extname} from 'path';
import {Diagnostics} from '../../../diagnostics';
import {BaseVisitor} from '../base_visitor'; import {BaseVisitor} from '../base_visitor';
import {MessageSerializer} from '../message_serialization/message_serializer'; import {MessageSerializer} from '../message_serialization/message_serializer';
import {TargetMessageRenderer} from '../message_serialization/target_message_renderer'; import {TargetMessageRenderer} from '../message_serialization/target_message_renderer';
@ -61,7 +62,8 @@ class Xliff2TranslationBundleVisitor extends BaseVisitor {
} else if (element.name === 'file') { } else if (element.name === 'file') {
this.bundle = { this.bundle = {
locale: parsedLocale, locale: parsedLocale,
translations: Xliff2TranslationVisitor.extractTranslations(element) translations: Xliff2TranslationVisitor.extractTranslations(element),
diagnostics: new Diagnostics(),
}; };
} else { } else {
return visitAll(this, element.children, {parsedLocale}); return visitAll(this, element.children, {parsedLocale});

View File

@ -24,7 +24,7 @@ import {getAttrOrThrow, parseInnerRange} from './translation_utils';
* A translation parser that can load XB files. * A translation parser that can load XB files.
*/ */
export class XtbTranslationParser implements TranslationParser { export class XtbTranslationParser implements TranslationParser {
constructor(private diagnostics: Diagnostics) {} constructor(private diagnostics: Diagnostics = new Diagnostics()) {}
canParse(filePath: string, contents: string): boolean { canParse(filePath: string, contents: string): boolean {
const extension = extname(filePath); const extension = extname(filePath);
@ -61,7 +61,11 @@ class XtbVisitor extends BaseVisitor {
element.sourceSpan, '<translationbundle> elements can not be nested'); element.sourceSpan, '<translationbundle> elements can not be nested');
} }
const langAttr = element.attrs.find((attr) => attr.name === 'lang'); const langAttr = element.attrs.find((attr) => attr.name === 'lang');
bundle = {locale: langAttr && langAttr.value, translations: {}}; bundle = {
locale: langAttr && langAttr.value,
translations: {},
diagnostics: this.diagnostics
};
visitAll(this, element.children, bundle); visitAll(this, element.children, bundle);
return bundle; return bundle;

View File

@ -19,6 +19,7 @@ import {OutputPathFn} from './output_path';
export interface TranslationBundle { export interface TranslationBundle {
locale: string; locale: string;
translations: Record<ɵMessageId, ɵParsedTranslation>; translations: Record<ɵMessageId, ɵParsedTranslation>;
diagnostics?: Diagnostics;
} }
/** /**

View File

@ -0,0 +1,48 @@
/**
* @license
* Copyright Google Inc. 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 {Diagnostics} from '../src/diagnostics';
describe('Diagnostics', () => {
describe('formatDiagnostics', () => {
it('should just return the message passed in if there are no errors nor warnings', () => {
const diagnostics = new Diagnostics();
expect(diagnostics.formatDiagnostics('This is a message')).toEqual('This is a message');
});
it('should return a string with all the errors listed', () => {
const diagnostics = new Diagnostics();
diagnostics.error('Error 1');
diagnostics.error('Error 2');
diagnostics.error('Error 3');
expect(diagnostics.formatDiagnostics('This is a message'))
.toEqual('This is a message\nERRORS:\n - Error 1\n - Error 2\n - Error 3');
});
it('should return a string with all the warnings listed', () => {
const diagnostics = new Diagnostics();
diagnostics.warn('Warning 1');
diagnostics.warn('Warning 2');
diagnostics.warn('Warning 3');
expect(diagnostics.formatDiagnostics('This is a message'))
.toEqual('This is a message\nWARNINGS:\n - Warning 1\n - Warning 2\n - Warning 3');
});
it('should return a string with all the errors and warnings listed', () => {
const diagnostics = new Diagnostics();
diagnostics.warn('Warning 1');
diagnostics.warn('Warning 2');
diagnostics.error('Error 1');
diagnostics.error('Error 2');
diagnostics.warn('Warning 3');
diagnostics.error('Error 3');
expect(diagnostics.formatDiagnostics('This is a message'))
.toEqual(
'This is a message\nERRORS:\n - Error 1\n - Error 2\n - Error 3\nWARNINGS:\n - Warning 1\n - Warning 2\n - Warning 3');
});
});
});

View File

@ -58,8 +58,8 @@ describe('TranslationLoader', () => {
const result = const result =
loader.loadBundles(['/src/locale/messages.en.xlf', '/src/locale/messages.fr.xlf'], []); loader.loadBundles(['/src/locale/messages.en.xlf', '/src/locale/messages.fr.xlf'], []);
expect(result).toEqual([ expect(result).toEqual([
{locale: 'pl', translations}, {locale: 'pl', translations, diagnostics: new Diagnostics()},
{locale: 'pl', translations}, {locale: 'pl', translations, diagnostics: new Diagnostics()},
]); ]);
}); });
@ -71,8 +71,8 @@ describe('TranslationLoader', () => {
const result = loader.loadBundles( const result = loader.loadBundles(
['/src/locale/messages.en.xlf', '/src/locale/messages.fr.xlf'], ['en', 'fr']); ['/src/locale/messages.en.xlf', '/src/locale/messages.fr.xlf'], ['en', 'fr']);
expect(result).toEqual([ expect(result).toEqual([
{locale: 'en', translations}, {locale: 'en', translations, diagnostics: new Diagnostics()},
{locale: 'fr', translations}, {locale: 'fr', translations, diagnostics: new Diagnostics()},
]); ]);
}); });
@ -127,6 +127,6 @@ class MockTranslationParser implements TranslationParser {
parse(filePath: string, fileContents: string) { parse(filePath: string, fileContents: string) {
this.log.push(`parse(${filePath}, ${fileContents})`); this.log.push(`parse(${filePath}, ${fileContents})`);
return {locale: this._locale, translations: this._translations}; return {locale: this._locale, translations: this._translations, diagnostics: new Diagnostics()};
} }
} }