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 committed Apr 25, 2024
2 parents 955503e + dfc9bd2 commit 5f13038
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading

0 comments on commit 5f13038

Please sign in to comment.