Skip to content

Commit fe01e2e

Browse files
vicbvicb
authored
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
1 parent 7afee97 commit fe01e2e

8 files changed

Lines changed: 112 additions & 104 deletions

File tree

‎modules/@angular/compiler-cli/src/extract_i18n.ts‎

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,33 +19,34 @@ import {CompileMetadataResolver, HtmlParser, DirectiveNormalizer, Lexer, Parser,
1919
import {ReflectorHost} from './reflector_host';
2020
import {StaticAndDynamicReflectionCapabilities} from './static_reflection_capabilities';
2121

22-
2322
function extract(
2423
ngOptions: tsc.AngularCompilerOptions, program: ts.Program, host: ts.CompilerHost) {
2524
return Extractor.create(ngOptions, program, host).extract();
2625
}
2726

28-
const GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/;
27+
const _dirPaths = new Map<compiler.CompileDirectiveMetadata, string>();
28+
29+
const _GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/;
2930

3031
class Extractor {
3132
constructor(
32-
private options: tsc.AngularCompilerOptions, private program: ts.Program,
33+
private _options: tsc.AngularCompilerOptions, private _program: ts.Program,
3334
public host: ts.CompilerHost, private staticReflector: StaticReflector,
34-
private resolver: CompileMetadataResolver, private compiler: compiler.OfflineCompiler,
35-
private reflectorHost: ReflectorHost, private _extractor: MessageExtractor) {}
35+
private _resolver: CompileMetadataResolver, private _compiler: compiler.OfflineCompiler,
36+
private _reflectorHost: ReflectorHost, private _extractor: MessageExtractor) {}
3637

37-
private extractCmpMessages(metadatas: compiler.CompileDirectiveMetadata[]):
38+
private _extractCmpMessages(metadatas: compiler.CompileDirectiveMetadata[]):
3839
Promise<ExtractionResult> {
3940
if (!metadatas || !metadatas.length) {
4041
return null;
4142
}
4243

4344
const normalize = (metadata: compiler.CompileDirectiveMetadata) => {
4445
const directiveType = metadata.type.runtime;
45-
const directives = this.resolver.getViewDirectivesMetadata(directiveType);
46-
return Promise.all(directives.map(d => this.compiler.normalizeDirectiveMetadata(d)))
46+
const directives = this._resolver.getViewDirectivesMetadata(directiveType);
47+
return Promise.all(directives.map(d => this._compiler.normalizeDirectiveMetadata(d)))
4748
.then(normalizedDirectives => {
48-
const pipes = this.resolver.getViewPipesMetadata(directiveType);
49+
const pipes = this._resolver.getViewPipesMetadata(directiveType);
4950
return new compiler.NormalizedComponentWithViewDirectives(
5051
metadata, normalizedDirectives, pipes);
5152
});
@@ -56,8 +57,8 @@ class Extractor {
5657
let messages: Message[] = [];
5758
let errors: ParseError[] = [];
5859
cmps.forEach(cmp => {
59-
// TODO(vicb): url
60-
let result = this._extractor.extract(cmp.component.template.template, 'url');
60+
let url = _dirPaths.get(cmp.component);
61+
let result = this._extractor.extract(cmp.component.template.template, url);
6162
errors = errors.concat(result.errors);
6263
messages = messages.concat(result.messages);
6364
});
@@ -67,7 +68,7 @@ class Extractor {
6768
});
6869
}
6970

70-
private readComponents(absSourcePath: string) {
71+
private _readComponents(absSourcePath: string): Promise<compiler.CompileDirectiveMetadata>[] {
7172
const result: Promise<compiler.CompileDirectiveMetadata>[] = [];
7273
const metadata = this.staticReflector.getModuleMetadata(absSourcePath);
7374
if (!metadata) {
@@ -80,26 +81,29 @@ class Extractor {
8081
return result;
8182
}
8283
for (const symbol of symbols) {
83-
const staticType = this.reflectorHost.findDeclaration(absSourcePath, symbol, absSourcePath);
84+
const staticType = this._reflectorHost.findDeclaration(absSourcePath, symbol, absSourcePath);
8485
let directive: compiler.CompileDirectiveMetadata;
85-
directive = this.resolver.maybeGetDirectiveMetadata(<any>staticType);
86+
directive = this._resolver.maybeGetDirectiveMetadata(<any>staticType);
8687

87-
if (!directive || !directive.isComponent) {
88-
continue;
88+
if (directive && directive.isComponent) {
89+
let promise = this._compiler.normalizeDirectiveMetadata(directive);
90+
promise.then(md => _dirPaths.set(md, absSourcePath));
91+
result.push(promise);
8992
}
90-
result.push(this.compiler.normalizeDirectiveMetadata(directive));
9193
}
9294
return result;
9395
}
9496

9597
extract(): Promise<any> {
96-
const promises = this.program.getSourceFiles()
98+
_dirPaths.clear();
99+
100+
const promises = this._program.getSourceFiles()
97101
.map(sf => sf.fileName)
98-
.filter(f => !GENERATED_FILES.test(f))
102+
.filter(f => !_GENERATED_FILES.test(f))
99103
.map(
100104
(absSourcePath: string): Promise<any> =>
101-
Promise.all(this.readComponents(absSourcePath))
102-
.then(metadatas => this.extractCmpMessages(metadatas))
105+
Promise.all(this._readComponents(absSourcePath))
106+
.then(metadatas => this._extractCmpMessages(metadatas))
103107
.catch(e => console.error(e.stack)));
104108

105109
let messages: Message[] = [];
@@ -112,12 +116,12 @@ class Extractor {
112116
});
113117

114118
if (errors.length) {
115-
throw errors;
119+
throw new Error(errors.map(e => e.toString()).join('\n'));
116120
}
117121

118122
messages = removeDuplicates(messages);
119123

120-
let genPath = path.join(this.options.genDir, 'messages.xmb');
124+
let genPath = path.join(this._options.genDir, 'messages.xmb');
121125
let msgBundle = serializeXmb(messages);
122126

123127
this.host.writeFile(genPath, msgBundle, false);

‎modules/@angular/compiler/src/i18n/expander.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ const PLURAL_CASES: string[] = ['zero', 'one', 'two', 'few', 'many', 'other'];
2323
*
2424
* ```
2525
* <ul [ngPlural]="messages.length">
26-
* <template [ngPluralCase]="'=0'"><li i18n="plural_=0">zero</li></template>
27-
* <template [ngPluralCase]="'=1'"><li i18n="plural_=1">one</li></template>
28-
* <template [ngPluralCase]="'other'"><li i18n="plural_other">more than one</li></template>
26+
* <template ngPluralCase="=0"><li i18n="plural_=0">zero</li></template>
27+
* <template ngPluralCase="=1"><li i18n="plural_=1">one</li></template>
28+
* <template ngPluralCase="other"><li i18n="plural_other">more than one</li></template>
2929
* </ul>
3030
* ```
3131
*/
@@ -39,6 +39,11 @@ export class ExpansionResult {
3939
constructor(public nodes: HtmlAst[], public expanded: boolean, public errors: ParseError[]) {}
4040
}
4141

42+
/**
43+
* Expand expansion forms (plural, select) to directives
44+
*
45+
* @internal
46+
*/
4247
class _Expander implements HtmlAstVisitor {
4348
expanded: boolean = false;
4449
errors: ParseError[] = [];
@@ -73,7 +78,7 @@ function _expandPluralForm(ast: HtmlExpansionAst, errors: ParseError[]): HtmlEle
7378
`Plural cases should be "=<number>" or one of ${PLURAL_CASES.join(", ")}`));
7479
}
7580
let expansionResult = expandNodes(c.expression);
76-
expansionResult.errors.forEach(e => errors.push(e));
81+
errors.push(...expansionResult.errors);
7782
let i18nAttrs = expansionResult.expanded ?
7883
[] :
7984
[new HtmlAttrAst('i18n', `${ast.type}_${c.value}`, c.valueSourceSpan)];

‎modules/@angular/compiler/src/i18n/i18n_html_parser.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,16 +113,16 @@ export class I18nHtmlParser implements HtmlParser {
113113
} else {
114114
let expanded = expandNodes(res.rootNodes);
115115
let nodes = this._recurse(expanded.nodes);
116-
this.errors = this.errors.concat(expanded.errors);
116+
this.errors.push(...expanded.errors);
117117

118118
return this.errors.length > 0 ? new HtmlParseTreeResult([], this.errors) :
119119
new HtmlParseTreeResult(nodes, []);
120120
}
121121
}
122122

123-
private _processI18nPart(p: Part): HtmlAst[] {
123+
private _processI18nPart(part: Part): HtmlAst[] {
124124
try {
125-
return p.hasI18n ? this._mergeI18Part(p) : this._recurseIntoI18nPart(p);
125+
return part.hasI18n ? this._mergeI18Part(part) : this._recurseIntoI18nPart(part);
126126
} catch (e) {
127127
if (e instanceof I18nError) {
128128
this.errors.push(e);
@@ -133,16 +133,17 @@ export class I18nHtmlParser implements HtmlParser {
133133
}
134134
}
135135

136-
private _mergeI18Part(p: Part): HtmlAst[] {
137-
let message = p.createMessage(this._parser);
136+
private _mergeI18Part(part: Part): HtmlAst[] {
137+
let message = part.createMessage(this._parser);
138138
let messageId = id(message);
139139
if (!StringMapWrapper.contains(this._messages, messageId)) {
140140
throw new I18nError(
141-
p.sourceSpan, `Cannot find message for id '${messageId}', content '${message.content}'.`);
141+
part.sourceSpan,
142+
`Cannot find message for id '${messageId}', content '${message.content}'.`);
142143
}
143144

144145
let parsedMessage = this._messages[messageId];
145-
return this._mergeTrees(p, parsedMessage, p.children);
146+
return this._mergeTrees(part, parsedMessage, part.children);
146147
}
147148

148149
private _recurseIntoI18nPart(p: Part): HtmlAst[] {
@@ -166,8 +167,8 @@ export class I18nHtmlParser implements HtmlParser {
166167
}
167168

168169
private _recurse(nodes: HtmlAst[]): HtmlAst[] {
169-
let ps = partition(nodes, this.errors, this._implicitTags);
170-
return ListWrapper.flatten(ps.map(p => this._processI18nPart(p)));
170+
let parts = partition(nodes, this.errors, this._implicitTags);
171+
return ListWrapper.flatten(parts.map(p => this._processI18nPart(p)));
171172
}
172173

173174
private _mergeTrees(p: Part, translated: HtmlAst[], original: HtmlAst[]): HtmlAst[] {

‎modules/@angular/compiler/src/i18n/message_extractor.ts‎

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -90,44 +90,44 @@ export function removeDuplicates(messages: Message[]): Message[] {
9090
* 4. If a part has the i18n attribute, stringify the nodes to create a Message.
9191
*/
9292
export class MessageExtractor {
93-
messages: Message[];
94-
errors: ParseError[];
93+
private _messages: Message[];
94+
private _errors: ParseError[];
9595

9696
constructor(
9797
private _htmlParser: HtmlParser, private _parser: Parser, private _implicitTags: string[],
9898
private _implicitAttrs: {[k: string]: string[]}) {}
9999

100100
extract(template: string, sourceUrl: string): ExtractionResult {
101-
this.messages = [];
102-
this.errors = [];
101+
this._messages = [];
102+
this._errors = [];
103103

104104
let res = this._htmlParser.parse(template, sourceUrl, true);
105105
if (res.errors.length > 0) {
106106
return new ExtractionResult([], res.errors);
107107
} else {
108108
let expanded = expandNodes(res.rootNodes);
109109
this._recurse(expanded.nodes);
110-
return new ExtractionResult(this.messages, this.errors.concat(expanded.errors));
110+
return new ExtractionResult(this._messages, this._errors.concat(expanded.errors));
111111
}
112112
}
113113

114-
private _extractMessagesFromPart(p: Part): void {
115-
if (p.hasI18n) {
116-
this.messages.push(p.createMessage(this._parser));
117-
this._recurseToExtractMessagesFromAttributes(p.children);
114+
private _extractMessagesFromPart(part: Part): void {
115+
if (part.hasI18n) {
116+
this._messages.push(part.createMessage(this._parser));
117+
this._recurseToExtractMessagesFromAttributes(part.children);
118118
} else {
119-
this._recurse(p.children);
119+
this._recurse(part.children);
120120
}
121121

122-
if (isPresent(p.rootElement)) {
123-
this._extractMessagesFromAttributes(p.rootElement);
122+
if (isPresent(part.rootElement)) {
123+
this._extractMessagesFromAttributes(part.rootElement);
124124
}
125125
}
126126

127127
private _recurse(nodes: HtmlAst[]): void {
128128
if (isPresent(nodes)) {
129-
let ps = partition(nodes, this.errors, this._implicitTags);
130-
ps.forEach(p => this._extractMessagesFromPart(p));
129+
let parts = partition(nodes, this._errors, this._implicitTags);
130+
parts.forEach(part => this._extractMessagesFromPart(part));
131131
}
132132
}
133133

@@ -145,24 +145,22 @@ export class MessageExtractor {
145145
isPresent(this._implicitAttrs[p.name]) ? this._implicitAttrs[p.name] : [];
146146
let explicitAttrs: string[] = [];
147147

148-
p.attrs.forEach(attr => {
149-
if (attr.name.startsWith(I18N_ATTR_PREFIX)) {
150-
try {
151-
explicitAttrs.push(attr.name.substring(I18N_ATTR_PREFIX.length));
152-
this.messages.push(messageFromI18nAttribute(this._parser, p, attr));
153-
} catch (e) {
154-
if (e instanceof I18nError) {
155-
this.errors.push(e);
156-
} else {
157-
throw e;
158-
}
148+
p.attrs.filter(attr => attr.name.startsWith(I18N_ATTR_PREFIX)).forEach(attr => {
149+
try {
150+
explicitAttrs.push(attr.name.substring(I18N_ATTR_PREFIX.length));
151+
this._messages.push(messageFromI18nAttribute(this._parser, p, attr));
152+
} catch (e) {
153+
if (e instanceof I18nError) {
154+
this._errors.push(e);
155+
} else {
156+
throw e;
159157
}
160158
}
161159
});
162160

163161
p.attrs.filter(attr => !attr.name.startsWith(I18N_ATTR_PREFIX))
164162
.filter(attr => explicitAttrs.indexOf(attr.name) == -1)
165163
.filter(attr => transAttrs.indexOf(attr.name) > -1)
166-
.forEach(attr => this.messages.push(messageFromAttribute(this._parser, attr)));
164+
.forEach(attr => this._messages.push(messageFromAttribute(this._parser, attr)));
167165
}
168166
}

0 commit comments

Comments
 (0)