Skip to content

Commit

Permalink
Rollup merge of #108588 - ehuss:lint-docs-produces, r=eholk
Browse files Browse the repository at this point in the history
Fix the ffi_unwind_calls lint documentation

This fixes the [`ffi_unwind_calls`](https://doc.rust-lang.org/nightly/rustc/lints/listing/allowed-by-default.html#ffi-unwind-calls) documentation to show its output correctly. Currently it is showing the text `{{produces}}` which is not how it should look.

This fixes it by not ignoring the example. I'm not sure why it was ignored, as the way the lint currently works it doesn't seem to require external linkage. This also fixes several mistakes in the example:

* There is no `ffi_unwind_calls` feature.
* Denies the lint (which is otherwise allow be default).
* Removes the `mod impl` which is not valid Rust syntax, and doesn't appear to be needed anyways.

The output now looks like:

```
warning: call to foreign function with FFI-unwind ABI
  --> lint_example.rs:10:14
   |
10 |     unsafe { foo(); }
   |              ^^^^^ call to foreign function with FFI-unwind ABI
   |
note: the lint level is defined here
  --> lint_example.rs:2:9
   |
2  | #![warn(ffi_unwind_calls)]
   |         ^^^^^^^^^^^^^^^^

warning: call to function pointer with FFI-unwind ABI
  --> lint_example.rs:12:14
   |
12 |     unsafe { ptr(); }
   |              ^^^^^ call to function pointer with FFI-unwind ABI

```

This also includes some updates to the lint-docs tool to help with this issue:

* Adds a check if a lint documentation has `{{produces}}` with an ignored example, and generates an error.
* All instances of a lint are now displayed. Previously it only showed the first time the lint fires. Some examples may trigger a lint multiple times, and they are all now displayed.
  • Loading branch information
matthiaskrgr authored Mar 23, 2023
2 parents 5a017d3 + ab2508a commit fc5516b
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 40 deletions.
9 changes: 2 additions & 7 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4008,14 +4008,9 @@ declare_lint! {
///
/// ### Example
///
/// ```rust,ignore (need FFI)
/// #![feature(ffi_unwind_calls)]
/// ```rust
/// #![feature(c_unwind)]
///
/// # mod impl {
/// # #[no_mangle]
/// # pub fn "C-unwind" fn foo() {}
/// # }
/// #![warn(ffi_unwind_calls)]
///
/// extern "C-unwind" {
/// fn foo();
Expand Down
95 changes: 62 additions & 33 deletions src/tools/lint-docs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,36 @@ impl Lint {
fn check_style(&self) -> Result<(), Box<dyn Error>> {
for &expected in &["### Example", "### Explanation", "{{produces}}"] {
if expected == "{{produces}}" && self.is_ignored() {
if self.doc_contains("{{produces}}") {
return Err(format!(
"the lint example has `ignore`, but also contains the {{{{produces}}}} marker\n\
\n\
The documentation generator cannot generate the example output when the \
example is ignored.\n\
Manually include the sample output below the example. For example:\n\
\n\
/// ```rust,ignore (needs command line option)\n\
/// #[cfg(widnows)]\n\
/// fn foo() {{}}\n\
/// ```\n\
///\n\
/// This will produce:\n\
/// \n\
/// ```text\n\
/// warning: unknown condition name used\n\
/// --> lint_example.rs:1:7\n\
/// |\n\
/// 1 | #[cfg(widnows)]\n\
/// | ^^^^^^^\n\
/// |\n\
/// = note: `#[warn(unexpected_cfgs)]` on by default\n\
/// ```\n\
\n\
Replacing the output with the text of the example you \
compiled manually yourself.\n\
"
).into());
}
continue;
}
if !self.doc_contains(expected) {
Expand Down Expand Up @@ -317,10 +347,10 @@ impl<'a> LintExtractor<'a> {
..,
&format!(
"This will produce:\n\
\n\
```text\n\
{}\
```",
\n\
```text\n\
{}\
```",
output
),
);
Expand Down Expand Up @@ -392,37 +422,36 @@ impl<'a> LintExtractor<'a> {
.filter(|line| line.starts_with('{'))
.map(serde_json::from_str)
.collect::<Result<Vec<serde_json::Value>, _>>()?;
match msgs
// First try to find the messages with the `code` field set to our lint.
let matches: Vec<_> = msgs
.iter()
.find(|msg| matches!(&msg["code"]["code"], serde_json::Value::String(s) if s==name))
{
Some(msg) => {
let rendered = msg["rendered"].as_str().expect("rendered field should exist");
Ok(rendered.to_string())
}
None => {
match msgs.iter().find(
|msg| matches!(&msg["rendered"], serde_json::Value::String(s) if s.contains(name)),
) {
Some(msg) => {
let rendered = msg["rendered"].as_str().expect("rendered field should exist");
Ok(rendered.to_string())
}
None => {
let rendered: Vec<&str> =
msgs.iter().filter_map(|msg| msg["rendered"].as_str()).collect();
let non_json: Vec<&str> =
stderr.lines().filter(|line| !line.starts_with('{')).collect();
Err(format!(
"did not find lint `{}` in output of example, got:\n{}\n{}",
name,
non_json.join("\n"),
rendered.join("\n")
)
.into())
}
}
.filter(|msg| matches!(&msg["code"]["code"], serde_json::Value::String(s) if s==name))
.map(|msg| msg["rendered"].as_str().expect("rendered field should exist").to_string())
.collect();
if matches.is_empty() {
// Some lints override their code to something else (E0566).
// Try to find something that looks like it could be our lint.
let matches: Vec<_> = msgs.iter().filter(|msg|
matches!(&msg["rendered"], serde_json::Value::String(s) if s.contains(name)))
.map(|msg| msg["rendered"].as_str().expect("rendered field should exist").to_string())
.collect();
if matches.is_empty() {
let rendered: Vec<&str> =
msgs.iter().filter_map(|msg| msg["rendered"].as_str()).collect();
let non_json: Vec<&str> =
stderr.lines().filter(|line| !line.starts_with('{')).collect();
Err(format!(
"did not find lint `{}` in output of example, got:\n{}\n{}",
name,
non_json.join("\n"),
rendered.join("\n")
)
.into())
} else {
Ok(matches.join("\n"))
}
} else {
Ok(matches.join("\n"))
}
}

Expand Down

0 comments on commit fc5516b

Please sign in to comment.