mirror of
https://github.com/microsoft/vscode.git
synced 2026-04-28 04:23:32 +01:00
Add cyclic dependency check script and tests (#296035)
* eng: add cyclic dependency check script and update workflow * refactor: export classes and functions in checkCyclicDependencies for better accessibility; add comprehensive tests for cycle detection and file processing * Update build/lib/checkCyclicDependencies.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Johannes Rieken
GitHub
3
.github/workflows/pr.yml
vendored
3
.github/workflows/pr.yml
vendored
@@ -81,6 +81,9 @@ jobs:
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
|
||||
- name: Check cyclic dependencies
|
||||
run: node build/lib/checkCyclicDependencies.ts out-build
|
||||
|
||||
linux-cli-tests:
|
||||
name: Linux
|
||||
uses: ./.github/workflows/pr-linux-cli-test.yml
|
||||
|
||||
173
build/lib/checkCyclicDependencies.ts
Normal file
173
build/lib/checkCyclicDependencies.ts
Normal file
@@ -0,0 +1,173 @@
|
||||
/*---------------------------------------------------------------------------------------------
|
||||
* Copyright (c) Microsoft Corporation. All rights reserved.
|
||||
* Licensed under the MIT License. See License.txt in the project root for license information.
|
||||
*--------------------------------------------------------------------------------------------*/
|
||||
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import * as ts from 'typescript';
|
||||
|
||||
// --- Graph (extracted from build/lib/tsb/utils.ts) ---
|
||||
|
||||
export class Node {
|
||||
readonly incoming = new Map<string, Node>();
|
||||
readonly outgoing = new Map<string, Node>();
|
||||
|
||||
readonly data: string;
|
||||
|
||||
constructor(data: string) {
|
||||
this.data = data;
|
||||
}
|
||||
}
|
||||
|
||||
export class Graph {
|
||||
private _nodes = new Map<string, Node>();
|
||||
|
||||
inertEdge(from: string, to: string): void {
|
||||
const fromNode = this.lookupOrInsertNode(from);
|
||||
const toNode = this.lookupOrInsertNode(to);
|
||||
fromNode.outgoing.set(toNode.data, toNode);
|
||||
toNode.incoming.set(fromNode.data, fromNode);
|
||||
}
|
||||
|
||||
lookupOrInsertNode(data: string): Node {
|
||||
let node = this._nodes.get(data);
|
||||
if (!node) {
|
||||
node = new Node(data);
|
||||
this._nodes.set(data, node);
|
||||
}
|
||||
return node;
|
||||
}
|
||||
|
||||
lookup(data: string): Node | undefined {
|
||||
return this._nodes.get(data);
|
||||
}
|
||||
|
||||
findCycles(allData: string[]): Map<string, string[] | undefined> {
|
||||
const result = new Map<string, string[] | undefined>();
|
||||
const checked = new Set<string>();
|
||||
for (const data of allData) {
|
||||
const node = this.lookup(data);
|
||||
if (!node) {
|
||||
continue;
|
||||
}
|
||||
const cycle = this._findCycle(node, checked, new Set());
|
||||
result.set(node.data, cycle);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
private _findCycle(node: Node, checked: Set<string>, seen: Set<string>): string[] | undefined {
|
||||
if (checked.has(node.data)) {
|
||||
return undefined;
|
||||
}
|
||||
for (const child of node.outgoing.values()) {
|
||||
if (seen.has(child.data)) {
|
||||
const seenArr = Array.from(seen);
|
||||
const idx = seenArr.indexOf(child.data);
|
||||
seenArr.push(child.data);
|
||||
return idx > 0 ? seenArr.slice(idx) : seenArr;
|
||||
}
|
||||
seen.add(child.data);
|
||||
const result = this._findCycle(child, checked, seen);
|
||||
seen.delete(child.data);
|
||||
if (result) {
|
||||
return result;
|
||||
}
|
||||
}
|
||||
checked.add(node.data);
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
// --- Dependency scanning & cycle detection ---
|
||||
|
||||
export function normalize(p: string): string {
|
||||
return p.replace(/\\/g, '/');
|
||||
}
|
||||
|
||||
export function collectJsFiles(dir: string): string[] {
|
||||
const results: string[] = [];
|
||||
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
results.push(...collectJsFiles(full));
|
||||
} else if (entry.isFile() && entry.name.endsWith('.js')) {
|
||||
results.push(full);
|
||||
}
|
||||
}
|
||||
return results;
|
||||
}
|
||||
|
||||
export function processFile(filename: string, graph: Graph): void {
|
||||
const content = fs.readFileSync(filename, 'utf-8');
|
||||
const info = ts.preProcessFile(content, true);
|
||||
|
||||
for (const ref of info.importedFiles) {
|
||||
if (!ref.fileName.startsWith('.')) {
|
||||
continue; // skip node_modules
|
||||
}
|
||||
if (ref.fileName.endsWith('.css')) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const dir = path.dirname(filename);
|
||||
let resolvedPath = path.resolve(dir, ref.fileName);
|
||||
if (resolvedPath.endsWith('.js')) {
|
||||
resolvedPath = resolvedPath.slice(0, -3);
|
||||
}
|
||||
const normalizedResolved = normalize(resolvedPath);
|
||||
|
||||
if (fs.existsSync(normalizedResolved + '.js')) {
|
||||
graph.inertEdge(normalize(filename), normalizedResolved + '.js');
|
||||
} else if (fs.existsSync(normalizedResolved + '.ts')) {
|
||||
graph.inertEdge(normalize(filename), normalizedResolved + '.ts');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function main(): void {
|
||||
const folder = process.argv[2];
|
||||
if (!folder) {
|
||||
console.error('Usage: node build/lib/checkCyclicDependencies.ts <folder>');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const rootDir = path.resolve(folder);
|
||||
if (!fs.existsSync(rootDir) || !fs.statSync(rootDir).isDirectory()) {
|
||||
console.error(`Not a directory: ${rootDir}`);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const files = collectJsFiles(rootDir);
|
||||
const graph = new Graph();
|
||||
|
||||
for (const file of files) {
|
||||
processFile(file, graph);
|
||||
}
|
||||
|
||||
const allNormalized = files.map(normalize).sort((a, b) => a.localeCompare(b));
|
||||
const cycles = graph.findCycles(allNormalized);
|
||||
|
||||
const cyclicPaths = new Set<string>();
|
||||
for (const [_filename, cycle] of cycles) {
|
||||
if (cycle) {
|
||||
const path = cycle.join(' -> ');
|
||||
if (cyclicPaths.has(path)) {
|
||||
continue;
|
||||
}
|
||||
cyclicPaths.add(path);
|
||||
console.error(`CYCLIC dependency: ${path}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (cyclicPaths.size > 0) {
|
||||
process.exit(1);
|
||||
} else {
|
||||
console.log(`No cyclic dependencies found in ${files.length} files.`);
|
||||
}
|
||||
}
|
||||
|
||||
if (process.argv[1] && normalize(path.resolve(process.argv[1])).endsWith('checkCyclicDependencies.ts')) {
|
||||
main();
|
||||
}
|
||||
181
build/lib/test/checkCyclicDependencies.test.ts
Normal file
181
build/lib/test/checkCyclicDependencies.test.ts
Normal file
@@ -0,0 +1,181 @@
|
||||
/*---------------------------------------------------------------------------------------------
|
||||
* Copyright (c) Microsoft Corporation. All rights reserved.
|
||||
* Licensed under the MIT License. See License.txt in the project root for license information.
|
||||
*--------------------------------------------------------------------------------------------*/
|
||||
|
||||
import assert from 'assert';
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import os from 'os';
|
||||
import { Graph, collectJsFiles, processFile, normalize } from '../checkCyclicDependencies.ts';
|
||||
|
||||
suite('checkCyclicDependencies', () => {
|
||||
|
||||
suite('Graph', () => {
|
||||
|
||||
test('no cycles in linear chain', () => {
|
||||
const graph = new Graph();
|
||||
graph.inertEdge('a', 'b');
|
||||
graph.inertEdge('b', 'c');
|
||||
const cycles = graph.findCycles(['a', 'b', 'c']);
|
||||
for (const [, cycle] of cycles) {
|
||||
assert.strictEqual(cycle, undefined);
|
||||
}
|
||||
});
|
||||
|
||||
test('detects simple cycle', () => {
|
||||
const graph = new Graph();
|
||||
graph.inertEdge('a', 'b');
|
||||
graph.inertEdge('b', 'a');
|
||||
const cycles = graph.findCycles(['a', 'b']);
|
||||
const hasCycle = Array.from(cycles.values()).some(c => c !== undefined);
|
||||
assert.ok(hasCycle);
|
||||
});
|
||||
|
||||
test('detects 3-node cycle', () => {
|
||||
const graph = new Graph();
|
||||
graph.inertEdge('a', 'b');
|
||||
graph.inertEdge('b', 'c');
|
||||
graph.inertEdge('c', 'a');
|
||||
const cycles = graph.findCycles(['a', 'b', 'c']);
|
||||
const hasCycle = Array.from(cycles.values()).some(c => c !== undefined);
|
||||
assert.ok(hasCycle);
|
||||
});
|
||||
|
||||
test('no false positives with shared dependencies', () => {
|
||||
const graph = new Graph();
|
||||
// diamond: a -> b, a -> c, b -> d, c -> d
|
||||
graph.inertEdge('a', 'b');
|
||||
graph.inertEdge('a', 'c');
|
||||
graph.inertEdge('b', 'd');
|
||||
graph.inertEdge('c', 'd');
|
||||
const cycles = graph.findCycles(['a', 'b', 'c', 'd']);
|
||||
for (const [, cycle] of cycles) {
|
||||
assert.strictEqual(cycle, undefined);
|
||||
}
|
||||
});
|
||||
|
||||
test('lookupOrInsertNode returns same node for same data', () => {
|
||||
const graph = new Graph();
|
||||
const node1 = graph.lookupOrInsertNode('x');
|
||||
const node2 = graph.lookupOrInsertNode('x');
|
||||
assert.strictEqual(node1, node2);
|
||||
});
|
||||
|
||||
test('lookup returns undefined for unknown node', () => {
|
||||
const graph = new Graph();
|
||||
assert.strictEqual(graph.lookup('unknown'), undefined);
|
||||
});
|
||||
|
||||
test('findCycles skips unknown data', () => {
|
||||
const graph = new Graph();
|
||||
graph.inertEdge('a', 'b');
|
||||
const cycles = graph.findCycles(['nonexistent']);
|
||||
assert.strictEqual(cycles.get('nonexistent'), undefined);
|
||||
});
|
||||
|
||||
test('cycle path contains the cycle nodes', () => {
|
||||
const graph = new Graph();
|
||||
graph.inertEdge('a', 'b');
|
||||
graph.inertEdge('b', 'c');
|
||||
graph.inertEdge('c', 'b');
|
||||
const cycles = graph.findCycles(['a', 'b', 'c']);
|
||||
const cyclePath = Array.from(cycles.values()).find(c => c !== undefined);
|
||||
assert.ok(cyclePath);
|
||||
assert.ok(cyclePath.includes('b'));
|
||||
assert.ok(cyclePath.includes('c'));
|
||||
// cycle should start and end with same node
|
||||
assert.strictEqual(cyclePath[0], cyclePath[cyclePath.length - 1]);
|
||||
});
|
||||
});
|
||||
|
||||
suite('normalize', () => {
|
||||
|
||||
test('replaces backslashes with forward slashes', () => {
|
||||
assert.strictEqual(normalize('a\\b\\c'), 'a/b/c');
|
||||
});
|
||||
|
||||
test('leaves forward slashes unchanged', () => {
|
||||
assert.strictEqual(normalize('a/b/c'), 'a/b/c');
|
||||
});
|
||||
});
|
||||
|
||||
suite('collectJsFiles and processFile', () => {
|
||||
|
||||
let tmpDir: string;
|
||||
|
||||
setup(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cyclic-test-'));
|
||||
});
|
||||
|
||||
teardown(() => {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('collectJsFiles finds .js files recursively', () => {
|
||||
fs.writeFileSync(path.join(tmpDir, 'a.js'), '');
|
||||
fs.writeFileSync(path.join(tmpDir, 'b.ts'), '');
|
||||
fs.mkdirSync(path.join(tmpDir, 'sub'));
|
||||
fs.writeFileSync(path.join(tmpDir, 'sub', 'c.js'), '');
|
||||
const files = collectJsFiles(tmpDir);
|
||||
assert.strictEqual(files.length, 2);
|
||||
assert.ok(files.some(f => f.endsWith('a.js')));
|
||||
assert.ok(files.some(f => f.endsWith('c.js')));
|
||||
});
|
||||
|
||||
test('processFile adds edges for relative imports', () => {
|
||||
fs.writeFileSync(path.join(tmpDir, 'a.js'), 'import { x } from "./b";');
|
||||
fs.writeFileSync(path.join(tmpDir, 'b.js'), '');
|
||||
const graph = new Graph();
|
||||
processFile(path.join(tmpDir, 'a.js'), graph);
|
||||
const aNode = graph.lookup(normalize(path.join(tmpDir, 'a.js')));
|
||||
assert.ok(aNode);
|
||||
assert.strictEqual(aNode.outgoing.size, 1);
|
||||
});
|
||||
|
||||
test('processFile skips non-relative imports', () => {
|
||||
fs.writeFileSync(path.join(tmpDir, 'a.js'), 'import fs from "fs";');
|
||||
const graph = new Graph();
|
||||
processFile(path.join(tmpDir, 'a.js'), graph);
|
||||
// no relative imports, so no edges and no node created
|
||||
assert.strictEqual(graph.lookup(normalize(path.join(tmpDir, 'a.js'))), undefined);
|
||||
});
|
||||
|
||||
test('processFile skips CSS imports', () => {
|
||||
fs.writeFileSync(path.join(tmpDir, 'a.js'), 'import "./styles.css";');
|
||||
const graph = new Graph();
|
||||
processFile(path.join(tmpDir, 'a.js'), graph);
|
||||
// CSS imports are ignored, so no edges and no node created
|
||||
assert.strictEqual(graph.lookup(normalize(path.join(tmpDir, 'a.js'))), undefined);
|
||||
});
|
||||
|
||||
test('end-to-end: detects cycle in JS files', () => {
|
||||
fs.writeFileSync(path.join(tmpDir, 'a.js'), 'import { x } from "./b";');
|
||||
fs.writeFileSync(path.join(tmpDir, 'b.js'), 'import { y } from "./a";');
|
||||
const files = collectJsFiles(tmpDir);
|
||||
const graph = new Graph();
|
||||
for (const file of files) {
|
||||
processFile(file, graph);
|
||||
}
|
||||
const allNormalized = files.map(normalize);
|
||||
const cycles = graph.findCycles(allNormalized);
|
||||
const hasCycle = Array.from(cycles.values()).some(c => c !== undefined);
|
||||
assert.ok(hasCycle);
|
||||
});
|
||||
|
||||
test('end-to-end: no cycle in acyclic JS files', () => {
|
||||
fs.writeFileSync(path.join(tmpDir, 'a.js'), 'import { x } from "./b";');
|
||||
fs.writeFileSync(path.join(tmpDir, 'b.js'), '');
|
||||
const files = collectJsFiles(tmpDir);
|
||||
const graph = new Graph();
|
||||
for (const file of files) {
|
||||
processFile(file, graph);
|
||||
}
|
||||
const allNormalized = files.map(normalize);
|
||||
const cycles = graph.findCycles(allNormalized);
|
||||
for (const [, cycle] of cycles) {
|
||||
assert.strictEqual(cycle, undefined);
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -16,6 +16,7 @@
|
||||
"test-node": "mocha test/unit/node/index.js --delay --ui=tdd --timeout=5000 --exit",
|
||||
"test-extension": "vscode-test",
|
||||
"test-build-scripts": "cd build && npm run test",
|
||||
"check-cyclic-dependencies": "node build/lib/checkCyclicDependencies.ts out",
|
||||
"preinstall": "node build/npm/preinstall.ts",
|
||||
"postinstall": "node build/npm/postinstall.ts",
|
||||
"compile": "npm run gulp compile",
|
||||
|
||||
Reference in New Issue
Block a user