Skip to content

Commit

Permalink
feat: Avoid creating implicit features on edition 2024
Browse files Browse the repository at this point in the history
  • Loading branch information
Muscraft committed Apr 19, 2024
1 parent d8340a1 commit c258fbf
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 68 deletions.
133 changes: 79 additions & 54 deletions src/cargo/util/lints.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::core::dependency::DepKind;
use crate::core::FeatureValue::Dep;
use crate::core::{Edition, FeatureValue, Package};
use crate::util::interning::InternedString;
Expand Down Expand Up @@ -260,63 +261,87 @@ pub fn unused_dependencies(
if lint_level == LintLevel::Allow {
return Ok(());
}

let mut emitted_source = None;
let manifest = pkg.manifest();
let activated_opt_deps = manifest
.resolved_toml()
.features()
.map(|map| {
map.values()
.flatten()
.filter_map(|f| match FeatureValue::new(InternedString::new(f)) {
Dep { dep_name } => Some(dep_name.as_str()),
_ => None,
})
.collect::<HashSet<_>>()
})
.unwrap_or_default();
let original_toml = manifest.original_toml();
let all_dependencies = manifest
.dependencies()
.into_iter()
.map(|d| d.name_in_toml().to_string())
.collect::<HashSet<String>>();
let mut orig_deps = vec![
(
original_toml.dependencies.as_ref(),
vec![DepKind::Normal.kind_table()],
),
(
original_toml.dev_dependencies.as_ref(),
vec![DepKind::Development.kind_table()],
),
(
original_toml.build_dependencies.as_ref(),
vec![DepKind::Build.kind_table()],
),
];
for (name, platform) in original_toml.target.iter().flatten() {
orig_deps.push((
platform.dependencies.as_ref(),
vec!["target", name, DepKind::Normal.kind_table()],
));
orig_deps.push((
platform.dev_dependencies.as_ref(),
vec!["target", name, DepKind::Development.kind_table()],
));
orig_deps.push((
platform.build_dependencies.as_ref(),
vec!["target", name, DepKind::Normal.kind_table()],
));
}
for (deps, toml_path) in orig_deps {
if let Some(deps) = deps {
for (name, _) in deps {
if !all_dependencies.contains(name.as_str()) {
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
*error_count += 1;
}
let toml_path = toml_path
.iter()
.map(|s| *s)
.chain(std::iter::once(name.as_str()))
.collect::<Vec<_>>();
let level = lint_level.to_diagnostic_level();
let manifest_path = rel_cwd_manifest_path(path, gctx);

let mut emitted_source = None;
for dep in manifest.dependencies() {
let dep_name_in_toml = dep.name_in_toml();
if !dep.is_optional() || activated_opt_deps.contains(dep_name_in_toml.as_str()) {
continue;
}
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
*error_count += 1;
}
let mut toml_path = vec![dep.kind().kind_table(), dep_name_in_toml.as_str()];
let platform = dep.platform().map(|p| p.to_string());
if let Some(platform) = platform.as_ref() {
toml_path.insert(0, platform);
toml_path.insert(0, "target");
}
let level = lint_level.to_diagnostic_level();
let manifest_path = rel_cwd_manifest_path(path, gctx);
let mut message = level.title(UNUSED_OPTIONAL_DEPENDENCY.desc).snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(level.span(get_span(manifest.document(), &toml_path, false).unwrap()))
.fold(true),
);
if emitted_source.is_none() {
emitted_source = Some(format!(
"`cargo::{}` is set to `{lint_level}`",
UNUSED_OPTIONAL_DEPENDENCY.name
));
message = message.footer(Level::Note.title(emitted_source.as_ref().unwrap()));
let mut message = level.title(UNUSED_OPTIONAL_DEPENDENCY.desc).snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(level.span(
get_span(manifest.document(), toml_path.as_slice(), false).unwrap(),
))
.fold(true),
);
if emitted_source.is_none() {
emitted_source = Some(format!(
"`cargo::{}` is set to `{lint_level}`",
UNUSED_OPTIONAL_DEPENDENCY.name
));
message =
message.footer(Level::Note.title(emitted_source.as_ref().unwrap()));
}
let help = format!(
"remove the dependency or activate it in a feature with `dep:{name}`"
);
message = message.footer(Level::Help.title(&help));
let renderer = Renderer::styled().term_width(
gctx.shell()
.err_width()
.diagnostic_terminal_width()
.unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH),
);
writeln!(gctx.shell().err(), "{}", renderer.render(message))?;
}
}
}
let help = format!(
"remove the dependency or activate it in a feature with `dep:{dep_name_in_toml}`"
);
message = message.footer(Level::Help.title(&help));
let renderer = Renderer::styled().term_width(
gctx.shell()
.err_width()
.diagnostic_terminal_width()
.unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH),
);
writeln!(gctx.shell().err(), "{}", renderer.render(message))?;
}
Ok(())
}
53 changes: 48 additions & 5 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use annotate_snippets::{Level, Renderer, Snippet};
use std::collections::{BTreeMap, BTreeSet, HashMap};
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
use std::ffi::OsStr;
use std::path::{Path, PathBuf};
use std::rc::Rc;
Expand All @@ -20,6 +20,7 @@ use crate::core::compiler::{CompileKind, CompileTarget};
use crate::core::dependency::{Artifact, ArtifactTarget, DepKind};
use crate::core::manifest::{ManifestMetadata, TargetSourcePath};
use crate::core::resolver::ResolveBehavior;
use crate::core::FeatureValue::Dep;
use crate::core::{find_workspace_root, resolve_relative_path, CliUnstable, FeatureValue};
use crate::core::{Dependency, Manifest, Package, PackageId, Summary, Target};
use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest, Workspace};
Expand Down Expand Up @@ -300,30 +301,56 @@ fn resolve_toml(

if let Some(original_package) = original_toml.package() {
let resolved_package = resolve_package_toml(original_package, package_root, &inherit)?;
let edition = resolved_package
.resolved_edition()
.expect("previously resolved")
.map_or(Edition::default(), |e| {
Edition::from_str(&e).unwrap_or_default()
});
resolved_toml.package = Some(resolved_package);

let activated_opt_deps = resolved_toml
.features
.as_ref()
.map(|map| {
map.values()
.flatten()
.filter_map(|f| match FeatureValue::new(InternedString::new(f)) {
Dep { dep_name } => Some(dep_name.as_str()),
_ => None,
})
.collect::<HashSet<_>>()
})
.unwrap_or_default();

resolved_toml.dependencies = resolve_dependencies(
gctx,
edition,
&features,
original_toml.dependencies.as_ref(),
&activated_opt_deps,
None,
&inherit,
package_root,
warnings,
)?;
resolved_toml.dev_dependencies = resolve_dependencies(
gctx,
edition,
&features,
original_toml.dev_dependencies(),
&activated_opt_deps,
Some(DepKind::Development),
&inherit,
package_root,
warnings,
)?;
resolved_toml.build_dependencies = resolve_dependencies(
gctx,
edition,
&features,
original_toml.build_dependencies(),
&activated_opt_deps,
Some(DepKind::Build),
&inherit,
package_root,
Expand All @@ -333,26 +360,32 @@ fn resolve_toml(
for (name, platform) in original_toml.target.iter().flatten() {
let resolved_dependencies = resolve_dependencies(
gctx,
edition,
&features,
platform.dependencies.as_ref(),
&activated_opt_deps,
None,
&inherit,
package_root,
warnings,
)?;
let resolved_dev_dependencies = resolve_dependencies(
gctx,
edition,
&features,
platform.dev_dependencies(),
&activated_opt_deps,
Some(DepKind::Development),
&inherit,
package_root,
warnings,
)?;
let resolved_build_dependencies = resolve_dependencies(
gctx,
edition,
&features,
platform.build_dependencies(),
&activated_opt_deps,
Some(DepKind::Build),
&inherit,
package_root,
Expand Down Expand Up @@ -561,8 +594,10 @@ fn default_readme_from_package_root(package_root: &Path) -> Option<String> {
#[tracing::instrument(skip_all)]
fn resolve_dependencies<'a>(
gctx: &GlobalContext,
edition: Edition,
features: &Features,
orig_deps: Option<&BTreeMap<manifest::PackageName, manifest::InheritableDependency>>,
activated_opt_deps: &HashSet<&str>,
kind: Option<DepKind>,
inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>,
package_root: &Path,
Expand Down Expand Up @@ -610,10 +645,18 @@ fn resolve_dependencies<'a>(
}
}

deps.insert(
name_in_toml.clone(),
manifest::InheritableDependency::Value(resolved.clone()),
);
// if the dependency is not optional, it is always used
// if the dependency is optional and activated, it is used
// if the dependency is optional and not activated, it is not used
let is_dep_activated =
!resolved.is_optional() || activated_opt_deps.contains(name_in_toml.as_str());
// If the edition is less than 2024, we don't need to check for unused optional dependencies
if edition < Edition::Edition2024 || is_dep_activated {
deps.insert(
name_in_toml.clone(),
manifest::InheritableDependency::Value(resolved.clone()),
);
}
}
Ok(Some(deps))
}
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit c258fbf

Please sign in to comment.