Add stricter lints and make them happy

Author: tombh

.github/workflows/push.yaml

@@ -1,8 +1,8 @@
 name: push
 
-on: 
+on:
   push:
-    branches: 
+    branches:
       - main
   pull_request:
 
@@ -13,18 +13,18 @@ env:
 jobs:
   install-and-build-shaders:
     strategy:
-      matrix: 
+      matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
     runs-on: ${{ matrix.os }}
     defaults:
-      run: 
+      run:
         shell: bash
-    env: 
+    env:
       RUST_LOG: debug
     steps:
       - uses: actions/checkout@v2
       - uses: moonrepo/setup-rust@v1
-      - run: rustup default stable 
+      - run: rustup default stable
       - run: rustup update
       - run: cargo test
       - run: cargo install --path crates/cargo-gpu

Cargo.toml

@@ -1,6 +1,6 @@
 [workspace]
 members = [
-    "crates/cargo-gpu", 
+    "crates/cargo-gpu",
     "crates/shader-crate-template"
 ]
 
@@ -21,3 +21,32 @@ relative-path = "1.9.3"
 serde = { version = "1.0.214", features = ["derive"] }
 serde_json = "1.0.132"
 toml = "0.8.19"
+
+[workspace.lints.rust]
+missing_docs = "warn"
+
+[workspace.lints.clippy]
+all = { level = "warn", priority = 0 }
+pedantic = { level = "warn", priority = 0 }
+nursery = { level = "warn", priority = 0 }
+cargo = { level = "warn", priority = 0 }
+restriction = { level = "warn", priority = 0 }
+blanket_clippy_restriction_lints = { level = "allow", priority = 1 }
+
+arithmetic_side_effects = { level = "allow", priority = 1 }
+absolute_paths = { level = "allow", priority = 1 }
+cargo_common_metadata = { level = "allow", priority = 1 }
+implicit_return = { level = "allow", priority = 1 }
+single_call_fn = { level = "allow", priority = 1 }
+question_mark_used = { level = "allow", priority = 1 }
+multiple_crate_versions = { level = "allow", priority = 1 }
+pub_with_shorthand = { level = "allow", priority = 1 }
+partial_pub_fields = { level = "allow", priority = 1 }
+pattern_type_mismatch = { level = "allow", priority = 1 }
+print_stdout = { level = "allow", priority = 1 }
+std_instead_of_alloc = { level = "allow", priority = 1 }
+
+# TODO: Try to not depend on these lints
+unwrap_used = { level = "allow", priority = 1 }
+panic = { level = "allow", priority = 1 }
+

crates/cargo-gpu/Cargo.toml

@@ -3,8 +3,9 @@ name = "cargo-gpu"
 version = "0.1.0"
 edition = "2021"
 description = "Generates shader .spv files from rust-gpu shader crates"
+repository = "https://github.com/Rust-GPU/cargo-gpu"
 readme = "../../README.md"
-
+keywords = ["gpu", "compiler"]
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
@@ -33,3 +34,6 @@ codegen-units = 256
 opt-level = 3
 incremental = true
 codegen-units = 256
+
+[lints]
+workspace = true

crates/cargo-gpu/src/builder.rs

@@ -1,12 +1,16 @@
-use std::io::Write;
+//! `cargo gpu build`, analogous to `cargo build`
+
+use std::io::Write as _;
 
 use clap::Parser;
 use spirv_builder_cli::{Linkage, ShaderModule};
 
 use crate::{install::Install, target_spec_dir};
 
+/// `cargo build` subcommands
 #[derive(Parser, Debug)]
