diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 831a02068fb..b988d19f49e 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -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 diff --git a/build/lib/checkCyclicDependencies.ts b/build/lib/checkCyclicDependencies.ts new file mode 100644 index 00000000000..dddaf76ad5a --- /dev/null +++ b/build/lib/checkCyclicDependencies.ts @@ -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(); + readonly outgoing = new Map(); + + readonly data: string; + + constructor(data: string) { + this.data = data; + } +} + +export class Graph { + private _nodes = new Map(); + + 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 { + const result = new Map(); + const checked = new Set(); + 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, seen: Set): 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 '); + 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(); + 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(); +} diff --git a/build/lib/test/checkCyclicDependencies.test.ts b/build/lib/test/checkCyclicDependencies.test.ts new file mode 100644 index 00000000000..bbffbc55fab --- /dev/null +++ b/build/lib/test/checkCyclicDependencies.test.ts @@ -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); + } + }); + }); +}); diff --git a/package.json b/package.json index 4a17850e8f0..f73c3d47057 100644 --- a/package.json +++ b/package.json @@ -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",