Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix(compiler-cli): catch function instance properties in interpolated signal diagnostic (#54325)

Browse source 
Currently, the following template compiles without error, even if the signal is not properly called:

```
<div>{{ mySignal.name }}</div>
```

This is because `name` is one of the instance properties of Function (see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function#instance_properties).

The interpolated signal diagnostic is now extended to catch such issues.

PR Close #54325
This commit is contained in:
cexbrayat 2024-02-07 22:23:26 +01:00 committed by Dylan Hunn
parent 684290976b
commit f578889ca2
2 changed files with 134 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked/index.ts
View file

@ -22,6 +22,15 @@ const SIGNAL_FNS = new Set([
'ModelSignal',
]);
/** Names of known signal instance properties. */
const SIGNAL_INSTANCE_PROPERTIES = new Set(['set', 'update', 'asReadonly']);
/**
* Names of known function instance properties.
* https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function#instance_properties
*/
const FUNCTION_INSTANCE_PROPERTIES = new Set(['name', 'length', 'prototype']);
/**
* Ensures Signals are invoked when used in template interpolations.
*/
@ -53,12 +62,20 @@ function isSignal(symbol: ts.Symbol|undefined): boolean {
});
}
function isFunctionInstanceProperty(name: string): boolean {
return FUNCTION_INSTANCE_PROPERTIES.has(name);
}
function isSignalInstanceProperty(name: string): boolean {
return SIGNAL_INSTANCE_PROPERTIES.has(name);
}
function buildDiagnosticForSignal(
ctx: TemplateContext<ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED>, node: PropertyRead,
component: ts.ClassDeclaration):
Array<NgTemplateDiagnostic<ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED>> {
// check for `{{ mySignal }}`
const symbol = ctx.templateTypeChecker.getSymbolOfNode(node, component);
if (symbol?.kind === SymbolKind.Expression &&
(isSignal(symbol.tsType.symbol) || isSignal(symbol.tsType.aliasSymbol))) {
const templateMapping =
@ -68,6 +85,25 @@ function buildDiagnosticForSignal(
return [diagnostic];
}
// check for `{{ mySignal.name }}` or `{{ mySignal.length }}` or `{{ mySignal.prototype }}`
// as these are the names of instance properties of Function, the compiler does _not_ throw an
// error.
// We also check for `{{ mySignal.set }}` or `{{ mySignal.update }}` or
// `{{ mySignal.asReadonly }}` as these are the names of instance properties of Signal
const symbolOfReceiver = ctx.templateTypeChecker.getSymbolOfNode(node.receiver, component);
if ((isFunctionInstanceProperty(node.name) || isSignalInstanceProperty(node.name)) &&
symbolOfReceiver?.kind === SymbolKind.Expression &&
(isSignal(symbolOfReceiver.tsType.symbol) || isSignal(symbolOfReceiver.tsType.aliasSymbol))) {
const templateMapping =
ctx.templateTypeChecker.getTemplateMappingAtTcbLocation(symbolOfReceiver.tcbLocation)!;
const errorString =
`${(node.receiver as PropertyRead).name} is a function and should be invoked: ${
(node.receiver as PropertyRead).name}()`;
const diagnostic = ctx.makeTemplateDiagnostic(templateMapping.span, errorString);
return [diagnostic];
}
return [];
}

102
packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/interpolated_signal_not_invoked/interpolated_signal_not_invoked_spec.ts
View file

@ -371,7 +371,7 @@ runInEachFileSystem(() => {
'TestCmp': `<div id="{{mySignal}}"></div>`,
},
source: `
import {signal, Signal, computed} from '@angular/core';
import {signal} from '@angular/core';
export class TestCmp {
mySignal = signal<number>(0);
@ -399,7 +399,7 @@ runInEachFileSystem(() => {
'TestCmp': `<div id="{{mySignal()}}"></div>`,
},
source: `
import {signal, Signal, computed} from '@angular/core';
import {signal} from '@angular/core';
export class TestCmp {
mySignal = signal<number>(0);
@ -424,7 +424,7 @@ runInEachFileSystem(() => {
'TestCmp': `<div attr.id="my-{{mySignal}}-item"></div>`,
},
source: `
import {signal, Signal, computed} from '@angular/core';
import {signal} from '@angular/core';
export class TestCmp {
mySignal = signal<number>(0);
@ -453,7 +453,7 @@ runInEachFileSystem(() => {
'TestCmp': `<div attr.id="my-{{mySignal()}}-item"></div>`,
},
source: `
import {signal, Signal, computed} from '@angular/core';
import {signal} from '@angular/core';
export class TestCmp {
mySignal = signal<number>(0);
@ -479,7 +479,7 @@ runInEachFileSystem(() => {
'TestCmp': `<div id="{{myObject.myObject2.myNestedSignal}}"></div>`,
},
source: `
import {signal, Signal, computed} from '@angular/core';
import {signal} from '@angular/core';
export class TestCmp {
myObject = {myObject2: {myNestedSignal: signal<number>(0)}};
@ -553,4 +553,96 @@ runInEachFileSystem(() => {
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
['name', 'length', 'prototype', 'set', 'update', 'asReadonly'].forEach(
functionInstanceProperty => {
it(`should produce a warning when a property named '${
functionInstanceProperty}' of a not invoked signal is used in interpolation`,
() => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
{
fileName,
templates: {
'TestCmp': `<div>{{myObject.mySignal.${functionInstanceProperty}}}</div>`,
},
source: `
import {signal} from '@angular/core';
export class TestCmp {
myObject = { mySignal: signal<{ ${functionInstanceProperty}: string }>({ ${
functionInstanceProperty}: 'foo' }) };
}`,
},
]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [interpolatedSignalFactory], {}
/* options */
);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED));
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`mySignal`);
});
it(`should not produce a warning when a property named ${
functionInstanceProperty} of an invoked signal is used in interpolation`,
() => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
{
fileName,
templates: {
'TestCmp': `<div>{{mySignal().${functionInstanceProperty}}}</div>`,
},
source: `
import {signal} from '@angular/core';
export class TestCmp {
mySignal = signal<{ ${functionInstanceProperty}: string }>({ ${
functionInstanceProperty}: 'foo' });
}`,
},
]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [interpolatedSignalFactory], {}
/* options */
);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
it(`should not produce a warning when a property named ${
functionInstanceProperty} of an object is used in interpolation`,
() => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
{
fileName,
templates: {
'TestCmp': `<div>{{myObject.${functionInstanceProperty}}}</div>`,
},
source: `
import {signal} from '@angular/core';
export class TestCmp {
myObject = { ${functionInstanceProperty}: 'foo' };
}`,
},
]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [interpolatedSignalFactory], {}
/* options */
);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
});
});