Skip to content
Draft
Show file tree
Hide file tree
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
20 changes: 12 additions & 8 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2000,9 +2000,9 @@ struct ManifestErrorContext {
/// The path to the manifest.
path: PathBuf,
/// The locations of various spans within the manifest.
spans: toml::Spanned<toml::de::DeTable<'static>>,
spans: Option<toml::Spanned<toml::de::DeTable<'static>>>,
/// The raw manifest contents.
contents: String,
contents: Option<String>,
/// A lookup for all the unambiguous renamings, mapping from the original package
/// name to the renamed one.
rename_table: HashMap<InternedString, InternedString>,
Expand Down Expand Up @@ -2117,6 +2117,7 @@ fn on_stderr_line_inner(
static PRIV_DEP_REGEX: LazyLock<Regex> =
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(ref contents) = manifest.contents
&& let Some(span) = manifest.find_crate_span(crate_name.as_str())
{
let rel_path = pathdiff::diff_paths(&manifest.path, &manifest.cwd)
Expand All @@ -2128,7 +2129,7 @@ fn on_stderr_line_inner(
crate_name.as_str()
)))
.element(
Snippet::source(&manifest.contents)
Snippet::source(contents)
.path(rel_path)
.annotation(AnnotationKind::Context.span(span)),
)];
Expand Down Expand Up @@ -2366,8 +2367,8 @@ impl ManifestErrorContext {
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(),
spans: unit.pkg.manifest().document().cloned(),
contents: unit.pkg.manifest().contents().map(String::from),
requested_kinds: bcx.target_data.requested_kinds().to_owned(),
host_name: bcx.rustc().host,
rename_table,
Expand Down Expand Up @@ -2408,9 +2409,13 @@ impl ManifestErrorContext {
/// baz = { path = "../bar", package = "bar" }
/// ```
fn find_crate_span(&self, unrenamed: &str) -> Option<Range<usize>> {
let Some(ref spans) = self.spans else {
return None;
};

let orig_name = self.rename_table.get(unrenamed)?.as_str();

if let Some((k, v)) = get_key_value(&self.spans, &["dependencies", orig_name]) {
if let Some((k, v)) = get_key_value(&spans, &["dependencies", orig_name]) {
// We make some effort to find the unrenamed text: in
//
// ```
Expand All @@ -2430,8 +2435,7 @@ impl ManifestErrorContext {
// [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
if let Some(target) = spans
.as_ref()
.get("target")
.and_then(|t| t.as_ref().as_table())
Expand Down
48 changes: 24 additions & 24 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ impl EitherManifest {
#[derive(Clone, Debug)]
pub struct Manifest {
// alternate forms of manifests:
contents: Rc<String>,
document: Rc<toml::Spanned<toml::de::DeTable<'static>>>,
original_toml: Rc<TomlManifest>,
contents: Option<Rc<String>>,
document: Option<Rc<toml::Spanned<toml::de::DeTable<'static>>>>,
original_toml: Option<Rc<TomlManifest>>,
normalized_toml: Rc<TomlManifest>,
summary: Summary,

Expand Down Expand Up @@ -109,9 +109,9 @@ pub struct Warnings(Vec<DelayedWarning>);
#[derive(Clone, Debug)]
pub struct VirtualManifest {
// alternate forms of manifests:
contents: Rc<String>,
document: Rc<toml::Spanned<toml::de::DeTable<'static>>>,
original_toml: Rc<TomlManifest>,
contents: Option<Rc<String>>,
document: Option<Rc<toml::Spanned<toml::de::DeTable<'static>>>>,
original_toml: Option<Rc<TomlManifest>>,
normalized_toml: Rc<TomlManifest>,

// this form of manifest:
Expand Down Expand Up @@ -496,9 +496,9 @@ compact_debug! {

impl Manifest {
pub fn new(
contents: Rc<String>,
document: Rc<toml::Spanned<toml::de::DeTable<'static>>>,
original_toml: Rc<TomlManifest>,
contents: Option<Rc<String>>,
document: Option<Rc<toml::Spanned<toml::de::DeTable<'static>>>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like removing the .unwrap()s would need to happen before this

original_toml: Option<Rc<TomlManifest>>,
normalized_toml: Rc<TomlManifest>,
summary: Summary,

Expand Down Expand Up @@ -559,21 +559,21 @@ impl Manifest {
}

/// The raw contents of the original TOML
pub fn contents(&self) -> &str {
self.contents.as_str()
pub fn contents(&self) -> Option<&str> {
self.contents.as_deref().map(|c| c.as_str())
}
/// See [`Manifest::normalized_toml`] for what "normalized" means
pub fn to_normalized_contents(&self) -> CargoResult<String> {
let toml = toml::to_string_pretty(self.normalized_toml())?;
Ok(format!("{}\n{}", MANIFEST_PREAMBLE, toml))
}
/// Collection of spans for the original TOML
pub fn document(&self) -> &toml::Spanned<toml::de::DeTable<'static>> {
&self.document
pub fn document(&self) -> Option<&toml::Spanned<toml::de::DeTable<'static>>> {
self.document.as_deref()
}
/// The [`TomlManifest`] as parsed from [`Manifest::document`]
pub fn original_toml(&self) -> &TomlManifest {
&self.original_toml
pub fn original_toml(&self) -> Option<&TomlManifest> {
self.original_toml.as_deref()
}
/// The [`TomlManifest`] with all fields expanded
///
Expand Down Expand Up @@ -744,9 +744,9 @@ impl Manifest {

impl VirtualManifest {
pub fn new(
contents: Rc<String>,
document: Rc<toml::Spanned<toml::de::DeTable<'static>>>,
original_toml: Rc<TomlManifest>,
contents: Option<Rc<String>>,
document: Option<Rc<toml::Spanned<toml::de::DeTable<'static>>>>,
original_toml: Option<Rc<TomlManifest>>,
normalized_toml: Rc<TomlManifest>,
replace: Vec<(PackageIdSpec, Dependency)>,
patch: HashMap<Url, Vec<Dependency>>,
Expand All @@ -769,16 +769,16 @@ impl VirtualManifest {
}

/// The raw contents of the original TOML
pub fn contents(&self) -> &str {
self.contents.as_str()
pub fn contents(&self) -> Option<&str> {
self.contents.as_deref().map(|c| c.as_str())
}
/// Collection of spans for the original TOML
pub fn document(&self) -> &toml::Spanned<toml::de::DeTable<'static>> {
&self.document
pub fn document(&self) -> Option<&toml::Spanned<toml::de::DeTable<'static>>> {
self.document.as_deref()
}
/// The [`TomlManifest`] as parsed from [`VirtualManifest::document`]
pub fn original_toml(&self) -> &TomlManifest {
&self.original_toml
pub fn original_toml(&self) -> Option<&TomlManifest> {
self.original_toml.as_deref()
}
/// The [`TomlManifest`] with all fields expanded
pub fn normalized_toml(&self) -> &TomlManifest {
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1985,14 +1985,14 @@ impl MaybePackage {
}
}

pub fn contents(&self) -> &str {
pub fn contents(&self) -> Option<&str> {
match self {
MaybePackage::Package(p) => p.manifest().contents(),
MaybePackage::Virtual(v) => v.contents(),
}
}

pub fn document(&self) -> &toml::Spanned<toml::de::DeTable<'static>> {
pub fn document(&self) -> Option<&toml::Spanned<toml::de::DeTable<'static>>> {
match self {
MaybePackage::Package(p) => p.manifest().document(),
MaybePackage::Virtual(v) => v.document(),
Expand Down
29 changes: 17 additions & 12 deletions src/cargo/lints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::core::{Edition, Feature, Features, MaybePackage, Package};
use crate::{CargoResult, GlobalContext};

use annotate_snippets::AnnotationKind;
use annotate_snippets::Group;
use annotate_snippets::Level;
use annotate_snippets::Snippet;
use cargo_util_schemas::manifest::TomlLintLevel;
Expand Down Expand Up @@ -31,14 +32,14 @@ impl ManifestFor<'_> {
lint.level(pkg_lints, self.edition(), self.unstable_features())
}

pub fn contents(&self) -> &str {
pub fn contents(&self) -> Option<&str> {
match self {
ManifestFor::Package(p) => p.manifest().contents(),
ManifestFor::Workspace(p) => p.contents(),
}
}

pub fn document(&self) -> &toml::Spanned<toml::de::DeTable<'static>> {
pub fn document(&self) -> Option<&toml::Spanned<toml::de::DeTable<'static>>> {
match self {
ManifestFor::Package(p) => p.manifest().document(),
ManifestFor::Workspace(p) => p.document(),
Expand Down Expand Up @@ -164,8 +165,6 @@ fn report_feature_not_enabled(
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
let document = manifest.document();
let contents = manifest.contents();
let dash_feature_name = feature_gate.name().replace("_", "-");
let title = format!("use of unstable lint `{}`", lint_name);
let label = format!(
Expand All @@ -181,19 +180,25 @@ fn report_feature_not_enabled(
ManifestFor::Package(_) => &["lints", "cargo", lint_name][..],
ManifestFor::Workspace(_) => &["workspace", "lints", "cargo", lint_name][..],
};
let Some(span) = get_key_value_span(document, key_path) else {
// This lint is handled by either package or workspace lint.
return Ok(());
};

let report = [Level::ERROR
.primary_title(title)
.element(
let mut error = Group::with_title(Level::ERROR.primary_title(title));

if let Some(document) = manifest.document()
&& let Some(contents) = manifest.contents()
{
let Some(span) = get_key_value_span(document, key_path) else {
// This lint is handled by either package or workspace lint.
return Ok(());
};

error = error.element(
Snippet::source(contents)
.path(manifest_path)
.annotation(AnnotationKind::Primary.span(span.key).label(label)),
)
.element(Level::HELP.message(help))];
}

let report = [error.element(Level::HELP.message(help))];

*error_count += 1;
gctx.shell().print_report(&report, true)?;
Expand Down
69 changes: 40 additions & 29 deletions src/cargo/lints/rules/blanket_hint_mostly_unused.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::path::Path;

use annotate_snippets::AnnotationKind;
use annotate_snippets::Group;
use annotate_snippets::Level;
use annotate_snippets::Patch;
use annotate_snippets::Snippet;
Expand Down Expand Up @@ -120,45 +121,55 @@ pub fn blanket_hint_mostly_unused(
let title = "`hint-mostly-unused` is being blanket applied to all dependencies";
let help_txt =
"scope `hint-mostly-unused` to specific packages with a lot of unused object code";
if let (Some(span), Some(table_span)) = (
get_key_value_span(maybe_pkg.document(), &path),
get_key_value_span(maybe_pkg.document(), &path[..path.len() - 1]),
) {
let mut report = Vec::new();
let mut primary_group = level.clone().primary_title(title).element(
Snippet::source(maybe_pkg.contents())
let mut report = Vec::new();
let mut primary_group = Group::with_title(level.clone().primary_title(title));

if let Some(contents) = maybe_pkg.contents()
&& let Some(document) = maybe_pkg.document()
&& let Some(span) = get_key_value_span(document, &path)
&& let Some(table_span) = get_key_value_span(document, &path[..path.len() - 1])
{
primary_group = primary_group.element(
Snippet::source(contents)
.path(&manifest_path)
.annotation(
AnnotationKind::Primary.span(table_span.key.start..table_span.key.end),
)
.annotation(AnnotationKind::Context.span(span.key.start..span.value.end)),
);
}

if *show_per_pkg_suggestion {
report.push(
Level::HELP.secondary_title(help_txt).element(
Snippet::source(maybe_pkg.contents())
.path(&manifest_path)
.patch(Patch::new(
table_span.key.end..table_span.key.end,
".package.<pkg_name>",
)),
),
);
} else {
primary_group = primary_group.element(Level::HELP.message(help_txt));
}
if *show_per_pkg_suggestion {
let help_group = Group::with_title(Level::HELP.secondary_title(help_txt));

report.push(
if let Some(contents) = maybe_pkg.contents()
&& let Some(document) = maybe_pkg.document()
&& let Some(table_span) = get_key_value_span(document, &path[..path.len() - 1])
{
help_group.element(Snippet::source(contents).path(&manifest_path).patch(
Patch::new(
table_span.key.end..table_span.key.end,
".package.<pkg_name>",
),
))
} else {
help_group
},
);
} else {
primary_group = primary_group.element(Level::HELP.message(help_txt));
}

if i == 0 {
primary_group = primary_group
.element(Level::NOTE.message(LINT.emitted_source(lint_level, reason)));
}
if i == 0 {
primary_group =
primary_group.element(Level::NOTE.message(LINT.emitted_source(lint_level, reason)));
}

// The primary group should always be first
report.insert(0, primary_group);
// The primary group should always be first
report.insert(0, primary_group);

gctx.shell().print_report(&report, lint_level.force())?;
}
gctx.shell().print_report(&report, lint_level.force())?;
}

Ok(())
Expand Down
17 changes: 11 additions & 6 deletions src/cargo/lints/rules/im_a_teapot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,20 @@ pub fn check_im_a_teapot(
let manifest_path = rel_cwd_manifest_path(path, gctx);
let emitted_reason = LINT.emitted_source(lint_level, reason);

let span = get_key_value_span(manifest.document(), &["package", "im-a-teapot"]).unwrap();
let mut desc = Group::with_title(level.primary_title(LINT.desc));

let report = &[Group::with_title(level.primary_title(LINT.desc))
.element(
Snippet::source(manifest.contents())
if let Some(document) = manifest.document()
&& let Some(contents) = manifest.contents()
{
let span = get_key_value_span(document, &["package", "im-a-teapot"]).unwrap();
desc = desc.element(
Snippet::source(contents)
.path(&manifest_path)
.annotation(AnnotationKind::Primary.span(span.key.start..span.value.end)),
)
.element(Level::NOTE.message(&emitted_reason))];
);
}

let report = &[desc.element(Level::NOTE.message(&emitted_reason))];

gctx.shell().print_report(report, lint_level.force())?;
}
Expand Down
Loading