diff --git a/src/librustc/front/intrinsic.rs b/src/librustc/front/intrinsic.rs index ece53451ccf..fcb08180a5e 100644 --- a/src/librustc/front/intrinsic.rs +++ b/src/librustc/front/intrinsic.rs @@ -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 diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs index 56c024f12ae..c42c8b8bb84 100644 --- a/src/librustc/middle/lint.rs +++ b/src/librustc/middle/lint.rs @@ -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(v: visit::vt) -> visit::vt { +fn item_stopping_visitor(outer: visit::vt) -> visit::vt { 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(v: visit::vt) -> visit::vt { @@ -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| { diff --git a/src/test/compile-fail/lint-missing-doc.rs b/src/test/compile-fail/lint-missing-doc.rs new file mode 100644 index 00000000000..fd0b0fb80f8 --- /dev/null +++ b/src/test/compile-fail/lint-missing-doc.rs @@ -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 or the MIT license +// , 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() {}