Document and clean code. cargo clippy now passes

src/builders/builder_trait.rs

@@ -5,14 +5,42 @@ use std::fmt::Debug;
 use std::path::Path;
 
 pub trait BuilderImpl: Sized {
+    /// Generate a builder object from a python object.
+    ///
+    /// Returns [`Some(obj)`] if the operation was successful, otherwise [`None`]
     fn from_py(object: &Bound<PyAny>) -> Option<Self>;
 
+    /// Perform the build operation specified by the struct.
+    ///
+    /// For example, if this is a [`CMake`] instance, `cmake` is run on the
+    /// `source_path` and the resulting build files are written to `output_path`.
+    ///
+    /// # Errors
+    ///
+    /// Returns [`Err(string)`] if the build script fails to run. This could happen
+    /// for many reasons, including:
+    ///  - Source directory does not exist
+    ///  - Source directory does not contain a valid build script configuration
+    ///  - The code fails to compile
+    ///  - The build files cannot be written to the build directory
     fn build<P0: AsRef<Path> + Debug, P1: AsRef<Path> + Debug>(
         &self,
         source_path: &P0,
         output_path: &P1,
     ) -> Result<(), String>;
 
+    /// Perform the install operation specified by the struct.
+    ///
+    /// For example, if this is a [`CMake`] instance, `cmake --install ...` is run
+    /// on the `build_path` and the install files are installed in `install_path`.
+    ///
+    /// # Errors
+    ///
+    /// Returns [`Err(string)`] if the install script fails to run. This could
+    /// happen for many reasons, including:
+    ///  - Build directory does not exist
+    ///  - Build directory does not contain valid installation information
+    ///  - The install files cannot be written to `install_path`
     fn install<P0: AsRef<Path>, P1: AsRef<Path>>(
         &self,
         build_path: &P0,

src/builders/cmake.rs

@@ -67,7 +67,7 @@ impl CMake {
 
     fn compile<P: AsRef<Path> + std::fmt::Debug>(&self, path: &P) -> Result<(), String> {
         let mut cmake = Command::new("cmake");
-        cmake.current_dir(&path);
+        cmake.current_dir(path);
         cmake.arg("--build");
         cmake.arg(".");
         cmake.arg(format!("--config {:?}", self.build_type));

src/callbacks.rs

@@ -1,16 +1,27 @@
 use crate::{config, log, module::get_modules};
-use colored::*;
+use colored::Colorize;
 
+/// A callback function to list all available modules
+///
+/// # Errors
+///
+/// This function will error if an invalid modulefile is found.
 pub fn list_callback(_config: &config::Config) -> Result<(), String> {
     println!("{}", "Available Modules:".bold().purple());
 
     for p in &get_modules()? {
-        println!("{} {}", " >", p.identifier.bold().cyan());
+        println!(" > {}", p.identifier.bold().cyan());
     }
 
     Ok(())
 }
 
+/// A callback function to download a module from its name.
+///
+/// # Errors
+///
+/// Will error if a single module cannot be resolved from the specified name,
+/// or if the call to [`Module.download`] fails.
 pub fn download_module(name: &str, _config: &config::Config) -> Result<(), String> {
     for module in &get_modules()? {
         if name == module.identifier {
@@ -22,6 +33,12 @@ pub fn download_module(name: &str, _config: &config::Config) -> Result<(), Strin
     Ok(())
 }
 
+/// A callback function to build a module based on its name.
+///
+/// # Errors
+///
+/// Errors if a single module cannot be resolved from the specified name,
+/// or if the call to [`Module.build`] fails.
 pub fn build_module(name: &str, _config: &config::Config) -> Result<(), String> {
     for module in &get_modules()? {
         if name == module.identifier {
@@ -36,6 +53,12 @@ pub fn build_module(name: &str, _config: &config::Config) -> Result<(), String>
     Ok(())
 }
 
+/// A callback function to install a module from its name.
+///
+/// # Errors
+///
+/// Returns [`Err(string)`] if a single module cannot be resolved from the
+/// specified name, or if the call to [`Module.install`] fails.
 pub fn install_module(name: &str, _config: &config::Config) -> Result<(), String> {
     for module in &get_modules()? {
         if name == module.identifier {

src/cli.rs

@@ -166,6 +166,11 @@ impl Command {
         res
     }
 
+    /// Consume a command recursively, running callbacks where suitable.
+    ///
+    /// # Errors
+    ///
+    /// Errors if invalid commands were passed or if the callback fails.
     pub fn consume(
         &self,
         config: &config::Config,

src/log.rs

@@ -39,6 +39,13 @@ pub fn status(message: &str) {
     );
 }
 
+/// Print `message` to the console, marked as `information`, but
+/// append a carriage return so the cursor will print from the
+/// start of the line on the next output.
+///
+/// # Panics
+///
+/// Panics if the call to [`std::io::stdout().flush()`] fails.
 pub fn info_carriage(message: &str) {
     // Clear the line
     print!("\x1b[K");
@@ -52,6 +59,13 @@ pub fn info_carriage(message: &str) {
     std::io::stdout().flush().unwrap();
 }
 
+/// Print `message` to the console, marked as `warn`, but
+/// append a carriage return so the cursor will print from the
+/// start of the line on the next output.
+///
+/// # Panics
+///
+/// Panics if the call to [`std::io::stdout().flush()`] fails.
 pub fn warn_carriage(message: &str) {
     // Clear the line
     print!("\x1b[K");

src/main.rs

@@ -63,5 +63,5 @@ fn cli(config: &config::Config) -> Result<(), String> {
     };
 
     let cmd = command.generate_command();
-    command.consume(&config, &cmd.get_matches())
+    command.consume(config, &cmd.get_matches())
 }

src/module.rs

@@ -39,12 +39,17 @@ impl Module {
     /// Download the source code for the module, based on its [`Downloader`].
     ///
     /// # Errors
-    /// This will error if the download fails, with an error [`String`] containing either an
-    /// error message or the output of the errored command.
+    /// This will error if the download fails, with an error [`String`] containing
+    /// either an error message or the output of the errored command.
     pub fn download<P: AsRef<Path>>(&self, path: &P) -> Result<(), String> {
         self.downloader.download(path)
     }
 
+    /// Build the source code for this module, based on its [`Builder`].
+    ///
+    /// # Errors
+    /// This will error if the build fails, with an error [`String`] containing
+    /// either an error message or the output of the errored command.
     pub fn build<P0: AsRef<Path> + std::fmt::Debug, P1: AsRef<Path> + std::fmt::Debug>(
         &self,
         source_path: &P0,
@@ -53,6 +58,11 @@ impl Module {
         self.builder.build(source_path, output_path)
     }
 
+    /// Install the source code for this module based on its [`Builder`].
+    ///
+    /// # Errors
+    /// Errors if the installation fails. The [`Result`] output contains a [`String`]
+    /// with either an error message or the output of the errored program.
     pub fn install<P0: AsRef<Path> + std::fmt::Debug, P1: AsRef<Path> + std::fmt::Debug>(
         &self,
         build_path: &P0,
@@ -61,11 +71,16 @@ impl Module {
         self.builder.install(build_path, install_path)
     }
 
+    /// Extract a [`Module`] object from a python object.
+    ///
+    /// # Errors
+    /// This method will return [`Err(msg)`] if the object cannot be parsed
+    /// successfully. `msg` is a string and contains the error message.
     pub fn from_object<P0: AsRef<Path>>(
         object: &Bound<PyAny>,
         path: &P0,
         config: &config::Config,
-    ) -> Result<Module, String> {
+    ) -> Result<Self, String> {
         Python::with_gil(|_| {
             let metadata: HashMap<String, String> = extract_object(object, "metadata")?
                 .call0()
@@ -97,34 +112,29 @@ impl Module {
                 .to_str()
                 .ok_or("Failed to convert filename to string")?
                 .split(PATH_SEP)
-                .map(|x| x.to_string())
+                .map(std::string::ToString::to_string)
                 .collect();
 
             let identifier = metadata
                 .get("identifier")
                 .ok_or("Metadata does not contain 'identifier' tag")?
-                .replace(" ", "_")
-                .replace("\t", "_");
+                .replace([' ', '\t'], "_");
 
             // Download path is ${root}/(${download_path} or ${identifier})
             let download_path = format!(
                 "{}{}{}",
                 config.build_root,
                 PATH_SEP,
-                match metadata.get("download_path") {
-                    Some(path) => path,
-                    None => &identifier,
-                }
+                metadata
+                    .get("download_path")
+                    .map_or(&identifier, |path| path)
             );
 
             let build_path = format!(
                 "{}{}{}",
                 config.build_root,
                 PATH_SEP,
-                match metadata.get("build_path") {
-                    Some(path) => path,
-                    None => &identifier,
-                }
+                metadata.get("build_path").map_or(&identifier, |path| path)
             );
 
             let install_path = format!("{}{}{}", config.install_root, PATH_SEP, identifier);
@@ -161,7 +171,7 @@ pub fn get_modules() -> Result<Vec<Module>, String> {
                     || vec![Err("Failed to extract paths".to_string())],
                     |paths| {
                         // Map path -> Ok(path)
-                        paths.into_iter().map(|p| Ok(p)).collect()
+                        paths.into_iter().map(Ok).collect()
                     },
                 )
             })
@@ -198,25 +208,32 @@ pub fn get_modules() -> Result<Vec<Module>, String> {
     })
 }
 
-pub fn download_module(module: &Module) -> Result<(), String> {
+/// Download a module.
+///
+/// # Errors
+/// Errors if [`Module.download`] fails.
+pub fn download(module: &Module) -> Result<(), String> {
     log::status(&format!("Downloading '{}'", module.identifier));
     module.download(&module.download_path)
 }
 
-pub fn build_module(module: &Module) -> Result<(), String> {
-    log::status(&format!("Downloading '{}'", module.identifier));
-    module.download(&module.download_path)?;
+/// Download and build a module.
+///
+/// # Errors
+/// Errors if [`Module.download`] fails or [`Module.build`] fails.
+pub fn build(module: &Module) -> Result<(), String> {
+    download(module)?;
 
     log::status(&format!("Building '{}'", module.identifier));
     module.build(&module.download_path, &module.install_path)
 }
 
-pub fn install_module(module: &Module) -> Result<(), String> {
-    log::status(&format!("Downloading '{}'", module.identifier));
-    module.download(&module.download_path)?;
-
-    log::status(&format!("Building '{}'", module.identifier));
-    module.build(&module.download_path, &module.build_path)?;
+/// Download, build and install a module.
+///
+/// # Errors
+/// Errors if [`Module.download`], [`Module.build`] or [`Module.install`] fails.
+pub fn install(module: &Module) -> Result<(), String> {
+    build(module)?;
 
     log::status(&format!("Installing '{}'", module.identifier));
     module.install(&module.build_path, &module.install_path)

src/python_interop.rs

@@ -2,6 +2,11 @@ use pyo3::prelude::*;
 use std::fs::read_to_string;
 use std::path::Path;
 
+/// Load a Python program from a file path.
+///
+/// # Errors
+/// Errors if the file does not exist, cannot be read or contains invalid
+/// Python code.
 pub fn load_program<'a, P: AsRef<Path> + std::fmt::Debug>(
     py: &'a Python,
     path: &P,
@@ -16,6 +21,10 @@ pub fn load_program<'a, P: AsRef<Path> + std::fmt::Debug>(
         .map_err(|err| format!("Failed to load python program: {err}"))
 }
 
+/// Extract a named attribute of a Python object.
+///
+/// # Errors
+/// Errors if the attribute cannot be found.
 pub fn extract_object<'a>(
     object: &Bound<'a, PyAny>,
     name: &str,