From 15b3a1b847cb2efc869a1ee78aba45989e783dbd Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Thu, 21 Aug 2025 21:13:57 +0000 Subject: [PATCH] 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 --- bazel/ts_project/strict_deps/diagnostic.mts | 8 +++ bazel/ts_project/strict_deps/index.bzl | 22 ++++++-- bazel/ts_project/strict_deps/index.mts | 55 ++++++++++++++++--- bazel/ts_project/strict_deps/manifest.mts | 9 +++ bazel/ts_project/strict_deps/test/BUILD.bazel | 7 +++ .../BUILD.bazel | 1 + bazel/ts_project/strict_deps/tsconfig.mts | 25 +++++++++ bazel/ts_project/strict_deps/visitor.mts | 8 +++ 8 files changed, 124 insertions(+), 11 deletions(-) create mode 100644 bazel/ts_project/strict_deps/tsconfig.mts diff --git a/bazel/ts_project/strict_deps/diagnostic.mts b/bazel/ts_project/strict_deps/diagnostic.mts index 2bc222205..e83a39395 100644 --- a/bazel/ts_project/strict_deps/diagnostic.mts +++ b/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 { diff --git a/bazel/ts_project/strict_deps/index.bzl b/bazel/ts_project/strict_deps/index.bzl index 451277c8e..c98686f82 100644 --- a/bazel/ts_project/strict_deps/index.bzl +++ b/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", diff --git a/bazel/ts_project/strict_deps/index.mts b/bazel/ts_project/strict_deps/index.mts index 448bbaad0..365e335bc 100644 --- a/bazel/ts_project/strict_deps/index.mts +++ b/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(manifest.allowedModuleNames); +const moduleSpeciferMatcher = /^(@[\w\d-_\.]+\/)?([\w\d-_\.]+)/; +const extensionRemoveRegex = /\.[mc]?(js|(d\.)?[mc]?tsx?)$/; +const allowedModuleNames = new Set( + 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( 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): 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), ); diff --git a/bazel/ts_project/strict_deps/manifest.mts b/bazel/ts_project/strict_deps/manifest.mts index ab547c130..bec7ceda4 100644 --- a/bazel/ts_project/strict_deps/manifest.mts +++ b/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; } diff --git a/bazel/ts_project/strict_deps/test/BUILD.bazel b/bazel/ts_project/strict_deps/test/BUILD.bazel index d4f3d7d2e..494f26f3a 100644 --- a/bazel/ts_project/strict_deps/test/BUILD.bazel +++ b/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", ], diff --git a/bazel/ts_project/strict_deps/test/import_from_mts_cts_extensions/BUILD.bazel b/bazel/ts_project/strict_deps/test/import_from_mts_cts_extensions/BUILD.bazel index 6e4ec953e..e6b77542c 100644 --- a/bazel/ts_project/strict_deps/test/import_from_mts_cts_extensions/BUILD.bazel +++ b/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"], ) diff --git a/bazel/ts_project/strict_deps/tsconfig.mts b/bazel/ts_project/strict_deps/tsconfig.mts new file mode 100644 index 000000000..9ac5b7f60 --- /dev/null +++ b/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; +} diff --git a/bazel/ts_project/strict_deps/visitor.mts b/bazel/ts_project/strict_deps/visitor.mts index deeaeddd7..5c9cc7505 100644 --- a/bazel/ts_project/strict_deps/visitor.mts +++ b/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 {