From c11527fc68965fee9799cc4d1f6f0ea33fce98ce Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sat, 24 May 2025 09:11:39 +0200 Subject: [PATCH] refactor(compiler): remove unused code (#61668) Removes the following unused code from the compiler: * The ICU expander appears to be some really old code converting ICUs into legacy-style control flow directives. It's annoying to keep this one since we need to update it any time the AST changes. * The `PipeCollector` class wasn't used anywhere. * The `partitionArray` function wasn't used anywhere. * The `newArray` function was used only in one test and it can be easily replaced with `new Array().fill`. PR Close #61668 --- .../src/ml_parser/icu_ast_expander.ts | 270 ------------------ .../src/template_parser/binding_parser.ts | 11 - packages/compiler/src/util.ts | 30 -- .../test/ml_parser/icu_ast_expander_spec.ts | 144 ---------- .../compiler/test/output/output_jit_spec.ts | 13 +- packages/compiler/test/util_spec.ts | 19 +- 6 files changed, 7 insertions(+), 480 deletions(-) delete mode 100644 packages/compiler/src/ml_parser/icu_ast_expander.ts delete mode 100644 packages/compiler/test/ml_parser/icu_ast_expander_spec.ts diff --git a/packages/compiler/src/ml_parser/icu_ast_expander.ts b/packages/compiler/src/ml_parser/icu_ast_expander.ts deleted file mode 100644 index 9920655a353..00000000000 --- a/packages/compiler/src/ml_parser/icu_ast_expander.ts +++ /dev/null @@ -1,270 +0,0 @@ -/** - * @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.dev/license - */ - -import {ParseError, ParseSourceSpan} from '../parse_util'; - -import * as html from './ast'; - -// http://cldr.unicode.org/index/cldr-spec/plural-rules -const PLURAL_CASES: string[] = ['zero', 'one', 'two', 'few', 'many', 'other']; - -/** - * Expands special forms into elements. - * - * For example, - * - * ``` - * { messages.length, plural, - * =0 {zero} - * =1 {one} - * other {more than one} - * } - * ``` - * - * will be expanded into - * - * ```html - * - * zero - * one - * more than one - * - * ``` - */ -export function expandNodes(nodes: html.Node[]): ExpansionResult { - const expander = new _Expander(); - return new ExpansionResult(html.visitAll(expander, nodes), expander.isExpanded, expander.errors); -} - -export class ExpansionResult { - constructor( - public nodes: html.Node[], - public expanded: boolean, - public errors: ParseError[], - ) {} -} - -export class ExpansionError extends ParseError { - constructor(span: ParseSourceSpan, errorMsg: string) { - super(span, errorMsg); - } -} - -/** - * Expand expansion forms (plural, select) to directives - * - * @internal - */ -class _Expander implements html.Visitor { - isExpanded: boolean = false; - errors: ParseError[] = []; - - visitElement(element: html.Element, context: any): any { - return new html.Element( - element.name, - element.attrs, - element.directives, - html.visitAll(this, element.children), - element.isSelfClosing, - element.sourceSpan, - element.startSourceSpan, - element.endSourceSpan, - ); - } - - visitAttribute(attribute: html.Attribute, context: any): any { - return attribute; - } - - visitText(text: html.Text, context: any): any { - return text; - } - - visitComment(comment: html.Comment, context: any): any { - return comment; - } - - visitExpansion(icu: html.Expansion, context: any): any { - this.isExpanded = true; - return icu.type === 'plural' - ? _expandPluralForm(icu, this.errors) - : _expandDefaultForm(icu, this.errors); - } - - visitExpansionCase(icuCase: html.ExpansionCase, context: any): any { - throw new Error('Should not be reached'); - } - - visitBlock(block: html.Block, context: any) { - return new html.Block( - block.name, - block.parameters, - html.visitAll(this, block.children), - block.sourceSpan, - block.nameSpan, - block.startSourceSpan, - block.endSourceSpan, - ); - } - - visitBlockParameter(parameter: html.BlockParameter, context: any) { - return parameter; - } - - visitLetDeclaration(decl: html.LetDeclaration, context: any) { - return decl; - } - - visitComponent(node: html.Component, context: any): any { - return new html.Component( - node.componentName, - node.tagName, - node.fullName, - node.attrs, - node.directives, - html.visitAll(this, node.children), - node.isSelfClosing, - node.sourceSpan, - node.startSourceSpan, - node.endSourceSpan, - ); - } - - visitDirective(directive: html.Directive, context: any) { - return directive; - } -} - -// Plural forms are expanded to `NgPlural` and `NgPluralCase`s -function _expandPluralForm(ast: html.Expansion, errors: ParseError[]): html.Element { - const children = ast.cases.map((c) => { - if (PLURAL_CASES.indexOf(c.value) === -1 && !c.value.match(/^=\d+$/)) { - errors.push( - new ExpansionError( - c.valueSourceSpan, - `Plural cases should be "=" or one of ${PLURAL_CASES.join(', ')}`, - ), - ); - } - - const expansionResult = expandNodes(c.expression); - errors.push(...expansionResult.errors); - - return new html.Element( - `ng-template`, - [ - new html.Attribute( - 'ngPluralCase', - `${c.value}`, - c.valueSourceSpan, - undefined /* keySpan */, - undefined /* valueSpan */, - undefined /* valueTokens */, - undefined /* i18n */, - ), - ], - [], - expansionResult.nodes, - false, - c.sourceSpan, - c.sourceSpan, - c.sourceSpan, - ); - }); - const switchAttr = new html.Attribute( - '[ngPlural]', - ast.switchValue, - ast.switchValueSourceSpan, - undefined /* keySpan */, - undefined /* valueSpan */, - undefined /* valueTokens */, - undefined /* i18n */, - ); - return new html.Element( - 'ng-container', - [switchAttr], - [], - children, - false, - ast.sourceSpan, - ast.sourceSpan, - ast.sourceSpan, - ); -} - -// ICU messages (excluding plural form) are expanded to `NgSwitch` and `NgSwitchCase`s -function _expandDefaultForm(ast: html.Expansion, errors: ParseError[]): html.Element { - const children = ast.cases.map((c) => { - const expansionResult = expandNodes(c.expression); - errors.push(...expansionResult.errors); - - if (c.value === 'other') { - // other is the default case when no values match - return new html.Element( - `ng-template`, - [ - new html.Attribute( - 'ngSwitchDefault', - '', - c.valueSourceSpan, - undefined /* keySpan */, - undefined /* valueSpan */, - undefined /* valueTokens */, - undefined /* i18n */, - ), - ], - [], - expansionResult.nodes, - false, - c.sourceSpan, - c.sourceSpan, - c.sourceSpan, - ); - } - - return new html.Element( - `ng-template`, - [ - new html.Attribute( - 'ngSwitchCase', - `${c.value}`, - c.valueSourceSpan, - undefined /* keySpan */, - undefined /* valueSpan */, - undefined /* valueTokens */, - undefined /* i18n */, - ), - ], - [], - expansionResult.nodes, - false, - c.sourceSpan, - c.sourceSpan, - c.sourceSpan, - ); - }); - const switchAttr = new html.Attribute( - '[ngSwitch]', - ast.switchValue, - ast.switchValueSourceSpan, - undefined /* keySpan */, - undefined /* valueSpan */, - undefined /* valueTokens */, - undefined /* i18n */, - ); - return new html.Element( - 'ng-container', - [switchAttr], - children, - [], - false, - ast.sourceSpan, - ast.sourceSpan, - ast.sourceSpan, - ); -} diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index 3948c807c63..8970751eb86 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -26,7 +26,6 @@ import { ParsedVariable, ParserError, PropertyRead, - RecursiveAstVisitor, TemplateBinding, ThisReceiver, VariableBinding, @@ -838,16 +837,6 @@ export class BindingParser { } } -export class PipeCollector extends RecursiveAstVisitor { - pipes = new Map(); - override visitPipe(ast: BindingPipe, context: any): any { - this.pipes.set(ast.name, ast); - ast.exp.visit(this); - this.visitAll(ast.args, context); - return null; - } -} - function isAnimationLabel(name: string): boolean { return name[0] == '@'; } diff --git a/packages/compiler/src/util.ts b/packages/compiler/src/util.ts index 35772a503bd..25c8ae86044 100644 --- a/packages/compiler/src/util.ts +++ b/packages/compiler/src/util.ts @@ -138,36 +138,6 @@ export interface Console { const _global: {[name: string]: any} = globalThis; export {_global as global}; -export function newArray(size: number): T[]; -export function newArray(size: number, value: T): T[]; -export function newArray(size: number, value?: T): T[] { - const list: T[] = []; - for (let i = 0; i < size; i++) { - list.push(value!); - } - return list; -} - -/** - * Partitions a given array into 2 arrays, based on a boolean value returned by the condition - * function. - * - * @param arr Input array that should be partitioned - * @param conditionFn Condition function that is called for each item in a given array and returns a - * boolean value. - */ -export function partitionArray( - arr: (T | F)[], - conditionFn: (value: T | F) => boolean, -): [T[], F[]] { - const truthy: T[] = []; - const falsy: F[] = []; - for (const item of arr) { - (conditionFn(item) ? truthy : falsy).push(item as any); - } - return [truthy, falsy]; -} - const V1_TO_18 = /^([1-9]|1[0-8])\./; export function getJitStandaloneDefaultForVersion(version: string): boolean { diff --git a/packages/compiler/test/ml_parser/icu_ast_expander_spec.ts b/packages/compiler/test/ml_parser/icu_ast_expander_spec.ts deleted file mode 100644 index e9a89b6f5f1..00000000000 --- a/packages/compiler/test/ml_parser/icu_ast_expander_spec.ts +++ /dev/null @@ -1,144 +0,0 @@ -/** - * @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.dev/license - */ - -import * as html from '../../src/ml_parser/ast'; -import {HtmlParser} from '../../src/ml_parser/html_parser'; -import {expandNodes, ExpansionResult} from '../../src/ml_parser/icu_ast_expander'; -import {TokenizeOptions} from '../../src/ml_parser/lexer'; -import {ParseError} from '../../src/parse_util'; - -import {humanizeNodes} from './ast_spec_utils'; - -describe('Expander', () => { - function expand(template: string, options: TokenizeOptions = {}): ExpansionResult { - const htmlParser = new HtmlParser(); - const res = htmlParser.parse(template, 'url', {tokenizeExpansionForms: true, ...options}); - return expandNodes(res.rootNodes); - } - - it('should handle the plural expansion form', () => { - const res = expand(`{messages.length, plural,=0 {zerobold}}`); - - expect(humanizeNodes(res.nodes)).toEqual([ - [html.Element, 'ng-container', 0], - [html.Attribute, '[ngPlural]', 'messages.length'], - [html.Element, 'ng-template', 1], - [html.Attribute, 'ngPluralCase', '=0'], - [html.Text, 'zero', 2, ['zero']], - [html.Element, 'b', 2], - [html.Text, 'bold', 3, ['bold']], - ]); - }); - - it('should handle nested expansion forms', () => { - const res = expand(`{messages.length, plural, =0 { {p.gender, select, male {m}} }}`); - - expect(humanizeNodes(res.nodes)).toEqual([ - [html.Element, 'ng-container', 0], - [html.Attribute, '[ngPlural]', 'messages.length'], - [html.Element, 'ng-template', 1], - [html.Attribute, 'ngPluralCase', '=0'], - [html.Element, 'ng-container', 2], - [html.Attribute, '[ngSwitch]', 'p.gender'], - [html.Element, 'ng-template', 3], - [html.Attribute, 'ngSwitchCase', 'male'], - [html.Text, 'm', 4, ['m']], - [html.Text, ' ', 2, [' ']], - ]); - }); - - it('should correctly set source code positions', () => { - 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.endSourceSpan!.start.col).toEqual(0); - expect(container.endSourceSpan!.end.col).toEqual(42); - - const switchExp = container.attrs[0]; - expect(switchExp.sourceSpan.start.col).toEqual(1); - 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); - - 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.endSourceSpan!.end.col).toEqual(40); - }); - - it('should handle other special forms', () => { - const res = expand(`{person.gender, select, male {m} other {default}}`); - - expect(humanizeNodes(res.nodes)).toEqual([ - [html.Element, 'ng-container', 0], - [html.Attribute, '[ngSwitch]', 'person.gender'], - [html.Element, 'ng-template', 1], - [html.Attribute, 'ngSwitchCase', 'male'], - [html.Text, 'm', 2, ['m']], - [html.Element, 'ng-template', 1], - [html.Attribute, 'ngSwitchDefault', ''], - [html.Text, 'default', 2, ['default']], - ]); - }); - - it('should parse an expansion form as a tag single child', () => { - const res = expand(`
{a, b, =4 {c}}
`); - - expect(humanizeNodes(res.nodes)).toEqual([ - [html.Element, 'div', 0], - [html.Element, 'span', 1], - [html.Element, 'ng-container', 2], - [html.Attribute, '[ngSwitch]', 'a'], - [html.Element, 'ng-template', 3], - [html.Attribute, 'ngSwitchCase', '=4'], - [html.Text, 'c', 4, ['c']], - ]); - }); - - it('should parse an expansion forms inside of blocks', () => { - const res = expand('@if (cond) {{a, b, =4 {c}}@if (otherCond) {{d, e, =4 {f}}}}'); - - expect(humanizeNodes(res.nodes)).toEqual([ - [html.Block, 'if', 0], - [html.BlockParameter, 'cond'], - [html.Element, 'ng-container', 1], - [html.Attribute, '[ngSwitch]', 'a'], - [html.Element, 'ng-template', 2], - [html.Attribute, 'ngSwitchCase', '=4'], - [html.Text, 'c', 3, ['c']], - [html.Block, 'if', 1], - [html.BlockParameter, 'otherCond'], - [html.Element, 'ng-container', 2], - [html.Attribute, '[ngSwitch]', 'd'], - [html.Element, 'ng-template', 3], - [html.Attribute, 'ngSwitchCase', '=4'], - [html.Text, 'f', 4, ['f']], - ]); - }); - - describe('errors', () => { - it('should error on unknown plural cases', () => { - expect(humanizeErrors(expand('{n, plural, unknown {-}}').errors)).toEqual([ - `Plural cases should be "=" or one of zero, one, two, few, many, other`, - ]); - }); - }); -}); - -function humanizeErrors(errors: ParseError[]): string[] { - return errors.map((error) => error.msg); -} diff --git a/packages/compiler/test/output/output_jit_spec.ts b/packages/compiler/test/output/output_jit_spec.ts index 147916b3fb1..16349b99da0 100644 --- a/packages/compiler/test/output/output_jit_spec.ts +++ b/packages/compiler/test/output/output_jit_spec.ts @@ -10,17 +10,16 @@ import {EmitterVisitorContext} from '../../src/output/abstract_emitter'; import * as o from '../../src/output/output_ast'; import {JitEmitterVisitor, JitEvaluator} from '../../src/output/output_jit'; import {R3JitReflector} from '../../src/render3/r3_jit'; -import {newArray} from '../../src/util'; describe('Output JIT', () => { describe('regression', () => { it('should generate unique argument names', () => { - const externalIds = newArray(10, 1).map( - (_, index) => new o.ExternalReference('@angular/core', `id_${index}_`), - ); - const externalIds1 = newArray(10, 1).map( - (_, index) => new o.ExternalReference('@angular/core', `id_${index}_1`), - ); + const externalIds = new Array(10) + .fill(null) + .map((_, index) => new o.ExternalReference('@angular/core', `id_${index}_`)); + const externalIds1 = new Array(10) + .fill(null) + .map((_, index) => new o.ExternalReference('@angular/core', `id_${index}_1`)); const ctx = EmitterVisitorContext.createRoot(); const reflectorContext: {[key: string]: string} = {}; for (const {name} of externalIds) { diff --git a/packages/compiler/test/util_spec.ts b/packages/compiler/test/util_spec.ts index fee6a3619e6..d432e6ccfbb 100644 --- a/packages/compiler/test/util_spec.ts +++ b/packages/compiler/test/util_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import {escapeRegExp, partitionArray, splitAtColon, stringify, utf8Encode} from '../src/util'; +import {escapeRegExp, splitAtColon, stringify, utf8Encode} from '../src/util'; describe('util', () => { describe('splitAtColon', () => { @@ -86,21 +86,4 @@ describe('util', () => { expect(stringify(Object.create(null))).toEqual('object'); }); }); - - describe('partitionArray()', () => { - it('should handle empty arrays', () => { - expect(partitionArray([], () => true)).toEqual([[], []]); - }); - - it('should handle arrays with primitive type values', () => { - expect(partitionArray([1, 2, 3], (el: number) => el < 2)).toEqual([[1], [2, 3]]); - }); - - it('should handle arrays of objects', () => { - expect(partitionArray([{id: 1}, {id: 2}, {id: 3}], (el: any) => el.id < 2)).toEqual([ - [{id: 1}], - [{id: 2}, {id: 3}], - ]); - }); - }); });