-
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
clippy::question_mark lint suggests code that does not compile #8281
Comments
I couldn't get a minimal reproduction but this also triggered on some of my code:
clippy's suggestion does not compile because |
This lint has a bug that causes false positives with suggestions that do not compile. See: rust-lang/rust-clippy#8281
This is fixed in nightly in #8080 (playground), but I can confirm this is a problem in 1.58.0 |
Still present on: Clippy version
Rustc version
The actual issueGot a suggestion like: error: this block may be rewritten with the `?` operator
--> crates/client/src/client/async_client.rs:857:21
|
857 | / if let Err(e) = self.state.process(ok.answers()) {
858 | | Err(e)
859 | | } else {
860 | | Ok(ok)
861 | | }
| |_____________________^ help: replace it with: `self.state.process(ok.answers())?`
|
= note: `-D clippy::question-mark` implied by `-D warnings`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#question_mark Code impacted: https://github.com/bluejekyll/trust-dns/blob/main/crates/client/src/client/async_client.rs#L854-L868 impl<R> Stream for ClientStreamXfr<R>
where
R: Stream<Item = Result<DnsResponse, ProtoError>> + Send + Unpin + 'static,
{
type Item = Result<DnsResponse, ClientError>;
fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
use ClientStreamXfrState::*;
if matches!(self.state, Ended) {
return Poll::Ready(None);
}
let message = if let Some(response) = ready!(self.state.inner().poll_next_unpin(cx)) {
Some(match response {
Ok(ok) => {
if let Err(e) = self.state.process(ok.answers()) {
Err(e)
} else {
Ok(ok)
}
}
Err(e) => Err(e.into()),
})
} else {
None
};
Poll::Ready(message)
}
} the
Running a warning: failed to automatically apply fixes suggested by rustc to crate `trust_dns_client`
after fixes were automatically applied the compiler reported errors within these files:
* crates/client/src/client/async_client.rs
This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag
The following errors were reported:
error[E0308]: `match` arms have incompatible types
--> crates/client/src/client/async_client.rs:858:27
|
854 | Some(match response {
| __________________-
855 | | Ok(ok) => {
856 | | self.state.process(ok.answers())?
| | --------------------------------- this is found to be of type `()`
857 | | }
858 | | Err(e) => Err(e.into()),
| | ^^^^^^^^^^^^^ expected `()`, found enum `std::result::Result`
859 | | })
| |_____________- `match` arms have incompatible types
|
= note: expected unit type `()`
found enum `std::result::Result<_, _>`
error: aborting due to previous error
For more information about this error, try `rustc --explain E0308`.
Original diagnostics will follow. A possible change is doing, but it's questionable. let message = if let Some(response) = ready!(self.state.inner().poll_next_unpin(cx)) {
Some(match response {
Ok(ok) => self.state.process(ok.answers()).map(|_| ok),
Err(e) => Err(e.into()),
})
} else {
None
}; |
Found another instance of the suggested fix not compiling (playground link): let foo = &Some(());
let Some(_) = foo else { return None }; The fix ( |
This might have been fixed by #11983. |
It has not been completely fixed. This will not compile for example: #[derive(Debug, Eq, PartialEq)]
enum B { C, D }
#[derive(Debug)]
struct A(Option<B>);
fn take_c(maybe_b: &mut A) -> Option<B> {
// Take a reference to Some value if it exists or return None
// let Some(ref maybe_c) = maybe_b.0 else { return None; };
// Clippy suggests this instead:
let ref maybe_c = maybe_b.0?;
// Which won't compile because it would move the value..
if *maybe_c == B::C {
maybe_b.0.take()
} else {
None
}
} |
Summary
As part of upgrading pantsbuild to Rust v1.58.0, we had to disable the
clippy::question_mark
lint on one segment of code because it suggested code that (a) is semantically different and (b) does not actually compile.Lint Name
clippy::question_mark
Reproducer
Code at issue: https://github.com/pantsbuild/pants/blob/6759bd7ef7b792f26391e70b6b40556020365e81/src/rust/engine/async_value/src/lib.rs#L102-L105
Clippy error:
Applied this diff:
Following clippy's suggestion results in this error:
Version
Additional Labels
No response
The text was updated successfully, but these errors were encountered: