Skip to content

Commit

Permalink
Auto merge of #69084 - yaahc:delayed-doc-lint, r=yaahc
Browse files Browse the repository at this point in the history
Split non macro portion of unused_doc_comment from macro part into two passes/lints

## Motivation

This change is motivated by the needs of the [spandoc library](https://github.com/yaahc/spandoc). The specific use case is that my macro is removing doc comments when an attribute is applied to a fn with doc comments, but I would like the lint to still appear when I forget to add the `#[spandoc]` attribute to a fn, so I don't want to have to silence the lint globally.

## Approach

This change splits the `unused _doc_comment` lint into two lints, `unused_macro_doc_comment` and `unused_doc_comment`. The non macro portion is moved into an `early_lint_pass` rather than a pre_expansion_pass. This allows proc macros to silence `unused_doc_comment` warnings by either adding an attribute to silence it or by removing the doc comment before the early_pass runs.

The `unused_macro_doc_comment` lint however will still be impossible for proc-macros to silence, but the only alternative that I can see is to remove this lint entirely, which I don't think is acceptable / is a decision I'm not comfortable making personally, so instead I opted to split the macro portion of the check into a separate lint so that it can be silenced globally with an attribute if necessary without needing to globally silence the `unused_doc_comment` lint as well, which is still desireable.

fixes #67838
  • Loading branch information
bors committed Feb 23, 2020
2 parents 436494b + 09bc5e3 commit ca07a23
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 82 deletions.
12 changes: 12 additions & 0 deletions src/librustc_expand/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use rustc_parse::configure;
use rustc_parse::parser::Parser;
use rustc_parse::validate_attr;
use rustc_parse::DirectoryOwnership;
use rustc_session::lint::builtin::UNUSED_DOC_COMMENTS;
use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_session::parse::{feature_err, ParseSess};
use rustc_span::source_map::respan;
use rustc_span::symbol::{sym, Symbol};
Expand Down Expand Up @@ -1087,6 +1089,16 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
.note("this may become a hard error in a future release")
.emit();
}

if attr.doc_str().is_some() {
self.cx.parse_sess.buffer_lint_with_diagnostic(
&UNUSED_DOC_COMMENTS,
attr.span,
ast::CRATE_NODE_ID,
"unused doc comment",
BuiltinLintDiagnostics::UnusedDocComment(attr.span),
);
}
}
}
}
Expand Down
89 changes: 29 additions & 60 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,87 +738,56 @@ impl EarlyLintPass for DeprecatedAttr {
}
}

