impl proper error handling

frb_codegen/Cargo.toml

@@ -33,3 +33,4 @@ anyhow = "1.0.44"
 pathdiff = "0.2.1"
 cargo_metadata = "0.14.1"
 enum_dispatch = "0.3.8"
+thiserror = "1"

frb_codegen/src/commands.rs

@@ -3,17 +3,8 @@ use std::path::Path;
 use std::process::Command;
 use std::process::Output;
 
-use log::{debug, error, info, warn};
-
-/// Known failures that occur from external commands.
-/// If an error occurs frequently enough, consider adding it here and use
-/// [std::process::exit] explicitly instead of panicking.
-enum Failures {
-    Rustfmt = 1,
-    Dartfmt,
-    FfigenLlvm,
-    MissingExe,
-}
+use crate::error::{Error, Result};
+use log::{debug, info, warn};
 
 #[must_use]
 fn call_shell(cmd: &str) -> Output {
@@ -24,21 +15,17 @@ fn call_shell(cmd: &str) -> Output {
     execute_command("sh", &["-c", cmd], None)
 }
 
-pub fn ensure_tools_available() {
+pub fn ensure_tools_available() -> Result {
     let output = call_shell("dart pub global list");
     let output = String::from_utf8_lossy(&output.stdout);
     if !output.contains("ffigen") {
-        error!(
-            "
-ffigen is not available, please run \"dart pub global activate ffigen\" first."
-        );
-        std::process::exit(Failures::MissingExe as _);
+        return Err(Error::MissingExe(String::from("ffigen")));
     }
 
-    check_shell_executable("cbindgen");
+    check_shell_executable("cbindgen")
 }
 
-pub fn check_shell_executable(cmd: &'static str) {
+pub fn check_shell_executable(cmd: &'static str) -> Result {
     #[cfg(windows)]
     let res = execute_command("where", &[cmd], None);
     #[cfg(not(windows))]
@@ -48,17 +35,10 @@ pub fn check_shell_executable(cmd: &'static str) {
         None,
     );
     if !res.status.success() {
-        error!(
-            "
-{cmd} is not a command, or not executable.
-Note: This command might be available via cargo, in which case it can be installed with:
-
-    cargo install {cmd}",
-            cmd = cmd
-        );
-        std::process::exit(Failures::MissingExe as _);
+        return Err(Error::MissingExe(cmd.to_string()));
     }
     debug!("{}", String::from_utf8_lossy(&res.stdout));
+    Ok(())
 }
 
 pub fn bindgen_rust_to_dart(
@@ -69,15 +49,15 @@ pub fn bindgen_rust_to_dart(
     c_struct_names: Vec<String>,
     llvm_install_path: &[String],
     llvm_compiler_opts: &str,
-) {
+) -> anyhow::Result<()> {
     cbindgen(rust_crate_dir, c_output_path, c_struct_names);
     ffigen(
         c_output_path,
         dart_output_path,
         dart_class_name,
         llvm_install_path,
         llvm_compiler_opts,
-    );
+    )
 }
 
 #[must_use = "Error path must be handled."]
@@ -193,7 +173,7 @@ fn ffigen(
     dart_class_name: &str,
     llvm_path: &[String],
     llvm_compiler_opts: &str,
-) {
+) -> anyhow::Result<()> {
     debug!(
         "execute ffigen c_path={} dart_path={} llvm_path={:?}",
         c_path, dart_path, llvm_path
@@ -219,10 +199,9 @@ fn ffigen(
             &mut config,
             "
         llvm-path:\n"
-        )
-        .unwrap();
+        )?;
         for path in llvm_path {
-            writeln!(&mut config, "           - '{}'", path).unwrap();
+            writeln!(&mut config, "           - '{}'", path)?;
         }
     }
 
@@ -237,8 +216,8 @@ fn ffigen(
 
     debug!("ffigen config: {}", config);
 
-    let mut config_file = tempfile::NamedTempFile::new().unwrap();
-    std::io::Write::write_all(&mut config_file, config.as_bytes()).unwrap();
+    let mut config_file = tempfile::NamedTempFile::new()?;
+    std::io::Write::write_all(&mut config_file, config.as_bytes())?;
     debug!("ffigen config_file: {:?}", config_file);
 
     // NOTE please install ffigen globally first: `dart pub global activate ffigen`
@@ -249,29 +228,25 @@ fn ffigen(
     if !res.status.success() {
         let err = String::from_utf8_lossy(&res.stderr);
         if err.contains("Couldn't find dynamic library in default locations.") {
-            error!(
-                "
-ffigen could not find LLVM.
-Please supply --llvm-path to flutter_rust_bridge_codegen, e.g.:
-
-    flutter_rust_bridge_codegen .. --llvm-path <path_to_llvm>"
-            );
-            std::process::exit(Failures::FfigenLlvm as _);
+            return Err(Error::FfigenLlvm.into());
         }
-        panic!("ffigen failed:\n{}", err);
+        return Err(Error::string(format!("ffigen failed:\n{}", err)).into());
     }
+    Ok(())
 }
 
-pub fn format_rust(path: &str) {
+pub fn format_rust(path: &str) -> Result {
     debug!("execute format_rust path={}", path);
     let res = execute_command("rustfmt", &[path], None);
     if !res.status.success() {
-        error!("rustfmt failed: {}", String::from_utf8_lossy(&res.stderr));
-        std::process::exit(Failures::Rustfmt as _);
+        return Err(Error::Rustfmt(
+            String::from_utf8_lossy(&res.stderr).to_string(),
+        ));
     }
+    Ok(())
 }
 
-pub fn format_dart(path: &str, line_length: i32) {
+pub fn format_dart(path: &str, line_length: i32) -> Result {
     debug!(
         "execute format_dart path={} line_length={}",
         path, line_length
@@ -281,15 +256,14 @@ pub fn format_dart(path: &str, line_length: i32) {
         path, line_length
     ));
     if !res.status.success() {
-        error!(
-            "dart format failed: {}",
-            String::from_utf8_lossy(&res.stderr)
-        );
-        std::process::exit(Failures::Dartfmt as _);
+        return Err(Error::Dartfmt(
+            String::from_utf8_lossy(&res.stderr).to_string(),
+        ));
     }
+    Ok(())
 }
 
-pub fn build_runner(dart_root: &str) {
+pub fn build_runner(dart_root: &str) -> Result {
     info!("Running build_runner at {}", dart_root);
     let out = if cfg!(windows) {
         call_shell(&format!(
@@ -303,10 +277,11 @@ pub fn build_runner(dart_root: &str) {
         ))
     };
     if !out.status.success() {
-        error!(
+        return Err(Error::StringError(format!(
             "Failed to run build_runner for {}: {}",
             dart_root,
             String::from_utf8_lossy(&out.stdout)
-        );
+        )));
     }
+    Ok(())
 }

frb_codegen/src/error.rs

@@ -1,34 +1,32 @@
-#[derive(Debug)]
-pub struct Error {
-    msg: String,
-}
+use thiserror::Error;
 
-impl Error {
-    pub fn new(msg: &str) -> Self {
-        Self {
-            msg: msg.to_owned(),
-        }
-    }
-}
+pub type Result = std::result::Result<(), Error>;
 
-impl std::fmt::Display for Error {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(&self.msg)
-    }
+#[derive(Error, Debug)]
+pub enum Error {
+    #[error("rustfmt failed: {0}")]
+    Rustfmt(String),
+    #[error("dart fmt failed: {0}")]
+    Dartfmt(String),
+    #[error(
+        "ffigen could not find LLVM.
+    Please supply --llvm-path to flutter_rust_bridge_codegen, e.g.:
+    
+        flutter_rust_bridge_codegen .. --llvm-path <path_to_llvm>"
+    )]
+    FfigenLlvm,
+    #[error("{0} is not a command, or not executable.")]
+    MissingExe(String),
+    #[error("{0}")]
+    StringError(String),
 }
 
-impl From<String> for Error {
-    fn from(error: String) -> Self {
-        Self { msg: error }
+impl Error {
+    pub fn str(msg: &str) -> Self {
+        Self::StringError(msg.to_owned())
     }
-}
 
-impl From<&str> for Error {
-    fn from(error: &str) -> Self {
-        Self {
-            msg: error.to_string(),
-        }
+    pub fn string(msg: String) -> Self {
+        Self::StringError(msg)
     }
 }
-
-impl std::error::Error for Error {}

frb_codegen/src/lib.rs

@@ -24,7 +24,7 @@ mod utils;
 use error::*;
 
 pub fn frb_codegen(raw_opts: Opts) -> anyhow::Result<()> {
-    ensure_tools_available();
+    ensure_tools_available()?;
 
     let config = config::parse(raw_opts);
     info!("Picked config: {:?}", &config);
@@ -60,12 +60,12 @@ pub fn frb_codegen(raw_opts: Opts) -> anyhow::Result<()> {
         &config.dart_wire_class_name(),
         config
             .dart_output_path_name()
-            .ok_or_else(|| Error::new("Invalid dart_output_path_name"))?,
+            .ok_or_else(|| Error::str("Invalid dart_output_path_name"))?,
     );
 
     info!("Phase: Other things");
 
-    commands::format_rust(&config.rust_output_path);
+    commands::format_rust(&config.rust_output_path)?;
 
     if !config.skip_add_mod_to_lib {
         others::try_add_mod_to_lib(&config.rust_crate_dir, &config.rust_output_path);
@@ -101,9 +101,9 @@ pub fn frb_codegen(raw_opts: Opts) -> anyhow::Result<()> {
                 c_struct_names,
                 &config.llvm_path[..],
                 &config.llvm_compiler_opts,
-            );
+            )
         },
-    );
+    )?;
 
     let effective_func_names = [
         generated_rust.extern_func_names,
@@ -126,7 +126,7 @@ pub fn frb_codegen(raw_opts: Opts) -> anyhow::Result<()> {
         &config.dart_wire_class_name(),
     ));
 
-    sanity_check(&generated_dart_wire.body, &config.dart_wire_class_name());
+    sanity_check(&generated_dart_wire.body, &config.dart_wire_class_name())?;
 
     let generated_dart_decl_all = generated_dart.decl_code;
     let generated_dart_impl_all = &generated_dart.impl_code + &generated_dart_wire;
@@ -161,23 +161,23 @@ pub fn frb_codegen(raw_opts: Opts) -> anyhow::Result<()> {
     let dart_root = &config.dart_root;
     if needs_freezed && config.build_runner {
         let dart_root = dart_root.as_ref().ok_or_else(|| {
-            Error::new(
+            Error::str(
                 "build_runner configured to run, but Dart root could not be inferred.
         Please specify --dart-root, or disable build_runner with --no-build-runner.",
             )
         })?;
-        commands::build_runner(dart_root);
+        commands::build_runner(dart_root)?;
         commands::format_dart(
             &config
                 .dart_output_freezed_path()
-                .ok_or_else(|| Error::new("Invalid freezed file path"))?,
+                .ok_or_else(|| Error::str("Invalid freezed file path"))?,
             config.dart_format_line_length,
-        );
+        )?;
     }
 
-    commands::format_dart(&config.dart_output_path, config.dart_format_line_length);
+    commands::format_dart(&config.dart_output_path, config.dart_format_line_length)?;
     if let Some(dart_decl_output_path) = &config.dart_decl_output_path {
-        commands::format_dart(dart_decl_output_path, config.dart_format_line_length);
+        commands::format_dart(dart_decl_output_path, config.dart_format_line_length)?;
     }
 
     info!("Success!");

frb_codegen/src/others.rs

@@ -4,7 +4,7 @@ use std::path::Path;
 
 use anyhow::{anyhow, Result};
 use lazy_static::lazy_static;
-use log::{error, info, warn};
+use log::{info, warn};
 use pathdiff::diff_paths;
 use regex::RegexBuilder;
 
@@ -107,13 +107,18 @@ pub fn extract_dart_wire_content(content: &str) -> DartBasicCode {
     }
 }
 
-pub fn sanity_check(generated_dart_wire_code: &str, dart_wire_class_name: &str) {
+pub fn sanity_check(
+    generated_dart_wire_code: &str,
+    dart_wire_class_name: &str,
+) -> anyhow::Result<()> {
     if !generated_dart_wire_code.contains(dart_wire_class_name) {
-        error!(
+        return Err(crate::error::Error::str(
             "Nothing is generated for dart wire class. \
-            Maybe you forget to put code like `mod the_generated_bridge_code;` to your `lib.rs`?"
-        );
+            Maybe you forget to put code like `mod the_generated_bridge_code;` to your `lib.rs`?",
+        )
+        .into());
     }
+    Ok(())
 }
 
 pub fn try_add_mod_to_lib(rust_crate_dir: &str, rust_output_path: &str) {

frb_codegen/src/utils.rs

@@ -12,11 +12,15 @@ pub fn mod_from_rust_path(code_path: &str, crate_path: &str) -> String {
         .replace('/', "::")
 }
 
-pub fn with_changed_file<F: FnOnce()>(path: &str, append_content: &str, f: F) {
-    let content_original = fs::read_to_string(&path).unwrap();
-    fs::write(&path, content_original.clone() + append_content).unwrap();
+pub fn with_changed_file<F: FnOnce() -> anyhow::Result<()>>(
+    path: &str,
+    append_content: &str,
+    f: F,
+) -> anyhow::Result<()> {
+    let content_original = fs::read_to_string(&path)?;
+    fs::write(&path, content_original.clone() + append_content)?;
 
-    f();
+    f()?;
 
-    fs::write(&path, content_original).unwrap();
+    Ok(fs::write(&path, content_original)?)
 }