Skip to content

Commit f9f4f58

Browse files
Fix invalid missing documentation lint reporting for re-exports
1 parent b949e8e commit f9f4f58

File tree

3 files changed

+114
-6
lines changed

3 files changed

+114
-6
lines changed

compiler/rustc_lint/messages.ftl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,7 @@ lint_builtin_decl_unsafe_method = declaration of an `unsafe` method
370370
lint_builtin_impl_unsafe_method = implementation of an `unsafe` method
371371
372372
lint_builtin_missing_doc = missing documentation for {$article} {$desc}
373+
.note = because it is re-exported here
373374
374375
lint_builtin_missing_copy_impl = type could implement `Copy`; consider adding `impl Copy`
375376

compiler/rustc_lint/src/builtin.rs

Lines changed: 111 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ use rustc_middle::ty::subst::GenericArgKind;
6666
use rustc_middle::ty::{self, Instance, Ty, TyCtxt, VariantDef};
6767
use rustc_session::lint::{BuiltinLintDiagnostics, FutureIncompatibilityReason};
6868
use rustc_span::edition::Edition;
69+
use rustc_span::hygiene::MacroKind;
6970
use rustc_span::source_map::Spanned;
7071
use rustc_span::symbol::{kw, sym, Ident, Symbol};
7172
use rustc_span::{BytePos, InnerSpan, Span};
@@ -450,6 +451,8 @@ declare_lint! {
450451
pub struct MissingDoc {
451452
/// Stack of whether `#[doc(hidden)]` is set at each level which has lint attributes.
452453
doc_hidden_stack: Vec<bool>,
454+
/// Set of types being re-exported while not directly public.
455+
reexported_items: FxHashSet<DefId>,
453456
}
454457

455458
impl_lint_pass!(MissingDoc => [MISSING_DOCS]);
@@ -480,13 +483,18 @@ fn has_doc(attr: &ast::Attribute) -> bool {
480483

481484
impl MissingDoc {
482485
pub fn new() -> MissingDoc {
483-
MissingDoc { doc_hidden_stack: vec![false] }
486+
MissingDoc { doc_hidden_stack: vec![false], reexported_items: FxHashSet::default() }
484487
}
485488

486489
fn doc_hidden(&self) -> bool {
487490
*self.doc_hidden_stack.last().expect("empty doc_hidden_stack")
488491
}
489492

493+
fn has_docs(&self, cx: &LateContext<'_>, def_id: DefId) -> bool {
494+
let attrs = cx.tcx.get_attrs_unchecked(def_id);
495+
attrs.iter().any(has_doc)
496+
}
497+
490498
fn check_missing_docs_attrs(
491499
&self,
492500
cx: &LateContext<'_>,
@@ -509,18 +517,21 @@ impl MissingDoc {
509517
// It's an option so the crate root can also use this function (it doesn't
510518
// have a `NodeId`).
511519
if def_id != CRATE_DEF_ID {
512-
if !cx.effective_visibilities.is_exported(def_id) {
520+
// We will check re-exported items at a later stage.
521+
if matches!(cx.tcx.def_kind(def_id), DefKind::Macro(MacroKind::Bang)) {
522+
if !cx.effective_visibilities.is_exported(def_id) {
523+
return;
524+
}
525+
} else if !cx.effective_visibilities.is_directly_public(def_id) {
513526
return;
514527
}
515528
}
516529

517-
let attrs = cx.tcx.hir().attrs(cx.tcx.hir().local_def_id_to_hir_id(def_id));
518-
let has_doc = attrs.iter().any(has_doc);
519-
if !has_doc {
530+
if !self.has_docs(cx, def_id.to_def_id()) {
520531
cx.emit_spanned_lint(
521532
MISSING_DOCS,
522533
cx.tcx.def_span(def_id),
523-
BuiltinMissingDoc { article, desc },
534+
BuiltinMissingDoc { article, desc, reexport: None },
524535
);
525536
}
526537
}
@@ -570,6 +581,100 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
570581
| hir::ItemKind::Const(..)
571582
| hir::ItemKind::Static(..) => {}
572583

584+
hir::ItemKind::Use(ref use_path, hir::UseKind::Single) => {
585+
let use_def_id = it.owner_id.def_id;
586+
if !cx.tcx.is_doc_hidden(use_def_id) &&
587+
// If the re-export is `#[doc(hidden)]` or has an ancestor with this attribute,
588+
// we ignore it.
589+
!cx.tcx.inherits_doc_hidden(use_def_id) &&
590+
let Some(last_res) = use_path.res.last() &&
591+
let Some(res_def_id) = last_res.opt_def_id() &&
592+
// If the original item has `#[doc(hidden)]`, we don't consider it because it
593+
// won't appear into the documentation, unless it's a macro.
594+
(!cx.tcx.is_doc_hidden(res_def_id) || matches!(cx.tcx.def_kind(res_def_id), DefKind::Macro(_))) &&
595+
// We use this to differentiate betweeen `pub use` and `use` since we're only
596+
// interested about re-exports.
597+
match cx.tcx.local_visibility(use_def_id) {
598+
ty::Visibility::Public => true,
599+
ty::Visibility::Restricted(level) => {
600+
level != cx.tcx.parent_module_from_def_id(use_def_id)
601+
}
602+
} &&
603+
// If the re-exported item is not local, it'll be inlined in the documentation
604+
// so it needs to be checked. If it's local and the re-export is the "top one",
605+
// then we check it.
606+
(!res_def_id.is_local() || cx.effective_visibilities.is_directly_public(use_def_id)) &&
607+
!self.has_docs(cx, use_def_id.to_def_id()) &&
608+
!self.has_docs(cx, res_def_id) &&
609+
self.reexported_items.insert(res_def_id)
610+
{
611+
let (article, desc) = cx.tcx.article_and_description(res_def_id);
612+
613+
cx.emit_spanned_lint(
614+
MISSING_DOCS,
615+
cx.tcx.def_span(res_def_id),
616+
BuiltinMissingDoc { article, desc, reexport: Some(it.span) },
617+
);
618+
}
619+
return;
620+
}
621+
hir::ItemKind::Use(ref use_path, hir::UseKind::Glob) => {
622+
let use_def_id = it.owner_id.def_id;
623+
if cx.effective_visibilities.is_directly_public(use_def_id) &&
624+
!cx.tcx.is_doc_hidden(use_def_id) &&
625+
// If the re-export is `#[doc(hidden)]` or has an ancestor with this attribute,
626+
// we ignore it.
627+
!cx.tcx.inherits_doc_hidden(use_def_id) &&
628+
// We use this to differentiate betweeen `pub use` and `use` since we're only
629+
// interested about re-exports.
630+
match cx.tcx.local_visibility(use_def_id) {
631+
ty::Visibility::Public => true,
632+
ty::Visibility::Restricted(level) => {
633+
level != cx.tcx.parent_module_from_def_id(use_def_id)
634+
}
635+
} &&
636+
let Some(last_res) = use_path.res.last() &&
637+
let Some(res_def_id) = last_res.opt_def_id() &&
638+
let Some(res_def_id) = res_def_id.as_local() &&
639+
// We only check check glob re-exports of modules.
640+
cx.tcx.def_kind(res_def_id) == DefKind::Mod
641+
{
642+
for child in cx.tcx.hir().module_items(res_def_id) {
643+
let child_def_id = child.owner_id.to_def_id();
644+
// With glob re-exports, only public items are re-exported.
645+
if cx.effective_visibilities.is_exported(child.owner_id.def_id) &&
646+
// If the original item has `#[doc(hidden)]`, we don't consider it because it
647+
// won't appear into the documentation.
648+
!cx.tcx.is_doc_hidden(child_def_id) &&
649+
!self.has_docs(cx, child_def_id) &&
650+
self.reexported_items.insert(child_def_id) &&
651+
matches!(
652+
cx.tcx.def_kind(child_def_id),
653+
DefKind::TyAlias
654+
| DefKind::Trait
655+
| DefKind::Fn
656+
| DefKind::Macro(..)
657+
| DefKind::Mod
658+
| DefKind::Enum
659+
| DefKind::Struct
660+
| DefKind::Union
661+
| DefKind::Const
662+
| DefKind::Static(..)
663+
)
664+
{
665+
let (article, desc) = cx.tcx.article_and_description(child_def_id);
666+
667+
cx.emit_spanned_lint(
668+
MISSING_DOCS,
669+
cx.tcx.def_span(child_def_id),
670+
BuiltinMissingDoc { article, desc, reexport: Some(it.span) },
671+
);
672+
}
673+
}
674+
}
675+
return;
676+
}
677+
573678
_ => return,
574679
};
575680

compiler/rustc_lint/src/lints.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ pub enum BuiltinUnsafe {
120120
pub struct BuiltinMissingDoc<'a> {
121121
pub article: &'a str,
122122
pub desc: &'a str,
123+
#[note]
124+
pub reexport: Option<Span>,
123125
}
124126

125127
#[derive(LintDiagnostic)]

0 commit comments

Comments
 (0)