Skip to content

Commit

Permalink
Merge pull request rust-marker#246 from Veetaha/feat/reorganize-cli-s…
Browse files Browse the repository at this point in the history
…tructure

Reorganize the CLI to a consistent module structure
  • Loading branch information
xFrednet authored Sep 18, 2023
2 parents cfe7199 + 072854c commit 60e6a2c
Show file tree
Hide file tree
Showing 8 changed files with 216 additions and 220 deletions.
36 changes: 12 additions & 24 deletions cargo-marker/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@ use crate::config::LintDependencyEntry;
use crate::error::prelude::*;
use crate::observability::display::{self, print_stage};
use crate::observability::prelude::*;
use std::{
collections::HashMap,
ffi::{OsStr, OsString},
};

use camino::Utf8PathBuf;
use itertools::Itertools;
use std::collections::BTreeMap;

pub mod cargo;
pub mod driver;
Expand All @@ -34,7 +31,7 @@ pub struct Config {
/// a lint for uitests.
pub marker_dir: Utf8PathBuf,
/// The list of lints.
pub lints: HashMap<String, LintDependencyEntry>,
pub lints: BTreeMap<String, LintDependencyEntry>,
/// Additional flags, which should be passed to rustc during the compilation
/// of crates.
pub build_rustc_flags: String,
Expand All @@ -47,7 +44,7 @@ impl Config {
pub fn try_base_from(toolchain: Toolchain) -> Result<Self> {
Ok(Self {
marker_dir: toolchain.find_target_dir()?.join("marker"),
lints: HashMap::default(),
lints: BTreeMap::default(),
build_rustc_flags: String::new(),
debug_build: false,
toolchain,
Expand All @@ -64,18 +61,22 @@ impl Config {
}

/// This struct contains all information to use rustc as a driver.
#[derive(Debug)]
pub struct CheckInfo {
pub env: Vec<(&'static str, OsString)>,
pub env: Vec<(&'static str, String)>,
}

pub fn prepare_check(config: &Config) -> Result<CheckInfo> {
print_stage("compiling lints");
let lints = lints::build_lints(config)?;
let lints = lints::build_lints(config)?
.iter()
.map(|LintCrate { name, file }| format!("{name}:{file}"))
.join(";");

#[rustfmt::skip]
let mut env = vec![
("RUSTC_WORKSPACE_WRAPPER", config.toolchain.driver_path.as_os_str().to_os_string()),
("MARKER_LINT_CRATES", to_marker_lint_crates_env(&lints)),
("RUSTC_WORKSPACE_WRAPPER", config.toolchain.driver_path.clone().into_string()),
("MARKER_LINT_CRATES", lints),
];
if let Some(toolchain) = &config.toolchain.cargo.toolchain {
env.push(("RUSTUP_TOOLCHAIN", toolchain.into()));
Expand Down Expand Up @@ -107,16 +108,3 @@ pub fn run_check(config: &Config, info: CheckInfo, additional_cargo_args: &[Stri

Err(Error::root(format!("{} finished with an error", display::stage(stage))))
}

pub fn to_marker_lint_crates_env(lints: &[LintCrate]) -> OsString {
let lint_paths: Vec<_> = lints
.iter()
.map(|krate| {
let mut env_str = OsString::from(&krate.name);
env_str.push(":");
env_str.push(krate.file.as_os_str());
env_str
})
.collect();
lint_paths.join(OsStr::new(";"))
}
4 changes: 2 additions & 2 deletions cargo-marker/src/backend/lints/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::observability::prelude::*;
use crate::{backend::Config, config::LintDependencyEntry};
use camino::{Utf8Path, Utf8PathBuf};
use cargo_metadata::Metadata;
use std::collections::HashMap;
use std::collections::BTreeMap;

/// This function fetches and locates all lint crates specified in the given
/// configuration.
Expand All @@ -39,7 +39,7 @@ fn setup_dummy_crate(config: &Config) -> Result<Utf8PathBuf> {
/// A small hack, to have the lints namespaced under the `[dependencies]` section
#[derive(serde::Serialize)]
struct DepNamespace<'a> {
dependencies: &'a HashMap<String, LintDependencyEntry>,
dependencies: &'a BTreeMap<String, LintDependencyEntry>,
}

// Manifest
Expand Down
102 changes: 38 additions & 64 deletions cargo-marker/src/cli.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use crate::config::{Config, LintDependency};
mod check;
mod setup;
mod test_setup;

use crate::config::Config;
use crate::error::prelude::*;
use camino::Utf8Path;
use clap::{Args, Parser, Subcommand};
use std::collections::HashMap;
use clap::{Parser, Subcommand};

/// Marker's CLI interface
///
Expand All @@ -11,7 +13,7 @@ use std::collections::HashMap;
#[derive(Parser, Debug)]
#[command(name = "cargo")]
#[command(author, version, about)]
struct MarkerApp {
struct CargoPlugin {
#[clap(subcommand)]
subcommand: MarkerSubcommand,
}
Expand All @@ -24,76 +26,48 @@ enum MarkerSubcommand {
#[derive(Parser, Debug)]
#[command(author, version, about)]
#[command(override_usage = "cargo marker [OPTIONS] [COMMAND] -- <CARGO ARGS>")]
pub struct MarkerCli {
pub(crate) struct MarkerCli {
#[command(subcommand)]
pub command: Option<CliCommand>,
pub(crate) command: Option<CliCommand>,

/// Used as the arguments to run Marker, when no command was specified
#[clap(flatten)]
pub check_args: CheckArgs,
}

impl MarkerCli {
/// Prefer using this over the normal `parse` method, to split the arguments
pub fn parse_args() -> Self {
let MarkerSubcommand::Marker(cli) = MarkerApp::parse().subcommand;
cli
}
pub(crate) check: check::CheckCommand,
}

#[derive(Subcommand, Debug)]
pub enum CliCommand {
pub(crate) enum CliCommand {
/// Run Marker on the current package
Check(CheckArgs),
Check(check::CheckCommand),

/// Setup the rustc driver for Marker
Setup(SetupArgs),
Setup(setup::SetupCommand),

/// **UNSTABLE** Setup the specified lint crate for ui tests
#[command(hide = true)]
TestSetup(CheckArgs),
TestSetup(test_setup::TestSetupCommand),
}

#[derive(Args, Debug)]
#[command(override_usage = "cargo marker check [OPTIONS] -- <CARGO ARGS>")]
pub struct CheckArgs {
/// Specifies lint crates which should be used. (Lints in `Cargo.toml` will be ignored)
#[arg(short, long)]
pub lints: Vec<String>,
/// Forwards the current `RUSTFLAGS` value during driver and lint crate compilation
#[arg(long)]
pub forward_rust_flags: bool,

/// Arguments which will be forwarded to Cargo. See `cargo check --help`
#[clap(last = true)]
pub cargo_args: Vec<String>,
}

#[derive(Args, Debug)]
pub struct SetupArgs {
/// Automatically installs the required toolchain using rustup
#[arg(long)]
pub auto_install_toolchain: bool,
/// Forwards the current `RUSTFLAGS` value during driver and lint crate compilation
#[arg(long)]
pub forward_rust_flags: bool,
}

pub fn collect_lint_deps(args: &CheckArgs) -> Result<Option<HashMap<String, LintDependency>>> {
if args.lints.is_empty() {
return Ok(None);
impl MarkerCli {
/// Prefer using this over the normal `parse` method, to split the arguments
pub(crate) fn parse_args() -> Self {
let MarkerSubcommand::Marker(cli) = CargoPlugin::parse().subcommand;
cli
}

let mut virtual_manifest = "[workspace.metadata.marker.lints]\n".to_string();
for dep in &args.lints {
virtual_manifest.push_str(dep);
virtual_manifest.push('\n');
pub(crate) fn run(self) -> Result {
let manifest_path = crate::backend::cargo::Cargo::default().cargo_locate_project()?;
let config = Config::try_from_manifest(&manifest_path)?;

let Some(command) = self.command else {
return self.check.run(config);
};
match command {
CliCommand::Setup(cmd) => cmd.run(),
CliCommand::Check(cmd) => cmd.run(config),
CliCommand::TestSetup(cmd) => cmd.run(config),
}
}

let path = Utf8Path::new(".");

let Config { lints } = Config::try_from_str(&virtual_manifest, path)?.unwrap_or_else(|| {
panic!("BUG: the config must definitely contain the marker metadata:\n---\n{virtual_manifest}\n---");
});
Ok(Some(lints))
}

#[cfg(test)]
Expand All @@ -112,16 +86,16 @@ mod tests {

let cli = MarkerCli::parse_from(["cargo-marker"]);
assert!(cli.command.is_none());
assert!(cli.check_args.cargo_args.is_empty());
assert!(cli.check.cargo_args.is_empty());

let cli = MarkerCli::parse_from(["cargo-marker", "--", "ducks", "penguins"]);
assert!(cli.command.is_none());
assert!(cli.check_args.cargo_args.len() == 2);
assert!(cli.check_args.cargo_args[0] == "ducks");
assert!(cli.check_args.cargo_args[1] == "penguins");
assert!(cli.check.cargo_args.len() == 2);
assert!(cli.check.cargo_args[0] == "ducks");
assert!(cli.check.cargo_args[1] == "penguins");

let cli = MarkerCli::parse_from(["cargo-marker", "check", "--", "ducks", "penguins"]);
assert!(cli.check_args.cargo_args.is_empty());
assert!(cli.check.cargo_args.is_empty());
if let Some(CliCommand::Check(check_args)) = cli.command {
assert!(check_args.cargo_args.len() == 2);
assert!(check_args.cargo_args[0] == "ducks");
Expand Down
102 changes: 102 additions & 0 deletions cargo-marker/src/cli/check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
use crate::config::{Config, LintDependency};
use crate::error::prelude::*;
use crate::{backend, utils};
use camino::Utf8Path;
use clap::Args;
use std::collections::BTreeMap;

#[derive(Args, Debug)]
#[command(override_usage = "cargo marker check [OPTIONS] -- <CARGO ARGS>")]
pub(crate) struct CheckCommand {
/// Specifies lint crates which should be used. (Lints in `Cargo.toml` will be ignored)
#[arg(short, long)]
pub(crate) lints: Vec<String>,

/// Forward the current `RUSTFLAGS` value during the lint crate compilation
#[arg(long)]
pub(crate) forward_rust_flags: bool,

/// Arguments which will be forwarded to Cargo. See `cargo check --help`
#[clap(last = true)]
pub(crate) cargo_args: Vec<String>,
}

impl CheckCommand {
pub(crate) fn run(self, config: Option<Config>) -> Result {
self.compile_lints(config)?.lint()
}

pub(crate) fn compile_lints(self, config: Option<Config>) -> Result<CompiledLints> {
// determine lints
let lints: BTreeMap<_, _> = self
.lints_from_cli()?
.or_else(|| config.map(|config| config.lints))
.into_iter()
.flatten()
.map(|(name, dep)| (name, dep.into_dep_entry()))
.collect();

// Validation
if lints.is_empty() {
return Err(Error::from_kind(ErrorKind::LintsNotFound));
}

// If this is a dev build, we want to rebuild the driver before checking
if utils::is_local_driver() {
backend::driver::install_driver(false, None)?;
}

// Configure backend
let toolchain = backend::toolchain::Toolchain::try_find_toolchain()?;
let backend_conf = backend::Config {
lints,
..backend::Config::try_base_from(toolchain)?
};

// Prepare backend
let info = backend::prepare_check(&backend_conf)?;

Ok(CompiledLints {
backend_conf,
info,
cargo_args: self.cargo_args,
})
}

fn lints_from_cli(&self) -> Result<Option<BTreeMap<String, LintDependency>>> {
if self.lints.is_empty() {
return Ok(None);
}

let mut virtual_manifest = "[workspace.metadata.marker.lints]\n".to_string();
for dep in &self.lints {
virtual_manifest.push_str(dep);
virtual_manifest.push('\n');
}

let path = Utf8Path::new(".");

let Config { lints } = Config::try_from_str(&virtual_manifest, path)?.unwrap_or_else(|| {
panic!(
"BUG: the config must definitely contain the marker metadata:\
\n---\n{virtual_manifest}\n---"
);
});

Ok(Some(lints))
}
}

/// The result of discovering and compiling the lint libraries
#[derive(Debug)]
pub(crate) struct CompiledLints {
pub(crate) backend_conf: backend::Config,
pub(crate) info: backend::CheckInfo,
pub(crate) cargo_args: Vec<String>,
}

impl CompiledLints {
fn lint(self) -> Result {
backend::run_check(&self.backend_conf, self.info, &self.cargo_args)
}
}
24 changes: 24 additions & 0 deletions cargo-marker/src/cli/setup.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
use crate::error::prelude::*;
use clap::Args;

#[derive(Args, Debug)]
pub struct SetupCommand {
/// Automatically installs the required toolchain using rustup
#[arg(long)]
pub auto_install_toolchain: bool,

/// Forward the current `RUSTFLAGS` value during the driver compilation
#[arg(long)]
pub forward_rust_flags: bool,
}

impl SetupCommand {
pub(crate) fn run(self) -> Result {
let rustc_flags = self
.forward_rust_flags
.then(|| std::env::var("RUSTFLAGS").ok())
.flatten();

crate::backend::driver::install_driver(self.auto_install_toolchain, rustc_flags)
}
}
Loading

0 comments on commit 60e6a2c

Please sign in to comment.