Skip to content

Commit

Permalink
feat: Add missing_dep_diagnostic thanks to @Muscraft
Browse files Browse the repository at this point in the history
  • Loading branch information
linyihai committed Jul 3, 2024
1 parent 3eb6bdf commit b28eef9
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 38 deletions.
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
97 changes: 76 additions & 21 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,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 @@ -29,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 @@ -1459,7 +1461,14 @@ fn to_real_manifest(
// 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::<MissingDependencyError>() {
check_weak_optional_dep_unused(&original_toml, missing_dep)?;
missing_dep_diagnostic(
missing_dep,
&original_toml,
&document,
&contents,
manifest_file,
gctx,
)?;
}
}
summary?
Expand Down Expand Up @@ -1570,37 +1579,83 @@ fn to_real_manifest(
Ok(manifest)
}

fn check_weak_optional_dep_unused(
original_toml: &TomlManifest,
fn missing_dep_diagnostic(
missing_dep: &MissingDependencyError,
orig_toml: &TomlManifest,
document: &ImDocument<String>,
contents: &str,
manifest_file: &Path,
gctx: &GlobalContext,
) -> CargoResult<()> {
if missing_dep.weak_optional {
// dev-dependencies are not allowed to be optional
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![
original_toml.dependencies.as_ref(),
original_toml.build_dependencies.as_ref(),
(
orig_toml.dependencies.as_ref(),
vec![DepKind::Normal.kind_table()],
),
(
orig_toml.build_dependencies.as_ref(),
vec![DepKind::Build.kind_table()],
),
];
for (_, platform) in original_toml.target.iter().flatten() {
orig_deps.extend(vec![
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()],
));
}
for deps in orig_deps {

if let Some((_, toml_path)) = orig_deps.iter().find(|(deps, _)| {
if let Some(deps) = deps {
if deps.keys().any(|p| *p.as_str() == *missing_dep.dep_name) {
bail!(
"feature `{feature}` includes `{feature_value}`, but missing `dep:{dep_name}` to activate it",
feature = missing_dep.feature,
feature_value = missing_dep.feature_value,
dep_name = missing_dep.dep_name,
)
}
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)
};

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

fn to_virtual_manifest(
Expand Down
18 changes: 12 additions & 6 deletions tests/testsuite/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,14 @@ fn invalid6() {
p.cargo("check --features foo")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] feature `foo` includes `bar/baz`, but `bar` is not a dependency
--> Cargo.toml:9:23
|
9 | foo = ["bar/baz"]
| ^^^^^^^^^^^
|
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `foo` includes `bar/baz`, but `bar` is not a dependency
"#]])
.run();
}
Expand Down Expand Up @@ -288,11 +291,14 @@ fn invalid7() {
p.cargo("check --features foo")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] feature `foo` includes `bar/baz`, but `bar` is not a dependency
--> Cargo.toml:9:23
|
9 | foo = ["bar/baz"]
| ^^^^^^^^^^^
|
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `foo` includes `bar/baz`, but `bar` is not a dependency
"#]])
.run();
}
Expand Down
37 changes: 28 additions & 9 deletions tests/testsuite/lints/unused_optional_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,14 @@ fn inactive_weak_optional_dep() {
.masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"])
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency
--> Cargo.toml:11:27
|
11 | foo_feature = ["dep_name?/dep_feature"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency
"#]])
.run();

Expand Down Expand Up @@ -287,11 +290,19 @@ Caused by:
.masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"])
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency
--> Cargo.toml:12:31
|
9 | dep_name = { version = "0.1.0", optional = true }
| -------- `dep_name` is an unused optional dependency since no feature enables it
10 |
11 | [features]
12 | foo_feature = ["dep_name?/dep_feature"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= [HELP] enable the dependency with `dep:dep_name`
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `foo_feature` includes `dep_name?/dep_feature`, but missing `dep:dep_name` to activate it
"#]])
.run();
// Check target.'cfg(unix)'.dependencies can work
Expand Down Expand Up @@ -319,11 +330,19 @@ Caused by:
.masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"])
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] feature `foo_feature` includes `dep_name?/dep_feature`, but `dep_name` is not a dependency
--> Cargo.toml:12:27
|
9 | dep_name = { version = "0.1.0", optional = true }
| -------- `dep_name` is an unused optional dependency since no feature enables it
10 |
11 | [features]
12 | foo_feature = ["dep_name?/dep_feature"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= [HELP] enable the dependency with `dep:dep_name`
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `foo_feature` includes `dep_name?/dep_feature`, but missing `dep:dep_name` to activate it
"#]])
.run();
}

0 comments on commit b28eef9

Please sign in to comment.