Report missing import error (#91)

# Objective

If an import is missing from the module it currently just unwraps and
panics without any explanations.

# Solution

Don't unwrap and bubble up the error instead.

The error reporting is a bit weird because it just points at the start
of the file and I'm not sure how to improve it.

---------

Co-authored-by: robtfm <[email protected]>

Author: IceSentry

src/compose/mod.rs

@@ -1318,20 +1318,22 @@ impl Composer {
         &mut self,
         module_set: &ComposableModuleDefinition,
         shader_defs: &HashMap<String, ShaderDefValue>,
-    ) -> Result<ComposableModule, ComposerError> {
+    ) -> Result<ComposableModule, EnsureImportsError> {
         let PreprocessOutput {
             preprocessed_source,
             imports,
         } = self
             .preprocessor
             .preprocess(&module_set.sanitized_source, shader_defs)
-            .map_err(|inner| ComposerError {
-                inner,
-                source: ErrSource::Module {
-                    name: module_set.name.to_owned(),
-                    offset: 0,
-                    defs: shader_defs.clone(),
-                },
+            .map_err(|inner| {
+                EnsureImportsError::from(ComposerError {
+                    inner,
+                    source: ErrSource::Module {
+                        name: module_set.name.to_owned(),
+                        offset: 0,
+                        defs: shader_defs.clone(),
+                    },
+                })
             })?;
 
         self.ensure_imports(imports.iter().map(|import| &import.definition), shader_defs)?;
@@ -1346,17 +1348,19 @@ impl Composer {
             &preprocessed_source,
             imports,
         )
+        .map_err(|err| err.into())
     }
 
     // build required ComposableModules for a given set of shader_defs
     fn ensure_imports<'a>(
         &mut self,
         imports: impl IntoIterator<Item = &'a ImportDefinition>,
         shader_defs: &HashMap<String, ShaderDefValue>,
-    ) -> Result<(), ComposerError> {
+    ) -> Result<(), EnsureImportsError> {
         for ImportDefinition { import, .. } in imports.into_iter() {
-            // we've already ensured imports exist when they were added
-            let module_set = self.module_sets.get(import).unwrap();
+            let Some(module_set) = self.module_sets.get(import) else {
+                return Err(EnsureImportsError::MissingImport(import.to_owned()));
+            };
             if module_set.get_module(shader_defs).is_some() {
                 continue;
             }
@@ -1381,6 +1385,29 @@ impl Composer {
     }
 }
 
+pub enum EnsureImportsError {
+    MissingImport(String),
+    ComposerError(ComposerError),
+}
+
+impl EnsureImportsError {
+    fn into_composer_error(self, err_source: ErrSource) -> ComposerError {
+        match self {
+            EnsureImportsError::MissingImport(import) => ComposerError {
+                inner: ComposerErrorInner::ImportNotFound(import.to_owned(), 0),
+                source: err_source,
+            },
+            EnsureImportsError::ComposerError(err) => err,
+        }
+    }
+}
+
+impl From<ComposerError> for EnsureImportsError {
+    fn from(value: ComposerError) -> Self {
+        EnsureImportsError::ComposerError(value)
+    }
+}
+
 #[derive(Default)]
 pub struct ComposableModuleDescriptor<'a> {
     pub source: &'a str,
@@ -1594,12 +1621,7 @@ impl Composer {
 
         let sanitized_source = self.sanitize_and_set_auto_bindings(source);
 
-        let PreprocessorMetaData {
-            name,
-            defines,
-            imports,
-            ..
-        } = self
+        let PreprocessorMetaData { name, defines, .. } = self
             .preprocessor
             .get_preprocessor_metadata(&sanitized_source, true)
             .map_err(|inner| ComposerError {
@@ -1614,6 +1636,18 @@ impl Composer {
 
         let name = name.unwrap_or_default();
 
+        let PreprocessOutput { imports, .. } = self
+            .preprocessor
+            .preprocess(&sanitized_source, &shader_defs)
+            .map_err(|inner| ComposerError {
+                inner,
+                source: ErrSource::Constructing {
+                    path: file_path.to_owned(),
+                    source: sanitized_source.to_owned(),
+                    offset: 0,
+                },
+            })?;
+
         // make sure imports have been added
         // and gather additional defs specified at module level
         for (import_name, offset) in imports
@@ -1643,7 +1677,7 @@ impl Composer {
                     inner: ComposerErrorInner::ImportNotFound(import_name.clone(), offset),
                     source: ErrSource::Constructing {
                         path: file_path.to_owned(),
-                        source: sanitized_source,
+                        source: sanitized_source.to_owned(),
                         offset: 0,
                     },
                 });
@@ -1652,8 +1686,22 @@ impl Composer {
         self.ensure_imports(
             imports.iter().map(|import| &import.definition),
             &shader_defs,
-        )?;
-        self.ensure_imports(additional_imports, &shader_defs)?;
+        )
+        .map_err(|err| {
+            err.into_composer_error(ErrSource::Constructing {
+                path: file_path.to_owned(),
+                source: sanitized_source.to_owned(),
+                offset: 0,
+            })
+        })?;
+        self.ensure_imports(additional_imports, &shader_defs)
+            .map_err(|err| {
+                err.into_composer_error(ErrSource::Constructing {
+                    path: file_path.to_owned(),
+                    source: sanitized_source.to_owned(),
+                    offset: 0,
+                })
+            })?;
 
         let definition = ComposableModuleDefinition {
             name,

src/compose/test.rs

@@ -999,6 +999,74 @@ mod test {
         output_eq!(wgsl, "tests/expected/conditional_import_b.txt");
     }
 
+    #[test]
+    fn conditional_missing_import() {
+        let mut composer = Composer::default();
+
+        composer
+            .add_composable_module(ComposableModuleDescriptor {
+                source: include_str!("tests/conditional_import_fail/mod_a_b.wgsl"),
+                file_path: "tests/conditional_import_fail/mod_a_b.wgsl",
+                ..Default::default()
+            })
+            .unwrap();
+
+        let error = composer
+            .make_naga_module(NagaModuleDescriptor {
+                source: include_str!("tests/conditional_import_fail/top.wgsl"),
+                file_path: "tests/conditional_import_fail/top.wgsl",
+                ..Default::default()
+            })
+            .err()
+            .unwrap();
+
+        let text = error.emit_to_string(&composer);
+
+        // let mut f = std::fs::File::create("conditional_missing_import.txt").unwrap();
+        // f.write_all(text.as_bytes()).unwrap();
+        // drop(f);
+
+        output_eq!(text, "tests/expected/conditional_missing_import.txt");
+    }
+
+    #[test]
+    fn conditional_missing_import_nested() {
+        let mut composer = Composer::default();
+
+        composer
+            .add_composable_module(ComposableModuleDescriptor {
+                source: include_str!("tests/conditional_import_fail/mod_a_b.wgsl"),
+                file_path: "tests/conditional_import_fail/mod_a_b.wgsl",
+                ..Default::default()
+            })
+            .unwrap();
+
+        composer
+            .add_composable_module(ComposableModuleDescriptor {
+                source: include_str!("tests/conditional_import_fail/middle.wgsl"),
+                file_path: "tests/conditional_import_fail/middle.wgsl",
+                ..Default::default()
+            })
+            .unwrap();
+
+        let error = composer
+            .make_naga_module(NagaModuleDescriptor {
+                source: include_str!("tests/conditional_import_fail/top_nested.wgsl"),
+                file_path: "tests/conditional_import_fail/top_nested.wgsl",
+                ..Default::default()
+            })
+            .err()
+            .unwrap();
+
+        let text = error.emit_to_string(&composer);
+
+        // let mut f = std::fs::File::create("conditional_missing_import_nested.txt").unwrap();
+        // f.write_all(text.as_bytes()).unwrap();
+        // drop(f);
+
+        output_eq!(text, "tests/expected/conditional_missing_import_nested.txt");
+    }
+
     #[cfg(feature = "test_shader")]
     #[test]
     fn rusty_imports() {

src/compose/tests/conditional_import_fail/middle.wgsl

@@ -0,0 +1,9 @@
+#define_import_path middle
+
+#ifdef USE_A
+    #import a::b
+#endif
+
+fn mid_fn() -> u32 {
+    return b::C;
+}

src/compose/tests/conditional_import_fail/mod_a_b.wgsl

@@ -0,0 +1,3 @@
+#define_import_path a::b
+
+const C: u32 = 1u;

src/compose/tests/conditional_import_fail/top.wgsl

@@ -0,0 +1,7 @@
+#ifdef USE_A
+    #import a::b
+#endif
+
+fn main() -> u32 {
+    return b::C;
+}

src/compose/tests/conditional_import_fail/top_nested.wgsl

@@ -0,0 +1,5 @@
+#import middle
+
+fn main() -> u32 {
+    return middle::mid_fn();
+}

src/compose/tests/expected/conditional_missing_import.txt

@@ -0,0 +1,8 @@
+error: required import 'b' not found
+  ┌─ tests/conditional_import_fail/top.wgsl:6:12
+  │
+6 │     return b::C;
+  │            ^
+  │
+  = missing import 'b'
+

src/compose/tests/expected/conditional_missing_import_nested.txt

@@ -0,0 +1,8 @@
+error: required import 'b' not found
+  ┌─ tests/conditional_import_fail/top_nested.wgsl:1:1
+  │
+1 │ #import middle
+  │ ^
+  │
+  = missing import 'b'
+