Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(localize): render context of translation file parse errors #38673

Closed
wants to merge 2 commits into from
Closed

fix(localize): render context of translation file parse errors #38673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb87459
fix(localize): render context of translation file parse errors
petebacondarwin Sep 2, 2020
9e3d0f8
fixup! fix(localize): render context of translation file parse errors
petebacondarwin Sep 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ize/src/tools/src/translate/translation_files/message_serialization/message_serializer.ts
View file
Open in desktop
Expand Up @@ -13,7 +13,7 @@ import {getAttribute, getAttrOrThrow} from '../translation_parsers/translation_u

import {MessageRenderer} from './message_renderer';

interface MessageSerializerConfig {
export interface MessageSerializerConfig {
inlineElements: string[];
placeholder?: {elementName: string; nameAttribute: string; bodyAttribute?: string;};
placeholderContainer?: {elementName: string; startAttribute: string; endAttribute: string;};
Expand Down
32 changes: 32 additions & 0 deletions ...ools/src/translate/translation_files/translation_parsers/serialize_translation_message.ts
View file
Open in desktop
@@ -0,0 +1,32 @@
/**
* @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 {Element, ParseError} from '@angular/compiler';
import {ɵParsedTranslation} from '@angular/localize';

import {MessageSerializer, MessageSerializerConfig} from '../message_serialization/message_serializer';
import {TargetMessageRenderer} from '../message_serialization/target_message_renderer';

import {parseInnerRange} from './translation_utils';

/**
* Serialize the given `element` into a parsed translation using the given `serializer`.
*/
export function serializeTranslationMessage(element: Element, config: MessageSerializerConfig): {
translation: ɵParsedTranslation|null,
parseErrors: ParseError[],
serializeErrors: ParseError[]
} {
const {rootNodes, errors: parseErrors} = parseInnerRange(element);
try {
const serializer = new MessageSerializer(new TargetMessageRenderer(), config);
const translation = serializer.serialize(rootNodes);
return {translation, parseErrors, serializeErrors: []};
} catch (e) {
return {translation: null, parseErrors, serializeErrors: [e]};
}
}
18 changes: 8 additions & 10 deletions .../src/tools/src/translate/translation_files/translation_parsers/translation_parse_error.ts
View file
Open in desktop
Expand Up @@ -14,17 +14,15 @@ export class TranslationParseError extends Error {
constructor(
public span: ParseSourceSpan, public msg: string,
public level: ParseErrorLevel = ParseErrorLevel.ERROR) {
super(msg);
}

contextualMessage(): string {
const ctx = this.span.start.getContext(100, 3);
return ctx ? `${this.msg} ("${ctx.before}[${ParseErrorLevel[this.level]} ->]${ctx.after}")` :
this.msg;
super(contextualMessage(span, msg, level));
}
}

toString(): string {
const details = this.span.details ? `, ${this.span.details}` : '';
return `${this.contextualMessage()}: ${this.span.start}${details}`;
function contextualMessage(span: ParseSourceSpan, msg: string, level: ParseErrorLevel): string {
const ctx = span.start.getContext(100, 2);
msg += `\nAt ${span.start}${span.details ? `, ${span.details}` : ''}:\n`;
if (ctx) {
msg += `...${ctx.before}[${ParseErrorLevel[level]} ->]${ctx.after}...\n`;
}
return msg;
}
25 changes: 17 additions & 8 deletions ...calize/src/tools/src/translate/translation_files/translation_parsers/translation_utils.ts
View file
Open in desktop
Expand Up @@ -5,10 +5,12 @@
* 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 {Element, LexerRange, Node, ParseError, ParseErrorLevel, ParseSourceSpan, XmlParser} from '@angular/compiler';
import {Element, LexerRange, Node, ParseError, ParseErrorLevel, ParseSourceSpan, ParseTreeResult, XmlParser} from '@angular/compiler';

import {Diagnostics} from '../../../diagnostics';

import {TranslationParseError} from './translation_parse_error';
import {ParseAnalysis} from './translation_parser';
import {ParseAnalysis, ParsedTranslationBundle} from './translation_parser';

export function getAttrOrThrow(element: Element, attrName: string): string {
const attrValue = getAttribute(element, attrName);
Expand All @@ -30,17 +32,15 @@ export function getAttribute(element: Element, attrName: string): string|undefin
* This would be equivalent to parsing the `innerHTML` string of an HTML document.
*
* @param element The element whose inner range we want to parse.
* @returns a collection of XML `Node` objects that were parsed from the element's contents.
* @returns a collection of XML `Node` objects and any errors that were parsed from the element's
* contents.
*/
export function parseInnerRange(element: Element): Node[] {
export function parseInnerRange(element: Element): ParseTreeResult {
const xmlParser = new XmlParser();
const xml = xmlParser.parse(
element.sourceSpan.start.file.content, element.sourceSpan.start.file.url,
{tokenizeExpansionForms: true, range: getInnerRange(element)});
if (xml.errors.length) {
throw xml.errors.map(e => new TranslationParseError(e.span, e.msg).toString()).join('\n');
}
return xml.rootNodes;
return xml;
}

/**
Expand Down Expand Up @@ -155,3 +155,12 @@ export function addParseError(diagnostics: Diagnostics, parseError: ParseError):
diagnostics.warn(parseError.toString());
}
}

/**
* Add the provided `errors` to the `bundle` diagnostics.
*/
export function addErrorsToBundle(bundle: ParsedTranslationBundle, errors: ParseError[]): void {
for (const error of errors) {
addParseError(bundle.diagnostics, error);
}
}
31 changes: 10 additions & 21 deletions ...rc/tools/src/translate/translation_files/translation_parsers/xliff1_translation_parser.ts
View file
Open in desktop
Expand Up @@ -6,15 +6,13 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Element, ParseErrorLevel, visitAll} from '@angular/compiler';
import {ɵParsedTranslation} from '@angular/localize';

import {Diagnostics} from '../../../diagnostics';
import {BaseVisitor} from '../base_visitor';
import {MessageSerializer} from '../message_serialization/message_serializer';
import {TargetMessageRenderer} from '../message_serialization/target_message_renderer';

import {serializeTranslationMessage} from './serialize_translation_message';
import {ParseAnalysis, ParsedTranslationBundle, TranslationParser} from './translation_parser';
import {addParseDiagnostic, addParseError, canParseXml, getAttribute, isNamedElement, parseInnerRange, XmlTranslationParserHint} from './translation_utils';
import {addErrorsToBundle, addParseDiagnostic, addParseError, canParseXml, getAttribute, isNamedElement, XmlTranslationParserHint} from './translation_utils';

/**
* A translation parser that can load XLIFF 1.2 files.
Expand Down Expand Up @@ -150,23 +148,14 @@ class XliffTranslationVisitor extends BaseVisitor {
return;
}

try {
bundle.translations[id] = serializeTargetMessage(targetMessage);
} catch (e) {
// Capture any errors from serialize the target message
if (e.span && e.msg && e.level) {
addParseDiagnostic(bundle.diagnostics, e.span, e.msg, e.level);
} else {
throw e;
}
const {translation, parseErrors, serializeErrors} = serializeTranslationMessage(targetMessage, {
inlineElements: ['g', 'bx', 'ex', 'bpt', 'ept', 'ph', 'it', 'mrk'],
placeholder: {elementName: 'x', nameAttribute: 'id'}
});
if (translation !== null) {
bundle.translations[id] = translation;
}
addErrorsToBundle(bundle, parseErrors);
addErrorsToBundle(bundle, serializeErrors);
}
}

function serializeTargetMessage(source: Element): ɵParsedTranslation {
const serializer = new MessageSerializer(new TargetMessageRenderer(), {
inlineElements: ['g', 'bx', 'ex', 'bpt', 'ept', 'ph', 'it', 'mrk'],
placeholder: {elementName: 'x', nameAttribute: 'id'}
});
return serializer.serialize(parseInnerRange(source));
}
35 changes: 12 additions & 23 deletions ...rc/tools/src/translate/translation_files/translation_parsers/xliff2_translation_parser.ts
View file
Open in desktop
Expand Up @@ -6,15 +6,13 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Element, Node, ParseErrorLevel, visitAll} from '@angular/compiler';
import {ɵParsedTranslation} from '@angular/localize';

import {Diagnostics} from '../../../diagnostics';
import {BaseVisitor} from '../base_visitor';
import {MessageSerializer} from '../message_serialization/message_serializer';
import {TargetMessageRenderer} from '../message_serialization/target_message_renderer';

import {serializeTranslationMessage} from './serialize_translation_message';
import {ParseAnalysis, ParsedTranslationBundle, TranslationParser} from './translation_parser';
import {addParseDiagnostic, addParseError, canParseXml, getAttribute, isNamedElement, parseInnerRange, XmlTranslationParserHint} from './translation_utils';
import {addErrorsToBundle, addParseDiagnostic, addParseError, canParseXml, getAttribute, isNamedElement, XmlTranslationParserHint} from './translation_utils';

/**
* A translation parser that can load translations from XLIFF 2 files.
Expand Down Expand Up @@ -141,29 +139,20 @@ class Xliff2TranslationVisitor extends BaseVisitor {
return;
}

try {
bundle.translations[unit] = serializeTargetMessage(targetMessage);
} catch (e) {
// Capture any errors from serialize the target message
if (e.span && e.msg && e.level) {
addParseDiagnostic(bundle.diagnostics, e.span, e.msg, e.level);
} else {
throw e;
}
const {translation, parseErrors, serializeErrors} = serializeTranslationMessage(targetMessage, {
inlineElements: ['cp', 'sc', 'ec', 'mrk', 'sm', 'em'],
placeholder: {elementName: 'ph', nameAttribute: 'equiv', bodyAttribute: 'disp'},
placeholderContainer:
{elementName: 'pc', startAttribute: 'equivStart', endAttribute: 'equivEnd'}
});
if (translation !== null) {
bundle.translations[unit] = translation;
}
addErrorsToBundle(bundle, parseErrors);
addErrorsToBundle(bundle, serializeErrors);
}
}

function serializeTargetMessage(source: Element): ɵParsedTranslation {
const serializer = new MessageSerializer(new TargetMessageRenderer(), {
inlineElements: ['cp', 'sc', 'ec', 'mrk', 'sm', 'em'],
placeholder: {elementName: 'ph', nameAttribute: 'equiv', bodyAttribute: 'disp'},
placeholderContainer:
{elementName: 'pc', startAttribute: 'equivStart', endAttribute: 'equivEnd'}
});
return serializer.serialize(parseInnerRange(source));
}

function isFileElement(node: Node): node is Element {
return node instanceof Element && node.name === 'file';
}
40 changes: 17 additions & 23 deletions ...e/src/tools/src/translate/translation_files/translation_parsers/xtb_translation_parser.ts
View file
Open in desktop
Expand Up @@ -5,17 +5,15 @@
* 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 {Element, ParseErrorLevel, visitAll} from '@angular/compiler';
import {ɵParsedTranslation} from '@angular/localize';
import {Element, ParseError, ParseErrorLevel, visitAll} from '@angular/compiler';
import {extname} from 'path';

import {Diagnostics} from '../../../diagnostics';
import {BaseVisitor} from '../base_visitor';
import {MessageSerializer} from '../message_serialization/message_serializer';
import {TargetMessageRenderer} from '../message_serialization/target_message_renderer';

import {serializeTranslationMessage} from './serialize_translation_message';
import {ParseAnalysis, ParsedTranslationBundle, TranslationParser} from './translation_parser';
import {addParseDiagnostic, addParseError, canParseXml, getAttribute, parseInnerRange, XmlTranslationParserHint} from './translation_utils';
import {addErrorsToBundle, addParseDiagnostic, addParseError, canParseXml, getAttribute, XmlTranslationParserHint} from './translation_utils';


/**
Expand Down Expand Up @@ -103,20 +101,17 @@ class XtbVisitor extends BaseVisitor {
return;
}

try {
bundle.translations[id] = serializeTargetMessage(element);
} catch (error) {
if (typeof error === 'string') {
bundle.diagnostics.warn(
`Could not parse message with id "${
id}" - perhaps it has an unrecognised ICU format?\n` +
error);
} else if (error.span && error.msg && error.level) {
addParseDiagnostic(bundle.diagnostics, error.span, error.msg, error.level);
} else {
throw error;
}
const {translation, parseErrors, serializeErrors} = serializeTranslationMessage(
element, {inlineElements: [], placeholder: {elementName: 'ph', nameAttribute: 'name'}});
if (parseErrors.length) {
// We only want to warn (not error) if there were problems parsing the translation for
// XTB formatted files. See https://github.com/angular/angular/issues/14046.
bundle.diagnostics.warn(computeParseWarning(id, parseErrors));
} else if (translation !== null) {
// Only store the translation if there were no parse errors
bundle.translations[id] = translation;
}
addErrorsToBundle(bundle, serializeErrors);
break;

default:
Expand All @@ -127,9 +122,8 @@ class XtbVisitor extends BaseVisitor {
}
}

function serializeTargetMessage(source: Element): ɵParsedTranslation {
const serializer = new MessageSerializer(
new TargetMessageRenderer(),
{inlineElements: [], placeholder: {elementName: 'ph', nameAttribute: 'name'}});
return serializer.serialize(parseInnerRange(source));
function computeParseWarning(id: string, errors: ParseError[]): string {
const msg = errors.map(e => e.toString()).join('\n');
return `Could not parse message with id "${id}" - perhaps it has an unrecognised ICU format?\n` +
msg;
}
28 changes: 15 additions & 13 deletions ...ls/test/translate/translation_files/translation_parsers/xliff1_translation_parser_spec.ts
View file
Open in desktop
Expand Up @@ -621,7 +621,7 @@ describe('Xliff1TranslationParser', () => {
});

describe('[structure errors]', () => {
it('should throw when a trans-unit has no translation', () => {
it('should fail when a trans-unit has no translation', () => {
const XLIFF = [
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`,
Expand All @@ -646,7 +646,7 @@ describe('Xliff1TranslationParser', () => {
});


it('should throw when a trans-unit has no id attribute', () => {
it('should fail when a trans-unit has no id attribute', () => {
const XLIFF = [
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`,
Expand All @@ -671,7 +671,7 @@ describe('Xliff1TranslationParser', () => {
].join('\n'));
});

it('should throw on duplicate trans-unit id', () => {
it('should fail on duplicate trans-unit id', () => {
const XLIFF = [
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`,
Expand Down Expand Up @@ -703,7 +703,7 @@ describe('Xliff1TranslationParser', () => {
});

describe('[message errors]', () => {
it('should throw on unknown message tags', () => {
it('should fail on unknown message tags', () => {
const XLIFF = [
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`,
Expand All @@ -719,17 +719,18 @@ describe('Xliff1TranslationParser', () => {
].join('\n');

expectToFail('/some/file.xlf', XLIFF, /Invalid element found in message/, [
`Invalid element found in message. ("`,
` <trans-unit id="deadbeef" datatype="html">`,
`Error: Invalid element found in message.`,
`At /some/file.xlf@6:16:`,
`...`,
` <source/>`,
` <target>[ERROR ->]<b>msg should contain only ph tags</b></target>`,
` </trans-unit>`,
` </body>`,
`"): /some/file.xlf@6:16`,
`...`,
``,
].join('\n'));
});

it('should throw when a placeholder misses an id attribute', () => {
it('should fail when a placeholder misses an id attribute', () => {
const XLIFF = [
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`,
Expand All @@ -745,13 +746,14 @@ describe('Xliff1TranslationParser', () => {
].join('\n');

expectToFail('/some/file.xlf', XLIFF, /required "id" attribute/gi, [
`Missing required "id" attribute: ("`,
` <trans-unit id="deadbeef" datatype="html">`,
`Error: Missing required "id" attribute:`,
`At /some/file.xlf@6:16:`,
`...`,
` <source/>`,
` <target>[ERROR ->]<x/></target>`,
` </trans-unit>`,
` </body>`,
`"): /some/file.xlf@6:16`,
`...`,
``,
].join('\n'));
});
});
Expand Down