Detect file dependency cycles

Author: z-silver

lib/ModuleName.ml

@@ -1,2 +1,2 @@
 let separator = "."
-let of_filepath = Fun.compose Filename.basename Filename.chop_extension
+let of_filepath = Fun.compose Filename.basename Filename.remove_extension

lib/Preprocessor.ml

@@ -215,10 +215,36 @@ module DependencyGraph = struct
 
   type expansion_result = expansion_state Error.attempt
 
+  (** Returns true if l depends on r in the given fully expanded graph, false
+      otherwise *)
+  let depends expanded_graph l r =
+    match BatMap.String.find_opt l expanded_graph with
+    | None -> false
+    | Some deps -> BatSet.String.mem (ModuleName.of_filepath r) deps
+
+  let no_cycles graph =
+    let module Map = BatMap.String in
+    if
+      BatEnum.fold
+        (fun has_cycle k ->
+          if depends graph k k then (
+            Logger.simply_error @@ "File " ^ k ^ " depends on itself";
+            true)
+          else has_cycle)
+        false
+      @@ Map.keys graph
+    then exit 1;
+    graph
+
   let expand graph : t Error.attempt =
     let module FT = BatFingerTree in
     let module Set = BatSet.String in
     let module Map = BatMap.String in
+    let atom_to_filepath =
+      Map.of_enum
+      @@ BatEnum.map (fun k -> (ModuleName.of_filepath k, k))
+      @@ Map.keys graph
+    in
     let bridge_ancestor relatives node graph =
       if Set.is_empty relatives then graph
       else Map.modify_def Set.empty node (Set.union relatives) graph
@@ -230,9 +256,12 @@ module DependencyGraph = struct
       | Some (more, current) when Set.mem current visited ->
           go @@ ok { visited; forward; next = more; backward }
       | Some (more, current) -> (
-          (* TODO: check for cycles *)
           let visited = Set.add current visited in
-          match Map.find_opt current forward with
+          match
+            Map.find_opt
+              (Map.find_default current current atom_to_filepath)
+              forward
+          with
           | None -> go @@ ok { visited; forward; next = more; backward }
           | Some children ->
               let parents =
@@ -257,7 +286,7 @@ module DependencyGraph = struct
                    })
       | None -> Ok { visited; forward; next; backward }
     in
-    Error.map (fun { forward; _ } -> forward)
+    Error.map (fun { forward; _ } -> no_cycles forward)
     @@ go
     @@ Error.ok
          {
@@ -267,13 +296,6 @@ module DependencyGraph = struct
            next = BatFingerTree.of_enum (BatMap.String.keys graph);
          }
 
-  (** Returns true if l depends on r in the given fully expanded graph, false
-      otherwise *)
-  let depends expanded_graph l r =
-    match BatMap.String.find_opt l expanded_graph with
-    | None -> false
-    | Some deps -> BatSet.String.mem (ModuleName.of_filepath r) deps
-
   let sort (expanded_graph : t) (files : string list) =
     let compare_files l r =
       if depends expanded_graph l r then 1