Skip to content

Commit 0042081

Browse files
authored
test(scripts): add cycle detector, cleanup (#8048)
1 parent 2981565 commit 0042081

16 files changed

Lines changed: 567 additions & 161 deletions

Makefile

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ test-types: reset-test-credentials
3737
npx tsc -p tsconfig.test.index-types.json
3838

3939
test-indices:
40-
node ./scripts/validation/client-index-tests.mjs
40+
node ./scripts/validation/client-indexes.mjs
4141

4242
test-protocols: bundles
4343
yarn g:vitest run -c vitest.config.protocols.integ.mts
@@ -66,7 +66,7 @@ test-integration: bundles
6666
make test-endpoints
6767

6868
api-snapshot:
69-
node ./scripts/validation/api-snapshot-validation.js
69+
node ./scripts/validation/api-snapshot.js
7070
git diff --exit-code scripts/validation/api.json
7171

7272
test-endpoints:
@@ -137,8 +137,8 @@ unbuilt:
137137
node scripts/build-only-unbuilt.js
138138

139139
static-analysis:
140-
node ./scripts/validation/no-generic-byte-arrays.js
141-
node ./scripts/validation/validate-all.js --all;
140+
node ./scripts/validation/generic-byte-arrays.js
141+
node ./scripts/validation/validate-all.js;
142142
make api-snapshot
143143

144144
# Clears the Turborepo local build cache

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
"generate:client:tarball:since": "node ./scripts/generate-client-tarball/generate-client-tarball-since.mjs",
3434
"generate:defaults-mode-provider": "./scripts/generate-defaults-mode-provider/index.js",
3535
"lerna:version": "yarn update:versions:current && node ./scripts/update-versions/bumpReleaseCandidates.mjs && yarn update:versions:default",
36-
"lint:api": "node scripts/validation/api-snapshot-validation.js",
36+
"lint:api": "node scripts/validation/api-snapshot.js",
3737
"lint:ci": "lerna exec --since origin/main --exclude-dependents --ignore '@aws-sdk/client-*' --ignore '@aws-sdk/aws-*' 'eslint --quiet src/**/*.ts'",
3838
"lint:dependencies": "node scripts/runtime-dependency-version-check/check-dependencies.js",
3939
"lint:release": "lerna exec --ignore '@aws-sdk/client-*' --ignore '@aws-sdk/aws-*' 'eslint --quiet src/**/*.ts'",
File renamed without changes.

scripts/validation/validate-banned-imports.js renamed to scripts/validation/banned-imports.js

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
#!/usr/bin/env node
22

33
/**
4-
* AST-based banned import checker for generated code (clients/, private/).
5-
* Enforces the same no-restricted-imports rules as .eslintrc.js but without
6-
* spawning eslint — much faster on 450+ packages.
4+
* AST-based banned import checker for dist-cjs and dist-es output.
5+
* Enforces no-restricted-imports rules via direct AST parsing.
76
*
8-
* Usage: node validate-banned-imports.js <packageDir> [...]
7+
* Usage: node banned-imports.js
98
*/
109

1110
const fs = require("node:fs");
1211
const path = require("node:path");
1312
const walk = require("../utils/walk");
14-
const { extractImports } = require("./validation-shared");
13+
const { extractImports, getPackageDirs } = require("./validation-shared");
1514

1615
const root = path.join(__dirname, "..", "..");
1716

@@ -144,14 +143,10 @@ async function validate(packageDir) {
144143
}
145144

146145
async function main() {
147-
const dirs = process.argv.slice(2);
148-
if (!dirs.length) {
149-
console.error("Usage: validate-banned-imports.js <packageDir> [...]");
150-
process.exit(1);
151-
}
146+
const packages = getPackageDirs();
152147
const errors = [];
153-
for (const dir of dirs) {
154-
errors.push(...(await validate(path.resolve(dir))));
148+
for (const { dir } of packages) {
149+
errors.push(...(await validate(dir)));
155150
}
156151
if (errors.length) {
157152
console.error(`❌ ${errors.length} banned import(s):\n ${[...new Set(errors)].join("\n ")}`);

scripts/validation/cycles.js

Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,283 @@
1+
#!/usr/bin/env node
2+
3+
/**
4+
* Detects cyclical dependencies at two levels:
5+
* 1. Module-level: cycles among @aws-sdk/* package imports across all packages.
6+
* 2. File-level: cycles among relative imports within each dist-cjs and dist-es folder.
7+
*
8+
* Usage: node cycles.js
9+
*/
10+
11+
const fs = require("node:fs");
12+
const path = require("node:path");
13+
const walk = require("../utils/walk");
14+
const { extractImports, getPackageName, resolveRelative, getPackageDirs } = require("./validation-shared");
15+
16+
/**
17+
* Finds all cycles in a directed graph using Tarjan's algorithm.
18+
*
19+
* @param graph - adjacency list (Map of node -> Set of neighbors).
20+
* @returns array of { cycle, sccSize } objects.
21+
*/
22+
function findAllCycles(graph) {
23+
const discoveryIndex = new Map();
24+
const lowlink = new Map();
25+
const onStack = new Set();
26+
const stack = [];
27+
let i = 0;
28+
const connectedComponents = [];
29+
30+
function visit(node) {
31+
discoveryIndex.set(node, i);
32+
lowlink.set(node, i);
33+
i++;
34+
stack.push(node);
35+
onStack.add(node);
36+
37+
for (const neighbor of graph.get(node) || []) {
38+
if (!discoveryIndex.has(neighbor)) {
39+
visit(neighbor);
40+
lowlink.set(node, Math.min(lowlink.get(node), lowlink.get(neighbor)));
41+
} else if (onStack.has(neighbor)) {
42+
lowlink.set(node, Math.min(lowlink.get(node), discoveryIndex.get(neighbor)));
43+
}
44+
}
45+
46+
if (lowlink.get(node) === discoveryIndex.get(node)) {
47+
const component = [];
48+
let popped;
49+
do {
50+
popped = stack.pop();
51+
onStack.delete(popped);
52+
component.push(popped);
53+
} while (popped !== node);
54+
if (component.length > 1) {
55+
connectedComponents.push(component);
56+
}
57+
}
58+
}
59+
60+
for (const node of graph.keys()) {
61+
if (!discoveryIndex.has(node)) {
62+
visit(node);
63+
}
64+
}
65+
66+
const cycles = [];
67+
for (const component of connectedComponents) {
68+
const componentSet = new Set(component);
69+
const start = component[0];
70+
const tracePath = [];
71+
const visited = new Set();
72+
73+
function traceBackToStart(node) {
74+
if (node === start && tracePath.length > 0) {
75+
cycles.push({ cycle: [...tracePath, start], sccSize: component.length });
76+
return true;
77+
}
78+
if (visited.has(node)) {
79+
return false;
80+
}
81+
visited.add(node);
82+
tracePath.push(node);
83+
for (const neighbor of graph.get(node) || []) {
84+
if (!componentSet.has(neighbor)) {
85+
continue;
86+
}
87+
if (neighbor === start && tracePath.length > 1) {
88+
cycles.push({ cycle: [...tracePath, start], sccSize: component.length });
89+
return true;
90+
}
91+
if (!visited.has(neighbor)) {
92+
if (traceBackToStart(neighbor)) {
93+
return true;
94+
}
95+
}
96+
}
97+
tracePath.pop();
98+
return false;
99+
}
100+
101+
visited.add(start);
102+
tracePath.push(start);
103+
for (const neighbor of graph.get(start) || []) {
104+
if (!componentSet.has(neighbor)) {
105+
continue;
106+
}
107+
if (neighbor === start) {
108+
continue;
109+
}
110+
visited.delete(neighbor);
111+
if (traceBackToStart(neighbor)) {
112+
break;
113+
}
114+
}
115+
}
116+
117+
return cycles;
118+
}
119+
120+
/**
121+
* Builds a module-level dependency graph from @aws-sdk/* imports across packages.
122+
*
123+
* @param packageDirs - list of package root paths.
124+
* @returns adjacency list of package name -> Set of @aws-sdk/* dependency names.
125+
*/
126+
async function buildModuleGraph(packageDirs) {
127+
const graph = new Map();
128+
129+
for (const packageDir of packageDirs) {
130+
const pkgJsonPath = path.join(packageDir, "package.json");
131+
if (!fs.existsSync(pkgJsonPath)) {
132+
continue;
133+
}
134+
const pkgJson = JSON.parse(fs.readFileSync(pkgJsonPath, "utf-8"));
135+
const name = pkgJson.name;
136+
if (!name || !name.startsWith("@aws-sdk/")) {
137+
continue;
138+
}
139+
if (!graph.has(name)) {
140+
graph.set(name, new Set());
141+
}
142+
143+
for (const dist of ["dist-cjs", "dist-es"]) {
144+
const distDir = path.join(packageDir, dist);
145+
if (!fs.existsSync(distDir)) {
146+
continue;
147+
}
148+
for await (const file of walk(distDir, ["node_modules"])) {
149+
if (!file.endsWith(".js")) {
150+
continue;
151+
}
152+
const code = fs.readFileSync(file, "utf-8");
153+
for (const specifier of extractImports(code)) {
154+
if (specifier.startsWith(".") || specifier.startsWith("node:")) {
155+
continue;
156+
}
157+
const pkg = getPackageName(specifier);
158+
if (pkg.startsWith("@aws-sdk/") && pkg !== name) {
159+
graph.get(name).add(pkg);
160+
}
161+
}
162+
}
163+
}
164+
}
165+
166+
return graph;
167+
}
168+
169+
/**
170+
* Resolves a self-referencing package import (e.g. "@aws-sdk/core/client")
171+
* to a file path within the package using the exports map.
172+
*
173+
* @param specifier - the import specifier.
174+
* @param pkgJson - parsed package.json.
175+
* @param packageDir - package root.
176+
* @param distName - "dist-cjs" or "dist-es".
177+
* @returns absolute file path, or null if not a self-reference.
178+
*/
179+
function resolveSelfImport(specifier, pkgJson, packageDir, distName) {
180+
const pkg = getPackageName(specifier);
181+
if (pkg !== pkgJson.name) {
182+
return null;
183+
}
184+
const subpath = "./" + specifier.slice(pkg.length + 1);
185+
const exportConfig = pkgJson.exports?.[subpath];
186+
if (!exportConfig) {
187+
return null;
188+
}
189+
const conditionKeys = distName === "dist-es" ? ["module", "import"] : ["node", "require"];
190+
for (const key of conditionKeys) {
191+
const val = typeof exportConfig === "string" ? exportConfig : exportConfig[key];
192+
if (typeof val === "string" && val.includes(distName)) {
193+
return path.resolve(packageDir, val);
194+
}
195+
}
196+
return null;
197+
}
198+
199+
/**
200+
* Builds a file-level dependency graph within a single dist directory.
201+
*
202+
* @param distDir - absolute path to dist-cjs or dist-es.
203+
* @param pkgJson - parsed package.json.
204+
* @param packageDir - package root.
205+
* @param distName - "dist-cjs" or "dist-es".
206+
* @returns adjacency list of absolute file path -> Set of absolute file paths.
207+
*/
208+
async function buildFileGraph(distDir, pkgJson, packageDir, distName) {
209+
const graph = new Map();
210+
211+
for await (const file of walk(distDir, ["node_modules"])) {
212+
if (!file.endsWith(".js")) {
213+
continue;
214+
}
215+
if (!graph.has(file)) {
216+
graph.set(file, new Set());
217+
}
218+
const code = fs.readFileSync(file, "utf-8");
219+
for (const specifier of extractImports(code)) {
220+
let target = null;
221+
if (specifier.startsWith(".")) {
222+
target = resolveRelative(file, specifier);
223+
} else {
224+
target = resolveSelfImport(specifier, pkgJson, packageDir, distName);
225+
}
226+
if (target) {
227+
graph.get(file).add(target);
228+
}
229+
}
230+
}
231+
232+
return graph;
233+
}
234+
235+
async function validate(packageDirs) {
236+
const errors = [];
237+
238+
// Module-level cycle detection.
239+
const moduleGraph = await buildModuleGraph(packageDirs);
240+
const moduleCycles = findAllCycles(moduleGraph);
241+
for (const { cycle, sccSize } of moduleCycles) {
242+
const sccNote = sccSize > cycle.length - 1 ? ` (${sccSize} packages in cycle group)` : "";
243+
errors.push(`module-level cycle${sccNote}:\n ${cycle.join(" →\n ")}`);
244+
}
245+
246+
// File-level cycle detection per package per dist.
247+
for (const packageDir of packageDirs) {
248+
const pkgJsonPath = path.join(packageDir, "package.json");
249+
if (!fs.existsSync(pkgJsonPath)) {
250+
continue;
251+
}
252+
const pkgJson = JSON.parse(fs.readFileSync(pkgJsonPath, "utf-8"));
253+
254+
for (const dist of ["dist-cjs", "dist-es"]) {
255+
const distDir = path.join(packageDir, dist);
256+
if (!fs.existsSync(distDir)) {
257+
continue;
258+
}
259+
const fileGraph = await buildFileGraph(distDir, pkgJson, packageDir, dist);
260+
const fileCycles = findAllCycles(fileGraph);
261+
for (const { cycle, sccSize } of fileCycles) {
262+
const relCycle = cycle.map((f) => path.relative(packageDir, f));
263+
const sccNote = sccSize > cycle.length - 1 ? ` (${sccSize} files in cycle group)` : "";
264+
errors.push(`[${pkgJson.name}/${dist}] file-level cycle${sccNote}:\n ${relCycle.join(" →\n ")}`);
265+
}
266+
}
267+
}
268+
269+
return errors;
270+
}
271+
272+
async function main() {
273+
const packages = getPackageDirs();
274+
const packageDirs = packages.map((p) => p.dir);
275+
const errors = await validate(packageDirs);
276+
if (errors.length) {
277+
console.error(`❌ ${errors.length} cycle(s) detected:\n ${errors.join("\n ")}`);
278+
process.exit(1);
279+
}
280+
console.log("✅ No cyclical file or package dependencies.");
281+
}
282+
283+
main();

0 commit comments

Comments
 (0)