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

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

Author: josephperrott

MODULE.bazel

@@ -1,4 +1,5 @@
 """Rules/toolchains for devinfra with Bazel."""
+
 module(
     name = "devinfra",
 )
@@ -26,6 +27,7 @@ git_override(
     commit = "155c3367c0140dc3c8ff0d0ee050f5cc6aa0a643",
     remote = "https://github.com/devversion/rules_browsers.git",
 )
+
 bazel_dep(name = "rules_sass")
 git_override(
     module_name = "rules_sass",
@@ -43,7 +45,6 @@ git_override(
 yq = use_extension("@yq.bzl//yq:extensions.bzl", "yq")
 use_repo(yq, "yq_toolchains")
 
-
 rules_ts_ext = use_extension("@aspect_rules_ts//ts:extensions.bzl", "ext")
 rules_ts_ext.deps(
     # Obtained by: curl --silent https://registry.npmjs.org/typescript/5.9.2 | jq -r '.dist.integrity'
@@ -67,43 +68,40 @@ node.toolchain(
 )
 use_repo(node, "nodejs_toolchains")
 
-
 rules_angular = use_extension("@rules_angular//setup:extensions.bzl", "rules_angular")
 rules_angular.setup(
     name = "dev_infra_rules_angular_configurable_deps",
     angular_compiler_cli = "//:node_modules/@angular/compiler-cli",
     typescript = "//:node_modules/typescript-local",
 )
-use_repo(rules_angular, **{"rules_angular_configurable_deps":"dev_infra_rules_angular_configurable_deps"})
-
+use_repo(rules_angular, **{"rules_angular_configurable_deps": "dev_infra_rules_angular_configurable_deps"})
 
 npm = use_extension("@aspect_rules_js//npm:extensions.bzl", "npm")
-
 npm.npm_translate_lock(
-     name = "devinfra_npm",
-        data = [
-            "@devinfra//:package.json",
-            "@devinfra//bazel:package.json",
-            "@devinfra//.github/local-actions/branch-manager:package.json",
-            "@devinfra//.github/local-actions/labels-sync:package.json",
-            "@devinfra//.github/local-actions/lock-closed:package.json",
-            "@devinfra//github-actions/google-internal-tests:package.json",
-            "@devinfra//github-actions/pull-request-labeling:package.json",
-            "@devinfra//github-actions/unified-status-check:package.json",
-            "@devinfra//github-actions/previews/pack-and-upload-artifact:package.json",
-            "@devinfra//github-actions/previews/upload-artifacts-to-firebase:package.json",
-            "@devinfra//github-actions/post-approval-changes:package.json",
-            "@devinfra//github-actions/branch-manager:package.json",
-            "@devinfra//github-actions/saucelabs:package.json",
-            "@devinfra//github-actions/browserstack:package.json",
-            "@devinfra//github-actions/bazel/configure-remote:package.json",
-            "@devinfra//github-actions/org-file-sync:package.json",
-            "@devinfra//github-actions/feature-request:package.json",
-            "@devinfra//ng-dev:package.json",
-            "@devinfra//bazel/spec-bundling/test:package.json",
-            "@devinfra//:pnpm-workspace.yaml",
-        ],
-        pnpm_lock = "@devinfra//:pnpm-lock.yaml",
-        npmrc = "@devinfra//:.npmrc",
+    name = "devinfra_npm",
+    data = [
+        "@devinfra//.github/local-actions/branch-manager:package.json",
+        "@devinfra//.github/local-actions/labels-sync:package.json",
+        "@devinfra//.github/local-actions/lock-closed:package.json",
+        "@devinfra//:package.json",
+        "@devinfra//:pnpm-workspace.yaml",
+        "@devinfra//bazel:package.json",
+        "@devinfra//bazel/spec-bundling/test:package.json",
+        "@devinfra//github-actions/bazel/configure-remote:package.json",
+        "@devinfra//github-actions/branch-manager:package.json",
+        "@devinfra//github-actions/browserstack:package.json",
+        "@devinfra//github-actions/feature-request:package.json",
+        "@devinfra//github-actions/google-internal-tests:package.json",
+        "@devinfra//github-actions/org-file-sync:package.json",
+        "@devinfra//github-actions/post-approval-changes:package.json",
+        "@devinfra//github-actions/previews/pack-and-upload-artifact:package.json",
+        "@devinfra//github-actions/previews/upload-artifacts-to-firebase:package.json",
+        "@devinfra//github-actions/pull-request-labeling:package.json",
+        "@devinfra//github-actions/saucelabs:package.json",
+        "@devinfra//github-actions/unified-status-check:package.json",
+        "@devinfra//ng-dev:package.json",
+    ],
+    npmrc = "@devinfra//:.npmrc",
+    pnpm_lock = "@devinfra//:pnpm-lock.yaml",
 )
 use_repo(npm, "devinfra_npm")

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 {