-pub(crate) struct Build {
+pub struct Build {
+    /// Install the `rust-gpu` compiler and components
     #[clap(flatten)]
     install: Install,
 
@@ -32,6 +36,7 @@ pub(crate) struct Build {
 }
 
 impl Build {
+    /// Entrypoint
     pub fn run(&mut self) {
         let (dylib_path, spirv_builder_cli_path) = self.install.run();
 
@@ -93,7 +98,7 @@ impl Build {
                      entry,
                      path: filepath,
                  }| {
-                    use relative_path::PathExt;
+                    use relative_path::PathExt as _;
                     let path = self.output_dir.join(filepath.file_name().unwrap());
                     std::fs::copy(&filepath, &path).unwrap();
                     let path_relative_to_shader_crate =
@@ -109,19 +114,19 @@ impl Build {
         linkage.sort();
         // UNWRAP: safe because we know this always serializes
         let json = serde_json::to_string_pretty(&linkage).unwrap();
-        let mut file = std::fs::File::create(&manifest_path).unwrap_or_else(|e| {
+        let mut file = std::fs::File::create(&manifest_path).unwrap_or_else(|error| {
             log::error!(
-                "could not create shader manifest file '{}': {e}",
+                "could not create shader manifest file '{}': {error}",
                 manifest_path.display(),
             );
-            panic!("{e}")
+            panic!("{error}")
         });
-        file.write_all(json.as_bytes()).unwrap_or_else(|e| {
+        file.write_all(json.as_bytes()).unwrap_or_else(|error| {
             log::error!(
-                "could not write shader manifest file '{}': {e}",
+                "could not write shader manifest file '{}': {error}",
                 manifest_path.display(),
             );
-            panic!("{e}")
+            panic!("{error}")
         });
 
         log::info!("wrote manifest to '{}'", manifest_path.display());

crates/cargo-gpu/src/install.rs

@@ -1,16 +1,25 @@
-use std::io::Write;
+//! Install a dedicated per-shader crate that has the `rust-gpu` compiler in it.
+use std::io::Write as _;
 
 use crate::{cache_dir, spirv::Spirv, target_spec_dir};
 
-const SPIRV_BUILDER_CLI_CARGO_TOML: &str = include_str!("../../spirv-builder-cli/Cargo.toml");
-const SPIRV_BUILDER_CLI_MAIN: &str = include_str!("../../spirv-builder-cli/src/main.rs");
-const SPIRV_BUILDER_CLI_LIB: &str = include_str!("../../spirv-builder-cli/src/lib.rs");
+/// These are the files needed to create the dedicated, per-shader `rust-gpu` builder create.
 const SPIRV_BUILDER_FILES: &[(&str, &str)] = &[
-    ("Cargo.toml", SPIRV_BUILDER_CLI_CARGO_TOML),
-    ("src/main.rs", SPIRV_BUILDER_CLI_MAIN),
-    ("src/lib.rs", SPIRV_BUILDER_CLI_LIB),
+    (
+        "Cargo.toml",
+        include_str!("../../spirv-builder-cli/Cargo.toml"),
+    ),
+    (
+        "src/main.rs",
+        include_str!("../../spirv-builder-cli/src/main.rs"),
+    ),
+    (
+        "src/lib.rs",
+        include_str!("../../spirv-builder-cli/src/lib.rs"),
+    ),
 ];
 
+/// Metadata for the compile targets supported by `rust-gpu`
 const TARGET_SPECS: &[(&str, &str)] = &[
     (
         "spirv-unknown-opengl4.0.json",
@@ -74,15 +83,16 @@ const TARGET_SPECS: &[(&str, &str)] = &[
     ),
 ];
 
+/// `cargo gpu install`
 #[derive(clap::Parser, Debug)]
-pub(crate) struct Install {
+pub struct Install {
     /// spirv-builder dependency, written just like in a Cargo.toml file.
     #[clap(long, default_value = Spirv::DEFAULT_DEP)]
     spirv_builder: String,
 
     /// Rust toolchain channel to use to build `spirv-builder`.
     ///
-    /// This must match the `spirv_builder` argument.
+    /// This must be compatible with the `spirv_builder` argument as defined in the `rust-gpu` repo.
     #[clap(long, default_value = Spirv::DEFAULT_CHANNEL)]
     rust_toolchain: String,
 
@@ -92,18 +102,20 @@ pub(crate) struct Install {
 }
 
 impl Install {
+    /// Returns a [`Spirv`] instance, responsible for ensuring the right version of the `spirv-builder-cli` crate.
     fn spirv_cli(&self) -> Spirv {
         Spirv {
             dep: self.spirv_builder.clone(),
             channel: self.rust_toolchain.clone(),
         }
     }
 
+    /// Create the `spirv-builder-cli` crate.
     fn write_source_files(&self) {
         let cli = self.spirv_cli();
         let checkout = cli.cached_checkout_path();
         std::fs::create_dir_all(checkout.join("src")).unwrap();
-        for (filename, contents) in SPIRV_BUILDER_FILES.iter() {
+        for (filename, contents) in SPIRV_BUILDER_FILES {
             log::debug!("writing {filename}");
             let path = checkout.join(filename);
             let mut file = std::fs::File::create(&path).unwrap();
@@ -114,8 +126,9 @@ impl Install {
         }
     }
 
+    /// Add the target spec files to the crate.
     fn write_target_spec_files(&self) {
-        for (filename, contents) in TARGET_SPECS.iter() {
+        for (filename, contents) in TARGET_SPECS {
             let path = target_spec_dir().join(filename);
             if !path.is_file() || self.force_spirv_cli_rebuild {
                 let mut file = std::fs::File::create(&path).unwrap();
@@ -124,14 +137,14 @@ impl Install {
         }
     }
 
-    // Install the binary pair and return the paths, (dylib, cli).
+    /// Install the binary pair and return the paths, (dylib, cli).
     pub fn run(&self) -> (std::path::PathBuf, std::path::PathBuf) {
         // Ensure the cache dir exists
         let cache_dir = cache_dir();
         log::info!("cache directory is '{}'", cache_dir.display());
-        std::fs::create_dir_all(&cache_dir).unwrap_or_else(|e| {
+        std::fs::create_dir_all(&cache_dir).unwrap_or_else(|error| {
             log::error!(
-                "could not create cache directory '{}': {e}",
+                "could not create cache directory '{}': {error}",
                 cache_dir.display()
             );
             panic!("could not create cache dir");
@@ -178,7 +191,7 @@ impl Install {
 
             command.args([
                 "--features",
-                &Self::get_required_spirv_builder_version(spirv_version.channel),
+                &Self::get_required_spirv_builder_version(&spirv_version.channel),
             ]);
 
             log::debug!("building artifacts with `{:?}`", command);
@@ -209,8 +222,8 @@ impl Install {
             } else {
                 log::error!("could not find {}", cli_path.display());
                 log::debug!("contents of '{}':", release.display());
-                for entry in std::fs::read_dir(&release).unwrap() {
-                    let entry = entry.unwrap();
+                for maybe_entry in std::fs::read_dir(&release).unwrap() {
+                    let entry = maybe_entry.unwrap();
                     log::debug!("{}", entry.file_name().to_string_lossy());
                 }
                 panic!("spirv-builder-cli build failed");
@@ -228,9 +241,9 @@ impl Install {
     ///     `spirv-builder` version from there.
     ///   * Warn the user that certain `cargo-gpu` features aren't available when building with
     ///     older versions of `spirv-builder`, eg setting the target spec.
-    fn get_required_spirv_builder_version(toolchain_channel: String) -> String {
+    fn get_required_spirv_builder_version(toolchain_channel: &str) -> String {
         let parse_date = chrono::NaiveDate::parse_from_str;
-        let datetime = parse_date(&toolchain_channel, "nightly-%Y-%m-%d").unwrap();
+        let datetime = parse_date(toolchain_channel, "nightly-%Y-%m-%d").unwrap();
         let pre_cli_date = parse_date("2024-04-24", "%Y-%m-%d").unwrap();
 
         if datetime < pre_cli_date {