From db6b2e450fd2c72e35bbfaafb50de05afd20be6c Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Tue, 23 Sep 2025 11:31:33 -0500 Subject: [PATCH 1/2] Add new tests Add a workspace test --- tests/testsuite/pub_priv.rs | 479 +++++++++++++++++++++++++++++++++++- 1 file changed, 477 insertions(+), 2 deletions(-) diff --git a/tests/testsuite/pub_priv.rs b/tests/testsuite/pub_priv.rs index 85af838ec5e..6260d7176f2 100644 --- a/tests/testsuite/pub_priv.rs +++ b/tests/testsuite/pub_priv.rs @@ -1,9 +1,9 @@ //! Tests for public/private dependencies. use crate::prelude::*; -use cargo_test_support::project; use cargo_test_support::registry::{Dependency, Package}; -use cargo_test_support::str; +use cargo_test_support::{git, str}; +use cargo_test_support::{project, registry}; #[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] fn exported_priv_warning() { @@ -682,3 +682,478 @@ src/lib.rs:6:13: [WARNING] type `FromPriv` from private dependency 'priv_dep' in ) .run(); } + +#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] +fn manifest_location() { + Package::new("dep", "0.1.0") + .file("src/lib.rs", "pub struct FromDep;") + .publish(); + Package::new("priv_dep", "0.1.0") + .file("src/lib.rs", "pub struct FromPriv;") + .publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + + [dependencies] + dep = "0.1.0" + priv_dep = "0.1.0" + "#, + ) + .file( + "src/lib.rs", + " + extern crate dep; + extern crate priv_dep; + pub fn use_dep(_: dep::FromDep) {} + pub use priv_dep::FromPriv; + ", + ) + .build(); + + p.cargo("check -Zpublic-dependency") + .masquerade_as_nightly_cargo(&["public-dependency"]) + .with_stderr_data( + str![[r#" +... +[WARNING] struct `FromPriv` from private dependency 'priv_dep' is re-exported + --> src/lib.rs:5:21 + | +5 | pub use priv_dep::FromPriv; + | ^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `#[warn(exported_private_dependencies)]` on by default + | + +[WARNING] type `FromDep` from private dependency 'dep' in public interface + --> src/lib.rs:4:13 +4 | pub fn use_dep(_: dep::FromDep) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +"#]] + .unordered(), + ) + .run(); +} + +#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] +fn renamed_dependency() { + Package::new("dep", "0.1.0") + .file("src/lib.rs", "pub struct FromDep;") + .publish(); + Package::new("priv_dep", "0.1.0") + .file("src/lib.rs", "pub struct FromPriv;") + .publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + + [dependencies] + dep = { version = "0.1.0", package = "dep" } + renamed_dep = {version = "0.1.0", package = "priv_dep" } + "#, + ) + .file( + "src/lib.rs", + " + extern crate dep; + extern crate renamed_dep; + pub fn use_dep(_: dep::FromDep) {} + pub fn use_priv(_: renamed_dep::FromPriv) {} + ", + ) + .build(); + + p.cargo("check -Zpublic-dependency") + .masquerade_as_nightly_cargo(&["public-dependency"]) + .with_stderr_data( + str![[r#" +... +[WARNING] type `FromDep` from private dependency 'dep' in public interface + --> src/lib.rs:4:13 + | +4 | pub fn use_dep(_: dep::FromDep) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `#[warn(exported_private_dependencies)]` on by default + + | +[WARNING] type `FromPriv` from private dependency 'priv_dep' in public interface + --> src/lib.rs:5:13 +5 | pub fn use_priv(_: renamed_dep::FromPriv) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +... +"#]] + .unordered(), + ) + .run(); +} + +// We don't point to the toml locations if the crate is ambiguous. +#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] +fn duplicate_renamed_dependency() { + registry::alt_init(); + Package::new("dep", "0.1.0") + .file("src/lib.rs", "pub struct FromDep;") + .publish(); + Package::new("dep", "0.1.0") + .file("src/lib.rs", "pub struct FromPriv;") + .alternative(true) + .publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + + [dependencies] + dep = { version = "0.1.0", package = "dep" } + renamed_dep = {version = "0.1.0", package = "dep", registry = "alternative" } + "#, + ) + .file( + "src/lib.rs", + " + extern crate dep; + extern crate renamed_dep; + pub fn use_dep(_: dep::FromDep) {} + pub fn use_priv(_: renamed_dep::FromPriv) {} + ", + ) + .build(); + + p.cargo("check -Zpublic-dependency") + .masquerade_as_nightly_cargo(&["public-dependency"]) + .with_stderr_data( + str![[r#" +... +[WARNING] type `FromDep` from private dependency 'dep' in public interface + --> src/lib.rs:4:13 + | +4 | pub fn use_dep(_: dep::FromDep) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `#[warn(exported_private_dependencies)]` on by default + +[WARNING] type `FromPriv` from private dependency 'dep' in public interface + --> src/lib.rs:5:13 + | +5 | pub fn use_priv(_: renamed_dep::FromPriv) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +"#]] + .unordered(), + ) + .run(); +} + +#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] +fn dependency_location_in_target_table() { + if crate::utils::cross_compile::disabled() { + return; + } + + let native = cargo_test_support::cross_compile::native(); + let alt = cargo_test_support::cross_compile::alternate(); + + Package::new("dep", "0.1.0") + .file("src/lib.rs", "pub struct FromDep;") + .publish(); + Package::new("native_priv_dep", "0.1.0") + .file("src/lib.rs", "pub struct FromPriv;") + .publish(); + Package::new("alt_priv_dep", "0.1.0") + .file("src/lib.rs", "pub struct FromPriv;") + .publish(); + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + + [target.{native}.dependencies] + dep = {{ version = "0.1.0" }} + renamed_dep = {{ version = "0.1.0", package = "native_priv_dep" }} + + [target.{alt}.dependencies] + dep = {{ version = "0.1.0" }} + renamed_dep = {{ version = "0.1.0", package = "alt_priv_dep" }} + "# + ), + ) + .file( + "src/lib.rs", + " + extern crate dep; + extern crate renamed_dep; + pub fn use_dep(_: dep::FromDep) {} + pub fn use_priv(_: renamed_dep::FromPriv) {} + ", + ) + .build(); + + p.cargo("check -Zpublic-dependency") + .masquerade_as_nightly_cargo(&["public-dependency"]) + .with_stderr_data( + str![[r#" +... +[WARNING] type `FromDep` from private dependency 'dep' in public interface + --> src/lib.rs:4:13 + | +4 | pub fn use_dep(_: dep::FromDep) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `#[warn(exported_private_dependencies)]` on by default + | + +[WARNING] type `FromPriv` from private dependency 'native_priv_dep' in public interface + --> src/lib.rs:5:13 +5 | pub fn use_priv(_: renamed_dep::FromPriv) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +"#]] + .unordered(), + ) + .run(); + + p.cargo(&format!("check -Zpublic-dependency --target={alt}")) + .masquerade_as_nightly_cargo(&["public-dependency"]) + .with_stderr_data( + str![[r#" +... +[WARNING] type `FromDep` from private dependency 'dep' in public interface + --> src/lib.rs:4:13 + | +4 | pub fn use_dep(_: dep::FromDep) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `#[warn(exported_private_dependencies)]` on by default + +[WARNING] type `FromPriv` from private dependency 'alt_priv_dep' in public interface + --> src/lib.rs:5:13 + | +5 | pub fn use_priv(_: renamed_dep::FromPriv) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +"#]] + .unordered(), + ) + .run(); +} + +#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] +fn dependency_location_in_target_table_with_cfg() { + if crate::utils::cross_compile::disabled() { + return; + } + + let native_arch = cargo_test_support::cross_compile::native_arch(); + let alt = cargo_test_support::cross_compile::alternate(); + + Package::new("dep", "0.1.0") + .file("src/lib.rs", "pub struct FromDep;") + .publish(); + Package::new("native_priv_dep", "0.1.0") + .file("src/lib.rs", "pub struct FromPriv;") + .publish(); + Package::new("alt_priv_dep", "0.1.0") + .file("src/lib.rs", "pub struct FromPriv;") + .publish(); + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + + [target.'cfg(target_arch = "{native_arch}")'.dependencies] + dep = {{ version = "0.1.0" }} + renamed_dep = {{ version = "0.1.0", package = "native_priv_dep" }} + + [target.'cfg(not(target_arch = "{native_arch}"))'.dependencies] + dep = {{ version = "0.1.0" }} + renamed_dep = {{ version = "0.1.0", package = "alt_priv_dep" }} + "# + ), + ) + .file( + "src/lib.rs", + " + extern crate dep; + extern crate renamed_dep; + pub fn use_dep(_: dep::FromDep) {} + pub fn use_priv(_: renamed_dep::FromPriv) {} + ", + ) + .build(); + + p.cargo("check -Zpublic-dependency") + .masquerade_as_nightly_cargo(&["public-dependency"]) + .with_stderr_data( + str![[r#" +... +[WARNING] type `FromDep` from private dependency 'dep' in public interface + --> src/lib.rs:4:13 + | +4 | pub fn use_dep(_: dep::FromDep) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `#[warn(exported_private_dependencies)]` on by default + | + +[WARNING] type `FromPriv` from private dependency 'native_priv_dep' in public interface + --> src/lib.rs:5:13 +5 | pub fn use_priv(_: renamed_dep::FromPriv) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +"#]] + .unordered(), + ) + .run(); + + p.cargo(&format!("check -Zpublic-dependency --target={alt}")) + .masquerade_as_nightly_cargo(&["public-dependency"]) + .with_stderr_data( + str![[r#" +... +[WARNING] type `FromDep` from private dependency 'dep' in public interface + --> src/lib.rs:4:13 + | +4 | pub fn use_dep(_: dep::FromDep) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `#[warn(exported_private_dependencies)]` on by default + +[WARNING] type `FromPriv` from private dependency 'alt_priv_dep' in public interface + --> src/lib.rs:5:13 + | +5 | pub fn use_priv(_: renamed_dep::FromPriv) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +"#]] + .unordered(), + ) + .run(); +} + +#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] +fn dependency_location_in_workspace() { + Package::new("dep", "0.1.0") + .file("src/lib.rs", "pub struct FromDep;") + .publish(); + let (p, repo) = git::new_repo("foo", |p| { + p.file( + "Cargo.toml", + r#" + [workspace] + members = ["pkg"] + [workspace.package] + edition = "2015" + [workspace.dependencies] + dep = "0.1.0" + "#, + ) + .file( + "pkg/Cargo.toml", + r#" + [package] + name = "pkg" + edition.workspace = true + [dependencies] + dep.workspace = true + "#, + ) + .file( + "pkg/src/lib.rs", + " + extern crate dep; + pub fn use_dep(_: dep::FromDep) {} + ", + ) + }); + git::commit(&repo); + p.cargo(&format!("check -Zpublic-dependency")) + .masquerade_as_nightly_cargo(&["public-dependency"]) + .with_stderr_data(str![[r#" +... +[WARNING] type `FromDep` from private dependency 'dep' in public interface + --> pkg/src/lib.rs:3:13 + | +3 | pub fn use_dep(_: dep::FromDep) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `#[warn(exported_private_dependencies)]` on by default +... +"#]]) + .run(); +} + +#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")] +fn relative_display_path() { + Package::new("priv_dep", "0.1.0") + .file("src/lib.rs", "pub struct FromPriv;") + .publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["foo"] + "#, + ) + .file( + "foo/Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + + [dependencies] + priv_dep = "0.1.0" + "#, + ) + .file( + "foo/src/lib.rs", + " + extern crate priv_dep; + pub use priv_dep::FromPriv; + ", + ) + .build(); + p.cargo("check -Zpublic-dependency") + .cwd("foo") + .masquerade_as_nightly_cargo(&["public-dependency"]) + .with_stderr_data(str![[r#" +... +[WARNING] struct `FromPriv` from private dependency 'priv_dep' is re-exported + --> foo/src/lib.rs:3:21 + | +3 | pub use priv_dep::FromPriv; + | ^^^^^^^^^^^^^^^^^^ + | +... +"#]]) + .run(); +} From ea98c6b9e63a877b906124f5df6bdf331cf77ca8 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Mon, 22 Sep 2025 22:28:54 -0500 Subject: [PATCH 2/2] Add a manifest location to public-in-private errors Co-authored-by: Scott Schafer --- .../compiler/build_context/target_info.rs | 4 + src/cargo/core/compiler/mod.rs | 292 ++++++++++++++++-- src/cargo/util/lints.rs | 30 +- tests/testsuite/pub_priv.rs | 69 ++++- 4 files changed, 350 insertions(+), 45 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index ce1ab9a83a5..f3a00a00750 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -1105,6 +1105,10 @@ impl<'gctx> RustcTargetData<'gctx> { } unsupported } + + pub fn requested_kinds(&self) -> &[CompileKind] { + &self.requested_kinds + } } /// Structure used to deal with Rustdoc fingerprinting diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 90a8bd0a893..510b2d539e6 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -62,11 +62,15 @@ use std::ffi::{OsStr, OsString}; use std::fmt::Display; use std::fs::{self, File}; use std::io::{BufRead, BufWriter, Write}; +use std::ops::Range; use std::path::{Path, PathBuf}; -use std::sync::Arc; +use std::sync::{Arc, LazyLock}; +use annotate_snippets::{AnnotationKind, Group, Level, Renderer, Snippet}; use anyhow::{Context as _, Error}; +use cargo_platform::{Cfg, Platform}; use lazycell::LazyCell; +use regex::Regex; use tracing::{debug, instrument, trace}; pub use self::build_config::UserIntent; @@ -98,8 +102,9 @@ use crate::core::{Feature, PackageId, Target, Verbosity}; use crate::util::context::WarningHandling; use crate::util::errors::{CargoResult, VerboseError}; use crate::util::interning::InternedString; +use crate::util::lints::get_key_value; use crate::util::machine_message::{self, Message}; -use crate::util::{add_path_args, internal}; +use crate::util::{add_path_args, internal, path_args}; use cargo_util::{ProcessBuilder, ProcessError, paths}; use cargo_util_schemas::manifest::TomlDebugInfo; use cargo_util_schemas::manifest::TomlTrimPaths; @@ -215,9 +220,10 @@ fn compile<'gctx>( // since it might contain future-incompat-report messages let show_diagnostics = unit.show_warnings(bcx.gctx) && build_runner.bcx.gctx.warning_handling()? != WarningHandling::Allow; + let manifest = ManifestErrorContext::new(build_runner, unit); let work = replay_output_cache( unit.pkg.package_id(), - PathBuf::from(unit.pkg.manifest_path()), + manifest, &unit.target, build_runner.files().message_cache_path(unit), build_runner.bcx.build_config.message_format, @@ -283,7 +289,7 @@ fn rustc( // Prepare the native lib state (extra `-L` and `-l` flags). let build_script_outputs = Arc::clone(&build_runner.build_script_outputs); let current_id = unit.pkg.package_id(); - let manifest_path = PathBuf::from(unit.pkg.manifest_path()); + let manifest = ManifestErrorContext::new(build_runner, unit); let build_scripts = build_runner.build_scripts.get(unit).cloned(); // If we are a binary and the package also contains a library, then we @@ -424,7 +430,7 @@ fn rustc( state, line, package_id, - &manifest_path, + &manifest, &target, &mut output_options, ) @@ -905,8 +911,8 @@ fn rustdoc(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResult, unit: &Unit) -> CargoResult>, + /// The raw manifest contents. + contents: String, + /// A lookup for all the unambiguous renamings, mapping from the original package + /// name to the renamed one. + rename_table: HashMap, + /// A list of targets we're compiling for, to determine which of the `[target..dependencies]` + /// tables might be of interest. + requested_kinds: Vec, + /// A list of all the collections of cfg values, one collection for each target, to determine + /// which of the `[target.'cfg(...)'.dependencies]` tables might be of interest. + cfgs: Vec>, + host_name: InternedString, + /// Cargo's working directory (for printing out a more friendly manifest path). + cwd: PathBuf, + /// Terminal width for formatting diagnostics. + term_width: usize, +} + fn on_stdout_line( state: &JobState<'_, '_>, line: &str, @@ -1905,11 +1939,11 @@ fn on_stderr_line( state: &JobState<'_, '_>, line: &str, package_id: PackageId, - manifest_path: &std::path::Path, + manifest: &ManifestErrorContext, target: &Target, options: &mut OutputOptions, ) -> CargoResult<()> { - if on_stderr_line_inner(state, line, package_id, manifest_path, target, options)? { + if on_stderr_line_inner(state, line, package_id, manifest, target, options)? { // Check if caching is enabled. if let Some((path, cell)) = &mut options.cache_cell { // Cache the output, which will be replayed later when Fresh. @@ -1927,7 +1961,7 @@ fn on_stderr_line_inner( state: &JobState<'_, '_>, line: &str, package_id: PackageId, - manifest_path: &std::path::Path, + manifest: &ManifestErrorContext, target: &Target, options: &mut OutputOptions, ) -> CargoResult { @@ -1976,6 +2010,45 @@ fn on_stderr_line_inner( return Ok(false); } + // Returns `true` if the diagnostic was modified. + let add_pub_in_priv_diagnostic = |diag: &mut String| -> bool { + // We are parsing the compiler diagnostic here, as this information isn't + // currently exposed elsewhere. + // At the time of writing this comment, rustc emits two different + // "exported_private_dependencies" errors: + // - type `FromPriv` from private dependency 'priv_dep' in public interface + // - struct `FromPriv` from private dependency 'priv_dep' is re-exported + // This regex matches them both. To see if it needs to be updated, grep the rust + // source for "EXPORTED_PRIVATE_DEPENDENCIES". + static PRIV_DEP_REGEX: LazyLock = + LazyLock::new(|| Regex::new("from private dependency '([A-Za-z0-9-_]+)'").unwrap()); + if let Some(crate_name) = PRIV_DEP_REGEX.captures(diag).and_then(|m| m.get(1)) + && let Some(span) = manifest.find_crate_span(crate_name.as_str()) + { + let rel_path = pathdiff::diff_paths(&manifest.path, &manifest.cwd) + .unwrap_or_else(|| manifest.path.clone()) + .display() + .to_string(); + let report = [Group::with_title(Level::NOTE.secondary_title(format!( + "dependency `{}` declared here", + crate_name.as_str() + ))) + .element( + Snippet::source(&manifest.contents) + .path(rel_path) + .annotation(AnnotationKind::Context.span(span)), + )]; + + let rendered = Renderer::styled() + .term_width(manifest.term_width) + .render(&report); + diag.push_str(&rendered); + diag.push('\n'); + return true; + } + false + }; + // Depending on what we're emitting from Cargo itself, we figure out what to // do with this JSON message. match options.format { @@ -2000,6 +2073,7 @@ fn on_stderr_line_inner( #[serde(borrow)] level: Cow<'a, str>, children: Vec, + code: Option, } // A partial rustfix::diagnostics::Diagnostic. We deserialize only a @@ -2021,6 +2095,11 @@ fn on_stderr_line_inner( suggestion_applicability: Option, } + #[derive(serde::Deserialize)] + struct DiagnosticCode { + code: String, + } + if let Ok(mut msg) = serde_json::from_str::>(compiler_message.get()) { if msg.message.starts_with("aborting due to") @@ -2034,7 +2113,7 @@ fn on_stderr_line_inner( if msg.rendered.ends_with('\n') { msg.rendered.pop(); } - let rendered = msg.rendered; + let mut rendered = msg.rendered; if options.show_diagnostics { let machine_applicable: bool = msg .children @@ -2048,34 +2127,58 @@ fn on_stderr_line_inner( }) .any(|b| b); count_diagnostic(&msg.level, options); + if msg + .code + .is_some_and(|c| c.code == "exported_private_dependencies") + && options.format != MessageFormat::Short + { + add_pub_in_priv_diagnostic(&mut rendered); + } state.emit_diag(&msg.level, rendered, machine_applicable)?; } return Ok(true); } } - // Remove color information from the rendered string if color is not - // enabled. Cargo always asks for ANSI colors from rustc. This allows - // cached replay to enable/disable colors without re-invoking rustc. - MessageFormat::Json { ansi: false, .. } => { + MessageFormat::Json { ansi, .. } => { #[derive(serde::Deserialize, serde::Serialize)] struct CompilerMessage<'a> { rendered: String, #[serde(flatten, borrow)] other: std::collections::BTreeMap, serde_json::Value>, + code: Option, + } + + #[derive(serde::Deserialize, serde::Serialize)] + struct DiagnosticCode { + code: String, } + if let Ok(mut error) = serde_json::from_str::>(compiler_message.get()) { - error.rendered = anstream::adapter::strip_str(&error.rendered).to_string(); - let new_line = serde_json::to_string(&error)?; - compiler_message = serde_json::value::RawValue::from_string(new_line)?; + let modified_diag = if error + .code + .as_ref() + .is_some_and(|c| c.code == "exported_private_dependencies") + { + add_pub_in_priv_diagnostic(&mut error.rendered) + } else { + false + }; + + // Remove color information from the rendered string if color is not + // enabled. Cargo always asks for ANSI colors from rustc. This allows + // cached replay to enable/disable colors without re-invoking rustc. + if !ansi { + error.rendered = anstream::adapter::strip_str(&error.rendered).to_string(); + } + if !ansi || modified_diag { + let new_line = serde_json::to_string(&error)?; + compiler_message = serde_json::value::RawValue::from_string(new_line)?; + } } } - - // If ansi colors are desired then we should be good to go! We can just - // pass through this message as-is. - MessageFormat::Json { ansi: true, .. } => {} } // We always tell rustc to emit messages about artifacts being produced. @@ -2128,7 +2231,7 @@ fn on_stderr_line_inner( let msg = machine_message::FromCompiler { package_id: package_id.to_spec(), - manifest_path, + manifest_path: &manifest.path, target, message: compiler_message, } @@ -2141,12 +2244,144 @@ fn on_stderr_line_inner( Ok(true) } +impl ManifestErrorContext { + fn new(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> ManifestErrorContext { + let mut duplicates = HashSet::new(); + let mut rename_table = HashMap::new(); + + for dep in build_runner.unit_deps(unit) { + let unrenamed_id = dep.unit.pkg.package_id().name(); + if duplicates.contains(&unrenamed_id) { + continue; + } + match rename_table.entry(unrenamed_id) { + std::collections::hash_map::Entry::Occupied(occ) => { + occ.remove_entry(); + duplicates.insert(unrenamed_id); + } + std::collections::hash_map::Entry::Vacant(vac) => { + vac.insert(dep.extern_crate_name); + } + } + } + + let bcx = build_runner.bcx; + ManifestErrorContext { + path: unit.pkg.manifest_path().to_owned(), + spans: unit.pkg.manifest().document().clone(), + contents: unit.pkg.manifest().contents().to_owned(), + requested_kinds: bcx.target_data.requested_kinds().to_owned(), + host_name: bcx.rustc().host, + rename_table, + cwd: path_args(build_runner.bcx.ws, unit).1, + cfgs: bcx + .target_data + .requested_kinds() + .iter() + .map(|k| bcx.target_data.cfg(*k).to_owned()) + .collect(), + term_width: bcx + .gctx + .shell() + .err_width() + .diagnostic_terminal_width() + .unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH), + } + } + + fn requested_target_names(&self) -> impl Iterator { + self.requested_kinds.iter().map(|kind| match kind { + CompileKind::Host => &self.host_name, + CompileKind::Target(target) => target.short_name(), + }) + } + + /// Find a span for the dependency that specifies this unrenamed crate, if it's unique. + /// + /// rustc diagnostics (at least for public-in-private) mention the un-renamed + /// crate: if you have `foo = { package = "bar" }`, the rustc diagnostic will + /// say "bar". + /// + /// This function does its best to find a span for "bar", but it could fail if + /// there are multiple candidates: + /// + /// ```toml + /// foo = { package = "bar" } + /// baz = { path = "../bar", package = "bar" } + /// ``` + fn find_crate_span(&self, unrenamed: &str) -> Option> { + let orig_name = self.rename_table.get(unrenamed)?.as_str(); + + if let Some((k, v)) = get_key_value(&self.spans, &["dependencies", orig_name]) { + // We make some effort to find the unrenamed text: in + // + // ``` + // foo = { package = "bar" } + // ``` + // + // we try to find the "bar", but fall back to "foo" if we can't (which might + // happen if the renaming took place in the workspace, for example). + if let Some(package) = v.get_ref().as_table().and_then(|t| t.get("package")) { + return Some(package.span()); + } else { + return Some(k.span()); + } + } + + // The dependency could also be in a target-specific table, like + // [target.x86_64-unknown-linux-gnu.dependencies] or + // [target.'cfg(something)'.dependencies]. We filter out target tables + // that don't match a requested target or a requested cfg. + if let Some(target) = self + .spans + .as_ref() + .get("target") + .and_then(|t| t.as_ref().as_table()) + { + for (platform, platform_table) in target.iter() { + match platform.as_ref().parse::() { + Ok(Platform::Name(name)) => { + if !self.requested_target_names().any(|n| n == name) { + continue; + } + } + Ok(Platform::Cfg(cfg_expr)) => { + if !self.cfgs.iter().any(|cfgs| cfg_expr.matches(cfgs)) { + continue; + } + } + Err(_) => continue, + } + + let Some(platform_table) = platform_table.as_ref().as_table() else { + continue; + }; + + if let Some(deps) = platform_table + .get("dependencies") + .and_then(|d| d.as_ref().as_table()) + { + if let Some((k, v)) = deps.get_key_value(orig_name) { + if let Some(package) = v.get_ref().as_table().and_then(|t| t.get("package")) + { + return Some(package.span()); + } else { + return Some(k.span()); + } + } + } + } + } + None + } +} + /// Creates a unit of work that replays the cached compiler message. /// /// Usually used when a job is fresh and doesn't need to recompile. fn replay_output_cache( package_id: PackageId, - manifest_path: PathBuf, + manifest: ManifestErrorContext, target: &Target, path: PathBuf, format: MessageFormat, @@ -2177,14 +2412,7 @@ fn replay_output_cache( break; } let trimmed = line.trim_end_matches(&['\n', '\r'][..]); - on_stderr_line( - state, - trimmed, - package_id, - &manifest_path, - &target, - &mut options, - )?; + on_stderr_line(state, trimmed, package_id, &manifest, &target, &mut options)?; line.clear(); } Ok(()) diff --git a/src/cargo/util/lints.rs b/src/cargo/util/lints.rs index 0763be70e28..da833b57554 100644 --- a/src/cargo/util/lints.rs +++ b/src/cargo/util/lints.rs @@ -3,6 +3,7 @@ use crate::{CargoResult, GlobalContext}; use annotate_snippets::{AnnotationKind, Group, Level, Snippet}; use cargo_util_schemas::manifest::{TomlLintLevel, TomlToolLints}; use pathdiff::diff_paths; +use std::borrow::Cow; use std::fmt::Display; use std::ops::Range; use std::path::Path; @@ -182,20 +183,20 @@ pub struct TomlSpan { pub value: Range, } -pub fn get_key_value_span( - document: &toml::Spanned>, +pub fn get_key_value<'doc>( + document: &'doc toml::Spanned>, path: &[&str], -) -> Option { +) -> Option<( + &'doc toml::Spanned>, + &'doc toml::Spanned>, +)> { let mut table = document.get_ref(); let mut iter = path.into_iter().peekable(); while let Some(key) = iter.next() { let key_s: &str = key.as_ref(); let (key, item) = table.get_key_value(key_s)?; if iter.peek().is_none() { - return Some(TomlSpan { - key: key.span(), - value: item.span(), - }); + return Some((key, item)); } if let Some(next_table) = item.get_ref().as_table() { table = next_table; @@ -204,10 +205,7 @@ pub fn get_key_value_span( if let Some(array) = item.get_ref().as_array() { let next = iter.next().unwrap(); return array.iter().find_map(|item| match item.get_ref() { - toml::de::DeValue::String(s) if s == next => Some(TomlSpan { - key: key.span(), - value: item.span(), - }), + toml::de::DeValue::String(s) if s == next => Some((key, item)), _ => None, }); } @@ -216,6 +214,16 @@ pub fn get_key_value_span( None } +pub fn get_key_value_span( + document: &toml::Spanned>, + path: &[&str], +) -> Option { + get_key_value(document, path).map(|(k, v)| TomlSpan { + key: k.span(), + value: v.span(), + }) +} + /// 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 pub fn rel_cwd_manifest_path(path: &Path, gctx: &GlobalContext) -> String { diff --git a/tests/testsuite/pub_priv.rs b/tests/testsuite/pub_priv.rs index 6260d7176f2..fc1612d26fc 100644 --- a/tests/testsuite/pub_priv.rs +++ b/tests/testsuite/pub_priv.rs @@ -728,12 +728,22 @@ fn manifest_location() { | ^^^^^^^^^^^^^^^^^^ | = [NOTE] `#[warn(exported_private_dependencies)]` on by default +[NOTE] dependency `priv_dep` declared here + --> Cargo.toml:9:17 | +9 | priv_dep = "0.1.0" + | -------- [WARNING] type `FromDep` from private dependency 'dep' in public interface --> src/lib.rs:4:13 + | 4 | pub fn use_dep(_: dep::FromDep) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +[NOTE] dependency `dep` declared here + --> Cargo.toml:8:17 + | +8 | dep = "0.1.0" + | --- ... "#]] .unordered(), @@ -786,13 +796,22 @@ fn renamed_dependency() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = [NOTE] `#[warn(exported_private_dependencies)]` on by default - +[NOTE] dependency `dep` declared here + --> Cargo.toml:8:54 | +8 | dep = { version = "0.1.0", package = "dep" } + | ----- + [WARNING] type `FromPriv` from private dependency 'priv_dep' in public interface --> src/lib.rs:5:13 + | 5 | pub fn use_priv(_: renamed_dep::FromPriv) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - +[NOTE] dependency `priv_dep` declared here + --> Cargo.toml:9:61 + | +9 | renamed_dep = {version = "0.1.0", package = "priv_dep" } + | ---------- ... "#]] .unordered(), @@ -922,12 +941,22 @@ fn dependency_location_in_target_table() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = [NOTE] `#[warn(exported_private_dependencies)]` on by default +[NOTE] dependency `dep` declared here + --> Cargo.toml:8:17 | +8 | dep = { version = "0.1.0" } + | --- [WARNING] type `FromPriv` from private dependency 'native_priv_dep' in public interface --> src/lib.rs:5:13 + | 5 | pub fn use_priv(_: renamed_dep::FromPriv) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +[NOTE] dependency `native_priv_dep` declared here + --> Cargo.toml:9:62 + | +9 | renamed_dep = { version = "0.1.0", package = "native_priv_dep" } + | ----------------- ... "#]] .unordered(), @@ -946,12 +975,22 @@ fn dependency_location_in_target_table() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = [NOTE] `#[warn(exported_private_dependencies)]` on by default +[NOTE] dependency `dep` declared here + --> Cargo.toml:12:17 + | +12 | dep = { version = "0.1.0" } + | --- [WARNING] type `FromPriv` from private dependency 'alt_priv_dep' in public interface --> src/lib.rs:5:13 | 5 | pub fn use_priv(_: renamed_dep::FromPriv) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +[NOTE] dependency `alt_priv_dep` declared here + --> Cargo.toml:13:62 + | +13 | renamed_dep = { version = "0.1.0", package = "alt_priv_dep" } + | -------------- ... "#]] .unordered(), @@ -1020,12 +1059,22 @@ fn dependency_location_in_target_table_with_cfg() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = [NOTE] `#[warn(exported_private_dependencies)]` on by default +[NOTE] dependency `dep` declared here + --> Cargo.toml:8:17 | +8 | dep = { version = "0.1.0" } + | --- [WARNING] type `FromPriv` from private dependency 'native_priv_dep' in public interface --> src/lib.rs:5:13 + | 5 | pub fn use_priv(_: renamed_dep::FromPriv) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +[NOTE] dependency `native_priv_dep` declared here + --> Cargo.toml:9:62 + | +9 | renamed_dep = { version = "0.1.0", package = "native_priv_dep" } + | ----------------- ... "#]] .unordered(), @@ -1044,12 +1093,22 @@ fn dependency_location_in_target_table_with_cfg() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = [NOTE] `#[warn(exported_private_dependencies)]` on by default +[NOTE] dependency `dep` declared here + --> Cargo.toml:12:17 + | +12 | dep = { version = "0.1.0" } + | --- [WARNING] type `FromPriv` from private dependency 'alt_priv_dep' in public interface --> src/lib.rs:5:13 | 5 | pub fn use_priv(_: renamed_dep::FromPriv) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +[NOTE] dependency `alt_priv_dep` declared here + --> Cargo.toml:13:62 + | +13 | renamed_dep = { version = "0.1.0", package = "alt_priv_dep" } + | -------------- ... "#]] .unordered(), @@ -1153,6 +1212,12 @@ fn relative_display_path() { 3 | pub use priv_dep::FromPriv; | ^^^^^^^^^^^^^^^^^^ | + = [NOTE] `#[warn(exported_private_dependencies)]` on by default +[NOTE] dependency `priv_dep` declared here + --> foo/Cargo.toml:8:17 + | +8 | priv_dep = "0.1.0" + | -------- ... "#]]) .run();