Skip to content

Support overriding warnings level for a specific lint via command line #113307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
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
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ pub static TRACK_DIAGNOSTICS: AtomicRef<fn(&mut Diagnostic, &mut dyn FnMut(&mut
#[derive(Copy, Clone, Default)]
pub struct HandlerFlags {
/// If false, warning-level lints are suppressed.
/// (rustc: see `--allow warnings` and `--cap-lints`)
/// (rustc: see `--cap-lints`)
pub can_emit_warnings: bool,
/// If true, error-level diagnostics are upgraded to bug-level.
/// (rustc: see `-Z treat-err-as-bug`)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ fn test_switch_implies_cfg_test_unless_cfg_test() {
#[test]
fn test_can_print_warnings() {
rustc_span::create_default_session_globals_then(|| {
let matches = optgroups().parse(&["-Awarnings".to_string()]).unwrap();
let matches = optgroups().parse(&["--cap-lints=allow".to_string()]).unwrap();
let mut handler = EarlyErrorHandler::new(ErrorOutputType::default());
let (sess, _) = mk_session(&mut handler, matches);
assert!(!sess.diagnostic().can_emit_warnings());
Expand Down
18 changes: 15 additions & 3 deletions compiler/rustc_middle/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub fn reveal_actual_level(
src: &mut LintLevelSource,
sess: &Session,
lint: LintId,
probe_for_lint_level: impl FnOnce(LintId) -> (Option<Level>, LintLevelSource),
probe_for_lint_level: impl Fn(LintId) -> (Option<Level>, LintLevelSource),
) -> Level {
// If `level` is none then we actually assume the default level for this lint.
let mut level = level.unwrap_or_else(|| lint.lint.default_level(sess.edition()));
Expand All @@ -90,8 +90,20 @@ pub fn reveal_actual_level(
// future compatibility warning.
if level == Level::Warn && lint != LintId::of(FORBIDDEN_LINT_GROUPS) {
let (warnings_level, warnings_src) = probe_for_lint_level(LintId::of(builtin::WARNINGS));
if let Some(configured_warning_level) = warnings_level {
if configured_warning_level != Level::Warn {
if let Some(configured_warning_level) = warnings_level
&& configured_warning_level != Level::Warn
{
if matches!(warnings_src, LintLevelSource::CommandLine(..))
&& lint != LintId::of(builtin::WARNINGS)
&& let (Some(lint_level), lint_src) = probe_for_lint_level(lint)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are level and src arguments to the current function. We shouldn't recompute them.

&& matches!(lint_src, LintLevelSource::CommandLine(..))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care about the order in which the command line arguments are passed?
What about -Wdead-code -Awarnings?

{
// We have a command-line `-Awarnings` *and* a command-line e.g. `-Wspecific-lint`.
// Let's have the `specific-lint`'s command-line level override the command-line
// `warnings` level.
level = lint_level;
*src = lint_src;
} else {
level = configured_warning_level;
*src = warnings_src;
}
Expand Down
10 changes: 1 addition & 9 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1393,16 +1393,8 @@ pub fn build_session(
target_override: Option<Target>,
cfg_version: &'static str,
) -> Session {
// FIXME: This is not general enough to make the warning lint completely override
// normal diagnostic warnings, since the warning lint can also be denied and changed
// later via the source code.
let warnings_allow = sopts
.lint_opts
.iter()
.rfind(|&(key, _)| *key == "warnings")
.is_some_and(|&(_, level)| level == lint::Allow);
let cap_lints_allow = sopts.lint_cap.is_some_and(|cap| cap == lint::Allow);
let can_emit_warnings = !(warnings_allow || cap_lints_allow);
let can_emit_warnings = !cap_lints_allow;

let sysroot = match &sopts.maybe_sysroot {
Some(sysroot) => sysroot.clone(),
Expand Down
27 changes: 27 additions & 0 deletions tests/ui/lint/command-line-allow-warnings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// compile-flags: -A warnings

fn main() {
// This will be overriden by the `-A warnings` command line option.
#[warn(non_snake_case)]
let _OwO = 0u8;

// But this should not.
#[deny(non_snake_case)]
let _UwU = 0u8;
//~^ ERROR variable `_UwU` should have a snake case name

bar();
baz();
}

#[warn(warnings)]
fn bar() {
let _OwO = 0u8;
//~^ WARN variable `_OwO` should have a snake case name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how this work.
In this case in-source annotations take priority over command line option, but above (#[warn(non_snake_case)]) command line takes priority over in-source annotation.

Also, what -A warnings means exactly

  • allow all lints (not true according to this test)
  • allow lints with default level warn
  • allow lints with effective level warn (explicit level with fallback to default)

?

Copy link
Member Author

@jieyouxu jieyouxu Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case in-source annotations take priority over command line option, but above (#[warn(non_snake_case)]) command line takes priority over in-source annotation.

The warnings lint and -A warnings is... special. To the best of my knowedge, -A warnings suppresses warn level lints even when there is a source code annotation for the specific non-warnings lint (of warn level). However, when there is a #[level(warnings)] source code annotation, it overrides -A warnings for the lexical scope affected by the annotation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, lints with levels higher than warn are not affected by -A warnings or #[allow(warnings)] at all, such as deny-level lints.

}

#[deny(warnings)]
fn baz() {
let _OwO = 0u8;
//~^ ERROR variable `_OwO` should have a snake case name
}
35 changes: 35 additions & 0 deletions tests/ui/lint/command-line-allow-warnings.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: variable `_UwU` should have a snake case name
--> $DIR/command-line-allow-warnings.rs:10:9
|
LL | let _UwU = 0u8;
| ^^^^ help: convert the identifier to snake case: `_uw_u`
|
note: the lint level is defined here
--> $DIR/command-line-allow-warnings.rs:9:12
|
LL | #[deny(non_snake_case)]
| ^^^^^^^^^^^^^^

warning: variable `_OwO` should have a snake case name
--> $DIR/command-line-allow-warnings.rs:19:9
|
LL | let _OwO = 0u8;
| ^^^^ help: convert the identifier to snake case: `_ow_o`
|
= note: `#[warn(non_snake_case)]` on by default

error: variable `_OwO` should have a snake case name
--> $DIR/command-line-allow-warnings.rs:25:9
|
LL | let _OwO = 0u8;
| ^^^^ help: convert the identifier to snake case: `_ow_o`
|
note: the lint level is defined here
--> $DIR/command-line-allow-warnings.rs:23:8
|
LL | #[deny(warnings)]
| ^^^^^^^^
= note: `#[deny(non_snake_case)]` implied by `#[deny(warnings)]`

error: aborting due to 2 previous errors; 1 warning emitted

11 changes: 11 additions & 0 deletions tests/ui/lint/command-line-lint-level-specific-lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// compile-flags: -A warnings -W unused-variables

fn main() {
let x = 0u8;
//~^ WARNING unused variable

// Source code lint level should override command-line lint level in any case.
#[deny(unused_variables)]
let y = 0u8;
//~^ ERROR unused variable
}
22 changes: 22 additions & 0 deletions tests/ui/lint/command-line-lint-level-specific-lint.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
warning: unused variable: `x`
--> $DIR/command-line-lint-level-specific-lint.rs:4:9
|
LL | let x = 0u8;
| ^ help: if this is intentional, prefix it with an underscore: `_x`
|
= note: requested on the command line with `-W unused-variables`

error: unused variable: `y`
--> $DIR/command-line-lint-level-specific-lint.rs:9:9
|
LL | let y = 0u8;
| ^ help: if this is intentional, prefix it with an underscore: `_y`
|
note: the lint level is defined here
--> $DIR/command-line-lint-level-specific-lint.rs:8:12
|
LL | #[deny(unused_variables)]
| ^^^^^^^^^^^^^^^^

error: aborting due to previous error; 1 warning emitted