From 72d015e263581cb7602bb5f7036c3416867ebeb6 Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Tue, 18 Feb 2025 15:23:45 +0000 Subject: [PATCH] feat(bazel): create strict_deps test for ts_project based build targets Checks to ensure that all of the dependencies relied on within the ts_project sources are directly provided as dependencies within that target, rather than being provided transitively. --- bazel/BUILD.bazel | 1 + bazel/ts_project/BUILD.bazel | 9 ++ bazel/ts_project/index.bzl | 3 + bazel/ts_project/strict_deps/BUILD.bazel | 26 ++++ bazel/ts_project/strict_deps/diagnostic.mts | 12 ++ bazel/ts_project/strict_deps/index.bzl | 135 ++++++++++++++++++ bazel/ts_project/strict_deps/index.mts | 77 ++++++++++ bazel/ts_project/strict_deps/manifest.mts | 5 + bazel/ts_project/strict_deps/test/BUILD.bazel | 56 ++++++++ .../strict_deps/test/depth/BUILD.bazel | 8 ++ .../ts_project/strict_deps/test/depth/file.ts | 1 + .../strict_deps/test/depth/tsconfig.json | 7 + .../strict_deps/test/import_from_depth.ts | 3 + .../strict_deps/test/import_node_module.ts | 4 + .../strict_deps/test/import_npm_module.ts | 3 + .../test/import_npm_module/BUILD.bazel | 10 ++ .../test/import_npm_module/index.ts | 3 + .../test/import_npm_module/tsconfig.json | 6 + .../test/sibling_import_from_depth.ts | 3 + .../strict_deps/test/transitive_from_depth.ts | 1 + .../ts_project/strict_deps/test/tsconfig.json | 6 + bazel/ts_project/strict_deps/tsconfig.json | 9 ++ bazel/ts_project/strict_deps/visitor.mts | 40 ++++++ 23 files changed, 428 insertions(+) create mode 100644 bazel/ts_project/BUILD.bazel create mode 100644 bazel/ts_project/index.bzl create mode 100644 bazel/ts_project/strict_deps/BUILD.bazel create mode 100644 bazel/ts_project/strict_deps/diagnostic.mts create mode 100644 bazel/ts_project/strict_deps/index.bzl create mode 100644 bazel/ts_project/strict_deps/index.mts create mode 100644 bazel/ts_project/strict_deps/manifest.mts create mode 100644 bazel/ts_project/strict_deps/test/BUILD.bazel create mode 100644 bazel/ts_project/strict_deps/test/depth/BUILD.bazel create mode 100644 bazel/ts_project/strict_deps/test/depth/file.ts create mode 100644 bazel/ts_project/strict_deps/test/depth/tsconfig.json create mode 100644 bazel/ts_project/strict_deps/test/import_from_depth.ts create mode 100644 bazel/ts_project/strict_deps/test/import_node_module.ts create mode 100644 bazel/ts_project/strict_deps/test/import_npm_module.ts create mode 100644 bazel/ts_project/strict_deps/test/import_npm_module/BUILD.bazel create mode 100644 bazel/ts_project/strict_deps/test/import_npm_module/index.ts create mode 100644 bazel/ts_project/strict_deps/test/import_npm_module/tsconfig.json create mode 100644 bazel/ts_project/strict_deps/test/sibling_import_from_depth.ts create mode 100644 bazel/ts_project/strict_deps/test/transitive_from_depth.ts create mode 100644 bazel/ts_project/strict_deps/test/tsconfig.json create mode 100644 bazel/ts_project/strict_deps/tsconfig.json create mode 100644 bazel/ts_project/strict_deps/visitor.mts diff --git a/bazel/BUILD.bazel b/bazel/BUILD.bazel index 927517910..34d85382f 100644 --- a/bazel/BUILD.bazel +++ b/bazel/BUILD.bazel @@ -33,6 +33,7 @@ filegroup( "//bazel/private:files", "//bazel/remote-execution:files", "//bazel/spec-bundling:files", + "//bazel/ts_project:files", ], visibility = ["//:npm"], ) diff --git a/bazel/ts_project/BUILD.bazel b/bazel/ts_project/BUILD.bazel new file mode 100644 index 000000000..e25fbc468 --- /dev/null +++ b/bazel/ts_project/BUILD.bazel @@ -0,0 +1,9 @@ +package(default_visibility = ["//visibility:public"]) + +# Make source files available for distribution +filegroup( + name = "files", + srcs = glob(["*"]) + [ + "//bazel/ts_project/strict_deps:files", + ], +) diff --git a/bazel/ts_project/index.bzl b/bazel/ts_project/index.bzl new file mode 100644 index 000000000..8bce3696f --- /dev/null +++ b/bazel/ts_project/index.bzl @@ -0,0 +1,3 @@ +load("//bazel/ts_project/strict_deps:index.bzl", _strict_deps_test = "strict_deps_test") + +strict_deps_test = _strict_deps_test diff --git a/bazel/ts_project/strict_deps/BUILD.bazel b/bazel/ts_project/strict_deps/BUILD.bazel new file mode 100644 index 000000000..8ca3bd0a4 --- /dev/null +++ b/bazel/ts_project/strict_deps/BUILD.bazel @@ -0,0 +1,26 @@ +load("@aspect_rules_js//js:defs.bzl", "js_binary") +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +package(default_visibility = ["//visibility:public"]) + +# Make source files available for distribution +filegroup( + name = "files", + srcs = glob(["*"]), +) + +ts_project( + name = "lib", + srcs = glob(["*.mts"]), + deps = [ + "//bazel:node_modules/@types/node", + "//bazel:node_modules/typescript", + ], +) + +js_binary( + name = "bin", + data = [":lib"], + entry_point = ":index.mjs", + visibility = ["//visibility:public"], +) diff --git a/bazel/ts_project/strict_deps/diagnostic.mts b/bazel/ts_project/strict_deps/diagnostic.mts new file mode 100644 index 000000000..2bc222205 --- /dev/null +++ b/bazel/ts_project/strict_deps/diagnostic.mts @@ -0,0 +1,12 @@ +import ts from 'typescript'; + +export function createDiagnostic(message: string, node: ts.Node): ts.Diagnostic { + return { + category: ts.DiagnosticCategory.Error, + code: -1, + file: node.getSourceFile(), + start: node.getStart(), + length: node.getWidth(), + messageText: message, + }; +} diff --git a/bazel/ts_project/strict_deps/index.bzl b/bazel/ts_project/strict_deps/index.bzl new file mode 100644 index 000000000..451277c8e --- /dev/null +++ b/bazel/ts_project/strict_deps/index.bzl @@ -0,0 +1,135 @@ +load("@aspect_rules_js//js:providers.bzl", "JsInfo") + +# A custom provider to pass along the npm package name for linked npm packages +NpmPackage = provider() + +def _npm_package_aspect_impl(target, ctx): + if (ctx.rule.kind == "npm_link_package_store"): + package_name = ctx.rule.attr.package + + # TODO: Determine how to include the package field information in locally built npm package targets + if package_name == "": + package_name = target[JsInfo].npm_package_store_infos.to_list()[0].package + return [NpmPackage(name = package_name)] + return [] + +# Aspect to include the npm package name for use in strict deps checking. +_npm_package_aspect = aspect( + implementation = _npm_package_aspect_impl, + required_providers = [], +) + +def _strict_deps_impl(ctx): + sources = [] + + allowed_sources = [] + allowed_module_names = [] + test_files = [] + + # Whether or not the strict_deps check is expected to fail. + expect_failure = "true" if ctx.attr.will_fail else "false" + + for dep in ctx.attr.deps: + if JsInfo in dep: + # Because each source maps to a corresponding type file, we can simply look at the type + # files for the sources, this also allows for situations in which we only provide types. + sources.append(dep[JsInfo].types) + if NpmPackage in dep: + allowed_module_names.append(dep[NpmPackage].name) + + for source in depset(transitive = sources).to_list(): + allowed_sources.append(source.short_path) + + for file in ctx.files.srcs: + allowed_sources.append(file.short_path) + if file.is_source: + test_files.append(file.short_path) + + manifest = ctx.actions.declare_file("%s_strict_deps_manifest.json" % ctx.attr.name) + ctx.actions.write( + output = manifest, + content = json.encode({ + # Note: Ensure this matches `StrictDepsManifest` from `manifest.mts` + "testFiles": test_files, + "allowedModuleNames": allowed_module_names, + "allowedSources": allowed_sources, + }), + ) + + launcher = ctx.actions.declare_file("%s_launcher.sh" % ctx.attr.name) + ctx.actions.write( + output = launcher, + is_executable = True, + # Bash runfile library taken from: + # https://github.com/bazelbuild/bazel/blob/master/tools/bash/runfiles/runfiles.bash. + content = """ + #!/usr/bin/env bash + + # --- begin runfiles.bash initialization v3 --- + # Copy-pasted from the Bazel Bash runfiles library v3. + set -uo pipefail; set +e; f=bazel_tools/tools/bash/runfiles/runfiles.bash + # shellcheck disable=SC1090 + source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \ + source "$0.runfiles/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e + # --- end runfiles.bash initialization v3 --- + + exec $(rlocation %s) $(rlocation %s) %s + """ % ( + "%s/%s" % (ctx.workspace_name, ctx.files._bin[0].short_path), + "%s/%s" % (ctx.workspace_name, manifest.short_path), + expect_failure, + ), + ) + + bin_runfiles = ctx.attr._bin[DefaultInfo].default_runfiles + + return [ + DefaultInfo( + executable = launcher, + runfiles = ctx.runfiles( + files = ctx.files._runfiles_lib + ctx.files.srcs + [manifest], + ).merge(bin_runfiles), + ), + ] + +_strict_deps_test = rule( + implementation = _strict_deps_impl, + test = True, + doc = "Rule to verify that specified TS files only import from explicitly listed deps.", + attrs = { + "deps": attr.label_list( + aspects = [_npm_package_aspect], + doc = "Direct dependencies that are allowed", + default = [], + ), + "srcs": attr.label_list( + doc = "TS files to be checked", + allow_files = True, + mandatory = True, + ), + "will_fail": attr.bool( + doc = "Whether the test is expected to fail", + default = False, + ), + "_runfiles_lib": attr.label( + default = "@bazel_tools//tools/bash/runfiles", + ), + "_bin": attr.label( + default = "@devinfra//bazel/ts_project/strict_deps:bin", + executable = True, + cfg = "exec", + ), + }, +) + +def strict_deps_test(**kwargs): + kwargs["will_fail"] = False + _strict_deps_test(**kwargs) + +def invalid_strict_deps_test(**kwargs): + kwargs["will_fail"] = True + _strict_deps_test(**kwargs) diff --git a/bazel/ts_project/strict_deps/index.mts b/bazel/ts_project/strict_deps/index.mts new file mode 100644 index 000000000..7364b6fc6 --- /dev/null +++ b/bazel/ts_project/strict_deps/index.mts @@ -0,0 +1,77 @@ +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'; + +const [manifestExecPath, expectedFailureRaw] = process.argv.slice(2); +const expectedFailure = expectedFailureRaw === 'true'; + +const manifest: StrictDepsManifest = JSON.parse(await fs.readFile(manifestExecPath, 'utf8')); + +/** + * 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\.)?ts)$/; +const allowedModuleNames = new Set(manifest.allowedModuleNames); +const allowedSources = new Set( + manifest.allowedSources.map((s) => s.replace(extensionRemoveRegex, '')), +); + +const diagnostics: ts.Diagnostic[] = []; + +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, ''); + // When the module specified is the file itself this is always a valid dep. + if (i.moduleSpecifier === '') { + continue; + } + if (moduleSpecifier.startsWith('.')) { + const targetFilePath = path.posix.join( + path.dirname(i.diagnosticNode.getSourceFile().fileName), + moduleSpecifier, + ); + + if (allowedSources.has(targetFilePath) || allowedSources.has(`${targetFilePath}/index`)) { + continue; + } + } + + if (moduleSpecifier.startsWith('node:') && allowedModuleNames.has('@types/node')) { + continue; + } + + if ( + allowedModuleNames.has(moduleSpecifier.match(moduleSpeciferMatcher)?.[0] || moduleSpecifier) + ) { + continue; + } + + diagnostics.push( + createDiagnostic(`No explicit Bazel dependency for this module.`, i.diagnosticNode), + ); + } +} + +if (diagnostics.length > 0) { + const formattedDiagnostics = ts.formatDiagnosticsWithColorAndContext(diagnostics, { + getCanonicalFileName: (f) => f, + getCurrentDirectory: () => '', + getNewLine: () => '\n', + }); + console.error(formattedDiagnostics); + process.exitCode = 1; +} + +if (expectedFailure && process.exitCode !== 0) { + console.log('Strict deps testing was marked as expected to fail, marking test as passing.'); + // Force the exit code back to 0 as the process was expected to fail. + process.exitCode = 0; +} diff --git a/bazel/ts_project/strict_deps/manifest.mts b/bazel/ts_project/strict_deps/manifest.mts new file mode 100644 index 000000000..ab547c130 --- /dev/null +++ b/bazel/ts_project/strict_deps/manifest.mts @@ -0,0 +1,5 @@ +export interface StrictDepsManifest { + allowedModuleNames: string[]; + allowedSources: string[]; + testFiles: string[]; +} diff --git a/bazel/ts_project/strict_deps/test/BUILD.bazel b/bazel/ts_project/strict_deps/test/BUILD.bazel new file mode 100644 index 000000000..d4f3d7d2e --- /dev/null +++ b/bazel/ts_project/strict_deps/test/BUILD.bazel @@ -0,0 +1,56 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") +load("//bazel/ts_project/strict_deps:index.bzl", "invalid_strict_deps_test", "strict_deps_test") + +ts_project( + name = "sibling_import_from_depth", + srcs = ["sibling_import_from_depth.ts"], + deps = [ + "//bazel/ts_project/strict_deps/test/depth", + ], +) + +strict_deps_test( + name = "import_node_module", + srcs = ["import_node_module.ts"], + deps = [ + "//bazel:node_modules/@types/node", + ], +) + +invalid_strict_deps_test( + name = "invalid_import_node_module", + srcs = ["import_node_module.ts"], +) + +strict_deps_test( + name = "import_npm_module", + srcs = ["import_npm_module.ts"], + deps = ["//bazel:node_modules/@microsoft/api-extractor"], +) + +invalid_strict_deps_test( + name = "invalid_import_npm_module_transitively", + srcs = ["import_npm_module.ts"], + deps = [ + "//bazel/ts_project/strict_deps/test/import_npm_module", + ], +) + +invalid_strict_deps_test( + name = "invalid_import_npm_module", + srcs = ["import_npm_module.ts"], +) + +strict_deps_test( + name = "import_from_depth", + srcs = ["import_from_depth.ts"], + deps = ["//bazel/ts_project/strict_deps/test/depth"], +) + +invalid_strict_deps_test( + name = "invalid_import_from_depth", + srcs = ["import_from_depth.ts"], + deps = [ + ":sibling_import_from_depth", + ], +) diff --git a/bazel/ts_project/strict_deps/test/depth/BUILD.bazel b/bazel/ts_project/strict_deps/test/depth/BUILD.bazel new file mode 100644 index 000000000..dc7818df9 --- /dev/null +++ b/bazel/ts_project/strict_deps/test/depth/BUILD.bazel @@ -0,0 +1,8 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +ts_project( + name = "depth", + srcs = ["file.ts"], + declaration = True, + visibility = ["//visibility:public"], +) diff --git a/bazel/ts_project/strict_deps/test/depth/file.ts b/bazel/ts_project/strict_deps/test/depth/file.ts new file mode 100644 index 000000000..cd2dcde1e --- /dev/null +++ b/bazel/ts_project/strict_deps/test/depth/file.ts @@ -0,0 +1 @@ +export const depthValue = 42; diff --git a/bazel/ts_project/strict_deps/test/depth/tsconfig.json b/bazel/ts_project/strict_deps/test/depth/tsconfig.json new file mode 100644 index 000000000..94205b059 --- /dev/null +++ b/bazel/ts_project/strict_deps/test/depth/tsconfig.json @@ -0,0 +1,7 @@ +{ + "compilerOptions": { + "strict": true, + "lib": ["esnext", "DOM"], + "declaration": true + } +} diff --git a/bazel/ts_project/strict_deps/test/import_from_depth.ts b/bazel/ts_project/strict_deps/test/import_from_depth.ts new file mode 100644 index 000000000..2a3affeca --- /dev/null +++ b/bazel/ts_project/strict_deps/test/import_from_depth.ts @@ -0,0 +1,3 @@ +import {depthValue} from './depth/file'; + +console.log(`The value from deeper down is ${depthValue}`); diff --git a/bazel/ts_project/strict_deps/test/import_node_module.ts b/bazel/ts_project/strict_deps/test/import_node_module.ts new file mode 100644 index 000000000..12ff64316 --- /dev/null +++ b/bazel/ts_project/strict_deps/test/import_node_module.ts @@ -0,0 +1,4 @@ +import {basename, join} from 'node:path'; +import {cwd} from 'node:process'; + +join(basename(cwd()), 'test.txt'); diff --git a/bazel/ts_project/strict_deps/test/import_npm_module.ts b/bazel/ts_project/strict_deps/test/import_npm_module.ts new file mode 100644 index 000000000..b349d2182 --- /dev/null +++ b/bazel/ts_project/strict_deps/test/import_npm_module.ts @@ -0,0 +1,3 @@ +import {Extractor} from '@microsoft/api-extractor'; + +export const AnExtractor = Extractor; diff --git a/bazel/ts_project/strict_deps/test/import_npm_module/BUILD.bazel b/bazel/ts_project/strict_deps/test/import_npm_module/BUILD.bazel new file mode 100644 index 000000000..911045acd --- /dev/null +++ b/bazel/ts_project/strict_deps/test/import_npm_module/BUILD.bazel @@ -0,0 +1,10 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +ts_project( + name = "import_npm_module", + srcs = ["index.ts"], + visibility = ["//visibility:public"], + deps = [ + "//bazel:node_modules/@microsoft/api-extractor", + ], +) diff --git a/bazel/ts_project/strict_deps/test/import_npm_module/index.ts b/bazel/ts_project/strict_deps/test/import_npm_module/index.ts new file mode 100644 index 000000000..36fe84eb4 --- /dev/null +++ b/bazel/ts_project/strict_deps/test/import_npm_module/index.ts @@ -0,0 +1,3 @@ +import {Extractor} from '@microsoft/api-extractor'; + +console.log(Extractor); diff --git a/bazel/ts_project/strict_deps/test/import_npm_module/tsconfig.json b/bazel/ts_project/strict_deps/test/import_npm_module/tsconfig.json new file mode 100644 index 000000000..93c008b62 --- /dev/null +++ b/bazel/ts_project/strict_deps/test/import_npm_module/tsconfig.json @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "strict": true, + "lib": ["esnext", "DOM"] + } +} diff --git a/bazel/ts_project/strict_deps/test/sibling_import_from_depth.ts b/bazel/ts_project/strict_deps/test/sibling_import_from_depth.ts new file mode 100644 index 000000000..0b0baacdd --- /dev/null +++ b/bazel/ts_project/strict_deps/test/sibling_import_from_depth.ts @@ -0,0 +1,3 @@ +import {depthValue} from './depth/file'; + +console.log(depthValue); diff --git a/bazel/ts_project/strict_deps/test/transitive_from_depth.ts b/bazel/ts_project/strict_deps/test/transitive_from_depth.ts new file mode 100644 index 000000000..e80e6e247 --- /dev/null +++ b/bazel/ts_project/strict_deps/test/transitive_from_depth.ts @@ -0,0 +1 @@ +export {depthValue} from './depth/file'; diff --git a/bazel/ts_project/strict_deps/test/tsconfig.json b/bazel/ts_project/strict_deps/test/tsconfig.json new file mode 100644 index 000000000..93c008b62 --- /dev/null +++ b/bazel/ts_project/strict_deps/test/tsconfig.json @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "strict": true, + "lib": ["esnext", "DOM"] + } +} diff --git a/bazel/ts_project/strict_deps/tsconfig.json b/bazel/ts_project/strict_deps/tsconfig.json new file mode 100644 index 000000000..c553a5d72 --- /dev/null +++ b/bazel/ts_project/strict_deps/tsconfig.json @@ -0,0 +1,9 @@ +{ + "compilerOptions": { + "strict": true, + "module": "NodeNext", + "moduleResolution": "nodenext", + "types": ["node"] + }, + "files": ["index.mts"] +} diff --git a/bazel/ts_project/strict_deps/visitor.mts b/bazel/ts_project/strict_deps/visitor.mts new file mode 100644 index 000000000..deeaeddd7 --- /dev/null +++ b/bazel/ts_project/strict_deps/visitor.mts @@ -0,0 +1,40 @@ +import ts from 'typescript'; + +export interface Import { + diagnosticNode: ts.Node; + moduleSpecifier: string; +} + +export function getImportsInSourceFile(sf: ts.SourceFile): Import[] { + const result: Import[] = []; + + const visitor = (node: ts.Node) => { + if (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) { + const moduleSpecifier = node.moduleSpecifier as ts.StringLiteral; + // If not moduleSpecifier is included in the declaration, it is infered to be the local file, + // essentially a self import and can be ignored. + if (moduleSpecifier) { + result.push({ + diagnosticNode: moduleSpecifier, + moduleSpecifier: moduleSpecifier.text, + }); + } + } + if ( + ts.isCallExpression(node) && + node.expression.kind === ts.SyntaxKind.ImportKeyword && + node.arguments.length >= 1 && + ts.isStringLiteralLike(node.arguments[0]) + ) { + result.push({ + diagnosticNode: node, + moduleSpecifier: node.arguments[0].text, + }); + } + ts.forEachChild(node, visitor); + }; + + ts.forEachChild(sf, visitor); + + return result; +}