feat(I18nExtractor): Add file paths to error messages (#9177)

* feat(I18nExtractor): Add file paths to error messages

relates to #9071

* feat(i18n): allow i18n start comments without meaning

* refactor(i18n): cleanup

* test(HtmlParser): Add depth to expansion forms
This commit is contained in:
Victor Berchet 2016-06-14 17:50:23 -07:00 committed by GitHub
parent 7afee97d1b
commit fe01e2efb7
8 changed files with 112 additions and 104 deletions

View File

@ -19,22 +19,23 @@ import {CompileMetadataResolver, HtmlParser, DirectiveNormalizer, Lexer, Parser,
import {ReflectorHost} from './reflector_host';
import {StaticAndDynamicReflectionCapabilities} from './static_reflection_capabilities';
function extract(
ngOptions: tsc.AngularCompilerOptions, program: ts.Program, host: ts.CompilerHost) {
return Extractor.create(ngOptions, program, host).extract();
}
const GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/;
const _dirPaths = new Map<compiler.CompileDirectiveMetadata, string>();
const _GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/;
class Extractor {
constructor(
private options: tsc.AngularCompilerOptions, private program: ts.Program,
private _options: tsc.AngularCompilerOptions, private _program: ts.Program,
public host: ts.CompilerHost, private staticReflector: StaticReflector,
private resolver: CompileMetadataResolver, private compiler: compiler.OfflineCompiler,
private reflectorHost: ReflectorHost, private _extractor: MessageExtractor) {}
private _resolver: CompileMetadataResolver, private _compiler: compiler.OfflineCompiler,
private _reflectorHost: ReflectorHost, private _extractor: MessageExtractor) {}
private extractCmpMessages(metadatas: compiler.CompileDirectiveMetadata[]):
private _extractCmpMessages(metadatas: compiler.CompileDirectiveMetadata[]):
Promise<ExtractionResult> {
if (!metadatas || !metadatas.length) {
return null;
@ -42,10 +43,10 @@ class Extractor {
const normalize = (metadata: compiler.CompileDirectiveMetadata) => {
const directiveType = metadata.type.runtime;
const directives = this.resolver.getViewDirectivesMetadata(directiveType);
return Promise.all(directives.map(d => this.compiler.normalizeDirectiveMetadata(d)))
const directives = this._resolver.getViewDirectivesMetadata(directiveType);
return Promise.all(directives.map(d => this._compiler.normalizeDirectiveMetadata(d)))
.then(normalizedDirectives => {
const pipes = this.resolver.getViewPipesMetadata(directiveType);
const pipes = this._resolver.getViewPipesMetadata(directiveType);
return new compiler.NormalizedComponentWithViewDirectives(
metadata, normalizedDirectives, pipes);
});
@ -56,8 +57,8 @@ class Extractor {
let messages: Message[] = [];
let errors: ParseError[] = [];
cmps.forEach(cmp => {
// TODO(vicb): url
let result = this._extractor.extract(cmp.component.template.template, 'url');
let url = _dirPaths.get(cmp.component);
let result = this._extractor.extract(cmp.component.template.template, url);
errors = errors.concat(result.errors);
messages = messages.concat(result.messages);
});
@ -67,7 +68,7 @@ class Extractor {
});
}
private readComponents(absSourcePath: string) {
private _readComponents(absSourcePath: string): Promise<compiler.CompileDirectiveMetadata>[] {
const result: Promise<compiler.CompileDirectiveMetadata>[] = [];
const metadata = this.staticReflector.getModuleMetadata(absSourcePath);
if (!metadata) {
@ -80,26 +81,29 @@ class Extractor {
return result;
}
for (const symbol of symbols) {
const staticType = this.reflectorHost.findDeclaration(absSourcePath, symbol, absSourcePath);
const staticType = this._reflectorHost.findDeclaration(absSourcePath, symbol, absSourcePath);
let directive: compiler.CompileDirectiveMetadata;
directive = this.resolver.maybeGetDirectiveMetadata(<any>staticType);
directive = this._resolver.maybeGetDirectiveMetadata(<any>staticType);
if (!directive || !directive.isComponent) {
continue;
if (directive && directive.isComponent) {
let promise = this._compiler.normalizeDirectiveMetadata(directive);
promise.then(md => _dirPaths.set(md, absSourcePath));
result.push(promise);
}
result.push(this.compiler.normalizeDirectiveMetadata(directive));
}
return result;
}
extract(): Promise<any> {
const promises = this.program.getSourceFiles()
_dirPaths.clear();
const promises = this._program.getSourceFiles()
.map(sf => sf.fileName)
.filter(f => !GENERATED_FILES.test(f))
.filter(f => !_GENERATED_FILES.test(f))
.map(
(absSourcePath: string): Promise<any> =>
Promise.all(this.readComponents(absSourcePath))
.then(metadatas => this.extractCmpMessages(metadatas))
Promise.all(this._readComponents(absSourcePath))
.then(metadatas => this._extractCmpMessages(metadatas))
.catch(e => console.error(e.stack)));
let messages: Message[] = [];
@ -112,12 +116,12 @@ class Extractor {
});
if (errors.length) {
throw errors;
throw new Error(errors.map(e => e.toString()).join('\n'));
}
messages = removeDuplicates(messages);
let genPath = path.join(this.options.genDir, 'messages.xmb');
let genPath = path.join(this._options.genDir, 'messages.xmb');
let msgBundle = serializeXmb(messages);
this.host.writeFile(genPath, msgBundle, false);

View File

@ -23,9 +23,9 @@ const PLURAL_CASES: string[] = ['zero', 'one', 'two', 'few', 'many', 'other'];
*
* ```
* <ul [ngPlural]="messages.length">
* <template [ngPluralCase]="'=0'"><li i18n="plural_=0">zero</li></template>
* <template [ngPluralCase]="'=1'"><li i18n="plural_=1">one</li></template>
* <template [ngPluralCase]="'other'"><li i18n="plural_other">more than one</li></template>
* <template ngPluralCase="=0"><li i18n="plural_=0">zero</li></template>
* <template ngPluralCase="=1"><li i18n="plural_=1">one</li></template>
* <template ngPluralCase="other"><li i18n="plural_other">more than one</li></template>
* </ul>
* ```
*/
@ -39,6 +39,11 @@ export class ExpansionResult {
constructor(public nodes: HtmlAst[], public expanded: boolean, public errors: ParseError[]) {}
}
/**
* Expand expansion forms (plural, select) to directives
*
* @internal
*/
class _Expander implements HtmlAstVisitor {
expanded: boolean = false;
errors: ParseError[] = [];
@ -73,7 +78,7 @@ function _expandPluralForm(ast: HtmlExpansionAst, errors: ParseError[]): HtmlEle
`Plural cases should be "=<number>" or one of ${PLURAL_CASES.join(", ")}`));
}
let expansionResult = expandNodes(c.expression);
expansionResult.errors.forEach(e => errors.push(e));
errors.push(...expansionResult.errors);
let i18nAttrs = expansionResult.expanded ?
[] :
[new HtmlAttrAst('i18n', `${ast.type}_${c.value}`, c.valueSourceSpan)];

View File

@ -113,16 +113,16 @@ export class I18nHtmlParser implements HtmlParser {
} else {
let expanded = expandNodes(res.rootNodes);
let nodes = this._recurse(expanded.nodes);
this.errors = this.errors.concat(expanded.errors);
this.errors.push(...expanded.errors);
return this.errors.length > 0 ? new HtmlParseTreeResult([], this.errors) :
new HtmlParseTreeResult(nodes, []);
}
}
private _processI18nPart(p: Part): HtmlAst[] {
private _processI18nPart(part: Part): HtmlAst[] {
try {
return p.hasI18n ? this._mergeI18Part(p) : this._recurseIntoI18nPart(p);
return part.hasI18n ? this._mergeI18Part(part) : this._recurseIntoI18nPart(part);
} catch (e) {
if (e instanceof I18nError) {
this.errors.push(e);
@ -133,16 +133,17 @@ export class I18nHtmlParser implements HtmlParser {
}
}
private _mergeI18Part(p: Part): HtmlAst[] {
let message = p.createMessage(this._parser);
private _mergeI18Part(part: Part): HtmlAst[] {
let message = part.createMessage(this._parser);
let messageId = id(message);
if (!StringMapWrapper.contains(this._messages, messageId)) {
throw new I18nError(
p.sourceSpan, `Cannot find message for id '${messageId}', content '${message.content}'.`);
part.sourceSpan,
`Cannot find message for id '${messageId}', content '${message.content}'.`);
}
let parsedMessage = this._messages[messageId];
return this._mergeTrees(p, parsedMessage, p.children);
return this._mergeTrees(part, parsedMessage, part.children);
}
private _recurseIntoI18nPart(p: Part): HtmlAst[] {
@ -166,8 +167,8 @@ export class I18nHtmlParser implements HtmlParser {
}
private _recurse(nodes: HtmlAst[]): HtmlAst[] {
let ps = partition(nodes, this.errors, this._implicitTags);
return ListWrapper.flatten(ps.map(p => this._processI18nPart(p)));
let parts = partition(nodes, this.errors, this._implicitTags);
return ListWrapper.flatten(parts.map(p => this._processI18nPart(p)));
}
private _mergeTrees(p: Part, translated: HtmlAst[], original: HtmlAst[]): HtmlAst[] {

View File

@ -90,16 +90,16 @@ export function removeDuplicates(messages: Message[]): Message[] {
* 4. If a part has the i18n attribute, stringify the nodes to create a Message.
*/
export class MessageExtractor {
messages: Message[];
errors: ParseError[];
private _messages: Message[];
private _errors: ParseError[];
constructor(
private _htmlParser: HtmlParser, private _parser: Parser, private _implicitTags: string[],
private _implicitAttrs: {[k: string]: string[]}) {}
extract(template: string, sourceUrl: string): ExtractionResult {
this.messages = [];
this.errors = [];
this._messages = [];
this._errors = [];
let res = this._htmlParser.parse(template, sourceUrl, true);
if (res.errors.length > 0) {
@ -107,27 +107,27 @@ export class MessageExtractor {
} else {
let expanded = expandNodes(res.rootNodes);
this._recurse(expanded.nodes);
return new ExtractionResult(this.messages, this.errors.concat(expanded.errors));
return new ExtractionResult(this._messages, this._errors.concat(expanded.errors));
}
}
private _extractMessagesFromPart(p: Part): void {
if (p.hasI18n) {
this.messages.push(p.createMessage(this._parser));
this._recurseToExtractMessagesFromAttributes(p.children);
private _extractMessagesFromPart(part: Part): void {
if (part.hasI18n) {
this._messages.push(part.createMessage(this._parser));
this._recurseToExtractMessagesFromAttributes(part.children);
} else {
this._recurse(p.children);
this._recurse(part.children);
}
if (isPresent(p.rootElement)) {
this._extractMessagesFromAttributes(p.rootElement);
if (isPresent(part.rootElement)) {
this._extractMessagesFromAttributes(part.rootElement);
}
}
private _recurse(nodes: HtmlAst[]): void {
if (isPresent(nodes)) {
let ps = partition(nodes, this.errors, this._implicitTags);
ps.forEach(p => this._extractMessagesFromPart(p));
let parts = partition(nodes, this._errors, this._implicitTags);
parts.forEach(part => this._extractMessagesFromPart(part));
}
}
@ -145,17 +145,15 @@ export class MessageExtractor {
isPresent(this._implicitAttrs[p.name]) ? this._implicitAttrs[p.name] : [];
let explicitAttrs: string[] = [];
p.attrs.forEach(attr => {
if (attr.name.startsWith(I18N_ATTR_PREFIX)) {
try {
explicitAttrs.push(attr.name.substring(I18N_ATTR_PREFIX.length));
this.messages.push(messageFromI18nAttribute(this._parser, p, attr));
} catch (e) {
if (e instanceof I18nError) {
this.errors.push(e);
} else {
throw e;
}
p.attrs.filter(attr => attr.name.startsWith(I18N_ATTR_PREFIX)).forEach(attr => {
try {
explicitAttrs.push(attr.name.substring(I18N_ATTR_PREFIX.length));
this._messages.push(messageFromI18nAttribute(this._parser, p, attr));
} catch (e) {
if (e instanceof I18nError) {
this._errors.push(e);
} else {
throw e;
}
}
});
@ -163,6 +161,6 @@ export class MessageExtractor {
p.attrs.filter(attr => !attr.name.startsWith(I18N_ATTR_PREFIX))
.filter(attr => explicitAttrs.indexOf(attr.name) == -1)
.filter(attr => transAttrs.indexOf(attr.name) > -1)
.forEach(attr => this.messages.push(messageFromAttribute(this._parser, attr)));
.forEach(attr => this._messages.push(messageFromAttribute(this._parser, attr)));
}
}

View File

@ -2,7 +2,6 @@ import {Parser} from '../expression_parser/parser';
import {StringWrapper, isBlank, isPresent} from '../facade/lang';
import {HtmlAst, HtmlAstVisitor, HtmlAttrAst, HtmlCommentAst, HtmlElementAst, HtmlExpansionAst, HtmlExpansionCaseAst, HtmlTextAst, htmlVisitAll} from '../html_ast';
import {ParseError, ParseSourceSpan} from '../parse_util';
import {Message} from './message';
export const I18N_ATTR = 'i18n';
@ -16,16 +15,14 @@ export class I18nError extends ParseError {
constructor(span: ParseSourceSpan, msg: string) { super(span, msg); }
}
// Man, this is so ugly!
export function partition(nodes: HtmlAst[], errors: ParseError[], implicitTags: string[]): Part[] {
let res: Part[] = [];
let parts: Part[] = [];
for (let i = 0; i < nodes.length; ++i) {
let n = nodes[i];
let temp: HtmlAst[] = [];
if (_isOpeningComment(n)) {
let i18n = (<HtmlCommentAst>n).value.substring(5).trim();
let i18n = (<HtmlCommentAst>n).value.replace(/^i18n:?/, '').trim();
i++;
while (!_isClosingComment(nodes[i])) {
temp.push(nodes[i++]);
@ -34,18 +31,18 @@ export function partition(nodes: HtmlAst[], errors: ParseError[], implicitTags:
break;
}
}
res.push(new Part(null, null, temp, i18n, true));
parts.push(new Part(null, null, temp, i18n, true));
} else if (n instanceof HtmlElementAst) {
let i18n = _findI18nAttr(n);
let hasI18n: boolean = isPresent(i18n) || implicitTags.indexOf(n.name) > -1;
res.push(new Part(n, null, n.children, isPresent(i18n) ? i18n.value : null, hasI18n));
parts.push(new Part(n, null, n.children, isPresent(i18n) ? i18n.value : null, hasI18n));
} else if (n instanceof HtmlTextAst) {
res.push(new Part(null, n, null, null, false));
parts.push(new Part(null, n, null, null, false));
}
}
return res;
return parts;
}
export class Part {
@ -54,12 +51,14 @@ export class Part {
public children: HtmlAst[], public i18n: string, public hasI18n: boolean) {}
get sourceSpan(): ParseSourceSpan {
if (isPresent(this.rootElement))
if (isPresent(this.rootElement)) {
return this.rootElement.sourceSpan;
else if (isPresent(this.rootTextNode))
}
if (isPresent(this.rootTextNode)) {
return this.rootTextNode.sourceSpan;
else
return this.children[0].sourceSpan;
}
return this.children[0].sourceSpan;
}
createMessage(parser: Parser): Message {
@ -69,7 +68,7 @@ export class Part {
}
function _isOpeningComment(n: HtmlAst): boolean {
return n instanceof HtmlCommentAst && isPresent(n.value) && n.value.startsWith('i18n:');
return n instanceof HtmlCommentAst && isPresent(n.value) && n.value.startsWith('i18n');
}
function _isClosingComment(n: HtmlAst): boolean {
@ -77,8 +76,13 @@ function _isClosingComment(n: HtmlAst): boolean {
}
function _findI18nAttr(p: HtmlElementAst): HtmlAttrAst {
let i18n = p.attrs.filter(a => a.name == I18N_ATTR);
return i18n.length == 0 ? null : i18n[0];
let attrs = p.attrs;
for (let i = 0; i < attrs.length; i++) {
if (attrs[i].name === I18N_ATTR) {
return attrs[i];
}
}
return null;
}
export function meaning(i18n: string): string {
@ -88,7 +92,7 @@ export function meaning(i18n: string): string {
export function description(i18n: string): string {
if (isBlank(i18n) || i18n == '') return null;
let parts = i18n.split('|');
let parts = i18n.split('|', 2);
return parts.length > 1 ? parts[1] : null;
}
@ -100,11 +104,10 @@ export function description(i18n: string): string {
export function messageFromI18nAttribute(
parser: Parser, p: HtmlElementAst, i18nAttr: HtmlAttrAst): Message {
let expectedName = i18nAttr.name.substring(5);
let matching = p.attrs.filter(a => a.name == expectedName);
let attr = p.attrs.find(a => a.name == expectedName);
if (matching.length > 0) {
return messageFromAttribute(
parser, matching[0], meaning(i18nAttr.value), description(i18nAttr.value));
if (attr) {
return messageFromAttribute(parser, attr, meaning(i18nAttr.value), description(i18nAttr.value));
}
throw new I18nError(p.sourceSpan, `Missing attribute '${expectedName}'.`);
@ -179,9 +182,8 @@ class _StringifyVisitor implements HtmlAstVisitor {
let noInterpolation = removeInterpolation(ast.value, ast.sourceSpan, this._parser);
if (noInterpolation != ast.value) {
return `<ph name="t${index}">${noInterpolation}</ph>`;
} else {
return ast.value;
}
return ast.value;
}
visitComment(ast: HtmlCommentAst, context: any): any { return ''; }

View File

@ -42,38 +42,34 @@ class _Humanizer implements HtmlAstVisitor {
htmlVisitAll(this, ast.attrs);
htmlVisitAll(this, ast.children);
this.elDepth--;
return null;
}
visitAttr(ast: HtmlAttrAst, context: any): any {
var res = this._appendContext(ast, [HtmlAttrAst, ast.name, ast.value]);
this.result.push(res);
return null;
}
visitText(ast: HtmlTextAst, context: any): any {
var res = this._appendContext(ast, [HtmlTextAst, ast.value, this.elDepth]);
this.result.push(res);
return null;
}
visitComment(ast: HtmlCommentAst, context: any): any {
var res = this._appendContext(ast, [HtmlCommentAst, ast.value, this.elDepth]);
this.result.push(res);
return null;
}
visitExpansion(ast: HtmlExpansionAst, context: any): any {
var res = this._appendContext(ast, [HtmlExpansionAst, ast.switchValue, ast.type]);
var res =
this._appendContext(ast, [HtmlExpansionAst, ast.switchValue, ast.type, this.elDepth++]);
this.result.push(res);
htmlVisitAll(this, ast.cases);
return null;
this.elDepth--;
}
visitExpansionCase(ast: HtmlExpansionCaseAst, context: any): any {
var res = this._appendContext(ast, [HtmlExpansionCaseAst, ast.value]);
var res = this._appendContext(ast, [HtmlExpansionCaseAst, ast.value, this.elDepth]);
this.result.push(res);
return null;
}
private _appendContext(ast: HtmlAst, input: any[]): any[] {

View File

@ -234,9 +234,9 @@ export function main() {
expect(humanizeDom(parsed)).toEqual([
[HtmlElementAst, 'div', 0],
[HtmlTextAst, 'before', 1],
[HtmlExpansionAst, 'messages.length', 'plural'],
[HtmlExpansionCaseAst, '=0'],
[HtmlExpansionCaseAst, '=1'],
[HtmlExpansionAst, 'messages.length', 'plural', 1],
[HtmlExpansionCaseAst, '=0', 2],
[HtmlExpansionCaseAst, '=1', 2],
[HtmlTextAst, 'after', 1],
]);
let cases = (<any>parsed.rootNodes[0]).children[1].cases;
@ -256,15 +256,15 @@ export function main() {
let parsed = parser.parse(
`{messages.length, plural, =0 { {p.gender, gender, =m {m}} }}`, 'TestComp', true);
expect(humanizeDom(parsed)).toEqual([
[HtmlExpansionAst, 'messages.length', 'plural'],
[HtmlExpansionCaseAst, '=0'],
[HtmlExpansionAst, 'messages.length', 'plural', 0],
[HtmlExpansionCaseAst, '=0', 1],
]);
let firstCase = (<any>parsed.rootNodes[0]).cases[0];
expect(humanizeDom(new HtmlParseTreeResult(firstCase.expression, []))).toEqual([
[HtmlExpansionAst, 'p.gender', 'gender'],
[HtmlExpansionCaseAst, '=m'],
[HtmlExpansionAst, 'p.gender', 'gender', 0],
[HtmlExpansionCaseAst, '=m', 1],
[HtmlTextAst, ' ', 0],
]);
});

View File

@ -49,11 +49,14 @@ export function main() {
let res = extractor.extract(
`
<!-- i18n: meaning1|desc1 -->message1<!-- /i18n -->
<!-- i18n: meaning2|desc2 -->message2<!-- /i18n -->`,
<!-- i18n: meaning2 -->message2<!-- /i18n -->
<!-- i18n -->message3<!-- /i18n -->`,
'someUrl');
expect(res.messages).toEqual([
new Message('message1', 'meaning1', 'desc1'), new Message('message2', 'meaning2', 'desc2')
new Message('message1', 'meaning1', 'desc1'),
new Message('message2', 'meaning2'),
new Message('message3', null),
]);
});
@ -212,8 +215,7 @@ export function main() {
`
<div
title1='message1' i18n-title1='meaning1|desc1' i18n-title2='meaning2|desc2'>
</div>
`,
</div>`,
'someurl');
expect(res.errors.length).toEqual(1);