diff --git a/src/cargo/core/compiler/job_queue/mod.rs b/src/cargo/core/compiler/job_queue/mod.rs index ad0c3e37844..563328d48df 100644 --- a/src/cargo/core/compiler/job_queue/mod.rs +++ b/src/cargo/core/compiler/job_queue/mod.rs @@ -850,6 +850,7 @@ impl<'gctx> DrainState<'gctx> { &mut global_stats, )); errors.count += global_stats.error_count(); + build_runner.compilation.lint_warning_count += global_stats.lint_warning_count(); } let profile_name = build_runner.bcx.build_config.requested_profile; diff --git a/src/cargo/diagnostics/lint.rs b/src/cargo/diagnostics/lint.rs index 4be36552136..db9b9c37b34 100644 --- a/src/cargo/diagnostics/lint.rs +++ b/src/cargo/diagnostics/lint.rs @@ -5,6 +5,8 @@ use cargo_util_schemas::manifest; use cargo_util_terminal::report::Level; use crate::core::{Feature, Features}; +use crate::util::GlobalContext; +use crate::util::context::WarningHandling; #[derive(Clone, Debug)] pub struct Lint { @@ -30,6 +32,7 @@ impl Lint { pkg_lints: &manifest::TomlToolLints, pkg_rust_version: Option<&manifest::RustVersion>, unstable_features: &Features, + gctx: &GlobalContext, ) -> LintLevelProduct { // We should return `Allow` if a lint is behind a feature, but it is // not enabled, that way the lint does not run. @@ -83,6 +86,15 @@ impl Lint { ) }) .unwrap(); + + let (level, source) = match (level, gctx.warning_handling().ok()) { + // `Deny` needs to be handled later, at the end of the operation + (LintLevel::Warn, Some(WarningHandling::Allow)) => { + (LintLevel::Allow, LintLevelSource::Default) + } + _ => (level, source), + }; + LintLevelProduct { level, source } } @@ -215,6 +227,15 @@ mod tests { hidden: false, }; + fn gctx() -> GlobalContext { + let cwd = std::env::current_dir().unwrap(); + GlobalContext::new( + cargo_util_terminal::Shell::new(), + cwd.clone(), + home::cargo_home_with_cwd(&cwd).unwrap(), + ) + } + fn test_lint(name: &'static str, group: &'static LintGroup) -> Lint { Lint { name, @@ -237,7 +258,7 @@ mod tests { ); let features = Features::default(); - let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features); + let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features, &gctx()); assert_eq!(level, LintLevel::Deny); assert_eq!(source, LintLevelSource::Package); } @@ -253,7 +274,7 @@ mod tests { ); let features = Features::default(); - let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features); + let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features, &gctx()); assert_eq!(level, LintLevel::Deny); assert_eq!(source, LintLevelSource::Package); } @@ -269,7 +290,7 @@ mod tests { ); let features = Features::default(); - let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features); + let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features, &gctx()); assert_eq!(level, LintLevel::Deny); assert_eq!(source, LintLevelSource::Package); } @@ -289,7 +310,7 @@ mod tests { ); let features = Features::default(); - let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features); + let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features, &gctx()); assert_eq!(level, LintLevel::Deny); assert_eq!(source, LintLevelSource::Package); } @@ -309,7 +330,7 @@ mod tests { ); let features = Features::default(); - let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features); + let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features, &gctx()); assert_eq!(level, LintLevel::Deny); assert_eq!(source, LintLevelSource::Package); } @@ -337,7 +358,7 @@ mod tests { ); let features = Features::default(); - let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features); + let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features, &gctx()); assert_eq!(level, LintLevel::Deny); assert_eq!(source, LintLevelSource::Package); } @@ -365,7 +386,7 @@ mod tests { ); let features = Features::default(); - let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features); + let LintLevelProduct { level, source } = lint.level(&pkg_lints, None, &features, &gctx()); assert_eq!(level, LintLevel::Allow); assert_eq!(source, LintLevelSource::Package); } diff --git a/src/cargo/diagnostics/mod.rs b/src/cargo/diagnostics/mod.rs index 2186bc21fa9..82381ab8274 100644 --- a/src/cargo/diagnostics/mod.rs +++ b/src/cargo/diagnostics/mod.rs @@ -72,19 +72,26 @@ pub use lint::{Lint, LintGroup, LintLevel, LintLevelProduct, LintLevelSource}; pub use report::{AsIndex, cwd_rel_path, get_key_value, get_key_value_span, workspace_rel_path}; pub use rules::{LINT_GROUPS, LINTS}; +pub struct PassOutput { + pub lint_warning_count: usize, +} + pub struct GlobalDiagnosticStats { error_count: usize, + lint_warning_count: usize, } impl GlobalDiagnosticStats { pub fn new() -> Self { - Self { error_count: 0 } + Self { + error_count: 0, + lint_warning_count: 0, + } } pub fn scope(&mut self) -> ScopedDiagnosticStats<'_> { ScopedDiagnosticStats { warning_count: 0, - lint_warning_count: 0, error_count: 0, global: self, } @@ -94,29 +101,30 @@ impl GlobalDiagnosticStats { self.error_count } - pub fn ok(&self) -> CargoResult<()> { + pub fn lint_warning_count(&self) -> usize { + self.lint_warning_count + } + + pub fn ok(&self) -> CargoResult { if 0 < self.error_count { Err(crate::Error::new(crate::AlreadyPrintedError::new( anyhow::format_err!("see above"), ))) } else { - Ok(()) + Ok(PassOutput { + lint_warning_count: self.lint_warning_count, + }) } } } pub struct ScopedDiagnosticStats<'g> { warning_count: usize, - lint_warning_count: usize, error_count: usize, global: &'g mut GlobalDiagnosticStats, } impl ScopedDiagnosticStats<'_> { - pub fn lint_warning_count(&self) -> usize { - self.lint_warning_count - } - pub fn warning_count(&self) -> usize { self.warning_count } @@ -140,7 +148,7 @@ impl ScopedDiagnosticStats<'_> { self.record_error(); } LintLevel::Warn => { - self.lint_warning_count += 1; + self.global.lint_warning_count += 1; self.record_warning(); } LintLevel::Allow => {} @@ -194,8 +202,18 @@ pub enum ManifestFor<'a> { } impl ManifestFor<'_> { - fn lint_level(&self, pkg_lints: &TomlToolLints, lint: &Lint) -> LintLevelProduct { - lint.level(pkg_lints, self.rust_version(), self.unstable_features()) + fn lint_level( + &self, + pkg_lints: &TomlToolLints, + lint: &Lint, + gctx: &GlobalContext, + ) -> LintLevelProduct { + lint.level( + pkg_lints, + self.rust_version(), + self.unstable_features(), + gctx, + ) } pub fn rust_version(&self) -> Option<&RustVersion> { diff --git a/src/cargo/diagnostics/passes.rs b/src/cargo/diagnostics/passes.rs index 35bc90ab18a..7575fc63250 100644 --- a/src/cargo/diagnostics/passes.rs +++ b/src/cargo/diagnostics/passes.rs @@ -12,6 +12,7 @@ use crate::diagnostics::Lint; use crate::diagnostics::LintLevel; use crate::diagnostics::LintLevelProduct; use crate::diagnostics::ManifestFor; +use crate::diagnostics::PassOutput; use crate::diagnostics::ScopedDiagnosticStats; #[derive(Clone)] @@ -93,7 +94,7 @@ type FnLintPackage = fn( pub fn emit_parse_diagnostics( workspace: &Workspace<'_>, rules: &[ParsePassRule<'_>], -) -> CargoResult<()> { +) -> CargoResult { let mut stats = GlobalDiagnosticStats::new(); if is_local_workspace(workspace) { @@ -154,7 +155,7 @@ fn emit_parse_pkg_diagnostics( ParsePassRule::LintManifest { rule, lint } => { if workspace.gctx().cli_unstable().cargo_lints { let manifest: ManifestFor<'_> = pkg.into(); - let level = manifest.lint_level(&cargo_lints, lint); + let level = manifest.lint_level(&cargo_lints, lint, workspace.gctx()); if level.level != LintLevel::Allow { rule( workspace, @@ -177,6 +178,7 @@ fn emit_parse_pkg_diagnostics( &cargo_lints, pkg.rust_version(), pkg.manifest().unstable_features(), + workspace.gctx(), ); if level.level != LintLevel::Allow { @@ -242,7 +244,7 @@ fn emit_parse_ws_diagnostics( ParsePassRule::LintManifest { rule, lint } => { if workspace.gctx().cli_unstable().cargo_lints { let manifest: ManifestFor<'_> = (workspace, workspace.root_maybe()).into(); - let level = manifest.lint_level(&cargo_lints, lint); + let level = manifest.lint_level(&cargo_lints, lint, workspace.gctx()); if level.level != LintLevel::Allow { rule( workspace, @@ -270,6 +272,7 @@ fn emit_parse_ws_diagnostics( &cargo_lints, workspace.lowest_rust_version(), workspace.root_maybe().unstable_features(), + workspace.gctx(), ); if level.level != LintLevel::Allow { rule( diff --git a/src/cargo/diagnostics/rules/unused_dependencies.rs b/src/cargo/diagnostics/rules/unused_dependencies.rs index 8bce4e2ee2e..64ff8e63a55 100644 --- a/src/cargo/diagnostics/rules/unused_dependencies.rs +++ b/src/cargo/diagnostics/rules/unused_dependencies.rs @@ -194,6 +194,7 @@ pub fn lint_build_results( &cargo_lints, pkg.rust_version(), pkg.manifest().unstable_features(), + build_runner.bcx.gctx, ); if !pkg_id.source_id().is_path() { for (dep_kind, state) in states.iter() { diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index e9c2d4c705a..93768c1cdaf 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -143,12 +143,13 @@ pub fn compile_with_exec<'a>( options: &CompileOptions, exec: &Arc, ) -> CargoResult> { - crate::diagnostics::passes::emit_parse_diagnostics( + let parse_pass_output = crate::diagnostics::passes::emit_parse_diagnostics( ws, crate::diagnostics::rules::PARSE_PASS_RULES, )?; let compilation = compile_ws(ws, options, exec)?; - if ws.gctx().warning_handling()? == WarningHandling::Deny && compilation.lint_warning_count > 0 + if ws.gctx().warning_handling()? == WarningHandling::Deny + && (compilation.lint_warning_count + parse_pass_output.lint_warning_count) > 0 { anyhow::bail!("warnings are denied by `build.warnings` configuration") } diff --git a/src/cargo/ops/cargo_fetch.rs b/src/cargo/ops/cargo_fetch.rs index 4c55c45ef7b..5742556215e 100644 --- a/src/cargo/ops/cargo_fetch.rs +++ b/src/cargo/ops/cargo_fetch.rs @@ -7,6 +7,7 @@ use crate::ops; use crate::util::CargoResult; use crate::util::GlobalContext; use crate::util::context::JobsConfig; +use crate::util::context::WarningHandling; use std::collections::HashSet; pub struct FetchOptions<'a> { @@ -20,7 +21,7 @@ pub fn fetch<'a>( ws: &Workspace<'a>, options: &FetchOptions<'a>, ) -> CargoResult<(Resolve, PackageSet<'a>)> { - crate::diagnostics::passes::emit_parse_diagnostics( + let parse_pass_output = crate::diagnostics::passes::emit_parse_diagnostics( ws, crate::diagnostics::rules::PARSE_PASS_RULES, )?; @@ -83,5 +84,11 @@ pub fn fetch<'a>( packages.get_many(to_download)?; crate::core::gc::auto_gc(gctx); + if ws.gctx().warning_handling()? == WarningHandling::Deny + && parse_pass_output.lint_warning_count > 0 + { + anyhow::bail!("warnings are denied by `build.warnings` configuration") + } + Ok((resolve, packages)) } diff --git a/tests/testsuite/warning_override.rs b/tests/testsuite/warning_override.rs index 951be589e13..dc4f6baaea7 100644 --- a/tests/testsuite/warning_override.rs +++ b/tests/testsuite/warning_override.rs @@ -227,6 +227,236 @@ fn rustc_caching_deny_first() { .run(); } +#[cargo_test] +fn lint_parse_pass() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + edition = "2018" + repository = "https://github.com/rust-lang/cargo/" + homepage = "https://github.com/rust-lang/cargo/" + + [lints.cargo] + default = { level = "allow", priority = -1 } + redundant_homepage = "warn" + "#, + ) + .file( + "src/main.rs", + r#" + fn main() {} + "#, + ) + .build(); + + // Verify the lints fire + p.cargo("fetch -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .with_stderr_data(str![[r#" +[WARNING] `package.homepage` is redundant with another manifest field + --> Cargo.toml:8:24 + | +7 | repository = "https://github.com/rust-lang/cargo/" + | ------------------------------------- +8 | homepage = "https://github.com/rust-lang/cargo/" + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `cargo::redundant_homepage` is set to `warn` in `[lints]` +[HELP] consider removing `package.homepage` + | +8 - homepage = "https://github.com/rust-lang/cargo/" + | +[WARNING] `foo` (manifest) generated 1 warning + +"#]]) + .run(); + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .with_stderr_data(str![[r#" +[WARNING] `package.homepage` is redundant with another manifest field + --> Cargo.toml:8:24 + | +7 | repository = "https://github.com/rust-lang/cargo/" + | ------------------------------------- +8 | homepage = "https://github.com/rust-lang/cargo/" + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `cargo::redundant_homepage` is set to `warn` in `[lints]` +[HELP] consider removing `package.homepage` + | +8 - homepage = "https://github.com/rust-lang/cargo/" + | +[WARNING] `foo` (manifest) generated 1 warning +[CHECKING] foo v0.1.0 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); + + p.cargo("fetch -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .arg("--config") + .arg("build.warnings='allow'") + .with_stderr_data(str![""]) + .run(); + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .arg("--config") + .arg("build.warnings='allow'") + .with_stderr_data(str![[r#" +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); + + p.cargo("fetch -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .arg("--config") + .arg("build.warnings='deny'") + .with_status(101) + .with_stderr_data(str![[r#" +[WARNING] `package.homepage` is redundant with another manifest field + --> Cargo.toml:8:24 + | +7 | repository = "https://github.com/rust-lang/cargo/" + | ------------------------------------- +8 | homepage = "https://github.com/rust-lang/cargo/" + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `cargo::redundant_homepage` is set to `warn` in `[lints]` +[HELP] consider removing `package.homepage` + | +8 - homepage = "https://github.com/rust-lang/cargo/" + | +[WARNING] `foo` (manifest) generated 1 warning +[ERROR] warnings are denied by `build.warnings` configuration + +"#]]) + .run(); + p.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .arg("--config") + .arg("build.warnings='deny'") + .with_status(101) + .with_stderr_data(str![[r#" +[WARNING] `package.homepage` is redundant with another manifest field + --> Cargo.toml:8:24 + | +7 | repository = "https://github.com/rust-lang/cargo/" + | ------------------------------------- +8 | homepage = "https://github.com/rust-lang/cargo/" + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `cargo::redundant_homepage` is set to `warn` in `[lints]` +[HELP] consider removing `package.homepage` + | +8 - homepage = "https://github.com/rust-lang/cargo/" + | +[WARNING] `foo` (manifest) generated 1 warning +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +[ERROR] warnings are denied by `build.warnings` configuration + +"#]]) + .run(); +} + +#[cargo_test] +fn lint_build_result_pass() { + // Cover each lint pass + Package::new("unused", "0.1.0").publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + edition = "2018" + + [dependencies] + unused = "0.1.0" + + [lints.cargo] + default = { level = "allow", priority = -1 } + unused_dependencies = "warn" + "#, + ) + .file( + "src/main.rs", + r#" + fn main() {} + "#, + ) + .build(); + + // Verify the lints fire + p.cargo("check --all-targets -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[LOCKING] 1 package to latest compatible version +[DOWNLOADING] crates ... +[DOWNLOADED] unused v0.1.0 (registry `dummy-registry`) +[CHECKING] unused v0.1.0 +[CHECKING] foo v0.1.0 ([ROOT]/foo) +[WARNING] unused dependency + --> Cargo.toml:9:13 + | +9 | unused = "0.1.0" + | ^^^^^^^^^^^^^^^^ + | + = [NOTE] `cargo::unused_dependencies` is set to `warn` in `[lints]` +[HELP] remove the dependency + | +9 - unused = "0.1.0" + | +[WARNING] `foo` (manifest) generated 1 warning +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); + + p.cargo("check --all-targets -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .arg("--config") + .arg("build.warnings='allow'") + .with_stderr_data(str![[r#" +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); + + p.cargo("check --all-targets -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .arg("--config") + .arg("build.warnings='deny'") + .with_status(101) + .with_stderr_data(str![[r#" +[WARNING] unused dependency + --> Cargo.toml:9:13 + | +9 | unused = "0.1.0" + | ^^^^^^^^^^^^^^^^ + | + = [NOTE] `cargo::unused_dependencies` is set to `warn` in `[lints]` +[HELP] remove the dependency + | +9 - unused = "0.1.0" + | +[WARNING] `foo` (manifest) generated 1 warning +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +[ERROR] warnings are denied by `build.warnings` configuration + +"#]]) + .run(); +} + #[cargo_test] fn hard_warning_deny() { let p = project()