fix(compiler): Raise appropriate exception when modules are missing during dependency graph construction (#1485)

Author: peblair

compiler/src/parsing/driver.re

@@ -126,7 +126,8 @@ let parse = (~name=?, lexbuf, source): Parsetree.parsed_program => {
   };
 };
 
-let scan_for_imports = (~defer_errors=true, filename: string): list(string) => {
+let scan_for_imports =
+    (~defer_errors=true, filename: string): list(loc(string)) => {
   let ic = open_in(filename);
   let lexbuf = Sedlexing.Utf8.from_channel(ic);
   try({
@@ -142,8 +143,9 @@ let scan_for_imports = (~defer_errors=true, filename: string): list(string) => {
       List.map(
         o => {
           switch (o) {
-          | Grain_utils.Config.Pervasives_mod => "pervasives"
-          | Grain_utils.Config.Gc_mod => "runtime/gc"
+          | Grain_utils.Config.Pervasives_mod =>
+            Location.mknoloc("pervasives")
+          | Grain_utils.Config.Gc_mod => Location.mknoloc("runtime/gc")
           }
         },
         switch (comments) {
@@ -158,12 +160,12 @@ let scan_for_imports = (~defer_errors=true, filename: string): list(string) => {
       );
     let found_imports = ref([]);
     let iter_mod = (self, import) =>
-      found_imports := [import.Parsetree.pimp_path.txt, ...found_imports^];
+      found_imports := [import.Parsetree.pimp_path, ...found_imports^];
     let iterator = {...Ast_iterator.default_iterator, import: iter_mod};
     List.iter(iterator.toplevel(iterator), statements);
     close_in(ic);
     List.sort_uniq(
-      String.compare,
+      (a, b) => String.compare(a.txt, b.txt),
       List.append(implicit_opens, found_imports^),
     );
   }) {

compiler/src/parsing/driver.rei

@@ -4,4 +4,5 @@ let parse:
   (~name: string=?, Sedlexing.lexbuf, unit => string) =>
   Parsetree.parsed_program;
 
-let scan_for_imports: (~defer_errors: bool=?, string) => list(string);
+let scan_for_imports:
+  (~defer_errors: bool=?, string) => list(Location.loc(string));

compiler/src/typed/module_resolution.re

@@ -4,7 +4,7 @@ open Cmi_format;
 
 type error =
   | Missing_module(Location.t, Path.t, Path.t)
-  | No_module_file(string, option(string));
+  | No_module_file(Location.t, string, option(string));
 
 exception Error(error);
 
@@ -280,16 +280,32 @@ module Dependency_graph =
         let from_srcpath = srcpath => {
           List.map(
             name => {
-              let located = locate_module(base_dir, active_search_path, name);
+              let located =
+                try(
+                  locate_module(
+                    base_dir,
+                    active_search_path,
+                    name.Location.txt,
+                  )
+                ) {
+                | Not_found =>
+                  error(
+                    No_module_file(
+                      name.Location.loc,
+                      name.Location.txt,
+                      None,
+                    ),
+                  )
+                };
               let out_file_name = located_to_out_file_name(located);
               let existing_dependency = lookup(out_file_name);
               switch (existing_dependency) {
               | Some(ed) =>
-                Hashtbl.add(ed.dn_unit_name, Some(dn), name);
+                Hashtbl.add(ed.dn_unit_name, Some(dn), name.Location.txt);
                 ed;
               | None =>
                 let tbl = Hashtbl.create(8);
-                Hashtbl.add(tbl, Some(dn), name);
+                Hashtbl.add(tbl, Some(dn), name.Location.txt);
                 {
                   dn_unit_name: tbl,
                   dn_file_name: out_file_name,
@@ -315,7 +331,16 @@ module Dependency_graph =
             List.map(
               ((name, _)) => {
                 let located =
-                  locate_module(base_dir, active_search_path, name);
+                  try(locate_module(base_dir, active_search_path, name)) {
+                  | Not_found =>
+                    error(
+                      No_module_file(
+                        Location.in_file(dn.dn_file_name),
+                        name,
+                        None,
+                      ),
+                    )
+                  };
                 let out_file_name = located_to_out_file_name(located);
                 let existing_dependency = lookup(out_file_name);
                 switch (existing_dependency) {
@@ -435,7 +460,7 @@ let locate_module_file = (~loc, ~disable_relpath=false, unit_name) => {
   let path = Config.module_search_path();
   let located =
     try(locate_module(~disable_relpath, base_dir, path, unit_name)) {
-    | Not_found => error(No_module_file(unit_name, None))
+    | Not_found => error(No_module_file(loc, unit_name, None))
     };
   let out_file = located_to_out_file_name(located);
   let current_dep_node =
@@ -509,14 +534,16 @@ let report_error = ppf =>
         "was not found",
       );
     }
-  | No_module_file(m, None) => fprintf(ppf, "Missing file for module %s", m)
-  | No_module_file(m, Some(msg)) =>
+  | No_module_file(_, m, None) =>
+    fprintf(ppf, "Missing file for module %s", m)
+  | No_module_file(_, m, Some(msg)) =>
     fprintf(ppf, "Missing file for module %s: %s", m, msg);
 
 let () =
   Location.register_error_of_exn(
     fun
-    | Error(Missing_module(loc, _, _) as err) when loc != Location.dummy_loc =>
+    | Error(Missing_module(loc, _, _) as err)
+    | Error(No_module_file(loc, _, _) as err) when loc != Location.dummy_loc =>
       Some(Location.error_of_printer(loc, report_error, err))
     | Error(err) => Some(Location.error_of_printer_file(report_error, err))
     | _ => None,

compiler/test/input/brokenImports/broken.gr

@@ -0,0 +1,4 @@
+import Number from "number"
+import Foo from "./foo"
+
+export let min = Number.min

compiler/test/input/brokenImports/main.gr

@@ -0,0 +1,3 @@
+import Broken from "./broken"
+
+print(Broken.min(2, 3))

compiler/test/runner.re

@@ -295,6 +295,24 @@ let makeFileRunner =
   });
 };
 
+let makeFileCompileErrorRunner = (test, name, filename, expected) => {
+  test(
+    name,
+    ({expect}) => {
+      let error =
+        try({
+          let infile = grainfile(filename);
+          let outfile = wasmfile(name);
+          ignore @@ compile_file(infile, outfile);
+          "";
+        }) {
+        | exn => Printexc.to_string(exn)
+        };
+      expect.string(error).toMatch(expected);
+    },
+  );
+};
+
 let makeFileErrorRunner = (test, name, filename, expected) => {
   test(
     name,

compiler/test/suites/imports.re

@@ -9,6 +9,7 @@ describe("imports", ({test, testSkip}) => {
   let assertCompileError = makeCompileErrorRunner(test);
   let assertRun = makeRunner(test_or_skip);
   let assertFileRun = makeFileRunner(test_or_skip);
+  let assertFileCompileError = makeFileCompileErrorRunner(test_or_skip);
   let assertFileSnapshot = makeSnapshotFileRunner(test);
 
   /* import * tests */
@@ -212,4 +213,9 @@ describe("imports", ({test, testSkip}) => {
     "import TList, { Empty } from \"tlists\"; let foo : TList.TList<String> = Empty; foo",
   );
   assertFileRun("relative_import_linking", "relativeImportLinking", "2\n2\n");
+  assertFileCompileError(
+    "import_broken",
+    "brokenImports/main",
+    "./broken.gr\", line 2, characters 16-23",
+  );
 });