Merge pull request #907 from robamu/ci-update-clippy-and-docs

burrbull · web-flow · commit a89a90d58c32 · 2025-03-07T06:32:34.000Z
update CI to run clippy and docs
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -10,7 +10,7 @@ jobs:
   ci:
     name: CI
     runs-on: ubuntu-latest
-    needs: [check, ci-linux, ci-clippy, ci-serde]
+    needs: [check, ci-linux, ci-docs-clippy, ci-serde]
     if: always()
     steps:
       - name: Done
@@ -127,15 +127,33 @@ jobs:
         # stable.
         run: cargo +stable regress tests --toolchain 1.76.0 -m Nordic -- --strict --atomics
 
-  ci-clippy:
+  ci-docs-clippy:
     runs-on: ubuntu-latest
     needs: [check]
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          # STMicro
+          - { chip: STM32F030 }
+          - { chip: STM32F410 }
+          - { chip: STM32L1xx }
+          # Espressif
+          - { chip: esp32c3 }
+          # Freescale
+          - { chip: MKW22D5 }
+          - { chip: MK02F12810 }
+          # Silicon Labs
+          # TODO: fix doc rendering bug when math `>` is present in description
+          #- { chip: SIM3L1x8_SVD }
+          # Nordic chips
+          - { chip: nrf51, options: "-- -f register_mod::s:_mod" }
+          - { chip: nrf52, options: "-- -f register_mod::s:_mod" }
     steps:
       - uses: actions/checkout@v4
 
-      - uses: dtolnay/rust-toolchain@master
-        with:
-          toolchain: stable
+      - uses: dtolnay/rust-toolchain@nightly
+      - uses: dtolnay/rust-toolchain@stable
 
       - name: Cache
         uses: Swatinem/rust-cache@v2
@@ -144,12 +162,8 @@ jobs:
         run: |
           cargo install svd2rust --path .
 
-      - name: Run CI script
-        env:
-          VENDOR: RISC-V
-          OPTIONS: ""
-          COMMAND: clippy
-        run: bash ci/script.sh
+      - name: Check docs and clippy on generated PACs
+        run: cargo regress test -c ${{ matrix.chip }} --docs-stable --docs-nightly --clippy ${{ matrix.options }}
 
   ci-serde:
     runs-on: ubuntu-latest
