From 0bc6ab46a45d2ac20eab386a45b79b4dad027e1a Mon Sep 17 00:00:00 2001 From: Tomasz Nowak Date: Sat, 4 Feb 2023 19:40:00 +0100 Subject: [PATCH 1/4] Changed some load_rustdoc arguments to a struct --- src/baseline.rs | 88 +++++++++++++++++++++++++++++++++---------------- src/main.rs | 23 +++++++++---- 2 files changed, 76 insertions(+), 35 deletions(-) diff --git a/src/baseline.rs b/src/baseline.rs index b3cc5809..2bc50f3d 100644 --- a/src/baseline.rs +++ b/src/baseline.rs @@ -33,14 +33,8 @@ impl<'a> CrateSource<'a> { /// fn regular_features(&self) -> Vec { match self { - Self::Registry { crate_ } => crate_.features().keys().cloned().into_iter().collect(), - Self::ManifestPath { manifest } => manifest - .parsed - .features - .keys() - .cloned() - .into_iter() - .collect(), + Self::Registry { crate_ } => crate_.features().keys().cloned().collect(), + Self::ManifestPath { manifest } => manifest.parsed.features.keys().cloned().collect(), } } @@ -182,11 +176,39 @@ fn save_placeholder_rustdoc_manifest( Ok(placeholder_manifest_path) } +#[allow(dead_code)] +pub(crate) enum CrateType<'a> { + Current, + Baseline { + /// When the baseline is being generated from registry + /// and no specific version was chosen, we want to select a version + /// that is the same or older than the version of the current crate. + highest_allowed_version: Option<&'a semver::Version>, + }, +} + +pub(crate) struct CrateDataForRustdoc<'a> { + pub(crate) crate_type: CrateType<'a>, + pub(crate) name: &'a str, + // TODO: pass an enum describing which features to enable +} + +impl<'a> ToString for CrateType<'a> { + fn to_string(&self) -> String { + match self { + CrateType::Current => "current", + CrateType::Baseline { .. } => "baseline", + } + .to_string() + } +} + fn generate_rustdoc( config: &mut GlobalConfig, rustdoc: &RustDocCommand, target_root: PathBuf, crate_source: CrateSource, + crate_data: CrateDataForRustdoc, ) -> anyhow::Result { let name = crate_source.name()?; let version = crate_source.version()?; @@ -214,7 +236,10 @@ fn generate_rustdoc( if cached_rustdoc.exists() { config.shell_status( "Parsing", - format_args!("{name} v{version} (baseline, cached)"), + format_args!( + "{name} v{version} ({}, cached)", + crate_data.crate_type.to_string() + ), )?; // TODO: replace "baseline" with a string passed as a function argument // (the plan is to make this function work for both baseline and current). @@ -232,7 +257,10 @@ fn generate_rustdoc( save_placeholder_rustdoc_manifest(build_dir.as_path(), placeholder_manifest) .context("failed to save placeholder rustdoc manifest")?; - config.shell_status("Parsing", format_args!("{name} v{version} (baseline)"))?; + config.shell_status( + "Parsing", + format_args!("{name} v{version} ({})", crate_data.crate_type.to_string()), + )?; // TODO: replace "baseline" with a string passed as a function argument // (the plan is to make this function work for both baseline and current). @@ -270,8 +298,7 @@ pub(crate) trait BaselineLoader { &self, config: &mut GlobalConfig, rustdoc: &RustDocCommand, - name: &str, - version_current: Option<&semver::Version>, + crate_data: CrateDataForRustdoc, ) -> anyhow::Result; } @@ -290,8 +317,7 @@ impl BaselineLoader for RustdocBaseline { &self, _config: &mut GlobalConfig, _rustdoc: &RustDocCommand, - _name: &str, - _version_current: Option<&semver::Version>, + _crate_data: CrateDataForRustdoc, ) -> anyhow::Result { Ok(self.path.clone()) } @@ -335,13 +361,12 @@ impl BaselineLoader for PathBaseline { &self, config: &mut GlobalConfig, rustdoc: &RustDocCommand, - name: &str, - _version_current: Option<&semver::Version>, + crate_data: CrateDataForRustdoc, ) -> anyhow::Result { - let manifest: &Manifest = self.lookup.get(name).with_context(|| { + let manifest: &Manifest = self.lookup.get(crate_data.name).with_context(|| { format!( "package `{}` not found in {}", - name, + crate_data.name, self.project_root.display() ) })?; @@ -350,6 +375,7 @@ impl BaselineLoader for PathBaseline { rustdoc, self.target_root.clone(), CrateSource::ManifestPath { manifest }, + crate_data, ) } } @@ -419,11 +445,9 @@ impl BaselineLoader for GitBaseline { &self, config: &mut GlobalConfig, rustdoc: &RustDocCommand, - name: &str, - version_current: Option<&semver::Version>, + crate_data: CrateDataForRustdoc, ) -> anyhow::Result { - self.path - .load_rustdoc(config, rustdoc, name, version_current) + self.path.load_rustdoc(config, rustdoc, crate_data) } } @@ -522,18 +546,25 @@ impl BaselineLoader for RegistryBaseline { &self, config: &mut GlobalConfig, rustdoc: &RustDocCommand, - name: &str, - version_current: Option<&semver::Version>, + crate_data: CrateDataForRustdoc, ) -> anyhow::Result { let crate_ = self .index - .crate_(name) - .with_context(|| anyhow::format_err!("{} not found in registry", name))?; + .crate_(crate_data.name) + .with_context(|| anyhow::format_err!("{} not found in registry", crate_data.name))?; let base_version = if let Some(base) = self.version.as_ref() { base.to_string() } else { - choose_baseline_version(&crate_, version_current)? + choose_baseline_version( + &crate_, + match crate_data.crate_type { + CrateType::Current => None, + CrateType::Baseline { + highest_allowed_version, + } => highest_allowed_version, + }, + )? }; let crate_ = crate_ @@ -543,7 +574,7 @@ impl BaselineLoader for RegistryBaseline { .with_context(|| { anyhow::format_err!( "Version {} of crate {} not found in registry", - name, + crate_data.name, base_version ) })?; @@ -553,6 +584,7 @@ impl BaselineLoader for RegistryBaseline { rustdoc, self.target_root.clone(), CrateSource::Registry { crate_ }, + crate_data, ) } } diff --git a/src/main.rs b/src/main.rs index e12933e9..7e247a76 100644 --- a/src/main.rs +++ b/src/main.rs @@ -131,14 +131,14 @@ fn main() -> anyhow::Result<()> { args.current_rustdoc.as_deref() { let name = ""; - let version = None; + let baseline_highest_allowed_version = None; let (current_crate, baseline_crate) = generate_versioned_crates( &mut config, CurrentCratePath::CurrentRustdocPath(current_rustdoc_path), &*loader, &rustdoc_cmd, name, - version, + baseline_highest_allowed_version, )?; let success = run_check_release(&mut config, name, current_crate, baseline_crate)?; @@ -151,21 +151,21 @@ fn main() -> anyhow::Result<()> { .map(|selected| { let manifest_path = selected.manifest_path.as_std_path(); let crate_name = &selected.name; - let version = &selected.version; + let current_version = &selected.version; let is_implied = args.workspace.all || args.workspace.workspace; if is_implied && selected.publish == Some(vec![]) { config.verbose(|config| { config.shell_status( "Skipping", - format_args!("{crate_name} v{version} (current)"), + format_args!("{crate_name} v{current_version} (current)"), ) })?; Ok(true) } else { config.shell_status( "Parsing", - format_args!("{crate_name} v{version} (current)"), + format_args!("{crate_name} v{current_version} (current)"), )?; let (current_crate, baseline_crate) = generate_versioned_crates( @@ -174,7 +174,7 @@ fn main() -> anyhow::Result<()> { &*loader, &rustdoc_cmd, crate_name, - Some(version), + Some(current_version), )?; Ok(run_check_release( @@ -230,7 +230,16 @@ fn generate_versioned_crates( // For example, this happens when target-dir is specified in `.cargo/config.toml`. // That's the reason why we're immediately loading the rustdocs into memory. // See: https://github.com/obi1kenobi/cargo-semver-checks/issues/269 - let baseline_path = loader.load_rustdoc(config, rustdoc_cmd, crate_name, version)?; + let baseline_path = loader.load_rustdoc( + config, + rustdoc_cmd, + baseline::CrateDataForRustdoc { + name: crate_name, + crate_type: baseline::CrateType::Baseline { + highest_allowed_version: version, + }, + }, + )?; let baseline_crate = load_rustdoc(&baseline_path)?; Ok((current_crate, baseline_crate)) From d175abc68d5009348bf7b99a07a69c1d258775f5 Mon Sep 17 00:00:00 2001 From: Tomasz Nowak Date: Sat, 4 Feb 2023 21:16:21 +0100 Subject: [PATCH 2/4] Renamed baseline.rs and dump.rs files --- src/main.rs | 26 +++++++++++++------------- src/{dump.rs => rustdoc_cmd.rs} | 16 ++++++++-------- src/{baseline.rs => rustdoc_gen.rs} | 14 +++++++------- 3 files changed, 28 insertions(+), 28 deletions(-) rename src/{dump.rs => rustdoc_cmd.rs} (96%) rename src/{baseline.rs => rustdoc_gen.rs} (99%) diff --git a/src/main.rs b/src/main.rs index 7e247a76..ef037b44 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,16 +1,16 @@ #![forbid(unsafe_code)] -mod baseline; mod check_release; mod config; -mod dump; mod manifest; mod query; +mod rustdoc_cmd; +mod rustdoc_gen; mod templating; mod util; -use dump::RustDocCommand; use itertools::Itertools; +use rustdoc_cmd::RustdocCommand; use semver::Version; use std::path::{Path, PathBuf}; @@ -91,13 +91,13 @@ fn main() -> anyhow::Result<()> { Some(SemverChecksCommands::CheckRelease(args)) => { let mut config = GlobalConfig::new().set_level(args.verbosity.log_level()); - let loader: Box = + let loader: Box = if let Some(path) = args.baseline_rustdoc.as_deref() { - Box::new(baseline::RustdocBaseline::new(path.to_owned())) + Box::new(rustdoc_gen::RustdocBaseline::new(path.to_owned())) } else if let Some(root) = args.baseline_root.as_deref() { let metadata = args.manifest.metadata().no_deps().exec()?; let target = metadata.target_directory.as_std_path().join(util::SCOPE); - Box::new(baseline::PathBaseline::new(root, &target)?) + Box::new(rustdoc_gen::PathBaseline::new(root, &target)?) } else if let Some(rev) = args.baseline_rev.as_deref() { let metadata = args.manifest.metadata().no_deps().exec()?; let source = metadata.workspace_root.as_std_path(); @@ -107,7 +107,7 @@ fn main() -> anyhow::Result<()> { .as_std_path() .join(util::SCOPE) .join(format!("git-{slug}")); - Box::new(baseline::GitBaseline::with_rev( + Box::new(rustdoc_gen::GitBaseline::with_rev( source, &target, rev, @@ -116,14 +116,14 @@ fn main() -> anyhow::Result<()> { } else { let metadata = args.manifest.metadata().no_deps().exec()?; let target = metadata.target_directory.as_std_path().join(util::SCOPE); - let mut registry = baseline::RegistryBaseline::new(&target, &mut config)?; + let mut registry = rustdoc_gen::RegistryBaseline::new(&target, &mut config)?; if let Some(version) = args.baseline_version.as_deref() { let version = semver::Version::parse(version)?; registry.set_version(version); } Box::new(registry) }; - let rustdoc_cmd = dump::RustDocCommand::new() + let rustdoc_cmd = rustdoc_cmd::RustdocCommand::new() .deps(false) .silence(!config.is_verbose()); @@ -212,8 +212,8 @@ enum CurrentCratePath<'a> { fn generate_versioned_crates( config: &mut GlobalConfig, current_crate_path: CurrentCratePath, - loader: &dyn baseline::BaselineLoader, - rustdoc_cmd: &RustDocCommand, + loader: &dyn rustdoc_gen::BaselineLoader, + rustdoc_cmd: &RustdocCommand, crate_name: &str, version: Option<&Version>, ) -> anyhow::Result<(VersionedCrate, VersionedCrate)> { @@ -233,9 +233,9 @@ fn generate_versioned_crates( let baseline_path = loader.load_rustdoc( config, rustdoc_cmd, - baseline::CrateDataForRustdoc { + rustdoc_gen::CrateDataForRustdoc { name: crate_name, - crate_type: baseline::CrateType::Baseline { + crate_type: rustdoc_gen::CrateType::Baseline { highest_allowed_version: version, }, }, diff --git a/src/dump.rs b/src/rustdoc_cmd.rs similarity index 96% rename from src/dump.rs rename to src/rustdoc_cmd.rs index 67a6cbfd..a18776bd 100644 --- a/src/dump.rs +++ b/src/rustdoc_cmd.rs @@ -1,10 +1,10 @@ #[derive(Clone, Debug, PartialEq, Eq)] -pub struct RustDocCommand { +pub struct RustdocCommand { deps: bool, silence: bool, } -impl RustDocCommand { +impl RustdocCommand { pub fn new() -> Self { Self { deps: false, @@ -152,7 +152,7 @@ impl RustDocCommand { } } -impl Default for RustDocCommand { +impl Default for RustdocCommand { fn default() -> Self { Self::new() } @@ -162,11 +162,11 @@ impl Default for RustDocCommand { mod tests { use std::path::Path; - use super::RustDocCommand; + use super::RustdocCommand; #[test] fn rustdoc_for_lib_crate_without_lib_section() { - RustDocCommand::default() + RustdocCommand::default() .dump( Path::new("./test_rustdoc/implicit_lib/Cargo.toml"), None, @@ -177,7 +177,7 @@ mod tests { #[test] fn rustdoc_for_lib_crate_with_lib_section() { - RustDocCommand::default() + RustdocCommand::default() .dump( Path::new("./test_rustdoc/renamed_lib/Cargo.toml"), None, @@ -188,7 +188,7 @@ mod tests { #[test] fn rustdoc_for_bin_crate_without_bin_section() { - RustDocCommand::default() + RustdocCommand::default() .dump( Path::new("./test_rustdoc/implicit_bin/Cargo.toml"), None, @@ -199,7 +199,7 @@ mod tests { #[test] fn rustdoc_for_bin_crate_with_bin_section() { - RustDocCommand::default() + RustdocCommand::default() .dump( Path::new("./test_rustdoc/renamed_bin/Cargo.toml"), None, diff --git a/src/baseline.rs b/src/rustdoc_gen.rs similarity index 99% rename from src/baseline.rs rename to src/rustdoc_gen.rs index 2bc50f3d..145108fe 100644 --- a/src/baseline.rs +++ b/src/rustdoc_gen.rs @@ -3,8 +3,8 @@ use std::path::{Path, PathBuf}; use anyhow::Context as _; use crates_index::Crate; -use crate::dump::RustDocCommand; use crate::manifest::Manifest; +use crate::rustdoc_cmd::RustdocCommand; use crate::util::slugify; use crate::GlobalConfig; @@ -205,7 +205,7 @@ impl<'a> ToString for CrateType<'a> { fn generate_rustdoc( config: &mut GlobalConfig, - rustdoc: &RustDocCommand, + rustdoc: &RustdocCommand, target_root: PathBuf, crate_source: CrateSource, crate_data: CrateDataForRustdoc, @@ -297,7 +297,7 @@ pub(crate) trait BaselineLoader { fn load_rustdoc( &self, config: &mut GlobalConfig, - rustdoc: &RustDocCommand, + rustdoc: &RustdocCommand, crate_data: CrateDataForRustdoc, ) -> anyhow::Result; } @@ -316,7 +316,7 @@ impl BaselineLoader for RustdocBaseline { fn load_rustdoc( &self, _config: &mut GlobalConfig, - _rustdoc: &RustDocCommand, + _rustdoc: &RustdocCommand, _crate_data: CrateDataForRustdoc, ) -> anyhow::Result { Ok(self.path.clone()) @@ -360,7 +360,7 @@ impl BaselineLoader for PathBaseline { fn load_rustdoc( &self, config: &mut GlobalConfig, - rustdoc: &RustDocCommand, + rustdoc: &RustdocCommand, crate_data: CrateDataForRustdoc, ) -> anyhow::Result { let manifest: &Manifest = self.lookup.get(crate_data.name).with_context(|| { @@ -444,7 +444,7 @@ impl BaselineLoader for GitBaseline { fn load_rustdoc( &self, config: &mut GlobalConfig, - rustdoc: &RustDocCommand, + rustdoc: &RustdocCommand, crate_data: CrateDataForRustdoc, ) -> anyhow::Result { self.path.load_rustdoc(config, rustdoc, crate_data) @@ -545,7 +545,7 @@ impl BaselineLoader for RegistryBaseline { fn load_rustdoc( &self, config: &mut GlobalConfig, - rustdoc: &RustDocCommand, + rustdoc: &RustdocCommand, crate_data: CrateDataForRustdoc, ) -> anyhow::Result { let crate_ = self From 9142425f9314a4bcccd1d28b19a484c5eefdbf9f Mon Sep 17 00:00:00 2001 From: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> Date: Sat, 4 Feb 2023 22:14:56 -0500 Subject: [PATCH 3/4] Apply suggestions from code review --- src/rustdoc_gen.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/rustdoc_gen.rs b/src/rustdoc_gen.rs index 145108fe..154882c2 100644 --- a/src/rustdoc_gen.rs +++ b/src/rustdoc_gen.rs @@ -177,6 +177,7 @@ fn save_placeholder_rustdoc_manifest( } #[allow(dead_code)] +#[derive(Debug, Clone)] pub(crate) enum CrateType<'a> { Current, Baseline { @@ -187,19 +188,19 @@ pub(crate) enum CrateType<'a> { }, } +#[derive(Debug, Clone)] pub(crate) struct CrateDataForRustdoc<'a> { pub(crate) crate_type: CrateType<'a>, pub(crate) name: &'a str, // TODO: pass an enum describing which features to enable } -impl<'a> ToString for CrateType<'a> { - fn to_string(&self) -> String { +impl<'a> CrateType<'a> { + fn type_name(&self) -> &'static str { match self { CrateType::Current => "current", CrateType::Baseline { .. } => "baseline", } - .to_string() } } @@ -238,11 +239,9 @@ fn generate_rustdoc( "Parsing", format_args!( "{name} v{version} ({}, cached)", - crate_data.crate_type.to_string() + crate_data.crate_type.type_name() ), )?; - // TODO: replace "baseline" with a string passed as a function argument - // (the plan is to make this function work for both baseline and current). return Ok(cached_rustdoc); } From cf8f1479e523ddfb4061f2cb8e56e40b744f43d8 Mon Sep 17 00:00:00 2001 From: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> Date: Sat, 4 Feb 2023 22:18:04 -0500 Subject: [PATCH 4/4] Update src/rustdoc_gen.rs --- src/rustdoc_gen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rustdoc_gen.rs b/src/rustdoc_gen.rs index 154882c2..7f34a592 100644 --- a/src/rustdoc_gen.rs +++ b/src/rustdoc_gen.rs @@ -258,7 +258,7 @@ fn generate_rustdoc( config.shell_status( "Parsing", - format_args!("{name} v{version} ({})", crate_data.crate_type.to_string()), + format_args!("{name} v{version} ({})", crate_data.crate_type.type_name()), )?; // TODO: replace "baseline" with a string passed as a function argument // (the plan is to make this function work for both baseline and current).