Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flag to get current rustdoc from registry #273

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 58 additions & 45 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
#![forbid(unsafe_code)]

mod baseline;
mod check_release;
mod config;
mod dump;
mod manifest;
mod query;
mod rustdoc_generation;
mod templating;
mod util;

use std::path::PathBuf;

use clap::{Args, Parser, Subcommand};
use trustfall_rustdoc::load_rustdoc;
use trustfall_rustdoc::{load_rustdoc, VersionedCrate};

use crate::{check_release::run_check_release, config::GlobalConfig, util::slugify};
use crate::{config::GlobalConfig, util::slugify};

fn main() -> anyhow::Result<()> {
human_panic::setup_panic!();
Expand Down Expand Up @@ -88,53 +88,70 @@ fn main() -> anyhow::Result<()> {
Some(SemverChecksCommands::CheckRelease(args)) => {
let mut config = GlobalConfig::new().set_level(args.verbosity.log_level());

let loader: Box<dyn baseline::BaselineLoader> =
let metadata = args.manifest.metadata().no_deps().exec()?;
let target_path = metadata.target_directory.as_std_path().join(util::SCOPE);
let source_path = metadata.workspace_root.as_std_path();
Comment on lines +91 to +93
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason to move these out of their original block and make them always get executed? I'm confused.


let baseline_rustdoc_generator: Box<dyn rustdoc_generation::RustdocGenerator> =
if let Some(path) = args.baseline_rustdoc.as_deref() {
Box::new(baseline::RustdocBaseline::new(path.to_owned()))
Box::new(rustdoc_generation::FromRustdoc::new(path.to_owned()))
} else if let Some(root) = args.baseline_root.as_deref() {
Box::new(baseline::PathBaseline::new(root)?)
Box::new(rustdoc_generation::FromPath::new(root)?)
} 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();
let slug = slugify(rev);
let target = metadata
.target_directory
.as_std_path()
.join(util::SCOPE)
.join(format!("git-{slug}"));
Box::new(baseline::GitBaseline::with_rev(
source,
&target,
Box::new(rustdoc_generation::FromGitRevision::with_rev(
source_path,
&target_path.join(format!("git-{}", slugify(rev))),
rev,
&mut config,
)?)
} 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_generation::FromRegistry::new(&target_path, &mut config)?;
if let Some(version) = args.baseline_version.as_deref() {
let version = semver::Version::parse(version)?;
registry.set_version(version);
registry.set_exact_version(version);
}
Box::new(registry)
};
let rustdoc_cmd = dump::RustDocCommand::new()
.deps(false)
.silence(!config.is_verbose());

let rustdoc_paths = if let Some(current_rustdoc_path) = args.current_rustdoc.as_deref()
{
let mut success = true;
let mut run_check_release = |config: &mut GlobalConfig,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very much not a fan of complex inline function definitions like this. It is unidiomatic and makes the code difficult to read. The reader has to jump back and forth between the uses and the definition of the closure, which also has complex arguments and touches external state, while not even including a doc comment.

Please try to find another way to structure the code with readability and ease of understanding in mind.

crate_name: &str,
current_crate: VersionedCrate,
baseline_crate: VersionedCrate|
-> anyhow::Result<()> {
if !check_release::run_check_release(
config,
crate_name,
current_crate,
baseline_crate,
)? {
success = false;
}
Ok(())
};

if let Some(current_rustdoc_path) = args.current_rustdoc.as_deref() {
let current_crate = load_rustdoc(current_rustdoc_path)?;

let name = "<unknown>";
let version = None;
vec![(
name.to_owned(),
loader.load_rustdoc(&mut config, &rustdoc_cmd, name, version)?,
current_rustdoc_path.to_owned(),
)]
let baseline_path = baseline_rustdoc_generator.generate_rustdoc(
&mut config,
&rustdoc_cmd,
name,
version,
"baseline",
)?;
let baseline_crate = load_rustdoc(&baseline_path)?;

run_check_release(&mut config, name, current_crate, baseline_crate)?;
} else {
let metadata = args.manifest.metadata().exec()?;
let (selected, _) = args.workspace.partition_packages(&metadata);
let mut rustdoc_paths = Vec::with_capacity(selected.len());
for selected in selected {
let manifest_path = selected.manifest_path.as_std_path();
let crate_name = &selected.name;
Expand All @@ -155,31 +172,27 @@ fn main() -> anyhow::Result<()> {
"Parsing",
format_args!("{crate_name} v{version} (current)"),
)?;

let rustdoc_path = rustdoc_cmd.dump(manifest_path, None, true)?;
let baseline_path = loader.load_rustdoc(
let current_crate = load_rustdoc(&rustdoc_path)?;

// Can overwrite rustdoc of current crate.
// For example, this happens when target-dir is specified in `.cargo/config.toml`.
// That's the reason why we're immediately loading the rustdocs to memory.
let baseline_path = baseline_rustdoc_generator.generate_rustdoc(
&mut config,
&rustdoc_cmd,
crate_name,
Some(version),
"baseline",
)?;
rustdoc_paths.push((crate_name.clone(), baseline_path, rustdoc_path));
}
rustdoc_paths
};
let mut success = true;
for (crate_name, baseline_path, current_path) in rustdoc_paths {
let baseline_crate = load_rustdoc(&baseline_path)?;
let current_crate = load_rustdoc(&current_path)?;
let baseline_crate = load_rustdoc(&baseline_path)?;

if !run_check_release(&mut config, &crate_name, current_crate, baseline_crate)? {
success = false;
run_check_release(&mut config, crate_name, current_crate, baseline_crate)?;
}
}
if success {
std::process::exit(0);
} else {
std::process::exit(1);
}

std::process::exit(i32::from(!success));
}
None => {
anyhow::bail!("subcommand required");
Expand Down
Loading