Skip to content

Commit

Permalink
Auto merge of #13801 - Muscraft:add-lint-reason, r=epage
Browse files Browse the repository at this point in the history 
Add where lint was set

`rustc` and `clippy` both show why the lint was emitted and where the level was set the first time it was emitted for a package. We already showed why the list was being emitted but did not show where the lint level was set. This PR adds where the lint was set at.
  • Loading branch information
@bors
bors committed Apr 25, 2024
2 parents 955503e + dfc9bd2 commit 93edfb9
Show file tree
Hide file tree
Showing 7 changed files with 207 additions and 71 deletions.
42 changes: 39 additions & 3 deletions src/cargo/core/workspace.rs
View file
Expand Up @@ -1180,6 +1180,21 @@ impl<'gctx> Workspace<'gctx> {
}

pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
let ws_lints = self
.root_maybe()
.workspace_config()
.inheritable()
.and_then(|i| i.lints().ok())
.unwrap_or_default();

let ws_cargo_lints = ws_lints
.get("cargo")
.cloned()
.unwrap_or_default()
.into_iter()
.map(|(k, v)| (k.replace('-', "_"), v))
.collect();

let mut error_count = 0;
let toml_lints = pkg
.manifest()
Expand All @@ -1197,9 +1212,30 @@ impl<'gctx> Workspace<'gctx> {
.map(|(name, lint)| (name.replace('-', "_"), lint))
.collect();

check_im_a_teapot(pkg, &path, &normalized_lints, &mut error_count, self.gctx)?;
check_implicit_features(pkg, &path, &normalized_lints, &mut error_count, self.gctx)?;
unused_dependencies(pkg, &path, &normalized_lints, &mut error_count, self.gctx)?;
check_im_a_teapot(
pkg,
&path,
&normalized_lints,
&ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
check_implicit_features(
pkg,
&path,
&normalized_lints,
&ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
unused_dependencies(
pkg,
&path,
&normalized_lints,
&ws_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
145 changes: 105 additions & 40 deletions src/cargo/util/lints.rs
View file
Expand Up @@ -85,40 +85,41 @@ pub struct Lint {
}

impl Lint {
pub fn level(&self, lints: &TomlToolLints, edition: Edition) -> LintLevel {
let edition_level = self
.edition_lint_opts
.filter(|(e, _)| edition >= *e)
.map(|(_, l)| l);

if self.default_level == LintLevel::Forbid || edition_level == Some(LintLevel::Forbid) {
return LintLevel::Forbid;
}

let level = self
.groups
pub fn level(
&self,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
edition: Edition,
) -> (LintLevel, LintLevelReason) {
self.groups
.iter()
.map(|g| g.name)
.chain(std::iter::once(self.name))
.filter_map(|n| lints.get(n).map(|l| (n, l)))
.max_by_key(|(n, l)| {
.map(|g| {
(
l.level() == TomlLintLevel::Forbid,
l.priority(),
std::cmp::Reverse(*n),
g.name,
level_priority(
g.name,
g.default_level,
g.edition_lint_opts,
pkg_lints,
ws_lints,
edition,
),
)
});

match level {
Some((_, toml_lint)) => toml_lint.level().into(),
None => {
if let Some(level) = edition_level {
level
} else {
self.default_level
}
}
}
})
.chain(std::iter::once((
self.name,
level_priority(
self.name,
self.default_level,
self.edition_lint_opts,
pkg_lints,
ws_lints,
edition,
),
)))
.max_by_key(|(n, (l, _, p))| (l == &LintLevel::Forbid, *p, std::cmp::Reverse(*n)))
.map(|(_, (l, r, _))| (l, r))
.unwrap()
}
}

Expand Down Expand Up @@ -163,6 +164,64 @@ impl From<TomlLintLevel> for LintLevel {
}
}

#[derive(Copy, Clone, Debug)]
pub enum LintLevelReason {
Default,
Edition(Edition),
Package,
Workspace,
}

impl Display for LintLevelReason {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
LintLevelReason::Default => write!(f, "by default"),
LintLevelReason::Edition(edition) => write!(f, "in edition {}", edition),
LintLevelReason::Package => write!(f, "in `[lints]`"),
LintLevelReason::Workspace => write!(f, "in `[workspace.lints]`"),
}
}
}

fn level_priority(
name: &str,
default_level: LintLevel,
edition_lint_opts: Option<(Edition, LintLevel)>,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
edition: Edition,
) -> (LintLevel, LintLevelReason, i8) {
let (unspecified_level, reason) = if let Some(level) = edition_lint_opts
.filter(|(e, _)| edition >= *e)
.map(|(_, l)| l)
{
(level, LintLevelReason::Edition(edition))
} else {
(default_level, LintLevelReason::Default)
};

// Don't allow the group to be overridden if the level is `Forbid`
if unspecified_level == LintLevel::Forbid {
return (unspecified_level, reason, 0);
}

if let Some(defined_level) = pkg_lints.get(name) {
(
defined_level.level().into(),
LintLevelReason::Package,
defined_level.priority(),
)
} else if let Some(defined_level) = ws_lints.get(name) {
(
defined_level.level().into(),
LintLevelReason::Workspace,
defined_level.priority(),
)
} else {
(unspecified_level, reason, 0)
}
}

const IM_A_TEAPOT: Lint = Lint {
name: "im_a_teapot",
desc: "`im_a_teapot` is specified",
Expand All @@ -174,12 +233,13 @@ const IM_A_TEAPOT: Lint = Lint {
pub fn check_im_a_teapot(
pkg: &Package,
path: &Path,
lints: &TomlToolLints,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
let manifest = pkg.manifest();
let lint_level = IM_A_TEAPOT.level(lints, manifest.edition());
let (lint_level, reason) = IM_A_TEAPOT.level(pkg_lints, ws_lints, manifest.edition());
if lint_level == LintLevel::Allow {
return Ok(());
}
Expand All @@ -194,7 +254,10 @@ pub fn check_im_a_teapot(
}
let level = lint_level.to_diagnostic_level();
let manifest_path = rel_cwd_manifest_path(path, gctx);
let emitted_reason = format!("`cargo::{}` is set to `{lint_level}`", IM_A_TEAPOT.name);
let emitted_reason = format!(
"`cargo::{}` is set to `{lint_level}` {reason}",
IM_A_TEAPOT.name
);

let key_span = get_span(manifest.document(), &["package", "im-a-teapot"], false).unwrap();
let value_span = get_span(manifest.document(), &["package", "im-a-teapot"], true).unwrap();
Expand Down Expand Up @@ -242,7 +305,8 @@ const IMPLICIT_FEATURES: Lint = Lint {
pub fn check_implicit_features(
pkg: &Package,
path: &Path,
lints: &TomlToolLints,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
Expand All @@ -253,7 +317,7 @@ pub fn check_implicit_features(
return Ok(());
}

let lint_level = IMPLICIT_FEATURES.level(lints, edition);
let (lint_level, reason) = IMPLICIT_FEATURES.level(pkg_lints, ws_lints, edition);
if lint_level == LintLevel::Allow {
return Ok(());
}
Expand Down Expand Up @@ -298,7 +362,7 @@ pub fn check_implicit_features(
);
if emitted_source.is_none() {
emitted_source = Some(format!(
"`cargo::{}` is set to `{lint_level}`",
"`cargo::{}` is set to `{lint_level}` {reason}",
IMPLICIT_FEATURES.name
));
message = message.footer(Level::Note.title(emitted_source.as_ref().unwrap()));
Expand All @@ -325,7 +389,8 @@ const UNUSED_OPTIONAL_DEPENDENCY: Lint = Lint {
pub fn unused_dependencies(
pkg: &Package,
path: &Path,
lints: &TomlToolLints,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
Expand All @@ -335,7 +400,7 @@ pub fn unused_dependencies(
return Ok(());
}

let lint_level = UNUSED_OPTIONAL_DEPENDENCY.level(lints, edition);
let (lint_level, reason) = UNUSED_OPTIONAL_DEPENDENCY.level(pkg_lints, ws_lints, edition);
if lint_level == LintLevel::Allow {
return Ok(());
}
Expand Down Expand Up @@ -401,7 +466,7 @@ pub fn unused_dependencies(
);
if emitted_source.is_none() {
emitted_source = Some(format!(
"`cargo::{}` is set to `{lint_level}`",
"`cargo::{}` is set to `{lint_level}` {reason}",
UNUSED_OPTIONAL_DEPENDENCY.name
));
message =
Expand Down
36 changes: 14 additions & 22 deletions src/cargo/util/toml/mod.rs
View file
Expand Up @@ -411,15 +411,7 @@ fn resolve_toml(
}
resolved_toml.target = (!resolved_target.is_empty()).then_some(resolved_target);

let resolved_lints = original_toml
.lints
.clone()
.map(|value| lints_inherit_with(value, || inherit()?.lints()))
.transpose()?;
resolved_toml.lints = resolved_lints.map(|lints| manifest::InheritableLints {
workspace: false,
lints,
});
resolved_toml.lints = original_toml.lints.clone();

let resolved_badges = original_toml
.badges
Expand Down Expand Up @@ -803,7 +795,7 @@ impl InheritableFields {
}

/// Gets the field `workspace.lint`.
fn lints(&self) -> CargoResult<manifest::TomlLints> {
pub fn lints(&self) -> CargoResult<manifest::TomlLints> {
let Some(val) = &self.lints else {
bail!("`workspace.lints` was not defined");
};
Expand Down Expand Up @@ -1276,18 +1268,18 @@ fn to_real_manifest(
}
}

verify_lints(
resolved_toml.resolved_lints().expect("previously resolved"),
gctx,
warnings,
)?;
let default = manifest::TomlLints::default();
let rustflags = lints_to_rustflags(
resolved_toml
.resolved_lints()
.expect("previously resolved")
.unwrap_or(&default),
);
let resolved_lints = resolved_toml
.lints
.clone()
.map(|value| {
lints_inherit_with(value, || {
load_inheritable_fields(gctx, manifest_file, &workspace_config)?.lints()
})
})
.transpose()?;

verify_lints(resolved_lints.as_ref(), gctx, warnings)?;
let rustflags = lints_to_rustflags(&resolved_lints.unwrap_or_default());

let metadata = ManifestMetadata {
description: resolved_package
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/lints/implicit_features/edition_2021_warn/stderr.term.svg
View file
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion tests/testsuite/lints/unused_optional_dependencies/edition_2024/stderr.term.svg
View file
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion tests/testsuite/lints/unused_optional_dependencies/renamed_deps/stderr.term.svg
View file
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 93edfb9

Please sign in to comment.