about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAaron Hill <aa1ronham@gmail.com>2020-10-23 18:17:00 -0400
committerAaron Hill <aa1ronham@gmail.com>2020-10-24 11:55:48 -0400
commit33f3cfcadc10222273ef96e38cccf6e1ddbb882c (patch)
treebbef8a664359e64799049e7eb10ac54610cbdc3f
parentfcde7683fe7ca10c83e5bc17f0969d2284affcd2 (diff)
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
-rw-r--r--clippy_lints/src/utils/author.rs2
-rw-r--r--clippy_lints/src/utils/inspector.rs2
2 files changed, 2 insertions, 2 deletions
diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs
index 89425437eee..7250de3a41c 100644
--- a/clippy_lints/src/utils/author.rs
+++ b/clippy_lints/src/utils/author.rs
@@ -130,7 +130,7 @@ impl<'tcx> LateLintPass<'tcx> for Author {
     }
 
     fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'_>) {
-        if !has_attr(cx.sess(), stmt.kind.attrs()) {
+        if !has_attr(cx.sess(), stmt.kind.attrs(|id| cx.tcx.hir().item(id.id))) {
             return;
         }
         prelude();
diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs
index 93bd8299446..4fbfb3be32c 100644
--- a/clippy_lints/src/utils/inspector.rs
+++ b/clippy_lints/src/utils/inspector.rs
@@ -109,7 +109,7 @@ impl<'tcx> LateLintPass<'tcx> for DeepCodeInspector {
     }
 
     fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'_>) {
-        if !has_attr(cx.sess(), stmt.kind.attrs()) {
+        if !has_attr(cx.sess(), stmt.kind.attrs(|id| cx.tcx.hir().item(id.id))) {
             return;
         }
         match stmt.kind {