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

clippy::if_then_panic warning (nightly) #73

Closed
Bromeon opened this issue Oct 12, 2021 · 3 comments
Closed

clippy::if_then_panic warning (nightly) #73

Bromeon opened this issue Oct 12, 2021 · 3 comments

Comments

@Bromeon
Copy link

Bromeon commented Oct 12, 2021

This is not a pressing issue yet, but it appears in a nightly version of clippy, so you might be interested in addressing the lint ahead of time.

I'm using this simple snippet:

use approx::assert_relative_eq;

fn main() {
    assert_relative_eq!(1.0, 2.0);
}

When running cargo +nightly clippy, I get:

warning: only a `panic!` in `if`-then statement
 --> src\main.rs:4:2
  |
4 |     assert_relative_eq!(1.0, 2.0);
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(clippy::if_then_panic)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_then_panic
  = note: this warning originates in the macro `__assert_approx` (in Nightly builds, run with -Z macro-backtrace for more info)

This occurred in the CI of the downstream project godot-rust, which treats clippy warnings as errors (the nightly workflow still passes though).


It has to do with how the __assert_approx macro is implemented.
Basically, the lint tries to detect the structure

if !cond { panic!(msg); }

and transform it into

assert!(cond, msg);

This would be one option to fix it, another would be to #[allow(clippy::if_then_panic)] the relevant statements.

I'm using rustc 1.57.0-nightly (5b210643e 2021-10-11).

bors bot added a commit to godot-rust/gdnative that referenced this issue Oct 14, 2021
793: Fix clippy lints in nightly r=Bromeon a=Bromeon

Where "fix" means mostly "disable".

Lints turned off globally for the time being (might change in the future):
* `clippy::missing_safety_doc` -- requires each unsafe fn/trait to have a `# Safety` section.
   Nice in theory, annoying for internals, and boilerplate-enforcing for obvious public cases.
* `clippy::if_then_panic` -- suggests that `if (!cond) { panic!(msg); }` be written as `assert!(cond, msg);`
   Probably makes sense to enable later, but currently not possible due to [approx/#73](brendanzab/approx#73).

Lints turned of locally:
* `clippy::redundant_closure` -- works around false positive [rust-clippy/#7812](rust-lang/rust-clippy#7812)
* `dead_code` (compiler warning, not clippy) -- generated field `this` not always used. Naming it `_this` doesn't seem right, either.

Co-authored-by: Jan Haller <[email protected]>
@EFanZh EFanZh mentioned this issue Dec 4, 2021
@EFanZh
Copy link
Contributor

EFanZh commented Jan 21, 2022

I think #72 has fixed this issue, so maybe this can be closed?

@Bromeon
Copy link
Author

Bromeon commented Jan 22, 2022

The Clippy lint has been renamed and moved categories in the meantime: rust-lang/rust-clippy#7810

However, it still seems to occur with approx v0.5.0.
A crate with only this code:

fn main() {
    approx::assert_relative_eq!(1.0, 1.0);
}

results in:

cargo clippy -- -D clippy::pedantic
    Checking ctest v0.1.0 (<path/to/project>)
error: only a `panic!` in `if`-then statement
 --> src\main.rs:2:5
  |
2 |     approx::assert_relative_eq!(1.0, 1.0);
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `-D clippy::manual-assert` implied by `-D clippy::pedantic`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
  = note: this error originates in the macro `__assert_approx` (in Nightly builds, run with -Z macro-backtrace for more info)

error: could not compile `<crate>` due to previous error

@EFanZh
Copy link
Contributor

EFanZh commented Jan 22, 2022

The fix was just merged a few days ago, new version containing the fix hasn’t been released yet.

KarimHamidou added a commit to KarimHamidou/godot-rust that referenced this issue Feb 6, 2023
793: Fix clippy lints in nightly r=Bromeon a=Bromeon

Where "fix" means mostly "disable".

Lints turned off globally for the time being (might change in the future):
* `clippy::missing_safety_doc` -- requires each unsafe fn/trait to have a `# Safety` section.
   Nice in theory, annoying for internals, and boilerplate-enforcing for obvious public cases.
* `clippy::if_then_panic` -- suggests that `if (!cond) { panic!(msg); }` be written as `assert!(cond, msg);`
   Probably makes sense to enable later, but currently not possible due to [approx/#73](brendanzab/approx#73).

Lints turned of locally:
* `clippy::redundant_closure` -- works around false positive [rust-clippy/#7812](rust-lang/rust-clippy#7812)
* `dead_code` (compiler warning, not clippy) -- generated field `this` not always used. Naming it `_this` doesn't seem right, either.

Co-authored-by: Jan Haller <[email protected]>
GuilhermeOrceziae added a commit to GuilhermeOrceziae/godot-rust that referenced this issue Feb 9, 2023
793: Fix clippy lints in nightly r=Bromeon a=Bromeon

Where "fix" means mostly "disable".

Lints turned off globally for the time being (might change in the future):
* `clippy::missing_safety_doc` -- requires each unsafe fn/trait to have a `# Safety` section.
   Nice in theory, annoying for internals, and boilerplate-enforcing for obvious public cases.
* `clippy::if_then_panic` -- suggests that `if (!cond) { panic!(msg); }` be written as `assert!(cond, msg);`
   Probably makes sense to enable later, but currently not possible due to [approx/#73](brendanzab/approx#73).

Lints turned of locally:
* `clippy::redundant_closure` -- works around false positive [rust-clippy/#7812](rust-lang/rust-clippy#7812)
* `dead_code` (compiler warning, not clippy) -- generated field `this` not always used. Naming it `_this` doesn't seem right, either.

Co-authored-by: Jan Haller <[email protected]>
hesuteia added a commit to hesuteia/godot-rust that referenced this issue Feb 11, 2023
793: Fix clippy lints in nightly r=Bromeon a=Bromeon

Where "fix" means mostly "disable".

Lints turned off globally for the time being (might change in the future):
* `clippy::missing_safety_doc` -- requires each unsafe fn/trait to have a `# Safety` section.
   Nice in theory, annoying for internals, and boilerplate-enforcing for obvious public cases.
* `clippy::if_then_panic` -- suggests that `if (!cond) { panic!(msg); }` be written as `assert!(cond, msg);`
   Probably makes sense to enable later, but currently not possible due to [approx/#73](brendanzab/approx#73).

Lints turned of locally:
* `clippy::redundant_closure` -- works around false positive [rust-clippy/#7812](rust-lang/rust-clippy#7812)
* `dead_code` (compiler warning, not clippy) -- generated field `this` not always used. Naming it `_this` doesn't seem right, either.

Co-authored-by: Jan Haller <[email protected]>
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

2 participants