diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index a5971b595b8f..4864dfaec28a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; +import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstIcu, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; import * as ts from 'typescript'; import {Reference} from '../../imports'; @@ -1333,6 +1333,8 @@ class Scope { this.checkAndAppendReferencesOfNode(node); } else if (node instanceof TmplAstBoundText) { this.opQueue.push(new TcbTextInterpolationOp(this.tcb, this, node)); + } else if (node instanceof TmplAstIcu) { + this.appendIcuExpressions(node); } } @@ -1459,6 +1461,17 @@ class Scope { this.appendDeepSchemaChecks(node.children); } } + + private appendIcuExpressions(node: TmplAstIcu): void { + for (const variable of Object.values(node.vars)) { + this.opQueue.push(new TcbTextInterpolationOp(this.tcb, this, variable)); + } + for (const placeholder of Object.values(node.placeholders)) { + if (placeholder instanceof TmplAstBoundText) { + this.opQueue.push(new TcbTextInterpolationOp(this.tcb, this, placeholder)); + } + } + } } interface TcbBoundInput { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index 5838f2fc8e1a..0104b024a285 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -176,6 +176,21 @@ runInEachFileSystem(() => { ]); }); + it('checks expressions in ICUs', () => { + const messages = diagnose( + `{switch, plural, other { {{interpolation}} + {nestedSwitch, plural, other { {{nestedInterpolation}} }} + }}`, + `class TestComponent {}`); + + expect(messages.sort()).toEqual([ + `TestComponent.html(1, 13): Property 'switch' does not exist on type 'TestComponent'.`, + `TestComponent.html(1, 39): Property 'interpolation' does not exist on type 'TestComponent'.`, + `TestComponent.html(2, 14): Property 'nestedSwitch' does not exist on type 'TestComponent'.`, + `TestComponent.html(2, 46): Property 'nestedInterpolation' does not exist on type 'TestComponent'.`, + ]); + }); + it('produces diagnostics for pipes', () => { const messages = diagnose( `
{{ person.name | pipe:person.age:1 }}
`, ` diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index c734932b8856..e60ef28312c0 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -92,7 +92,7 @@ export {getParseErrors, isSyntaxError, syntaxError, Version} from './util'; export {SourceMap} from './output/source_map'; export * from './injectable_compiler_2'; export * from './render3/view/api'; -export {BoundAttribute as TmplAstBoundAttribute, BoundEvent as TmplAstBoundEvent, BoundText as TmplAstBoundText, Content as TmplAstContent, Element as TmplAstElement, Node as TmplAstNode, RecursiveVisitor as TmplAstRecursiveVisitor, Reference as TmplAstReference, Template as TmplAstTemplate, Text as TmplAstText, TextAttribute as TmplAstTextAttribute, Variable as TmplAstVariable,} from './render3/r3_ast'; +export {BoundAttribute as TmplAstBoundAttribute, BoundEvent as TmplAstBoundEvent, BoundText as TmplAstBoundText, Content as TmplAstContent, Element as TmplAstElement, Icu as TmplAstIcu, Node as TmplAstNode, RecursiveVisitor as TmplAstRecursiveVisitor, Reference as TmplAstReference, Template as TmplAstTemplate, Text as TmplAstText, TextAttribute as TmplAstTextAttribute, Variable as TmplAstVariable} from './render3/r3_ast'; export * from './render3/view/t2_api'; export * from './render3/view/t2_binder'; export {Identifiers as R3Identifiers} from './render3/r3_identifiers'; diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index 4b5b28b8b5ec..707f4f02a257 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -179,10 +179,33 @@ export class Parser { expressions.push(ast); } - const span = new ParseSpan(0, input == null ? 0 : input.length); - return new ASTWithSource( - new Interpolation(span, span.toAbsolute(absoluteOffset), split.strings, expressions), input, - location, absoluteOffset, this.errors); + return this.createInterpolationAst(split.strings, expressions, input, location, absoluteOffset); + } + + /** + * Similar to `parseInterpolation`, but treats the provided string as a single expression + * element that would normally appear within the interpolation prefix and suffix (`{{` and `}}`). + * This is used for parsing the switch expression in ICUs. + */ + parseInterpolationExpression(expression: string, location: any, absoluteOffset: number): + ASTWithSource { + const sourceToLex = this._stripComments(expression); + const tokens = this._lexer.tokenize(sourceToLex); + const ast = new _ParseAST( + expression, location, absoluteOffset, tokens, sourceToLex.length, + /* parseAction */ false, this.errors, 0) + .parseChain(); + const strings = ['', '']; // The prefix and suffix strings are both empty + return this.createInterpolationAst(strings, [ast], expression, location, absoluteOffset); + } + + private createInterpolationAst( + strings: string[], expressions: AST[], input: string, location: string, + absoluteOffset: number): ASTWithSource { + const span = new ParseSpan(0, input.length); + const interpolation = + new Interpolation(span, span.toAbsolute(absoluteOffset), strings, expressions); + return new ASTWithSource(interpolation, input, location, absoluteOffset, this.errors); } /** diff --git a/packages/compiler/src/i18n/i18n_ast.ts b/packages/compiler/src/i18n/i18n_ast.ts index 9bd6259bd30c..f202ba9e067b 100644 --- a/packages/compiler/src/i18n/i18n_ast.ts +++ b/packages/compiler/src/i18n/i18n_ast.ts @@ -8,6 +8,18 @@ import {ParseSourceSpan} from '../parse_util'; +/** + * Describes the text contents of a placeholder as it appears in an ICU expression, including its + * source span information. + */ +export interface MessagePlaceholder { + /** The text contents of the placeholder */ + text: string; + + /** The source span of the placeholder */ + sourceSpan: ParseSourceSpan; +} + export class Message { sources: MessageSpan[]; id: string = this.customId; @@ -16,14 +28,14 @@ export class Message { /** * @param nodes message AST - * @param placeholders maps placeholder names to static content + * @param placeholders maps placeholder names to static content and their source spans * @param placeholderToMessage maps placeholder names to messages (used for nested ICU messages) * @param meaning * @param description * @param customId */ constructor( - public nodes: Node[], public placeholders: {[phName: string]: string}, + public nodes: Node[], public placeholders: {[phName: string]: MessagePlaceholder}, public placeholderToMessage: {[phName: string]: Message}, public meaning: string, public description: string, public customId: string) { if (nodes.length) { diff --git a/packages/compiler/src/i18n/i18n_parser.ts b/packages/compiler/src/i18n/i18n_parser.ts index a7d22a1f8f85..4fa1f55f4b5e 100644 --- a/packages/compiler/src/i18n/i18n_parser.ts +++ b/packages/compiler/src/i18n/i18n_parser.ts @@ -39,7 +39,7 @@ interface I18nMessageVisitorContext { isIcu: boolean; icuDepth: number; placeholderRegistry: PlaceholderRegistry; - placeholderToContent: {[phName: string]: string}; + placeholderToContent: {[phName: string]: i18n.MessagePlaceholder}; placeholderToMessage: {[phName: string]: i18n.Message}; visitNodeFn: VisitNodeFn; } @@ -83,13 +83,19 @@ class _I18nVisitor implements html.Visitor { const isVoid: boolean = getHtmlTagDefinition(el.name).isVoid; const startPhName = context.placeholderRegistry.getStartTagPlaceholderName(el.name, attrs, isVoid); - context.placeholderToContent[startPhName] = el.startSourceSpan.toString(); + context.placeholderToContent[startPhName] = { + text: el.startSourceSpan.toString(), + sourceSpan: el.startSourceSpan, + }; let closePhName = ''; if (!isVoid) { closePhName = context.placeholderRegistry.getCloseTagPlaceholderName(el.name); - context.placeholderToContent[closePhName] = ``; + context.placeholderToContent[closePhName] = { + text: ``, + sourceSpan: el.endSourceSpan ?? el.sourceSpan, + }; } const node = new i18n.TagPlaceholder( @@ -128,7 +134,10 @@ class _I18nVisitor implements html.Visitor { // - the ICU message is nested. const expPh = context.placeholderRegistry.getUniquePlaceholder(`VAR_${icu.type}`); i18nIcu.expressionPlaceholder = expPh; - context.placeholderToContent[expPh] = icu.switchValue; + context.placeholderToContent[expPh] = { + text: icu.switchValue, + sourceSpan: icu.switchValueSourceSpan, + }; return context.visitNodeFn(icu, i18nIcu); } @@ -175,7 +184,10 @@ class _I18nVisitor implements html.Visitor { const expressionSpan = getOffsetSourceSpan(sourceSpan, splitInterpolation.expressionsSpans[i]); nodes.push(new i18n.Placeholder(expression, phName, expressionSpan)); - context.placeholderToContent[phName] = sDelimiter + expression + eDelimiter; + context.placeholderToContent[phName] = { + text: sDelimiter + expression + eDelimiter, + sourceSpan: expressionSpan, + }; } // The last index contains no expression diff --git a/packages/compiler/src/i18n/translation_bundle.ts b/packages/compiler/src/i18n/translation_bundle.ts index 5fa66c6ae466..79e899e2e095 100644 --- a/packages/compiler/src/i18n/translation_bundle.ts +++ b/packages/compiler/src/i18n/translation_bundle.ts @@ -109,7 +109,7 @@ class I18nToHtmlVisitor implements i18n.Visitor { // TODO(vicb): Once all format switch to using expression placeholders // we should throw when the placeholder is not in the source message const exp = this._srcMsg.placeholders.hasOwnProperty(icu.expression) ? - this._srcMsg.placeholders[icu.expression] : + this._srcMsg.placeholders[icu.expression].text : icu.expression; return `{${exp}, ${icu.type}, ${cases.join(' ')}}`; @@ -118,7 +118,7 @@ class I18nToHtmlVisitor implements i18n.Visitor { visitPlaceholder(ph: i18n.Placeholder, context?: any): string { const phName = this._mapper(ph.name); if (this._srcMsg.placeholders.hasOwnProperty(phName)) { - return this._srcMsg.placeholders[phName]; + return this._srcMsg.placeholders[phName].text; } if (this._srcMsg.placeholderToMessage.hasOwnProperty(phName)) { diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts index 92d652d16d7b..fe29a9bc40c3 100644 --- a/packages/compiler/src/render3/r3_template_transform.ts +++ b/packages/compiler/src/render3/r3_template_transform.ts @@ -266,12 +266,6 @@ class HtmlAstToIvyAst implements html.Visitor { Object.keys(message.placeholders).forEach(key => { const value = message.placeholders[key]; if (key.startsWith(I18N_ICU_VAR_PREFIX)) { - const config = this.bindingParser.interpolationConfig; - - // ICU expression is a plain string, not wrapped into start - // and end tags, so we wrap it before passing to binding parser - const wrapped = `${config.start}${value}${config.end}`; - // Currently when the `plural` or `select` keywords in an ICU contain trailing spaces (e.g. // `{count, select , ...}`), these spaces are also included into the key names in ICU vars // (e.g. "VAR_SELECT "). These trailing spaces are not desirable, since they will later be @@ -279,10 +273,11 @@ class HtmlAstToIvyAst implements html.Visitor { // mismatches at runtime (i.e. placeholder will not be replaced with the correct value). const formattedKey = key.trim(); - vars[formattedKey] = - this._visitTextWithInterpolation(wrapped, expansion.sourceSpan) as t.BoundText; + const ast = this.bindingParser.parseInterpolationExpression(value.text, value.sourceSpan); + + vars[formattedKey] = new t.BoundText(ast, value.sourceSpan); } else { - placeholders[key] = this._visitTextWithInterpolation(value, expansion.sourceSpan); + placeholders[key] = this._visitTextWithInterpolation(value.text, value.sourceSpan); } }); return new t.Icu(vars, placeholders, expansion.sourceSpan, message); diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index bc4ccf9cd1f7..81e0b69b1d03 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -127,6 +127,26 @@ export class BindingParser { } } + /** + * Similar to `parseInterpolation`, but treats the provided string as a single expression + * element that would normally appear within the interpolation prefix and suffix (`{{` and `}}`). + * This is used for parsing the switch expression in ICUs. + */ + parseInterpolationExpression(expression: string, sourceSpan: ParseSourceSpan): ASTWithSource { + const sourceInfo = sourceSpan.start.toString(); + + try { + const ast = this._exprParser.parseInterpolationExpression( + expression, sourceInfo, sourceSpan.start.offset); + if (ast) this._reportExpressionParserErrors(ast.errors, sourceSpan); + this._checkPipes(ast, sourceSpan); + return ast; + } catch (e) { + this._reportError(`${e}`, sourceSpan); + return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo, sourceSpan.start.offset); + } + } + /** * Parses the bindings in a microsyntax expression, and converts them to * `ParsedProperty` or `ParsedVariable`. diff --git a/packages/compiler/test/i18n/i18n_parser_spec.ts b/packages/compiler/test/i18n/i18n_parser_spec.ts index dc84283cb389..96945dfb4a55 100644 --- a/packages/compiler/test/i18n/i18n_parser_spec.ts +++ b/packages/compiler/test/i18n/i18n_parser_spec.ts @@ -324,7 +324,7 @@ function _humanizePlaceholders( // clang-format off // https://github.com/angular/clang-format/issues/35 return _extractMessages(html, implicitTags, implicitAttrs).map( - msg => Object.keys(msg.placeholders).map((name) => `${name}=${msg.placeholders[name]}`).join(', ')); + msg => Object.keys(msg.placeholders).map((name) => `${name}=${msg.placeholders[name].text}`).join(', ')); // clang-format on } diff --git a/packages/compiler/test/i18n/translation_bundle_spec.ts b/packages/compiler/test/i18n/translation_bundle_spec.ts index 01f322adca99..b7aa29059971 100644 --- a/packages/compiler/test/i18n/translation_bundle_spec.ts +++ b/packages/compiler/test/i18n/translation_bundle_spec.ts @@ -50,7 +50,7 @@ import {_extractMessages} from './i18n_parser_spec'; ] }; const phMap = { - ph1: '*phContent*', + ph1: createPlaceholder('*phContent*'), }; const tb = new TranslationBundle(msgMap, null, (_) => 'foo'); const msg = new i18n.Message([srcNode], phMap, {}, 'm', 'd', 'i'); @@ -160,7 +160,7 @@ import {_extractMessages} from './i18n_parser_spec'; ] }; const phMap = { - ph1: '', + ph1: createPlaceholder(''), }; const tb = new TranslationBundle(msgMap, null, (_) => 'foo'); const msg = new i18n.Message([srcNode], phMap, {}, 'm', 'd', 'i'); @@ -169,3 +169,13 @@ import {_extractMessages} from './i18n_parser_spec'; }); }); } + +function createPlaceholder(text: string): i18n.MessagePlaceholder { + const file = new ParseSourceFile(text, 'file://test'); + const start = new ParseLocation(file, 0, 0, 0); + const end = new ParseLocation(file, text.length, 0, text.length); + return { + text, + sourceSpan: new ParseSourceSpan(start, end), + }; +} diff --git a/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts index 4f3425d07c4c..bb9082b24545 100644 --- a/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts +++ b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts @@ -339,4 +339,25 @@ describe('expression AST absolute source spans', () => { [['As', new AbsoluteSourceSpan(22, 24)], ['Bs', new AbsoluteSourceSpan(35, 37)]])); }); }); + + describe('ICU expressions', () => { + it('is correct for variables and placeholders', () => { + const spans = humanizeExpressionSource( + parse('{item.var, plural, other { {{item.placeholder}} items } }') + .nodes); + expect(spans).toContain(['item.var', new AbsoluteSourceSpan(12, 20)]); + expect(spans).toContain(['item.placeholder', new AbsoluteSourceSpan(40, 56)]); + }); + + it('is correct for variables and placeholders', () => { + const spans = humanizeExpressionSource( + parse( + '{item.var, plural, other { {{item.placeholder}} {nestedVar, plural, other { {{nestedPlaceholder}} }}} }') + .nodes); + expect(spans).toContain(['item.var', new AbsoluteSourceSpan(12, 20)]); + expect(spans).toContain(['item.placeholder', new AbsoluteSourceSpan(40, 56)]); + expect(spans).toContain(['nestedVar', new AbsoluteSourceSpan(60, 69)]); + expect(spans).toContain(['nestedPlaceholder', new AbsoluteSourceSpan(89, 106)]); + }); + }); }); diff --git a/packages/compiler/test/render3/r3_ast_spans_spec.ts b/packages/compiler/test/render3/r3_ast_spans_spec.ts index 1f82c8a78bf3..7cbfc704ad7e 100644 --- a/packages/compiler/test/render3/r3_ast_spans_spec.ts +++ b/packages/compiler/test/render3/r3_ast_spans_spec.ts @@ -89,7 +89,13 @@ class R3AstSourceSpans implements t.Visitor { } visitIcu(icu: t.Icu) { - return null; + this.result.push(['Icu', humanizeSpan(icu.sourceSpan)]); + for (const key of Object.keys(icu.vars)) { + this.result.push(['Icu:Var', humanizeSpan(icu.vars[key].sourceSpan)]); + } + for (const key of Object.keys(icu.placeholders)) { + this.result.push(['Icu:Placeholder', humanizeSpan(icu.placeholders[key].sourceSpan)]); + } } private visitAll(nodes: t.Node[][]) { @@ -420,4 +426,40 @@ describe('R3 AST source spans', () => { ]); }); }); + + describe('ICU expressions', () => { + it('is correct for variables and placeholders', () => { + expectFromHtml('{item.var, plural, other { {{item.placeholder}} items } }') + .toEqual([ + [ + 'Element', + '{item.var, plural, other { {{item.placeholder}} items } }', + '', '' + ], + ['Icu', '{item.var, plural, other { {{item.placeholder}} items } }'], + ['Icu:Var', 'item.var'], + ['Icu:Placeholder', '{{item.placeholder}}'], + ]); + }); + + it('is correct for nested ICUs', () => { + expectFromHtml( + '{item.var, plural, other { {{item.placeholder}} {nestedVar, plural, other { {{nestedPlaceholder}} }}} }') + .toEqual([ + [ + 'Element', + '{item.var, plural, other { {{item.placeholder}} {nestedVar, plural, other { {{nestedPlaceholder}} }}} }', + '', '' + ], + [ + 'Icu', + '{item.var, plural, other { {{item.placeholder}} {nestedVar, plural, other { {{nestedPlaceholder}} }}} }' + ], + ['Icu:Var', 'nestedVar'], + ['Icu:Var', 'item.var'], + ['Icu:Placeholder', '{{item.placeholder}}'], + ['Icu:Placeholder', '{{nestedPlaceholder}}'], + ]); + }); + }); }); diff --git a/packages/compiler/test/render3/util/expression.ts b/packages/compiler/test/render3/util/expression.ts index 183151315820..a0af48ba91e0 100644 --- a/packages/compiler/test/render3/util/expression.ts +++ b/packages/compiler/test/render3/util/expression.ts @@ -141,7 +141,14 @@ class ExpressionSourceHumanizer extends e.RecursiveAstVisitor implements t.Visit } visitContent(ast: t.Content) {} visitText(ast: t.Text) {} - visitIcu(ast: t.Icu) {} + visitIcu(ast: t.Icu) { + for (const key of Object.keys(ast.vars)) { + ast.vars[key].visit(this); + } + for (const key of Object.keys(ast.placeholders)) { + ast.placeholders[key].visit(this); + } + } } /** diff --git a/packages/language-service/ivy/test/hybrid_visitor_spec.ts b/packages/language-service/ivy/test/hybrid_visitor_spec.ts index 556ac0a601ef..04d357823863 100644 --- a/packages/language-service/ivy/test/hybrid_visitor_spec.ts +++ b/packages/language-service/ivy/test/hybrid_visitor_spec.ts @@ -324,6 +324,45 @@ describe('findNodeAtPosition for template AST', () => { expect(node).toBeInstanceOf(e.PropertyRead); expect((node as e.PropertyRead).name).toBe('bar'); }); + + it('should locate switch value in ICUs', () => { + const {errors, nodes, position} = parse(`{sw¦itch, plural, other {text}}">`); + expect(errors).toBe(null); + const node = findNodeAtPosition(nodes, position); + expect(isExpressionNode(node!)).toBe(true); + expect(node).toBeInstanceOf(e.PropertyRead); + expect((node as e.PropertyRead).name).toBe('switch'); + }); + + it('should locate switch value in nested ICUs', () => { + const {errors, nodes, position} = parse( + `{expr, plural, other { {ne¦sted, plural, =1 { {{nestedInterpolation}} }} }}">`); + expect(errors).toBe(null); + const node = findNodeAtPosition(nodes, position); + expect(isExpressionNode(node!)).toBe(true); + expect(node).toBeInstanceOf(e.PropertyRead); + expect((node as e.PropertyRead).name).toBe('nested'); + }); + + it('should locate interpolation expressions in ICUs', () => { + const {errors, nodes, position} = + parse(`{expr, plural, other { {{ i¦nterpolation }} }}">`); + expect(errors).toBe(null); + const node = findNodeAtPosition(nodes, position); + expect(isExpressionNode(node!)).toBe(true); + expect(node).toBeInstanceOf(e.PropertyRead); + expect((node as e.PropertyRead).name).toBe('interpolation'); + }); + + it('should locate interpolation expressions in nested ICUs', () => { + const {errors, nodes, position} = parse( + `{expr, plural, other { {nested, plural, =1 { {{n¦estedInterpolation}} }} }}">`); + expect(errors).toBe(null); + const node = findNodeAtPosition(nodes, position); + expect(isExpressionNode(node!)).toBe(true); + expect(node).toBeInstanceOf(e.PropertyRead); + expect((node as e.PropertyRead).name).toBe('nestedInterpolation'); + }); }); describe('findNodeAtPosition for expression AST', () => {