-
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
Implement cargo lint
and fix some clippy errors
#1374
Conversation
We should turn it on after Iterator::copied stabilizes
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.
Awesome, thank you!
It all looks good to me except the one lint that suggest flipping if let Err(_) = foo
to if foo.is_err()
, which I don't really agree with :-)
r=me with that lint silenced!
bors delegate+
@@ -334,7 +334,7 @@ fn best_action_for_target<'b, 'a: 'b>( | |||
.filter_map(ast::UseItem::use_tree) | |||
.map(|u| walk_use_tree_for_best_action(&mut storage, None, u, target)) | |||
.fold(None, |best, a| { | |||
best.and_then(|best| Some(*ImportAction::better(&best, &a))).or(Some(a)) | |||
best.and_then(|best| Some(*ImportAction::better(&best, &a))).or_else(|| Some(a)) |
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.
I feel conflicted about this, it seems like closure creates noise for little benefit here. But I guess it's ok?
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.
Yeah, I think the fact that clippy interprets these constructors as "function calls" is pretty unfortunate, It looks like a bug (rust-lang/rust-clippy#1609), but for now I figured that it'd be better to make the changes just so we can get all the lint errors out of the way.
crates/ra_project_model/src/lib.rs
Outdated
@@ -183,7 +183,7 @@ impl ProjectWorkspace { | |||
if let (Some(&from), Some(&to)) = | |||
(sysroot_crates.get(&from), sysroot_crates.get(&to)) | |||
{ | |||
if let Err(_) = crate_graph.add_dep(from, name.into(), to) { |
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.
Let's disable this lint? I find if let
somewhat more readable, and, with result, Err(_)
also sends a strong signal that an error is ignored.
✌️ alanhdu can now approve this pull request |
bors r+ |
1374: Implement `cargo lint` and fix some clippy errors r=alanhdu a=alanhdu This creates a `cargo lint` command that runs clippy with certain lints disabled. I've also gone ahead and fixed some of the lint errors, although there are many more still to go. cc #848 Co-authored-by: Alan Du <[email protected]>
Build succeeded |
This creates a
cargo lint
command that runs clippy with certain lints disabled. I've also gone ahead and fixed some of the lint errors, although there are many more still to go.cc #848