Skip to content

Fix some clippy warnings #9329

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

Closed
wants to merge 5 commits into from
Closed
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
2 changes: 1 addition & 1 deletion crates/cargo-util/src/process_error.rs
Original file line number Diff line number Diff line change
@@ -190,5 +190,5 @@ pub fn is_simple_exit_code(code: i32) -> bool {
// codes are very large. This is just a rough
// approximation of which codes are "normal" and which
// ones are abnormal termination.
code >= 0 && code <= 127
(0..=127).contains(&code)
}
3 changes: 1 addition & 2 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
@@ -788,8 +788,7 @@ impl RustDocFingerprint {
doc_dirs
.iter()
.filter(|path| path.exists())
.map(|path| paths::remove_dir_all(&path))
.collect::<CargoResult<()>>()
.try_for_each(|path| paths::remove_dir_all(&path))
}

/// This function checks whether the latest version of `Rustc` used to compile this
81 changes: 43 additions & 38 deletions src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
@@ -1685,23 +1685,25 @@ pub fn parse_dep_info(
Ok(data) => data,
Err(_) => return Ok(None),
};
let info = match EncodedDepInfo::parse(&data) {
let EncodedDepInfo {
files: ty_paths,
env,
} = match EncodedDepInfo::parse(&data) {
Some(info) => info,
None => {
log::warn!("failed to parse cargo's dep-info at {:?}", dep_info);
return Ok(None);
}
};
let mut ret = RustcDepInfo::default();
ret.env = info.env;
for (ty, path) in info.files {
let path = match ty {
let files = ty_paths
.iter()
.map(|(ty, path)| match ty {
DepInfoPathType::PackageRootRelative => pkg_root.join(path),
// N.B. path might be absolute here in which case the join will have no effect
DepInfoPathType::TargetRootRelative => target_root.join(path),
};
ret.files.push(path);
}
})
.collect();
let ret = RustcDepInfo { env, files };
Ok(Some(ret))
}

