From 2ae8b300a7f55f5cf132b640ad6a5727ba282d1e Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 13 Apr 2022 00:36:13 -0400 Subject: [PATCH 1/6] Don't lint `unit_arg` when expanded from a proc-macro --- clippy_lints/src/matches/mod.rs | 6 +- clippy_lints/src/unit_types/unit_arg.rs | 3 +- clippy_utils/src/check_proc_macro.rs | 167 ++++++++++++++++++++++++ clippy_utils/src/lib.rs | 2 + clippy_utils/src/source.rs | 18 --- tests/ui/unit_arg.rs | 12 +- tests/ui/unit_arg.stderr | 20 +-- 7 files changed, 195 insertions(+), 33 deletions(-) create mode 100644 clippy_utils/src/check_proc_macro.rs diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index b638f271602..a74166c32d1 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -21,8 +21,8 @@ mod try_err; mod wild_in_or_pats; -use clippy_utils::source::{snippet_opt, span_starts_with, walk_span_to_context}; -use clippy_utils::{higher, in_constant, meets_msrv, msrvs}; +use clippy_utils::source::{snippet_opt, walk_span_to_context}; +use clippy_utils::{higher, in_constant, is_span_match, meets_msrv, msrvs}; use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat}; use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -949,7 +949,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { let from_expansion = expr.span.from_expansion(); if let ExprKind::Match(ex, arms, source) = expr.kind { - if source == MatchSource::Normal && !span_starts_with(cx, expr.span, "match") { + if source == MatchSource::Normal && !is_span_match(cx, expr.span) { return; } if matches!(source, MatchSource::Normal | MatchSource::ForLoopDesugar) { diff --git a/clippy_lints/src/unit_types/unit_arg.rs b/clippy_lints/src/unit_types/unit_arg.rs index 97d92f10e1c..cd38720f722 100644 --- a/clippy_lints/src/unit_types/unit_arg.rs +++ b/clippy_lints/src/unit_types/unit_arg.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_expr_from_proc_macro; use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt}; use if_chain::if_chain; use rustc_errors::Applicability; @@ -44,7 +45,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { } }) .collect::>(); - if !args_to_recover.is_empty() { + if !args_to_recover.is_empty() && !is_expr_from_proc_macro(cx, expr) { lint_unit_args(cx, expr, &args_to_recover); } }, diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs new file mode 100644 index 00000000000..387bf720008 --- /dev/null +++ b/clippy_utils/src/check_proc_macro.rs @@ -0,0 +1,167 @@ +//! This module handles checking if the span given is from a proc-macro or not. +//! +//! Proc-macros are capable of setting the span of every token they output to a few possible spans. +//! This includes spans we can detect easily as coming from a proc-macro (e.g. the call site +//! or the def site), and spans we can't easily detect as such (e.g. the span of any token +//! passed into the proc macro). This capability means proc-macros are capable of generating code +//! with a span that looks like it was written by the user, but which should not be linted by clippy +//! as it was generated by an external macro. +//! +//! That brings us to this module. The current approach is to determine a small bit of text which +//! must exist at both the start and the end of an item (e.g. an expression or a path) assuming the +//! code was written, and check if the span contains that text. Note this will only work correctly +//! if the span is not from a `macro_rules` based macro. + +use rustc_ast::ast::{IntTy, LitIntType, LitKind, StrStyle, UintTy}; +use rustc_hir::{ + Block, BlockCheckMode, Closure, Destination, Expr, ExprKind, LoopSource, MatchSource, QPath, UnOp, UnsafeSource, + YieldSource, +}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::ty::TyCtxt; +use rustc_session::Session; +use rustc_span::{Span, Symbol}; + +#[derive(Clone, Copy)] +enum Pat { + Str(&'static str), + Sym(Symbol), + Num, +} + +/// Checks if the start and the end of the span's text matches the patterns. This will return false +/// if the span crosses multiple files or if source is not available. +fn span_matches_pat(sess: &Session, span: Span, start_pat: Pat, end_pat: Pat) -> bool { + let pos = sess.source_map().lookup_byte_offset(span.lo()); + let Some(ref src) = pos.sf.src else { + return false; + }; + let end = span.hi() - pos.sf.start_pos; + src.get(pos.pos.0 as usize..end.0 as usize).map_or(false, |s| { + // Spans can be wrapped in a mixture or parenthesis, whitespace, and trailing commas. + let start_str = s.trim_start_matches(|c: char| c.is_whitespace() || c == '('); + let end_str = s.trim_end_matches(|c: char| c.is_whitespace() || c == ')' || c == ','); + (match start_pat { + Pat::Str(text) => start_str.starts_with(text), + Pat::Sym(sym) => start_str.starts_with(sym.as_str()), + Pat::Num => start_str.as_bytes().first().map_or(false, u8::is_ascii_digit), + } && match end_pat { + Pat::Str(text) => end_str.ends_with(text), + Pat::Sym(sym) => end_str.ends_with(sym.as_str()), + Pat::Num => end_str.as_bytes().last().map_or(false, u8::is_ascii_hexdigit), + }) + }) +} + +/// Get the search patterns to use for the given literal +fn lit_search_pat(lit: &LitKind) -> (Pat, Pat) { + match lit { + LitKind::Str(_, StrStyle::Cooked) => (Pat::Str("\""), Pat::Str("\"")), + LitKind::Str(_, StrStyle::Raw(0)) => (Pat::Str("r"), Pat::Str("\"")), + LitKind::Str(_, StrStyle::Raw(_)) => (Pat::Str("r#"), Pat::Str("#")), + LitKind::ByteStr(_) => (Pat::Str("b\""), Pat::Str("\"")), + LitKind::Byte(_) => (Pat::Str("b'"), Pat::Str("'")), + LitKind::Char(_) => (Pat::Str("'"), Pat::Str("'")), + LitKind::Int(_, LitIntType::Signed(IntTy::Isize)) => (Pat::Num, Pat::Str("isize")), + LitKind::Int(_, LitIntType::Unsigned(UintTy::Usize)) => (Pat::Num, Pat::Str("usize")), + LitKind::Int(..) => (Pat::Num, Pat::Num), + LitKind::Float(..) => (Pat::Num, Pat::Str("")), + LitKind::Bool(true) => (Pat::Str("true"), Pat::Str("true")), + LitKind::Bool(false) => (Pat::Str("false"), Pat::Str("false")), + _ => (Pat::Str(""), Pat::Str("")), + } +} + +/// Get the search patterns to use for the given path +fn qpath_search_pat(path: &QPath<'_>) -> (Pat, Pat) { + match path { + QPath::Resolved(ty, path) => { + let start = if ty.is_some() { + Pat::Str("<") + } else { + path.segments + .first() + .map_or(Pat::Str(""), |seg| Pat::Sym(seg.ident.name)) + }; + let end = path.segments.last().map_or(Pat::Str(""), |seg| { + if seg.args.is_some() { + Pat::Str(">") + } else { + Pat::Sym(seg.ident.name) + } + }); + (start, end) + }, + QPath::TypeRelative(_, name) => (Pat::Str(""), Pat::Sym(name.ident.name)), + QPath::LangItem(..) => (Pat::Str(""), Pat::Str("")), + } +} + +/// Get the search patterns to use for the given expression +fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) { + match e.kind { + ExprKind::Box(e) => (Pat::Str("box"), expr_search_pat(tcx, e).1), + ExprKind::ConstBlock(_) => (Pat::Str("const"), Pat::Str("}")), + ExprKind::Tup([]) => (Pat::Str(")"), Pat::Str("(")), + ExprKind::Unary(UnOp::Deref, _) => (Pat::Str("*"), expr_search_pat(tcx, e).1), + ExprKind::Unary(UnOp::Not, _) => (Pat::Str("!"), expr_search_pat(tcx, e).1), + ExprKind::Unary(UnOp::Neg, _) => (Pat::Str("-"), expr_search_pat(tcx, e).1), + ExprKind::Lit(ref lit) => lit_search_pat(&lit.node), + ExprKind::Array(_) | ExprKind::Repeat(..) => (Pat::Str("["), Pat::Str("]")), + ExprKind::Call(e, []) | ExprKind::MethodCall(_, [e], _) => (expr_search_pat(tcx, e).0, Pat::Str("(")), + ExprKind::Call(first, [.., last]) + | ExprKind::MethodCall(_, [first, .., last], _) + | ExprKind::Binary(_, first, last) + | ExprKind::Tup([first, .., last]) + | ExprKind::Assign(first, last, _) + | ExprKind::AssignOp(_, first, last) => (expr_search_pat(tcx, first).0, expr_search_pat(tcx, last).1), + ExprKind::Tup([e]) | ExprKind::DropTemps(e) => expr_search_pat(tcx, e), + ExprKind::Cast(e, _) | ExprKind::Type(e, _) => (expr_search_pat(tcx, e).0, Pat::Str("")), + ExprKind::Let(let_expr) => (Pat::Str("let"), expr_search_pat(tcx, let_expr.init).1), + ExprKind::If(..) => (Pat::Str("if"), Pat::Str("}")), + ExprKind::Loop(_, Some(_), _, _) | ExprKind::Block(_, Some(_)) => (Pat::Str("'"), Pat::Str("}")), + ExprKind::Loop(_, None, LoopSource::Loop, _) => (Pat::Str("loop"), Pat::Str("}")), + ExprKind::Loop(_, None, LoopSource::While, _) => (Pat::Str("while"), Pat::Str("}")), + ExprKind::Loop(_, None, LoopSource::ForLoop, _) | ExprKind::Match(_, _, MatchSource::ForLoopDesugar) => { + (Pat::Str("for"), Pat::Str("}")) + }, + ExprKind::Match(_, _, MatchSource::Normal) => (Pat::Str("match"), Pat::Str("}")), + ExprKind::Match(e, _, MatchSource::TryDesugar) => (expr_search_pat(tcx, e).0, Pat::Str("?")), + ExprKind::Match(e, _, MatchSource::AwaitDesugar) | ExprKind::Yield(e, YieldSource::Await { .. }) => { + (expr_search_pat(tcx, e).0, Pat::Str("await")) + }, + ExprKind::Closure(&Closure { body, .. }) => (Pat::Str(""), expr_search_pat(tcx, &tcx.hir().body(body).value).1), + ExprKind::Block( + Block { + rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided), + .. + }, + None, + ) => (Pat::Str("unsafe"), Pat::Str("}")), + ExprKind::Block(_, None) => (Pat::Str("{"), Pat::Str("}")), + ExprKind::Field(e, name) => (expr_search_pat(tcx, e).0, Pat::Sym(name.name)), + ExprKind::Index(e, _) => (expr_search_pat(tcx, e).0, Pat::Str("]")), + ExprKind::Path(ref path) => qpath_search_pat(path), + ExprKind::AddrOf(_, _, e) => (Pat::Str("&"), expr_search_pat(tcx, e).1), + ExprKind::Break(Destination { label: None, .. }, None) => (Pat::Str("break"), Pat::Str("break")), + ExprKind::Break(Destination { label: Some(name), .. }, None) => (Pat::Str("break"), Pat::Sym(name.ident.name)), + ExprKind::Break(_, Some(e)) => (Pat::Str("break"), expr_search_pat(tcx, e).1), + ExprKind::Continue(Destination { label: None, .. }) => (Pat::Str("continue"), Pat::Str("continue")), + ExprKind::Continue(Destination { label: Some(name), .. }) => (Pat::Str("continue"), Pat::Sym(name.ident.name)), + ExprKind::Ret(None) => (Pat::Str("return"), Pat::Str("return")), + ExprKind::Struct(path, _, _) => (qpath_search_pat(path).0, Pat::Str("}")), + ExprKind::Yield(e, YieldSource::Yield) => (Pat::Str("yield"), expr_search_pat(tcx, e).1), + _ => (Pat::Str(""), Pat::Str("")), + } +} + +/// Checks if the expression likely came from a proc-macro +pub fn is_expr_from_proc_macro(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { + let (start_pat, end_pat) = expr_search_pat(cx.tcx, e); + !span_matches_pat(cx.sess(), e.span, start_pat, end_pat) +} + +/// Checks if the span actually refers to a match expression +pub fn is_span_match(cx: &LateContext<'_>, span: Span) -> bool { + span_matches_pat(cx.sess(), span, Pat::Str("match"), Pat::Str("}")) +} diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 9ffbc73dc62..2d051e3a815 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -38,6 +38,7 @@ pub mod ast_utils; pub mod attrs; +mod check_proc_macro; pub mod comparisons; pub mod consts; pub mod diagnostics; @@ -58,6 +59,7 @@ pub mod visitors; pub use self::attrs::*; +pub use self::check_proc_macro::{is_expr_from_proc_macro, is_span_match}; pub use self::hir_utils::{ both, count_eq, eq_expr_value, hash_expr, hash_stmt, over, HirEqInterExpr, SpanlessEq, SpanlessHash, }; diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index 1197fe914de..d85f591fb9a 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -11,24 +11,6 @@ use rustc_span::{BytePos, Pos, Span, SpanData, SyntaxContext}; use std::borrow::Cow; -/// Checks if the span starts with the given text. This will return false if the span crosses -/// multiple files or if source is not available. -/// -/// This is used to check for proc macros giving unhelpful spans to things. -pub fn span_starts_with(cx: &T, span: Span, text: &str) -> bool { - fn helper(sm: &SourceMap, span: Span, text: &str) -> bool { - let pos = sm.lookup_byte_offset(span.lo()); - let Some(ref src) = pos.sf.src else { - return false; - }; - let end = span.hi() - pos.sf.start_pos; - src.get(pos.pos.0 as usize..end.0 as usize) - // Expression spans can include wrapping parenthesis. Remove them first. - .map_or(false, |s| s.trim_start_matches('(').starts_with(text)) - } - helper(cx.sess().source_map(), span, text) -} - /// Like `snippet_block`, but add braces if the expr is not an `ExprKind::Block`. /// Also takes an `Option` which can be put inside the braces. pub fn expr_block<'a, T: LintContext>( diff --git a/tests/ui/unit_arg.rs b/tests/ui/unit_arg.rs index 38be87bddf1..7bf3adc07ac 100644 --- a/tests/ui/unit_arg.rs +++ b/tests/ui/unit_arg.rs @@ -1,3 +1,5 @@ +// aux-build: proc_macro_with_span.rs + #![warn(clippy::unit_arg)] #![allow( clippy::no_effect, @@ -8,9 +10,13 @@ clippy::or_fun_call, clippy::needless_question_mark, clippy::self_named_constructors, - clippy::let_unit_value + clippy::let_unit_value, + clippy::never_loop )] +extern crate proc_macro_with_span; + +use proc_macro_with_span::with_span; use std::fmt::Debug; fn foo(t: T) { @@ -127,6 +133,10 @@ fn returning_expr() -> Option<()> { fn taking_multiple_units(a: (), b: ()) {} +fn proc_macro() { + with_span!(span taking_multiple_units(unsafe { (); }, 'x: loop { break 'x (); })); +} + fn main() { bad(); ok(); diff --git a/tests/ui/unit_arg.stderr b/tests/ui/unit_arg.stderr index 11cfe66a30e..1de9d44bb0d 100644 --- a/tests/ui/unit_arg.stderr +++ b/tests/ui/unit_arg.stderr @@ -1,5 +1,5 @@ error: passing a unit value to a function - --> $DIR/unit_arg.rs:57:5 + --> $DIR/unit_arg.rs:63:5 | LL | / foo({ LL | | 1; @@ -20,7 +20,7 @@ LL ~ foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:60:5 + --> $DIR/unit_arg.rs:66:5 | LL | foo(foo(1)); | ^^^^^^^^^^^ @@ -32,7 +32,7 @@ LL ~ foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:61:5 + --> $DIR/unit_arg.rs:67:5 | LL | / foo({ LL | | foo(1); @@ -54,7 +54,7 @@ LL ~ foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:66:5 + --> $DIR/unit_arg.rs:72:5 | LL | / b.bar({ LL | | 1; @@ -74,7 +74,7 @@ LL ~ b.bar(()); | error: passing unit values to a function - --> $DIR/unit_arg.rs:69:5 + --> $DIR/unit_arg.rs:75:5 | LL | taking_multiple_units(foo(0), foo(1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -87,7 +87,7 @@ LL ~ taking_multiple_units((), ()); | error: passing unit values to a function - --> $DIR/unit_arg.rs:70:5 + --> $DIR/unit_arg.rs:76:5 | LL | / taking_multiple_units(foo(0), { LL | | foo(1); @@ -110,7 +110,7 @@ LL ~ taking_multiple_units((), ()); | error: passing unit values to a function - --> $DIR/unit_arg.rs:74:5 + --> $DIR/unit_arg.rs:80:5 | LL | / taking_multiple_units( LL | | { @@ -146,7 +146,7 @@ LL ~ ); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:85:13 + --> $DIR/unit_arg.rs:91:13 | LL | None.or(Some(foo(2))); | ^^^^^^^^^^^^ @@ -160,7 +160,7 @@ LL ~ }); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:88:5 + --> $DIR/unit_arg.rs:94:5 | LL | foo(foo(())); | ^^^^^^^^^^^^ @@ -172,7 +172,7 @@ LL ~ foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:125:5 + --> $DIR/unit_arg.rs:131:5 | LL | Some(foo(1)) | ^^^^^^^^^^^^ From 37e838f759b99dd3ac042a61f60a4f14272b99c2 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 13 Apr 2022 01:13:47 -0400 Subject: [PATCH 2/6] Use new util function in `suspicious_else_formatting` --- clippy_lints/src/formatting.rs | 6 +++--- clippy_utils/src/check_proc_macro.rs | 7 ++++++- clippy_utils/src/lib.rs | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index db0166da57f..01cefe4af85 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note}; +use clippy_utils::is_span_if; use clippy_utils::source::snippet_opt; use if_chain::if_chain; use rustc_ast::ast::{BinOpKind, Block, Expr, ExprKind, StmtKind, UnOp}; @@ -297,12 +298,11 @@ fn check_array(cx: &EarlyContext<'_>, expr: &Expr) { fn check_missing_else(cx: &EarlyContext<'_>, first: &Expr, second: &Expr) { if_chain! { if !first.span.from_expansion() && !second.span.from_expansion(); - if let ExprKind::If(cond_expr, ..) = &first.kind; + if matches!(first.kind, ExprKind::If(..)); if is_block(second) || is_if(second); // Proc-macros can give weird spans. Make sure this is actually an `if`. - if let Some(if_snip) = snippet_opt(cx, first.span.until(cond_expr.span)); - if if_snip.starts_with("if"); + if is_span_if(cx, first.span); // If there is a line break between the two expressions, don't lint. // If there is a non-whitespace character, this span came from a proc-macro. diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 387bf720008..51cd70f5e7c 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -162,6 +162,11 @@ pub fn is_expr_from_proc_macro(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { } /// Checks if the span actually refers to a match expression -pub fn is_span_match(cx: &LateContext<'_>, span: Span) -> bool { +pub fn is_span_match(cx: &impl LintContext, span: Span) -> bool { span_matches_pat(cx.sess(), span, Pat::Str("match"), Pat::Str("}")) } + +/// Checks if the span actually refers to an if expression +pub fn is_span_if(cx: &impl LintContext, span: Span) -> bool { + span_matches_pat(cx.sess(), span, Pat::Str("if"), Pat::Str("}")) +} diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 2d051e3a815..086e6d6e3a2 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -59,7 +59,7 @@ pub mod visitors; pub use self::attrs::*; -pub use self::check_proc_macro::{is_expr_from_proc_macro, is_span_match}; +pub use self::check_proc_macro::{is_expr_from_proc_macro, is_span_if, is_span_match}; pub use self::hir_utils::{ both, count_eq, eq_expr_value, hash_expr, hash_stmt, over, HirEqInterExpr, SpanlessEq, SpanlessHash, }; From 4ae582ef88adbdcd3a01686a7a61120efd7a208b Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 13 Apr 2022 09:23:12 -0400 Subject: [PATCH 3/6] Don't lint `missing_docs_in_private_items` on proc-macro output --- clippy_lints/src/missing_doc.rs | 21 ++- clippy_lints/src/unit_types/mod.rs | 2 +- clippy_lints/src/unit_types/unit_arg.rs | 6 +- clippy_utils/src/check_proc_macro.rs | 130 +++++++++++++++++- clippy_utils/src/lib.rs | 2 +- tests/ui/{missing-doc.rs => missing_doc.rs} | 13 ++ ...{missing-doc.stderr => missing_doc.stderr} | 48 +++---- ...sing-doc-crate.rs => missing_doc_crate.rs} | 0 ...issing.rs => missing_doc_crate_missing.rs} | 0 ...tderr => missing_doc_crate_missing.stderr} | 2 +- ...issing-doc-impl.rs => missing_doc_impl.rs} | 15 ++ ...oc-impl.stderr => missing_doc_impl.stderr} | 30 ++-- 12 files changed, 212 insertions(+), 57 deletions(-) rename tests/ui/{missing-doc.rs => missing_doc.rs} (82%) rename tests/ui/{missing-doc.stderr => missing_doc.stderr} (79%) rename tests/ui/{missing-doc-crate.rs => missing_doc_crate.rs} (100%) rename tests/ui/{missing-doc-crate-missing.rs => missing_doc_crate_missing.rs} (100%) rename tests/ui/{missing-doc-crate-missing.stderr => missing_doc_crate_missing.stderr} (86%) rename tests/ui/{missing-doc-impl.rs => missing_doc_impl.rs} (83%) rename tests/ui/{missing-doc-impl.stderr => missing_doc_impl.stderr} (78%) diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index b99052e66ba..7ef80b1a596 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -7,6 +7,7 @@ use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::span_lint; +use clippy_utils::is_from_proc_macro; use if_chain::if_chain; use rustc_ast::ast::{self, MetaItem, MetaItemKind}; use rustc_hir as hir; @@ -158,14 +159,18 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::Item<'_>) { let (article, desc) = cx.tcx.article_and_description(it.def_id.to_def_id()); let attrs = cx.tcx.hir().attrs(it.hir_id()); - self.check_missing_docs_attrs(cx, attrs, it.span, article, desc); + if !is_from_proc_macro(cx, it) { + self.check_missing_docs_attrs(cx, attrs, it.span, article, desc); + } } fn check_trait_item(&mut self, cx: &LateContext<'tcx>, trait_item: &'tcx hir::TraitItem<'_>) { let (article, desc) = cx.tcx.article_and_description(trait_item.def_id.to_def_id()); let attrs = cx.tcx.hir().attrs(trait_item.hir_id()); - self.check_missing_docs_attrs(cx, attrs, trait_item.span, article, desc); + if !is_from_proc_macro(cx, trait_item) { + self.check_missing_docs_attrs(cx, attrs, trait_item.span, article, desc); + } } fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::ImplItem<'_>) { @@ -181,18 +186,24 @@ fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::Impl let (article, desc) = cx.tcx.article_and_description(impl_item.def_id.to_def_id()); let attrs = cx.tcx.hir().attrs(impl_item.hir_id()); - self.check_missing_docs_attrs(cx, attrs, impl_item.span, article, desc); + if !is_from_proc_macro(cx, impl_item) { + self.check_missing_docs_attrs(cx, attrs, impl_item.span, article, desc); + } } fn check_field_def(&mut self, cx: &LateContext<'tcx>, sf: &'tcx hir::FieldDef<'_>) { if !sf.is_positional() { let attrs = cx.tcx.hir().attrs(sf.hir_id); - self.check_missing_docs_attrs(cx, attrs, sf.span, "a", "struct field"); + if !is_from_proc_macro(cx, sf) { + self.check_missing_docs_attrs(cx, attrs, sf.span, "a", "struct field"); + } } } fn check_variant(&mut self, cx: &LateContext<'tcx>, v: &'tcx hir::Variant<'_>) { let attrs = cx.tcx.hir().attrs(v.id); - self.check_missing_docs_attrs(cx, attrs, v.span, "a", "variant"); + if !is_from_proc_macro(cx, v) { + self.check_missing_docs_attrs(cx, attrs, v.span, "a", "variant"); + } } } diff --git a/clippy_lints/src/unit_types/mod.rs b/clippy_lints/src/unit_types/mod.rs index 6aa86a57c9b..546242ebd9a 100644 --- a/clippy_lints/src/unit_types/mod.rs +++ b/clippy_lints/src/unit_types/mod.rs @@ -103,7 +103,7 @@ fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) { let_unit_value::check(cx, local); } - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { unit_cmp::check(cx, expr); unit_arg::check(cx, expr); } diff --git a/clippy_lints/src/unit_types/unit_arg.rs b/clippy_lints/src/unit_types/unit_arg.rs index cd38720f722..16da2f11b81 100644 --- a/clippy_lints/src/unit_types/unit_arg.rs +++ b/clippy_lints/src/unit_types/unit_arg.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::is_expr_from_proc_macro; +use clippy_utils::is_from_proc_macro; use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt}; use if_chain::if_chain; use rustc_errors::Applicability; @@ -8,7 +8,7 @@ use super::{utils, UNIT_ARG}; -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if expr.span.from_expansion() { return; } @@ -45,7 +45,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { } }) .collect::>(); - if !args_to_recover.is_empty() && !is_expr_from_proc_macro(cx, expr) { + if !args_to_recover.is_empty() && !is_from_proc_macro(cx, expr) { lint_unit_args(cx, expr, &args_to_recover); } }, diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 51cd70f5e7c..4dd0ec6b6d0 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -14,17 +14,20 @@ use rustc_ast::ast::{IntTy, LitIntType, LitKind, StrStyle, UintTy}; use rustc_hir::{ - Block, BlockCheckMode, Closure, Destination, Expr, ExprKind, LoopSource, MatchSource, QPath, UnOp, UnsafeSource, - YieldSource, + Block, BlockCheckMode, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, Impl, ImplItem, ImplItemKind, + IsAuto, Item, ItemKind, LoopSource, MatchSource, QPath, TraitItem, TraitItemKind, UnOp, UnsafeSource, Unsafety, + Variant, VariantData, VisibilityKind, YieldSource, }; use rustc_lint::{LateContext, LintContext}; use rustc_middle::ty::TyCtxt; use rustc_session::Session; use rustc_span::{Span, Symbol}; +use rustc_target::spec::abi::Abi; #[derive(Clone, Copy)] -enum Pat { +pub enum Pat { Str(&'static str), + MultiStr(&'static [&'static str]), Sym(Symbol), Num, } @@ -43,10 +46,12 @@ fn span_matches_pat(sess: &Session, span: Span, start_pat: Pat, end_pat: Pat) -> let end_str = s.trim_end_matches(|c: char| c.is_whitespace() || c == ')' || c == ','); (match start_pat { Pat::Str(text) => start_str.starts_with(text), + Pat::MultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)), Pat::Sym(sym) => start_str.starts_with(sym.as_str()), Pat::Num => start_str.as_bytes().first().map_or(false, u8::is_ascii_digit), } && match end_pat { Pat::Str(text) => end_str.ends_with(text), + Pat::MultiStr(texts) => texts.iter().any(|s| start_str.ends_with(s)), Pat::Sym(sym) => end_str.ends_with(sym.as_str()), Pat::Num => end_str.as_bytes().last().map_or(false, u8::is_ascii_hexdigit), }) @@ -155,10 +160,121 @@ fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) { } } -/// Checks if the expression likely came from a proc-macro -pub fn is_expr_from_proc_macro(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { - let (start_pat, end_pat) = expr_search_pat(cx.tcx, e); - !span_matches_pat(cx.sess(), e.span, start_pat, end_pat) +fn fn_header_search_pat(header: FnHeader) -> Pat { + if header.is_async() { + Pat::Str("async") + } else if header.is_const() { + Pat::Str("const") + } else if header.is_unsafe() { + Pat::Str("unsafe") + } else if header.abi != Abi::Rust { + Pat::Str("extern") + } else { + Pat::MultiStr(&["fn", "extern"]) + } +} + +fn item_search_pat(item: &Item<'_>) -> (Pat, Pat) { + let (start_pat, end_pat) = match &item.kind { + ItemKind::ExternCrate(_) => (Pat::Str("extern"), Pat::Str(";")), + ItemKind::Static(..) => (Pat::Str("static"), Pat::Str(";")), + ItemKind::Const(..) => (Pat::Str("const"), Pat::Str(";")), + ItemKind::Fn(sig, ..) => (fn_header_search_pat(sig.header), Pat::Str("")), + ItemKind::ForeignMod { .. } => (Pat::Str("extern"), Pat::Str("}")), + ItemKind::TyAlias(..) | ItemKind::OpaqueTy(_) => (Pat::Str("type"), Pat::Str(";")), + ItemKind::Enum(..) => (Pat::Str("enum"), Pat::Str("}")), + ItemKind::Struct(VariantData::Struct(..), _) => (Pat::Str("struct"), Pat::Str("}")), + ItemKind::Struct(..) => (Pat::Str("struct"), Pat::Str(";")), + ItemKind::Union(..) => (Pat::Str("union"), Pat::Str("}")), + ItemKind::Trait(_, Unsafety::Unsafe, ..) + | ItemKind::Impl(Impl { + unsafety: Unsafety::Unsafe, + .. + }) => (Pat::Str("unsafe"), Pat::Str("}")), + ItemKind::Trait(IsAuto::Yes, ..) => (Pat::Str("auto"), Pat::Str("}")), + ItemKind::Trait(..) => (Pat::Str("trait"), Pat::Str("}")), + ItemKind::Impl(_) => (Pat::Str("impl"), Pat::Str("}")), + _ => return (Pat::Str(""), Pat::Str("")), + }; + if matches!(item.vis.node, VisibilityKind::Inherited) { + (start_pat, end_pat) + } else { + (Pat::Str("pub"), end_pat) + } +} + +fn trait_item_search_pat(item: &TraitItem<'_>) -> (Pat, Pat) { + match &item.kind { + TraitItemKind::Const(..) => (Pat::Str("const"), Pat::Str(";")), + TraitItemKind::Type(..) => (Pat::Str("type"), Pat::Str(";")), + TraitItemKind::Fn(sig, ..) => (fn_header_search_pat(sig.header), Pat::Str("")), + } +} + +fn impl_item_search_pat(item: &ImplItem<'_>) -> (Pat, Pat) { + let (start_pat, end_pat) = match &item.kind { + ImplItemKind::Const(..) => (Pat::Str("const"), Pat::Str(";")), + ImplItemKind::TyAlias(..) => (Pat::Str("type"), Pat::Str(";")), + ImplItemKind::Fn(sig, ..) => (fn_header_search_pat(sig.header), Pat::Str("")), + }; + if matches!(item.vis.node, VisibilityKind::Inherited) { + (start_pat, end_pat) + } else { + (Pat::Str("pub"), end_pat) + } +} + +fn field_def_search_pat(def: &FieldDef<'_>) -> (Pat, Pat) { + if matches!(def.vis.node, VisibilityKind::Inherited) { + if def.is_positional() { + (Pat::Str(""), Pat::Str("")) + } else { + (Pat::Sym(def.ident.name), Pat::Str("")) + } + } else { + (Pat::Str("pub"), Pat::Str("")) + } +} + +fn variant_search_pat(v: &Variant<'_>) -> (Pat, Pat) { + match v.data { + VariantData::Struct(..) => (Pat::Sym(v.ident.name), Pat::Str("}")), + VariantData::Tuple(..) => (Pat::Sym(v.ident.name), Pat::Str("")), + VariantData::Unit(..) => (Pat::Sym(v.ident.name), Pat::Sym(v.ident.name)), + } +} + +pub trait WithSearchPat { + type Context: LintContext; + fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat); + fn span(&self) -> Span; +} +macro_rules! impl_with_search_pat { + ($cx:ident: $ty:ident with $fn:ident $(($tcx:ident))?) => { + impl<'cx> WithSearchPat for $ty<'cx> { + type Context = $cx<'cx>; + #[allow(unused_variables)] + fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat) { + $(let $tcx = cx.tcx;)? + $fn($($tcx,)? self) + } + fn span(&self) -> Span { + self.span + } + } + }; +} +impl_with_search_pat!(LateContext: Expr with expr_search_pat(tcx)); +impl_with_search_pat!(LateContext: Item with item_search_pat); +impl_with_search_pat!(LateContext: TraitItem with trait_item_search_pat); +impl_with_search_pat!(LateContext: ImplItem with impl_item_search_pat); +impl_with_search_pat!(LateContext: FieldDef with field_def_search_pat); +impl_with_search_pat!(LateContext: Variant with variant_search_pat); + +/// Checks if the item likely came from a proc-macro +pub fn is_from_proc_macro(cx: &T::Context, item: &T) -> bool { + let (start_pat, end_pat) = item.search_pat(cx); + !span_matches_pat(cx.sess(), item.span(), start_pat, end_pat) } /// Checks if the span actually refers to a match expression diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 086e6d6e3a2..dcfc03475b4 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -59,7 +59,7 @@ pub mod visitors; pub use self::attrs::*; -pub use self::check_proc_macro::{is_expr_from_proc_macro, is_span_if, is_span_match}; +pub use self::check_proc_macro::{is_from_proc_macro, is_span_if, is_span_match}; pub use self::hir_utils::{ both, count_eq, eq_expr_value, hash_expr, hash_stmt, over, HirEqInterExpr, SpanlessEq, SpanlessHash, }; diff --git a/tests/ui/missing-doc.rs b/tests/ui/missing_doc.rs similarity index 82% rename from tests/ui/missing-doc.rs rename to tests/ui/missing_doc.rs index 6e2e710e21c..29cc026a8fd 100644 --- a/tests/ui/missing-doc.rs +++ b/tests/ui/missing_doc.rs @@ -1,3 +1,5 @@ +// aux-build: proc_macro_with_span.rs + #![warn(clippy::missing_docs_in_private_items)] // When denying at the crate level, be sure to not get random warnings from the // injected intrinsics by the compiler. @@ -5,6 +7,9 @@ //! Some garbage docs for the crate here #![doc = "More garbage"] +extern crate proc_macro_with_span; + +use proc_macro_with_span::with_span; use std::arch::global_asm; type Typedef = String; @@ -100,3 +105,11 @@ fn main() {} // Ensure global asm doesn't require documentation. global_asm! { "" } + +// Don't lint proc macro output with an unexpected span. +with_span!(span pub struct FooPm { pub field: u32}); +with_span!(span pub struct FooPm2;); +with_span!(span pub enum FooPm3 { A, B(u32), C { field: u32 }}); +with_span!(span pub fn foo_pm() {}); +with_span!(span pub static FOO_PM: u32 = 0;); +with_span!(span pub const FOO2_PM: u32 = 0;); diff --git a/tests/ui/missing-doc.stderr b/tests/ui/missing_doc.stderr similarity index 79% rename from tests/ui/missing-doc.stderr rename to tests/ui/missing_doc.stderr index a876dc078eb..6c8e66f4643 100644 --- a/tests/ui/missing-doc.stderr +++ b/tests/ui/missing_doc.stderr @@ -1,5 +1,5 @@ error: missing documentation for a type alias - --> $DIR/missing-doc.rs:10:1 + --> $DIR/missing_doc.rs:15:1 | LL | type Typedef = String; | ^^^^^^^^^^^^^^^^^^^^^^ @@ -7,37 +7,37 @@ LL | type Typedef = String; = note: `-D clippy::missing-docs-in-private-items` implied by `-D warnings` error: missing documentation for a type alias - --> $DIR/missing-doc.rs:11:1 + --> $DIR/missing_doc.rs:16:1 | LL | pub type PubTypedef = String; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a module - --> $DIR/missing-doc.rs:13:1 + --> $DIR/missing_doc.rs:18:1 | LL | mod module_no_dox {} | ^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a module - --> $DIR/missing-doc.rs:14:1 + --> $DIR/missing_doc.rs:19:1 | LL | pub mod pub_module_no_dox {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a function - --> $DIR/missing-doc.rs:18:1 + --> $DIR/missing_doc.rs:23:1 | LL | pub fn foo2() {} | ^^^^^^^^^^^^^^^^ error: missing documentation for a function - --> $DIR/missing-doc.rs:19:1 + --> $DIR/missing_doc.rs:24:1 | LL | fn foo3() {} | ^^^^^^^^^^^^ error: missing documentation for an enum - --> $DIR/missing-doc.rs:33:1 + --> $DIR/missing_doc.rs:38:1 | LL | / enum Baz { LL | | BazA { a: isize, b: isize }, @@ -46,31 +46,31 @@ LL | | } | |_^ error: missing documentation for a variant - --> $DIR/missing-doc.rs:34:5 + --> $DIR/missing_doc.rs:39:5 | LL | BazA { a: isize, b: isize }, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a struct field - --> $DIR/missing-doc.rs:34:12 + --> $DIR/missing_doc.rs:39:12 | LL | BazA { a: isize, b: isize }, | ^^^^^^^^ error: missing documentation for a struct field - --> $DIR/missing-doc.rs:34:22 + --> $DIR/missing_doc.rs:39:22 | LL | BazA { a: isize, b: isize }, | ^^^^^^^^ error: missing documentation for a variant - --> $DIR/missing-doc.rs:35:5 + --> $DIR/missing_doc.rs:40:5 | LL | BarB, | ^^^^ error: missing documentation for an enum - --> $DIR/missing-doc.rs:38:1 + --> $DIR/missing_doc.rs:43:1 | LL | / pub enum PubBaz { LL | | PubBazA { a: isize }, @@ -78,43 +78,43 @@ LL | | } | |_^ error: missing documentation for a variant - --> $DIR/missing-doc.rs:39:5 + --> $DIR/missing_doc.rs:44:5 | LL | PubBazA { a: isize }, | ^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a struct field - --> $DIR/missing-doc.rs:39:15 + --> $DIR/missing_doc.rs:44:15 | LL | PubBazA { a: isize }, | ^^^^^^^^ error: missing documentation for a constant - --> $DIR/missing-doc.rs:59:1 + --> $DIR/missing_doc.rs:64:1 | LL | const FOO: u32 = 0; | ^^^^^^^^^^^^^^^^^^^ error: missing documentation for a constant - --> $DIR/missing-doc.rs:66:1 + --> $DIR/missing_doc.rs:71:1 | LL | pub const FOO4: u32 = 0; | ^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a static - --> $DIR/missing-doc.rs:68:1 + --> $DIR/missing_doc.rs:73:1 | LL | static BAR: u32 = 0; | ^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a static - --> $DIR/missing-doc.rs:75:1 + --> $DIR/missing_doc.rs:80:1 | LL | pub static BAR4: u32 = 0; | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a module - --> $DIR/missing-doc.rs:77:1 + --> $DIR/missing_doc.rs:82:1 | LL | / mod internal_impl { LL | | /// dox @@ -126,31 +126,31 @@ LL | | } | |_^ error: missing documentation for a function - --> $DIR/missing-doc.rs:80:5 + --> $DIR/missing_doc.rs:85:5 | LL | pub fn undocumented1() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a function - --> $DIR/missing-doc.rs:81:5 + --> $DIR/missing_doc.rs:86:5 | LL | pub fn undocumented2() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a function - --> $DIR/missing-doc.rs:82:5 + --> $DIR/missing_doc.rs:87:5 | LL | fn undocumented3() {} | ^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a function - --> $DIR/missing-doc.rs:87:9 + --> $DIR/missing_doc.rs:92:9 | LL | pub fn also_undocumented1() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a function - --> $DIR/missing-doc.rs:88:9 + --> $DIR/missing_doc.rs:93:9 | LL | fn also_undocumented2() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/missing-doc-crate.rs b/tests/ui/missing_doc_crate.rs similarity index 100% rename from tests/ui/missing-doc-crate.rs rename to tests/ui/missing_doc_crate.rs diff --git a/tests/ui/missing-doc-crate-missing.rs b/tests/ui/missing_doc_crate_missing.rs similarity index 100% rename from tests/ui/missing-doc-crate-missing.rs rename to tests/ui/missing_doc_crate_missing.rs diff --git a/tests/ui/missing-doc-crate-missing.stderr b/tests/ui/missing_doc_crate_missing.stderr similarity index 86% rename from tests/ui/missing-doc-crate-missing.stderr rename to tests/ui/missing_doc_crate_missing.stderr index d56c5cc4c3a..19516bf5fab 100644 --- a/tests/ui/missing-doc-crate-missing.stderr +++ b/tests/ui/missing_doc_crate_missing.stderr @@ -1,5 +1,5 @@ error: missing documentation for the crate - --> $DIR/missing-doc-crate-missing.rs:1:1 + --> $DIR/missing_doc_crate_missing.rs:1:1 | LL | / #![warn(clippy::missing_docs_in_private_items)] LL | | diff --git a/tests/ui/missing-doc-impl.rs b/tests/ui/missing_doc_impl.rs similarity index 83% rename from tests/ui/missing-doc-impl.rs rename to tests/ui/missing_doc_impl.rs index d5724bf661c..0396d1193ff 100644 --- a/tests/ui/missing-doc-impl.rs +++ b/tests/ui/missing_doc_impl.rs @@ -1,3 +1,5 @@ +// aux-build: proc_macro_with_span.rs + #![warn(clippy::missing_docs_in_private_items)] #![allow(dead_code)] #![feature(associated_type_defaults)] @@ -5,6 +7,9 @@ //! Some garbage docs for the crate here #![doc = "More garbage"] +extern crate proc_macro_with_span; +use proc_macro_with_span::with_span; + struct Foo { a: isize, b: isize, @@ -90,3 +95,13 @@ fn b(&self) {} } fn main() {} + +// don't lint proc macro output +with_span!(span + pub struct FooPm; + impl FooPm { + pub fn foo() {} + pub const fn bar() {} + pub const X: u32 = 0; + } +); diff --git a/tests/ui/missing-doc-impl.stderr b/tests/ui/missing_doc_impl.stderr similarity index 78% rename from tests/ui/missing-doc-impl.stderr rename to tests/ui/missing_doc_impl.stderr index bda63d66a17..f22fa19dbca 100644 --- a/tests/ui/missing-doc-impl.stderr +++ b/tests/ui/missing_doc_impl.stderr @@ -1,5 +1,5 @@ error: missing documentation for a struct - --> $DIR/missing-doc-impl.rs:8:1 + --> $DIR/missing_doc_impl.rs:13:1 | LL | / struct Foo { LL | | a: isize, @@ -10,19 +10,19 @@ LL | | } = note: `-D clippy::missing-docs-in-private-items` implied by `-D warnings` error: missing documentation for a struct field - --> $DIR/missing-doc-impl.rs:9:5 + --> $DIR/missing_doc_impl.rs:14:5 | LL | a: isize, | ^^^^^^^^ error: missing documentation for a struct field - --> $DIR/missing-doc-impl.rs:10:5 + --> $DIR/missing_doc_impl.rs:15:5 | LL | b: isize, | ^^^^^^^^ error: missing documentation for a struct - --> $DIR/missing-doc-impl.rs:13:1 + --> $DIR/missing_doc_impl.rs:18:1 | LL | / pub struct PubFoo { LL | | pub a: isize, @@ -31,19 +31,19 @@ LL | | } | |_^ error: missing documentation for a struct field - --> $DIR/missing-doc-impl.rs:14:5 + --> $DIR/missing_doc_impl.rs:19:5 | LL | pub a: isize, | ^^^^^^^^^^^^ error: missing documentation for a struct field - --> $DIR/missing-doc-impl.rs:15:5 + --> $DIR/missing_doc_impl.rs:20:5 | LL | b: isize, | ^^^^^^^^ error: missing documentation for a trait - --> $DIR/missing-doc-impl.rs:38:1 + --> $DIR/missing_doc_impl.rs:43:1 | LL | / pub trait C { LL | | fn foo(&self); @@ -52,31 +52,31 @@ LL | | } | |_^ error: missing documentation for an associated function - --> $DIR/missing-doc-impl.rs:39:5 + --> $DIR/missing_doc_impl.rs:44:5 | LL | fn foo(&self); | ^^^^^^^^^^^^^^ error: missing documentation for an associated function - --> $DIR/missing-doc-impl.rs:40:5 + --> $DIR/missing_doc_impl.rs:45:5 | LL | fn foo_with_impl(&self) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for an associated type - --> $DIR/missing-doc-impl.rs:50:5 + --> $DIR/missing_doc_impl.rs:55:5 | LL | type AssociatedType; | ^^^^^^^^^^^^^^^^^^^^ error: missing documentation for an associated type - --> $DIR/missing-doc-impl.rs:51:5 + --> $DIR/missing_doc_impl.rs:56:5 | LL | type AssociatedTypeDef = Self; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for an associated function - --> $DIR/missing-doc-impl.rs:62:5 + --> $DIR/missing_doc_impl.rs:67:5 | LL | / pub fn new() -> Self { LL | | Foo { a: 0, b: 0 } @@ -84,19 +84,19 @@ LL | | } | |_____^ error: missing documentation for an associated function - --> $DIR/missing-doc-impl.rs:65:5 + --> $DIR/missing_doc_impl.rs:70:5 | LL | fn bar() {} | ^^^^^^^^^^^ error: missing documentation for an associated function - --> $DIR/missing-doc-impl.rs:69:5 + --> $DIR/missing_doc_impl.rs:74:5 | LL | pub fn foo() {} | ^^^^^^^^^^^^^^^ error: missing documentation for an associated function - --> $DIR/missing-doc-impl.rs:73:5 + --> $DIR/missing_doc_impl.rs:78:5 | LL | / fn foo2() -> u32 { LL | | 1 From 670efd5720b31e235c8e97f49f612e685db97b93 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 13 Apr 2022 10:13:39 -0400 Subject: [PATCH 4/6] Don't lint `default_trait_access` in proc-macro expansions --- clippy_lints/src/default.rs | 5 ++++- clippy_utils/src/check_proc_macro.rs | 8 ++++---- tests/ui/default_trait_access.fixed | 6 ++++++ tests/ui/default_trait_access.rs | 6 ++++++ tests/ui/default_trait_access.stderr | 18 +++++++++--------- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/default.rs b/clippy_lints/src/default.rs index d99a1aa2969..74f7df61177 100644 --- a/clippy_lints/src/default.rs +++ b/clippy_lints/src/default.rs @@ -1,7 +1,9 @@ use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_sugg}; use clippy_utils::source::snippet_with_macro_callsite; use clippy_utils::ty::{has_drop, is_copy}; -use clippy_utils::{any_parent_is_automatically_derived, contains_name, get_parent_expr, match_def_path, paths}; +use clippy_utils::{ + any_parent_is_automatically_derived, contains_name, get_parent_expr, is_from_proc_macro, match_def_path, paths, +}; use if_chain::if_chain; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; @@ -94,6 +96,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if let QPath::Resolved(None, _path) = qpath; let expr_ty = cx.typeck_results().expr_ty(expr); if let ty::Adt(def, ..) = expr_ty.kind(); + if !is_from_proc_macro(cx, expr); then { // TODO: Work out a way to put "whatever the imported way of referencing // this type in this file" rather than a fully-qualified type. diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 4dd0ec6b6d0..f546a788c5d 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -16,7 +16,7 @@ use rustc_hir::{ Block, BlockCheckMode, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, Impl, ImplItem, ImplItemKind, IsAuto, Item, ItemKind, LoopSource, MatchSource, QPath, TraitItem, TraitItemKind, UnOp, UnsafeSource, Unsafety, - Variant, VariantData, VisibilityKind, YieldSource, + Variant, VariantData, YieldSource, }; use rustc_lint::{LateContext, LintContext}; use rustc_middle::ty::TyCtxt; @@ -196,7 +196,7 @@ fn item_search_pat(item: &Item<'_>) -> (Pat, Pat) { ItemKind::Impl(_) => (Pat::Str("impl"), Pat::Str("}")), _ => return (Pat::Str(""), Pat::Str("")), }; - if matches!(item.vis.node, VisibilityKind::Inherited) { + if item.vis_span.is_empty() { (start_pat, end_pat) } else { (Pat::Str("pub"), end_pat) @@ -217,7 +217,7 @@ fn impl_item_search_pat(item: &ImplItem<'_>) -> (Pat, Pat) { ImplItemKind::TyAlias(..) => (Pat::Str("type"), Pat::Str(";")), ImplItemKind::Fn(sig, ..) => (fn_header_search_pat(sig.header), Pat::Str("")), }; - if matches!(item.vis.node, VisibilityKind::Inherited) { + if item.vis_span.is_empty() { (start_pat, end_pat) } else { (Pat::Str("pub"), end_pat) @@ -225,7 +225,7 @@ fn impl_item_search_pat(item: &ImplItem<'_>) -> (Pat, Pat) { } fn field_def_search_pat(def: &FieldDef<'_>) -> (Pat, Pat) { - if matches!(def.vis.node, VisibilityKind::Inherited) { + if def.vis_span.is_empty() { if def.is_positional() { (Pat::Str(""), Pat::Str("")) } else { diff --git a/tests/ui/default_trait_access.fixed b/tests/ui/default_trait_access.fixed index 264dd4efaeb..fce66eb1759 100644 --- a/tests/ui/default_trait_access.fixed +++ b/tests/ui/default_trait_access.fixed @@ -1,8 +1,12 @@ // run-rustfix +// aux-build: proc_macro_with_span.rs #![allow(unused_imports, dead_code)] #![deny(clippy::default_trait_access)] +extern crate proc_macro_with_span; + +use proc_macro_with_span::with_span; use std::default; use std::default::Default as D2; use std::string; @@ -51,6 +55,8 @@ fn main() { ..Default::default() }; + let _s21: String = with_span!(s Default::default()); + println!( "[{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}]", s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15, s16, s17, s18, s19, s20, diff --git a/tests/ui/default_trait_access.rs b/tests/ui/default_trait_access.rs index a0930fab8e7..3e8e898b7bc 100644 --- a/tests/ui/default_trait_access.rs +++ b/tests/ui/default_trait_access.rs @@ -1,8 +1,12 @@ // run-rustfix +// aux-build: proc_macro_with_span.rs #![allow(unused_imports, dead_code)] #![deny(clippy::default_trait_access)] +extern crate proc_macro_with_span; + +use proc_macro_with_span::with_span; use std::default; use std::default::Default as D2; use std::string; @@ -51,6 +55,8 @@ fn main() { ..Default::default() }; + let _s21: String = with_span!(s Default::default()); + println!( "[{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}]", s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15, s16, s17, s18, s19, s20, diff --git a/tests/ui/default_trait_access.stderr b/tests/ui/default_trait_access.stderr index df8a5b94ddc..3493de37a55 100644 --- a/tests/ui/default_trait_access.stderr +++ b/tests/ui/default_trait_access.stderr @@ -1,53 +1,53 @@ error: calling `std::string::String::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:11:22 + --> $DIR/default_trait_access.rs:15:22 | LL | let s1: String = Default::default(); | ^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()` | note: the lint level is defined here - --> $DIR/default_trait_access.rs:4:9 + --> $DIR/default_trait_access.rs:5:9 | LL | #![deny(clippy::default_trait_access)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: calling `std::string::String::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:15:22 + --> $DIR/default_trait_access.rs:19:22 | LL | let s3: String = D2::default(); | ^^^^^^^^^^^^^ help: try: `std::string::String::default()` error: calling `std::string::String::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:17:22 + --> $DIR/default_trait_access.rs:21:22 | LL | let s4: String = std::default::Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()` error: calling `std::string::String::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:21:22 + --> $DIR/default_trait_access.rs:25:22 | LL | let s6: String = default::Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()` error: calling `GenericDerivedDefault::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:31:46 + --> $DIR/default_trait_access.rs:35:46 | LL | let s11: GenericDerivedDefault = Default::default(); | ^^^^^^^^^^^^^^^^^^ help: try: `GenericDerivedDefault::default()` error: calling `TupleDerivedDefault::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:37:36 + --> $DIR/default_trait_access.rs:41:36 | LL | let s14: TupleDerivedDefault = Default::default(); | ^^^^^^^^^^^^^^^^^^ help: try: `TupleDerivedDefault::default()` error: calling `ArrayDerivedDefault::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:39:36 + --> $DIR/default_trait_access.rs:43:36 | LL | let s15: ArrayDerivedDefault = Default::default(); | ^^^^^^^^^^^^^^^^^^ help: try: `ArrayDerivedDefault::default()` error: calling `TupleStructDerivedDefault::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:43:42 + --> $DIR/default_trait_access.rs:47:42 | LL | let s17: TupleStructDerivedDefault = Default::default(); | ^^^^^^^^^^^^^^^^^^ help: try: `TupleStructDerivedDefault::default()` From 8dda974a2753c0cf4d552e6054303376bc62c7bb Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sun, 24 Apr 2022 10:04:33 -0400 Subject: [PATCH 5/6] Add note to the docs on `is_from_proc_macro` --- clippy_utils/src/check_proc_macro.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index f546a788c5d..9f2c412eb85 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -271,7 +271,10 @@ fn span(&self) -> Span { impl_with_search_pat!(LateContext: FieldDef with field_def_search_pat); impl_with_search_pat!(LateContext: Variant with variant_search_pat); -/// Checks if the item likely came from a proc-macro +/// Checks if the item likely came from a proc-macro. +/// +/// This should be called after `in_external_macro` and the initial pattern matching of the ast as +/// it is significantly slower than both of those. pub fn is_from_proc_macro(cx: &T::Context, item: &T) -> bool { let (start_pat, end_pat) = item.search_pat(cx); !span_matches_pat(cx.sess(), item.span(), start_pat, end_pat) From 745b1942925c53156389f3ea731c360e507f71eb Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sun, 7 Aug 2022 21:44:32 -0400 Subject: [PATCH 6/6] Small cleanup for `check_proc_macro.rs` --- clippy_utils/src/check_proc_macro.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 9f2c412eb85..0f00a1cdf0f 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -24,11 +24,16 @@ use rustc_span::{Span, Symbol}; use rustc_target::spec::abi::Abi; +/// The search pattern to look for. Used by `span_matches_pat` #[derive(Clone, Copy)] pub enum Pat { + /// A single string. Str(&'static str), + /// Any of the given strings. MultiStr(&'static [&'static str]), + /// The string representation of the symbol. Sym(Symbol), + /// Any decimal or hexadecimal digit depending on the location. Num, } @@ -108,9 +113,9 @@ fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) { ExprKind::Box(e) => (Pat::Str("box"), expr_search_pat(tcx, e).1), ExprKind::ConstBlock(_) => (Pat::Str("const"), Pat::Str("}")), ExprKind::Tup([]) => (Pat::Str(")"), Pat::Str("(")), - ExprKind::Unary(UnOp::Deref, _) => (Pat::Str("*"), expr_search_pat(tcx, e).1), - ExprKind::Unary(UnOp::Not, _) => (Pat::Str("!"), expr_search_pat(tcx, e).1), - ExprKind::Unary(UnOp::Neg, _) => (Pat::Str("-"), expr_search_pat(tcx, e).1), + ExprKind::Unary(UnOp::Deref, e) => (Pat::Str("*"), expr_search_pat(tcx, e).1), + ExprKind::Unary(UnOp::Not, e) => (Pat::Str("!"), expr_search_pat(tcx, e).1), + ExprKind::Unary(UnOp::Neg, e) => (Pat::Str("-"), expr_search_pat(tcx, e).1), ExprKind::Lit(ref lit) => lit_search_pat(&lit.node), ExprKind::Array(_) | ExprKind::Repeat(..) => (Pat::Str("["), Pat::Str("]")), ExprKind::Call(e, []) | ExprKind::MethodCall(_, [e], _) => (expr_search_pat(tcx, e).0, Pat::Str("(")), @@ -154,6 +159,7 @@ fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) { ExprKind::Continue(Destination { label: None, .. }) => (Pat::Str("continue"), Pat::Str("continue")), ExprKind::Continue(Destination { label: Some(name), .. }) => (Pat::Str("continue"), Pat::Sym(name.ident.name)), ExprKind::Ret(None) => (Pat::Str("return"), Pat::Str("return")), + ExprKind::Ret(Some(e)) => (Pat::Str("return"), expr_search_pat(tcx, e).1), ExprKind::Struct(path, _, _) => (qpath_search_pat(path).0, Pat::Str("}")), ExprKind::Yield(e, YieldSource::Yield) => (Pat::Str("yield"), expr_search_pat(tcx, e).1), _ => (Pat::Str(""), Pat::Str("")),