Skip to content

Commit 84d4a73

Browse files
committed
feat(bazel): create strict_deps test for ts_project based build targets (#2597)
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. PR Close #2597
1 parent 2b69897 commit 84d4a73

23 files changed

+428
-0
lines changed

bazel/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ filegroup(
3333
"//bazel/private:files",
3434
"//bazel/remote-execution:files",
3535
"//bazel/spec-bundling:files",
36+
"//bazel/ts_project:files",
3637
],
3738
visibility = ["//:npm"],
3839
)

bazel/ts_project/BUILD.bazel

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package(default_visibility = ["//visibility:public"])
2+
3+
# Make source files available for distribution
4+
filegroup(
5+
name = "files",
6+
srcs = glob(["*"]) + [
7+
"//bazel/ts_project/strict_deps:files",
8+
],
9+
)

bazel/ts_project/index.bzl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
load("//bazel/ts_project/strict_deps:index.bzl", _strict_deps_test = "strict_deps_test")
2+
3+
strict_deps_test = _strict_deps_test
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
load("@aspect_rules_js//js:defs.bzl", "js_binary")
2+
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")
3+
4+
package(default_visibility = ["//visibility:public"])
5+
6+
# Make source files available for distribution
7+
filegroup(
8+
name = "files",
9+
srcs = glob(["*"]),
10+
)
11+
12+
ts_project(
13+
name = "lib",
14+
srcs = glob(["*.mts"]),
15+
deps = [
16+
"//bazel:node_modules/@types/node",
17+
"//bazel:node_modules/typescript",
18+
],
19+
)
20+
21+
js_binary(
22+
name = "bin",
23+
data = [":lib"],
24+
entry_point = ":index.mjs",
25+
visibility = ["//visibility:public"],
26+
)
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import ts from 'typescript';
2+
3+
export function createDiagnostic(message: string, node: ts.Node): ts.Diagnostic {
4+
return {
5+
category: ts.DiagnosticCategory.Error,
6+
code: -1,
7+
file: node.getSourceFile(),
8+
start: node.getStart(),
9+
length: node.getWidth(),
10+
messageText: message,
11+
};
12+
}
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
load("@aspect_rules_js//js:providers.bzl", "JsInfo")
2+
3+
# A custom provider to pass along the npm package name for linked npm packages
4+
NpmPackage = provider()
5+
6+
def _npm_package_aspect_impl(target, ctx):
7+
if (ctx.rule.kind == "npm_link_package_store"):
8+
package_name = ctx.rule.attr.package
9+
10+
# TODO: Determine how to include the package field information in locally built npm package targets
11+
if package_name == "":
12+
package_name = target[JsInfo].npm_package_store_infos.to_list()[0].package
13+
return [NpmPackage(name = package_name)]
14+
return []
15+
16+
# Aspect to include the npm package name for use in strict deps checking.
17+
_npm_package_aspect = aspect(
18+
implementation = _npm_package_aspect_impl,
19+
required_providers = [],
20+
)
21+
22+
def _strict_deps_impl(ctx):
23+
sources = []
24+
25+
allowed_sources = []
26+
allowed_module_names = []
27+
test_files = []
28+
29+
# Whether or not the strict_deps check is expected to fail.
30+
expect_failure = "true" if ctx.attr.will_fail else "false"
31+
32+
for dep in ctx.attr.deps:
33+
if JsInfo in dep:
34+
# Because each source maps to a corresponding type file, we can simply look at the type
35+
# files for the sources, this also allows for situations in which we only provide types.
36+
sources.append(dep[JsInfo].types)
37+
if NpmPackage in dep:
38+
allowed_module_names.append(dep[NpmPackage].name)
39+
40+
for source in depset(transitive = sources).to_list():
41+
allowed_sources.append(source.short_path)
42+
43+
for file in ctx.files.srcs:
44+
allowed_sources.append(file.short_path)
45+
if file.is_source:
46+
test_files.append(file.short_path)
47+
48+
manifest = ctx.actions.declare_file("%s_strict_deps_manifest.json" % ctx.attr.name)
49+
ctx.actions.write(
50+
output = manifest,
51+
content = json.encode({
52+
# Note: Ensure this matches `StrictDepsManifest` from `manifest.mts`
53+
"testFiles": test_files,
54+
"allowedModuleNames": allowed_module_names,
55+
"allowedSources": allowed_sources,
56+
}),
57+
)
58+
59+
launcher = ctx.actions.declare_file("%s_launcher.sh" % ctx.attr.name)
60+
ctx.actions.write(
61+
output = launcher,
62+
is_executable = True,
63+
# Bash runfile library taken from:
64+
# https://github.com/bazelbuild/bazel/blob/master/tools/bash/runfiles/runfiles.bash.
65+
content = """
66+
#!/usr/bin/env bash
67+
68+
# --- begin runfiles.bash initialization v3 ---
69+
# Copy-pasted from the Bazel Bash runfiles library v3.
70+
set -uo pipefail; set +e; f=bazel_tools/tools/bash/runfiles/runfiles.bash
71+
# shellcheck disable=SC1090
72+
source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \
73+
source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \
74+
source "$0.runfiles/$f" 2>/dev/null || \
75+
source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
76+
source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
77+
{ echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e
78+
# --- end runfiles.bash initialization v3 ---
79+
80+
exec $(rlocation %s) $(rlocation %s) %s
81+
""" % (
82+
"%s/%s" % (ctx.workspace_name, ctx.files._bin[0].short_path),
83+
"%s/%s" % (ctx.workspace_name, manifest.short_path),
84+
expect_failure,
85+
),
86+
)
87+
88+
bin_runfiles = ctx.attr._bin[DefaultInfo].default_runfiles
89+
90+
return [
91+
DefaultInfo(
92+
executable = launcher,
93+
runfiles = ctx.runfiles(
94+
files = ctx.files._runfiles_lib + ctx.files.srcs + [manifest],
95+
).merge(bin_runfiles),
96+
),
97+
]
98+
99+
_strict_deps_test = rule(
100+
implementation = _strict_deps_impl,
101+
test = True,
102+
doc = "Rule to verify that specified TS files only import from explicitly listed deps.",
103+
attrs = {
104+
"deps": attr.label_list(
105+
aspects = [_npm_package_aspect],
106+
doc = "Direct dependencies that are allowed",
107+
default = [],
108+
),
109+
"srcs": attr.label_list(
110+
doc = "TS files to be checked",
111+
allow_files = True,
112+
mandatory = True,
113+
),
114+
"will_fail": attr.bool(
115+
doc = "Whether the test is expected to fail",
116+
default = False,
117+
),
118+
"_runfiles_lib": attr.label(
119+
default = "@bazel_tools//tools/bash/runfiles",
120+
),
121+
"_bin": attr.label(
122+
default = "@devinfra//bazel/ts_project/strict_deps:bin",
123+
executable = True,
124+
cfg = "exec",
125+
),
126+
},
127+
)
128+
129+
def strict_deps_test(**kwargs):
130+
kwargs["will_fail"] = False
131+
_strict_deps_test(**kwargs)
132+
133+
def invalid_strict_deps_test(**kwargs):
134+
kwargs["will_fail"] = True
135+
_strict_deps_test(**kwargs)
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import fs from 'node:fs/promises';
2+
import path from 'node:path';
3+
import ts from 'typescript';
4+
import {createDiagnostic} from './diagnostic.mjs';
5+
import {StrictDepsManifest} from './manifest.mjs';
6+
import {getImportsInSourceFile} from './visitor.mjs';
7+
8+
const [manifestExecPath, expectedFailureRaw] = process.argv.slice(2);
9+
const expectedFailure = expectedFailureRaw === 'true';
10+
11+
const manifest: StrictDepsManifest = JSON.parse(await fs.readFile(manifestExecPath, 'utf8'));
12+
13+
/**
14+
* Regex matcher to extract a npm package name, potentially with scope from a subpackage import path.
15+
*/
16+
const moduleSpeciferMatcher = /^(@[\w\d-_]+\/)?([\w\d-_]+)/;
17+
const extensionRemoveRegex = /\.[mc]?(js|(d\.)?ts)$/;
18+
const allowedModuleNames = new Set<string>(manifest.allowedModuleNames);
19+
const allowedSources = new Set<string>(
20+
manifest.allowedSources.map((s) => s.replace(extensionRemoveRegex, '')),
21+
);
22+
23+
const diagnostics: ts.Diagnostic[] = [];
24+
25+
for (const fileExecPath of manifest.testFiles) {
26+
const content = await fs.readFile(fileExecPath, 'utf8');
27+
const sf = ts.createSourceFile(fileExecPath, content, ts.ScriptTarget.ESNext, true);
28+
const imports = getImportsInSourceFile(sf);
29+
30+
for (const i of imports) {
31+
const moduleSpecifier = i.moduleSpecifier.replace(extensionRemoveRegex, '');
32+
// When the module specified is the file itself this is always a valid dep.
33+
if (i.moduleSpecifier === '') {
34+
continue;
35+
}
36+
if (moduleSpecifier.startsWith('.')) {
37+
const targetFilePath = path.posix.join(
38+
path.dirname(i.diagnosticNode.getSourceFile().fileName),
39+
moduleSpecifier,
40+
);
41+
42+
if (allowedSources.has(targetFilePath) || allowedSources.has(`${targetFilePath}/index`)) {
43+
continue;
44+
}
45+
}
46+
47+
if (moduleSpecifier.startsWith('node:') && allowedModuleNames.has('@types/node')) {
48+
continue;
49+
}
50+
51+
if (
52+
allowedModuleNames.has(moduleSpecifier.match(moduleSpeciferMatcher)?.[0] || moduleSpecifier)
53+
) {
54+
continue;
55+
}
56+
57+
diagnostics.push(
58+
createDiagnostic(`No explicit Bazel dependency for this module.`, i.diagnosticNode),
59+
);
60+
}
61+
}
62+
63+
if (diagnostics.length > 0) {
64+
const formattedDiagnostics = ts.formatDiagnosticsWithColorAndContext(diagnostics, {
65+
getCanonicalFileName: (f) => f,
66+
getCurrentDirectory: () => '',
67+
getNewLine: () => '\n',
68+
});
69+
console.error(formattedDiagnostics);
70+
process.exitCode = 1;
71+
}
72+
73+
if (expectedFailure && process.exitCode !== 0) {
74+
console.log('Strict deps testing was marked as expected to fail, marking test as passing.');
75+
// Force the exit code back to 0 as the process was expected to fail.
76+
process.exitCode = 0;
77+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export interface StrictDepsManifest {
2+
allowedModuleNames: string[];
3+
allowedSources: string[];
4+
testFiles: string[];
5+
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")
2+
load("//bazel/ts_project/strict_deps:index.bzl", "invalid_strict_deps_test", "strict_deps_test")
3+
4+
ts_project(
5+
name = "sibling_import_from_depth",
6+
srcs = ["sibling_import_from_depth.ts"],
7+
deps = [
8+
"//bazel/ts_project/strict_deps/test/depth",
9+
],
10+
)
11+
12+
strict_deps_test(
13+
name = "import_node_module",
14+
srcs = ["import_node_module.ts"],
15+
deps = [
16+
"//bazel:node_modules/@types/node",
17+
],
18+
)
19+
20+
invalid_strict_deps_test(
21+
name = "invalid_import_node_module",
22+
srcs = ["import_node_module.ts"],
23+
)
24+
25+
strict_deps_test(
26+
name = "import_npm_module",
27+
srcs = ["import_npm_module.ts"],
28+
deps = ["//bazel:node_modules/@microsoft/api-extractor"],
29+
)
30+
31+
invalid_strict_deps_test(
32+
name = "invalid_import_npm_module_transitively",
33+
srcs = ["import_npm_module.ts"],
34+
deps = [
35+
"//bazel/ts_project/strict_deps/test/import_npm_module",
36+
],
37+
)
38+
39+
invalid_strict_deps_test(
40+
name = "invalid_import_npm_module",
41+
srcs = ["import_npm_module.ts"],
42+
)
43+
44+
strict_deps_test(
45+
name = "import_from_depth",
46+
srcs = ["import_from_depth.ts"],
47+
deps = ["//bazel/ts_project/strict_deps/test/depth"],
48+
)
49+
50+
invalid_strict_deps_test(
51+
name = "invalid_import_from_depth",
52+
srcs = ["import_from_depth.ts"],
53+
deps = [
54+
":sibling_import_from_depth",
55+
],
56+
)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")
2+
3+
ts_project(
4+
name = "depth",
5+
srcs = ["file.ts"],
6+
declaration = True,
7+
visibility = ["//visibility:public"],
8+
)

0 commit comments

Comments
 (0)