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

packages/localize/src/tools/src/translate/translation_files/translation_parsers/serialize_translation_message.ts

@@ -0,0 +1,29 @@
+/**
+ * @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} from '../message_serialization/message_serializer';
+
+import {parseInnerRange} from './translation_utils';
+
+/**
+ * Serialize the given `element` into a parsed translation using the given `serializer`.
+ */
+export function serializeTranslationMessage(
+    element: Element, serializer: MessageSerializer<ɵParsedTranslation>):
+    {translation: ɵParsedTranslation|null, errors: ParseError[]} {
+  const {rootNodes, errors} = parseInnerRange(element);
+  let translation: ɵParsedTranslation|null = null;
+  try {
+    translation = serializer.serialize(rootNodes);
+  } catch (e) {
+    errors.push(e);
+  }
+  return {translation, errors};
+}

packages/localize/src/tools/src/translate/translation_files/translation_parsers/translation_parse_error.ts

@@ -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;
 }

packages/localize/src/tools/src/translate/translation_files/translation_parsers/translation_utils.ts

@@ -5,8 +5,10 @@
  * 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';
 
@@ -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;
 }
 
 /**

packages/localize/src/tools/src/translate/translation_files/translation_parsers/xliff1_translation_parser.ts

@@ -6,15 +6,15 @@
  * 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 {addParseDiagnostic, addParseError, canParseXml, getAttribute, isNamedElement, XmlTranslationParserHint} from './translation_utils';
 
 /**
  * A translation parser that can load XLIFF 1.2 files.
@@ -150,23 +150,16 @@ 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 serializer = new MessageSerializer(new TargetMessageRenderer(), {
+      inlineElements: ['g', 'bx', 'ex', 'bpt', 'ept', 'ph', 'it', 'mrk'],
+      placeholder: {elementName: 'x', nameAttribute: 'id'}
+    });
+    const {translation, errors} = serializeTranslationMessage(targetMessage, serializer);
+    if (translation !== null) {
+      bundle.translations[id] = translation;
+    }
+    for (const error of errors) {
+      addParseError(bundle.diagnostics, error);
     }
   }
 }
-
-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));
-}

packages/localize/src/tools/src/translate/translation_files/translation_parsers/xliff2_translation_parser.ts

@@ -6,15 +6,15 @@
  * 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 {addParseDiagnostic, addParseError, canParseXml, getAttribute, isNamedElement, XmlTranslationParserHint} from './translation_utils';
 
 /**
  * A translation parser that can load translations from XLIFF 2 files.
@@ -141,29 +141,22 @@ 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 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'}
+    });
+    const {translation, errors} = serializeTranslationMessage(targetMessage, serializer);
+    if (translation !== null) {
+      bundle.translations[unit] = translation;
+    }
+    for (const error of errors) {
+      addParseError(bundle.diagnostics, error);
     }
   }
 }
 
-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';
 }

packages/localize/src/tools/src/translate/translation_files/translation_parsers/xtb_translation_parser.ts

@@ -5,8 +5,8 @@
  * 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 {ɵParsedTranslation} from '@angular/localize/private';
 import {extname} from 'path';
 
 import {Diagnostics} from '../../../diagnostics';
@@ -103,19 +103,13 @@ 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 serializer = new MessageSerializer(
+            new TargetMessageRenderer(),
+            {inlineElements: [], placeholder: {elementName: 'ph', nameAttribute: 'name'}});
+        const translation =
+            serializeTranslationMessage(id, element, serializer, bundle.diagnostics);
+        if (translation !== null) {
+          bundle.translations[id] = translation;
         }
         break;
 
@@ -127,9 +121,25 @@ 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));
+/**
+ * Serialize the given `element` into a parsed translation using the given `serializer`.
+ */
+function serializeTranslationMessage(
+    id: string, element: Element, serializer: MessageSerializer<ɵParsedTranslation>,
+    diagnostics: Diagnostics): ɵParsedTranslation|null {
+  const {rootNodes, errors} = parseInnerRange(element);
+  if (errors.length) {
+    const msg = errors.map(e => e.toString()).join('\n');
+    diagnostics.warn(
+        `Could not parse message with id "${id}" - perhaps it has an unrecognised ICU format?\n` +
+        msg);
+    return null;
+  }
+
+  try {
+    return serializer.serialize(rootNodes);
+  } catch (e) {
+    addParseError(diagnostics, e);
+    return null;
+  }
 }

packages/localize/src/tools/test/translate/translation_files/translation_parsers/xliff1_translation_parser_spec.ts

@@ -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">`,
@@ -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">`,
@@ -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">`,
@@ -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">`,
@@ -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">`,
@@ -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'));
         });
       });

packages/localize/src/tools/test/translate/translation_files/translation_parsers/xliff2_translation_parser_spec.ts

@@ -650,13 +650,14 @@ describe(
                   ].join('\n');
 
                   expectToFail('/some/file.xlf', XLIFF, /Invalid element found in message/, [
-                    `Invalid element found in message. ("`,
-                    `      <segment>`,
+                    `Error: Invalid element found in message.`,
+                    `At /some/file.xlf@6:16:`,
+                    `...`,
                     `        <source/>`,
                     `        <target>[ERROR ->]<b>msg should contain only ph and pc tags</b></target>`,
                     `      </segment>`,
-                    `    </unit>`,
-                    `"): /some/file.xlf@6:16`,
+                    `...`,
+                    ``,
                   ].join('\n'));
                 });
 
@@ -677,13 +678,14 @@ describe(
                      ].join('\n');
 
                      expectToFail('/some/file.xlf', XLIFF, /Missing required "equiv" attribute/, [
-                       `Missing required "equiv" attribute: ("`,
-                       `      <segment>`,
+                       `Error: Missing required "equiv" attribute:`,
+                       `At /some/file.xlf@6:16:`,
+                       `...`,
                        `        <source/>`,
                        `        <target>[ERROR ->]<ph/></target>`,
                        `      </segment>`,
-                       `    </unit>`,
-                       `"): /some/file.xlf@6:16`,
+                       `...`,
+                       ``,
                      ].join('\n'));
                    });
               });