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

Improve suggestion of bool_assert_comparison lint #7888

Closed
wants to merge 1 commit into from
Closed

Improve suggestion of bool_assert_comparison lint #7888

wants to merge 1 commit into from

Conversation

dima74
Copy link
Contributor

@dima74 dima74 commented Oct 27, 2021

changelog: Improve suggestion of [bool_assert_comparison] lint

@rust-highfive
Copy link

r? @giraffate

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 27, 2021
@giraffate
Copy link
Contributor

This improvement is in the process of being made in #7612.


error: used `assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:111:5
|
LL | assert_eq!("a".is_empty(), false, "tadam {}", 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!"a".is_empty())`
Copy link
Contributor

@samueltardieu samueltardieu Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really the intended output? I would have expected to see

  replace it with: `assert!(!"a".is_empty(), "tadam {}", 1);`

otherwise the assertion message and its arguments are lost. This was previously covered by the "..". Same for other modified test outputs below.

@dima74 dima74 closed this Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants