feat(dev-infra):: add lint rule to enforce no-implicit-override abstr…

…act members

TypeScript introduced a new flag called `noImplicitOverride` as part
of TypeScript v4.3. This flag introduces a new keyword called `override`
that can be applied to members which override declarations from a base
class. This helps with code health as TS will report an error if e.g.
the base class changes the method name but the override would still
have the old method name. Similarly, if the base class removes the method
completely, TS would complain that the memeber with `override` no longer
overrides any method.

A similar concept applies to abstract methods, with the exception that
TypeScript's builtin `noImplicitOverride` option does not flag members
which are implemented as part of an abstract class. We want to enforce
this as a best-practice in the repository as adding `override` to such
implemented members will cause TS to complain if an abstract member is
removed, but still implemented by derived classes.

More details: microsoft/TypeScript#44457.

dev-infra/BUILD.bazel

@@ -78,6 +78,7 @@ pkg_npm(
         "//dev-infra/benchmark/driver-utilities",
         "//dev-infra/commit-message",
         "//dev-infra/ts-circular-dependencies",
+        "//dev-infra/tslint-rules",
     ],
 )
 

package.json

@@ -29,7 +29,7 @@
     "test-fixme-ivy-aot": "bazelisk test --config=ivy --build_tag_filters=-no-ivy-aot --test_tag_filters=-no-ivy-aot",
     "list-fixme-ivy-targets": "bazelisk query --output=label 'attr(\"tags\", \"\\[.*fixme-ivy.*\\]\", //...) except kind(\"sh_binary\", //...) except kind(\"devmode_js_sources\", //...)' | sort",
     "lint": "yarn -s tslint && yarn -s ng-dev format changed --check",
-    "tslint": "tsc -p tools/tsconfig.json && tslint -c tslint.json \"+(dev-infra|packages|modules|scripts|tools)/**/*.+(js|ts)\"",
+    "tslint": "tslint -c tslint.json --project tsconfig-tslint.json",
     "public-api:check": "node goldens/public-api/manage.js test",
     "public-api:update": "node goldens/public-api/manage.js accept",
     "symbol-extractor:check": "node tools/symbol-extractor/run_all_symbols_extractor_tests.js test",

tools/contributing-stats/get-data.ts

@@ -237,7 +237,8 @@ async function run(date: string) {
     console.info(['Username', ...buildQueryAndParams('', date).labels].join(','));
 
     for (const username of allOrgMembers) {
-      const results = await graphql(buildQueryAndParams(username, date).query.toString());
+      const results: [{issueCount: number}] =
+          await graphql(buildQueryAndParams(username, date).query.toString());
       const values = Object.values(results).map(result => `${result.issueCount}`);
       console.info([username, ...values].join(','));
     }

tools/size-tracking/size_tracker.ts

@@ -94,7 +94,7 @@ export class SizeTracker {
     filePath = filePath.replace(/\\/g, '/');
 
     // Workaround for https://github.com/angular/angular/issues/30060
-    if (process.env['BAZEL_TARGET'].includes('test/bundling/core_all:size_test')) {
+    if (process.env['BAZEL_TARGET']!.includes('test/bundling/core_all:size_test')) {
       return filePath.replace(/^(\.\.\/)+external/, 'external')
           .replace(/^(\.\.\/)+packages\/core\//, '@angular/core/')
           .replace(/^(\.\.\/){3}/, '@angular/core/');

tools/tsconfig.json

@@ -6,6 +6,7 @@
     "experimentalDecorators": true,
     "module": "commonjs",
     "moduleResolution": "node",
+    "strict": true,
     "outDir": "../dist/tools/",
     "noImplicitAny": true,
     "noFallthroughCasesInSwitch": true,

tools/tslint/requireInternalWithUnderscoreRule.ts

@@ -47,6 +47,6 @@ class TypedefWalker extends RuleWalker {
     }
     this.addFailure(this.createFailure(
         node.getStart(), node.getWidth(),
-        `module-private member ${node.name.getText()} must be annotated @internal`));
+        `module-private member ${node.name?.getText()} must be annotated @internal`));
   }
 }

tsconfig-tslint.json

@@ -0,0 +1,12 @@
+{
+  "compilerOptions": {
+    "allowJs": true
+  },
+  "include": [
+    "dev-infra/**/*",
+    "packages/**/*",
+    "modules/**/*",
+    "tools/**/*",
+    "scripts/**/*"
+  ]
+}

tslint.json

@@ -1,11 +1,19 @@
 {
   "rulesDirectory": [
-    "dist/tools/tslint",
+    "tools/tslint",
+    "dev-infra/tslint-rules",
     "node_modules/vrsource-tslint-rules/rules",
     "node_modules/tslint-eslint-rules/dist/rules",
     "node_modules/tslint-no-toplevel-property-access/rules"
   ],
   "rules": {
+    // The first rule needs to be `ts-node-loader` which sets up `ts-node` within TSLint so
+    // that rules written in TypeScript can be loaded without needing to be transpiled.
+    "ts-node-loader": true,
+    // Custom rules written in TypeScript.
+    "require-internal-with-underscore": true,
+    "no-implicit-override-abstract": true,
+
     "eofline": true,
     "file-header": [
       true,
@@ -26,7 +34,6 @@
       true,
       "object"
     ],
-    "require-internal-with-underscore": true,
     "no-toplevel-property-access": [
       true,
       "packages/animations/src/",
@@ -57,6 +64,12 @@
     ]
   },
   "jsRules": {
+    // The first rule needs to be `ts-node-loader` which sets up `ts-node` within TSLint so
+    // that rules written in TypeScript can be loaded without needing to be transpiled.
+    "ts-node-loader": true,
+    // Custom rules written in TypeScript.
+    "require-internal-with-underscore": true,
+
     "eofline": true,
     "file-header": [
       true,
@@ -71,7 +84,6 @@
     ],
     "no-duplicate-imports": true,
     "no-duplicate-variable": true,
-    "require-internal-with-underscore": true,
     "semicolon": [
       true
     ],