Move `if_then_panic` to pedantic

[ghost]

I ran this lint on a bunch of crates and it's firing a lot.

For readability, it's debatable whether it's recommendations are actually improvements. Especially where there are a lot of args or the condition is complicated.

I also think there is a semantic difference between an assert and if-panic. I see asserts as checking "my code". I'd never use asserts to check user input. I'd like to know if other people share this view.

(There are also bugs in the recommendations but that's not relevant to this issue. I'll log a separate issue for those later.)

[ghost]

Here is another example of the semantic difference between assert and if-panic.

If I understand this code, it's checking if the setting requires panicking and then panicking. I don't think assert is appropriate here.

---> ws-0.9.1/src/io.rs:883:33                                                                                                                                                 
warning: only a `panic!` in `if`-then statement                                                                                                                                
   --> src/io.rs:883:33
    |
883 | / ...                   if self.settings.panic_on_new_connection {
884 | | ...                       panic!("Unable to establish connection to {}: {:?}", url, err);
885 | | ...                   }
    | |_______________________^ help: try: `assert!(!self.settings.panic_on_new_connection, "Unable to establish connection to {}: {:?}", url, err);`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_then_panic

Below are the warning counts I got from testing around 1000 popular crates on crates.io. Just to make precise what I mean by "firing a lot".

Warning counts

Warnings | 333 ---| --: actix-0.12.0 | 5 actix-router-0.4.0 | 2 backtrace-0.3.61 | 1 base-x-0.2.8 | 1 base64-0.13.0 | 1 bech32-0.8.1 | 1 bindgen-0.59.1 | 1 bitvec-0.22.3 | 3 blake3-1.0.0 | 1 brotli-3.3.2 | 3 brotli-decompressor-2.3.2 | 1 buf_redux-0.8.4 | 1 chalk-engine-0.71.0 | 3 chrono-0.4.19 | 1 combine-4.6.1 | 2 compiletest_rs-0.7.0 | 6 criterion-0.3.5 | 5 crossbeam-channel-0.5.1 | 2 ctor-0.1.21 | 1 dbus-0.9.3 | 7 derive-new-0.5.9 | 3 derive_builder_core-0.10.2 | 1 derive_more-0.99.16 | 3 elasticlunr-rs-2.3.13 | 1 encode_unicode-0.3.6 | 1 encoding-0.2.33 | 1 failure_derive-0.1.8 | 2 fastrand-1.5.0 | 2 form_urlencoded-1.0.1 | 1 futures-channel-0.3.17 | 2 futures-timer-3.0.2 | 1 gethostname-0.2.1 | 1 gl_generator-0.14.0 | 2 glutin-0.27.0 | 2 h2-0.3.4 | 1 hex-literal-impl-0.2.2 | 1 html5ever-0.25.1 | 7 http-0.2.4 | 1 httpdate-1.0.1 | 1 hyper-rustls-0.22.1 | 1 ignore-0.4.18 | 1 im-15.0.0 | 8 im-rc-15.0.0 | 8 image-0.23.14 | 2 indexmap-1.7.0 | 1 indicatif-0.16.2 | 1 insta-1.7.2 | 2 jemalloc-sys-0.3.2 | 2 libssh2-sys-0.2.21 | 1 mio-0.7.13 | 1 nalgebra-0.29.0 | 3 ndarray-0.15.3 | 9 nibble_vec-0.1.0 | 1 nix-0.22.1 | 3 num-bigint-0.4.2 | 2 num-derive-0.3.3 | 1 num-rational-0.4.0 | 1 onig_sys-69.7.0 | 2 openssl-src-111.16.0+1.1.1l | 1 openssl-sys-0.9.66 | 2 parking_lot-0.11.2 | 1 pest-2.1.3 | 2 petgraph-0.6.0 | 1 phf_codegen-0.10.0 | 2 pq-sys-0.4.6 | 1 proc-macro-error-1.0.4 | 1 proc-macro2-1.0.29 | 4 proptest-1.0.0 | 13 prost-build-0.8.0 | 2 prost-derive-0.8.0 | 2 protobuf-codegen-2.25.1 | 1 pyo3-0.14.5 | 3 racer-2.1.48 | 1 radix_trie-0.2.1 | 1 rayon-1.5.1 | 1 regex-1.5.4 | 1 regex-automata-0.1.10 | 7 rental-impl-0.5.5 | 9 ring-0.16.20 | 2 rocksdb-0.17.0 | 12 rusqlite-0.25.3 | 4 rust-crypto-0.2.36 | 6 rustc-ap-rustc_errors-727.0.0 | 1 rusty-fork-0.3.0 | 1 scoped_threadpool-0.1.9 | 1 serde_codegen_internals-0.14.2 | 1 serde_derive-1.0.130 | 1 serde_derive_internals-0.26.0 | 1 serde_test-1.0.130 | 5 serde_yaml-0.8.20 | 1 sized-chunks-0.6.5 | 16 skeptic-0.13.6 | 1 slab-0.4.4 | 1 slog-term-2.8.0 | 1 smithay-client-toolkit-0.15.1 | 1 stdweb-0.4.20 | 42 stdweb-derive-0.5.3 | 2 stdweb-internal-macros-0.2.9 | 7 syn-1.0.76 | 7 syntex_errors-0.59.1 | 1 take_mut-0.2.2 | 1 tokio-util-0.6.8 | 2 tonic-0.5.2 | 1 trybuild-1.0.45 | 1 v_escape_derive-0.8.4 | 2 wasm-bindgen-macro-support-0.2.76 | 1 wasm-bindgen-test-0.3.26 | 1 wasm-timer-0.2.5 | 1 winit-0.25.0 | 7 ws-0.9.1 | 11 xcb-0.9.0 | 1 zstd-sys-1.6.1+zstd.1.5.0 | 1

[camsteffen]

Thanks for the data and the example. I can definitely see how assert! "feels weird" in that example. Though I do think that assert! is technically the same semantically since it expands to the same thing (except in the case of panic!() with no message: an "assertion failed: .." message is used). My intuition is that this lint fires a lot mostly because people simply don't consider using assert!, but they would if both options were considered. At least that was the case for me. That said, I'm totally convinced that this should be pedantic as this is all picky/debatable.

I'd never use asserts to check user input.

That is a good and typical use case for assert! IMO.