refactor: add override keyword to members implementing abstract declarations (#42512)

In combination with the TS `noImplicitOverride` compatibility changes,
we also want to follow the best-practice of adding `override` to
members which are implemented as part of abstract classes. This
commit fixes all instances which will be flagged as part of the
custom `no-implicit-override-abstract` TSLint rule.

PR Close #42512
This commit is contained in:
Paul Gschwendtner
2021-07-07 19:58:22 +02:00
committed by Andrew Kushnir
parent 04642e7985
commit b5ab7aff43
113 changed files with 517 additions and 511 deletions
@@ -16,11 +16,11 @@ import {DependencyHostBase} from './dependency_host';
* Helper functions for computing dependencies.
*/
export class CommonJsDependencyHost extends DependencyHostBase {
protected canSkipFile(fileContents: string): boolean {
protected override canSkipFile(fileContents: string): boolean {
return !hasRequireCalls(fileContents);
}
protected extractImports(file: AbsoluteFsPath, fileContents: string): Set<string> {
protected override extractImports(file: AbsoluteFsPath, fileContents: string): Set<string> {
// Parse the source into a TypeScript AST and then walk it looking for imports and re-exports.
const sf =
ts.createSourceFile(file, fileContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
@@ -23,7 +23,7 @@ export class EsmDependencyHost extends DependencyHostBase {
// It has no relevance to capturing imports.
private scanner = ts.createScanner(ts.ScriptTarget.Latest, /* skipTrivia */ true);
protected canSkipFile(fileContents: string): boolean {
protected override canSkipFile(fileContents: string): boolean {
return !hasImportOrReexportStatements(fileContents);
}
@@ -43,7 +43,7 @@ export class EsmDependencyHost extends DependencyHostBase {
* Specifically, backticked strings are particularly challenging since it is possible
* to recursively nest backticks and TypeScript expressions within each other.
*/
protected extractImports(file: AbsoluteFsPath, fileContents: string): Set<string> {
protected override extractImports(file: AbsoluteFsPath, fileContents: string): Set<string> {
const imports = new Set<string>();
const templateStack: ts.SyntaxKind[] = [];
let lastToken: ts.SyntaxKind = ts.SyntaxKind.Unknown;
@@ -17,11 +17,11 @@ import {DependencyHostBase} from './dependency_host';
* Helper functions for computing dependencies.
*/
export class UmdDependencyHost extends DependencyHostBase {
protected canSkipFile(fileContents: string): boolean {
protected override canSkipFile(fileContents: string): boolean {
return !hasRequireCalls(fileContents);
}
protected extractImports(file: AbsoluteFsPath, fileContents: string): Set<string> {
protected override extractImports(file: AbsoluteFsPath, fileContents: string): Set<string> {
// Parse the source into a TypeScript AST and then walk it looking for imports and re-exports.
const sf =
ts.createSourceFile(file, fileContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
@@ -45,7 +45,7 @@ export class ProgramBasedEntryPointFinder extends TracingEntryPointFinder {
* Return an array containing the external import paths that were extracted from the source-files
* of the program defined by the tsconfig.json.
*/
protected getInitialEntryPointPaths(): AbsoluteFsPath[] {
protected override getInitialEntryPointPaths(): AbsoluteFsPath[] {
const moduleResolver = new ModuleResolver(this.fs, this.pathMappings, ['', '.ts', '/index.ts']);
const host = new EsmDependencyHost(this.fs, moduleResolver);
const dependencies = createDependencyInfo();
@@ -71,7 +71,8 @@ export class ProgramBasedEntryPointFinder extends TracingEntryPointFinder {
* @returns the entry-point and its dependencies or `null` if the entry-point is not compiled by
* Angular or cannot be determined.
*/
protected getEntryPointWithDeps(entryPointPath: AbsoluteFsPath): EntryPointWithDependencies|null {
protected override getEntryPointWithDeps(entryPointPath: AbsoluteFsPath):
EntryPointWithDependencies|null {
const entryPoints = this.findOrLoadEntryPoints();
if (!entryPoints.has(entryPointPath)) {
return null;
@@ -83,7 +83,7 @@ export class TargetedEntryPointFinder extends TracingEntryPointFinder {
/**
* Return an array containing the `targetPath` from which to start the trace.
*/
protected getInitialEntryPointPaths(): AbsoluteFsPath[] {
protected override getInitialEntryPointPaths(): AbsoluteFsPath[] {
return [this.targetPath];
}
@@ -97,7 +97,8 @@ export class TargetedEntryPointFinder extends TracingEntryPointFinder {
* @returns the entry-point and its dependencies or `null` if the entry-point is not compiled by
* Angular or cannot be determined.
*/
protected getEntryPointWithDeps(entryPointPath: AbsoluteFsPath): EntryPointWithDependencies|null {
protected override getEntryPointWithDeps(entryPointPath: AbsoluteFsPath):
EntryPointWithDependencies|null {
const packagePath = this.computePackagePath(entryPointPath);
const entryPoint =
getEntryPointInfo(this.fs, this.config, this.logger, packagePath, entryPointPath);
@@ -27,7 +27,7 @@ export class ParallelTaskQueue extends BaseTaskQueue {
this.blockedTasks = getBlockedTasks(dependencies);
}
computeNextTask(): Task|null {
override computeNextTask(): Task|null {
// Look for the first available (i.e. not blocked) task.
// (NOTE: Since tasks are sorted by priority, the first available one is the best choice.)
const nextTaskIdx = this.tasks.findIndex(task => !this.blockedTasks.has(task));
@@ -17,7 +17,7 @@ import {BaseTaskQueue} from './base_task_queue';
* before requesting the next one.
*/
export class SerialTaskQueue extends BaseTaskQueue {
computeNextTask(): Task|null {
override computeNextTask(): Task|null {
const nextTask = this.tasks.shift() || null;
if (nextTask) {
@@ -64,7 +64,7 @@ export class DirectiveSymbol extends SemanticSymbol {
super(decl);
}
isPublicApiAffected(previousSymbol: SemanticSymbol): boolean {
override isPublicApiAffected(previousSymbol: SemanticSymbol): boolean {
// Note: since components and directives have exactly the same items contributing to their
// public API, it is okay for a directive to change into a component and vice versa without
// the API being affected.
@@ -83,7 +83,7 @@ export class DirectiveSymbol extends SemanticSymbol {
!isArrayEqual(this.exportAs, previousSymbol.exportAs);
}
isTypeCheckApiAffected(previousSymbol: SemanticSymbol): boolean {
override isTypeCheckApiAffected(previousSymbol: SemanticSymbol): boolean {
// If the public API of the directive has changed, then so has its type-check API.
if (this.isPublicApiAffected(previousSymbol)) {
return true;
@@ -58,7 +58,7 @@ export class NgModuleSymbol extends SemanticSymbol {
usedPipes: SemanticReference[]
}[] = [];
isPublicApiAffected(previousSymbol: SemanticSymbol): boolean {
override isPublicApiAffected(previousSymbol: SemanticSymbol): boolean {
if (!(previousSymbol instanceof NgModuleSymbol)) {
return true;
}
@@ -104,7 +104,7 @@ export class NgModuleSymbol extends SemanticSymbol {
return false;
}
isTypeCheckApiAffected(previousSymbol: SemanticSymbol): boolean {
override isTypeCheckApiAffected(previousSymbol: SemanticSymbol): boolean {
if (!(previousSymbol instanceof NgModuleSymbol)) {
return true;
}
@@ -38,7 +38,7 @@ export class PipeSymbol extends SemanticSymbol {
super(decl);
}
isPublicApiAffected(previousSymbol: SemanticSymbol): boolean {
override isPublicApiAffected(previousSymbol: SemanticSymbol): boolean {
if (!(previousSymbol instanceof PipeSymbol)) {
return true;
}
@@ -46,7 +46,7 @@ export class PipeSymbol extends SemanticSymbol {
return this.name !== previousSymbol.name;
}
isTypeCheckApiAffected(previousSymbol: SemanticSymbol): boolean {
override isTypeCheckApiAffected(previousSymbol: SemanticSymbol): boolean {
return this.isPublicApiAffected(previousSymbol);
}
}
@@ -21,20 +21,20 @@ export class MockFileSystemNative extends MockFileSystem {
// Delegate to the real NodeJSFileSystem for these path related methods
resolve(...paths: string[]): AbsoluteFsPath {
override resolve(...paths: string[]): AbsoluteFsPath {
return NodeJSFileSystem.prototype.resolve.call(this, this.pwd(), ...paths);
}
dirname<T extends string>(file: T): T {
override dirname<T extends string>(file: T): T {
return NodeJSFileSystem.prototype.dirname.call(this, file) as T;
}
join<T extends string>(basePath: T, ...paths: string[]): T {
override join<T extends string>(basePath: T, ...paths: string[]): T {
return NodeJSFileSystem.prototype.join.call(this, basePath, ...paths) as T;
}
relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
override relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
return NodeJSFileSystem.prototype.relative.call(this, from, to);
}
basename(filePath: string, extension?: string): PathSegment {
override basename(filePath: string, extension?: string): PathSegment {
return NodeJSFileSystem.prototype.basename.call(this, filePath, extension);
}
@@ -42,7 +42,7 @@ export class MockFileSystemNative extends MockFileSystem {
return NodeJSFileSystem.prototype.isCaseSensitive.call(this);
}
isRooted(path: string): boolean {
override isRooted(path: string): boolean {
return NodeJSFileSystem.prototype.isRooted.call(this, path);
}
@@ -50,7 +50,7 @@ export class MockFileSystemNative extends MockFileSystem {
return NodeJSFileSystem.prototype.isRoot.call(this, path);
}
normalize<T extends PathString>(path: T): T {
override normalize<T extends PathString>(path: T): T {
// When running in Windows, absolute paths are normalized to always include a drive letter. This
// ensures that rooted posix paths used in tests will be normalized to real Windows paths, i.e.
// including a drive letter. Note that the same normalization is done in emulated Windows mode
@@ -63,7 +63,7 @@ export class MockFileSystemNative extends MockFileSystem {
return NodeJSFileSystem.prototype.normalize.call(this, path) as T;
}
protected splitPath<T>(path: string): string[] {
protected override splitPath<T>(path: string): string[] {
return path.split(/[\\\/]/);
}
}
@@ -12,36 +12,36 @@ import {AbsoluteFsPath, PathSegment, PathString} from '../../src/types';
import {MockFileSystem} from './mock_file_system';
export class MockFileSystemPosix extends MockFileSystem {
resolve(...paths: string[]): AbsoluteFsPath {
override resolve(...paths: string[]): AbsoluteFsPath {
const resolved = p.posix.resolve(this.pwd(), ...paths);
return this.normalize(resolved) as AbsoluteFsPath;
}
dirname<T extends string>(file: T): T {
override dirname<T extends string>(file: T): T {
return this.normalize(p.posix.dirname(file)) as T;
}
join<T extends string>(basePath: T, ...paths: string[]): T {
override join<T extends string>(basePath: T, ...paths: string[]): T {
return this.normalize(p.posix.join(basePath, ...paths)) as T;
}
relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
override relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
return this.normalize(p.posix.relative(from, to)) as PathSegment | AbsoluteFsPath;
}
basename(filePath: string, extension?: string): PathSegment {
override basename(filePath: string, extension?: string): PathSegment {
return p.posix.basename(filePath, extension) as PathSegment;
}
isRooted(path: string): boolean {
override isRooted(path: string): boolean {
return path.startsWith('/');
}
protected splitPath<T extends PathString>(path: T): string[] {
protected override splitPath<T extends PathString>(path: T): string[] {
return path.split('/');
}
normalize<T extends PathString>(path: T): T {
override normalize<T extends PathString>(path: T): T {
return path.replace(/^[a-z]:\//i, '/').replace(/\\/g, '/') as T;
}
}
@@ -12,36 +12,36 @@ import {AbsoluteFsPath, PathSegment, PathString} from '../../src/types';
import {MockFileSystem} from './mock_file_system';
export class MockFileSystemWindows extends MockFileSystem {
resolve(...paths: string[]): AbsoluteFsPath {
override resolve(...paths: string[]): AbsoluteFsPath {
const resolved = p.win32.resolve(this.pwd(), ...paths);
return this.normalize(resolved as AbsoluteFsPath);
}
dirname<T extends string>(path: T): T {
override dirname<T extends string>(path: T): T {
return this.normalize(p.win32.dirname(path) as T);
}
join<T extends string>(basePath: T, ...paths: string[]): T {
override join<T extends string>(basePath: T, ...paths: string[]): T {
return this.normalize(p.win32.join(basePath, ...paths)) as T;
}
relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
override relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
return this.normalize(p.win32.relative(from, to)) as PathSegment | AbsoluteFsPath;
}
basename(filePath: string, extension?: string): PathSegment {
override basename(filePath: string, extension?: string): PathSegment {
return p.win32.basename(filePath, extension) as PathSegment;
}
isRooted(path: string): boolean {
override isRooted(path: string): boolean {
return /^([A-Z]:)?([\\\/]|$)/i.test(path);
}
protected splitPath<T extends PathString>(path: T): string[] {
protected override splitPath<T extends PathString>(path: T): string[] {
return path.split(/[\\\/]/);
}
normalize<T extends PathString>(path: T): T {
override normalize<T extends PathString>(path: T): T {
return path.replace(/^[\/\\]/i, 'C:/').replace(/\\/g, '/') as T;
}
}
@@ -35,11 +35,11 @@ export interface SemanticDependencyResult {
* compilation.
*/
class OpaqueSymbol extends SemanticSymbol {
isPublicApiAffected(): false {
override isPublicApiAffected(): false {
return false;
}
isTypeCheckApiAffected(): false {
override isTypeCheckApiAffected(): false {
return false;
}
}
@@ -16,7 +16,7 @@ export class ArraySliceBuiltinFn extends KnownFn {
super();
}
evaluate(node: ts.CallExpression, args: ResolvedValueArray): ResolvedValue {
override evaluate(node: ts.CallExpression, args: ResolvedValueArray): ResolvedValue {
if (args.length === 0) {
return this.lhs;
} else {
@@ -30,7 +30,7 @@ export class ArrayConcatBuiltinFn extends KnownFn {
super();
}
evaluate(node: ts.CallExpression, args: ResolvedValueArray): ResolvedValue {
override evaluate(node: ts.CallExpression, args: ResolvedValueArray): ResolvedValue {
const result: ResolvedValueArray = [...this.lhs];
for (const arg of args) {
if (arg instanceof DynamicValue) {
@@ -46,7 +46,7 @@ export class ArrayConcatBuiltinFn extends KnownFn {
}
export class ObjectAssignBuiltinFn extends KnownFn {
evaluate(node: ts.CallExpression, args: ResolvedValueArray): ResolvedValue {
override evaluate(node: ts.CallExpression, args: ResolvedValueArray): ResolvedValue {
if (args.length === 0) {
return DynamicValue.fromUnsupportedSyntax(node);
}
@@ -19,7 +19,7 @@ export class AssignHelperFn extends ObjectAssignBuiltinFn {}
// Used for both `__spread()` and `__spreadArrays()` TypeScript helper functions.
export class SpreadHelperFn extends KnownFn {
evaluate(node: ts.Node, args: ResolvedValueArray): ResolvedValueArray {
override evaluate(node: ts.Node, args: ResolvedValueArray): ResolvedValueArray {
const result: ResolvedValueArray = [];
for (const arg of args) {
@@ -38,7 +38,7 @@ export class SpreadHelperFn extends KnownFn {
// Used for `__spreadArray` TypeScript helper function.
export class SpreadArrayHelperFn extends KnownFn {
evaluate(node: ts.Node, args: ResolvedValueArray): ResolvedValue {
override evaluate(node: ts.Node, args: ResolvedValueArray): ResolvedValue {
if (args.length !== 2) {
return DynamicValue.fromUnknown(node);
}
@@ -62,7 +62,7 @@ export class SpreadArrayHelperFn extends KnownFn {
// Used for `__read` TypeScript helper function.
export class ReadHelperFn extends KnownFn {
evaluate(node: ts.Node, args: ResolvedValueArray): ResolvedValue {
override evaluate(node: ts.Node, args: ResolvedValueArray): ResolvedValue {
if (args.length !== 1) {
// The `__read` helper accepts a second argument `n` but that case is not supported.
return DynamicValue.fromUnknown(node);
@@ -61,7 +61,7 @@ class IvyCompilationVisitor extends Visitor {
super();
}
visitClassDeclaration(node: ts.ClassDeclaration):
override visitClassDeclaration(node: ts.ClassDeclaration):
VisitListEntryResult<ts.Statement, ts.ClassDeclaration> {
// Determine if this class has an Ivy field that needs to be added, and compile the field
// to an expression if so.
@@ -87,7 +87,7 @@ class IvyTransformationVisitor extends Visitor {
super();
}
visitClassDeclaration(node: ts.ClassDeclaration):
override visitClassDeclaration(node: ts.ClassDeclaration):
VisitListEntryResult<ts.Statement, ts.ClassDeclaration> {
// If this class is not registered in the map, it means that it doesn't have Angular decorators,
// thus no further processing is required.
@@ -192,14 +192,14 @@ class TcbElementOp extends TcbOp {
super();
}
get optional() {
override get optional() {
// The statement generated by this operation is only used for type-inference of the DOM
// element's type and won't report diagnostics by itself, so the operation is marked as optional
// to avoid generating statements for DOM elements that are never referenced.
return true;
}
execute(): ts.Identifier {
override execute(): ts.Identifier {
const id = this.tcb.allocateId();
// Add the declaration of the element using document.createElement.
const initializer = tsCreateElement(this.element.name);
@@ -222,11 +222,11 @@ class TcbVariableOp extends TcbOp {
super();
}
get optional() {
override get optional() {
return false;
}
execute(): ts.Identifier {
override execute(): ts.Identifier {
// Look for a context variable for the template.
const ctx = this.scope.resolve(this.template);
@@ -263,9 +263,9 @@ class TcbTemplateContextOp extends TcbOp {
}
// The declaration of the context variable is only needed when the context is actually referenced.
readonly optional = true;
override readonly optional = true;
execute(): ts.Identifier {
override execute(): ts.Identifier {
// Allocate a template ctx variable and declare it with an 'any' type. The type of this variable
// may be narrowed as a result of template guard conditions.
const ctx = this.tcb.allocateId();
@@ -287,11 +287,11 @@ class TcbTemplateBodyOp extends TcbOp {
super();
}
get optional() {
override get optional() {
return false;
}
execute(): null {
override execute(): null {
// An `if` will be constructed, within which the template's children will be type checked. The
// `if` is used for two reasons: it creates a new syntactic scope, isolating variables declared
// in the template's TCB from the outer context, and it allows any directives on the templates
@@ -414,11 +414,11 @@ class TcbTextInterpolationOp extends TcbOp {
super();
}
get optional() {
override get optional() {
return false;
}
execute(): null {
override execute(): null {
const expr = tcbExpression(this.binding.value, this.tcb, this.scope);
this.scope.addStatement(ts.createExpressionStatement(expr));
return null;
@@ -436,14 +436,14 @@ abstract class TcbDirectiveTypeOpBase extends TcbOp {
super();
}
get optional() {
override get optional() {
// The statement generated by this operation is only used to declare the directive's type and
// won't report diagnostics by itself, so the operation is marked as optional to avoid
// generating declarations for directives that don't have any inputs/outputs.
return true;
}
execute(): ts.Identifier {
override execute(): ts.Identifier {
const dirRef = this.dir.ref as Reference<ClassDeclaration<ts.ClassDeclaration>>;
const rawType = this.tcb.env.referenceType(this.dir.ref);
@@ -543,9 +543,9 @@ class TcbReferenceOp extends TcbOp {
// The statement generated by this operation is only used to for the Type Checker
// so it can map a reference variable in the template directly to a node in the TCB.
readonly optional = true;
override readonly optional = true;
execute(): ts.Identifier {
override execute(): ts.Identifier {
const id = this.tcb.allocateId();
let initializer =
this.target instanceof TmplAstTemplate || this.target instanceof TmplAstElement ?
@@ -591,9 +591,9 @@ class TcbInvalidReferenceOp extends TcbOp {
}
// The declaration of a missing reference is only needed when the reference is resolved.
readonly optional = true;
override readonly optional = true;
execute(): ts.Identifier {
override execute(): ts.Identifier {
const id = this.tcb.allocateId();
this.scope.addStatement(tsCreateVariable(id, NULL_AS_ANY));
return id;
@@ -619,13 +619,13 @@ class TcbDirectiveCtorOp extends TcbOp {
super();
}
get optional() {
override get optional() {
// The statement generated by this operation is only used to infer the directive's type and
// won't report diagnostics by itself, so the operation is marked as optional.
return true;
}
execute(): ts.Identifier {
override execute(): ts.Identifier {
const id = this.tcb.allocateId();
addExpressionIdentifier(id, ExpressionIdentifier.DIRECTIVE);
addParseSpanInfo(id, this.node.startSourceSpan || this.node.sourceSpan);
@@ -689,11 +689,11 @@ class TcbDirectiveInputsOp extends TcbOp {
super();
}
get optional() {
override get optional() {
return false;
}
execute(): null {
override execute(): null {
let dirId: ts.Expression|null = null;
// TODO(joost): report duplicate properties
@@ -815,11 +815,11 @@ class TcbDirectiveCtorCircularFallbackOp extends TcbOp {
super();
}
get optional() {
override get optional() {
return false;
}
execute(): ts.Identifier {
override execute(): ts.Identifier {
const id = this.tcb.allocateId();
const typeCtor = this.tcb.env.typeCtorFor(this.dir);
const circularPlaceholder = ts.createCall(
@@ -846,11 +846,11 @@ class TcbDomSchemaCheckerOp extends TcbOp {
super();
}
get optional() {
override get optional() {
return false;
}
execute(): ts.Expression|null {
override execute(): ts.Expression|null {
if (this.checkElement) {
this.tcb.domSchemaChecker.checkElement(this.tcb.id, this.element, this.tcb.schemas);
}
@@ -906,11 +906,11 @@ class TcbUnclaimedInputsOp extends TcbOp {
super();
}
get optional() {
override get optional() {
return false;
}
execute(): null {
override execute(): null {
// `this.inputs` contains only those bindings not matched by any directive. These bindings go to
// the element itself.
let elId: ts.Expression|null = null;
@@ -972,11 +972,11 @@ export class TcbDirectiveOutputsOp extends TcbOp {
super();
}
get optional() {
override get optional() {
return false;
}
execute(): null {
override execute(): null {
let dirId: ts.Expression|null = null;
const outputs = this.dir.outputs;
@@ -1035,11 +1035,11 @@ class TcbUnclaimedOutputsOp extends TcbOp {
super();
}
get optional() {
override get optional() {
return false;
}
execute(): null {
override execute(): null {
let elId: ts.Expression|null = null;
// TODO(alxhub): this could be more efficient.
@@ -1103,9 +1103,9 @@ class TcbComponentContextCompletionOp extends TcbOp {
super();
}
readonly optional = false;
override readonly optional = false;
execute(): null {
override execute(): null {
const ctx = ts.createIdentifier('ctx');
const ctxDot = ts.createPropertyAccess(ctx, '');
markIgnoreDiagnostics(ctxDot);
@@ -13,7 +13,7 @@ import {makeProgram} from '../../testing';
import {visit, VisitListEntryResult, Visitor} from '../src/visitor';
class TestAstVisitor extends Visitor {
visitClassDeclaration(node: ts.ClassDeclaration):
override visitClassDeclaration(node: ts.ClassDeclaration):
VisitListEntryResult<ts.Statement, ts.ClassDeclaration> {
const name = node.name!.text;
const statics = node.members.filter(