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

Enable lints to prevent unwanted usages of unwrap(), assert! and panic! #711

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Nov 2, 2023

The clippy::unwrap_used lint prevents usage of .unwrap() outside of tests:
https://rust-lang.github.io/rust-clippy/master/index.html#/unwrap_used

The clippy::panic_in_result_fn lint prevents usages of assert! or panic! in functions that already return a Result:
https://rust-lang.github.io/rust-clippy/master/index.html#/panic_in_result_fn

These lints catch cases like #709, #710 and #712.

The lints have not been enabled for the integration test modules (since using unwrap() is fine there), or to the example buildpacks, since it's a lot of boilerplate to add to an example.

Very soon we will be able to switch to the upcoming [lints] table feature, which will mean the duplication of lint definitions can be avoided entirely:
https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#lints

GUS-W-14413286.

…`panic!`

The `clippy::unwrap_used` lint prevent usage of `.unwrap()` outside of tests:
https://rust-lang.github.io/rust-clippy/master/index.html#/unwrap_used

The `clippy::panic_in_result_fn` lint prevent usages of `assert!` or
`panic!` in functions that already return a `Result`:
https://rust-lang.github.io/rust-clippy/master/index.html#/panic_in_result_fn

These lints catch cases like #709, #710 and #712.

The lints have not been enabled for the integration test modules (since it's
pointless, as they only apply to non-test usages), or to the example buildpacks,
since it's a lot of boilerplate to add to an example.

Very soon we will be able to switch to the upcoming `[lints]` table feature, which will
mean the duplication of lint definitions can be avoided entirely:
https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#lints
@edmorley edmorley force-pushed the edmorley/panic-lints branch from bebd998 to 2d6ba6e Compare November 2, 2023 12:01
@edmorley edmorley marked this pull request as ready for review November 2, 2023 12:08
@edmorley edmorley requested a review from a team as a code owner November 2, 2023 12:08
@edmorley edmorley enabled auto-merge (squash) November 2, 2023 13:00
@edmorley edmorley merged commit 5c314ee into main Nov 2, 2023
4 checks passed
@edmorley edmorley deleted the edmorley/panic-lints branch November 2, 2023 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants