10594: fix: Generate and complete rustdoc lints r=Veykril a=Veykril

Fixes https://github.com/rust-analyzer/rust-analyzer/issues/10572, https://github.com/rust-analyzer/rust-analyzer/issues/8349
bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
This commit is contained in:
bors[bot] 2021-10-20 12:05:54 +00:00 committed by GitHub
commit c73aa7a214
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 248 additions and 160 deletions

View file

@ -4,7 +4,7 @@
//! for built-in attributes. //! for built-in attributes.
use hir::HasAttrs; use hir::HasAttrs;
use ide_db::helpers::generated_lints::{CLIPPY_LINTS, DEFAULT_LINTS, FEATURES}; use ide_db::helpers::generated_lints::{CLIPPY_LINTS, DEFAULT_LINTS, FEATURES, RUSTDOC_LINTS};
use itertools::Itertools; use itertools::Itertools;
use once_cell::sync::Lazy; use once_cell::sync::Lazy;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
@ -29,12 +29,16 @@ pub(crate) fn complete_attribute(acc: &mut Completions, ctx: &CompletionContext)
}; };
match (name_ref, attribute.token_tree()) { match (name_ref, attribute.token_tree()) {
(Some(path), Some(token_tree)) => match path.text().as_str() { (Some(path), Some(token_tree)) => match path.text().as_str() {
"derive" => derive::complete_derive(acc, ctx, token_tree),
"repr" => repr::complete_repr(acc, ctx, token_tree), "repr" => repr::complete_repr(acc, ctx, token_tree),
"feature" => lint::complete_lint(acc, ctx, token_tree, FEATURES), "derive" => derive::complete_derive(acc, ctx, &parse_comma_sep_paths(token_tree)?),
"feature" => {
lint::complete_lint(acc, ctx, &parse_comma_sep_paths(token_tree)?, FEATURES)
}
"allow" | "warn" | "deny" | "forbid" => { "allow" | "warn" | "deny" | "forbid" => {
lint::complete_lint(acc, ctx, token_tree.clone(), DEFAULT_LINTS); let existing_lints = parse_comma_sep_paths(token_tree)?;
lint::complete_lint(acc, ctx, token_tree, CLIPPY_LINTS); lint::complete_lint(acc, ctx, &existing_lints, DEFAULT_LINTS);
lint::complete_lint(acc, ctx, &existing_lints, CLIPPY_LINTS);
lint::complete_lint(acc, ctx, &existing_lints, RUSTDOC_LINTS);
} }
"cfg" => { "cfg" => {
cfg::complete_cfg(acc, ctx); cfg::complete_cfg(acc, ctx);

View file

@ -14,9 +14,8 @@ use crate::{
pub(super) fn complete_derive( pub(super) fn complete_derive(
acc: &mut Completions, acc: &mut Completions,
ctx: &CompletionContext, ctx: &CompletionContext,
derive_input: ast::TokenTree, existing_derives: &[ast::Path],
) { ) {
if let Some(existing_derives) = super::parse_comma_sep_paths(derive_input.clone()) {
let core = FamousDefs(&ctx.sema, ctx.krate).core(); let core = FamousDefs(&ctx.sema, ctx.krate).core();
let existing_derives: FxHashSet<_> = existing_derives let existing_derives: FxHashSet<_> = existing_derives
.into_iter() .into_iter()
@ -57,8 +56,7 @@ pub(super) fn complete_derive(
_ => (&*name, None), _ => (&*name, None),
}; };
let mut item = let mut item = CompletionItem::new(CompletionKind::Attribute, ctx.source_range(), label);
CompletionItem::new(CompletionKind::Attribute, ctx.source_range(), label);
item.kind(CompletionItemKind::Attribute); item.kind(CompletionItemKind::Attribute);
if let Some(docs) = mac.docs(ctx.db) { if let Some(docs) = mac.docs(ctx.db) {
item.documentation(docs); item.documentation(docs);
@ -69,7 +67,6 @@ pub(super) fn complete_derive(
item.add_to(acc); item.add_to(acc);
} }
} }
}
fn get_derives_in_scope(ctx: &CompletionContext) -> Vec<(hir::Name, MacroDef)> { fn get_derives_in_scope(ctx: &CompletionContext) -> Vec<(hir::Name, MacroDef)> {
let mut result = Vec::default(); let mut result = Vec::default();

View file

@ -11,10 +11,10 @@ use crate::{
pub(super) fn complete_lint( pub(super) fn complete_lint(
acc: &mut Completions, acc: &mut Completions,
ctx: &CompletionContext, ctx: &CompletionContext,
derive_input: ast::TokenTree, existing_lints: &[ast::Path],
lints_completions: &[Lint], lints_completions: &[Lint],
) { ) {
if let Some(existing_lints) = super::parse_comma_sep_paths(derive_input) { let is_qualified = ctx.previous_token_is(T![:]);
for &Lint { label, description } in lints_completions { for &Lint { label, description } in lints_completions {
let (qual, name) = { let (qual, name) = {
// FIXME: change `Lint`'s label to not store a path in it but split the prefix off instead? // FIXME: change `Lint`'s label to not store a path in it but split the prefix off instead?
@ -29,6 +29,10 @@ pub(super) fn complete_lint(
None => (None, ns_or_label), None => (None, ns_or_label),
} }
}; };
if qual.is_none() && is_qualified {
// qualified completion requested, but this lint is unqualified
continue;
}
let lint_already_annotated = existing_lints let lint_already_annotated = existing_lints
.iter() .iter()
.filter_map(|path| { .filter_map(|path| {
@ -50,21 +54,13 @@ pub(super) fn complete_lint(
if lint_already_annotated { if lint_already_annotated {
continue; continue;
} }
let insert = match (qual, ctx.previous_token_is(T![:])) { let label = match qual {
(Some(qual), false) => format!("{}::{}", qual, name), Some(qual) if !is_qualified => format!("{}::{}", qual, name),
// user is completing a qualified path but this completion has no qualifier
// so discard this completion
// FIXME: This is currently very hacky and will propose odd completions if
// we add more qualified (tool) completions other than clippy
(None, true) => continue,
_ => name.to_owned(), _ => name.to_owned(),
}; };
let mut item = let mut item = CompletionItem::new(CompletionKind::Attribute, ctx.source_range(), label);
CompletionItem::new(CompletionKind::Attribute, ctx.source_range(), label);
item.kind(CompletionItemKind::Attribute) item.kind(CompletionItemKind::Attribute)
.insert_text(insert)
.documentation(hir::Documentation::new(description.to_owned())); .documentation(hir::Documentation::new(description.to_owned()));
item.add_to(acc) item.add_to(acc)
} }
} }
}

View file

@ -693,11 +693,29 @@ mod lint {
#[test] #[test]
fn lint_clippy_qualified() { fn lint_clippy_qualified() {
check_edit( check_edit(
"clippy::as_conversions", "as_conversions",
r#"#[allow(clippy::$0)] struct Test;"#, r#"#[allow(clippy::$0)] struct Test;"#,
r#"#[allow(clippy::as_conversions)] struct Test;"#, r#"#[allow(clippy::as_conversions)] struct Test;"#,
); );
} }
#[test]
fn lint_rustdoc_unqualified() {
check_edit(
"rustdoc::bare_urls",
r#"#[allow($0)] struct Test;"#,
r#"#[allow(rustdoc::bare_urls)] struct Test;"#,
);
}
#[test]
fn lint_rustdoc_qualified() {
check_edit(
"bare_urls",
r#"#[allow(rustdoc::$0)] struct Test;"#,
r#"#[allow(rustdoc::bare_urls)] struct Test;"#,
);
}
} }
mod repr { mod repr {

View file

@ -502,6 +502,46 @@ pub const DEFAULT_LINTS: &[Lint] = &[
}, },
]; ];
pub const RUSTDOC_LINTS: &[Lint] = &[
Lint {
label: "rustdoc::all",
description: r##"lint group for: rustdoc::broken-intra-doc-links, rustdoc::private-intra-doc-links, rustdoc::missing-doc-code-examples, rustdoc::private-doc-tests, rustdoc::invalid-codeblock-attributes, rustdoc::invalid-rust-codeblocks, rustdoc::invalid-html-tags, rustdoc::bare-urls, rustdoc::missing-crate-level-docs"##,
},
Lint { label: "rustdoc::bare_urls", description: r##"detects URLs that are not hyperlinks"## },
Lint {
label: "rustdoc::broken_intra_doc_links",
description: r##"failures in resolving intra-doc link targets"##,
},
Lint {
label: "rustdoc::invalid_codeblock_attributes",
description: r##"codeblock attribute looks a lot like a known one"##,
},
Lint {
label: "rustdoc::invalid_html_tags",
description: r##"detects invalid HTML tags in doc comments"##,
},
Lint {
label: "rustdoc::invalid_rust_codeblocks",
description: r##"codeblock could not be parsed as valid Rust or is empty"##,
},
Lint {
label: "rustdoc::missing_crate_level_docs",
description: r##"detects crates with no crate-level documentation"##,
},
Lint {
label: "rustdoc::missing_doc_code_examples",
description: r##"detects publicly-exported items without code samples in their documentation"##,
},
Lint {
label: "rustdoc::private_doc_tests",
description: r##"detects code samples in docs of private items not documented by rustdoc"##,
},
Lint {
label: "rustdoc::private_intra_doc_links",
description: r##"linking from a public item to a private one"##,
},
];
pub const FEATURES: &[Lint] = &[ pub const FEATURES: &[Lint] = &[
Lint { Lint {
label: "abi_c_cmse_nonsecure_call", label: "abi_c_cmse_nonsecure_call",
@ -5572,11 +5612,9 @@ outside ticks in documentation."##,
}, },
Lint { Lint {
label: "clippy::double_must_use", label: "clippy::double_must_use",
description: r##"Checks for a [`#[must_use]`] attribute without description: r##"Checks for a `#[must_use]` attribute without
further information on functions and methods that return a type already further information on functions and methods that return a type already
marked as `#[must_use]`. marked as `#[must_use]`."##,
[`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute"##,
}, },
Lint { Lint {
label: "clippy::double_neg", label: "clippy::double_neg",
@ -5821,6 +5859,12 @@ derives the Copy trait"##,
label: "clippy::forget_ref", label: "clippy::forget_ref",
description: r##"Checks for calls to `std::mem::forget` with a reference description: r##"Checks for calls to `std::mem::forget` with a reference
instead of an owned value."##, instead of an owned value."##,
},
Lint {
label: "clippy::format_in_format_args",
description: r##"Detects `format!` within the arguments of another macro that does
formatting such as `format!` itself, `write!` or `println!`. Suggests
inlining the `format!` call."##,
}, },
Lint { Lint {
label: "clippy::from_iter_instead_of_collect", label: "clippy::from_iter_instead_of_collect",
@ -6120,8 +6164,7 @@ where expr has a type that implements `Drop`"##,
}, },
Lint { Lint {
label: "clippy::let_underscore_must_use", label: "clippy::let_underscore_must_use",
description: r##"Checks for `let _ = <expr>` description: r##"Checks for `let _ = <expr>` where expr is `#[must_use]`"##,
where expr is #[must_use]"##,
}, },
Lint { label: "clippy::let_unit_value", description: r##"Checks for binding a unit value."## }, Lint { label: "clippy::let_unit_value", description: r##"Checks for binding a unit value."## },
Lint { Lint {
@ -6194,23 +6237,7 @@ be more readably expressed as `(3..8).contains(x)`."##,
}, },
Lint { Lint {
label: "clippy::manual_split_once", label: "clippy::manual_split_once",
description: r##"**What it does:** Checks for usages of `str::splitn(2, _)` description: r##"Checks for usages of `str::splitn(2, _)`"##,
**Why is this bad?** `split_once` is both clearer in intent and slightly more efficient.
**Known problems:** None.
**Example:**
```rust
// Bad
let (key, value) = _.splitn(2, '=').next_tuple()?;
let value = _.splitn(2, '=').nth(1)?;
// Good
let (key, value) = _.split_once('=')?;
let value = _.split_once('=')?.1;
```"##,
}, },
Lint { Lint {
label: "clippy::manual_str_repeat", label: "clippy::manual_str_repeat",
@ -6304,6 +6331,10 @@ instead. It also checks for `if let &foo = bar` blocks."##,
label: "clippy::match_single_binding", label: "clippy::match_single_binding",
description: r##"Checks for useless match that binds to only one value."##, description: r##"Checks for useless match that binds to only one value."##,
}, },
Lint {
label: "clippy::match_str_case_mismatch",
description: r##"Checks for `match` expressions modifying the case of a string with non-compliant arms"##,
},
Lint { Lint {
label: "clippy::match_wild_err_arm", label: "clippy::match_wild_err_arm",
description: r##"Checks for arm which matches all errors with `Err(_)` description: r##"Checks for arm which matches all errors with `Err(_)`
@ -6433,17 +6464,13 @@ used."##,
Lint { Lint {
label: "clippy::must_use_candidate", label: "clippy::must_use_candidate",
description: r##"Checks for public functions that have no description: r##"Checks for public functions that have no
[`#[must_use]`] attribute, but return something not already marked `#[must_use]` attribute, but return something not already marked
must-use, have no mutable arg and mutate no statics. must-use, have no mutable arg and mutate no statics."##,
[`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute"##,
}, },
Lint { Lint {
label: "clippy::must_use_unit", label: "clippy::must_use_unit",
description: r##"Checks for a [`#[must_use]`] attribute on description: r##"Checks for a `#[must_use]` attribute on
unit-returning functions and methods. unit-returning functions and methods."##,
[`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute"##,
}, },
Lint { Lint {
label: "clippy::mut_from_ref", label: "clippy::mut_from_ref",
@ -6591,6 +6618,10 @@ implementation of
label: "clippy::no_effect", label: "clippy::no_effect",
description: r##"Checks for statements which have no effect."##, description: r##"Checks for statements which have no effect."##,
}, },
Lint {
label: "clippy::no_effect_underscore_binding",
description: r##"Checks for binding to underscore prefixed variable without side-effects."##,
},
Lint { Lint {
label: "clippy::non_ascii_literal", label: "clippy::non_ascii_literal",
description: r##"Checks for non-ASCII characters in string literals."##, description: r##"Checks for non-ASCII characters in string literals."##,
@ -7155,6 +7186,12 @@ assign a value in it."##,
label: "clippy::to_string_in_display", label: "clippy::to_string_in_display",
description: r##"Checks for uses of `to_string()` in `Display` traits."##, description: r##"Checks for uses of `to_string()` in `Display` traits."##,
}, },
Lint {
label: "clippy::to_string_in_format_args",
description: r##"Checks for [`ToString::to_string`](https://doc.rust-lang.org/std/string/trait.ToString.html#tymethod.to_string)
applied to a type that implements [`Display`](https://doc.rust-lang.org/std/fmt/trait.Display.html)
in a macro that does formatting."##,
},
Lint { label: "clippy::todo", description: r##"Checks for usage of `todo!`."## }, Lint { label: "clippy::todo", description: r##"Checks for usage of `todo!`."## },
Lint { Lint {
label: "clippy::too_many_arguments", label: "clippy::too_many_arguments",
@ -7194,6 +7231,7 @@ syntax specifications for trait bounds are used simultaneously."##,
label: "clippy::transmute_int_to_float", label: "clippy::transmute_int_to_float",
description: r##"Checks for transmutes from an integer to a float."##, description: r##"Checks for transmutes from an integer to a float."##,
}, },
Lint { label: "clippy::transmute_num_to_bytes", description: r##""## },
Lint { Lint {
label: "clippy::transmute_ptr_to_ptr", label: "clippy::transmute_ptr_to_ptr",
description: r##"Checks for transmutes from a pointer to a pointer, or description: r##"Checks for transmutes from a pointer to a pointer, or
@ -7256,6 +7294,12 @@ that is not equal to its
label: "clippy::uninit_assumed_init", label: "clippy::uninit_assumed_init",
description: r##"Checks for `MaybeUninit::uninit().assume_init()`."##, description: r##"Checks for `MaybeUninit::uninit().assume_init()`."##,
}, },
Lint {
label: "clippy::uninit_vec",
description: r##"Checks for `set_len()` call that creates `Vec` with uninitialized elements.
This is commonly caused by calling `set_len()` right after allocating or
reserving a buffer with `new()`, `default()`, `with_capacity()`, or `reserve()`."##,
},
Lint { Lint {
label: "clippy::unit_arg", label: "clippy::unit_arg",
description: r##"Checks for passing a unit value as an argument to a function without using a description: r##"Checks for passing a unit value as an argument to a function without using a

View file

@ -1,6 +1,7 @@
//! Generates descriptors structure for unstable feature from Unstable Book //! Generates descriptors structure for unstable feature from Unstable Book
use std::{borrow::Cow, fs, path::Path}; use std::{borrow::Cow, fs, path::Path};
use itertools::Itertools;
use stdx::format_to; use stdx::format_to;
use test_utils::project_root; use test_utils::project_root;
use xshell::cmd; use xshell::cmd;
@ -43,37 +44,60 @@ pub struct Lint {
} }
fn generate_lint_descriptor(buf: &mut String) { fn generate_lint_descriptor(buf: &mut String) {
let stdout = cmd!("rustc -W help").read().unwrap(); // FIXME: rustdoc currently requires an input file for -Whelp cc https://github.com/rust-lang/rust/pull/88831
let file = project_root().join(file!());
let stdout = cmd!("rustdoc -W help {file}").read().unwrap();
let start_lints = stdout.find("---- ------- -------").unwrap(); let start_lints = stdout.find("---- ------- -------").unwrap();
let start_lint_groups = stdout.find("---- ---------").unwrap(); let start_lint_groups = stdout.find("---- ---------").unwrap();
let end_lints = stdout.find("Lint groups provided by rustc:").unwrap(); let start_lints_rustdoc =
let end_lint_groups = stdout stdout.find("Lint checks provided by plugins loaded by this crate:").unwrap();
.find("Lint tools like Clippy can provide additional lints and lint groups.") let start_lint_groups_rustdoc =
.unwrap(); stdout.find("Lint groups provided by plugins loaded by this crate:").unwrap();
buf.push_str(r#"pub const DEFAULT_LINTS: &[Lint] = &["#); buf.push_str(r#"pub const DEFAULT_LINTS: &[Lint] = &["#);
buf.push('\n'); buf.push('\n');
let mut lints = stdout[start_lints..end_lints]
.lines() let lints = stdout[start_lints..].lines().skip(1).take_while(|l| !l.is_empty()).map(|line| {
.skip(1) let (name, rest) = line.trim().split_once(char::is_whitespace).unwrap();
.filter(|l| !l.is_empty()) let (_default_level, description) = rest.trim().split_once(char::is_whitespace).unwrap();
.map(|line| { (name.trim(), Cow::Borrowed(description.trim()))
});
let lint_groups =
stdout[start_lint_groups..].lines().skip(1).take_while(|l| !l.is_empty()).map(|line| {
let (name, lints) = line.trim().split_once(char::is_whitespace).unwrap();
(name.trim(), format!("lint group for: {}", lints.trim()).into())
});
lints.chain(lint_groups).sorted_by(|(ident, _), (ident2, _)| ident.cmp(ident2)).for_each(
|(name, description)| push_lint_completion(buf, &name.replace("-", "_"), &description),
);
buf.push_str("];\n");
// rustdoc
buf.push('\n');
buf.push_str(r#"pub const RUSTDOC_LINTS: &[Lint] = &["#);
buf.push('\n');
let lints_rustdoc =
stdout[start_lints_rustdoc..].lines().skip(2).take_while(|l| !l.is_empty()).map(|line| {
let (name, rest) = line.trim().split_once(char::is_whitespace).unwrap(); let (name, rest) = line.trim().split_once(char::is_whitespace).unwrap();
let (_default_level, description) = let (_default_level, description) =
rest.trim().split_once(char::is_whitespace).unwrap(); rest.trim().split_once(char::is_whitespace).unwrap();
(name.trim(), Cow::Borrowed(description.trim())) (name.trim(), Cow::Borrowed(description.trim()))
}) });
.collect::<Vec<_>>(); let lint_groups_rustdoc =
lints.extend( stdout[start_lint_groups_rustdoc..].lines().skip(2).take_while(|l| !l.is_empty()).map(
stdout[start_lint_groups..end_lint_groups].lines().skip(1).filter(|l| !l.is_empty()).map(
|line| { |line| {
let (name, lints) = line.trim().split_once(char::is_whitespace).unwrap(); let (name, lints) = line.trim().split_once(char::is_whitespace).unwrap();
(name.trim(), format!("lint group for: {}", lints.trim()).into()) (name.trim(), format!("lint group for: {}", lints.trim()).into())
}, },
),
); );
lints.sort_by(|(ident, _), (ident2, _)| ident.cmp(ident2)); lints_rustdoc
lints.into_iter().for_each(|(name, description)| { .chain(lint_groups_rustdoc)
.sorted_by(|(ident, _), (ident2, _)| ident.cmp(ident2))
.for_each(|(name, description)| {
push_lint_completion(buf, &name.replace("-", "_"), &description) push_lint_completion(buf, &name.replace("-", "_"), &description)
}); });
buf.push_str("];\n"); buf.push_str("];\n");
@ -126,8 +150,13 @@ fn generate_descriptor_clippy(buf: &mut String, path: &Path) {
clippy_lints.push(clippy_lint) clippy_lints.push(clippy_lint)
} else if let Some(line) = line.strip_prefix(r#""docs": ""#) { } else if let Some(line) = line.strip_prefix(r#""docs": ""#) {
let prefix_to_strip = r#" ### What it does"#; let prefix_to_strip = r#" ### What it does"#;
// FIXME: replace unwrap_or with expect again, currently there is one lint that uses a different format in the json... let line = match line.strip_prefix(prefix_to_strip) {
let line = line.strip_prefix(prefix_to_strip).unwrap_or(line); Some(line) => line,
None => {
eprintln!("unexpected clippy prefix for {}", clippy_lints.last().unwrap().id);
continue;
}
};
// Only take the description, any more than this is a lot of additional data we would embed into the exe // Only take the description, any more than this is a lot of additional data we would embed into the exe
// which seems unnecessary // which seems unnecessary
let up_to = line.find(r#"###"#).expect("no second section found?"); let up_to = line.find(r#"###"#).expect("no second section found?");