Skip to content

Commit

Permalink
Merge cross-rs#915
Browse files Browse the repository at this point in the history
915: more clippy r=Alexhuszagh a=Emilgardis



Co-authored-by: Emil Gardström <[email protected]>
  • Loading branch information
bors[bot] and Emilgardis authored Jul 6, 2022
2 parents 3dc0d1f + 30e4bfa commit ebd6900
Show file tree
Hide file tree
Showing 21 changed files with 264 additions and 185 deletions.
2 changes: 2 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
disallowed-methods = [
{ path = "std::path::Path::display", reason = "incorrect handling of non-Unicode paths, use path.to_utf8() or debug (`{path:?}`) instead" },
]
# needs clippy 1.61
# allow-unwrap-in-tests = true
6 changes: 6 additions & 0 deletions src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub enum Subcommand {
}

impl Subcommand {
#[must_use]
pub fn needs_docker(self, is_remote: bool) -> bool {
match self {
Subcommand::Other | Subcommand::List => false,
Expand All @@ -32,14 +33,17 @@ impl Subcommand {
}
}

#[must_use]
pub fn needs_host(self, is_remote: bool) -> bool {
self == Subcommand::Clean && is_remote
}

#[must_use]
pub fn needs_interpreter(self) -> bool {
matches!(self, Subcommand::Run | Subcommand::Test | Subcommand::Bench)
}

#[must_use]
pub fn needs_target_in_command(self) -> bool {
!matches!(self, Subcommand::Metadata)
}
Expand Down Expand Up @@ -85,6 +89,7 @@ impl CargoMetadata {
}

#[cfg(feature = "dev")]
#[must_use]
pub fn get_package(&self, package: &str) -> Option<&Package> {
self.packages.iter().find(|p| p.name == package)
}
Expand Down Expand Up @@ -112,6 +117,7 @@ impl Package {
}
}

#[must_use]
pub fn cargo_command() -> Command {
Command::new("cargo")
}
Expand Down
18 changes: 10 additions & 8 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ pub fn fmt_subcommands(stdout: &str, msg_info: &mut MessageInfo) -> Result<()> {
let (cross, host) = group_subcommands(stdout);
if !cross.is_empty() {
msg_info.print("Cross Commands:")?;
for line in cross.iter() {
for line in &cross {
msg_info.print(line)?;
}
}
if !host.is_empty() {
msg_info.print("Host Commands:")?;
for line in cross.iter() {
for line in &cross {
msg_info.print(line)?;
}
}
Expand Down Expand Up @@ -116,7 +116,7 @@ fn parse_next_arg<T>(
}

fn parse_equal_arg<T>(arg: String, out: &mut Vec<String>, parse: impl Fn(&str) -> T) -> T {
let result = parse(arg.split_once('=').unwrap().1);
let result = parse(arg.split_once('=').expect("argument should contain `=`").1);
out.push(arg);

result
Expand Down Expand Up @@ -177,7 +177,7 @@ pub fn parse(target_list: &TargetList) -> Result<Args> {
ArgKind::Equal => parse_equal_arg(arg, &mut all, parse_manifest_path),
};
} else if let ("+", ch) = arg.split_at(1) {
channel = Some(ch.to_string());
channel = Some(ch.to_owned());
} else if let Some(kind) = is_value_arg(&arg, "--target") {
target = match kind {
ArgKind::Next => {
Expand All @@ -196,7 +196,7 @@ pub fn parse(target_list: &TargetList) -> Result<Args> {
}
}
ArgKind::Equal => {
features.push(parse_equal_arg(arg, &mut all, ToOwned::to_owned))
features.push(parse_equal_arg(arg, &mut all, ToOwned::to_owned));
}
}
} else if let Some(kind) = is_value_arg(&arg, "--target-dir") {
Expand All @@ -205,11 +205,13 @@ pub fn parse(target_list: &TargetList) -> Result<Args> {
all.push(arg);
if let Some(td) = args.next() {
target_dir = Some(parse_target_dir(&td)?);
all.push("/target".to_string());
all.push("/target".to_owned());
}
}
ArgKind::Equal => {
target_dir = Some(parse_target_dir(arg.split_once('=').unwrap().1)?);
target_dir = Some(parse_target_dir(
arg.split_once('=').expect("argument should contain `=`").1,
)?);
all.push("--target-dir=/target".into());
}
}
Expand All @@ -218,7 +220,7 @@ pub fn parse(target_list: &TargetList) -> Result<Args> {
sc = Some(Subcommand::from(arg.as_ref()));
}

all.push(arg.to_string());
all.push(arg.clone());
}
}
}
Expand Down
28 changes: 14 additions & 14 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl Environment {
fn get_var(&self, name: &str) -> Option<String> {
self.1
.as_ref()
.and_then(|internal_map| internal_map.get(name).map(|v| v.to_string()))
.and_then(|internal_map| internal_map.get(name).map(|v| (*v).to_owned()))
.or_else(|| env::var(name).ok())
}

Expand All @@ -45,7 +45,7 @@ impl Environment {
if !key.starts_with("BUILD_") {
format!("BUILD_{key}")
} else {
key.to_string()
key.to_owned()
}
}

Expand All @@ -70,11 +70,11 @@ impl Environment {
}

fn dockerfile(&self, target: &Target) -> (Option<String>, Option<String>) {
self.get_values_for("DOCKERFILE", target, |s| s.to_string())
self.get_values_for("DOCKERFILE", target, |s| s.to_owned())
}

fn dockerfile_context(&self, target: &Target) -> (Option<String>, Option<String>) {
self.get_values_for("DOCKERFILE_CONTEXT", target, |s| s.to_string())
self.get_values_for("DOCKERFILE_CONTEXT", target, |s| s.to_owned())
}

fn pre_build(&self, target: &Target) -> (Option<Vec<String>>, Option<Vec<String>>) {
Expand Down Expand Up @@ -384,8 +384,8 @@ mod tests {
fn target_list() -> TargetList {
TargetList {
triples: vec![
"aarch64-unknown-linux-gnu".to_string(),
"armv7-unknown-linux-musleabihf".to_string(),
"aarch64-unknown-linux-gnu".to_owned(),
"armv7-unknown-linux-musleabihf".to_owned(),
],
}
}
Expand Down Expand Up @@ -438,7 +438,7 @@ mod tests {
assert_eq!(
env.build_var_name("target-aarch64-unknown-linux-gnu_image"),
"CROSS_TARGET_AARCH64_UNKNOWN_LINUX_GNU_IMAGE"
)
);
}

#[test]
Expand All @@ -453,10 +453,10 @@ mod tests {
let env = Environment::new(Some(map));

let (build, target) = env.passthrough(&target());
assert!(build.as_ref().unwrap().contains(&"TEST1".to_string()));
assert!(build.as_ref().unwrap().contains(&"TEST2".to_string()));
assert!(target.as_ref().unwrap().contains(&"PASS1".to_string()));
assert!(target.as_ref().unwrap().contains(&"PASS2".to_string()));
assert!(build.as_ref().unwrap().contains(&"TEST1".to_owned()));
assert!(build.as_ref().unwrap().contains(&"TEST2".to_owned()));
assert!(target.as_ref().unwrap().contains(&"PASS1".to_owned()));
assert!(target.as_ref().unwrap().contains(&"PASS2".to_owned()));
}
}

Expand All @@ -467,7 +467,7 @@ mod tests {

macro_rules! s {
($x:literal) => {
$x.to_string()
$x.to_owned()
};
}

Expand Down Expand Up @@ -627,7 +627,7 @@ mod tests {
map.insert("CROSS_BUILD_ENV_VOLUMES", "VOLUME1 VOLUME2");
let env = Environment::new(Some(map));
let config = Config::new_with(Some(toml(TOML_BUILD_VOLUMES)?), env);
let expected = vec!["VOLUME1".to_string(), "VOLUME2".into()];
let expected = vec![s!("VOLUME1"), s!("VOLUME2")];

let result = config.env_volumes(&target()).unwrap().unwrap_or_default();
dbg!(&result);
Expand All @@ -643,7 +643,7 @@ mod tests {
let map = HashMap::new();
let env = Environment::new(Some(map));
let config = Config::new_with(Some(toml(TOML_BUILD_VOLUMES)?), env);
let expected = vec!["VOLUME3".to_string(), "VOLUME4".into()];
let expected = vec![s!("VOLUME3"), s!("VOLUME4")];

let result = config.env_volumes(&target()).unwrap().unwrap_or_default();
dbg!(&result);
Expand Down
28 changes: 14 additions & 14 deletions src/cross_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl FromStr for CrossTargetDockerfileConfig {

fn from_str(s: &str) -> Result<Self, Self::Err> {
Ok(CrossTargetDockerfileConfig {
file: s.to_string(),
file: s.to_owned(),
context: None,
build_args: None,
})
Expand Down Expand Up @@ -154,7 +154,7 @@ impl CrossToml {
.wrap_err("could not convert CrossToml to serde_json::Value")?
.as_object()
{
Ok(obj.to_owned())
Ok(obj.clone())
} else {
eyre::bail!("failed to serialize CrossToml as object");
}
Expand Down Expand Up @@ -408,7 +408,7 @@ mod tests {

macro_rules! s {
($x:literal) => {
$x.to_string()
$x.to_owned()
};
}

Expand All @@ -432,13 +432,13 @@ mod tests {
targets: HashMap::new(),
build: CrossBuildConfig {
env: CrossEnvConfig {
volumes: Some(vec!["VOL1_ARG".to_string(), "VOL2_ARG".to_string()]),
passthrough: Some(vec!["VAR1".to_string(), "VAR2".to_string()]),
volumes: Some(vec![s!("VOL1_ARG"), s!("VOL2_ARG")]),
passthrough: Some(vec![s!("VAR1"), s!("VAR2")]),
},
xargo: Some(true),
build_std: None,
default_target: None,
pre_build: Some(vec!["echo 'Hello World!'".to_string()]),
pre_build: Some(vec![s!("echo 'Hello World!'")]),
dockerfile: None,
},
};
Expand All @@ -465,16 +465,16 @@ mod tests {
let mut target_map = HashMap::new();
target_map.insert(
Target::BuiltIn {
triple: "aarch64-unknown-linux-gnu".to_string(),
triple: s!("aarch64-unknown-linux-gnu"),
},
CrossTargetConfig {
env: CrossEnvConfig {
passthrough: Some(vec!["VAR1".to_string(), "VAR2".to_string()]),
volumes: Some(vec!["VOL1_ARG".to_string(), "VOL2_ARG".to_string()]),
passthrough: Some(vec![s!("VAR1"), s!("VAR2")]),
volumes: Some(vec![s!("VOL1_ARG"), s!("VOL2_ARG")]),
},
xargo: Some(false),
build_std: Some(true),
image: Some("test-image".to_string()),
image: Some(s!("test-image")),
runner: None,
dockerfile: None,
pre_build: Some(vec![]),
Expand Down Expand Up @@ -509,22 +509,22 @@ mod tests {
let mut target_map = HashMap::new();
target_map.insert(
Target::BuiltIn {
triple: "aarch64-unknown-linux-gnu".to_string(),
triple: s!("aarch64-unknown-linux-gnu"),
},
CrossTargetConfig {
xargo: Some(false),
build_std: None,
image: None,
dockerfile: Some(CrossTargetDockerfileConfig {
file: "Dockerfile.test".to_string(),
file: s!("Dockerfile.test"),
context: None,
build_args: None,
}),
pre_build: Some(vec!["echo 'Hello'".to_string()]),
pre_build: Some(vec![s!("echo 'Hello'")]),
runner: None,
env: CrossEnvConfig {
passthrough: None,
volumes: Some(vec!["VOL".to_string()]),
volumes: Some(vec![s!("VOL")]),
},
},
);
Expand Down
8 changes: 4 additions & 4 deletions src/docker/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl<'a> Dockerfile<'a> {
let image_name = self.image_name(&options.target, &paths.metadata)?;
docker_build.args(["--tag", &image_name]);

for (key, arg) in build_args.into_iter() {
for (key, arg) in build_args {
docker_build.args(["--build-arg", &format!("{}={}", key.as_ref(), arg.as_ref())]);
}

Expand Down Expand Up @@ -107,7 +107,7 @@ impl<'a> Dockerfile<'a> {
match self {
Dockerfile::File {
name: Some(name), ..
} => Ok(name.to_string()),
} => Ok((*name).to_owned()),
_ => Ok(format!(
"{}{package_name}:{target_triple}-{path_hash}{custom}",
CROSS_CUSTOM_DOCKERFILE_IMAGE_PREFIX,
Expand Down Expand Up @@ -181,7 +181,7 @@ fn docker_tag_name(file_name: &str) -> String {

// in case all characters were invalid, use a non-empty filename
if result.is_empty() {
result = "empty".to_string();
result = "empty".to_owned();
}

result
Expand All @@ -193,7 +193,7 @@ mod tests {

macro_rules! s {
($s:literal) => {
$s.to_string()
$s.to_owned()
};
}

Expand Down
3 changes: 3 additions & 0 deletions src/docker/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ impl Engine {
is_remote: Option<bool>,
msg_info: &mut MessageInfo,
) -> Result<Engine> {
#[allow(clippy::map_err_ignore)]
let path = get_container_engine()
.map_err(|_| eyre::eyre!("no container engine found"))
.with_suggestion(|| "is docker or podman installed?")?;
Expand All @@ -58,6 +59,7 @@ impl Engine {
})
}

#[must_use]
pub fn needs_remote(&self) -> bool {
self.is_remote && self.kind == EngineType::Podman
}
Expand All @@ -80,6 +82,7 @@ impl Engine {
)
}

#[must_use]
pub fn is_remote() -> bool {
env::var("CROSS_REMOTE")
.map(|s| bool_from_envvar(&s))
Expand Down
2 changes: 1 addition & 1 deletion src/docker/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub(crate) fn run(
if options.needs_custom_image() {
image = options
.custom_image_build(&paths, msg_info)
.wrap_err("when building custom image")?
.wrap_err("when building custom image")?;
}

docker
Expand Down
Loading

0 comments on commit ebd6900

Please sign in to comment.