Skip to content
Merged
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
150 changes: 116 additions & 34 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ use crate::util::context::FeatureUnification;
use crate::util::edit_distance;
use crate::util::errors::{CargoResult, ManifestError};
use crate::util::interning::InternedString;
use crate::util::lints::{analyze_cargo_lints_table, check_im_a_teapot};
use crate::util::lints::{
analyze_cargo_lints_table, blanket_hint_mostly_unused, check_im_a_teapot,
};
use crate::util::toml::{InheritableFields, read_manifest};
use crate::util::{
Filesystem, GlobalContext, IntoUrl, context::CargoResolverConfig, context::ConfigRelativePath,
Expand Down Expand Up @@ -409,10 +411,7 @@ impl<'gctx> Workspace<'gctx> {
}

pub fn profiles(&self) -> Option<&TomlProfiles> {
match self.root_maybe() {
MaybePackage::Package(p) => p.manifest().profiles(),
MaybePackage::Virtual(vm) => vm.profiles(),
}
self.root_maybe().profiles()
}

/// Returns the root path of this workspace.
Expand Down Expand Up @@ -907,10 +906,7 @@ impl<'gctx> Workspace<'gctx> {

/// Returns the unstable nightly-only features enabled via `cargo-features` in the manifest.
pub fn unstable_features(&self) -> &Features {
match self.root_maybe() {
MaybePackage::Package(p) => p.manifest().unstable_features(),
MaybePackage::Virtual(vm) => vm.unstable_features(),
}
self.root_maybe().unstable_features()
}

pub fn resolve_behavior(&self) -> ResolveBehavior {
Expand Down Expand Up @@ -1206,14 +1202,17 @@ impl<'gctx> Workspace<'gctx> {

pub fn emit_warnings(&self) -> CargoResult<()> {
let mut first_emitted_error = None;

if let Err(e) = self.emit_ws_lints() {
first_emitted_error = Some(e);
}

for (path, maybe_pkg) in &self.packages.packages {
if let MaybePackage::Package(pkg) = maybe_pkg {
if self.gctx.cli_unstable().cargo_lints {
if let Err(e) = self.emit_lints(pkg, &path)
&& first_emitted_error.is_none()
{
first_emitted_error = Some(e);
}
if let Err(e) = self.emit_pkg_lints(pkg, &path)
&& first_emitted_error.is_none()
{
first_emitted_error = Some(e);
}
}
let warnings = match maybe_pkg {
Expand Down Expand Up @@ -1248,7 +1247,7 @@ impl<'gctx> Workspace<'gctx> {
}
}

pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
pub fn emit_pkg_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
let mut error_count = 0;
let toml_lints = pkg
.manifest()
Expand All @@ -1262,26 +1261,74 @@ impl<'gctx> Workspace<'gctx> {
.cloned()
.unwrap_or(manifest::TomlToolLints::default());

let ws_contents = match self.root_maybe() {
MaybePackage::Package(pkg) => pkg.manifest().contents(),
MaybePackage::Virtual(v) => v.contents(),
};
let ws_contents = self.root_maybe().contents();

let ws_document = match self.root_maybe() {
MaybePackage::Package(pkg) => pkg.manifest().document(),
MaybePackage::Virtual(v) => v.document(),
};
let ws_document = self.root_maybe().document();

if self.gctx.cli_unstable().cargo_lints {
analyze_cargo_lints_table(
pkg,
&path,
&cargo_lints,
ws_contents,
ws_document,
self.root_manifest(),
self.gctx,
)?;
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
}

if error_count > 0 {
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
"encountered {error_count} errors(s) while running lints"
))
.into())
} else {
Ok(())
}
}

pub fn emit_ws_lints(&self) -> CargoResult<()> {
let mut error_count = 0;

let cargo_lints = match self.root_maybe() {
MaybePackage::Package(pkg) => {
let toml = pkg.manifest().normalized_toml();
if let Some(ws) = &toml.workspace {
ws.lints.as_ref()
} else {
toml.lints.as_ref().map(|l| &l.lints)
}
}
MaybePackage::Virtual(vm) => vm
.normalized_toml()
.workspace
.as_ref()
.unwrap()
.lints
.as_ref(),
}
.and_then(|t| t.get("cargo"))
.cloned()
.unwrap_or(manifest::TomlToolLints::default());

if self.gctx.cli_unstable().cargo_lints {
// Calls to lint functions go in here
}

// This is a short term hack to allow `blanket_hint_mostly_unused`
// to run without requiring `-Zcargo-lints`, which should hopefully
// improve the testing expierience while we are collecting feedback
if self.gctx.cli_unstable().profile_hint_mostly_unused {
blanket_hint_mostly_unused(
self.root_maybe(),
self.root_manifest(),
&cargo_lints,
&mut error_count,
self.gctx,
)?;
}

analyze_cargo_lints_table(
pkg,
&path,
&cargo_lints,
ws_contents,
ws_document,
self.root_manifest(),
self.gctx,
)?;
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
if error_count > 0 {
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
"encountered {error_count} errors(s) while running lints"
Expand Down Expand Up @@ -1888,6 +1935,41 @@ impl MaybePackage {
MaybePackage::Virtual(_) => false,
}
}

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

pub fn document(&self) -> &toml::Spanned<toml::de::DeTable<'static>> {
match self {
MaybePackage::Package(p) => p.manifest().document(),
MaybePackage::Virtual(v) => v.document(),
}
}

pub fn edition(&self) -> Edition {
match self {
MaybePackage::Package(p) => p.manifest().edition(),
MaybePackage::Virtual(_) => Edition::default(),
}
}

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

pub fn unstable_features(&self) -> &Features {
match self {
MaybePackage::Package(p) => p.manifest().unstable_features(),
MaybePackage::Virtual(vm) => vm.unstable_features(),
}
}
}

impl WorkspaceRootConfig {
Expand Down
160 changes: 156 additions & 4 deletions src/cargo/util/lints.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use crate::core::{Edition, Feature, Features, Manifest, Package};
use crate::core::{Edition, Feature, Features, Manifest, MaybePackage, Package};
use crate::{CargoResult, GlobalContext};
use annotate_snippets::{AnnotationKind, Group, Level, Snippet};
use cargo_util_schemas::manifest::{TomlLintLevel, TomlToolLints};
use annotate_snippets::{AnnotationKind, Group, Level, Patch, Snippet};
use cargo_util_schemas::manifest::{ProfilePackageSpec, TomlLintLevel, TomlToolLints};
use pathdiff::diff_paths;
use std::fmt::Display;
use std::ops::Range;
use std::path::Path;

const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE];
pub const LINTS: &[Lint] = &[IM_A_TEAPOT, UNKNOWN_LINTS];
pub const LINTS: &[Lint] = &[BLANKET_HINT_MOSTLY_UNUSED, IM_A_TEAPOT, UNKNOWN_LINTS];

pub fn analyze_cargo_lints_table(
pkg: &Package,
Expand Down Expand Up @@ -473,6 +473,158 @@ pub fn check_im_a_teapot(
Ok(())
}

const BLANKET_HINT_MOSTLY_UNUSED: Lint = Lint {
name: "blanket_hint_mostly_unused",
desc: "blanket_hint_mostly_unused lint",
groups: &[],
default_level: LintLevel::Warn,
edition_lint_opts: None,
feature_gate: None,
docs: Some(
r#"
### What it does
Checks if `hint-mostly-unused` being applied to all dependencies.

### Why it is bad
`hint-mostly-unused` indicates that most of a crate's API surface will go
unused by anything depending on it; this hint can speed up the build by
attempting to minimize compilation time for items that aren't used at all.
Misapplication to crates that don't fit that criteria will slow down the build
rather than speeding it up. It should be selectively applied to dependencies
that meet these criteria. Applying it globally is always a misapplication and
will likely slow down the build.

### Example
```toml
[profile.dev.package."*"]
hint-mostly-unused = true
```

Should instead be:
```toml
[profile.dev.package.huge-mostly-unused-dependency]
hint-mostly-unused = true
```
"#,
),
};

pub fn blanket_hint_mostly_unused(
maybe_pkg: &MaybePackage,
path: &Path,
pkg_lints: &TomlToolLints,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
let (lint_level, reason) = BLANKET_HINT_MOSTLY_UNUSED.level(
pkg_lints,
maybe_pkg.edition(),
maybe_pkg.unstable_features(),
);

if lint_level == LintLevel::Allow {
return Ok(());
}

let level = lint_level.to_diagnostic_level();
let manifest_path = rel_cwd_manifest_path(path, gctx);
let mut paths = Vec::new();

if let Some(profiles) = maybe_pkg.profiles() {
for (profile_name, top_level_profile) in &profiles.0 {
if let Some(true) = top_level_profile.hint_mostly_unused {
paths.push((
vec!["profile", profile_name.as_str(), "hint-mostly-unused"],
true,
));
}

if let Some(build_override) = &top_level_profile.build_override
&& let Some(true) = build_override.hint_mostly_unused
{
paths.push((
vec![
"profile",
profile_name.as_str(),
"build-override",
"hint-mostly-unused",
],
false,
));
}

if let Some(packages) = &top_level_profile.package
&& let Some(profile) = packages.get(&ProfilePackageSpec::All)
&& let Some(true) = profile.hint_mostly_unused
{
paths.push((
vec![
"profile",
profile_name.as_str(),
"package",
"*",
"hint-mostly-unused",
],
false,
));
}
}
}

for (i, (path, show_per_pkg_suggestion)) in paths.iter().enumerate() {
if lint_level.is_error() {
*error_count += 1;
}
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())
.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 i == 0 {
primary_group =
primary_group
.element(Level::NOTE.message(
BLANKET_HINT_MOSTLY_UNUSED.emitted_source(lint_level, reason),
));
}

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

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

Ok(())
}

const UNKNOWN_LINTS: Lint = Lint {
name: "unknown_lints",
desc: "unknown lint",
Expand Down
Loading
Loading