@@ -1819,12 +1821,10 @@ pub fn translate_dep_info(
rustc_cmd: &ProcessBuilder,
allow_package: bool,
) -> CargoResult<()> {
let depinfo = parse_rustc_dep_info(rustc_dep_info)?;
let RustcDepInfo { files, mut env } = parse_rustc_dep_info(rustc_dep_info)?;

let target_root = target_root.canonicalize()?;
let pkg_root = pkg_root.canonicalize()?;
let mut on_disk_info = EncodedDepInfo::default();
on_disk_info.env = depinfo.env;

// This is a bit of a tricky statement, but here we're *removing* the
// dependency on environment variables that were defined specifically for
@@ -1850,34 +1850,39 @@ pub fn translate_dep_info(
// you write a binary that does `println!("{}", env!("OUT_DIR"))` we won't
// recompile that if you move the target directory. Hopefully that's not too
// bad of an issue for now...
on_disk_info
.env
.retain(|(key, _)| !rustc_cmd.get_envs().contains_key(key));

for file in depinfo.files {
// The path may be absolute or relative, canonical or not. Make sure
// it is canonicalized so we are comparing the same kinds of paths.
let abs_file = rustc_cwd.join(file);
// If canonicalization fails, just use the abs path. There is currently
// a bug where --remap-path-prefix is affecting .d files, causing them
// to point to non-existent paths.
let canon_file = abs_file.canonicalize().unwrap_or_else(|_| abs_file.clone());

let (ty, path) = if let Ok(stripped) = canon_file.strip_prefix(&target_root) {
(DepInfoPathType::TargetRootRelative, stripped)
} else if let Ok(stripped) = canon_file.strip_prefix(&pkg_root) {
if !allow_package {
continue;
env.retain(|(key, _)| !rustc_cmd.get_envs().contains_key(key));

let ty_paths = files
.into_iter()
.filter_map(|file| {
// The path may be absolute or relative, canonical or not. Make sure
// it is canonicalized so we are comparing the same kinds of paths.
let abs_file = rustc_cwd.join(file);
// If canonicalization fails, just use the abs path. There is currently
// a bug where --remap-path-prefix is affecting .d files, causing them
// to point to non-existent paths.
let canon_file = abs_file.canonicalize().unwrap_or_else(|_| abs_file.clone());

if let Ok(stripped) = canon_file.strip_prefix(&target_root) {
Some((DepInfoPathType::TargetRootRelative, stripped.to_owned()))
} else if let Ok(stripped) = canon_file.strip_prefix(&pkg_root) {
if allow_package {
Some((DepInfoPathType::PackageRootRelative, stripped.to_owned()))
} else {
None
}
} else {
// It's definitely not target root relative, but this is an absolute path (since it was
// joined to rustc_cwd) and as such re-joining it later to the target root will have no
// effect.
Some((DepInfoPathType::TargetRootRelative, abs_file))
}
(DepInfoPathType::PackageRootRelative, stripped)
} else {
// It's definitely not target root relative, but this is an absolute path (since it was
// joined to rustc_cwd) and as such re-joining it later to the target root will have no
// effect.
(DepInfoPathType::TargetRootRelative, &*abs_file)
};
on_disk_info.files.push((ty, path.to_owned()));
}
})
.collect();
let on_disk_info = EncodedDepInfo {
files: ty_paths,
env,
};
paths::write(cargo_dep_info, on_disk_info.serialize()?)?;
Ok(())
}
6 changes: 4 additions & 2 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
@@ -414,8 +414,10 @@ impl Features {
config: &Config,
warnings: &mut Vec<String>,
) -> CargoResult<Features> {
let mut ret = Features::default();
ret.nightly_features_allowed = config.nightly_features_allowed;
let mut ret = Features {
nightly_features_allowed: config.nightly_features_allowed,
..Default::default()
};
for feature in features {
ret.add(feature, config, warnings)?;
ret.activated.push(feature.to_string());
15 changes: 8 additions & 7 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
@@ -388,12 +388,13 @@ impl Profiles {
/// profile flags don't cause `build.rs` to needlessly run multiple
/// times).
pub fn get_profile_run_custom_build(&self, for_unit_profile: &Profile) -> Profile {
let mut result = Profile::default();
result.name = for_unit_profile.name;
result.root = for_unit_profile.root;
result.debuginfo = for_unit_profile.debuginfo;
result.opt_level = for_unit_profile.opt_level;
result
Profile {
name: for_unit_profile.name,
root: for_unit_profile.root,
debuginfo: for_unit_profile.debuginfo,
opt_level: for_unit_profile.opt_level,
..Default::default()
}
}

/// This returns the base profile. This is currently used for the
@@ -860,7 +861,7 @@ pub struct UnitFor {
/// uses the `get_profile_run_custom_build` method to get the correct
/// profile information for the unit. `host` needs to be true so that all
/// of the dependencies of that `RunCustomBuild` unit have this flag be
/// sticky (and forced to `true` for all further dependencies) — which is
/// sticky (and forced to `true` for all further dependencies) — which is
/// the whole point of `UnitFor`.
host: bool,
/// A target for a build dependency or proc-macro (or any of its
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/context.rs
Original file line number Diff line number Diff line change
@@ -170,7 +170,7 @@ impl Context {
// package again, which should only affect performance, but that
// should be rare. Cycles should still be detected since those
// will have `DepFeatures` edges.
RequestedFeatures::CliFeatures(_) => return Ok(false),
RequestedFeatures::CliFeatures(_) => Ok(false),
RequestedFeatures::DepFeatures {
features,
uses_default_features,
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/dep_cache.rs
Original file line number Diff line number Diff line change
@@ -333,7 +333,7 @@ fn build_requirements<'a, 'b: 'a>(
}
} else {
for fv in features.iter() {
if let Err(e) = reqs.require_value(&fv) {
if let Err(e) = reqs.require_value(fv) {
return Err(e.into_activate_error(parent, s));
}
}
4 changes: 2 additions & 2 deletions src/cargo/core/resolver/features.rs
Original file line number Diff line number Diff line change
@@ -367,7 +367,7 @@ impl ResolvedFeatures {
// The new resolver should never add features.
assert_eq!(new_features.difference(&old_features).next(), None);
let removed_features: BTreeSet<_> =
old_features.difference(&new_features).cloned().collect();
old_features.difference(new_features).cloned().collect();
if removed_features.is_empty() {
None
} else {
@@ -386,7 +386,7 @@ impl ResolvedFeatures {
};
// The new resolver should never add dependencies.
assert_eq!(new_deps.difference(&old_deps).next(), None);
let removed_deps: BTreeSet<_> = old_deps.difference(&new_deps).cloned().collect();
let removed_deps: BTreeSet<_> = old_deps.difference(new_deps).cloned().collect();
if removed_deps.is_empty() {
None
} else {
6 changes: 3 additions & 3 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
@@ -624,7 +624,7 @@ fn activate(
}
}

let activated = cx.flag_activated(&candidate, &opts, parent)?;
let activated = cx.flag_activated(&candidate, opts, parent)?;

let candidate = match registry.replacement_summary(candidate_pid) {
Some(replace) => {
@@ -633,7 +633,7 @@ fn activate(
// does. TBH it basically cause panics in the test suite if
// `parent` is passed through here and `[replace]` is otherwise
// on life support so it's not critical to fix bugs anyway per se.
if cx.flag_activated(replace, &opts, None)? && activated {
if cx.flag_activated(replace, opts, None)? && activated {
return Ok(None);
}
trace!(
@@ -654,7 +654,7 @@ fn activate(

let now = Instant::now();
let (used_features, deps) =
&*registry.build_deps(cx, parent.map(|p| p.0.package_id()), &candidate, &opts)?;
&*registry.build_deps(cx, parent.map(|p| p.0.package_id()), &candidate, opts)?;

// Record what list of features is active for this package.
if !used_features.is_empty() {
4 changes: 2 additions & 2 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
@@ -404,7 +404,7 @@ impl<'cfg> Workspace<'cfg> {
/* platform */ None,
// NOTE: Since we use ConfigRelativePath, this root isn't used as
// any relative paths are resolved before they'd be joined with root.
&Path::new("unused-relative-path"),
Path::new("unused-relative-path"),
self.unstable_features(),
/* kind */ None,
)
@@ -436,7 +436,7 @@ impl<'cfg> Workspace<'cfg> {
return Ok(from_manifest.clone());
}
if from_manifest.is_empty() {
return Ok(from_config.clone());
return Ok(from_config);
}

// We could just chain from_manifest and from_config,
11 changes: 6 additions & 5 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
@@ -791,12 +791,13 @@ impl CompileFilter {
}

pub fn is_all_targets(&self) -> bool {
match *self {
matches!(
*self,
CompileFilter::Only {
all_targets: true, ..
} => true,
_ => false,
}
all_targets: true,
..
}
)
}

pub(crate) fn contains_glob_patterns(&self) -> bool {
14 changes: 7 additions & 7 deletions src/cargo/ops/cargo_config.rs
Original file line number Diff line number Diff line change
@@ -123,14 +123,14 @@ fn print_toml(config: &Config, opts: &GetOptions<'_>, key: &ConfigKey, cv: &CV)
format!(" # {}", def)
};
match cv {
CV::Boolean(val, def) => drop_println!(config, "{} = {}{}", key, val, origin(&def)),
CV::Integer(val, def) => drop_println!(config, "{} = {}{}", key, val, origin(&def)),
CV::Boolean(val, def) => drop_println!(config, "{} = {}{}", key, val, origin(def)),
CV::Integer(val, def) => drop_println!(config, "{} = {}{}", key, val, origin(def)),
CV::String(val, def) => drop_println!(
config,
"{} = {}{}",
key,
toml::to_string(&val).unwrap(),
origin(&def)
origin(def)
),
CV::List(vals, _def) => {
if opts.show_origin {
@@ -145,13 +145,13 @@ fn print_toml(config: &Config, opts: &GetOptions<'_>, key: &ConfigKey, cv: &CV)
}
}
CV::Table(table, _def) => {
let mut key_vals: Vec<_> = table.into_iter().collect();
key_vals.sort_by(|a, b| a.0.cmp(&b.0));
let mut key_vals: Vec<_> = table.iter().collect();
key_vals.sort_by(|a, b| a.0.cmp(b.0));
for (table_key, val) in key_vals {
let mut subkey = key.clone();
// push or push_sensitive shouldn't matter here, since this is
// not dealing with environment variables.
subkey.push(&table_key);
subkey.push(table_key);
print_toml(config, opts, &subkey, val);
}
}
@@ -205,7 +205,7 @@ fn print_json(config: &Config, key: &ConfigKey, cv: &CV, include_key: bool) {
CV::Integer(val, _def) => json!(val),
CV::String(val, _def) => json!(val),
CV::List(vals, _def) => {
let jvals: Vec<_> = vals.into_iter().map(|(val, _def)| json!(val)).collect();
let jvals: Vec<_> = vals.iter().map(|(val, _def)| json!(val)).collect();
json!(jvals)
}
CV::Table(map, _def) => {
26 changes: 12 additions & 14 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
@@ -703,21 +703,19 @@ fn remove_orphaned_bins(
let all_self_names = exe_names(pkg, &filter);
let mut to_remove: HashMap<PackageId, BTreeSet<String>> = HashMap::new();
// For each package that we stomped on.
for other_pkg in duplicates.values() {
for other_pkg in duplicates.values().flatten() {
// Only for packages with the same name.
if let Some(other_pkg) = other_pkg {
if other_pkg.name() == pkg.name() {
// Check what the old package had installed.
if let Some(installed) = tracker.installed_bins(*other_pkg) {
// If the old install has any names that no longer exist,
// add them to the list to remove.
for installed_name in installed {
if !all_self_names.contains(installed_name.as_str()) {
to_remove
.entry(*other_pkg)
.or_default()
.insert(installed_name.clone());
}
if other_pkg.name() == pkg.name() {
// Check what the old package had installed.
if let Some(installed) = tracker.installed_bins(*other_pkg) {
// If the old install has any names that no longer exist,
// add them to the list to remove.
for installed_name in installed {
if !all_self_names.contains(installed_name.as_str()) {
to_remove
.entry(*other_pkg)
.or_default()
.insert(installed_name.clone());
}
}
}
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_run.rs
Original file line number Diff line number Diff line change
@@ -60,7 +60,7 @@ pub fn run(
.into_iter()
.map(|(_pkg, target)| target.name())
.collect();
names.sort();
names.sort_unstable();
anyhow::bail!(
"`cargo run` could not determine which binary to run. \
Use the `--bin` option to specify a binary, \
2 changes: 1 addition & 1 deletion src/cargo/ops/common_for_install_and_uninstall.rs
Original file line number Diff line number Diff line change
@@ -492,7 +492,7 @@ impl InstallInfo {
fn is_up_to_date(&self, opts: &CompileOptions, target: &str, exes: &BTreeSet<String>) -> bool {
self.features == feature_set(&opts.cli_features.features)
&& self.all_features == opts.cli_features.all_features
&& self.no_default_features == !opts.cli_features.uses_default_features
&& self.no_default_features != opts.cli_features.uses_default_features
&& self.profile.as_str() == opts.build_config.requested_profile.as_str()
&& (self.target.is_none() || self.target.as_deref() == Some(target))
&& &self.bins == exes
6 changes: 3 additions & 3 deletions src/cargo/util/config/key.rs
Original file line number Diff line number Diff line change
@@ -103,9 +103,9 @@ impl fmt::Display for ConfigKey {
}

fn escape_key_part<'a>(part: &'a str) -> Cow<'a, str> {
let ok = part.chars().all(|c| match c {
'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' => true,
_ => false,
let ok = part.chars().all(|c| {
matches!(c,
'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_')
});
if ok {
Cow::Borrowed(part)
2 changes: 1 addition & 1 deletion src/cargo/util/workspace.rs
Original file line number Diff line number Diff line change
@@ -26,7 +26,7 @@ fn get_available_targets<'a>(
.map(Target::name)
.collect();

targets.sort();
targets.sort_unstable();

Ok(targets)
}
Loading