Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace some desc logic in librustc_lint with article_and_desc #69740

Merged
merged 5 commits into from
Mar 24, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 32 additions & 32 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ impl MissingDoc {
id: Option<hir::HirId>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anything pass None for this? If not you could remove the Option and compute article_and_desc inside here from the HirId.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, it is passed with a Crate down below, so for now I will keep the option.

attrs: &[ast::Attribute],
sp: Span,
desc: &'static str,
desc: &str,
mark-i-m marked this conversation as resolved.
Show resolved Hide resolved
) {
// If we're building a test harness, then warning about
// documentation is probably not really relevant right now.
Expand Down Expand Up @@ -413,12 +413,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc {
}

fn check_item(&mut self, cx: &LateContext<'_, '_>, it: &hir::Item<'_>) {
let desc = match it.kind {
hir::ItemKind::Fn(..) => "a function",
hir::ItemKind::Mod(..) => "a module",
hir::ItemKind::Enum(..) => "an enum",
hir::ItemKind::Struct(..) => "a struct",
hir::ItemKind::Union(..) => "a union",
match it.kind {
hir::ItemKind::Trait(.., trait_item_refs) => {
// Issue #11592: traits are always considered exported, even when private.
if let hir::VisibilityKind::Inherited = it.vis.node {
Expand All @@ -428,52 +423,61 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc {
}
return;
}
"a trait"
}
hir::ItemKind::TyAlias(..) => "a type alias",
hir::ItemKind::Impl { of_trait: Some(ref trait_ref), items, .. } => {
// If the trait is private, add the impl items to `private_traits` so they don't get
// reported for missing docs.
let real_trait = trait_ref.path.res.def_id();
if let Some(hir_id) = cx.tcx.hir().as_local_hir_id(real_trait) {
match cx.tcx.hir().find(hir_id) {
Some(Node::Item(item)) => {
if let hir::VisibilityKind::Inherited = item.vis.node {
for impl_item_ref in items {
self.private_traits.insert(impl_item_ref.id.hir_id);
}
if let Some(Node::Item(item)) = cx.tcx.hir().find(hir_id) {
if let hir::VisibilityKind::Inherited = item.vis.node {
for impl_item_ref in items {
self.private_traits.insert(impl_item_ref.id.hir_id);
}
}
_ => {}
}
}
return;
}
hir::ItemKind::Const(..) => "a constant",
hir::ItemKind::Static(..) => "a static",

hir::ItemKind::TyAlias(..)
| hir::ItemKind::Fn(..)
| hir::ItemKind::Mod(..)
| hir::ItemKind::Enum(..)
| hir::ItemKind::Struct(..)
| hir::ItemKind::Union(..)
| hir::ItemKind::Const(..)
| hir::ItemKind::Static(..) => {}

_ => return,
};

self.check_missing_docs_attrs(cx, Some(it.hir_id), &it.attrs, it.span, desc);
let def_id = cx.tcx.hir().local_def_id(it.hir_id);
let (article, desc) = cx.tcx.article_and_description(def_id);

self.check_missing_docs_attrs(
cx,
Some(it.hir_id),
&it.attrs,
it.span,
&format!("{} {}", article, desc),
);
}

fn check_trait_item(&mut self, cx: &LateContext<'_, '_>, trait_item: &hir::TraitItem<'_>) {
if self.private_traits.contains(&trait_item.hir_id) {
return;
}

let desc = match trait_item.kind {
hir::TraitItemKind::Const(..) => "an associated constant",
hir::TraitItemKind::Fn(..) => "a trait method",
hir::TraitItemKind::Type(..) => "an associated type",
};
let def_id = cx.tcx.hir().local_def_id(trait_item.hir_id);
let (article, desc) = cx.tcx.article_and_description(def_id);

self.check_missing_docs_attrs(
cx,
Some(trait_item.hir_id),
&trait_item.attrs,
trait_item.span,
desc,
&format!("{} {}", article, desc),
);
}

Expand All @@ -483,18 +487,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc {
return;
}

let desc = match impl_item.kind {
hir::ImplItemKind::Const(..) => "an associated constant",
hir::ImplItemKind::Fn(..) => "a method",
hir::ImplItemKind::TyAlias(_) => "an associated type",
hir::ImplItemKind::OpaqueTy(_) => "an associated `impl Trait` type",
};
let def_id = cx.tcx.hir().local_def_id(impl_item.hir_id);
let (article, desc) = cx.tcx.article_and_description(def_id);
self.check_missing_docs_attrs(
cx,
Some(impl_item.hir_id),
&impl_item.attrs,
impl_item.span,
desc,
&format!("{} {}", article, desc),
);
}

Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/lint/lint-missing-doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ trait B {
}

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
fn foo(&self); //~ ERROR: missing documentation for an associated function
fn foo_with_impl(&self) {} //~ ERROR: missing documentation for an associated function
}

#[allow(missing_docs)]
Expand All @@ -78,7 +78,7 @@ impl Foo {
}

impl PubFoo {
pub fn foo() {} //~ ERROR: missing documentation for a method
pub fn foo() {} //~ ERROR: missing documentation for an associated function
/// dox
pub fn foo1() {}
fn foo2() {}
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/lint/lint-missing-doc.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ error: missing documentation for a trait
LL | pub trait C {
| ^^^^^^^^^^^

error: missing documentation for a trait method
error: missing documentation for an associated function
--> $DIR/lint-missing-doc.rs:53:5
|
LL | fn foo(&self);
| ^^^^^^^^^^^^^^

error: missing documentation for a trait method
error: missing documentation for an associated function
--> $DIR/lint-missing-doc.rs:54:5
|
LL | fn foo_with_impl(&self) {}
Expand All @@ -64,7 +64,7 @@ error: missing documentation for an associated type
LL | type AssociatedTypeDef = Self;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: missing documentation for a method
error: missing documentation for an associated function
--> $DIR/lint-missing-doc.rs:81:5
|
LL | pub fn foo() {}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/privacy/private-in-public-non-principal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub fn leak_dyn_nonprincipal() -> Box<dyn PubPrincipal + PrivNonPrincipal> { loo
#[deny(missing_docs)]
fn container() {
impl dyn PubPrincipal {
pub fn check_doc_lint() {} //~ ERROR missing documentation for a method
pub fn check_doc_lint() {} //~ ERROR missing documentation for an associated function
}
impl dyn PubPrincipal + PrivNonPrincipal {
pub fn check_doc_lint() {} // OK, no missing doc lint
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/privacy/private-in-public-non-principal.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | pub fn leak_dyn_nonprincipal() -> Box<dyn PubPrincipal + PrivNonPrincipal>
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: missing documentation for a method
error: missing documentation for an associated function
--> $DIR/private-in-public-non-principal.rs:13:9
|
LL | pub fn check_doc_lint() {}
Expand Down