refactor(compiler-cli): Add test to document use-case for invalid pipe argument nullish coallesce (#43419)

Adds a test to the nullish coalescing diagnostic check to serve as
self-documentation on how it works with nullish coalescing on pipes that
are often misconfigured.

This also removes that non null assertion operator, which is incorrect
because there _are_ situations where a symbol cannot be retrieved.

PR Close #43419
This commit is contained in:
Andrew Scott
2021-09-09 17:47:59 -07:00
committed by Jessica Janiuk
parent 2028c3933f
commit a360f4b0fc
2 changed files with 84 additions and 2 deletions
@@ -27,8 +27,8 @@ export class NullishCoalescingNotNullableCheck extends
NgTemplateDiagnostic<ErrorCode.NULLISH_COALESCING_NOT_NULLABLE>[] {
if (!(node instanceof Binary) || node.operation !== '??') return [];
const symbolLeft = ctx.templateTypeChecker.getSymbolOfNode(node.left, component)!;
if (symbolLeft.kind !== SymbolKind.Expression) {
const symbolLeft = ctx.templateTypeChecker.getSymbolOfNode(node.left, component);
if (symbolLeft === null || symbolLeft.kind !== SymbolKind.Expression) {
return [];
}
const typeLeft = symbolLeft.tsType;
@@ -71,5 +71,87 @@ runInEachFileSystem(() => {
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
it('warns for pipe arguments which are likely configured incorrectly (?? operates on "format" here)',
() => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
fileName,
templates: {
'TestCmp': `{{ 123 | date: 'format' ?? 'invalid date' }}`,
},
source: `
export class TestCmp { var1: string | undefined = "text"; }
export class DatePipe {
transform(value: string, format: string): string[] {
}
`,
declarations: [{
type: 'pipe',
name: 'DatePipe',
pipeName: 'date',
}],
}]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(),
[new NullishCoalescingNotNullableCheck()]);
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.NULLISH_COALESCING_NOT_NULLABLE));
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`'format' ?? 'invalid date'`);
});
it('does not warn for pipe arguments when parens are used', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
{
fileName,
templates: {
'TestCmp': `{{ (123 | date: 'format') ?? 'invalid date' }}`,
},
source: `
export class TestCmp { var1: string | undefined = "text"; }
export class DatePipe {
transform(value: string, format: string): string[] {
}
`,
declarations: [{
type: 'pipe',
name: 'DatePipe',
pipeName: 'date',
}],
},
]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [new NullishCoalescingNotNullableCheck()]);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
it('does not blow up when there is no symbol for the LHS of the operator', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
{
fileName,
templates: {
'TestCmp': `{{ (123 | doesNotExist) ?? 'invalid date' }}`,
},
source: `
export class TestCmp { var1: string | undefined = "text"; }
`,
},
]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [new NullishCoalescingNotNullableCheck()]);
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
});
});