declare_lint! {
pub UNUSED_DOC_COMMENTS,
Warn,
"detects doc comments that aren't used by rustdoc"
}
fn warn_if_doc(cx: &EarlyContext<'_>, node_span: Span, node_kind: &str, attrs: &[ast::Attribute]) {
let mut attrs = attrs.into_iter().peekable();

declare_lint_pass!(UnusedDocComment => [UNUSED_DOC_COMMENTS]);
// Accumulate a single span for sugared doc comments.
let mut sugared_span: Option<Span> = None;

impl UnusedDocComment {
fn warn_if_doc(
&self,
cx: &EarlyContext<'_>,
node_span: Span,
node_kind: &str,
is_macro_expansion: bool,
attrs: &[ast::Attribute],
) {
let mut attrs = attrs.into_iter().peekable();

// Accumulate a single span for sugared doc comments.
let mut sugared_span: Option<Span> = None;

while let Some(attr) = attrs.next() {
if attr.is_doc_comment() {
sugared_span = Some(
sugared_span.map_or_else(|| attr.span, |span| span.with_hi(attr.span.hi())),
);
}
while let Some(attr) = attrs.next() {
if attr.is_doc_comment() {
sugared_span =
Some(sugared_span.map_or_else(|| attr.span, |span| span.with_hi(attr.span.hi())));
}

if attrs.peek().map(|next_attr| next_attr.is_doc_comment()).unwrap_or_default() {
continue;
}
if attrs.peek().map(|next_attr| next_attr.is_doc_comment()).unwrap_or_default() {
continue;
}

let span = sugared_span.take().unwrap_or_else(|| attr.span);
let span = sugared_span.take().unwrap_or_else(|| attr.span);

if attr.is_doc_comment() || attr.check_name(sym::doc) {
cx.struct_span_lint(UNUSED_DOC_COMMENTS, span, |lint| {
let mut err = lint.build("unused doc comment");
err.span_label(
node_span,
format!("rustdoc does not generate documentation for {}", node_kind),
);
if is_macro_expansion {
err.help(
"to document an item produced by a macro, \
the macro must produce the documentation as part of its expansion",
);
}
err.emit();
});
}
if attr.is_doc_comment() || attr.check_name(sym::doc) {
cx.struct_span_lint(UNUSED_DOC_COMMENTS, span, |lint| {
let mut err = lint.build("unused doc comment");
err.span_label(
node_span,
format!("rustdoc does not generate documentation for {}", node_kind),
);
err.emit();
});
}
}
}

impl EarlyLintPass for UnusedDocComment {
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
if let ast::ItemKind::Mac(..) = item.kind {
self.warn_if_doc(cx, item.span, "macro expansions", true, &item.attrs);
}
}

fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &ast::Stmt) {
let (kind, is_macro_expansion) = match stmt.kind {
ast::StmtKind::Local(..) => ("statements", false),
ast::StmtKind::Item(..) => ("inner items", false),
ast::StmtKind::Mac(..) => ("macro expansions", true),
let kind = match stmt.kind {
ast::StmtKind::Local(..) => "statements",
ast::StmtKind::Item(..) => "inner items",
// expressions will be reported by `check_expr`.
ast::StmtKind::Semi(..) | ast::StmtKind::Expr(..) => return,
ast::StmtKind::Semi(..) | ast::StmtKind::Expr(..) | ast::StmtKind::Mac(..) => return,
};

self.warn_if_doc(cx, stmt.span, kind, is_macro_expansion, stmt.kind.attrs());
warn_if_doc(cx, stmt.span, kind, stmt.kind.attrs());
}

fn check_arm(&mut self, cx: &EarlyContext<'_>, arm: &ast::Arm) {
let arm_span = arm.pat.span.with_hi(arm.body.span.hi());
self.warn_if_doc(cx, arm_span, "match arms", false, &arm.attrs);
warn_if_doc(cx, arm_span, "match arms", &arm.attrs);
}

fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
self.warn_if_doc(cx, expr.span, "expressions", false, &expr.attrs);
warn_if_doc(cx, expr.span, "expressions", &expr.attrs);
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/librustc_lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,11 @@ pub trait LintContext: Sized {
BuiltinLintDiagnostics::DeprecatedMacro(suggestion, span) => {
stability::deprecation_suggestion(&mut db, suggestion, span)
}
BuiltinLintDiagnostics::UnusedDocComment(span) => {
db.span_label(span, "rustdoc does not generate documentation for macros");
db.help("to document an item produced by a macro, \
the macro must produce the documentation as part of its expansion");
}
}
// Rewrap `db`, and pass control to the user.
decorate(LintDiagnosticBuilder::new(db));
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fn lint_mod(tcx: TyCtxt<'_>, module_def_id: DefId) {

macro_rules! pre_expansion_lint_passes {
($macro:path, $args:tt) => {
$macro!($args, [KeywordIdents: KeywordIdents, UnusedDocComment: UnusedDocComment,]);
$macro!($args, [KeywordIdents: KeywordIdents,]);
};
}

Expand All @@ -114,6 +114,7 @@ macro_rules! early_lint_passes {
NonAsciiIdents: NonAsciiIdents,
IncompleteFeatures: IncompleteFeatures,
RedundantSemicolon: RedundantSemicolon,
UnusedDocComment: UnusedDocComment,
]
);
};
Expand Down
1 change: 1 addition & 0 deletions src/librustc_session/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ pub enum BuiltinLintDiagnostics {
UnusedImports(String, Vec<(Span, String)>),
RedundantImport(Vec<(Span, bool)>, Ident),
DeprecatedMacro(Option<Symbol>, Span),
UnusedDocComment(Span),
}

/// Lints that are buffered up early on in the `Session` before the
Expand Down
8 changes: 8 additions & 0 deletions src/librustc_session/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,3 +564,11 @@ declare_lint_pass! {
INLINE_NO_SANITIZE,
]
}

declare_lint! {
pub UNUSED_DOC_COMMENTS,
Warn,
"detects doc comments that aren't used by rustdoc"
}

declare_lint_pass!(UnusedDocComment => [UNUSED_DOC_COMMENTS]);
19 changes: 19 additions & 0 deletions src/librustc_session/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,25 @@ impl ParseSess {
});
}

