From a68f1a78a7929c65a9f3b5913c954db2b987f602 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 26 Aug 2020 12:21:29 +0100 Subject: [PATCH] refactor(compiler): element.startSourceSpan is required (#38581) Previously, the `startSourceSpan` property could be null but in reality it is always well defined - except for a legacy case in the old i18n extraction/merging code, where the typings for source-spans are already being undermined. Making this property non-null, simplifies code elsewhere in the project. PR Close #38581 --- .../compiler/src/i18n/extractor_merger.ts | 4 +-- packages/compiler/src/i18n/i18n_parser.ts | 6 ++-- .../compiler/src/i18n/serializers/xliff.ts | 12 +++---- .../compiler/src/i18n/serializers/xliff2.ts | 4 +-- packages/compiler/src/i18n/serializers/xtb.ts | 12 +++---- packages/compiler/src/ml_parser/ast.ts | 2 +- packages/compiler/src/render3/r3_ast.ts | 11 ++----- .../src/template_parser/template_parser.ts | 32 +++++++++---------- .../compiler/test/ml_parser/ast_spec_utils.ts | 4 +-- .../test/ml_parser/html_parser_spec.ts | 4 +-- .../test/ml_parser/icu_ast_expander_spec.ts | 14 ++++---- .../template_parser/template_parser_spec.ts | 2 +- packages/language-service/src/completions.ts | 2 +- .../translation_parsers/translation_utils.ts | 2 +- 14 files changed, 53 insertions(+), 58 deletions(-) diff --git a/packages/compiler/src/i18n/extractor_merger.ts b/packages/compiler/src/i18n/extractor_merger.ts index 771515575e..66a8e7a1df 100644 --- a/packages/compiler/src/i18n/extractor_merger.ts +++ b/packages/compiler/src/i18n/extractor_merger.ts @@ -124,7 +124,7 @@ class _Visitor implements html.Visitor { this._translations = translations; // Construct a single fake root element - const wrapper = new html.Element('wrapper', [], nodes, undefined!, undefined, undefined); + const wrapper = new html.Element('wrapper', [], nodes, undefined!, undefined!, undefined); const translatedNode = wrapper.visit(this, null); @@ -492,7 +492,7 @@ class _Visitor implements html.Visitor { } private _reportError(node: html.Node, msg: string): void { - this._errors.push(new I18nError(node.sourceSpan!, msg)); + this._errors.push(new I18nError(node.sourceSpan, msg)); } } diff --git a/packages/compiler/src/i18n/i18n_parser.ts b/packages/compiler/src/i18n/i18n_parser.ts index 1a1f7f35bd..d6d93f32f8 100644 --- a/packages/compiler/src/i18n/i18n_parser.ts +++ b/packages/compiler/src/i18n/i18n_parser.ts @@ -83,7 +83,7 @@ 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.sourceSpan!.toString(); + context.placeholderToContent[startPhName] = el.sourceSpan.toString(); let closePhName = ''; @@ -93,7 +93,7 @@ class _I18nVisitor implements html.Visitor { } const node = new i18n.TagPlaceholder( - el.name, attrs, startPhName, closePhName, children, isVoid, el.sourceSpan!); + el.name, attrs, startPhName, closePhName, children, isVoid, el.sourceSpan); return context.visitNodeFn(el, node); } @@ -103,7 +103,7 @@ class _I18nVisitor implements html.Visitor { } visitText(text: html.Text, context: I18nMessageVisitorContext): i18n.Node { - const node = this._visitTextWithInterpolation(text.value, text.sourceSpan!, context); + const node = this._visitTextWithInterpolation(text.value, text.sourceSpan, context); return context.visitNodeFn(text, node); } diff --git a/packages/compiler/src/i18n/serializers/xliff.ts b/packages/compiler/src/i18n/serializers/xliff.ts index 0278b517b7..8fca51e8d6 100644 --- a/packages/compiler/src/i18n/serializers/xliff.ts +++ b/packages/compiler/src/i18n/serializers/xliff.ts @@ -231,9 +231,9 @@ class XliffParser implements ml.Visitor { break; case _TARGET_TAG: - const innerTextStart = element.startSourceSpan!.end.offset; + const innerTextStart = element.startSourceSpan.end.offset; const innerTextEnd = element.endSourceSpan!.start.offset; - const content = element.startSourceSpan!.start.file.content; + const content = element.startSourceSpan.start.file.content; const innerText = content.slice(innerTextStart, innerTextEnd); this._unitMlString = innerText; break; @@ -264,7 +264,7 @@ class XliffParser implements ml.Visitor { visitExpansionCase(expansionCase: ml.ExpansionCase, context: any): any {} private _addError(node: ml.Node, message: string): void { - this._errors.push(new I18nError(node.sourceSpan!, message)); + this._errors.push(new I18nError(node.sourceSpan, message)); } } @@ -288,14 +288,14 @@ class XmlToI18n implements ml.Visitor { } visitText(text: ml.Text, context: any) { - return new i18n.Text(text.value, text.sourceSpan!); + return new i18n.Text(text.value, text.sourceSpan); } visitElement(el: ml.Element, context: any): i18n.Placeholder|ml.Node[]|null { if (el.name === _PLACEHOLDER_TAG) { const nameAttr = el.attrs.find((attr) => attr.name === 'id'); if (nameAttr) { - return new i18n.Placeholder('', nameAttr.value, el.sourceSpan!); + return new i18n.Placeholder('', nameAttr.value, el.sourceSpan); } this._addError(el, `<${_PLACEHOLDER_TAG}> misses the "id" attribute`); @@ -332,7 +332,7 @@ class XmlToI18n implements ml.Visitor { visitAttribute(attribute: ml.Attribute, context: any) {} private _addError(node: ml.Node, message: string): void { - this._errors.push(new I18nError(node.sourceSpan!, message)); + this._errors.push(new I18nError(node.sourceSpan, message)); } } diff --git a/packages/compiler/src/i18n/serializers/xliff2.ts b/packages/compiler/src/i18n/serializers/xliff2.ts index 7e97464f98..f3499e7fcd 100644 --- a/packages/compiler/src/i18n/serializers/xliff2.ts +++ b/packages/compiler/src/i18n/serializers/xliff2.ts @@ -246,9 +246,9 @@ class Xliff2Parser implements ml.Visitor { break; case _TARGET_TAG: - const innerTextStart = element.startSourceSpan!.end.offset; + const innerTextStart = element.startSourceSpan.end.offset; const innerTextEnd = element.endSourceSpan!.start.offset; - const content = element.startSourceSpan!.start.file.content; + const content = element.startSourceSpan.start.file.content; const innerText = content.slice(innerTextStart, innerTextEnd); this._unitMlString = innerText; break; diff --git a/packages/compiler/src/i18n/serializers/xtb.ts b/packages/compiler/src/i18n/serializers/xtb.ts index 5483e5029d..ed7eab95f1 100644 --- a/packages/compiler/src/i18n/serializers/xtb.ts +++ b/packages/compiler/src/i18n/serializers/xtb.ts @@ -130,9 +130,9 @@ class XtbParser implements ml.Visitor { if (this._msgIdToHtml.hasOwnProperty(id)) { this._addError(element, `Duplicated translations for msg ${id}`); } else { - const innerTextStart = element.startSourceSpan!.end.offset; + const innerTextStart = element.startSourceSpan.end.offset; const innerTextEnd = element.endSourceSpan!.start.offset; - const content = element.startSourceSpan!.start.file.content; + const content = element.startSourceSpan.start.file.content; const innerText = content.slice(innerTextStart!, innerTextEnd!); this._msgIdToHtml[id] = innerText; } @@ -155,7 +155,7 @@ class XtbParser implements ml.Visitor { visitExpansionCase(expansionCase: ml.ExpansionCase, context: any): any {} private _addError(node: ml.Node, message: string): void { - this._errors.push(new I18nError(node.sourceSpan!, message)); + this._errors.push(new I18nError(node.sourceSpan, message)); } } @@ -179,7 +179,7 @@ class XmlToI18n implements ml.Visitor { } visitText(text: ml.Text, context: any) { - return new i18n.Text(text.value, text.sourceSpan!); + return new i18n.Text(text.value, text.sourceSpan); } visitExpansion(icu: ml.Expansion, context: any) { @@ -203,7 +203,7 @@ class XmlToI18n implements ml.Visitor { if (el.name === _PLACEHOLDER_TAG) { const nameAttr = el.attrs.find((attr) => attr.name === 'name'); if (nameAttr) { - return new i18n.Placeholder('', nameAttr.value, el.sourceSpan!); + return new i18n.Placeholder('', nameAttr.value, el.sourceSpan); } this._addError(el, `<${_PLACEHOLDER_TAG}> misses the "name" attribute`); @@ -218,6 +218,6 @@ class XmlToI18n implements ml.Visitor { visitAttribute(attribute: ml.Attribute, context: any) {} private _addError(node: ml.Node, message: string): void { - this._errors.push(new I18nError(node.sourceSpan!, message)); + this._errors.push(new I18nError(node.sourceSpan, message)); } } diff --git a/packages/compiler/src/ml_parser/ast.ts b/packages/compiler/src/ml_parser/ast.ts index 05abef7ccc..ead5c11cce 100644 --- a/packages/compiler/src/ml_parser/ast.ts +++ b/packages/compiler/src/ml_parser/ast.ts @@ -64,7 +64,7 @@ export class Attribute extends NodeWithI18n { export class Element extends NodeWithI18n { constructor( public name: string, public attrs: Attribute[], public children: Node[], - sourceSpan: ParseSourceSpan, public startSourceSpan: ParseSourceSpan|null = null, + sourceSpan: ParseSourceSpan, public startSourceSpan: ParseSourceSpan, public endSourceSpan: ParseSourceSpan|null = null, i18n?: I18nMeta) { super(sourceSpan, i18n); } diff --git a/packages/compiler/src/render3/r3_ast.ts b/packages/compiler/src/render3/r3_ast.ts index 23ec4d6b69..996dacb5bd 100644 --- a/packages/compiler/src/render3/r3_ast.ts +++ b/packages/compiler/src/render3/r3_ast.ts @@ -79,13 +79,8 @@ export class Element implements Node { constructor( public name: string, public attributes: TextAttribute[], public inputs: BoundAttribute[], public outputs: BoundEvent[], public children: Node[], public references: Reference[], - public sourceSpan: ParseSourceSpan, public startSourceSpan: ParseSourceSpan|null, - public endSourceSpan: ParseSourceSpan|null, public i18n?: I18nMeta) { - // If the element is empty then the source span should include any closing tag - if (children.length === 0 && startSourceSpan && endSourceSpan) { - this.sourceSpan = new ParseSourceSpan(sourceSpan.start, endSourceSpan.end); - } - } + public sourceSpan: ParseSourceSpan, public startSourceSpan: ParseSourceSpan, + public endSourceSpan: ParseSourceSpan|null, public i18n?: I18nMeta) {} visit(visitor: Visitor): Result { return visitor.visitElement(this); } @@ -96,7 +91,7 @@ export class Template implements Node { public tagName: string, public attributes: TextAttribute[], public inputs: BoundAttribute[], public outputs: BoundEvent[], public templateAttrs: (BoundAttribute|TextAttribute)[], public children: Node[], public references: Reference[], public variables: Variable[], - public sourceSpan: ParseSourceSpan, public startSourceSpan: ParseSourceSpan|null, + public sourceSpan: ParseSourceSpan, public startSourceSpan: ParseSourceSpan, public endSourceSpan: ParseSourceSpan|null, public i18n?: I18nMeta) {} visit(visitor: Visitor): Result { return visitor.visitTemplate(this); diff --git a/packages/compiler/src/template_parser/template_parser.ts b/packages/compiler/src/template_parser/template_parser.ts index fde0f920c1..5c6bd78918 100644 --- a/packages/compiler/src/template_parser/template_parser.ts +++ b/packages/compiler/src/template_parser/template_parser.ts @@ -244,9 +244,9 @@ class TemplateParseVisitor implements html.Visitor { visitText(text: html.Text, parent: ElementContext): any { const ngContentIndex = parent.findNgContentIndex(TEXT_CSS_SELECTOR())!; const valueNoNgsp = replaceNgsp(text.value); - const expr = this._bindingParser.parseInterpolation(valueNoNgsp, text.sourceSpan!); - return expr ? new t.BoundTextAst(expr, ngContentIndex, text.sourceSpan!) : - new t.TextAst(valueNoNgsp, ngContentIndex, text.sourceSpan!); + const expr = this._bindingParser.parseInterpolation(valueNoNgsp, text.sourceSpan); + return expr ? new t.BoundTextAst(expr, ngContentIndex, text.sourceSpan) : + new t.TextAst(valueNoNgsp, ngContentIndex, text.sourceSpan); } visitAttribute(attribute: html.Attribute, context: any): any { @@ -335,14 +335,14 @@ class TemplateParseVisitor implements html.Visitor { const boundDirectivePropNames = new Set(); const directiveAsts = this._createDirectiveAsts( isTemplateElement, element.name, directiveMetas, elementOrDirectiveProps, - elementOrDirectiveRefs, element.sourceSpan!, references, boundDirectivePropNames); + elementOrDirectiveRefs, element.sourceSpan, references, boundDirectivePropNames); const elementProps: t.BoundElementPropertyAst[] = this._createElementPropertyAsts( element.name, elementOrDirectiveProps, boundDirectivePropNames); const isViewRoot = parent.isTemplateElement || hasInlineTemplates; const providerContext = new ProviderElementContext( this.providerViewContext, parent.providerContext!, isViewRoot, directiveAsts, attrs, - references, isTemplateElement, queryStartIndex, element.sourceSpan!); + references, isTemplateElement, queryStartIndex, element.sourceSpan); const children: t.TemplateAst[] = html.visitAll( preparsedElement.nonBindable ? NON_BINDABLE_VISITOR : this, element.children, @@ -360,26 +360,26 @@ class TemplateParseVisitor implements html.Visitor { if (preparsedElement.type === PreparsedElementType.NG_CONTENT) { // `` element if (element.children && !element.children.every(_isEmptyTextNode)) { - this._reportError(` element cannot have content.`, element.sourceSpan!); + this._reportError(` element cannot have content.`, element.sourceSpan); } parsedElement = new t.NgContentAst( - this.ngContentCount++, hasInlineTemplates ? null! : ngContentIndex, element.sourceSpan!); + this.ngContentCount++, hasInlineTemplates ? null! : ngContentIndex, element.sourceSpan); } else if (isTemplateElement) { // `` element this._assertAllEventsPublishedByDirectives(directiveAsts, events); this._assertNoComponentsNorElementBindingsOnTemplate( - directiveAsts, elementProps, element.sourceSpan!); + directiveAsts, elementProps, element.sourceSpan); parsedElement = new t.EmbeddedTemplateAst( attrs, events, references, elementVars, providerContext.transformedDirectiveAsts, providerContext.transformProviders, providerContext.transformedHasViewContainer, providerContext.queryMatches, children, hasInlineTemplates ? null! : ngContentIndex, - element.sourceSpan!); + element.sourceSpan); } else { // element other than `` and `` this._assertElementExists(matchElement, element); - this._assertOnlyOneComponent(directiveAsts, element.sourceSpan!); + this._assertOnlyOneComponent(directiveAsts, element.sourceSpan); const ngContentIndex = hasInlineTemplates ? null : parent.findNgContentIndex(projectionSelector); @@ -397,22 +397,22 @@ class TemplateParseVisitor implements html.Visitor { const {directives} = this._parseDirectives(this.selectorMatcher, templateSelector); const templateBoundDirectivePropNames = new Set(); const templateDirectiveAsts = this._createDirectiveAsts( - true, elName, directives, templateElementOrDirectiveProps, [], element.sourceSpan!, [], + true, elName, directives, templateElementOrDirectiveProps, [], element.sourceSpan, [], templateBoundDirectivePropNames); const templateElementProps: t.BoundElementPropertyAst[] = this._createElementPropertyAsts( elName, templateElementOrDirectiveProps, templateBoundDirectivePropNames); this._assertNoComponentsNorElementBindingsOnTemplate( - templateDirectiveAsts, templateElementProps, element.sourceSpan!); + templateDirectiveAsts, templateElementProps, element.sourceSpan); const templateProviderContext = new ProviderElementContext( this.providerViewContext, parent.providerContext!, parent.isTemplateElement, - templateDirectiveAsts, [], [], true, templateQueryStartIndex, element.sourceSpan!); + templateDirectiveAsts, [], [], true, templateQueryStartIndex, element.sourceSpan); templateProviderContext.afterElement(); parsedElement = new t.EmbeddedTemplateAst( [], [], [], templateElementVars, templateProviderContext.transformedDirectiveAsts, templateProviderContext.transformProviders, templateProviderContext.transformedHasViewContainer, templateProviderContext.queryMatches, - [parsedElement], ngContentIndex, element.sourceSpan!); + [parsedElement], ngContentIndex, element.sourceSpan); } return parsedElement; @@ -707,7 +707,7 @@ class TemplateParseVisitor implements html.Visitor { errorMsg += `2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`; } - this._reportError(errorMsg, element.sourceSpan!); + this._reportError(errorMsg, element.sourceSpan); } } @@ -815,7 +815,7 @@ class NonBindableVisitor implements html.Visitor { visitText(text: html.Text, parent: ElementContext): t.TextAst { const ngContentIndex = parent.findNgContentIndex(TEXT_CSS_SELECTOR())!; - return new t.TextAst(text.value, ngContentIndex, text.sourceSpan!); + return new t.TextAst(text.value, ngContentIndex, text.sourceSpan); } visitExpansion(expansion: html.Expansion, context: any): any { diff --git a/packages/compiler/test/ml_parser/ast_spec_utils.ts b/packages/compiler/test/ml_parser/ast_spec_utils.ts index 10060905bb..a6f7ca65df 100644 --- a/packages/compiler/test/ml_parser/ast_spec_utils.ts +++ b/packages/compiler/test/ml_parser/ast_spec_utils.ts @@ -42,7 +42,7 @@ class _Humanizer implements html.Visitor { visitElement(element: html.Element, context: any): any { const res = this._appendContext(element, [html.Element, element.name, this.elDepth++]); if (this.includeSourceSpan) { - res.push(element.startSourceSpan?.toString() ?? null); + res.push(element.startSourceSpan.toString() ?? null); res.push(element.endSourceSpan?.toString() ?? null); } this.result.push(res); @@ -82,7 +82,7 @@ class _Humanizer implements html.Visitor { private _appendContext(ast: html.Node, input: any[]): any[] { if (!this.includeSourceSpan) return input; - input.push(ast.sourceSpan!.toString()); + input.push(ast.sourceSpan.toString()); return input; } } diff --git a/packages/compiler/test/ml_parser/html_parser_spec.ts b/packages/compiler/test/ml_parser/html_parser_spec.ts index f6a67701b4..b3eb04fbe3 100644 --- a/packages/compiler/test/ml_parser/html_parser_spec.ts +++ b/packages/compiler/test/ml_parser/html_parser_spec.ts @@ -667,8 +667,8 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe it('should set the start and end source spans', () => { const node = parser.parse('
a
', 'TestComp').rootNodes[0]; - expect(node.startSourceSpan!.start.offset).toEqual(0); - expect(node.startSourceSpan!.end.offset).toEqual(5); + expect(node.startSourceSpan.start.offset).toEqual(0); + expect(node.startSourceSpan.end.offset).toEqual(5); expect(node.endSourceSpan!.start.offset).toEqual(6); expect(node.endSourceSpan!.end.offset).toEqual(12); diff --git a/packages/compiler/test/ml_parser/icu_ast_expander_spec.ts b/packages/compiler/test/ml_parser/icu_ast_expander_spec.ts index 881f09e58d..d9e8ed2407 100644 --- a/packages/compiler/test/ml_parser/icu_ast_expander_spec.ts +++ b/packages/compiler/test/ml_parser/icu_ast_expander_spec.ts @@ -56,10 +56,10 @@ import {humanizeNodes} from './ast_spec_utils'; const nodes = expand(`{messages.length, plural,=0 {bold}}`).nodes; const container: html.Element = nodes[0]; - expect(container.sourceSpan!.start.col).toEqual(0); - expect(container.sourceSpan!.end.col).toEqual(42); - expect(container.startSourceSpan!.start.col).toEqual(0); - expect(container.startSourceSpan!.end.col).toEqual(42); + expect(container.sourceSpan.start.col).toEqual(0); + expect(container.sourceSpan.end.col).toEqual(42); + expect(container.startSourceSpan.start.col).toEqual(0); + expect(container.startSourceSpan.end.col).toEqual(42); expect(container.endSourceSpan!.start.col).toEqual(0); expect(container.endSourceSpan!.end.col).toEqual(42); @@ -68,15 +68,15 @@ import {humanizeNodes} from './ast_spec_utils'; expect(switchExp.sourceSpan.end.col).toEqual(16); const template: html.Element = container.children[0]; - expect(template.sourceSpan!.start.col).toEqual(25); - expect(template.sourceSpan!.end.col).toEqual(41); + expect(template.sourceSpan.start.col).toEqual(25); + expect(template.sourceSpan.end.col).toEqual(41); const switchCheck = template.attrs[0]; expect(switchCheck.sourceSpan.start.col).toEqual(25); expect(switchCheck.sourceSpan.end.col).toEqual(28); const b: html.Element = template.children[0]; - expect(b.sourceSpan!.start.col).toEqual(29); + expect(b.sourceSpan.start.col).toEqual(29); expect(b.endSourceSpan!.end.col).toEqual(40); }); diff --git a/packages/compiler/test/template_parser/template_parser_spec.ts b/packages/compiler/test/template_parser/template_parser_spec.ts index fe620e0e6f..17f75b0957 100644 --- a/packages/compiler/test/template_parser/template_parser_spec.ts +++ b/packages/compiler/test/template_parser/template_parser_spec.ts @@ -140,7 +140,7 @@ class TemplateHumanizer implements TemplateAstVisitor { private _appendSourceSpan(ast: TemplateAst, input: any[]): any[] { if (!this.includeSourceSpan) return input; - input.push(ast.sourceSpan!.toString()); + input.push(ast.sourceSpan.toString()); return input; } } diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index b4cf641aff..6025848b9d 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -65,7 +65,7 @@ function getBoundedWordSpan( // The HTML tag may include `-` (e.g. `app-root`), // so use the HtmlAst to get the span before ayazhafiz refactor the code. return { - start: templateInfo.template.span.start + ast.startSourceSpan!.start.offset + 1, + start: templateInfo.template.span.start + ast.startSourceSpan.start.offset + 1, length: ast.name.length }; } diff --git a/packages/localize/src/tools/src/translate/translation_files/translation_parsers/translation_utils.ts b/packages/localize/src/tools/src/translate/translation_files/translation_parsers/translation_utils.ts index f44cd87e93..c310da8459 100644 --- a/packages/localize/src/tools/src/translate/translation_files/translation_parsers/translation_utils.ts +++ b/packages/localize/src/tools/src/translate/translation_files/translation_parsers/translation_utils.ts @@ -48,7 +48,7 @@ export function parseInnerRange(element: Element): ParseTreeResult { * @param element The element whose inner range we want to compute. */ function getInnerRange(element: Element): LexerRange { - const start = element.startSourceSpan!.end; + const start = element.startSourceSpan.end; const end = element.endSourceSpan!.start; return { startPos: start.offset,