fix(bazel): handle additional cases for strict deps testing (#2988)

Properly handle module names that come with packages in DefinitelyTyped
Properly handle module names that are described within tsconfig paths configuration
Properly handle edge case of `zone.js` containing an extension in its name

PR Close #2988

Author: josephperrott

bazel/ts_project/strict_deps/diagnostic.mts

@@ -1,3 +1,11 @@
+/**
+ * @license
+ * Copyright Google LLC
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
 import ts from 'typescript';
 
 export function createDiagnostic(message: string, node: ts.Node): ts.Diagnostic {

bazel/ts_project/strict_deps/index.bzl

@@ -53,6 +53,8 @@ def _strict_deps_impl(ctx):
             "testFiles": test_files,
             "allowedModuleNames": allowed_module_names,
             "allowedSources": allowed_sources,
+            # The tsconfig from rules_ts has a single src so we know it will be the first file.
+            "tsconfigPath": ctx.files.tsconfig[0].short_path,
         }),
     )
 
@@ -85,14 +87,21 @@ def _strict_deps_impl(ctx):
         ),
     )
 
-    bin_runfiles = ctx.attr._bin[DefaultInfo].default_runfiles
+    runfiles = ctx.runfiles(
+        files = [
+                    manifest,
+                ] + ctx.files.srcs +
+                ctx.files._runfiles_lib +
+                ctx.files.tsconfig,
+    ).merge_all([
+        ctx.attr._bin[DefaultInfo].default_runfiles,
+        ctx.attr.tsconfig[DefaultInfo].default_runfiles,
+    ])
 
     return [
         DefaultInfo(
             executable = launcher,
-            runfiles = ctx.runfiles(
-                files = ctx.files._runfiles_lib + ctx.files.srcs + [manifest],
-            ).merge(bin_runfiles),
+            runfiles = runfiles,
         ),
     ]
 
@@ -108,8 +117,13 @@ _strict_deps_test = rule(
         ),
         "srcs": attr.label_list(
             doc = "TS files to be checked",
+            mandatory = True,
             allow_files = True,
+        ),
+        "tsconfig": attr.label(
+            doc = "The tsconfig of the ts_project being checked",
             mandatory = True,
+            allow_files = True,
         ),
         "will_fail": attr.bool(
             doc = "Whether the test is expected to fail",

bazel/ts_project/strict_deps/index.mts

@@ -1,9 +1,19 @@
+/**
+ * @license
+ * Copyright Google LLC
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {isBuiltin} from 'node:module';
 import fs from 'node:fs/promises';
 import path from 'node:path';
 import ts from 'typescript';
 import {createDiagnostic} from './diagnostic.mjs';
 import {StrictDepsManifest} from './manifest.mjs';
 import {getImportsInSourceFile} from './visitor.mjs';
+import {readTsConfig} from './tsconfig.mjs';
 
 const [manifestExecPath, expectedFailureRaw] = process.argv.slice(2);
 const expectedFailure = expectedFailureRaw === 'true';
@@ -13,22 +23,45 @@ const manifest: StrictDepsManifest = JSON.parse(await fs.readFile(manifestExecPa
 /**
  * Regex matcher to extract a npm package name, potentially with scope from a subpackage import path.
  */
-const moduleSpeciferMatcher = /^(@[\w\d-_]+\/)?([\w\d-_]+)/;
-const extensionRemoveRegex = /\.[mc]?(js|(d\.)?[mc]?ts)$/;
-const allowedModuleNames = new Set<string>(manifest.allowedModuleNames);
+const moduleSpeciferMatcher = /^(@[\w\d-_\.]+\/)?([\w\d-_\.]+)/;
+const extensionRemoveRegex = /\.[mc]?(js|(d\.)?[mc]?tsx?)$/;
+const allowedModuleNames = new Set<string>(
+  manifest.allowedModuleNames.map((m) => {
+    return (
+      m
+        // Scoped types from DefinitelyTyped are split using a __ delimiter, so we put it back together.
+        .replace(/(?:@types\/)(.*)__(.*)/, '@$1/$2')
+        // Replace any unscoped types package from DefinitelyTyped with just to package name.
+        .replace(/(?:@types\/)(.*)/, '$1')
+    );
+  }),
+);
 const allowedSources = new Set<string>(
   manifest.allowedSources.map((s) => s.replace(extensionRemoveRegex, '')),
 );
-
+const tsconfig = readTsConfig(path.join(process.cwd(), manifest.tsconfigPath));
 const diagnostics: ts.Diagnostic[] = [];
 
+/** Check if the moduleSpecifier matches any of the provided paths. */
+function checkPathsForMatch(moduleSpecifier: string, paths?: ts.MapLike<string[]>): boolean {
+  for (const matcher of Object.keys(paths || {})) {
+    if (new RegExp(matcher).test(moduleSpecifier)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 for (const fileExecPath of manifest.testFiles) {
   const content = await fs.readFile(fileExecPath, 'utf8');
   const sf = ts.createSourceFile(fileExecPath, content, ts.ScriptTarget.ESNext, true);
   const imports = getImportsInSourceFile(sf);
 
   for (const i of imports) {
-    const moduleSpecifier = i.moduleSpecifier.replace(extensionRemoveRegex, '');
+    const moduleSpecifier =
+      i.moduleSpecifier === 'zone.js'
+        ? 'zone.js'
+        : i.moduleSpecifier.replace(extensionRemoveRegex, '');
     // When the module specified is the file itself this is always a valid dep.
     if (i.moduleSpecifier === '') {
       continue;
@@ -44,16 +77,24 @@ for (const fileExecPath of manifest.testFiles) {
       }
     }
 
-    if (moduleSpecifier.startsWith('node:') && allowedModuleNames.has('@types/node')) {
+    if (
+      isBuiltin(moduleSpecifier) &&
+      (allowedModuleNames.has('node') || tsconfig.options.types?.includes('node'))
+    ) {
       continue;
     }
 
     if (
-      allowedModuleNames.has(moduleSpecifier.match(moduleSpeciferMatcher)?.[0] || moduleSpecifier)
+      allowedModuleNames.has(moduleSpecifier.match(moduleSpeciferMatcher)?.[0] || '') ||
+      allowedModuleNames.has(moduleSpecifier)
     ) {
       continue;
     }
 
+    if (checkPathsForMatch(moduleSpecifier, tsconfig.options.paths)) {
+      continue;
+    }
+
     diagnostics.push(
       createDiagnostic(`No explicit Bazel dependency for this module.`, i.diagnosticNode),
     );

bazel/ts_project/strict_deps/manifest.mts

@@ -1,5 +1,14 @@
+/**
+ * @license
+ * Copyright Google LLC
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
 export interface StrictDepsManifest {
   allowedModuleNames: string[];
   allowedSources: string[];
   testFiles: string[];
+  tsconfigPath: string;
 }

bazel/ts_project/strict_deps/test/BUILD.bazel

@@ -12,6 +12,7 @@ ts_project(
 strict_deps_test(
     name = "import_node_module",
     srcs = ["import_node_module.ts"],
+    tsconfig = "//bazel:tsconfig",
     deps = [
         "//bazel:node_modules/@types/node",
     ],
@@ -20,17 +21,20 @@ strict_deps_test(
 invalid_strict_deps_test(
     name = "invalid_import_node_module",
     srcs = ["import_node_module.ts"],
+    tsconfig = "//bazel:tsconfig",
 )
 
 strict_deps_test(
     name = "import_npm_module",
     srcs = ["import_npm_module.ts"],
+    tsconfig = "//bazel:tsconfig",
     deps = ["//bazel:node_modules/@microsoft/api-extractor"],
 )
 
 invalid_strict_deps_test(
     name = "invalid_import_npm_module_transitively",
     srcs = ["import_npm_module.ts"],
+    tsconfig = "//bazel:tsconfig",
     deps = [
         "//bazel/ts_project/strict_deps/test/import_npm_module",
     ],
@@ -39,17 +43,20 @@ invalid_strict_deps_test(
 invalid_strict_deps_test(
     name = "invalid_import_npm_module",
     srcs = ["import_npm_module.ts"],
+    tsconfig = "//bazel:tsconfig",
 )
 
 strict_deps_test(
     name = "import_from_depth",
     srcs = ["import_from_depth.ts"],
+    tsconfig = "//bazel:tsconfig",
     deps = ["//bazel/ts_project/strict_deps/test/depth"],
 )
 
 invalid_strict_deps_test(
     name = "invalid_import_from_depth",
     srcs = ["import_from_depth.ts"],
+    tsconfig = "//bazel:tsconfig",
     deps = [
         ":sibling_import_from_depth",
     ],

bazel/ts_project/strict_deps/test/import_from_mts_cts_extensions/BUILD.bazel

@@ -4,6 +4,7 @@ load("//bazel/ts_project/strict_deps:index.bzl", "strict_deps_test")
 strict_deps_test(
     name = "import_from_mts_cts_extensions",
     srcs = ["index.ts"],
+    tsconfig = "//bazel:tsconfig",
     deps = [":mts_cts_extensions"],
 )
 

bazel/ts_project/strict_deps/tsconfig.mts

@@ -0,0 +1,25 @@
+/**
+ * @license
+ * Copyright Google LLC
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import ts from 'typescript';
+import {dirname} from 'path';
+
+export function readTsConfig(filePath: string) {
+  const configFile = ts.readConfigFile(filePath, ts.sys.readFile);
+  if (configFile.error) {
+    throw new Error(ts.formatDiagnostics([configFile.error], ts.createCompilerHost({})));
+  }
+
+  const parsedConfig = ts.parseJsonConfigFileContent(configFile.config, ts.sys, dirname(filePath));
+
+  if (parsedConfig.errors.length > 0) {
+    throw new Error(ts.formatDiagnostics(parsedConfig.errors, ts.createCompilerHost({})));
+  }
+
+  return parsedConfig;
+}

bazel/ts_project/strict_deps/visitor.mts

@@ -1,3 +1,11 @@
+/**
+ * @license
+ * Copyright Google LLC
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
 import ts from 'typescript';
 
 export interface Import {