-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 fails to check project but no error shown, due to doc comment #6022
Comments
I tried to poke around with some debug prints but I'm still not sure what causes this... diff --git a/src/main.rs b/src/main.rs
index ea0674339..05b5abfeb 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -53,7 +53,7 @@ pub fn main() {
return;
}
- if let Err(code) = process(env::args().skip(2)) {
+if let Err(code) = dbg!(process(env::args().skip(2))) {
process::exit(code);
}
}
@@ -157,12 +157,12 @@ impl ClippyCmd {
.collect();
cmd.env(self.path_env(), Self::path())
- .envs(ClippyCmd::target_dir())
- .env("CLIPPY_ARGS", clippy_args)
- .arg(self.cargo_subcommand)
+ .envs(dbg!(ClippyCmd::target_dir()))
+ .env("CLIPPY_ARGS", dbg!(clippy_args))
+ .arg(dbg!(self.cargo_subcommand))
.args(&self.args);
- cmd
+ dbg!(cmd)
}
}
@@ -172,7 +172,7 @@ where
{
let cmd = ClippyCmd::new(old_args);
- let mut cmd = cmd.into_std_cmd();
+ let mut cmd = dbg!(cmd.into_std_cmd());
let exit_status = cmd
.spawn()
@@ -180,6 +180,8 @@ where
.wait()
.expect("failed to wait for cargo?");
+ dbg!(exit_status);
+
if exit_status.success() {
Ok(())
} else {
|
Hmm, happens major release |
Someone encountered the same exit code in cargo here: rust-lang/cargo#7147 Maybe this is caused by our cargo tests? |
I would be very surprised since clippy should not execute cargo tests. I also don't understand why cargo should lock the cargo_home for clippy, hm... For the record I wondered if |
Oh wait, I meant cargo lints not tests 😅 |
|
Isn't this because cargo has a feature to deny warnings? You were running with |
I don't think this is the cause. |
So, I spent a some time reducing the issue and came down to this (as main.rs): /// Adds a platform-specific dependency. Example:
/// ```
/// [target.'cfg(windows)'.dependencies]
/// foo = {version = "1.0"}
/// ```
fn main() {
println!("Hello, world!");
} The problem seems to be the doc comment. |
I made it a bit smaller. /// ```
/// 'a()'
/// ```
fn main() {} |
I tested this a bit more, and it seems, that something with the string parsing/lexing in doc comments goes wrong. This happens when a string is not properly enclosed, like
All of this produces an error without a message when put in the doc comment. The example by @keiichiw is kind of an outlier. IIUC it parses Maybe a bug in |
We're interpreting the fenced code block as rust code (there's no label), and the parser chokes, so there's a panic. I'll see what would be the best way to recover from this later. Backtrace
|
clippy 1.48.0 has a bug in string parsing in doc comments. rust-lang/rust-clippy#6022 To avoid this issue, update to use double-quotation instead of single-quotation inside a code block in a doc comment. Also, this fixes wrong examples of commands there at the same time. BUG=none TEST=bin/clippy Change-Id: I90e5699f6d4e839304f9d4e05e5c7b21467744cd Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2592001 Tested-by: Keiichi Watanabe <keiichiw@chromium.org> Reviewed-by: Chirantan Ekbote <chirantan@chromium.org> Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Do you know why clippy did not properly panic here? |
@matthiaskrgr yep, that happened because of two reasons:
The PR adds a call to Note that stack unwinding is used during "normal operation", a fatal error is not an ICE. |
EDIT: reduced even further by
@keiichiw
This throws an error on my machine but it does not say what the actual problem is, as if something was eating the error message:
I tried
cargo check
but that seems to work and does not throw any errors.Meta
The text was updated successfully, but these errors were encountered: