Skip to content

Commit

Permalink
feat: Add Diagnostic for missing dependency
Browse files Browse the repository at this point in the history
  • Loading branch information
Muscraft committed Jul 2, 2024
1 parent 6ed64a7 commit c0b44e0
Show file tree
Hide file tree
Showing 5 changed files with 407 additions and 22 deletions.
37 changes: 36 additions & 1 deletion src/cargo/core/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,36 @@ struct Inner {
rust_version: Option<RustVersion>,
}

// Indicates the dependency inferred from the `dep` syntax that should exist, but missing on the resolved dependencies
#[derive(Debug)]
pub struct MissingDependency {
dep_name: InternedString,
feature: InternedString,
feature_value: FeatureValue,
weak_optional: bool, // Indicates the dependency inferred from the `dep?` syntax that is weak optional
}

impl MissingDependency {
pub fn dep_name(&self) -> String {
self.dep_name.to_string()
}
pub fn feature(&self) -> InternedString {
self.feature
}
pub fn feature_value(&self) -> FeatureValue {
self.feature_value.clone()
}
pub fn weak_optional(&self) -> bool {
self.weak_optional
}
}

impl fmt::Display for MissingDependency {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{:?}", self)
}
}

impl Summary {
#[tracing::instrument(skip_all)]
pub fn new(
Expand Down Expand Up @@ -274,7 +304,12 @@ fn build_feature_map(

// Validation of the feature name will be performed in the resolver.
if !is_any_dep {
bail!("feature `{feature}` includes `{fv}`, but `{dep_name}` is not a dependency");
bail!(MissingDependency {
feature: *feature,
feature_value: (*fv).clone(),
dep_name: *dep_name,
weak_optional: *weak,
})
}
if *weak && !is_optional_dep {
bail!(
Expand Down
8 changes: 6 additions & 2 deletions src/cargo/util/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,11 @@ fn verify_feature_enabled(
Ok(())
}

fn get_span(document: &ImDocument<String>, path: &[&str], get_value: bool) -> Option<Range<usize>> {
pub fn get_span(
document: &ImDocument<String>,
path: &[&str],
get_value: bool,
) -> Option<Range<usize>> {
let mut table = document.as_item().as_table_like()?;
let mut iter = path.into_iter().peekable();
while let Some(key) = iter.next() {
Expand Down Expand Up @@ -240,7 +244,7 @@ fn get_span(document: &ImDocument<String>, path: &[&str], get_value: bool) -> Op

/// Gets the relative path to a manifest from the current working directory, or
/// the absolute path of the manifest if a relative path cannot be constructed
fn rel_cwd_manifest_path(path: &Path, gctx: &GlobalContext) -> String {
pub fn rel_cwd_manifest_path(path: &Path, gctx: &GlobalContext) -> String {
diff_paths(path, gctx.cwd())
.unwrap_or_else(|| path.to_path_buf())
.display()
Expand Down
141 changes: 123 additions & 18 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::path::{Path, PathBuf};
use std::rc::Rc;
use std::str::{self, FromStr};

use crate::core::summary::MissingDependency;
use crate::AlreadyPrintedError;
use anyhow::{anyhow, bail, Context as _};
use cargo_platform::Platform;
Expand All @@ -14,6 +15,7 @@ use cargo_util_schemas::manifest::{RustVersion, StringOrBool};
use itertools::Itertools;
use lazycell::LazyCell;
use pathdiff::diff_paths;
use toml_edit::ImDocument;
use url::Url;

use crate::core::compiler::{CompileKind, CompileTarget};
Expand All @@ -28,6 +30,7 @@ use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, Worksp
use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY};
use crate::util::errors::{CargoResult, ManifestError};
use crate::util::interning::InternedString;
use crate::util::lints::{get_span, rel_cwd_manifest_path};
use crate::util::{self, context::ConfigRelativePath, GlobalContext, IntoUrl, OptVersionReq};

mod embedded;
Expand Down Expand Up @@ -1435,24 +1438,41 @@ fn to_real_manifest(
.unwrap_or_else(|| semver::Version::new(0, 0, 0)),
source_id,
);
let summary = Summary::new(
pkgid,
deps,
&resolved_toml
.features
.as_ref()
.unwrap_or(&Default::default())
.iter()
.map(|(k, v)| {
(
InternedString::new(k),
v.iter().map(InternedString::from).collect(),
)
})
.collect(),
resolved_package.links.as_deref(),
rust_version.clone(),
)?;
let summary = {
let summary = Summary::new(
pkgid,
deps,
&resolved_toml
.features
.as_ref()
.unwrap_or(&Default::default())
.iter()
.map(|(k, v)| {
(
InternedString::new(k),
v.iter().map(InternedString::from).collect(),
)
})
.collect(),
resolved_package.links.as_deref(),
rust_version.clone(),
);
// editon2024 stops expose implicit features, which will strip weak optional dependencies from `dependencies`,
// need to check whether `dep_name` is stripped as unused dependency
if let Err(ref err) = summary {
if let Some(missing_dep) = err.downcast_ref::<MissingDependency>() {
missing_dep_diagnostic(
missing_dep,
&original_toml,
&document,
&contents,
manifest_file,
gctx,
)?;
}
}
summary?
};
if summary.features().contains_key("default-features") {
warnings.push(
"`default-features = [\"..\"]` was found in [features]. \
Expand Down Expand Up @@ -1558,6 +1578,91 @@ fn to_real_manifest(
Ok(manifest)
}

fn missing_dep_diagnostic(
missing_dep: &MissingDependency,
orig_toml: &TomlManifest,
document: &ImDocument<String>,
contents: &str,
manifest_file: &Path,
gctx: &GlobalContext,
) -> CargoResult<()> {
let dep_name = missing_dep.dep_name();
let manifest_path = rel_cwd_manifest_path(manifest_file, gctx);
let feature_value_span = get_span(
&document,
&["features", missing_dep.feature().as_str()],
true,
)
.unwrap();

let title = format!(
"feature `{}` includes `{}`, but `{}` is not a dependency",
missing_dep.feature(),
missing_dep.feature_value(),
&dep_name
);
let help = format!("enable the dependency with `dep:{dep_name}`");
let info_label = format!(
"`{}` is an unused optional dependency since no feature enables it",
&dep_name
);
let message = Level::Error.title(&title);
let snippet = Snippet::source(&contents)
.origin(&manifest_path)
.fold(true)
.annotation(Level::Error.span(feature_value_span.start..feature_value_span.end));
let message = if missing_dep.weak_optional() {
let mut orig_deps = vec![
(
orig_toml.dependencies.as_ref(),
vec![DepKind::Normal.kind_table()],
),
(
orig_toml.build_dependencies.as_ref(),
vec![DepKind::Build.kind_table()],
),
];
for (name, platform) in orig_toml.target.iter().flatten() {
orig_deps.push((
platform.dependencies.as_ref(),
vec!["target", name, DepKind::Normal.kind_table()],
));
orig_deps.push((
platform.build_dependencies.as_ref(),
vec!["target", name, DepKind::Normal.kind_table()],
));
}

if let Some((_, toml_path)) = orig_deps.iter().find(|(deps, _)| {
if let Some(deps) = deps {
deps.keys().any(|p| *p.as_str() == dep_name)
} else {
false
}
}) {
let toml_path = toml_path
.iter()
.map(|s| *s)
.chain(std::iter::once(dep_name.as_str()))
.collect::<Vec<_>>();
let dep_span = get_span(&document, &toml_path, false).unwrap();

message
.snippet(snippet.annotation(Level::Warning.span(dep_span).label(&info_label)))
.footer(Level::Help.title(&help))
} else {
message.snippet(snippet)
}
} else {
message.snippet(snippet)
};

if let Err(err) = gctx.shell().print_message(message) {
return Err(err.into());
}
Err(AlreadyPrintedError::new(anyhow!("").into()).into())
}

fn to_virtual_manifest(
contents: String,
document: toml_edit::ImDocument<String>,
Expand Down
53 changes: 53 additions & 0 deletions tests/testsuite/lints/unused.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading

0 comments on commit c0b44e0

Please sign in to comment.