Skip to content

Commit 6c86ba1

Browse files
committed
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
1 parent 72f5a5f commit 6c86ba1

File tree

5 files changed

+90
-11
lines changed

5 files changed

+90
-11
lines changed

bazel/ts_project/strict_deps/diagnostic.mts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
19
import ts from 'typescript';
210

311
export function createDiagnostic(message: string, node: ts.Node): ts.Diagnostic {

bazel/ts_project/strict_deps/index.bzl

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ def _strict_deps_impl(ctx):
5353
"testFiles": test_files,
5454
"allowedModuleNames": allowed_module_names,
5555
"allowedSources": allowed_sources,
56+
# The tsconfig from rules_ts has a single src so we know it will be the first file.
57+
"tsconfigPath": ctx.files.tsconfig[0].short_path,
5658
}),
5759
)
5860

@@ -85,14 +87,21 @@ def _strict_deps_impl(ctx):
8587
),
8688
)
8789

88-
bin_runfiles = ctx.attr._bin[DefaultInfo].default_runfiles
90+
runfiles = ctx.runfiles(
91+
files = [
92+
manifest,
93+
] + ctx.files.srcs +
94+
ctx.files._runfiles_lib +
95+
ctx.files.tsconfig,
96+
).merge_all([
97+
ctx.attr._bin[DefaultInfo].default_runfiles,
98+
ctx.attr.tsconfig[DefaultInfo].default_runfiles,
99+
])
89100

90101
return [
91102
DefaultInfo(
92103
executable = launcher,
93-
runfiles = ctx.runfiles(
94-
files = ctx.files._runfiles_lib + ctx.files.srcs + [manifest],
95-
).merge(bin_runfiles),
104+
runfiles = runfiles,
96105
),
97106
]
98107

@@ -108,8 +117,13 @@ _strict_deps_test = rule(
108117
),
109118
"srcs": attr.label_list(
110119
doc = "TS files to be checked",
120+
mandatory = True,
111121
allow_files = True,
122+
),
123+
"tsconfig": attr.label(
124+
doc = "The tsconfig of the ts_project being checked",
112125
mandatory = True,
126+
allow_files = True,
113127
),
114128
"will_fail": attr.bool(
115129
doc = "Whether the test is expected to fail",

bazel/ts_project/strict_deps/index.mts

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
19
import fs from 'node:fs/promises';
210
import path from 'node:path';
311
import ts from 'typescript';
412
import {createDiagnostic} from './diagnostic.mjs';
513
import {StrictDepsManifest} from './manifest.mjs';
614
import {getImportsInSourceFile} from './visitor.mjs';
15+
import {readTsConfig} from './tsconfig.mjs';
716

817
const [manifestExecPath, expectedFailureRaw] = process.argv.slice(2);
918
const expectedFailure = expectedFailureRaw === 'true';
@@ -13,22 +22,45 @@ const manifest: StrictDepsManifest = JSON.parse(await fs.readFile(manifestExecPa
1322
/**
1423
* Regex matcher to extract a npm package name, potentially with scope from a subpackage import path.
1524
*/
16-
const moduleSpeciferMatcher = /^(@[\w\d-_]+\/)?([\w\d-_]+)/;
17-
const extensionRemoveRegex = /\.[mc]?(js|(d\.)?[mc]?ts)$/;
18-
const allowedModuleNames = new Set<string>(manifest.allowedModuleNames);
25+
const moduleSpeciferMatcher = /^(@[\w\d-_\.]+\/)?([\w\d-_\.]+)/;
26+
const extensionRemoveRegex = /\.[mc]?(js|(d\.)?[mc]?tsx?)$/;
27+
const allowedModuleNames = new Set<string>(
28+
manifest.allowedModuleNames.map((m) => {
29+
return (
30+
m
31+
// Scoped types from DefinitelyTyped are split using a __ delimiter, so we put it back together.
32+
.replace(/(?:@types\/)(.*)__(.*)/, '@$1/$2')
33+
// Replace any unscoped types package from DefinitelyTyped with just to package name.
34+
.replace(/(?:@types\/)(.*)/, '$1')
35+
);
36+
}),
37+
);
1938
const allowedSources = new Set<string>(
2039
manifest.allowedSources.map((s) => s.replace(extensionRemoveRegex, '')),
2140
);
22-
41+
const tsconfig = readTsConfig(path.join(process.cwd(), manifest.tsconfigPath));
2342
const diagnostics: ts.Diagnostic[] = [];
2443

44+
/** Check if the moduleSpecifier matches any of the provided paths. */
45+
function checkPathsForMatch(moduleSpecifier: string, paths?: ts.MapLike<string[]>): boolean {
46+
for (const matcher of Object.keys(paths || {})) {
47+
if (moduleSpecifier.match(matcher)) {
48+
return true;
49+
}
50+
}
51+
return false;
52+
}
53+
2554
for (const fileExecPath of manifest.testFiles) {
2655
const content = await fs.readFile(fileExecPath, 'utf8');
2756
const sf = ts.createSourceFile(fileExecPath, content, ts.ScriptTarget.ESNext, true);
2857
const imports = getImportsInSourceFile(sf);
2958

3059
for (const i of imports) {
31-
const moduleSpecifier = i.moduleSpecifier.replace(extensionRemoveRegex, '');
60+
const moduleSpecifier =
61+
i.moduleSpecifier === 'zone.js'
62+
? 'zone.js'
63+
: i.moduleSpecifier.replace(extensionRemoveRegex, '');
3264
// When the module specified is the file itself this is always a valid dep.
3365
if (i.moduleSpecifier === '') {
3466
continue;
@@ -44,16 +76,24 @@ for (const fileExecPath of manifest.testFiles) {
4476
}
4577
}
4678

47-
if (moduleSpecifier.startsWith('node:') && allowedModuleNames.has('@types/node')) {
79+
if (
80+
moduleSpecifier.startsWith('node:') &&
81+
(allowedModuleNames.has('node') || tsconfig.options.types?.includes('node'))
82+
) {
4883
continue;
4984
}
5085

5186
if (
52-
allowedModuleNames.has(moduleSpecifier.match(moduleSpeciferMatcher)?.[0] || moduleSpecifier)
87+
allowedModuleNames.has(moduleSpecifier.match(moduleSpeciferMatcher)?.[0] || '') ||
88+
allowedModuleNames.has(moduleSpecifier)
5389
) {
5490
continue;
5591
}
5692

93+
if (checkPathsForMatch(moduleSpecifier, tsconfig.options.paths)) {
94+
continue;
95+
}
96+
5797
diagnostics.push(
5898
createDiagnostic(`No explicit Bazel dependency for this module.`, i.diagnosticNode),
5999
);
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
19
export interface StrictDepsManifest {
210
allowedModuleNames: string[];
311
allowedSources: string[];
412
testFiles: string[];
13+
tsconfigPath: string;
514
}

bazel/ts_project/strict_deps/visitor.mts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
19
import ts from 'typescript';
210

311
export interface Import {

0 commit comments

Comments
 (0)