-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Issue4983 bool equal not bool #5144
Issue4983 bool equal not bool #5144
Conversation
if let BinOpKind::Eq = op.node; | ||
if one_side_is_unary_not(&left_side, &right_side); | ||
then { | ||
span_lint_and_help(cx, BOOL_COMPARISON, e.span, "Here comes", "the suggestion"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When building a suggestion, you have to understand how spans work. A lint with a suggestion has 2 spans: 1 span (L) for the lint message and 1 span (S) for the suggestion. When running cargo fix
on a suggestion like this, the span (S) is completely removed and replaced with the code inside the sugg
parameter of the span_lint_and_sugg
function. The output with differing spans may look like this:
error: Lint message
--> $DIR/bool_comparison.rs:119:8
|
LL | if a == !b {};
| ^^^^^^^
|
= help: replace it with:
|
LL | if a != b {}
| ^^^^^^
As you may have noticed, the span_lint_and_sugg
function only has one span
argument. This is because most of the time, the span of te lint message and of the suggestion are the same. This is also the case here. The output of this case is most of the time more compact and looks nicer:
error: Here comes
--> $DIR/bool_comparison.rs:119:8
|
LL | if a == !b {};
| ^^^^^^^ help: replace it with: `a != b`
The span of the thing you want to replace is e.span
. To build the suggestion, you can use utils::snippet_with_applicability
. Let's name some things:
a == !b
^^^^^^^ e
^ left_side
^^ right_side
^ ?
So you have to get the span of ?
. This is the span of the unop
expression in the is_unary_not
function above. Building the complete suggestion, you have to do:
format!(
"{} != {}",
snippet_with_applicability(cx, left_side.span, &mut applicability),
snippet_with_applicability(cx, ?.span, &mut applicability),
)
This won't be enough though, since you have to also build the suggestion for the symmetric case. But this is homework for you. 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the thorough explanation. I need some time to figure things out :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK,
At last, I have some spare time to work on the issue and... mess-up the PR in the process ;-)
Anyway, I think I got it and the message now looks like the one below. I will clean-up the code and submit a new PR for review soon:
error: This comparison might be written more concisely
--> $DIR/bool_comparison.rs:119:8
|
LL | if a == !b {};
| ^^^^^^^ help: try simplifying it as shown: `a != b`
error: This comparison might be written more concisely
--> $DIR/bool_comparison.rs:120:8
|
LL | if !a == b {};
| ^^^^^^^ help: try simplifying it as shown: `a != b`
Let update_lints also generate the internal lints r? @phansch changelog: none
Typo in literal_representation.rs Octal numbers can't have 8 in them ;) changelog: none
These regions will all be `ReErased` soon.
Avoid using regions from `TypeckTables` These regions will all be `ReErased` soon. (rust-lang/rust#69189) changelog: none
Rustup to rust-lang/rust#67953 changelog: none
Don't lint `single_component_path_imports` in macros Fixes #5154 changelog: Fix false positive in `single_component_path_imports`
Reduce `pulldown-cmark` size Should reduce `pulldown-cmark` size. ref. https://github.com/raphlinus/pulldown-cmark#build-options changelog: none
…al, r=flip1995 Lint lossy whole number float literals changelog: Extend `excessive_precision` to lint lossy whole number float literals Fixes #5160
Rename `FunctionRetTy` to `FnRetTy` Rustup to rust-lang/rust#69179 changelog: none
Expand `missing_errors_doc` to also work on async functions This adds the `missing_errors_doc` lint to async functions. changelog: Make [`missing_errors_doc`] lint also trigger on `async` functions
new_lint.rs: encourage authors to write more detailed code samples in lint descriptions (linted as well as fixed code) changelog: none
clean up a few lint docs changelog: none
integer_arithmetic: detect integer arithmetic on references. changelog: integer_arithmetic fix false negatives with references on integers Fixes #5328
Update changelog to 1.43.0 beta In the beta changelog update, I accidentally used the commit of the 1.43.0 beta, instead of the 1.42.0 beta. I fixed this in this PR. [Rendered](https://github.com/flip1995/rust-clippy/blob/changelog/CHANGELOG.md) r? @Manishearth changelog: none
Changes the span on which the lint is reported to point to only the function return type instead of the entire function body. Fixes #5284
rustup rust-lang/rust#69838 changelog: none
rustup rust-lang/rust#69189 rustups rust-lang/rust#69189 which is part of rust-lang/rust#70085 (at least I think this is the only pr that changes clippy test stdout) changelog: none
rustup rust-lang/rust#69920 changelog: none
Rustup to rust-lang/rust#66131 changelog: none
Rollup of 4 pull requests Successful merges: - #5326 (rustup rust-lang/rust#69838) - #5333 (rustup rust-lang/rust#69189) - #5336 (rustup rust-lang/rust#69920) - #5341 (Rustup to rust-lang/rust#66131) Failed merges: r? @ghost changelog: none
Improvement: Don't show function body in needless_lifetimes Changes the span on which the lint is reported to point to only the function return type instead of the entire function body. Fixes #5284 changelog: none
Fix documentation generation for configurable lints In #5135, the configuration macro changed, but the documentation generation script wasn't updated. This PR catches up on this. [Preview](https://flip1995.github.io/rust-clippy/master/index.html) r? @Manishearth changelog: Document configuration options of lints again.
rustup rust-lang/rust#69968 changelog: none
Fix single binding closure Fix the `match_single_binding` lint when triggered inside a closure. Fixes: #5347 changelog: Improve suggestion for [`match_single_binding`]
…fal/rust-clippy into issue4983_bool_equal_not_bool
Will start from scratch :) |
This is WIP-PR.
Fixes #4983.