X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;ds=sidebyside;f=clippy_lints%2Fsrc%2Fdoc.rs;h=c39829fdc7aad2b6bbc3e4885a3ed39d45750e80;hb=12c61612f7a91df64121dd9c991828c26d665325;hp=d9952325c4bbd7ddf2e56de9ea80a4b61befc4cf;hpb=8af28840d2b493897a54eb9a21e155c09e38f10f;p=rust.git diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index d9952325c4b..c39829fdc7a 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -1,6 +1,7 @@ -use crate::utils::{is_entrypoint_fn, is_expn_of, match_panic_def_id, method_chain_args, return_ty}; -use clippy_utils::diagnostics::{span_lint, span_lint_and_note}; +use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_note}; +use clippy_utils::source::first_line_of_span; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; +use clippy_utils::{is_entrypoint_fn, is_expn_of, match_panic_def_id, method_chain_args, return_ty}; use if_chain::if_chain; use itertools::Itertools; use rustc_ast::ast::{Async, AttrKind, Attribute, FnKind, FnRetTy, ItemKind}; @@ -11,7 +12,7 @@ use rustc_errors::Handler; use rustc_hir as hir; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; -use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_hir::{AnonConst, Expr, ExprKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; @@ -25,26 +26,31 @@ use rustc_span::{sym, FileName, Pos}; use std::io; use std::ops::Range; +use std::thread; use url::Url; declare_clippy_lint! { - /// **What it does:** Checks for the presence of `_`, `::` or camel-case words + /// ### What it does + /// Checks for the presence of `_`, `::` or camel-case words /// outside ticks in documentation. /// - /// **Why is this bad?** *Rustdoc* supports markdown formatting, `_`, `::` and + /// ### Why is this bad? + /// *Rustdoc* supports markdown formatting, `_`, `::` and /// camel-case probably indicates some code which should be included between /// ticks. `_` can also be used for emphasis in markdown, this lint tries to /// consider that. /// - /// **Known problems:** Lots of bad docs won’t be fixed, what the lint checks - /// for is limited, and there are still false positives. + /// ### Known problems + /// Lots of bad docs won’t be fixed, what the lint checks + /// for is limited, and there are still false positives. HTML elements and their + /// content are not linted. /// /// In addition, when writing documentation comments, including `[]` brackets /// inside a link text would trip the parser. Therfore, documenting link with /// `[`SmallVec<[T; INLINE_CAPACITY]>`]` and then [`SmallVec<[T; INLINE_CAPACITY]>`]: SmallVec /// would fail. /// - /// **Examples:** + /// ### Examples /// ```rust /// /// Do something with the foo_bar parameter. See also /// /// that::other::module::foo. @@ -65,15 +71,15 @@ } declare_clippy_lint! { - /// **What it does:** Checks for the doc comments of publicly visible + /// ### What it does + /// Checks for the doc comments of publicly visible /// unsafe functions and warns if there is no `# Safety` section. /// - /// **Why is this bad?** Unsafe functions should document their safety + /// ### Why is this bad? + /// Unsafe functions should document their safety /// preconditions, so that users can be sure they are using them safely. /// - /// **Known problems:** None. - /// - /// **Examples:** + /// ### Examples /// ```rust ///# type Universe = (); /// /// This function should really be documented @@ -99,16 +105,15 @@ } declare_clippy_lint! { - /// **What it does:** Checks the doc comments of publicly visible functions that + /// ### What it does + /// Checks the doc comments of publicly visible functions that /// return a `Result` type and warns if there is no `# Errors` section. /// - /// **Why is this bad?** Documenting the type of errors that can be returned from a + /// ### Why is this bad? + /// Documenting the type of errors that can be returned from a /// function can help callers write code to handle the errors appropriately. /// - /// **Known problems:** None. - /// - /// **Examples:** - /// + /// ### Examples /// Since the following function returns a `Result` it has an `# Errors` section in /// its doc comment: /// @@ -128,16 +133,15 @@ } declare_clippy_lint! { - /// **What it does:** Checks the doc comments of publicly visible functions that + /// ### What it does + /// Checks the doc comments of publicly visible functions that /// may panic and warns if there is no `# Panics` section. /// - /// **Why is this bad?** Documenting the scenarios in which panicking occurs + /// ### Why is this bad? + /// Documenting the scenarios in which panicking occurs /// can help callers who do not want to panic to avoid those situations. /// - /// **Known problems:** None. - /// - /// **Examples:** - /// + /// ### Examples /// Since the following function may panic it has a `# Panics` section in /// its doc comment: /// @@ -159,14 +163,14 @@ } declare_clippy_lint! { - /// **What it does:** Checks for `fn main() { .. }` in doctests + /// ### What it does + /// Checks for `fn main() { .. }` in doctests /// - /// **Why is this bad?** The test can be shorter (and likely more readable) + /// ### Why is this bad? + /// The test can be shorter (and likely more readable) /// if the `fn main()` is left implicit. /// - /// **Known problems:** None. - /// - /// **Examples:** + /// ### Examples /// ``````rust /// /// An example of a doctest with a `main()` function /// /// @@ -383,7 +387,7 @@ pub fn strip_doc_comment_decoration(doc: &str, comment_kind: CommentKind, span: let mut no_stars = String::with_capacity(doc.len()); for line in doc.lines() { let mut chars = line.chars(); - while let Some(c) = chars.next() { + for c in &mut chars { if c.is_whitespace() { no_stars.push(c); } else { @@ -469,11 +473,11 @@ fn check_doc<'a, Events: Iterator, Range DocHeaders { // true if a safety header was found - use pulldown_cmark::CodeBlockKind; use pulldown_cmark::Event::{ Code, End, FootnoteReference, HardBreak, Html, Rule, SoftBreak, Start, TaskListMarker, Text, }; - use pulldown_cmark::Tag::{CodeBlock, Heading, Link}; + use pulldown_cmark::Tag::{CodeBlock, Heading, Item, Link, Paragraph}; + use pulldown_cmark::{CodeBlockKind, CowStr}; let mut headers = DocHeaders { safety: false, @@ -485,6 +489,9 @@ fn check_doc<'a, Events: Iterator, Range, Span)> = Vec::new(); + let mut paragraph_span = spans.get(0).expect("function isn't called if doc comment is empty").1; for (event, range) in events { match event { Start(CodeBlock(ref kind)) => { @@ -510,13 +517,42 @@ fn check_doc<'a, Events: Iterator, Range in_link = Some(url), End(Link(..)) => in_link = None, - Start(Heading(_)) => in_heading = true, - End(Heading(_)) => in_heading = false, + Start(Heading(_) | Paragraph | Item) => { + if let Start(Heading(_)) = event { + in_heading = true; + } + ticks_unbalanced = false; + let (_, span) = get_current_span(spans, range.start); + paragraph_span = first_line_of_span(cx, span); + }, + End(Heading(_) | Paragraph | Item) => { + if let End(Heading(_)) = event { + in_heading = false; + } + if ticks_unbalanced { + span_lint_and_help( + cx, + DOC_MARKDOWN, + paragraph_span, + "backticks are unbalanced", + None, + "a backtick may be missing a pair", + ); + } else { + for (text, span) in text_to_check { + check_text(cx, valid_idents, &text, span); + } + } + text_to_check = Vec::new(); + }, Start(_tag) | End(_tag) => (), // We don't care about other tags Html(_html) => (), // HTML is weird, just ignore it SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (), FootnoteReference(text) | Text(text) => { - if Some(&text) == in_link.as_ref() { + let (begin, span) = get_current_span(spans, range.start); + paragraph_span = paragraph_span.with_hi(span.hi()); + ticks_unbalanced |= text.contains('`') && !in_code; + if Some(&text) == in_link.as_ref() || ticks_unbalanced { // Probably a link of the form `` // Which are represented as a link to "http://example.com" with // text "http://example.com" by pulldown-cmark @@ -525,11 +561,6 @@ fn check_doc<'a, Events: Iterator, Range o, - Err(e) => e - 1, - }; - let (begin, span) = spans[index]; if in_code { if is_rust { let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition()); @@ -538,8 +569,7 @@ fn check_doc<'a, Events: Iterator, Range, Range (usize, Span) { + let index = match spans.binary_search_by(|c| c.0.cmp(&idx)) { + Ok(o) => o, + Err(e) => e - 1, + }; + spans[index] +} + fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) { - fn has_needless_main(code: &str, edition: Edition) -> bool { + fn has_needless_main(code: String, edition: Edition) -> bool { rustc_driver::catch_fatal_errors(|| { - rustc_span::with_session_globals(edition, || { - let filename = FileName::anon_source_code(code); + rustc_span::create_session_globals_then(edition, || { + let filename = FileName::anon_source_code(&code); let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); let emitter = EmitterWriter::new(box io::sink(), None, false, false, false, None, false); let handler = Handler::with_emitter(false, None, box emitter); let sess = ParseSess::with_span_handler(handler, sm); - let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code.into()) { + let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code) { Ok(p) => p, Err(errs) => { for mut err in errs { @@ -583,7 +621,7 @@ fn has_needless_main(code: &str, edition: Edition) -> bool { let returns_nothing = match &sig.decl.output { FnRetTy::Default(..) => true, FnRetTy::Ty(ty) if ty.kind.is_unit() => true, - _ => false, + FnRetTy::Ty(_) => false, }; if returns_nothing && !is_async && !block.stmts.is_empty() { @@ -613,7 +651,13 @@ fn has_needless_main(code: &str, edition: Edition) -> bool { .unwrap_or_default() } - if has_needless_main(text, edition) { + // Because of the global session, we need to create a new session in a different thread with + // the edition we need. + let text = text.to_owned(); + if thread::spawn(move || has_needless_main(text, edition)) + .join() + .expect("thread::spawn failed") + { span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest"); } } @@ -710,16 +754,22 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { // check for `begin_panic` if_chain! { - if let ExprKind::Call(ref func_expr, _) = expr.kind; - if let ExprKind::Path(QPath::Resolved(_, ref path)) = func_expr.kind; + if let ExprKind::Call(func_expr, _) = expr.kind; + if let ExprKind::Path(QPath::Resolved(_, path)) = func_expr.kind; if let Some(path_def_id) = path.res.opt_def_id(); if match_panic_def_id(self.cx, path_def_id); if is_expn_of(expr.span, "unreachable").is_none(); + if !is_expn_of_debug_assertions(expr.span); then { self.panic_span = Some(expr.span); } } + // check for `assert_eq` or `assert_ne` + if is_expn_of(expr.span, "assert_eq").is_some() || is_expn_of(expr.span, "assert_ne").is_some() { + self.panic_span = Some(expr.span); + } + // check for `unwrap` if let Some(arglists) = method_chain_args(expr, &["unwrap"]) { let reciever_ty = self.typeck_results.expr_ty(&arglists[0][0]).peel_refs(); @@ -734,7 +784,15 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { intravisit::walk_expr(self, expr); } + // Panics in const blocks will cause compilation to fail. + fn visit_anon_const(&mut self, _: &'tcx AnonConst) {} + fn nested_visit_map(&mut self) -> NestedVisitorMap { NestedVisitorMap::OnlyBodies(self.cx.tcx.hir()) } } + +fn is_expn_of_debug_assertions(span: Span) -> bool { + const MACRO_NAMES: &[&str] = &["debug_assert", "debug_assert_eq", "debug_assert_ne"]; + MACRO_NAMES.iter().any(|name| is_expn_of(span, name).is_some()) +}