Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,57 @@ class UninvokedTrackFunctionCheck extends TemplateCheckWithVisitor<ErrorCode.UNI

if (symbol !== null && symbol.kind === SymbolKind.Expression) {
const type = ctx.templateTypeChecker.getTypeOfSymbol(symbol);
if (type && type.getCallSignatures()?.length > 0) {
const fullExpressionText = generateStringFromExpression(
node.trackBy.ast,
node.trackBy.source || '',
);

const errorString = formatExtendedError(
ErrorCode.UNINVOKED_TRACK_FUNCTION,
`The track function in the @for block should be invoked: ${fullExpressionText}(/* arguments */)`,
);

return [ctx.makeTemplateDiagnostic(node.sourceSpan, errorString)];
if (type) {
const callSignatures = type.getCallSignatures();
if (callSignatures.length > 0) {
const hasParameters = callSignatures.some((sig) => sig.parameters.length > 0);
const tsSymbol = ctx.templateTypeChecker.getTsSymbolOfSymbol(symbol);
const isMethod = isMethodSymbol(tsSymbol, callSignatures);

if (hasParameters || isMethod) {
const fullExpressionText = generateStringFromExpression(
node.trackBy.ast,
node.trackBy.source || '',
);

const errorString = formatExtendedError(
ErrorCode.UNINVOKED_TRACK_FUNCTION,
`The track function in the @for block should be invoked: ${fullExpressionText}(/* arguments */)`,
);

return [ctx.makeTemplateDiagnostic(node.sourceSpan, errorString)];
}
}
}
}

return [];
}
}

function isMethodSymbol(
tsSymbol: ts.Symbol | null,
callSignatures: readonly ts.Signature[],
): boolean {
if (tsSymbol !== null) {
if ((tsSymbol.flags & ts.SymbolFlags.Method) !== 0) {
return true;
}
const declarations = tsSymbol.getDeclarations();
if (declarations !== undefined) {
if (declarations.some((decl) => ts.isMethodDeclaration(decl) || ts.isMethodSignature(decl))) {
return true;
}
}
}

return callSignatures.some(
(sig) =>
sig.declaration !== undefined &&
(ts.isMethodDeclaration(sig.declaration) || ts.isMethodSignature(sig.declaration)),
);
}

function generateStringFromExpression(expression: AST, source: string): string {
return source.substring(expression.span.start, expression.span.end);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ jasmine_test(
data = [
":test_lib",
"//packages/core:npm_package",
"//packages/forms:npm_package",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@

import ts from 'typescript';

import {formatExtendedError} from '@angular/compiler-cli/src/ngtsc/typecheck/extended/api';
import {ErrorCode, ExtendedTemplateDiagnosticName, ngErrorCode} from '../../../../../diagnostics';
import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system';
import {runInEachFileSystem} from '../../../../../file_system/testing';
import {getSourceCodeForDiagnostic} from '../../../../../testing';
import {getClass, setup} from '../../../../testing';
import {factory as uninvokedTrackFunctionCheckFactory} from '../../../checks/uninvoked_track_function';
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker';
import {formatExtendedError} from '@angular/compiler-cli/src/ngtsc/typecheck/extended/api';

runInEachFileSystem(() => {
describe('UninvokedTrackFunctionCheck', () => {
Expand Down Expand Up @@ -75,23 +75,163 @@ runInEachFileSystem(() => {

expect(diags.length).toBe(0);
});

it('should not produce a warning when track is a FieldTree property', () => {
const diags = diagnoseTestComponent(
`
@for (row of rows; track row.field) {}
`,
`rows!: {field: FieldTree<string>}[];`,
`import type {FieldTree} from '@angular/forms/signals';`,
);

expect(diags.length).toBe(0);
});

it('should not produce a warning when track is a ReadonlyFieldTree property', () => {
const diags = diagnoseTestComponent(
`
@for (row of rows; track row.field) {}
`,
`rows!: {field: ReadonlyFieldTree<string>}[];`,
`import type {ReadonlyFieldTree} from '@angular/forms/signals';`,
);

expect(diags.length).toBe(0);
});

it('should not produce a warning when track is a Field property', () => {
const diags = diagnoseTestComponent(
`
@for (row of rows; track row.field) {}
`,
`rows!: {field: Field<string>}[];`,
`import type {Field} from '@angular/forms/signals';`,
);

expect(diags.length).toBe(0);
});

it('should not produce a warning when track is a nested FieldTree property', () => {
const diags = diagnoseTestComponent(
`
@for (row of rows; track row.field.subField) {}
`,
`rows!: {field: FieldTree<{subField: string}>}[];`,
`import type {FieldTree} from '@angular/forms/signals';`,
);

expect(diags.length).toBe(0);
});

it('should produce a warning when track is a regular function on an object', () => {
const diags = diagnoseTestComponent(
`
@for (row of rows; track row.trackFn) {}
`,
`rows!: {trackFn: (item: any) => string}[];`,
);

expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.UNINVOKED_TRACK_FUNCTION));
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`@for (row of rows; track row.trackFn) {}`);
expect(diags[0].messageText).toBe(generateDiagnosticText('row.trackFn'));
});

it('should not produce a warning when track is a simple callable object with no parameters', () => {
const diags = diagnoseTestComponent(
`
@for (item of callableItems; track item) {}
@for (row of rows; track row.item) {}
`,
`
callableItems!: (((() => string) & {id: number})[]);
rows!: {item: (() => string) & {id: number}}[];
`,
);

expect(diags.length).toBe(0);
});

it('should produce a warning when track is a method on an item', () => {
const diags = diagnoseTestComponent(
`
@for (item of itemsWithMethod; track item.getId) {}
`,
`itemsWithMethod!: {getId(): string}[];`,
);

expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.UNINVOKED_TRACK_FUNCTION));
expect(getSourceCodeForDiagnostic(diags[0])).toBe(
`@for (item of itemsWithMethod; track item.getId) {}`,
);
expect(diags[0].messageText).toBe(generateDiagnosticText('item.getId'));
});

it('should produce a warning when track is an arrow function property on component', () => {
const diags = diagnoseTestComponent(
`
@for (item of items; track trackFn) {}
`,
`trackFn = (item: any) => item.name;`,
);

expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.UNINVOKED_TRACK_FUNCTION));
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`@for (item of items; track trackFn) {}`);
expect(diags[0].messageText).toBe(generateDiagnosticText('trackFn'));
});

it('should not produce a warning with signal forms field tracking patterns', () => {
const diags = diagnoseTestComponent(
`
@for (email of emailsForm.emails; track email) {}
@for (row of rows(); track row.field) {}
`,
`
readonly model = signal({
emails: ['john.doe@mail.com', 'max.musterman@mail.com'],
});
readonly emailsForm = form(this.model);

readonly rows = computed(() =>
this.model().emails.map((_, index) => ({
index,
field: this.emailsForm.emails[index],
}))
);
`,
`import {computed, signal} from '@angular/core';\nimport {form} from '@angular/forms/signals';`,
);

expect(diags.length).toBe(0);
});
});
});

function diagnoseTestComponent(template: string, classField: string) {
function diagnoseTestComponent(template: string, classField: string, imports: string = '') {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
{
fileName,
templates: {'TestCmp': template},
source: `
const {program, templateTypeChecker} = setup(
[
{
fileName,
templates: {'TestCmp': template},
source: `
${imports}
export class TestCmp {
items = [{name: 'a'}, {name: 'b'}];
signalItems = [{name: signal('a')}, {name: signal('b')}];
${classField}
}`,
},
]);
},
],
{},
{forms: true},
);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
Expand Down
25 changes: 24 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,21 @@ export function angularCoreDtsFiles(): TestFile[] {
})));
}

let _angularFormsDts: TestFile[] | null = null;
export function angularFormsDtsFiles(): TestFile[] {
if (_angularFormsDts !== null) {
return _angularFormsDts;
}

const directory = resolveFromRunfiles('_main/packages/forms/npm_package');
const dtsFiles = globSync('**/*.d.ts', {cwd: directory});

return (_angularFormsDts = ['package.json', ...dtsFiles].map((fileName) => ({
name: absoluteFrom(`/node_modules/@angular/forms/${fileName}`),
contents: readFileSync(path.join(directory, fileName), 'utf8'),
})));
}

export function angularAnimationsDts(): TestFile {
return {
name: absoluteFrom('/node_modules/@angular/animations/index.d.ts'),
Expand Down Expand Up @@ -533,12 +548,20 @@ export function setup(
parseOptions?: ParseTemplateOptions;
referenceEmitter?: ReferenceEmitter;
} = {},
load: {
forms?: boolean;
} = {},
): {
templateTypeChecker: TemplateTypeChecker;
program: ts.Program;
programStrategy: TsCreateProgramDriver;
} {
const files = [typescriptLibDts(), ...angularCoreDtsFiles(), angularAnimationsDts()];
const files = [
typescriptLibDts(),
...angularCoreDtsFiles(),
angularAnimationsDts(),
...(load.forms ? angularFormsDtsFiles() : []),
];
const fakeMetadataRegistry = new Map();
const shims = new Map<AbsoluteFsPath, AbsoluteFsPath>();

Expand Down