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

tests: enable clippy::get_unwrap lint #87

Merged
merged 1 commit into from
May 1, 2024
Merged

Conversation

laniakea64
Copy link
Collaborator

Going through our test runner code, I noticed I had written code that follows this pattern -

let h = HashMap::<String, String>::new();
let _ = h.get("foo").unwrap();

But the second line can use &h["foo"] for equivalent behavior, clearer code, and a more informative error message if that key unexpectedly doesn't exist. I've already changed our code to use this clearer style, but I was like, wait why didn't Clippy warn about this?

Turns out the answer is, there is such Clippy lint, but it's allow-by-default.

This PR makes .get(...).unwrap() a Clippy warning, i.e. it's fine in dev (e.g. temporarily using .unwrap() where error handling will be added later) but we don't want code like this in production.

@NoahTheDuke
Copy link
Owner

we don't want code like this in production.

lol

@laniakea64 laniakea64 merged commit a621ede into main May 1, 2024
1 check passed
@laniakea64 laniakea64 deleted the lint-get_unwrap branch May 1, 2024 14:56
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

Successfully merging this pull request may close these issues.

2 participants