Skip to content
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

strings4 can pass checks even with a "useless_conversion" warning. #2190

Open
lindes opened this issue Jan 2, 2025 · 0 comments
Open

strings4 can pass checks even with a "useless_conversion" warning. #2190

lindes opened this issue Jan 2, 2025 · 0 comments

Comments

@lindes
Copy link

lindes commented Jan 2, 2025

On line 24 of exercises/09_strings/strings4.rs, we are instructed to "replace placeholder(…) with either string_slice(…) or string(…) depending on what you think each value is.". Presumably that's the only type of change we're meant to make. So, when I mistakenly replaced the placeholder on that line to string_slice (and changed all the other placeholders to what they need to be), I nominally pass the exercise (getting a green "Exercise done ✓"), but I'm still getting a warning:

warning: useless conversion to the same type: `&str`
  --> exercises/09_strings/strings4.rs:24:18
   |
24 |     string_slice("nice weather".into());
   |                  ^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"nice weather"`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
   = note: `#[warn(clippy::useless_conversion)]` on by default

Now, I think the right thing for me to have done here is to use string(...) instead of string_slice(...), so arguably this is an intended piece of the puzzle. But it passes, and that doesn't seem right. I'm not certain what the correct solution to this is (otherwise I'd probably submit a PR directly)... A few thoughts:

  • I know I can put #[warn(clippy::useless_conversion)] right above line 24, and that makes the warning go away even though I've chosen the wrong alternative.
  • On the flip side, I could put #[deny(clippy::useless_conversion)] (and/or this could be put into the exercise file, perhaps with a // do not remove this line comment after it), thus forcing this to be an error that needs to be fixed (hopefully by switching from string_slice to string).
  • Similarly, we could put #![deny(clippy::useless_conversion)] (and some appropriate comment) up at the top of the file (this maybe seems like the best solution to me, though see below), also forcing things.

The biggest uncertainty I have about those last two options is that doing those adds a note such as the following to the compiler output:

note: the lint level is defined here
  --> exercises/09_strings/strings4.rs:2:9
   |
2  | #![deny(clippy::useless_conversion)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^

Now, maybe that's fine, and that's the best solution? I'm not sure. So, hopefully someone more experienced (I'm running into this because I'm new to rust, naturally) can figure out what the best solution should be.

The one thing that I think is probably a "bug" is that as-is, I can pass the exercise while still getting compiler warnings. It seems to me that it should either be allowed without warnings, or give an error, and the latter seems more in-line with what this particular exercise is hoping to teach.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant