From 36d8ca04a158ba617ae020c73f14792d808ad103 Mon Sep 17 00:00:00 2001 From: mcarton Date: Tue, 23 Aug 2016 18:09:37 +0200 Subject: [PATCH] Add a `MISSING_DOCS_IN_PRIVATE_ITEMS` lint --- CHANGELOG.md | 3 +- README.md | 3 +- clippy_lints/src/lib.rs | 5 +- clippy_lints/src/missing_doc.rs | 172 +++++++++++++++++++++++ src/lib.rs | 1 + src/main.rs | 2 + tests/compile-fail/enum_glob_use.rs | 2 +- tests/compile-fail/filter_methods.rs | 2 + tests/compile-fail/methods.rs | 2 +- tests/compile-fail/missing-doc.rs | 202 +++++++++++++++++++++++++++ tests/compile-fail/shadow.rs | 2 +- 11 files changed, 390 insertions(+), 6 deletions(-) create mode 100644 clippy_lints/src/missing_doc.rs create mode 100644 tests/compile-fail/missing-doc.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e198b57073..7723b558376 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ All notable changes to this project will be documented in this file. ## 0.0.86 — ? -* New lint: [`zero_prefixed_literal`] +* New lints: [`missing_docs_in_private_items`], [`zero_prefixed_literal`] ## 0.0.85 — 2016-08-19 * Fix ICE with [`useless_attribute`] @@ -241,6 +241,7 @@ All notable changes to this project will be documented in this file. [`mem_forget`]: https://github.com/Manishearth/rust-clippy/wiki#mem_forget [`min_max`]: https://github.com/Manishearth/rust-clippy/wiki#min_max [`misrefactored_assign_op`]: https://github.com/Manishearth/rust-clippy/wiki#misrefactored_assign_op +[`missing_docs_in_private_items`]: https://github.com/Manishearth/rust-clippy/wiki#missing_docs_in_private_items [`mixed_case_hex_literals`]: https://github.com/Manishearth/rust-clippy/wiki#mixed_case_hex_literals [`module_inception`]: https://github.com/Manishearth/rust-clippy/wiki#module_inception [`modulo_one`]: https://github.com/Manishearth/rust-clippy/wiki#modulo_one diff --git a/README.md b/README.md index b294c75a756..45c20f06c06 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Table of contents: ## Lints -There are 167 lints included in this crate: +There are 168 lints included in this crate: name | default | triggers on ---------------------------------------------------------------------------------------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------- @@ -100,6 +100,7 @@ name [mem_forget](https://github.com/Manishearth/rust-clippy/wiki#mem_forget) | allow | `mem::forget` usage on `Drop` types, likely to cause memory leaks [min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max) | warn | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant [misrefactored_assign_op](https://github.com/Manishearth/rust-clippy/wiki#misrefactored_assign_op) | warn | having a variable on both sides of an assign op +[missing_docs_in_private_items](https://github.com/Manishearth/rust-clippy/wiki#missing_docs_in_private_items) | allow | detects missing documentation for public and private members [mixed_case_hex_literals](https://github.com/Manishearth/rust-clippy/wiki#mixed_case_hex_literals) | warn | hex literals whose letter digits are not consistently upper- or lowercased [module_inception](https://github.com/Manishearth/rust-clippy/wiki#module_inception) | warn | modules that have the same name as their parent module [modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one) | warn | taking a number modulo 1, which always returns 0 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index df0b0d9efff..f7b1dcb40f4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -10,7 +10,7 @@ #![feature(stmt_expr_attributes)] #![feature(type_macros)] -#![allow(indexing_slicing, shadow_reuse, unknown_lints)] +#![allow(indexing_slicing, shadow_reuse, unknown_lints, missing_docs_in_private_items)] #[macro_use] extern crate syntax; @@ -96,6 +96,7 @@ macro_rules! declare_restriction_lint { pub mod minmax; pub mod misc; pub mod misc_early; +pub mod missing_doc; pub mod module_inception; pub mod mut_mut; pub mod mut_reference; @@ -260,6 +261,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box assign_ops::AssignOps); reg.register_late_lint_pass(box let_if_seq::LetIfSeq); reg.register_late_lint_pass(box eval_order_dependence::EvalOrderDependence); + reg.register_late_lint_pass(box missing_doc::MissingDoc::new()); reg.register_lint_group("clippy_restrictions", vec![ arithmetic::FLOAT_ARITHMETIC, @@ -282,6 +284,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { methods::WRONG_PUB_SELF_CONVENTION, misc::USED_UNDERSCORE_BINDING, misc_early::UNSEPARATED_LITERAL_SUFFIX, + missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS, mut_mut::MUT_MUT, mutex_atomic::MUTEX_INTEGER, non_expressive_names::SIMILAR_NAMES, diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs new file mode 100644 index 00000000000..b2b52677038 --- /dev/null +++ b/clippy_lints/src/missing_doc.rs @@ -0,0 +1,172 @@ +/* This file incorporates work covered by the following copyright and + * permission notice: + * Copyright 2012-2015 The Rust Project Developers. See the COPYRIGHT + * file at the top-level directory of this distribution and at + * http://rust-lang.org/COPYRIGHT. + * + * Licensed under the Apache License, Version 2.0 or the MIT license + * , at your + * option. This file may not be copied, modified, or distributed + * except according to those terms. + */ + +/* Note: More specifically this lint is largely inspired (aka copied) from *rustc*'s + * [`missing_doc`]. + * + * [`missing_doc`]: https://github.com/rust-lang/rust/blob/d6d05904697d89099b55da3331155392f1db9c00/src/librustc_lint/builtin.rs#L246 + */ +use rustc::hir; +use rustc::lint::*; +use rustc::ty; +use syntax::ast; +use syntax::attr::{self, AttrMetaMethods}; +use syntax::codemap::Span; + +/// **What it does:** Warns if there is missing doc for any documentable item (public or private). +/// +/// **Why is this bad?** Doc is good. *rustc* has a `MISSING_DOCS` allowed-by-default lint for +/// public members, but has no way to enforce documentation of private items. This lint fixes that. +/// +/// **Known problems:** None. +declare_lint! { + pub MISSING_DOCS_IN_PRIVATE_ITEMS, + Allow, + "detects missing documentation for public and private members" +} + +pub struct MissingDoc { + /// Stack of whether #[doc(hidden)] is set + /// at each level which has lint attributes. + doc_hidden_stack: Vec, +} + +impl ::std::default::Default for MissingDoc { + fn default() -> MissingDoc { + MissingDoc::new() + } +} + +impl MissingDoc { + pub fn new() -> MissingDoc { + MissingDoc { + doc_hidden_stack: vec![false], + } + } + + fn doc_hidden(&self) -> bool { + *self.doc_hidden_stack.last().expect("empty doc_hidden_stack") + } + + fn check_missing_docs_attrs(&self, + cx: &LateContext, + attrs: &[ast::Attribute], + sp: Span, + desc: &'static str) { + // If we're building a test harness, then warning about + // documentation is probably not really relevant right now. + if cx.sess().opts.test { + return; + } + + // `#[doc(hidden)]` disables missing_docs check. + if self.doc_hidden() { + return; + } + + let has_doc = attrs.iter().any(|a| a.is_value_str() && a.name() == "doc"); + if !has_doc { + cx.span_lint(MISSING_DOCS_IN_PRIVATE_ITEMS, sp, + &format!("missing documentation for {}", desc)); + } + } +} + +impl LintPass for MissingDoc { + fn get_lints(&self) -> LintArray { + lint_array![MISSING_DOCS_IN_PRIVATE_ITEMS] + } +} + +impl LateLintPass for MissingDoc { + fn enter_lint_attrs(&mut self, _: &LateContext, attrs: &[ast::Attribute]) { + let doc_hidden = self.doc_hidden() || attrs.iter().any(|attr| { + attr.check_name("doc") && match attr.meta_item_list() { + None => false, + Some(l) => attr::contains_name(&l[..], "hidden"), + } + }); + self.doc_hidden_stack.push(doc_hidden); + } + + fn exit_lint_attrs(&mut self, _: &LateContext, _: &[ast::Attribute]) { + self.doc_hidden_stack.pop().expect("empty doc_hidden_stack"); + } + + fn check_crate(&mut self, cx: &LateContext, krate: &hir::Crate) { + self.check_missing_docs_attrs(cx, &krate.attrs, krate.span, "crate"); + } + + fn check_item(&mut self, cx: &LateContext, it: &hir::Item) { + let desc = match it.node { + hir::ItemConst(..) => "a constant", + hir::ItemEnum(..) => "an enum", + hir::ItemFn(..) => "a function", + hir::ItemMod(..) => "a module", + hir::ItemStatic(..) => "a static", + hir::ItemStruct(..) => "a struct", + hir::ItemTrait(..) => "a trait", + hir::ItemTy(..) => "a type alias", + hir::ItemDefaultImpl(..) | + hir::ItemExternCrate(..) | + hir::ItemForeignMod(..) | + hir::ItemImpl(..) | + hir::ItemUse(..) => return, + }; + + self.check_missing_docs_attrs(cx, &it.attrs, it.span, desc); + } + + fn check_trait_item(&mut self, cx: &LateContext, trait_item: &hir::TraitItem) { + let desc = match trait_item.node { + hir::ConstTraitItem(..) => "an associated constant", + hir::MethodTraitItem(..) => "a trait method", + hir::TypeTraitItem(..) => "an associated type", + }; + + self.check_missing_docs_attrs(cx, &trait_item.attrs, trait_item.span, desc); + } + + fn check_impl_item(&mut self, cx: &LateContext, impl_item: &hir::ImplItem) { + // If the method is an impl for a trait, don't doc. + let def_id = cx.tcx.map.local_def_id(impl_item.id); + match cx.tcx.impl_or_trait_items.borrow() + .get(&def_id) + .expect("missing method descriptor?!") + .container() { + ty::TraitContainer(_) => return, + ty::ImplContainer(cid) => { + if cx.tcx.impl_trait_ref(cid).is_some() { + return + } + } + } + + let desc = match impl_item.node { + hir::ImplItemKind::Const(..) => "an associated constant", + hir::ImplItemKind::Method(..) => "a method", + hir::ImplItemKind::Type(_) => "an associated type", + }; + self.check_missing_docs_attrs(cx, &impl_item.attrs, impl_item.span, desc); + } + + fn check_struct_field(&mut self, cx: &LateContext, sf: &hir::StructField) { + if !sf.is_positional() { + self.check_missing_docs_attrs(cx, &sf.attrs, sf.span, "a struct field"); + } + } + + fn check_variant(&mut self, cx: &LateContext, v: &hir::Variant, _: &hir::Generics) { + self.check_missing_docs_attrs(cx, &v.node.attrs, v.span, "a variant"); + } +} diff --git a/src/lib.rs b/src/lib.rs index f5229d3cb39..e0a6cc28a02 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ #![feature(plugin_registrar)] #![feature(rustc_private)] #![allow(unknown_lints)] +#![allow(missing_docs_in_private_items)] extern crate rustc_plugin; use rustc_plugin::Registry; diff --git a/src/main.rs b/src/main.rs index e5043c927bb..efe6459f587 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,6 +2,8 @@ #![feature(box_syntax)] #![feature(rustc_private)] +#![allow(unknown_lints, missing_docs_in_private_items)] + extern crate clippy_lints; extern crate getopts; extern crate rustc; diff --git a/tests/compile-fail/enum_glob_use.rs b/tests/compile-fail/enum_glob_use.rs index 27f0ff24579..12fd104312a 100644 --- a/tests/compile-fail/enum_glob_use.rs +++ b/tests/compile-fail/enum_glob_use.rs @@ -1,7 +1,7 @@ #![feature(plugin)] #![plugin(clippy)] #![deny(clippy, clippy_pedantic)] -#![allow(unused_imports, dead_code)] +#![allow(unused_imports, dead_code, missing_docs_in_private_items)] use std::cmp::Ordering::*; //~ ERROR: don't use glob imports for enum variants diff --git a/tests/compile-fail/filter_methods.rs b/tests/compile-fail/filter_methods.rs index bee9688f581..20803c8d0e8 100644 --- a/tests/compile-fail/filter_methods.rs +++ b/tests/compile-fail/filter_methods.rs @@ -2,6 +2,8 @@ #![plugin(clippy)] #![deny(clippy, clippy_pedantic)] +#![allow(missing_docs_in_private_items)] + fn main() { let _: Vec<_> = vec![5; 6].into_iter() //~ERROR called `filter(p).map(q)` on an `Iterator` .filter(|&x| x == 0) diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index 7235bad11bf..0412dfecac1 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -3,7 +3,7 @@ #![plugin(clippy)] #![deny(clippy, clippy_pedantic)] -#![allow(blacklisted_name, unused, print_stdout, non_ascii_literal, new_without_default, new_without_default_derive)] +#![allow(blacklisted_name, unused, print_stdout, non_ascii_literal, new_without_default, new_without_default_derive, missing_docs_in_private_items)] use std::collections::BTreeMap; use std::collections::HashMap; diff --git a/tests/compile-fail/missing-doc.rs b/tests/compile-fail/missing-doc.rs new file mode 100644 index 00000000000..acd86f18ea3 --- /dev/null +++ b/tests/compile-fail/missing-doc.rs @@ -0,0 +1,202 @@ +/* This file incorporates work covered by the following copyright and + * permission notice: + * Copyright 2013 The Rust Project Developers. See the COPYRIGHT + * file at the top-level directory of this distribution and at + * http://rust-lang.org/COPYRIGHT. + * + * Licensed under the Apache License, Version 2.0 or the MIT license + * , at your + * option. This file may not be copied, modified, or distributed + * except according to those terms. + */ + +#![feature(plugin)] +#![plugin(clippy)] +#![deny(missing_docs_in_private_items)] + +// When denying at the crate level, be sure to not get random warnings from the +// injected intrinsics by the compiler. +#![allow(dead_code)] +#![feature(associated_type_defaults)] + +//! Some garbage docs for the crate here +#![doc="More garbage"] + +type Typedef = String; //~ ERROR: missing documentation for a type alias +pub type PubTypedef = String; //~ ERROR: missing documentation for a type alias + +struct Foo { //~ ERROR: missing documentation for a struct + a: isize, //~ ERROR: missing documentation for a struct field + b: isize, //~ ERROR: missing documentation for a struct field +} + +pub struct PubFoo { //~ ERROR: missing documentation for a struct + pub a: isize, //~ ERROR: missing documentation for a struct field + b: isize, //~ ERROR: missing documentation for a struct field +} + +#[allow(missing_docs_in_private_items)] +pub struct PubFoo2 { + pub a: isize, + pub c: isize, +} + +mod module_no_dox {} //~ ERROR: missing documentation for a module +pub mod pub_module_no_dox {} //~ ERROR: missing documentation for a module + +/// dox +pub fn foo() {} +pub fn foo2() {} //~ ERROR: missing documentation for a function +fn foo3() {} //~ ERROR: missing documentation for a function +#[allow(missing_docs_in_private_items)] pub fn foo4() {} + +/// dox +pub trait A { + /// dox + fn foo(&self); + /// dox + fn foo_with_impl(&self) {} +} + +#[allow(missing_docs_in_private_items)] +trait B { + fn foo(&self); + fn foo_with_impl(&self) {} +} + +pub trait C { //~ ERROR: missing documentation for a trait + fn foo(&self); //~ ERROR: missing documentation for a trait method + fn foo_with_impl(&self) {} //~ ERROR: missing documentation for a trait method +} + +#[allow(missing_docs_in_private_items)] +pub trait D { + fn dummy(&self) { } +} + +/// dox +pub trait E { + type AssociatedType; //~ ERROR: missing documentation for an associated type + type AssociatedTypeDef = Self; //~ ERROR: missing documentation for an associated type + + /// dox + type DocumentedType; + /// dox + type DocumentedTypeDef = Self; + /// dox + fn dummy(&self) {} +} + +impl Foo { + pub fn foo() {} //~ ERROR: missing documentation for a method + fn bar() {} //~ ERROR: missing documentation for a method +} + +impl PubFoo { + pub fn foo() {} //~ ERROR: missing documentation for a method + /// dox + pub fn foo1() {} + fn foo2() {} //~ ERROR: missing documentation for a method + #[allow(missing_docs_in_private_items)] pub fn foo3() {} +} + +#[allow(missing_docs_in_private_items)] +trait F { + fn a(); + fn b(&self); +} + +// should need to redefine documentation for implementations of traits +impl F for Foo { + fn a() {} + fn b(&self) {} +} + +// It sure is nice if doc(hidden) implies allow(missing_docs), and that it +// applies recursively +#[doc(hidden)] +mod a { + pub fn baz() {} + pub mod b { + pub fn baz() {} + } +} + +enum Baz { //~ ERROR: missing documentation for an enum + BazA { //~ ERROR: missing documentation for a variant + a: isize, //~ ERROR: missing documentation for a struct field + b: isize //~ ERROR: missing documentation for a struct field + }, + BarB //~ ERROR: missing documentation for a variant +} + +pub enum PubBaz { //~ ERROR: missing documentation for an enum + PubBazA { //~ ERROR: missing documentation for a variant + a: isize, //~ ERROR: missing documentation for a struct field + }, +} + +/// dox +pub enum PubBaz2 { + /// dox + PubBaz2A { + /// dox + a: isize, + }, +} + +#[allow(missing_docs_in_private_items)] +pub enum PubBaz3 { + PubBaz3A { + b: isize + }, +} + +#[doc(hidden)] +pub fn baz() {} + + +const FOO: u32 = 0; //~ ERROR: missing documentation for a const +/// dox +pub const FOO1: u32 = 0; +#[allow(missing_docs_in_private_items)] +pub const FOO2: u32 = 0; +#[doc(hidden)] +pub const FOO3: u32 = 0; +pub const FOO4: u32 = 0; //~ ERROR: missing documentation for a const + + +static BAR: u32 = 0; //~ ERROR: missing documentation for a static +/// dox +pub static BAR1: u32 = 0; +#[allow(missing_docs_in_private_items)] +pub static BAR2: u32 = 0; +#[doc(hidden)] +pub static BAR3: u32 = 0; +pub static BAR4: u32 = 0; //~ ERROR: missing documentation for a static + + +mod internal_impl { //~ ERROR: missing documentation for a module + /// dox + pub fn documented() {} + pub fn undocumented1() {} //~ ERROR: missing documentation for a function + pub fn undocumented2() {} //~ ERROR: missing documentation for a function + fn undocumented3() {} //~ ERROR: missing documentation for a function + /// dox + pub mod globbed { + /// dox + pub fn also_documented() {} + pub fn also_undocumented1() {} //~ ERROR: missing documentation for a function + fn also_undocumented2() {} //~ ERROR: missing documentation for a function + } +} +/// dox +pub mod public_interface { + pub use internal_impl::documented as foo; + pub use internal_impl::undocumented1 as bar; + pub use internal_impl::{documented, undocumented2}; + pub use internal_impl::globbed::*; +} + +fn main() {} //~ ERROR: missing documentation for a function diff --git a/tests/compile-fail/shadow.rs b/tests/compile-fail/shadow.rs index 1200e25cdbc..fae87cd9750 100644 --- a/tests/compile-fail/shadow.rs +++ b/tests/compile-fail/shadow.rs @@ -1,8 +1,8 @@ #![feature(plugin)] #![plugin(clippy)] -#![allow(unused_parens, unused_variables)] #![deny(clippy, clippy_pedantic)] +#![allow(unused_parens, unused_variables, missing_docs_in_private_items)] fn id(x: T) -> T { x } -- 2.44.0