diff --git a/aio/content/guide/template-typecheck.md b/aio/content/guide/template-typecheck.md index 5ef3229329..d5612bb76a 100644 --- a/aio/content/guide/template-typecheck.md +++ b/aio/content/guide/template-typecheck.md @@ -114,6 +114,7 @@ In case of a false positive like these, there are a few options: |Strictness flag|Effect| |-|-| |`strictInputTypes`|Whether the assignability of a binding expression to the `@Input()` field is checked. Also affects the inference of directive generic types. | +|`strictInputAccessModifiers`|Whether access modifiers such as `private`/`protected`/`readonly` are honored when assigning a binding expression to an `@Input()`. If disabled, the access modifiers of the `@Input` are ignored; only the type is checked.| |`strictNullInputTypes`|Whether `strictNullChecks` is honored when checking `@Input()` bindings (per `strictInputTypes`). Turning this off can be useful when using a library that was not built with `strictNullChecks` in mind.| |`strictAttributeTypes`|Whether to check `@Input()` bindings that are made using text attributes (for example, `` vs ``). |`strictSafeNavigationTypes`|Whether the return type of safe navigation operations (for example, `user?.name`) will be correctly inferred based on the type of `user`). If disabled, `user?.name` will be of type `any`. diff --git a/goldens/public-api/compiler-cli/compiler_options.d.ts b/goldens/public-api/compiler-cli/compiler_options.d.ts index 92723d3050..2c462d513a 100644 --- a/goldens/public-api/compiler-cli/compiler_options.d.ts +++ b/goldens/public-api/compiler-cli/compiler_options.d.ts @@ -35,6 +35,7 @@ export interface StrictTemplateOptions { strictContextGenerics?: boolean; strictDomEventTypes?: boolean; strictDomLocalRefTypes?: boolean; + strictInputAccessModifiers?: boolean; strictInputTypes?: boolean; strictLiteralTypes?: boolean; strictNullInputTypes?: boolean; diff --git a/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts b/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts index bb682ce21f..98b41f8f07 100644 --- a/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts +++ b/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts @@ -147,6 +147,18 @@ export interface StrictTemplateOptions { */ strictInputTypes?: boolean; + /** + * Whether to check if the input binding attempts to assign to a restricted field (readonly, + * private, or protected) on the directive/component. + * + * Defaults to `false`, even if "fullTemplateTypeCheck", "strictTemplates" and/or + * "strictInputTypes" is set. Note that if `strictInputTypes` is not set, or set to `false`, this + * flag has no effect. + * + * Tracking issue for enabling this by default: https://github.com/angular/angular/issues/38400 + */ + strictInputAccessModifiers?: boolean; + /** * Whether to use strict null types for input bindings for directives. * diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 6e7cd3ff04..2aed139c01 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -415,6 +415,7 @@ export class NgCompiler { checkQueries: false, checkTemplateBodies: true, checkTypeOfInputBindings: strictTemplates, + honorAccessModifiersForInputBindings: false, strictNullInputBindings: strictTemplates, checkTypeOfAttributes: strictTemplates, // Even in full template type-checking mode, DOM binding checks are not quite ready yet. @@ -442,6 +443,7 @@ export class NgCompiler { checkTemplateBodies: false, checkTypeOfInputBindings: false, strictNullInputBindings: false, + honorAccessModifiersForInputBindings: false, checkTypeOfAttributes: false, checkTypeOfDomBindings: false, checkTypeOfOutputEvents: false, @@ -462,6 +464,10 @@ export class NgCompiler { typeCheckingConfig.checkTypeOfInputBindings = this.options.strictInputTypes; typeCheckingConfig.applyTemplateContextGuards = this.options.strictInputTypes; } + if (this.options.strictInputAccessModifiers !== undefined) { + typeCheckingConfig.honorAccessModifiersForInputBindings = + this.options.strictInputAccessModifiers; + } if (this.options.strictNullInputTypes !== undefined) { typeCheckingConfig.strictNullInputBindings = this.options.strictNullInputTypes; } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts index a9d58de7c7..8009207f50 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts @@ -60,6 +60,13 @@ export interface DirectiveTypeCheckMeta { */ restrictedInputFields: Set; + /** + * The set of input fields which are declared as string literal members in the Directive's class. + * We need to track these separately because these fields may not be valid JS identifiers so + * we cannot use them with property access expressions when assigning inputs. + */ + stringLiteralInputFields: Set; + /** * The set of input fields which do not have corresponding members in the Directive's class. */ diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts index ba775783e1..455d103d7d 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts @@ -27,9 +27,10 @@ export function flattenInheritedDirectiveMetadata( let inputs: {[key: string]: string|[string, string]} = {}; let outputs: {[key: string]: string} = {}; - let coercedInputFields = new Set(); - let undeclaredInputFields = new Set(); - let restrictedInputFields = new Set(); + const coercedInputFields = new Set(); + const undeclaredInputFields = new Set(); + const restrictedInputFields = new Set(); + const stringLiteralInputFields = new Set(); let isDynamic = false; const addMetadata = (meta: DirectiveMeta): void => { @@ -56,6 +57,9 @@ export function flattenInheritedDirectiveMetadata( for (const restrictedInputField of meta.restrictedInputFields) { restrictedInputFields.add(restrictedInputField); } + for (const field of meta.stringLiteralInputFields) { + stringLiteralInputFields.add(field); + } }; addMetadata(topMeta); @@ -67,6 +71,7 @@ export function flattenInheritedDirectiveMetadata( coercedInputFields, undeclaredInputFields, restrictedInputFields, + stringLiteralInputFields, baseClass: isDynamic ? 'dynamic' : null, }; } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts index 4cf0e5eabd..dbb7e77e94 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts @@ -98,15 +98,21 @@ export function extractDirectiveTypeCheckMeta( .filter((inputName): inputName is string => inputName !== null)); const restrictedInputFields = new Set(); + const stringLiteralInputFields = new Set(); const undeclaredInputFields = new Set(); for (const fieldName of Object.keys(inputs)) { const field = members.find(member => member.name === fieldName); if (field === undefined || field.node === null) { undeclaredInputFields.add(fieldName); - } else if (isRestricted(field.node)) { + continue; + } + if (isRestricted(field.node)) { restrictedInputFields.add(fieldName); } + if (field.nameNode !== null && ts.isStringLiteral(field.nameNode)) { + stringLiteralInputFields.add(fieldName); + } } const arity = reflector.getGenericArityOfClass(node); @@ -116,6 +122,7 @@ export function extractDirectiveTypeCheckMeta( ngTemplateGuards, coercedInputFields, restrictedInputFields, + stringLiteralInputFields, undeclaredInputFields, isGeneric: arity !== null && arity > 0, }; diff --git a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts index 60302988a1..f8b12d5fc0 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts @@ -244,6 +244,7 @@ function fakeDirective(ref: Reference): DirectiveMeta { ngTemplateGuards: [], coercedInputFields: new Set(), restrictedInputFields: new Set(), + stringLiteralInputFields: new Set(), undeclaredInputFields: new Set(), isGeneric: false, baseClass: null, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts index f0e241144a..658260fbf1 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts @@ -90,6 +90,14 @@ export interface TypeCheckingConfig { */ checkTypeOfInputBindings: boolean; + /** + * Whether to honor the access modifiers on input bindings for the component/directive. + * + * If a template binding attempts to assign to an input that is private/protected/readonly, + * this will produce errors when enabled but will not when disabled. + */ + honorAccessModifiersForInputBindings: boolean; + /** * Whether to use strict null types for input bindings for directives. * diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index d2d56de68c..fa46e7f1db 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -453,8 +453,13 @@ class TcbDirectiveInputsOp extends TcbOp { // declared in a `@Directive` or `@Component` decorator's `inputs` property) there is no // assignment target available, so this field is skipped. continue; - } else if (this.dir.restrictedInputFields.has(fieldName)) { - // To ignore errors, assign to temp variable with type of the field + } else if ( + !this.tcb.env.config.honorAccessModifiersForInputBindings && + this.dir.restrictedInputFields.has(fieldName)) { + // If strict checking of access modifiers is disabled and the field is restricted + // (i.e. private/protected/readonly), generate an assignment into a temporary variable + // that has the type of the field. This achieves type-checking but circumvents the access + // modifiers. const id = this.tcb.allocateId(); const dirTypeRef = this.tcb.env.referenceType(this.dir.ref); if (!ts.isTypeReferenceNode(dirTypeRef)) { @@ -464,23 +469,16 @@ class TcbDirectiveInputsOp extends TcbOp { const type = ts.createIndexedAccessTypeNode( ts.createTypeQueryNode(dirId as ts.Identifier), ts.createLiteralTypeNode(ts.createStringLiteral(fieldName))); - const temp = tsCreateVariable(id, ts.createNonNullExpression(ts.createNull()), type); - addParseSpanInfo(temp, input.attribute.sourceSpan); + const temp = tsDeclareVariable(id, type); this.scope.addStatement(temp); target = id; - - // TODO: To get errors assign directly to the fields on the instance, using dot access - // when possible - } else { - // Otherwise, a declaration exists in which case the `dir["fieldName"]` syntax is used - // as assignment target. An element access is used instead of a property access to - // support input names that are not valid JavaScript identifiers. Additionally, using - // element access syntax does not produce - // TS2341 "Property $prop is private and only accessible within class $class." nor - // TS2445 "Property $prop is protected and only accessible within class $class and its - // subclasses." - target = ts.createElementAccess(dirId, ts.createStringLiteral(fieldName)); + // To get errors assign directly to the fields on the instance, using property access + // when possible. String literal fields may not be valid JS identifiers so we use + // literal element access instead for those cases. + target = this.dir.stringLiteralInputFields.has(fieldName) ? + ts.createElementAccess(dirId, ts.createStringLiteral(fieldName)) : + ts.createPropertyAccess(dirId, ts.createIdentifier(fieldName)); } // Finally the assignment is extended by assigning it into the target expression. diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index f1e285c25a..37f66200f1 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -157,6 +157,7 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { checkQueries: false, checkTemplateBodies: true, checkTypeOfInputBindings: true, + honorAccessModifiersForInputBindings: true, strictNullInputBindings: true, checkTypeOfAttributes: true, // Feature is still in development. @@ -178,10 +179,11 @@ export type TestDirective = Partial>>&{ + 'ref'|'coercedInputFields'|'restrictedInputFields'|'stringLiteralInputFields'| + 'undeclaredInputFields'>>>&{ selector: string, name: string, file?: AbsoluteFsPath, type: 'directive', coercedInputFields?: string[], restrictedInputFields?: string[], - undeclaredInputFields?: string[], isGeneric?: boolean; + stringLiteralInputFields?: string[], undeclaredInputFields?: string[], isGeneric?: boolean; }; export type TestPipe = { name: string, @@ -212,6 +214,7 @@ export function tcb( applyTemplateContextGuards: true, checkQueries: false, checkTypeOfInputBindings: true, + honorAccessModifiersForInputBindings: false, strictNullInputBindings: true, checkTypeOfAttributes: true, checkTypeOfDomBindings: false, @@ -420,6 +423,7 @@ function prepareDeclarations( ngTemplateGuards: decl.ngTemplateGuards || [], coercedInputFields: new Set(decl.coercedInputFields || []), restrictedInputFields: new Set(decl.restrictedInputFields || []), + stringLiteralInputFields: new Set(decl.stringLiteralInputFields || []), undeclaredInputFields: new Set(decl.undeclaredInputFields || []), isGeneric: decl.isGeneric ?? false, outputs: decl.outputs || {}, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index d4a6b14a55..8a11d6d87c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -55,7 +55,7 @@ describe('type check blocks', () => { selector: '[dir]', inputs: {inputA: 'inputA'}, }]; - expect(tcb(TEMPLATE, DIRECTIVES)).toContain('_t2: DirA = (null!); _t2["inputA"] = ("value");'); + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('_t2: DirA = (null!); _t2.inputA = ("value");'); }); it('should handle multiple bindings to the same property', () => { @@ -67,8 +67,8 @@ describe('type check blocks', () => { inputs: {inputA: 'inputA'}, }]; const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('_t2["inputA"] = (1);'); - expect(block).toContain('_t2["inputA"] = (2);'); + expect(block).toContain('_t2.inputA = (1);'); + expect(block).toContain('_t2.inputA = (2);'); }); it('should handle empty bindings', () => { @@ -79,7 +79,7 @@ describe('type check blocks', () => { selector: '[dir-a]', inputs: {inputA: 'inputA'}, }]; - expect(tcb(TEMPLATE, DIRECTIVES)).toContain('_t2["inputA"] = (undefined);'); + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('_t2.inputA = (undefined);'); }); it('should handle bindings without value', () => { @@ -90,7 +90,7 @@ describe('type check blocks', () => { selector: '[dir-a]', inputs: {inputA: 'inputA'}, }]; - expect(tcb(TEMPLATE, DIRECTIVES)).toContain('_t2["inputA"] = (undefined);'); + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('_t2.inputA = (undefined);'); }); it('should handle implicit vars on ng-template', () => { @@ -322,7 +322,7 @@ describe('type check blocks', () => { expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( 'var _t2: Dir = (null!); ' + - '_t2["input"] = (_t2);'); + '_t2.input = (_t2);'); }); it('should generate circular references between two directives correctly', () => { @@ -350,9 +350,9 @@ describe('type check blocks', () => { .toContain( 'var _t2: DirA = (null!); ' + 'var _t3: DirB = (null!); ' + - '_t2["inputA"] = (_t3); ' + + '_t2.inputA = (_t3); ' + 'var _t4 = document.createElement("div"); ' + - '_t3["inputA"] = (_t2);'); + '_t3.inputA = (_t2);'); }); it('should handle undeclared properties', () => { @@ -372,7 +372,7 @@ describe('type check blocks', () => { '(((ctx).foo)); '); }); - it('should handle restricted properties', () => { + it('should assign restricted properties to temp variables by default', () => { const TEMPLATE = `
`; const DIRECTIVES: TestDeclaration[] = [{ type: 'directive', @@ -390,6 +390,24 @@ describe('type check blocks', () => { '_t3 = (((ctx).foo)); '); }); + it('should assign properties via element access for field names that are not JS identifiers', + () => { + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + 'some-input.xs': 'inputA', + }, + stringLiteralInputFields: ['some-input.xs'], + }]; + const block = tcb(TEMPLATE, DIRECTIVES); + expect(block).toContain( + 'var _t2: Dir = (null!); ' + + '_t2["some-input.xs"] = (((ctx).foo)); '); + }); + it('should handle a single property bound to multiple fields', () => { const TEMPLATE = `
`; const DIRECTIVES: TestDeclaration[] = [{ @@ -404,7 +422,7 @@ describe('type check blocks', () => { expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( 'var _t2: Dir = (null!); ' + - '_t2["field2"] = _t2["field1"] = (((ctx).foo));'); + '_t2.field2 = _t2.field1 = (((ctx).foo));'); }); it('should handle a single property bound to multiple fields, where one of them is coerced', @@ -424,7 +442,7 @@ describe('type check blocks', () => { .toContain( 'var _t2: Dir = (null!); ' + 'var _t3: typeof Dir.ngAcceptInputType_field1 = (null!); ' + - '_t2["field2"] = _t3 = (((ctx).foo));'); + '_t2.field2 = _t3 = (((ctx).foo));'); }); it('should handle a single property bound to multiple fields, where one of them is undeclared', @@ -443,7 +461,7 @@ describe('type check blocks', () => { expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( 'var _t2: Dir = (null!); ' + - '_t2["field2"] = (((ctx).foo));'); + '_t2.field2 = (((ctx).foo));'); }); it('should use coercion types if declared', () => { @@ -590,6 +608,7 @@ describe('type check blocks', () => { checkQueries: false, checkTemplateBodies: true, checkTypeOfInputBindings: true, + honorAccessModifiersForInputBindings: false, strictNullInputBindings: true, checkTypeOfAttributes: true, checkTypeOfDomBindings: false, @@ -639,14 +658,14 @@ describe('type check blocks', () => { it('should include null and undefined when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('_t2["dirInput"] = (((ctx).a));'); + expect(block).toContain('_t2.dirInput = (((ctx).a));'); expect(block).toContain('((ctx).b);'); }); it('should use the non-null assertion operator when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, strictNullInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('_t2["dirInput"] = (((ctx).a)!);'); + expect(block).toContain('_t2.dirInput = (((ctx).a)!);'); expect(block).toContain('((ctx).b)!;'); }); }); @@ -655,7 +674,7 @@ describe('type check blocks', () => { it('should check types of bindings when enabled', () => { const TEMPLATE = `
`; const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('_t2["dirInput"] = (((ctx).a));'); + expect(block).toContain('_t2.dirInput = (((ctx).a));'); expect(block).toContain('((ctx).b);'); }); @@ -664,7 +683,7 @@ describe('type check blocks', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('_t2["dirInput"] = ((((ctx).a) as any));'); + expect(block).toContain('_t2.dirInput = ((((ctx).a) as any));'); expect(block).toContain('(((ctx).b) as any);'); }); @@ -673,7 +692,7 @@ describe('type check blocks', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('_t2["dirInput"] = ((((((ctx).a)) === (((ctx).b))) as any));'); + expect(block).toContain('_t2.dirInput = ((((((ctx).a)) === (((ctx).b))) as any));'); }); }); @@ -793,9 +812,9 @@ describe('type check blocks', () => { it('should assign string value to the input when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('_t2["disabled"] = ("");'); - expect(block).toContain('_t2["cols"] = ("3");'); - expect(block).toContain('_t2["rows"] = (2);'); + expect(block).toContain('_t2.disabled = ("");'); + expect(block).toContain('_t2.cols = ("3");'); + expect(block).toContain('_t2.rows = (2);'); }); it('should use any for attributes but still check bound attributes when disabled', () => { @@ -803,7 +822,7 @@ describe('type check blocks', () => { const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); expect(block).not.toContain('"disabled"'); expect(block).not.toContain('"cols"'); - expect(block).toContain('_t2["rows"] = (2);'); + expect(block).toContain('_t2.rows = (2);'); }); }); @@ -873,5 +892,47 @@ describe('type check blocks', () => { expect(block).toContain('function Test_TCB(ctx: Test)'); }); }); + + describe('config.checkAccessModifiersForInputBindings', () => { + const TEMPLATE = `
`; + + it('should assign restricted properties via element access for field names that are not JS identifiers', + () => { + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + 'some-input.xs': 'inputA', + }, + restrictedInputFields: ['some-input.xs'], + stringLiteralInputFields: ['some-input.xs'], + }]; + const enableChecks: + TypeCheckingConfig = {...BASE_CONFIG, honorAccessModifiersForInputBindings: true}; + const block = tcb(TEMPLATE, DIRECTIVES, enableChecks); + expect(block).toContain( + 'var _t2: Dir = (null!); ' + + '_t2["some-input.xs"] = (((ctx).foo)); '); + }); + + it('should assign restricted properties via property access', () => { + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + fieldA: 'inputA', + }, + restrictedInputFields: ['fieldA'] + }]; + const enableChecks: + TypeCheckingConfig = {...BASE_CONFIG, honorAccessModifiersForInputBindings: true}; + const block = tcb(TEMPLATE, DIRECTIVES, enableChecks); + expect(block).toContain( + 'var _t2: Dir = (null!); ' + + '_t2.fieldA = (((ctx).foo)); '); + }); + }); }); }); diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 5646850c22..dc8dd1af02 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -1577,14 +1577,7 @@ export declare class AnimationEvent { } `; - describe('with strict inputs', () => { - beforeEach(() => { - env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true}); - }); - - it('should not produce diagnostics for correct inputs which assign to readonly, private, or protected fields', - () => { - env.write('test.ts', ` + const correctTypeInputsToRestrictedFields = ` import {Component, NgModule, Input, Directive} from '@angular/core'; @Component({ @@ -1601,14 +1594,9 @@ export declare class AnimationEvent { declarations: [FooCmp, TestDir], }) export class FooModule {} - `); - const diags = env.driveDiagnostics(); - expect(diags.length).toBe(0); - }); + `; - it('should not produce diagnostics for correct inputs which assign to readonly, private, or protected fields inherited from a base class', - () => { - env.write('test.ts', ` + const correctInputsToRestrictedFieldsFromBaseClass = ` import {Component, NgModule, Input, Directive} from '@angular/core'; @Component({ @@ -1629,7 +1617,85 @@ export declare class AnimationEvent { declarations: [FooCmp, ChildDir], }) export class FooModule {} - `); + `; + describe('with strictInputAccessModifiers', () => { + beforeEach(() => { + env.tsconfig({ + fullTemplateTypeCheck: true, + strictInputTypes: true, + strictInputAccessModifiers: true + }); + }); + + it('should produce diagnostics for inputs which assign to readonly, private, and protected fields', + () => { + env.write('test.ts', correctTypeInputsToRestrictedFields); + expectIllegalAssignmentErrors(env.driveDiagnostics()); + }); + + it('should produce diagnostics for inputs which assign to readonly, private, and protected fields inherited from a base class', + () => { + env.write('test.ts', correctInputsToRestrictedFieldsFromBaseClass); + expectIllegalAssignmentErrors(env.driveDiagnostics()); + }); + + function expectIllegalAssignmentErrors(diags: ReadonlyArray) { + expect(diags.length).toBe(3); + const actualMessages = diags.map(d => d.messageText).sort(); + const expectedMessages = [ + `Property 'protectedField' is protected and only accessible within class 'TestDir' and its subclasses.`, + `Property 'privateField' is private and only accessible within class 'TestDir'.`, + `Cannot assign to 'readonlyField' because it is a read-only property.`, + ].sort(); + expect(actualMessages).toEqual(expectedMessages); + } + + it('should report invalid type assignment when field name is not a valid JS identifier', + () => { + env.write('test.ts', ` + import {Component, NgModule, Input, Directive} from '@angular/core'; + + @Component({ + selector: 'blah', + template: '
', + }) + export class FooCmp { + value = 5; + } + + @Directive({selector: '[dir]'}) + export class TestDir { + @Input() + private 'private-input.xs'!: string; + } + + @NgModule({ + declarations: [FooCmp, TestDir], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText) + .toEqual(`Type 'number' is not assignable to type 'string'.`); + }); + }); + + describe('with strict inputs', () => { + beforeEach(() => { + env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true}); + }); + + it('should not produce diagnostics for correct inputs which assign to readonly, private, or protected fields', + () => { + env.write('test.ts', correctTypeInputsToRestrictedFields); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should not produce diagnostics for correct inputs which assign to readonly, private, or protected fields inherited from a base class', + () => { + env.write('test.ts', correctInputsToRestrictedFieldsFromBaseClass); const diags = env.driveDiagnostics(); expect(diags.length).toBe(0); });