1
Fork 0

Fix inconsistencies in handling of inert attributes on statements

When the 'early' and 'late' visitors visit an attribute target, they
activate any lint attributes (e.g. `#[allow]`) that apply to it.
This can affect warnings emitted on sibiling attributes. For example,
the following code does not produce an `unused_attributes` for
`#[inline]`, since the sibiling `#[allow(unused_attributes)]` suppressed
the warning.

```rust
trait Foo {
    #[allow(unused_attributes)] #[inline] fn first();
    #[inline] #[allow(unused_attributes)] fn second();
}
```

However, we do not do this for statements - instead, the lint attributes
only become active when we visit the struct nested inside `StmtKind`
(e.g. `Item`).

Currently, this is difficult to observe due to another issue - the
`HasAttrs` impl for `StmtKind` ignores attributes for `StmtKind::Item`.
As a result, the `unused_doc_comments` lint will never see attributes on
item statements.

This commit makes two interrelated fixes to the handling of inert
(non-proc-macro) attributes on statements:

* The `HasAttr` impl for `StmtKind` now returns attributes for
  `StmtKind::Item`, treating it just like every other `StmtKind`
  variant. The only place relying on the old behavior was macro
  which has been updated to explicitly ignore attributes on item
  statements. This allows the `unused_doc_comments` lint to fire for
  item statements.
* The `early` and `late` lint visitors now activate lint attributes when
  invoking the callback for `Stmt`. This ensures that a lint
  attribute (e.g. `#[allow(unused_doc_comments)]`) can be applied to
  sibiling attributes on an item statement.

For now, the `unused_doc_comments` lint is explicitly disabled on item
statements, which preserves the current behavior. The exact locatiosn
where this lint should fire are being discussed in PR #78306
This commit is contained in:
Aaron Hill 2020-10-23 18:17:00 -04:00
parent 07a63e6d1f
commit ac384ac2db
No known key found for this signature in database
GPG key ID: B4087E510E98B164
12 changed files with 94 additions and 22 deletions

View file

@ -994,7 +994,8 @@ impl EarlyLintPass for UnusedDocComment {
fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &ast::Stmt) {
let kind = match stmt.kind {
ast::StmtKind::Local(..) => "statements",
ast::StmtKind::Item(..) => "inner items",
// Disabled pending discussion in #78306
ast::StmtKind::Item(..) => return,
// expressions will be reported by `check_expr`.
ast::StmtKind::Empty
| ast::StmtKind::Semi(_)

View file

@ -18,6 +18,7 @@ use crate::context::{EarlyContext, LintContext, LintStore};
use crate::passes::{EarlyLintPass, EarlyLintPassObject};
use rustc_ast as ast;
use rustc_ast::visit as ast_visit;
use rustc_attr::HasAttrs;
use rustc_session::lint::{BufferedEarlyLint, LintBuffer, LintPass};
use rustc_session::Session;
use rustc_span::symbol::Ident;
@ -119,8 +120,22 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
}
fn visit_stmt(&mut self, s: &'a ast::Stmt) {
run_early_pass!(self, check_stmt, s);
self.check_id(s.id);
// Add the statement's lint attributes to our
// current state when checking the statement itself.
// This allows us to handle attributes like
// `#[allow(unused_doc_comments)]`, which apply to
// sibling attributes on the same target
//
// Note that statements get their attributes from
// the AST struct that they wrap (e.g. an item)
self.with_lint_attrs(s.id, s.attrs(), |cx| {
run_early_pass!(cx, check_stmt, s);
cx.check_id(s.id);
});
// The visitor for the AST struct wrapped
// by the statement (e.g. `Item`) will call
// `with_lint_attrs`, so do this walk
// outside of the above `with_lint_attrs` call
ast_visit::walk_stmt(self, s);
}

View file

@ -174,12 +174,13 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas
}
fn visit_stmt(&mut self, s: &'tcx hir::Stmt<'tcx>) {
// statement attributes are actually just attributes on one of
// - item
// - local
// - expression
// so we keep track of lint levels there
lint_callback!(self, check_stmt, s);
let get_item = |id: hir::ItemId| self.context.tcx.hir().item(id.id);
let attrs = &s.kind.attrs(get_item);
// See `EarlyContextAndPass::visit_stmt` for an explanation
// of why we call `walk_stmt` outside of `with_lint_attrs`
self.with_lint_attrs(s.hir_id, attrs, |cx| {
lint_callback!(cx, check_stmt, s);
});
hir_visit::walk_stmt(self, s);
}

View file

@ -562,6 +562,13 @@ impl<'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'_, 'tcx> {
})
}
fn visit_stmt(&mut self, e: &'tcx hir::Stmt<'tcx>) {
// We will call `with_lint_attrs` when we walk
// the `StmtKind`. The outer statement itself doesn't
// define the lint levels.
intravisit::walk_stmt(self, e);
}
fn visit_expr(&mut self, e: &'tcx hir::Expr<'tcx>) {
self.with_lint_attrs(e.hir_id, &e.attrs, |builder| {
intravisit::walk_expr(builder, e);