pub fn buffer_lint_with_diagnostic(
&self,
lint: &'static Lint,
span: impl Into<MultiSpan>,
node_id: NodeId,
msg: &str,
diagnostic: BuiltinLintDiagnostics,
) {
self.buffered_lints.with_lock(|buffered_lints| {
buffered_lints.push(BufferedEarlyLint {
span: span.into(),
node_id,
msg: msg.into(),
lint_id: LintId::of(lint),
diagnostic,
});
});
}

/// Extend an error with a suggestion to wrap an expression with parentheses to allow the
/// parser to continue parsing the following operation as part of the same expression.
pub fn expr_parentheses_needed(
Expand Down
12 changes: 4 additions & 8 deletions src/libstd/sys/wasi/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,15 @@ pub struct WasiFd {
fn iovec<'a>(a: &'a mut [IoSliceMut<'_>]) -> &'a [wasi::Iovec] {
assert_eq!(mem::size_of::<IoSliceMut<'_>>(), mem::size_of::<wasi::Iovec>());
assert_eq!(mem::align_of::<IoSliceMut<'_>>(), mem::align_of::<wasi::Iovec>());
/// SAFETY: `IoSliceMut` and `IoVec` have exactly the same memory layout
unsafe {
mem::transmute(a)
}
// SAFETY: `IoSliceMut` and `IoVec` have exactly the same memory layout
unsafe { mem::transmute(a) }
}

fn ciovec<'a>(a: &'a [IoSlice<'_>]) -> &'a [wasi::Ciovec] {
assert_eq!(mem::size_of::<IoSlice<'_>>(), mem::size_of::<wasi::Ciovec>());
assert_eq!(mem::align_of::<IoSlice<'_>>(), mem::align_of::<wasi::Ciovec>());
/// SAFETY: `IoSlice` and `CIoVec` have exactly the same memory layout
unsafe {
mem::transmute(a)
}
// SAFETY: `IoSlice` and `CIoVec` have exactly the same memory layout
unsafe { mem::transmute(a) }
}

impl WasiFd {
Expand Down
22 changes: 9 additions & 13 deletions src/test/ui/useless-comment.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ error: unused doc comment
--> $DIR/useless-comment.rs:9:1
|
LL | /// foo
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | mac!();
| ------- rustdoc does not generate documentation for macro expansions
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ rustdoc does not generate documentation for macros
|
note: the lint level is defined here
--> $DIR/useless-comment.rs:3:9
Expand All @@ -13,6 +11,14 @@ LL | #![deny(unused_doc_comments)]
| ^^^^^^^^^^^^^^^^^^^
= help: to document an item produced by a macro, the macro must produce the documentation as part of its expansion

error: unused doc comment
--> $DIR/useless-comment.rs:32:5
|
LL | /// bar
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ rustdoc does not generate documentation for macros
|
= help: to document an item produced by a macro, the macro must produce the documentation as part of its expansion

error: unused doc comment
--> $DIR/useless-comment.rs:13:5
|
Expand Down Expand Up @@ -68,16 +74,6 @@ LL | #[doc = "bar"]
LL | 3;
| - rustdoc does not generate documentation for expressions

error: unused doc comment
--> $DIR/useless-comment.rs:32:5
|
LL | /// bar
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | mac!();
| ------- rustdoc does not generate documentation for macro expansions
|
= help: to document an item produced by a macro, the macro must produce the documentation as part of its expansion

error: unused doc comment
--> $DIR/useless-comment.rs:35:13
|
Expand Down

0 comments on commit ca07a23

Please sign in to comment.