diff --git a/ci/svd2rust-regress/src/command.rs b/ci/svd2rust-regress/src/command.rs
@@ -7,7 +7,8 @@ pub trait CommandExt {
     fn run(&mut self, hide: bool) -> Result<(), anyhow::Error>;
 
     #[track_caller]
-    fn get_output(&mut self, can_fail: bool) -> Result<std::process::Output, anyhow::Error>;
+    fn run_and_get_output(&mut self, can_fail: bool)
+        -> Result<std::process::Output, anyhow::Error>;
 
     #[track_caller]
     fn get_output_string(&mut self) -> Result<String, anyhow::Error>;
@@ -33,7 +34,10 @@ impl CommandExt for Command {
     }
 
     #[track_caller]
-    fn get_output(&mut self, can_fail: bool) -> Result<std::process::Output, anyhow::Error> {
+    fn run_and_get_output(
+        &mut self,
+        can_fail: bool,
+    ) -> Result<std::process::Output, anyhow::Error> {
         let output = self
             .output()
             .with_context(|| format!("command `{}` couldn't be run", self.display()))?;
@@ -51,7 +55,7 @@ impl CommandExt for Command {
 
     #[track_caller]
     fn get_output_string(&mut self) -> Result<String, anyhow::Error> {
-        String::from_utf8(self.get_output(true)?.stdout).map_err(Into::into)
+        String::from_utf8(self.run_and_get_output(true)?.stdout).map_err(Into::into)
     }
 
     fn display(&self) -> String {
diff --git a/ci/svd2rust-regress/src/github.rs b/ci/svd2rust-regress/src/github.rs
@@ -148,7 +148,7 @@ pub fn get_release_binary_artifact(
                 Command::new("gzip")
                     .arg("-d")
                     .arg(output_dir.join(artifact))
-                    .get_output(false)?;
+                    .run_and_get_output(false)?;
             }
         }
         _ => {
diff --git a/ci/svd2rust-regress/src/main.rs b/ci/svd2rust-regress/src/main.rs
@@ -90,6 +90,18 @@ pub struct TestAll {
     /// Enable splitting `lib.rs` with `form`
     pub form_lib: bool,
 
+    /// Check generated crates with clippy.
+    #[clap(long)]
+    pub clippy: bool,
+
+    /// Check documentation build with stable.
+    #[clap(long)]
+    pub docs_stable: bool,
+
+    /// Check documentation build with nightly settings (docs.rs equivalent).
+    #[clap(long)]
+    pub docs_nightly: bool,
+
     /// Print all available test using the specified filters
     #[clap(long)]
     pub list: bool,
@@ -143,6 +155,18 @@ pub struct Test {
     /// Chip to use, use `--url` or `--svd-file` for another way to specify svd
     pub chip: Option<String>,
 
+    /// Check generated crate with clippy.
+    #[arg(long)]
+    pub clippy: bool,
+
+    /// Check documentation build with stable.
+    #[clap(long)]
+    pub docs_stable: bool,
+
+    /// Check documentation build with nightly settings (docs.rs equivalent).
+    #[clap(long)]
+    pub docs_nightly: bool,
+
     /// Path to an `svd2rust` binary, relative or absolute.
     /// Defaults to `target/release/svd2rust[.exe]` of this repository
     /// (which must be already built)
@@ -191,7 +215,14 @@ impl Test {
                 .ok_or_else(|| anyhow::anyhow!("no test found for chip"))?
                 .to_owned()
         };
-        test.test(opts, &self.current_bin_path, &self.passthrough_opts)?;
+        test.test(
+            opts,
+            &self.current_bin_path,
+            self.clippy,
+            self.docs_stable,
+            self.docs_nightly,
+            &self.passthrough_opts,
+        )?;
         Ok(())
     }
 }
@@ -247,7 +278,14 @@ impl TestAll {
         tests.par_iter().for_each(|t| {
             let start = Instant::now();
 
-            match t.test(opt, &self.current_bin_path, &self.passthrough_opts) {
+            match t.test(
+                opt,
+                &self.current_bin_path,
+                self.clippy,
+                self.docs_stable,
+                self.docs_nightly,
+                &self.passthrough_opts,
+            ) {
                 Ok(s) => {
                     if let Some(stderrs) = s {
                         let mut buf = String::new();
diff --git a/ci/svd2rust-regress/src/svd_test.rs b/ci/svd2rust-regress/src/svd_test.rs
@@ -71,27 +71,43 @@ impl std::fmt::Debug for ProcessFailed {
 }
 
 trait CommandHelper {
-    fn capture_outputs(
+    fn run_and_capture_outputs(
         &mut self,
         cant_fail: bool,
         name: &str,
         stdout: Option<&PathBuf>,
         stderr: Option<&PathBuf>,
         previous_processes_stderr: &[PathBuf],
     ) -> Result<(), TestError>;
+
+    fn run_and_capture_stderr(
+        &mut self,
+        cant_fail: bool,
+        name: &str,
+        stderr: &PathBuf,
+        previous_processes_stderr: &[PathBuf],
+    ) -> Result<(), TestError> {
+        self.run_and_capture_outputs(
+            cant_fail,
+            name,
+            None,
+            Some(stderr),
+            previous_processes_stderr,
+        )
+    }
 }
 
 impl CommandHelper for Command {
     #[tracing::instrument(skip_all, fields(stdout = tracing::field::Empty, stderr = tracing::field::Empty))]
-    fn capture_outputs(
+    fn run_and_capture_outputs(
         &mut self,
         cant_fail: bool,
         name: &str,
         stdout: Option<&PathBuf>,
         stderr: Option<&PathBuf>,
         previous_processes_stderr: &[PathBuf],
     ) -> Result<(), TestError> {
-        let output = self.get_output(true)?;
+        let output = self.run_and_get_output(true)?;
         let out_payload = String::from_utf8_lossy(&output.stdout);
         if let Some(out) = stdout {
             file_helper(&out_payload, out)?;
@@ -142,41 +158,98 @@ impl TestCase {
         &self,
         opts: &Opts,
         bin_path: &Path,
-        cli_opts: &Option<Vec<String>>,
+        run_clippy: bool,
+        run_docs_stable: bool,
+        run_docs_nightly: bool,
+        cli_passthrough_opts: &Option<Vec<String>>,
     ) -> Result<Option<Vec<PathBuf>>, TestError> {
         let (chip_dir, mut process_stderr_paths) = self
-            .setup_case(&opts.output_dir, bin_path, cli_opts)
+            .setup_case(&opts.output_dir, bin_path, cli_passthrough_opts)
             .with_context(|| anyhow!("when setting up case for {}", self.name()))?;
         // Run `cargo check`, capturing stderr to a log file
         if !self.skip_check {
             let cargo_check_err_file = path_helper_base(&chip_dir, &["cargo-check.err.log"]);
             Command::new("cargo")
                 .arg("check")
                 .current_dir(&chip_dir)
-                .capture_outputs(
+                .run_and_capture_stderr(
                     true,
                     "cargo check",
-                    None,
-                    Some(&cargo_check_err_file),
+                    &cargo_check_err_file,
                     &process_stderr_paths,
                 )
-                .with_context(|| "failed to check")?;
+                .with_context(|| "failed to check with cargo check")?;
             process_stderr_paths.push(cargo_check_err_file);
         }
+        if run_docs_nightly {
+            tracing::info!("Checking docs build with nightly");
+            let cargo_docs_err_file = path_helper_base(&chip_dir, &["cargo-docs-nightly.err.log"]);
+            // Docs are built like docs.rs would build them. Additionally, build with all features.
+
+            // Set the RUSTDOCFLAGS environment variable
+            let rustdocflags = "--cfg docsrs --generate-link-to-definition -Z unstable-options";
+            Command::new("cargo")
+                .arg("+nightly")
+                .arg("doc")
+                .arg("--all-features")
+                .env("RUSTDOCFLAGS", rustdocflags) // Set the environment variable
+                .current_dir(&chip_dir)
+                .run_and_capture_stderr(
+                    true,
+                    "cargo docs nightly",
+                    &cargo_docs_err_file,
+                    &process_stderr_paths,
+                )
+                .with_context(|| "failed to generate docs with cargo docs")?;
+        }
+        if run_docs_stable {
+            tracing::info!("Checking docs build with stable");
+            let cargo_docs_err_file = path_helper_base(&chip_dir, &["cargo-docs-stable.err.log"]);
+            // Docs are built like docs.rs would build them. Additionally, build with all features.
+            Command::new("cargo")
+                .arg("+stable")
+                .arg("doc")
+                .arg("--all-features")
+                .current_dir(&chip_dir)
+                .run_and_capture_stderr(
+                    true,
+                    "cargo docs stable",
+                    &cargo_docs_err_file,
+                    &process_stderr_paths,
+                )
+                .with_context(|| "failed to generate docs with cargo docs")?;
+        }
+        if run_clippy {
+            tracing::info!("Checking with clippy");
+            let cargo_clippy_err_file = path_helper_base(&chip_dir, &["cargo-clippy.err.log"]);
+            Command::new("cargo")
+                .arg("clippy")
+                .arg("--")
+                .arg("-D")
+                .arg("warnings")
+                .current_dir(&chip_dir)
+                .run_and_capture_stderr(
+                    true,
+                    "cargo clippy",
+                    &cargo_clippy_err_file,
+                    &process_stderr_paths,
+                )
+                .with_context(|| "failed to check with cargo clippy")?;
+        }
         Ok(if opts.verbose > 1 {
             Some(process_stderr_paths)
         } else {
             None
         })
     }
 
-    #[tracing::instrument(skip(self, output_dir, command), fields(name = %self.name(), chip_dir = tracing::field::Empty))]
+    #[tracing::instrument(skip(self, output_dir, passthrough_opts), fields(name = %self.name(), chip_dir = tracing::field::Empty))]
 
     pub fn setup_case(
         &self,
         output_dir: &Path,
         svd2rust_bin_path: &Path,
-        command: &Option<Vec<String>>,
+        passthrough_opts: &Option<Vec<String>>,
     ) -> Result<(PathBuf, Vec<PathBuf>), TestError> {
         let user = match std::env::var("USER") {
             Ok(val) => val,
@@ -210,10 +283,10 @@ impl TestCase {
             .arg("none")
             .arg("--lib")
             .arg(&chip_dir)
-            .capture_outputs(true, "cargo init", None, None, &[])
+            .run_and_capture_outputs(true, "cargo init", None, None, &[])
             .with_context(|| "Failed to cargo init")?;
 
-        self.prepare_chip_test_toml(&chip_dir, command)?;
+        self.prepare_chip_test_toml(&chip_dir, passthrough_opts)?;
         let chip_svd = self.prepare_svd_file(&chip_dir)?;
         self.prepare_rust_toolchain_file(&chip_dir)?;
 
@@ -226,7 +299,7 @@ impl TestCase {
             &chip_dir,
             &lib_rs_file,
             &svd2rust_err_file,
-            command,
+            passthrough_opts,
         )?;
         process_stderr_paths.push(svd2rust_err_file);
         match self.arch {
@@ -262,7 +335,7 @@ impl TestCase {
                 .arg(&new_lib_rs_file)
                 .arg("--outdir")
                 .arg(&src_dir)
-                .capture_outputs(
+                .run_and_capture_outputs(
                     true,
                     "form",
                     None,
@@ -291,7 +364,7 @@ impl TestCase {
                 Command::new(rustfmt_bin_path)
                     .arg(entry)
                     .args(["--edition", "2021"])
-                    .capture_outputs(
+                    .run_and_capture_outputs(
                         false,
                         "rustfmt",
                         None,
@@ -417,7 +490,7 @@ impl TestCase {
         if let Some(opts) = self.opts.as_ref() {
             base_cmd.args(opts);
         }
-        base_cmd.current_dir(chip_dir).capture_outputs(
+        base_cmd.current_dir(chip_dir).run_and_capture_outputs(
             true,
             "svd2rust",
             Some(lib_rs_file).filter(|_| {

Original file line number	Diff line number	Diff line change
`@@ -148,7 +148,7 @@ pub fn get_release_binary_artifact(`
`148`	`148`	`Command::new("gzip")`
`149`	`149`	`.arg("-d")`
`150`	`150`	`.arg(output_dir.join(artifact))`
`151`		`- .get_output(false)?;`
	`151`	`+ .run_and_get_output(false)?;`
`152`	`152`	`}`
`153`	`153`	`}`
`154`	`154`	`_ => {`