Merge pull request microsoft#251003 from microsoft/joh/systematic-mockingbird

Detect and fix cyclic runtime dependencies

Author: jrieken

build/lib/tsb/builder.js

@@ -44,7 +44,7 @@ export function createTypeScriptBuilder(config: IConfiguration, projectFile: str
 	const host = new LanguageServiceHost(cmd, projectFile, _log);
 
 	const outHost = new LanguageServiceHost({ ...cmd, options: { ...cmd.options, sourceRoot: cmd.options.outDir } }, cmd.options.outDir ?? '', _log);
-	let lastCycleCheckVersion: string;
+	const toBeCheckedForCycles: string[] = [];
 
 	const service = ts.createLanguageService(host, ts.createDocumentRegistry());
 	const lastBuildVersion: { [path: string]: string } = Object.create(null);
@@ -315,6 +315,7 @@ export function createTypeScriptBuilder(config: IConfiguration, projectFile: str
 						const jsValue = value.files.find(candidate => candidate.basename.endsWith('.js'));
 						if (jsValue) {
 							outHost.addScriptSnapshot(jsValue.path, new ScriptSnapshot(String(jsValue.contents), new Date()));
+							toBeCheckedForCycles.push(normalize(jsValue.path));
 						}
 
 					}).catch(e => {
@@ -424,25 +425,22 @@ export function createTypeScriptBuilder(config: IConfiguration, projectFile: str
 
 		}).then(() => {
 			// check for cyclic dependencies
-			const thisCycleCheckVersion = outHost.getProjectVersion();
-			if (thisCycleCheckVersion === lastCycleCheckVersion) {
-				return;
-			}
-			const oneCycle = outHost.hasCyclicDependency();
-			lastCycleCheckVersion = thisCycleCheckVersion;
-			delete oldErrors[projectFile];
-
-			if (oneCycle) {
-				const cycleError: ts.Diagnostic = {
-					category: ts.DiagnosticCategory.Error,
-					code: 1,
-					file: undefined,
-					start: undefined,
-					length: undefined,
-					messageText: `CYCLIC dependency between ${oneCycle}`
-				};
-				onError(cycleError);
-				newErrors[projectFile] = [cycleError];
+			const cycles = outHost.getCyclicDependencies(toBeCheckedForCycles);
+			toBeCheckedForCycles.length = 0;
+
+			for (const [filename, error] of cycles) {
+				const cyclicDepErrors: ts.Diagnostic[] = [];
+				if (error) {
+					cyclicDepErrors.push({
+						category: ts.DiagnosticCategory.Error,
+						code: 1,
+						file: undefined,
+						start: undefined,
+						length: undefined,
+						messageText: `CYCLIC dependency: ${error}`
+					});
+				}
+				newErrors[filename] = cyclicDepErrors;
 			}
 
 		}).then(() => {
@@ -666,15 +664,17 @@ class LanguageServiceHost implements ts.LanguageServiceHost {
 		}
 	}
 
-	hasCyclicDependency(): string | undefined {
+	getCyclicDependencies(filenames: string[]): Map<string, string | undefined> {
 		// Ensure dependencies are up to date
 		while (this._dependenciesRecomputeList.length) {
 			this._processFile(this._dependenciesRecomputeList.pop()!);
 		}
-		const cycle = this._dependencies.findCycle();
-		return cycle
-			? cycle.join(' -> ')
-			: undefined;
+		const cycles = this._dependencies.findCycles(filenames.sort((a, b) => a.localeCompare(b)));
+		const result = new Map<string, string | undefined>();
+		for (const [key, value] of cycles) {
+			result.set(key, value?.join(' -> '));
+		}
+		return result;
 	}
 
 	_processFile(filename: string): void {

build/lib/tsb/builder.ts

@@ -63,53 +63,42 @@ export namespace graph {
             return this._nodes.get(data) ?? null;
         }
 
-        findCycle(): T[] | undefined {
+        findCycles(allData: T[]): Map<T, T[] | undefined> {
+            const result = new Map<T, T[] | undefined>();
+            const checked = new Set<T>();
+            for (const data of allData) {
+                const node = this.lookup(data);
+                if (!node) {
+                    continue;
+                }
+                const r = this._findCycle(node, checked, new Set());
+                result.set(node.data, r);
+            }
+            return result;
+        }
 
-            let result: T[] | undefined;
-            let foundStartNodes = false;
-            const checked = new Set<Node<T>>();
+        private _findCycle(node: Node<T>, checked: Set<T>, seen: Set<T>): T[] | undefined {
 
-            for (const [_start, value] of this._nodes) {
+            if (checked.has(node.data)) {
+                return undefined;
+            }
 
-                if (Object.values(value.incoming).length > 0) {
-                    continue;
+            let result: T[] | 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;
                 }
-
-                foundStartNodes = true;
-
-                const dfs = (node: Node<T>, visited: Set<Node<T>>) => {
-
-                    if (checked.has(node)) {
-                        return;
-                    }
-
-                    if (visited.has(node)) {
-                        result = [...visited, node].map(n => n.data);
-                        const idx = result.indexOf(node.data);
-                        result = result.slice(idx);
-                        return;
-                    }
-                    visited.add(node);
-                    for (const child of Object.values(node.outgoing)) {
-                        dfs(child, visited);
-                        if (result) {
-                            break;
-                        }
-                    }
-                    visited.delete(node);
-                    checked.add(node);
-                };
-                dfs(value, new Set());
+                seen.add(child.data);
+                result = this._findCycle(child, checked, seen);
+                seen.delete(child.data);
                 if (result) {
                     break;
                 }
             }
-
-            if (!foundStartNodes) {
-                // everything is a cycle
-                return Array.from(this._nodes.keys());
-            }
-
+            checked.add(node.data);
             return result;
         }
     }