Add `print_stdout` and `print_stderr` lints (#17446) #18233

sirius94 · 2025-03-10T17:31:16Z

Objective

Prevent usage of println!, eprintln! and the like because they require std
Fixes Bevy should deny eprintln and println and prefer log and/or tracing #17446

Solution

Enable the print_stdout and print_stderr clippy lints
Replace all println! and eprintln! occurrences with log::* where applicable or alternatively ignore the warnings

Testing

Run cargo clippy --workspace to ensure that there are no warnings relating to printing to stdout or stderr

github-actions · 2025-03-10T17:31:29Z

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

alice-i-cecile · 2025-03-10T17:32:16Z

crates/bevy_ecs/examples/change_detection.rs

@@ -12,6 +12,7 @@
 )]

 use bevy_ecs::prelude::*;
+use log::info;


In examples we should be using bevy's re-exports of this, but since this is in bevy_ecs's dedicated examples this is fine.

alice-i-cecile

Really thorough work. Thank you!

sirius94 · 2025-03-10T18:33:10Z

I've missed some things. I'll change this back to 'ready for review' when all the automated checks pass.

bushrat011899

Nice! Standardising the code-base (and explicitly annotating exceptions with expect) is always worth it, even ignoring the no_std benefits.

mockersf

you changed a few tests where they should continue to use println. loggers are often not set for tests

mockersf · 2025-03-10T21:33:25Z

crates/bevy_ecs/examples/change_detection.rs

@@ -37,7 +38,7 @@ fn main() {

    // Simulate 10 frames in our world
    for iteration in 1..=10 {
-        println!("Simulating frame {iteration}/10");
+        info!("Simulating frame {iteration}/10");


logger is also not set in this file so it should still use println

should this have a #[cfg(feature="std)] then?

mockersf · 2025-03-10T21:34:07Z

crates/bevy_ecs/examples/events.rs

@@ -2,6 +2,7 @@
 //! If an event was send, it will be printed by the console in a receiving system.

 use bevy_ecs::{event::EventRegistry, prelude::*};
+use log::info;


same here, this should keep using println

mockersf · 2025-03-10T21:34:22Z

crates/bevy_ecs/examples/resources.rs

@@ -7,6 +7,7 @@
 )]

 use bevy_ecs::prelude::*;
+use log::info;


same here, this should keep using println

mockersf · 2025-03-10T21:35:42Z

crates/bevy_ecs/src/schedule/executor/multi_threaded.rs

@@ -283,7 +281,7 @@ impl<'scope, 'env: 'scope, 'sys> Context<'scope, 'env, 'sys> {
            .push(SystemResult { system_index })
            .unwrap_or_else(|error| unreachable!("{}", error));
        if let Err(payload) = res {
-            eprintln!("Encountered a panic in system `{}`!", &*system.name());
+            log::error!("Encountered a panic in system `{}`!", &*system.name());


there's no guarantee a logger is set here and this error is too important to risk to hide, it should keep using eprintln

mockersf · 2025-03-10T21:40:07Z

very few of the existing println should be replaced for this, they should be expect. most of them have already been checked

mockersf · 2025-03-11T01:39:29Z

this is still changing too many println to logs. those changes should be reverted

sirius94 · 2025-03-11T18:12:47Z

I reverted the changes from prints to logs and used expect instead.

All changes to log have been removed.

alice-i-cecile reviewed Mar 10, 2025

View reviewed changes

alice-i-cecile approved these changes Mar 10, 2025

View reviewed changes

alice-i-cecile requested a review from bushrat011899 March 10, 2025 17:33

alice-i-cecile mentioned this pull request Mar 10, 2025

bevy_ecs fails to compile if std feature enabled without backtrace feature due to panic hook #18231

Closed

sirius94 force-pushed the 17446-lint-stdout-stderr branch 2 times, most recently from a5138bc to c87d1d8 Compare March 10, 2025 18:22

sirius94 marked this pull request as draft March 10, 2025 18:32

sirius94 force-pushed the 17446-lint-stdout-stderr branch 3 times, most recently from 1d428ee to 8c32b8a Compare March 10, 2025 19:17

sirius94 marked this pull request as ready for review March 10, 2025 19:33

bushrat011899 approved these changes Mar 10, 2025

View reviewed changes

bushrat011899 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 10, 2025

mockersf previously requested changes Mar 10, 2025

View reviewed changes

mockersf reviewed Mar 10, 2025

View reviewed changes

alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Mar 10, 2025

sirius94 force-pushed the 17446-lint-stdout-stderr branch from 9dc3900 to 82fa507 Compare March 10, 2025 22:02

alice-i-cecile requested a review from mockersf March 11, 2025 01:22

alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Mar 11, 2025

mockersf added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 11, 2025

sirius94 force-pushed the 17446-lint-stdout-stderr branch from 82fa507 to 3fbd313 Compare March 11, 2025 16:36

Add print_stdout and print_stderr lints (bevyengine#17446)

e5b15f2

sirius94 force-pushed the 17446-lint-stdout-stderr branch from 3fbd313 to e5b15f2 Compare March 11, 2025 16:37

alice-i-cecile approved these changes Mar 11, 2025

View reviewed changes

alice-i-cecile added this pull request to the merge queue Mar 11, 2025

alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Mar 11, 2025

Merged via the queue into bevyengine:main with commit 8570af1 Mar 11, 2025
36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `print_stdout` and `print_stderr` lints (#17446) #18233

Add `print_stdout` and `print_stderr` lints (#17446) #18233

sirius94 commented Mar 10, 2025

github-actions bot commented Mar 10, 2025

alice-i-cecile Mar 10, 2025 •

edited

Loading

alice-i-cecile left a comment

sirius94 commented Mar 10, 2025

bushrat011899 left a comment

mockersf left a comment

mockersf Mar 10, 2025 •

edited

Loading

hukasu Mar 11, 2025

mockersf Mar 10, 2025

mockersf Mar 10, 2025

mockersf Mar 10, 2025

mockersf commented Mar 10, 2025

mockersf commented Mar 11, 2025

sirius94 commented Mar 11, 2025

Add print_stdout and print_stderr lints (#17446) #18233

Add print_stdout and print_stderr lints (#17446) #18233

Conversation

sirius94 commented Mar 10, 2025

Objective

Solution

Testing

github-actions bot commented Mar 10, 2025

alice-i-cecile Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

sirius94 commented Mar 10, 2025

bushrat011899 left a comment

Choose a reason for hiding this comment

mockersf left a comment

Choose a reason for hiding this comment

mockersf Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

hukasu Mar 11, 2025

Choose a reason for hiding this comment

mockersf Mar 10, 2025

Choose a reason for hiding this comment

mockersf Mar 10, 2025

Choose a reason for hiding this comment

mockersf Mar 10, 2025

Choose a reason for hiding this comment

mockersf commented Mar 10, 2025

mockersf commented Mar 11, 2025

sirius94 commented Mar 11, 2025

Add `print_stdout` and `print_stderr` lints (#17446) #18233

Add `print_stdout` and `print_stderr` lints (#17446) #18233

alice-i-cecile Mar 10, 2025 •

edited

Loading

mockersf Mar 10, 2025 •

edited

Loading