search

mainmatter / 100-exercises-to-learn-rust

A self-paced course to learn Rust, one exercise at a time.
https://rust-exercises.com
6.2k stars 1.08k forks source link

dead_code warnings #118

Open illicitonion opened 4 months ago

illicitonion commented 4 months ago

I've been testing out 100ETLR and am generally really loving it - it hits at exactly the right balance of explanation / exercise / exploration for the people I'm hoping to teach with it.

One issue I ran into is that by by default cargo test on exercises (e.g. 01_intro/00_welcome) produces warnings:

warning: function `greeting` is never used
  --> exercises/01_intro/00_welcome/src/lib.rs:18:4
   |
18 | fn greeting() -> &'static str {
   |    ^^^^^^^^
   |
   = note: `#[warn(dead_code)]` on by default

warning: `welcome_00` (lib) generated 1 warning

One of the big lessons I'm trying to impart on my students is that the Rust compiler has really, really good warnings and errors, and you should never ignore them because they almost always provide really high quality signal to point out a real problem you should address.

Accordingly, I'd much prefer if the code was free of benign warnings. How would y'all feel about making all of the top-level-just-for-tests functions pub, so that these warnings get quashed?

Alternatively, we could #[allow(dead_code)] them or similar, but that feels like it's more confusing and require more explanation...

LukeMathWalker commented 3 months ago

I'm against adding a pub modifier until visibility has been explained. I also agree that #[allow(dead_code)] would raise even more questions.

A potential solution: use the new lints table in Cargo.toml rather than a top-level attribute. What do you think?

Waheedsys commented 1 month ago

Hey, would it be okay if I worked on this issue?