Skip to content

Commit

Permalink
Auto merge of #13852 - Muscraft:revert-seperating-lints, r=epage
Browse files Browse the repository at this point in the history
fix(lints): Prevent inheritance from bring exposed for published packages

#13843 demonstrated a regression caused by #13801, where we started to keep `[lints]` and `[workspace.lints]` separate, and not truly resolve `[lints]`. This was a nice thing to have and made it easier to tell when a lint came from a workspace. The downside of doing so is the lints table would not get resolved when vendoring or publishing.

To fix this issue, I reverted the change for keeping `[lints]` and `[workspace.lints]` separate and modified how cargo's linting system figures out where a lint is coming from. Due to this change, we no longer specify that a lint was set by `[workspace.lints]`, only `[lints]`. It is true that a lint level is set by `[lints]` always, as it would've had to specify the lint outright or specify that it was inheriting it, seeing that, I do not think this is a regression in diagnostic quality. I still manage to keep the ability to render a lint's location in the workspace's manifest when running ` analyze_cargo_lints_table`, which I am pleased about.
  • Loading branch information
bors committed May 3, 2024
2 parents 97181c6 + 6c23346 commit cf7b3c4
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 215 deletions.
61 changes: 5 additions & 56 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,25 +1149,11 @@ impl<'gctx> Workspace<'gctx> {
}

pub fn emit_warnings(&self) -> 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()
.collect();

for (path, maybe_pkg) in &self.packages.packages {
let path = path.join("Cargo.toml");
if let MaybePackage::Package(pkg) = maybe_pkg {
if self.gctx.cli_unstable().cargo_lints {
self.emit_lints(pkg, &path, &ws_cargo_lints)?
self.emit_lints(pkg, &path)?
}
}
let warnings = match maybe_pkg {
Expand Down Expand Up @@ -1195,12 +1181,7 @@ impl<'gctx> Workspace<'gctx> {
Ok(())
}

pub fn emit_lints(
&self,
pkg: &Package,
path: &Path,
ws_cargo_lints: &manifest::TomlToolLints,
) -> CargoResult<()> {
pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
let mut error_count = 0;
let toml_lints = pkg
.manifest()
Expand All @@ -1214,16 +1195,6 @@ impl<'gctx> Workspace<'gctx> {
.cloned()
.unwrap_or(manifest::TomlToolLints::default());

// We should only be using workspace lints if the `[lints]` table is
// present in the manifest, and `workspace` is set to `true`
let ws_cargo_lints = pkg
.manifest()
.resolved_toml()
.lints
.as_ref()
.is_some_and(|l| l.workspace)
.then(|| ws_cargo_lints);

let ws_contents = match self.root_maybe() {
MaybePackage::Package(pkg) => pkg.manifest().contents(),
MaybePackage::Virtual(v) => v.contents(),
Expand All @@ -1238,36 +1209,14 @@ impl<'gctx> Workspace<'gctx> {
pkg,
&path,
&cargo_lints,
ws_cargo_lints,
ws_contents,
ws_document,
self.root_manifest(),
self.gctx,
)?;
check_im_a_teapot(
pkg,
&path,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
check_implicit_features(
pkg,
&path,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
unused_dependencies(
pkg,
&path,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
check_implicit_features(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
unused_dependencies(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
Loading

0 comments on commit cf7b3c4

Please sign in to comment.