Make missing documentation linting more robust

Add some more cases for warning about missing documentation, and also add a test
to make sure it doesn't die in the future.
This commit is contained in:
Alex Crichton 2013-05-28 15:44:53 -05:00
parent 3a3bf8bdef
commit af995ce1e7
3 changed files with 179 additions and 77 deletions

View File

@ -12,6 +12,8 @@
// and injected into each crate the compiler builds. Keep it small.
pub mod intrinsic {
#[allow(missing_doc)];
pub use intrinsic::rusti::visit_tydesc;
// FIXME (#3727): remove this when the interface has settled and the

View File

@ -95,8 +95,7 @@ pub enum lint {
unused_mut,
unnecessary_allocation,
missing_struct_doc,
missing_trait_doc,
missing_doc,
}
pub fn level_to_str(lv: level) -> &'static str {
@ -268,17 +267,10 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[
default: warn
}),
("missing_struct_doc",
("missing_doc",
LintSpec {
lint: missing_struct_doc,
desc: "detects missing documentation for structs",
default: allow
}),
("missing_trait_doc",
LintSpec {
lint: missing_trait_doc,
desc: "detects missing documentation for traits",
lint: missing_doc,
desc: "detects missing documentation for public members",
default: allow
}),
];
@ -302,6 +294,9 @@ struct Context {
curr: SmallIntMap<(level, LintSource)>,
// context we're checking in (used to access fields like sess)
tcx: ty::ctxt,
// Just a simple flag if we're currently recursing into a trait
// implementation. This is only used by the lint_missing_doc() pass
in_trait_impl: bool,
// When recursing into an attributed node of the ast which modifies lint
// levels, this stack keeps track of the previous lint levels of whatever
// was modified.
@ -311,7 +306,15 @@ struct Context {
// Others operate directly on @ast::item structures (or similar). Finally,
// others still are added to the Session object via `add_lint`, and these
// are all passed with the lint_session visitor.
visitors: ~[visit::vt<@mut Context>],
//
// This is a pair so every visitor can visit every node. When a lint pass is
// registered, another visitor is created which stops at all items which can
// alter the attributes of the ast. This "item stopping visitor" is the
// second element of the pair, while the original visitor is the first
// element. This means that when visiting a node, the original recursive
// call can used the original visitor's method, although the recursing
// visitor supplied to the method is the item stopping visitor.
visitors: ~[(visit::vt<@mut Context>, visit::vt<@mut Context>)],
}
impl Context {
@ -429,19 +432,21 @@ impl Context {
}
fn add_lint(&mut self, v: visit::vt<@mut Context>) {
self.visitors.push(item_stopping_visitor(v));
self.visitors.push((v, item_stopping_visitor(v)));
}
fn process(@mut self, n: AttributedNode) {
// see comment of the `visitors` field in the struct for why there's a
// pair instead of just one visitor.
match n {
Item(it) => {
for self.visitors.each |v| {
visit::visit_item(it, self, *v);
for self.visitors.each |&(orig, stopping)| {
(orig.visit_item)(it, self, stopping);
}
}
Crate(c) => {
for self.visitors.each |v| {
visit::visit_crate(c, self, *v);
for self.visitors.each |&(_, stopping)| {
visit::visit_crate(c, self, stopping);
}
}
// Can't use visit::visit_method_helper because the
@ -449,9 +454,9 @@ impl Context {
// to be a no-op, so manually invoke visit_fn.
Method(m) => {
let fk = visit::fk_method(copy m.ident, &m.generics, m);
for self.visitors.each |v| {
visit::visit_fn(&fk, &m.decl, &m.body, m.span, m.id,
self, *v);
for self.visitors.each |&(orig, stopping)| {
(orig.visit_fn)(&fk, &m.decl, &m.body, m.span, m.id,
self, stopping);
}
}
}
@ -495,16 +500,16 @@ pub fn each_lint(sess: session::Session,
// This is used to make the simple visitors used for the lint passes
// not traverse into subitems, since that is handled by the outer
// lint visitor.
fn item_stopping_visitor<E: Copy>(v: visit::vt<E>) -> visit::vt<E> {
fn item_stopping_visitor<E: Copy>(outer: visit::vt<E>) -> visit::vt<E> {
visit::mk_vt(@visit::Visitor {
visit_item: |_i, _e, _v| { },
visit_fn: |fk, fd, b, s, id, e, v| {
match *fk {
visit::fk_method(*) => {}
_ => visit::visit_fn(fk, fd, b, s, id, e, v)
_ => (outer.visit_fn)(fk, fd, b, s, id, e, v)
}
},
.. **(ty_stopping_visitor(v))})
.. **(ty_stopping_visitor(outer))})
}
fn ty_stopping_visitor<E>(v: visit::vt<E>) -> visit::vt<E> {
@ -972,68 +977,84 @@ fn lint_unnecessary_allocations() -> visit::vt<@mut Context> {
})
}
fn lint_missing_struct_doc() -> visit::vt<@mut Context> {
visit::mk_vt(@visit::Visitor {
visit_struct_field: |field, cx: @mut Context, vt| {
let relevant = match field.node.kind {
ast::named_field(_, vis) => vis != ast::private,
ast::unnamed_field => false,
};
fn lint_missing_doc() -> visit::vt<@mut Context> {
fn check_attrs(cx: @mut Context, attrs: &[ast::attribute],
sp: span, msg: &str) {
if !attrs.any(|a| a.node.is_sugared_doc) {
cx.span_lint(missing_doc, sp, msg);
}
}
if relevant {
let mut has_doc = false;
for field.node.attrs.each |attr| {
if attr.node.is_sugared_doc {
has_doc = true;
break;
}
}
if !has_doc {
cx.span_lint(missing_struct_doc, field.span, "missing documentation \
for a field.");
}
visit::mk_vt(@visit::Visitor {
visit_struct_method: |m, cx, vt| {
if m.vis == ast::public {
check_attrs(cx, m.attrs, m.span,
"missing documentation for a method");
}
visit::visit_struct_field(field, cx, vt);
visit::visit_struct_method(m, cx, vt);
},
.. *visit::default_visitor()
})
}
fn lint_missing_trait_doc() -> visit::vt<@mut Context> {
visit::mk_vt(@visit::Visitor {
visit_trait_method: |method, cx: @mut Context, vt| {
let mut has_doc = false;
let span = match copy *method {
ast::required(m) => {
for m.attrs.each |attr| {
if attr.node.is_sugared_doc {
has_doc = true;
break;
}
visit_ty_method: |m, cx, vt| {
// All ty_method objects are linted about because they're part of a
// trait (no visibility)
check_attrs(cx, m.attrs, m.span,
"missing documentation for a method");
visit::visit_ty_method(m, cx, vt);
},
visit_fn: |fk, d, b, sp, id, cx, vt| {
// Only warn about explicitly public methods. Soon implicit
// public-ness will hopefully be going away.
match *fk {
visit::fk_method(_, _, m) if m.vis == ast::public => {
// If we're in a trait implementation, no need to duplicate
// documentation
if !cx.in_trait_impl {
check_attrs(cx, m.attrs, sp,
"missing documentation for a method");
}
m.span
},
ast::provided(m) => {
if m.vis == ast::private {
has_doc = true;
} else {
for m.attrs.each |attr| {
if attr.node.is_sugared_doc {
has_doc = true;
break;
}
_ => {}
}
visit::visit_fn(fk, d, b, sp, id, cx, vt);
},
visit_item: |it, cx, vt| {
match it.node {
// Go ahead and match the fields here instead of using
// visit_struct_field while we have access to the enclosing
// struct's visibility
ast::item_struct(sdef, _) if it.vis == ast::public => {
check_attrs(cx, it.attrs, it.span,
"missing documentation for a struct");
for sdef.fields.each |field| {
match field.node.kind {
ast::named_field(_, vis) if vis != ast::private => {
check_attrs(cx, field.node.attrs, field.span,
"missing documentation for a field");
}
ast::unnamed_field | ast::named_field(*) => {}
}
}
m.span
}
ast::item_trait(*) if it.vis == ast::public => {
check_attrs(cx, it.attrs, it.span,
"missing documentation for a trait");
}
ast::item_fn(*) if it.vis == ast::public => {
check_attrs(cx, it.attrs, it.span,
"missing documentation for a function");
}
_ => {}
};
if !has_doc {
cx.span_lint(missing_trait_doc, span, "missing documentation \
for a method.");
}
visit::visit_trait_method(method, cx, vt);
visit::visit_item(it, cx, vt);
},
.. *visit::default_visitor()
})
}
@ -1045,6 +1066,7 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
tcx: tcx,
lint_stack: ~[],
visitors: ~[],
in_trait_impl: false,
};
// Install defaults.
@ -1066,8 +1088,7 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
cx.add_lint(lint_unused_mut());
cx.add_lint(lint_session());
cx.add_lint(lint_unnecessary_allocations());
cx.add_lint(lint_missing_struct_doc());
cx.add_lint(lint_missing_trait_doc());
cx.add_lint(lint_missing_doc());
// Actually perform the lint checks (iterating the ast)
do cx.with_lint_attrs(crate.node.attrs) {
@ -1076,6 +1097,12 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
visit::visit_crate(crate, cx, visit::mk_vt(@visit::Visitor {
visit_item: |it, cx: @mut Context, vt| {
do cx.with_lint_attrs(it.attrs) {
match it.node {
ast::item_impl(_, Some(*), _, _) => {
cx.in_trait_impl = true;
}
_ => {}
}
check_item_ctypes(cx, it);
check_item_non_camel_case_types(cx, it);
check_item_default_methods(cx, it);
@ -1083,6 +1110,7 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::crate) {
cx.process(Item(it));
visit::visit_item(it, cx, vt);
cx.in_trait_impl = false;
}
},
visit_fn: |fk, decl, body, span, id, cx, vt| {

View File

@ -0,0 +1,72 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// When denying at the crate level, be sure to not get random warnings from the
// injected intrinsics by the compiler.
#[deny(missing_doc)];
struct Foo {
a: int,
priv b: int,
pub c: int, // doesn't matter, Foo is private
}
pub struct PubFoo { //~ ERROR: missing documentation
a: int, //~ ERROR: missing documentation
priv b: int,
pub c: int, //~ ERROR: missing documentation
}
#[allow(missing_doc)]
pub struct PubFoo2 {
a: int,
pub c: int,
}
/// dox
pub fn foo() {}
pub fn foo2() {} //~ ERROR: missing documentation
fn foo3() {}
#[allow(missing_doc)] pub fn foo4() {}
/// dox
pub trait A {}
trait B {}
pub trait C {} //~ ERROR: missing documentation
#[allow(missing_doc)] pub trait D {}
trait Bar {
/// dox
pub fn foo();
fn foo2(); //~ ERROR: missing documentation
pub fn foo3(); //~ ERROR: missing documentation
}
impl Foo {
pub fn foo() {} //~ ERROR: missing documentation
/// dox
pub fn foo1() {}
fn foo2() {}
#[allow(missing_doc)] pub fn foo3() {}
}
#[allow(missing_doc)]
trait F {
pub fn a();
fn b(&self);
}
// should need to redefine documentation for implementations of traits
impl F for Foo {
pub fn a() {}
fn b(&self) {}
}
fn main() {}