]> git.lizzy.rs Git - rust.git/blob - clippy_lints/src/doc.rs
Add tests and improve checks.
[rust.git] / clippy_lints / src / doc.rs
1 use crate::utils::{get_trait_def_id, implements_trait, is_entrypoint_fn, match_type, paths, return_ty, span_lint};
2 use itertools::Itertools;
3 use rustc::lint::in_external_macro;
4 use rustc::ty::TyKind;
5 use rustc_data_structures::fx::FxHashSet;
6 use rustc_hir as hir;
7 use rustc_lint::{LateContext, LateLintPass};
8 use rustc_session::{declare_tool_lint, impl_lint_pass};
9 use rustc_span::source_map::{BytePos, MultiSpan, Span};
10 use rustc_span::Pos;
11 use std::ops::Range;
12 use syntax::ast::{AttrKind, Attribute};
13 use url::Url;
14
15 declare_clippy_lint! {
16     /// **What it does:** Checks for the presence of `_`, `::` or camel-case words
17     /// outside ticks in documentation.
18     ///
19     /// **Why is this bad?** *Rustdoc* supports markdown formatting, `_`, `::` and
20     /// camel-case probably indicates some code which should be included between
21     /// ticks. `_` can also be used for emphasis in markdown, this lint tries to
22     /// consider that.
23     ///
24     /// **Known problems:** Lots of bad docs won’t be fixed, what the lint checks
25     /// for is limited, and there are still false positives.
26     ///
27     /// **Examples:**
28     /// ```rust
29     /// /// Do something with the foo_bar parameter. See also
30     /// /// that::other::module::foo.
31     /// // ^ `foo_bar` and `that::other::module::foo` should be ticked.
32     /// fn doit(foo_bar: usize) {}
33     /// ```
34     pub DOC_MARKDOWN,
35     pedantic,
36     "presence of `_`, `::` or camel-case outside backticks in documentation"
37 }
38
39 declare_clippy_lint! {
40     /// **What it does:** Checks for the doc comments of publicly visible
41     /// unsafe functions and warns if there is no `# Safety` section.
42     ///
43     /// **Why is this bad?** Unsafe functions should document their safety
44     /// preconditions, so that users can be sure they are using them safely.
45     ///
46     /// **Known problems:** None.
47     ///
48     /// **Examples:**
49     /// ```rust
50     ///# type Universe = ();
51     /// /// This function should really be documented
52     /// pub unsafe fn start_apocalypse(u: &mut Universe) {
53     ///     unimplemented!();
54     /// }
55     /// ```
56     ///
57     /// At least write a line about safety:
58     ///
59     /// ```rust
60     ///# type Universe = ();
61     /// /// # Safety
62     /// ///
63     /// /// This function should not be called before the horsemen are ready.
64     /// pub unsafe fn start_apocalypse(u: &mut Universe) {
65     ///     unimplemented!();
66     /// }
67     /// ```
68     pub MISSING_SAFETY_DOC,
69     style,
70     "`pub unsafe fn` without `# Safety` docs"
71 }
72
73 declare_clippy_lint! {
74     /// **What it does:** Checks the doc comments of publicly visible functions that
75     /// return a `Result` type and warns if there is no `# Errors` section.
76     ///
77     /// **Why is this bad?** Documenting the type of errors that can be returned from a
78     /// function can help callers write code to handle the errors appropriately.
79     ///
80     /// **Known problems:** None.
81     ///
82     /// **Examples:**
83     ///
84     /// Since the following function returns a `Result` it has an `# Errors` section in
85     /// its doc comment:
86     ///
87     /// ```rust
88     ///# use std::io;
89     /// /// # Errors
90     /// ///
91     /// /// Will return `Err` if `filename` does not exist or the user does not have
92     /// /// permission to read it.
93     /// pub fn read(filename: String) -> io::Result<String> {
94     ///     unimplemented!();
95     /// }
96     /// ```
97     pub MISSING_ERRORS_DOC,
98     pedantic,
99     "`pub fn` returns `Result` without `# Errors` in doc comment"
100 }
101
102 declare_clippy_lint! {
103     /// **What it does:** Checks for `fn main() { .. }` in doctests
104     ///
105     /// **Why is this bad?** The test can be shorter (and likely more readable)
106     /// if the `fn main()` is left implicit.
107     ///
108     /// **Known problems:** None.
109     ///
110     /// **Examples:**
111     /// ``````rust
112     /// /// An example of a doctest with a `main()` function
113     /// ///
114     /// /// # Examples
115     /// ///
116     /// /// ```
117     /// /// fn main() {
118     /// ///     // this needs not be in an `fn`
119     /// /// }
120     /// /// ```
121     /// fn needless_main() {
122     ///     unimplemented!();
123     /// }
124     /// ``````
125     pub NEEDLESS_DOCTEST_MAIN,
126     style,
127     "presence of `fn main() {` in code examples"
128 }
129
130 #[allow(clippy::module_name_repetitions)]
131 #[derive(Clone)]
132 pub struct DocMarkdown {
133     valid_idents: FxHashSet<String>,
134     in_trait_impl: bool,
135 }
136
137 impl DocMarkdown {
138     pub fn new(valid_idents: FxHashSet<String>) -> Self {
139         Self {
140             valid_idents,
141             in_trait_impl: false,
142         }
143     }
144 }
145
146 impl_lint_pass!(DocMarkdown => [DOC_MARKDOWN, MISSING_SAFETY_DOC, MISSING_ERRORS_DOC, NEEDLESS_DOCTEST_MAIN]);
147
148 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
149     fn check_crate(&mut self, cx: &LateContext<'a, 'tcx>, krate: &'tcx hir::Crate<'_>) {
150         check_attrs(cx, &self.valid_idents, &krate.attrs);
151     }
152
153     fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item<'_>) {
154         let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
155         match item.kind {
156             hir::ItemKind::Fn(ref sig, ..) => {
157                 if !(is_entrypoint_fn(cx, cx.tcx.hir().local_def_id(item.hir_id))
158                     || in_external_macro(cx.tcx.sess, item.span))
159                 {
160                     lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers);
161                 }
162             },
163             hir::ItemKind::Impl {
164                 of_trait: ref trait_ref,
165                 ..
166             } => {
167                 self.in_trait_impl = trait_ref.is_some();
168             },
169             _ => {},
170         }
171     }
172
173     fn check_item_post(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item<'_>) {
174         if let hir::ItemKind::Impl { .. } = item.kind {
175             self.in_trait_impl = false;
176         }
177     }
178
179     fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem<'_>) {
180         let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
181         if let hir::TraitItemKind::Method(ref sig, ..) = item.kind {
182             if !in_external_macro(cx.tcx.sess, item.span) {
183                 lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers);
184             }
185         }
186     }
187
188     fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::ImplItem<'_>) {
189         let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
190         if self.in_trait_impl || in_external_macro(cx.tcx.sess, item.span) {
191             return;
192         }
193         if let hir::ImplItemKind::Method(ref sig, ..) = item.kind {
194             lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers);
195         }
196     }
197 }
198
199 fn lint_for_missing_headers<'a, 'tcx>(
200     cx: &LateContext<'a, 'tcx>,
201     hir_id: hir::HirId,
202     span: impl Into<MultiSpan> + Copy,
203     sig: &hir::FnSig<'_>,
204     headers: DocHeaders,
205 ) {
206     if !cx.access_levels.is_exported(hir_id) {
207         return; // Private functions do not require doc comments
208     }
209     if !headers.safety && sig.header.unsafety == hir::Unsafety::Unsafe {
210         span_lint(
211             cx,
212             MISSING_SAFETY_DOC,
213             span,
214             "unsafe function's docs miss `# Safety` section",
215         );
216     }
217     if !headers.errors {
218         if match_type(cx, return_ty(cx, hir_id), &paths::RESULT) {
219             span_lint(
220                 cx,
221                 MISSING_ERRORS_DOC,
222                 span,
223                 "docs for function returning `Result` missing `# Errors` section",
224             );
225         } else {
226             let def_id = cx.tcx.hir().local_def_id(hir_id);
227             let mir = cx.tcx.optimized_mir(def_id);
228             if let Some(future) = get_trait_def_id(cx, &paths::FUTURE) {
229                 if implements_trait(cx, mir.return_ty(), future, &[]) {
230                     use TyKind::*;
231
232                     if let Opaque(_, subs) = mir.return_ty().kind {
233                         if let Some(ty) = subs.types().next() {
234                             if let Generator(_, subs, _) = ty.kind {
235                                 if match_type(cx, subs.as_generator().return_ty(def_id, cx.tcx), &paths::RESULT) {
236                                     span_lint(
237                                         cx,
238                                         MISSING_ERRORS_DOC,
239                                         span,
240                                         "docs for function returning `Result` missing `# Errors` section",
241                                     );
242                                 }
243                             }
244                         }
245                     }
246                 }
247             }
248         }
249     }
250 }
251
252 /// Cleanup documentation decoration (`///` and such).
253 ///
254 /// We can't use `syntax::attr::AttributeMethods::with_desugared_doc` or
255 /// `syntax::parse::lexer::comments::strip_doc_comment_decoration` because we
256 /// need to keep track of
257 /// the spans but this function is inspired from the later.
258 #[allow(clippy::cast_possible_truncation)]
259 #[must_use]
260 pub fn strip_doc_comment_decoration(comment: &str, span: Span) -> (String, Vec<(usize, Span)>) {
261     // one-line comments lose their prefix
262     const ONELINERS: &[&str] = &["///!", "///", "//!", "//"];
263     for prefix in ONELINERS {
264         if comment.starts_with(*prefix) {
265             let doc = &comment[prefix.len()..];
266             let mut doc = doc.to_owned();
267             doc.push('\n');
268             return (
269                 doc.to_owned(),
270                 vec![(doc.len(), span.with_lo(span.lo() + BytePos(prefix.len() as u32)))],
271             );
272         }
273     }
274
275     if comment.starts_with("/*") {
276         let doc = &comment[3..comment.len() - 2];
277         let mut sizes = vec![];
278         let mut contains_initial_stars = false;
279         for line in doc.lines() {
280             let offset = line.as_ptr() as usize - comment.as_ptr() as usize;
281             debug_assert_eq!(offset as u32 as usize, offset);
282             contains_initial_stars |= line.trim_start().starts_with('*');
283             // +1 for the newline
284             sizes.push((line.len() + 1, span.with_lo(span.lo() + BytePos(offset as u32))));
285         }
286         if !contains_initial_stars {
287             return (doc.to_string(), sizes);
288         }
289         // remove the initial '*'s if any
290         let mut no_stars = String::with_capacity(doc.len());
291         for line in doc.lines() {
292             let mut chars = line.chars();
293             while let Some(c) = chars.next() {
294                 if c.is_whitespace() {
295                     no_stars.push(c);
296                 } else {
297                     no_stars.push(if c == '*' { ' ' } else { c });
298                     break;
299                 }
300             }
301             no_stars.push_str(chars.as_str());
302             no_stars.push('\n');
303         }
304         return (no_stars, sizes);
305     }
306
307     panic!("not a doc-comment: {}", comment);
308 }
309
310 #[derive(Copy, Clone)]
311 struct DocHeaders {
312     safety: bool,
313     errors: bool,
314 }
315
316 fn check_attrs<'a>(cx: &LateContext<'_, '_>, valid_idents: &FxHashSet<String>, attrs: &'a [Attribute]) -> DocHeaders {
317     let mut doc = String::new();
318     let mut spans = vec![];
319
320     for attr in attrs {
321         if let AttrKind::DocComment(ref comment) = attr.kind {
322             let comment = comment.to_string();
323             let (comment, current_spans) = strip_doc_comment_decoration(&comment, attr.span);
324             spans.extend_from_slice(&current_spans);
325             doc.push_str(&comment);
326         } else if attr.check_name(sym!(doc)) {
327             // ignore mix of sugared and non-sugared doc
328             // don't trigger the safety or errors check
329             return DocHeaders {
330                 safety: true,
331                 errors: true,
332             };
333         }
334     }
335
336     let mut current = 0;
337     for &mut (ref mut offset, _) in &mut spans {
338         let offset_copy = *offset;
339         *offset = current;
340         current += offset_copy;
341     }
342
343     if doc.is_empty() {
344         return DocHeaders {
345             safety: false,
346             errors: false,
347         };
348     }
349
350     let parser = pulldown_cmark::Parser::new(&doc).into_offset_iter();
351     // Iterate over all `Events` and combine consecutive events into one
352     let events = parser.coalesce(|previous, current| {
353         use pulldown_cmark::Event::*;
354
355         let previous_range = previous.1;
356         let current_range = current.1;
357
358         match (previous.0, current.0) {
359             (Text(previous), Text(current)) => {
360                 let mut previous = previous.to_string();
361                 previous.push_str(&current);
362                 Ok((Text(previous.into()), previous_range))
363             },
364             (previous, current) => Err(((previous, previous_range), (current, current_range))),
365         }
366     });
367     check_doc(cx, valid_idents, events, &spans)
368 }
369
370 fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize>)>>(
371     cx: &LateContext<'_, '_>,
372     valid_idents: &FxHashSet<String>,
373     events: Events,
374     spans: &[(usize, Span)],
375 ) -> DocHeaders {
376     // true if a safety header was found
377     use pulldown_cmark::Event::*;
378     use pulldown_cmark::Tag::*;
379
380     let mut headers = DocHeaders {
381         safety: false,
382         errors: false,
383     };
384     let mut in_code = false;
385     let mut in_link = None;
386     let mut in_heading = false;
387
388     for (event, range) in events {
389         match event {
390             Start(CodeBlock(_)) => in_code = true,
391             End(CodeBlock(_)) => in_code = false,
392             Start(Link(_, url, _)) => in_link = Some(url),
393             End(Link(..)) => in_link = None,
394             Start(Heading(_)) => in_heading = true,
395             End(Heading(_)) => in_heading = false,
396             Start(_tag) | End(_tag) => (), // We don't care about other tags
397             Html(_html) => (),             // HTML is weird, just ignore it
398             SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (),
399             FootnoteReference(text) | Text(text) => {
400                 if Some(&text) == in_link.as_ref() {
401                     // Probably a link of the form `<http://example.com>`
402                     // Which are represented as a link to "http://example.com" with
403                     // text "http://example.com" by pulldown-cmark
404                     continue;
405                 }
406                 headers.safety |= in_heading && text.trim() == "Safety";
407                 headers.errors |= in_heading && text.trim() == "Errors";
408                 let index = match spans.binary_search_by(|c| c.0.cmp(&range.start)) {
409                     Ok(o) => o,
410                     Err(e) => e - 1,
411                 };
412                 let (begin, span) = spans[index];
413                 if in_code {
414                     check_code(cx, &text, span);
415                 } else {
416                     // Adjust for the beginning of the current `Event`
417                     let span = span.with_lo(span.lo() + BytePos::from_usize(range.start - begin));
418
419                     check_text(cx, valid_idents, &text, span);
420                 }
421             },
422         }
423     }
424     headers
425 }
426
427 static LEAVE_MAIN_PATTERNS: &[&str] = &["static", "fn main() {}", "extern crate"];
428
429 fn check_code(cx: &LateContext<'_, '_>, text: &str, span: Span) {
430     if text.contains("fn main() {") && !LEAVE_MAIN_PATTERNS.iter().any(|p| text.contains(p)) {
431         span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest");
432     }
433 }
434
435 fn check_text(cx: &LateContext<'_, '_>, valid_idents: &FxHashSet<String>, text: &str, span: Span) {
436     for word in text.split(|c: char| c.is_whitespace() || c == '\'') {
437         // Trim punctuation as in `some comment (see foo::bar).`
438         //                                                   ^^
439         // Or even as in `_foo bar_` which is emphasized.
440         let word = word.trim_matches(|c: char| !c.is_alphanumeric());
441
442         if valid_idents.contains(word) {
443             continue;
444         }
445
446         // Adjust for the current word
447         let offset = word.as_ptr() as usize - text.as_ptr() as usize;
448         let span = Span::new(
449             span.lo() + BytePos::from_usize(offset),
450             span.lo() + BytePos::from_usize(offset + word.len()),
451             span.ctxt(),
452         );
453
454         check_word(cx, word, span);
455     }
456 }
457
458 fn check_word(cx: &LateContext<'_, '_>, word: &str, span: Span) {
459     /// Checks if a string is camel-case, i.e., contains at least two uppercase
460     /// letters (`Clippy` is ok) and one lower-case letter (`NASA` is ok).
461     /// Plurals are also excluded (`IDs` is ok).
462     fn is_camel_case(s: &str) -> bool {
463         if s.starts_with(|c: char| c.is_digit(10)) {
464             return false;
465         }
466
467         let s = if s.ends_with('s') { &s[..s.len() - 1] } else { s };
468
469         s.chars().all(char::is_alphanumeric)
470             && s.chars().filter(|&c| c.is_uppercase()).take(2).count() > 1
471             && s.chars().filter(|&c| c.is_lowercase()).take(1).count() > 0
472     }
473
474     fn has_underscore(s: &str) -> bool {
475         s != "_" && !s.contains("\\_") && s.contains('_')
476     }
477
478     fn has_hyphen(s: &str) -> bool {
479         s != "-" && s.contains('-')
480     }
481
482     if let Ok(url) = Url::parse(word) {
483         // try to get around the fact that `foo::bar` parses as a valid URL
484         if !url.cannot_be_a_base() {
485             span_lint(
486                 cx,
487                 DOC_MARKDOWN,
488                 span,
489                 "you should put bare URLs between `<`/`>` or make a proper Markdown link",
490             );
491
492             return;
493         }
494     }
495
496     // We assume that mixed-case words are not meant to be put inside bacticks. (Issue #2343)
497     if has_underscore(word) && has_hyphen(word) {
498         return;
499     }
500
501     if has_underscore(word) || word.contains("::") || is_camel_case(word) {
502         span_lint(
503             cx,
504             DOC_MARKDOWN,
505             span,
506             &format!("you should put `{}` between ticks in the documentation", word),
507         );
508     }
509 }