Merge commit '30448e8cf9' into clippyup

This commit is contained in:
Philipp Krones 2023-06-02 11:41:57 +02:00
parent ec2c6155aa
commit e6dc0efc00
114 changed files with 2970 additions and 653 deletions

View file

@ -40,7 +40,7 @@ declare_clippy_lint! {
/// a.len()
/// }
/// ```
#[clippy::version = "1.69.0"]
#[clippy::version = "1.70.0"]
pub ALLOW_ATTRIBUTES,
restriction,
"`#[allow]` will not trigger if a warning isn't found. `#[expect]` triggers if there are no warnings."

View file

@ -88,7 +88,6 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool {
NonminimalBoolVisitor { cx }.visit_body(body);
}
}
struct NonminimalBoolVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
}
@ -473,6 +472,10 @@ impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> {
self.bool_expr(e);
},
ExprKind::Unary(UnOp::Not, inner) => {
if let ExprKind::Unary(UnOp::Not, ex) = inner.kind &&
!self.cx.typeck_results().node_types()[ex.hir_id].is_bool() {
return;
}
if self.cx.typeck_results().node_types()[inner.hir_id].is_bool() {
self.bool_expr(e);
}

View file

@ -17,6 +17,7 @@ mod fn_to_numeric_cast;
mod fn_to_numeric_cast_any;
mod fn_to_numeric_cast_with_truncation;
mod ptr_as_ptr;
mod ptr_cast_constness;
mod unnecessary_cast;
mod utils;
@ -363,7 +364,7 @@ declare_clippy_lint! {
/// namely `*const T` to `*const U` and `*mut T` to `*mut U`.
///
/// ### Why is this bad?
/// Though `as` casts between raw pointers is not terrible, `pointer::cast` is safer because
/// Though `as` casts between raw pointers are not terrible, `pointer::cast` is safer because
/// it cannot accidentally change the pointer's mutability nor cast the pointer to other types like `usize`.
///
/// ### Example
@ -386,6 +387,34 @@ declare_clippy_lint! {
"casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `as` casts between raw pointers which change its constness, namely `*const T` to
/// `*mut T` and `*mut T` to `*const T`.
///
/// ### Why is this bad?
/// Though `as` casts between raw pointers are not terrible, `pointer::cast_mut` and
/// `pointer::cast_const` are safer because they cannot accidentally cast the pointer to another
/// type.
///
/// ### Example
/// ```rust
/// let ptr: *const u32 = &42_u32;
/// let mut_ptr = ptr as *mut u32;
/// let ptr = mut_ptr as *const u32;
/// ```
/// Use instead:
/// ```rust
/// let ptr: *const u32 = &42_u32;
/// let mut_ptr = ptr.cast_mut();
/// let ptr = mut_ptr.cast_const();
/// ```
#[clippy::version = "1.71.0"]
pub PTR_CAST_CONSTNESS,
pedantic,
"casting using `as` from and to raw pointers to change constness when specialized methods apply"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for casts from an enum type to an integral type which will definitely truncate the
@ -652,6 +681,7 @@ impl_lint_pass!(Casts => [
FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
CHAR_LIT_AS_U8,
PTR_AS_PTR,
PTR_CAST_CONSTNESS,
CAST_ENUM_TRUNCATION,
CAST_ENUM_CONSTRUCTOR,
CAST_ABS_TO_UNSIGNED,
@ -685,6 +715,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
return;
}
cast_slice_from_raw_parts::check(cx, expr, cast_expr, cast_to, &self.msrv);
ptr_cast_constness::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv);
as_ptr_cast_mut::check(cx, expr, cast_expr, cast_to);
fn_to_numeric_cast_any::check(cx, expr, cast_expr, cast_from, cast_to);
fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to);

View file

@ -0,0 +1,44 @@
use clippy_utils::msrvs::POINTER_CAST_CONSTNESS;
use clippy_utils::sugg::Sugg;
use clippy_utils::{diagnostics::span_lint_and_sugg, msrvs::Msrv};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, Mutability};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty, TypeAndMut};
use super::PTR_CAST_CONSTNESS;
pub(super) fn check(
cx: &LateContext<'_>,
expr: &Expr<'_>,
cast_expr: &Expr<'_>,
cast_from: Ty<'_>,
cast_to: Ty<'_>,
msrv: &Msrv,
) {
if_chain! {
if msrv.meets(POINTER_CAST_CONSTNESS);
if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind();
if let ty::RawPtr(TypeAndMut { mutbl: to_mutbl, .. }) = cast_to.kind();
if matches!((from_mutbl, to_mutbl),
(Mutability::Not, Mutability::Mut) | (Mutability::Mut, Mutability::Not));
then {
let sugg = Sugg::hir(cx, cast_expr, "_");
let constness = match *to_mutbl {
Mutability::Not => "const",
Mutability::Mut => "mut",
};
span_lint_and_sugg(
cx,
PTR_CAST_CONSTNESS,
expr.span,
"`as` casting between raw pointers while changing its constness",
&format!("try `pointer::cast_{constness}`, a safer alternative"),
format!("{}.cast_{constness}()", sugg.maybe_par()),
Applicability::MachineApplicable,
);
}
}
}

View file

@ -37,7 +37,7 @@ declare_clippy_lint! {
/// println!("{sample}");
/// }
/// ```
#[clippy::version = "1.69.0"]
#[clippy::version = "1.70.0"]
pub COLLECTION_IS_NEVER_READ,
nursery,
"a collection is never queried"

View file

@ -89,6 +89,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::casts::FN_TO_NUMERIC_CAST_ANY_INFO,
crate::casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION_INFO,
crate::casts::PTR_AS_PTR_INFO,
crate::casts::PTR_CAST_CONSTNESS_INFO,
crate::casts::UNNECESSARY_CAST_INFO,
crate::checked_conversions::CHECKED_CONVERSIONS_INFO,
crate::cognitive_complexity::COGNITIVE_COMPLEXITY_INFO,
@ -427,6 +428,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::missing_const_for_fn::MISSING_CONST_FOR_FN_INFO,
crate::missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS_INFO,
crate::missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES_INFO,
crate::missing_fields_in_debug::MISSING_FIELDS_IN_DEBUG_INFO,
crate::missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS_INFO,
crate::missing_trait_methods::MISSING_TRAIT_METHODS_INFO,
crate::mixed_read_write_in_expression::DIVERGING_SUB_EXPRESSION_INFO,
@ -447,6 +449,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::needless_bool::NEEDLESS_BOOL_ASSIGN_INFO,
crate::needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE_INFO,
crate::needless_continue::NEEDLESS_CONTINUE_INFO,
crate::needless_else::NEEDLESS_ELSE_INFO,
crate::needless_for_each::NEEDLESS_FOR_EACH_INFO,
crate::needless_late_init::NEEDLESS_LATE_INIT_INFO,
crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO,

View file

@ -1,4 +1,4 @@
use clippy_utils::{diagnostics::span_lint_and_sugg, match_def_path, paths};
use clippy_utils::{diagnostics::span_lint_and_sugg, is_ty_alias, match_def_path, paths};
use hir::{def::Res, ExprKind};
use rustc_errors::Applicability;
use rustc_hir as hir;
@ -43,12 +43,23 @@ declare_clippy_lint! {
}
declare_lint_pass!(DefaultConstructedUnitStructs => [DEFAULT_CONSTRUCTED_UNIT_STRUCTS]);
fn is_alias(ty: hir::Ty<'_>) -> bool {
if let hir::TyKind::Path(ref qpath) = ty.kind {
is_ty_alias(qpath)
} else {
false
}
}
impl LateLintPass<'_> for DefaultConstructedUnitStructs {
fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
if_chain!(
// make sure we have a call to `Default::default`
if let hir::ExprKind::Call(fn_expr, &[]) = expr.kind;
if let ExprKind::Path(ref qpath@ hir::QPath::TypeRelative(_,_)) = fn_expr.kind;
if let ExprKind::Path(ref qpath @ hir::QPath::TypeRelative(base, _)) = fn_expr.kind;
// make sure this isn't a type alias:
// `<Foo as Bar>::Assoc` cannot be used as a constructor
if !is_alias(*base);
if let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id);
if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD);
// make sure we have a struct with no fields (unit struct)

View file

@ -38,7 +38,7 @@ declare_clippy_lint! {
/// wait(fut).await;
/// }
/// ```
#[clippy::version = "1.68.0"]
#[clippy::version = "1.70.0"]
pub LARGE_FUTURES,
pedantic,
"large future may lead to unexpected stack overflows"

View file

@ -38,7 +38,7 @@ impl_lint_pass!(LargeStackArrays => [LARGE_STACK_ARRAYS]);
impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
if let ExprKind::Repeat(_, _) = expr.kind
if let ExprKind::Repeat(_, _) | ExprKind::Array(_) = expr.kind
&& let ty::Array(element_type, cst) = cx.typeck_results().expr_ty(expr).kind()
&& let ConstKind::Value(ty::ValTree::Leaf(element_count)) = cst.kind()
&& let Ok(element_count) = element_count.try_to_target_usize(cx.tcx)

View file

@ -17,7 +17,7 @@ declare_clippy_lint! {
/// ```rust,ignore
/// let my_number = 1;
/// ```
#[clippy::version = "1.69.0"]
#[clippy::version = "1.70.0"]
pub LET_WITH_TYPE_UNDERSCORE,
complexity,
"unneeded underscore type (`_`) in a variable declaration"

View file

@ -202,6 +202,7 @@ mod missing_assert_message;
mod missing_const_for_fn;
mod missing_doc;
mod missing_enforced_import_rename;
mod missing_fields_in_debug;
mod missing_inline;
mod missing_trait_methods;
mod mixed_read_write_in_expression;
@ -217,6 +218,7 @@ mod needless_arbitrary_self_type;
mod needless_bool;
mod needless_borrowed_ref;
mod needless_continue;
mod needless_else;
mod needless_for_each;
mod needless_late_init;
mod needless_parens_on_range_literals;
@ -333,7 +335,7 @@ mod zero_sized_map_values;
pub use crate::utils::conf::{lookup_conf_file, Conf};
use crate::utils::{
conf::{format_error, metadata::get_configuration_metadata, TryConf},
conf::{metadata::get_configuration_metadata, TryConf},
FindAll,
};
@ -369,23 +371,36 @@ pub fn read_conf(sess: &Session, path: &io::Result<(Option<PathBuf>, Vec<String>
},
};
let TryConf { conf, errors, warnings } = utils::conf::read(file_name);
let TryConf { conf, errors, warnings } = utils::conf::read(sess, file_name);
// all conf errors are non-fatal, we just use the default conf in case of error
for error in errors {
sess.err(format!(
"error reading Clippy's configuration file `{}`: {}",
file_name.display(),
format_error(error)
));
if let Some(span) = error.span {
sess.span_err(
span,
format!("error reading Clippy's configuration file: {}", error.message),
);
} else {
sess.err(format!(
"error reading Clippy's configuration file `{}`: {}",
file_name.display(),
error.message
));
}
}
for warning in warnings {
sess.struct_warn(format!(
"error reading Clippy's configuration file `{}`: {}",
file_name.display(),
format_error(warning)
))
.emit();
if let Some(span) = warning.span {
sess.span_warn(
span,
format!("error reading Clippy's configuration file: {}", warning.message),
);
} else {
sess.warn(format!(
"error reading Clippy's configuration file `{}`: {}",
file_name.display(),
warning.message
));
}
}
conf
@ -990,6 +1005,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(items_after_test_module::ItemsAfterTestModule));
store.register_early_pass(|| Box::new(ref_patterns::RefPatterns));
store.register_late_pass(|_| Box::new(default_constructed_unit_structs::DefaultConstructedUnitStructs));
store.register_early_pass(|| Box::new(needless_else::NeedlessElse));
store.register_late_pass(|_| Box::new(missing_fields_in_debug::MissingFieldsInDebug));
// add lints here, do not remove this comment, it's used in `new_lint`
}

View file

@ -77,53 +77,54 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
local.els.is_none() &&
local.ty.is_none() &&
init.span.ctxt() == stmt.span.ctxt() &&
let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init) {
match if_let_or_match {
IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => if_chain! {
if expr_is_simple_identity(let_pat, if_then);
if let Some(if_else) = if_else;
if expr_diverges(cx, if_else);
then {
emit_manual_let_else(cx, stmt.span, if_let_expr, local.pat, let_pat, if_else);
}
},
IfLetOrMatch::Match(match_expr, arms, source) => {
if self.matches_behaviour == MatchLintBehaviour::Never {
return;
}
if source != MatchSource::Normal {
return;
}
// Any other number than two arms doesn't (necessarily)
// have a trivial mapping to let else.
if arms.len() != 2 {
return;
}
// Guards don't give us an easy mapping either
if arms.iter().any(|arm| arm.guard.is_some()) {
return;
}
let check_types = self.matches_behaviour == MatchLintBehaviour::WellKnownTypes;
let diverging_arm_opt = arms
.iter()
.enumerate()
.find(|(_, arm)| expr_diverges(cx, arm.body) && pat_allowed_for_else(cx, arm.pat, check_types));
let Some((idx, diverging_arm)) = diverging_arm_opt else { return; };
// If the non-diverging arm is the first one, its pattern can be reused in a let/else statement.
// However, if it arrives in second position, its pattern may cover some cases already covered
// by the diverging one.
// TODO: accept the non-diverging arm as a second position if patterns are disjointed.
if idx == 0 {
return;
}
let pat_arm = &arms[1 - idx];
if !expr_is_simple_identity(pat_arm.pat, pat_arm.body) {
return;
}
let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init)
{
match if_let_or_match {
IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => if_chain! {
if expr_is_simple_identity(let_pat, if_then);
if let Some(if_else) = if_else;
if expr_diverges(cx, if_else);
then {
emit_manual_let_else(cx, stmt.span, if_let_expr, local.pat, let_pat, if_else);
}
},
IfLetOrMatch::Match(match_expr, arms, source) => {
if self.matches_behaviour == MatchLintBehaviour::Never {
return;
}
if source != MatchSource::Normal {
return;
}
// Any other number than two arms doesn't (necessarily)
// have a trivial mapping to let else.
if arms.len() != 2 {
return;
}
// Guards don't give us an easy mapping either
if arms.iter().any(|arm| arm.guard.is_some()) {
return;
}
let check_types = self.matches_behaviour == MatchLintBehaviour::WellKnownTypes;
let diverging_arm_opt = arms
.iter()
.enumerate()
.find(|(_, arm)| expr_diverges(cx, arm.body) && pat_allowed_for_else(cx, arm.pat, check_types));
let Some((idx, diverging_arm)) = diverging_arm_opt else { return; };
// If the non-diverging arm is the first one, its pattern can be reused in a let/else statement.
// However, if it arrives in second position, its pattern may cover some cases already covered
// by the diverging one.
// TODO: accept the non-diverging arm as a second position if patterns are disjointed.
if idx == 0 {
return;
}
let pat_arm = &arms[1 - idx];
if !expr_is_simple_identity(pat_arm.pat, pat_arm.body) {
return;
}
emit_manual_let_else(cx, stmt.span, match_expr, local.pat, pat_arm.pat, diverging_arm.body);
},
}
emit_manual_let_else(cx, stmt.span, match_expr, local.pat, pat_arm.pat, diverging_arm.body);
},
}
};
}
@ -145,10 +146,9 @@ fn emit_manual_let_else(
"this could be rewritten as `let...else`",
|diag| {
// This is far from perfect, for example there needs to be:
// * mut additions for the bindings
// * renamings of the bindings for `PatKind::Or`
// * tracking for multi-binding cases: let (foo, bar) = if let (Some(foo), Ok(bar)) = ...
// * renamings of the bindings for many `PatKind`s like structs, slices, etc.
// * unused binding collision detection with existing ones
// * putting patterns with at the top level | inside ()
// for this to be machine applicable.
let mut app = Applicability::HasPlaceholders;
let (sn_expr, _) = snippet_with_context(cx, expr.span, span.ctxt(), "", &mut app);
@ -159,28 +159,62 @@ fn emit_manual_let_else(
} else {
format!("{{ {sn_else} }}")
};
let sn_bl = match pat.kind {
PatKind::Or(..) => {
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app);
format!("({sn_pat})")
},
// Replace the variable name iff `TupleStruct` has one argument like `Variant(v)`.
PatKind::TupleStruct(ref w, args, ..) if args.len() == 1 => {
let sn_wrapper = cx.sess().source_map().span_to_snippet(w.span()).unwrap_or_default();
let (sn_inner, _) = snippet_with_context(cx, local.span, span.ctxt(), "", &mut app);
format!("{sn_wrapper}({sn_inner})")
},
_ => {
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app);
sn_pat.into_owned()
},
};
let sn_bl = replace_in_pattern(cx, span, local, pat, &mut app);
let sugg = format!("let {sn_bl} = {sn_expr} else {else_bl};");
diag.span_suggestion(span, "consider writing", sugg, app);
},
);
}
// replaces the locals in the pattern
fn replace_in_pattern(
cx: &LateContext<'_>,
span: Span,
local: &Pat<'_>,
pat: &Pat<'_>,
app: &mut Applicability,
) -> String {
let mut bindings_count = 0;
pat.each_binding_or_first(&mut |_, _, _, _| bindings_count += 1);
// If the pattern creates multiple bindings, exit early,
// as otherwise we might paste the pattern to the positions of multiple bindings.
if bindings_count > 1 {
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", app);
return sn_pat.into_owned();
}
match pat.kind {
PatKind::Binding(..) => {
let (sn_bdg, _) = snippet_with_context(cx, local.span, span.ctxt(), "", app);
return sn_bdg.to_string();
},
PatKind::Or(pats) => {
let patterns = pats
.iter()
.map(|pat| replace_in_pattern(cx, span, local, pat, app))
.collect::<Vec<_>>();
let or_pat = patterns.join(" | ");
return format!("({or_pat})");
},
// Replace the variable name iff `TupleStruct` has one argument like `Variant(v)`.
PatKind::TupleStruct(ref w, args, dot_dot_pos) => {
let mut args = args
.iter()
.map(|pat| replace_in_pattern(cx, span, local, pat, app))
.collect::<Vec<_>>();
if let Some(pos) = dot_dot_pos.as_opt_usize() {
args.insert(pos, "..".to_owned());
}
let args = args.join(", ");
let sn_wrapper = cx.sess().source_map().span_to_snippet(w.span()).unwrap_or_default();
return format!("{sn_wrapper}({args})");
},
_ => {},
}
let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", app);
sn_pat.into_owned()
}
/// Check whether an expression is divergent. May give false negatives.
fn expr_diverges(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
struct V<'cx, 'tcx> {

View file

@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::macros::{is_panic, root_macro_call};
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::is_local_used;
use clippy_utils::{is_wild, peel_blocks_with_stmt};
use clippy_utils::{in_constant, is_wild, peel_blocks_with_stmt};
use rustc_hir::{Arm, Expr, PatKind};
use rustc_lint::LateContext;
use rustc_span::symbol::{kw, sym};
@ -10,6 +10,11 @@ use rustc_span::symbol::{kw, sym};
use super::MATCH_WILD_ERR_ARM;
pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm<'tcx>]) {
// `unwrap`/`expect` is not (yet) const, so we want to allow this in const contexts for now
if in_constant(cx, ex.hir_id) {
return;
}
let ex_ty = cx.typeck_results().expr_ty(ex).peel_refs();
if is_type_diagnostic_item(cx, ex_ty, sym::Result) {
for arm in arms {

View file

@ -25,7 +25,7 @@ mod wild_in_or_pats;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::{snippet_opt, walk_span_to_context};
use clippy_utils::{higher, in_constant, is_span_match, tokenize_with_text};
use clippy_utils::{higher, in_constant, is_direct_expn_of, is_span_match, tokenize_with_text};
use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat};
use rustc_lexer::TokenKind;
use rustc_lint::{LateContext, LateLintPass, LintContext};
@ -974,12 +974,16 @@ impl_lint_pass!(Matches => [
impl<'tcx> LateLintPass<'tcx> for Matches {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if in_external_macro(cx.sess(), expr.span) {
if is_direct_expn_of(expr.span, "matches").is_none() && in_external_macro(cx.sess(), expr.span) {
return;
}
let from_expansion = expr.span.from_expansion();
if let ExprKind::Match(ex, arms, source) = expr.kind {
if is_direct_expn_of(expr.span, "matches").is_some() {
redundant_pattern_match::check_match(cx, expr, ex, arms);
}
if source == MatchSource::Normal && !is_span_match(cx, expr.span) {
return;
}

View file

@ -1,10 +1,10 @@
use super::REDUNDANT_PATTERN_MATCHING;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::{snippet, walk_span_to_context};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{is_type_diagnostic_item, needs_ordered_drop};
use clippy_utils::visitors::any_temporaries_need_ordered_drop;
use clippy_utils::{higher, is_trait_method};
use clippy_utils::{higher, is_expn_of, is_trait_method};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
@ -190,24 +190,19 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op
let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
if let Some(good_method) = found_good_method(cx, arms, node_pair) {
let span = expr.span.to(op.span);
let span = is_expn_of(expr.span, "matches").unwrap_or(expr.span.to(op.span));
let result_expr = match &op.kind {
ExprKind::AddrOf(_, _, borrowed) => borrowed,
_ => op,
};
span_lint_and_then(
span_lint_and_sugg(
cx,
REDUNDANT_PATTERN_MATCHING,
expr.span,
span,
&format!("redundant pattern matching, consider using `{good_method}`"),
|diag| {
diag.span_suggestion(
span,
"try this",
format!("{}.{good_method}", snippet(cx, result_expr.span, "_")),
Applicability::MaybeIncorrect, // snippet
);
},
"try this",
format!("{}.{good_method}", snippet(cx, result_expr.span, "_")),
Applicability::MachineApplicable,
);
}
}

View file

@ -3191,7 +3191,7 @@ declare_clippy_lint! {
/// let mut v = vec![1, 2, 3];
/// v.clear();
/// ```
#[clippy::version = "1.69.0"]
#[clippy::version = "1.70.0"]
pub CLEAR_WITH_DRAIN,
nursery,
"calling `drain` in order to `clear` a container"

View file

@ -36,7 +36,7 @@ declare_clippy_lint! {
/// assert!(service.ready, "`service.poll_ready()` must be called first to ensure that service is ready to receive requests");
/// }
/// ```
#[clippy::version = "1.69.0"]
#[clippy::version = "1.70.0"]
pub MISSING_ASSERT_MESSAGE,
restriction,
"checks assertions without a custom panic message"

View file

@ -0,0 +1,234 @@
use std::ops::ControlFlow;
use clippy_utils::{
diagnostics::span_lint_and_then,
is_path_lang_item, paths,
ty::match_type,
visitors::{for_each_expr, Visitable},
};
use rustc_ast::LitKind;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::Block;
use rustc_hir::{
def::{DefKind, Res},
Expr, ImplItemKind, LangItem, Node,
};
use rustc_hir::{ExprKind, Impl, ItemKind, QPath, TyKind};
use rustc_hir::{ImplItem, Item, VariantData};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::Ty;
use rustc_middle::ty::TypeckResults;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{sym, Span, Symbol};
declare_clippy_lint! {
/// ### What it does
/// Checks for manual [`core::fmt::Debug`](https://doc.rust-lang.org/core/fmt/trait.Debug.html) implementations that do not use all fields.
///
/// ### Why is this bad?
/// A common mistake is to forget to update manual `Debug` implementations when adding a new field
/// to a struct or a new variant to an enum.
///
/// At the same time, it also acts as a style lint to suggest using [`core::fmt::DebugStruct::finish_non_exhaustive`](https://doc.rust-lang.org/core/fmt/struct.DebugStruct.html#method.finish_non_exhaustive)
/// for the times when the user intentionally wants to leave out certain fields (e.g. to hide implementation details).
///
/// ### Known problems
/// This lint works based on the `DebugStruct` helper types provided by the `Formatter`,
/// so this won't detect `Debug` impls that use the `write!` macro.
/// Oftentimes there is more logic to a `Debug` impl if it uses `write!` macro, so it tries
/// to be on the conservative side and not lint in those cases in an attempt to prevent false positives.
///
/// This lint also does not look through function calls, so calling a function does not consider fields
/// used inside of that function as used by the `Debug` impl.
///
/// Lastly, it also ignores tuple structs as their `DebugTuple` formatter does not have a `finish_non_exhaustive`
/// method, as well as enums because their exhaustiveness is already checked by the compiler when matching on the enum,
/// making it much less likely to accidentally forget to update the `Debug` impl when adding a new variant.
///
/// ### Example
/// ```rust
/// use std::fmt;
/// struct Foo {
/// data: String,
/// // implementation detail
/// hidden_data: i32
/// }
/// impl fmt::Debug for Foo {
/// fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
/// formatter
/// .debug_struct("Foo")
/// .field("data", &self.data)
/// .finish()
/// }
/// }
/// ```
/// Use instead:
/// ```rust
/// use std::fmt;
/// struct Foo {
/// data: String,
/// // implementation detail
/// hidden_data: i32
/// }
/// impl fmt::Debug for Foo {
/// fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
/// formatter
/// .debug_struct("Foo")
/// .field("data", &self.data)
/// .finish_non_exhaustive()
/// }
/// }
/// ```
#[clippy::version = "1.70.0"]
pub MISSING_FIELDS_IN_DEBUG,
pedantic,
"missing fields in manual `Debug` implementation"
}
declare_lint_pass!(MissingFieldsInDebug => [MISSING_FIELDS_IN_DEBUG]);
fn report_lints(cx: &LateContext<'_>, span: Span, span_notes: Vec<(Span, &'static str)>) {
span_lint_and_then(
cx,
MISSING_FIELDS_IN_DEBUG,
span,
"manual `Debug` impl does not include all fields",
|diag| {
for (span, note) in span_notes {
diag.span_note(span, note);
}
diag.help("consider including all fields in this `Debug` impl")
.help("consider calling `.finish_non_exhaustive()` if you intend to ignore fields");
},
);
}
/// Checks if we should lint in a block of code
///
/// The way we check for this condition is by checking if there is
/// a call to `Formatter::debug_struct` but no call to `.finish_non_exhaustive()`.
fn should_lint<'tcx>(
cx: &LateContext<'tcx>,
typeck_results: &TypeckResults<'tcx>,
block: impl Visitable<'tcx>,
) -> bool {
// Is there a call to `DebugStruct::finish_non_exhaustive`? Don't lint if there is.
let mut has_finish_non_exhaustive = false;
// Is there a call to `DebugStruct::debug_struct`? Do lint if there is.
let mut has_debug_struct = false;
for_each_expr(block, |expr| {
if let ExprKind::MethodCall(path, recv, ..) = &expr.kind {
let recv_ty = typeck_results.expr_ty(recv).peel_refs();
if path.ident.name == sym::debug_struct && match_type(cx, recv_ty, &paths::FORMATTER) {
has_debug_struct = true;
} else if path.ident.name == sym!(finish_non_exhaustive) && match_type(cx, recv_ty, &paths::DEBUG_STRUCT) {
has_finish_non_exhaustive = true;
}
}
ControlFlow::<!, _>::Continue(())
});
!has_finish_non_exhaustive && has_debug_struct
}
/// Checks if the given expression is a call to `DebugStruct::field`
/// and the first argument to it is a string literal and if so, returns it
///
/// Example: `.field("foo", ....)` returns `Some("foo")`
fn as_field_call<'tcx>(
cx: &LateContext<'tcx>,
typeck_results: &TypeckResults<'tcx>,
expr: &Expr<'_>,
) -> Option<Symbol> {
if let ExprKind::MethodCall(path, recv, [debug_field, _], _) = &expr.kind
&& let recv_ty = typeck_results.expr_ty(recv).peel_refs()
&& match_type(cx, recv_ty, &paths::DEBUG_STRUCT)
&& path.ident.name == sym::field
&& let ExprKind::Lit(lit) = &debug_field.kind
&& let LitKind::Str(sym, ..) = lit.node
{
Some(sym)
} else {
None
}
}
/// Attempts to find unused fields assuming that the item is a struct
fn check_struct<'tcx>(
cx: &LateContext<'tcx>,
typeck_results: &TypeckResults<'tcx>,
block: &'tcx Block<'tcx>,
self_ty: Ty<'tcx>,
item: &'tcx Item<'tcx>,
data: &VariantData<'_>,
) {
// Is there a "direct" field access anywhere (i.e. self.foo)?
// We don't want to lint if there is not, because the user might have
// a newtype struct and use fields from the wrapped type only.
let mut has_direct_field_access = false;
let mut field_accesses = FxHashSet::default();
for_each_expr(block, |expr| {
if let ExprKind::Field(target, ident) = expr.kind
&& let target_ty = typeck_results.expr_ty_adjusted(target).peel_refs()
&& target_ty == self_ty
{
field_accesses.insert(ident.name);
has_direct_field_access = true;
} else if let Some(sym) = as_field_call(cx, typeck_results, expr) {
field_accesses.insert(sym);
}
ControlFlow::<!, _>::Continue(())
});
let span_notes = data
.fields()
.iter()
.filter_map(|field| {
if field_accesses.contains(&field.ident.name) || is_path_lang_item(cx, field.ty, LangItem::PhantomData) {
None
} else {
Some((field.span, "this field is unused"))
}
})
.collect::<Vec<_>>();
// only lint if there's also at least one direct field access to allow patterns
// where one might have a newtype struct and uses fields from the wrapped type
if !span_notes.is_empty() && has_direct_field_access {
report_lints(cx, item.span, span_notes);
}
}
impl<'tcx> LateLintPass<'tcx> for MissingFieldsInDebug {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx rustc_hir::Item<'tcx>) {
// is this an `impl Debug for X` block?
if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), self_ty, items, .. }) = item.kind
&& let Res::Def(DefKind::Trait, trait_def_id) = trait_ref.path.res
&& let TyKind::Path(QPath::Resolved(_, self_path)) = &self_ty.kind
&& cx.match_def_path(trait_def_id, &[sym::core, sym::fmt, sym::Debug])
// don't trigger if this impl was derived
&& !cx.tcx.has_attr(item.owner_id, sym::automatically_derived)
&& !item.span.from_expansion()
// find `Debug::fmt` function
&& let Some(fmt_item) = items.iter().find(|i| i.ident.name == sym::fmt)
&& let ImplItem { kind: ImplItemKind::Fn(_, body_id), .. } = cx.tcx.hir().impl_item(fmt_item.id)
&& let body = cx.tcx.hir().body(*body_id)
&& let ExprKind::Block(block, _) = body.value.kind
// inspect `self`
&& let self_ty = cx.tcx.type_of(self_path.res.def_id()).skip_binder().peel_refs()
&& let Some(self_adt) = self_ty.ty_adt_def()
&& let Some(self_def_id) = self_adt.did().as_local()
&& let Some(Node::Item(self_item)) = cx.tcx.hir().find_by_def_id(self_def_id)
// NB: can't call cx.typeck_results() as we are not in a body
&& let typeck_results = cx.tcx.typeck_body(*body_id)
&& should_lint(cx, typeck_results, block)
{
// we intentionally only lint structs, see lint description
if let ItemKind::Struct(data, _) = &self_item.kind {
check_struct(cx, typeck_results, block, self_ty, item, data);
}
}
}
}

View file

@ -0,0 +1,61 @@
use clippy_utils::source::snippet_opt;
use clippy_utils::{diagnostics::span_lint_and_sugg, source::trim_span};
use rustc_ast::ast::{Expr, ExprKind};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! {
/// ### What it does
/// Checks for empty `else` branches.
///
/// ### Why is this bad?
/// An empty else branch does nothing and can be removed.
///
/// ### Example
/// ```rust
///# fn check() -> bool { true }
/// if check() {
/// println!("Check successful!");
/// } else {
/// }
/// ```
/// Use instead:
/// ```rust
///# fn check() -> bool { true }
/// if check() {
/// println!("Check successful!");
/// }
/// ```
#[clippy::version = "1.71.0"]
pub NEEDLESS_ELSE,
style,
"empty else branch"
}
declare_lint_pass!(NeedlessElse => [NEEDLESS_ELSE]);
impl EarlyLintPass for NeedlessElse {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
if let ExprKind::If(_, then_block, Some(else_clause)) = &expr.kind
&& let ExprKind::Block(block, _) = &else_clause.kind
&& !expr.span.from_expansion()
&& !else_clause.span.from_expansion()
&& block.stmts.is_empty()
&& let Some(trimmed) = expr.span.trim_start(then_block.span)
&& let span = trim_span(cx.sess().source_map(), trimmed)
&& let Some(else_snippet) = snippet_opt(cx, span)
// Ignore else blocks that contain comments or #[cfg]s
&& !else_snippet.contains(['/', '#'])
{
span_lint_and_sugg(
cx,
NEEDLESS_ELSE,
span,
"this else branch is empty",
"you can remove it",
String::new(),
Applicability::MachineApplicable,
);
}
}
}

View file

@ -241,7 +241,7 @@ impl<'de> Deserialize<'de> for MacroMatcher {
V: de::MapAccess<'de>,
{
let mut name = None;
let mut brace: Option<&str> = None;
let mut brace: Option<String> = None;
while let Some(key) = map.next_key()? {
match key {
Field::Name => {

View file

@ -35,7 +35,7 @@ declare_clippy_lint! {
/// };
/// let fut = f;
/// ```
#[clippy::version = "1.69.0"]
#[clippy::version = "1.70.0"]
pub REDUNDANT_ASYNC_BLOCK,
complexity,
"`async { future.await }` can be replaced by `future`"

View file

@ -177,7 +177,7 @@ fn check_set<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, utf8: bool) {
}
fn check_regex<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, utf8: bool) {
let mut parser = regex_syntax::ParserBuilder::new().unicode(true).utf8(!utf8).build();
let mut parser = regex_syntax::ParserBuilder::new().unicode(true).utf8(utf8).build();
if let ExprKind::Lit(lit) = expr.kind {
if let LitKind::Str(ref r, style) = lit.node {

View file

@ -70,7 +70,7 @@ declare_clippy_lint! {
"using a return statement like `return expr;` where an expression would suffice"
}
#[derive(PartialEq, Eq, Clone)]
#[derive(PartialEq, Eq)]
enum RetReplacement<'tcx> {
Empty,
Block,
@ -80,7 +80,7 @@ enum RetReplacement<'tcx> {
}
impl<'tcx> RetReplacement<'tcx> {
fn sugg_help(self) -> &'static str {
fn sugg_help(&self) -> &'static str {
match self {
Self::Empty | Self::Expr(..) => "remove `return`",
Self::Block => "replace `return` with an empty block",
@ -88,10 +88,11 @@ impl<'tcx> RetReplacement<'tcx> {
Self::IfSequence(..) => "remove `return` and wrap the sequence with parentheses",
}
}
fn applicability(&self) -> Option<Applicability> {
fn applicability(&self) -> Applicability {
match self {
Self::Expr(_, ap) | Self::IfSequence(_, ap) => Some(*ap),
_ => None,
Self::Expr(_, ap) | Self::IfSequence(_, ap) => *ap,
_ => Applicability::MachineApplicable,
}
}
}
@ -271,7 +272,7 @@ fn check_final_expr<'tcx>(
return;
}
emit_return_lint(cx, ret_span, semi_spans, replacement);
emit_return_lint(cx, ret_span, semi_spans, &replacement);
},
ExprKind::If(_, then, else_clause_opt) => {
check_block_return(cx, &then.kind, peeled_drop_expr.span, semi_spans.clone());
@ -306,20 +307,17 @@ fn expr_contains_conjunctive_ifs<'tcx>(expr: &'tcx Expr<'tcx>) -> bool {
contains_if(expr, false)
}
fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, semi_spans: Vec<Span>, replacement: RetReplacement<'_>) {
fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, semi_spans: Vec<Span>, replacement: &RetReplacement<'_>) {
if ret_span.from_expansion() {
return;
}
let applicability = replacement.applicability().unwrap_or(Applicability::MachineApplicable);
let return_replacement = replacement.to_string();
let sugg_help = replacement.sugg_help();
span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| {
diag.span_suggestion_hidden(ret_span, sugg_help, return_replacement, applicability);
// for each parent statement, we need to remove the semicolon
for semi_stmt_span in semi_spans {
diag.tool_only_span_suggestion(semi_stmt_span, "remove this semicolon", "", applicability);
}
let suggestions = std::iter::once((ret_span, replacement.to_string()))
.chain(semi_spans.into_iter().map(|span| (span, String::new())))
.collect();
diag.multipart_suggestion_verbose(replacement.sugg_help(), suggestions, replacement.applicability());
});
}

View file

@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sug
use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::ty::is_type_lang_item;
use clippy_utils::{get_expr_use_or_unification_node, peel_blocks, SpanlessEq};
use clippy_utils::{get_parent_expr, is_lint_allowed, match_function_call, method_calls, paths};
use clippy_utils::{get_parent_expr, is_lint_allowed, is_path_diagnostic_item, method_calls};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::def_id::DefId;
@ -255,7 +255,8 @@ impl<'tcx> LateLintPass<'tcx> for StringLitAsBytes {
if_chain! {
// Find std::str::converts::from_utf8
if let Some(args) = match_function_call(cx, e, &paths::STR_FROM_UTF8);
if let ExprKind::Call(fun, args) = e.kind;
if is_path_diagnostic_item(cx, fun, sym::str_from_utf8);
// Find string::as_bytes
if let ExprKind::AddrOf(BorrowKind::Ref, _, args) = args[0].kind;

View file

@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_help;
use rustc_hir::intravisit::{walk_expr, walk_fn, FnKind, Visitor};
use clippy_utils::diagnostics::span_lint_and_then;
use rustc_hir::intravisit::{walk_body, walk_expr, walk_fn, FnKind, Visitor};
use rustc_hir::{Body, Expr, ExprKind, FnDecl, YieldSource};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::nested_filter;
@ -42,6 +42,10 @@ declare_lint_pass!(UnusedAsync => [UNUSED_ASYNC]);
struct AsyncFnVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
found_await: bool,
/// Also keep track of `await`s in nested async blocks so we can mention
/// it in a note
await_in_async_block: Option<Span>,
async_depth: usize,
}
impl<'a, 'tcx> Visitor<'tcx> for AsyncFnVisitor<'a, 'tcx> {
@ -49,7 +53,11 @@ impl<'a, 'tcx> Visitor<'tcx> for AsyncFnVisitor<'a, 'tcx> {
fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
if let ExprKind::Yield(_, YieldSource::Await { .. }) = ex.kind {
self.found_await = true;
if self.async_depth == 1 {
self.found_await = true;
} else if self.await_in_async_block.is_none() {
self.await_in_async_block = Some(ex.span);
}
}
walk_expr(self, ex);
}
@ -57,6 +65,20 @@ impl<'a, 'tcx> Visitor<'tcx> for AsyncFnVisitor<'a, 'tcx> {
fn nested_visit_map(&mut self) -> Self::Map {
self.cx.tcx.hir()
}
fn visit_body(&mut self, b: &'tcx Body<'tcx>) {
let is_async_block = matches!(b.generator_kind, Some(rustc_hir::GeneratorKind::Async(_)));
if is_async_block {
self.async_depth += 1;
}
walk_body(self, b);
if is_async_block {
self.async_depth -= 1;
}
}
}
impl<'tcx> LateLintPass<'tcx> for UnusedAsync {
@ -70,16 +92,30 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync {
def_id: LocalDefId,
) {
if !span.from_expansion() && fn_kind.asyncness().is_async() {
let mut visitor = AsyncFnVisitor { cx, found_await: false };
let mut visitor = AsyncFnVisitor {
cx,
found_await: false,
async_depth: 0,
await_in_async_block: None,
};
walk_fn(&mut visitor, fn_kind, fn_decl, body.id(), def_id);
if !visitor.found_await {
span_lint_and_help(
span_lint_and_then(
cx,
UNUSED_ASYNC,
span,
"unused `async` for function with no await statements",
None,
"consider removing the `async` from this function",
|diag| {
diag.help("consider removing the `async` from this function");
if let Some(span) = visitor.await_in_async_block {
diag.span_note(
span,
"`await` used in an async block, which does not require \
the enclosing function to be `async`",
);
}
},
);
}
}

View file

@ -2,12 +2,15 @@
#![allow(clippy::module_name_repetitions)]
use rustc_session::Session;
use rustc_span::{BytePos, Pos, SourceFile, Span, SyntaxContext};
use serde::de::{Deserializer, IgnoredAny, IntoDeserializer, MapAccess, Visitor};
use serde::Deserialize;
use std::error::Error;
use std::fmt::{Debug, Display, Formatter};
use std::ops::Range;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::{cmp, env, fmt, fs, io, iter};
use std::{cmp, env, fmt, fs, io};
#[rustfmt::skip]
const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[
@ -67,33 +70,70 @@ impl DisallowedPath {
#[derive(Default)]
pub struct TryConf {
pub conf: Conf,
pub errors: Vec<Box<dyn Error>>,
pub warnings: Vec<Box<dyn Error>>,
pub errors: Vec<ConfError>,
pub warnings: Vec<ConfError>,
}
impl TryConf {
fn from_error(error: impl Error + 'static) -> Self {
fn from_toml_error(file: &SourceFile, error: &toml::de::Error) -> Self {
ConfError::from_toml(file, error).into()
}
}
impl From<ConfError> for TryConf {
fn from(value: ConfError) -> Self {
Self {
conf: Conf::default(),
errors: vec![Box::new(error)],
errors: vec![value],
warnings: vec![],
}
}
}
#[derive(Debug)]
struct ConfError(String);
impl fmt::Display for ConfError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
<String as fmt::Display>::fmt(&self.0, f)
impl From<io::Error> for TryConf {
fn from(value: io::Error) -> Self {
ConfError::from(value).into()
}
}
impl Error for ConfError {}
#[derive(Debug)]
pub struct ConfError {
pub message: String,
pub span: Option<Span>,
}
fn conf_error(s: impl Into<String>) -> Box<dyn Error> {
Box::new(ConfError(s.into()))
impl ConfError {
fn from_toml(file: &SourceFile, error: &toml::de::Error) -> Self {
if let Some(span) = error.span() {
Self::spanned(file, error.message(), span)
} else {
Self {
message: error.message().to_string(),
span: None,
}
}
}
fn spanned(file: &SourceFile, message: impl Into<String>, span: Range<usize>) -> Self {
Self {
message: message.into(),
span: Some(Span::new(
file.start_pos + BytePos::from_usize(span.start),
file.start_pos + BytePos::from_usize(span.end),
SyntaxContext::root(),
None,
)),
}
}
}
impl From<io::Error> for ConfError {
fn from(value: io::Error) -> Self {
Self {
message: value.to_string(),
span: None,
}
}
}
macro_rules! define_Conf {
@ -117,20 +157,14 @@ macro_rules! define_Conf {
}
}
impl<'de> Deserialize<'de> for TryConf {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> where D: Deserializer<'de> {
deserializer.deserialize_map(ConfVisitor)
}
}
#[derive(Deserialize)]
#[serde(field_identifier, rename_all = "kebab-case")]
#[allow(non_camel_case_types)]
enum Field { $($name,)* third_party, }
struct ConfVisitor;
struct ConfVisitor<'a>(&'a SourceFile);
impl<'de> Visitor<'de> for ConfVisitor {
impl<'de> Visitor<'de> for ConfVisitor<'_> {
type Value = TryConf;
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
@ -141,32 +175,38 @@ macro_rules! define_Conf {
let mut errors = Vec::new();
let mut warnings = Vec::new();
$(let mut $name = None;)*
// could get `Field` here directly, but get `str` first for diagnostics
while let Some(name) = map.next_key::<&str>()? {
match Field::deserialize(name.into_deserializer())? {
$(Field::$name => {
$(warnings.push(conf_error(format!("deprecated field `{}`. {}", name, $dep)));)?
match map.next_value() {
Err(e) => errors.push(conf_error(e.to_string())),
// could get `Field` here directly, but get `String` first for diagnostics
while let Some(name) = map.next_key::<toml::Spanned<String>>()? {
match Field::deserialize(name.get_ref().as_str().into_deserializer()) {
Err(e) => {
let e: FieldError = e;
errors.push(ConfError::spanned(self.0, e.0, name.span()));
}
$(Ok(Field::$name) => {
$(warnings.push(ConfError::spanned(self.0, format!("deprecated field `{}`. {}", name.get_ref(), $dep), name.span()));)?
let raw_value = map.next_value::<toml::Spanned<toml::Value>>()?;
let value_span = raw_value.span();
match <$ty>::deserialize(raw_value.into_inner()) {
Err(e) => errors.push(ConfError::spanned(self.0, e.to_string().replace('\n', " ").trim(), value_span)),
Ok(value) => match $name {
Some(_) => errors.push(conf_error(format!("duplicate field `{}`", name))),
Some(_) => errors.push(ConfError::spanned(self.0, format!("duplicate field `{}`", name.get_ref()), name.span())),
None => {
$name = Some(value);
// $new_conf is the same as one of the defined `$name`s, so
// this variable is defined in line 2 of this function.
$(match $new_conf {
Some(_) => errors.push(conf_error(concat!(
Some(_) => errors.push(ConfError::spanned(self.0, concat!(
"duplicate field `", stringify!($new_conf),
"` (provided as `", stringify!($name), "`)"
))),
), name.span())),
None => $new_conf = $name.clone(),
})?
},
}
}
})*
// white-listed; ignore
Field::third_party => drop(map.next_value::<IgnoredAny>())
// ignore contents of the third_party key
Ok(Field::third_party) => drop(map.next_value::<IgnoredAny>())
}
}
let conf = Conf { $($name: $name.unwrap_or_else(defaults::$name),)* };
@ -486,7 +526,7 @@ pub fn lookup_conf_file() -> io::Result<(Option<PathBuf>, Vec<String>)> {
const CONFIG_FILE_NAMES: [&str; 2] = [".clippy.toml", "clippy.toml"];
// Start looking for a config file in CLIPPY_CONF_DIR, or failing that, CARGO_MANIFEST_DIR.
// If neither of those exist, use ".".
// If neither of those exist, use ".". (Update documentation if this priority changes)
let mut current = env::var_os("CLIPPY_CONF_DIR")
.or_else(|| env::var_os("CARGO_MANIFEST_DIR"))
.map_or_else(|| PathBuf::from("."), PathBuf::from)
@ -532,19 +572,19 @@ pub fn lookup_conf_file() -> io::Result<(Option<PathBuf>, Vec<String>)> {
/// Read the `toml` configuration file.
///
/// In case of error, the function tries to continue as much as possible.
pub fn read(path: &Path) -> TryConf {
let content = match fs::read_to_string(path) {
Err(e) => return TryConf::from_error(e),
Ok(content) => content,
pub fn read(sess: &Session, path: &Path) -> TryConf {
let file = match sess.source_map().load_file(path) {
Err(e) => return e.into(),
Ok(file) => file,
};
match toml::from_str::<TryConf>(&content) {
match toml::de::Deserializer::new(file.src.as_ref().unwrap()).deserialize_map(ConfVisitor(&file)) {
Ok(mut conf) => {
extend_vec_if_indicator_present(&mut conf.conf.doc_valid_idents, DEFAULT_DOC_VALID_IDENTS);
extend_vec_if_indicator_present(&mut conf.conf.disallowed_names, DEFAULT_DISALLOWED_NAMES);
conf
},
Err(e) => TryConf::from_error(e),
Err(e) => TryConf::from_toml_error(&file, &e),
}
}
@ -556,65 +596,42 @@ fn extend_vec_if_indicator_present(vec: &mut Vec<String>, default: &[&str]) {
const SEPARATOR_WIDTH: usize = 4;
// Check whether the error is "unknown field" and, if so, list the available fields sorted and at
// least one per line, more if `CLIPPY_TERMINAL_WIDTH` is set and allows it.
pub fn format_error(error: Box<dyn Error>) -> String {
let s = error.to_string();
#[derive(Debug)]
struct FieldError(String);
if_chain! {
if error.downcast::<toml::de::Error>().is_ok();
if let Some((prefix, mut fields, suffix)) = parse_unknown_field_message(&s);
then {
use fmt::Write;
impl std::error::Error for FieldError {}
fields.sort_unstable();
let (rows, column_widths) = calculate_dimensions(&fields);
let mut msg = String::from(prefix);
for row in 0..rows {
writeln!(msg).unwrap();
for (column, column_width) in column_widths.iter().copied().enumerate() {
let index = column * rows + row;
let field = fields.get(index).copied().unwrap_or_default();
write!(
msg,
"{:SEPARATOR_WIDTH$}{field:column_width$}",
" "
)
.unwrap();
}
}
write!(msg, "\n{suffix}").unwrap();
msg
} else {
s
}
impl Display for FieldError {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
f.pad(&self.0)
}
}
// `parse_unknown_field_message` will become unnecessary if
// https://github.com/alexcrichton/toml-rs/pull/364 is merged.
fn parse_unknown_field_message(s: &str) -> Option<(&str, Vec<&str>, &str)> {
// An "unknown field" message has the following form:
// unknown field `UNKNOWN`, expected one of `FIELD0`, `FIELD1`, ..., `FIELDN` at line X column Y
// ^^ ^^^^ ^^
if_chain! {
if s.starts_with("unknown field");
let slices = s.split("`, `").collect::<Vec<_>>();
let n = slices.len();
if n >= 2;
if let Some((prefix, first_field)) = slices[0].rsplit_once(" `");
if let Some((last_field, suffix)) = slices[n - 1].split_once("` ");
then {
let fields = iter::once(first_field)
.chain(slices[1..n - 1].iter().copied())
.chain(iter::once(last_field))
.collect::<Vec<_>>();
Some((prefix, fields, suffix))
} else {
None
impl serde::de::Error for FieldError {
fn custom<T: Display>(msg: T) -> Self {
Self(msg.to_string())
}
fn unknown_field(field: &str, expected: &'static [&'static str]) -> Self {
// List the available fields sorted and at least one per line, more if `CLIPPY_TERMINAL_WIDTH` is
// set and allows it.
use fmt::Write;
let mut expected = expected.to_vec();
expected.sort_unstable();
let (rows, column_widths) = calculate_dimensions(&expected);
let mut msg = format!("unknown field `{field}`, expected one of");
for row in 0..rows {
writeln!(msg).unwrap();
for (column, column_width) in column_widths.iter().copied().enumerate() {
let index = column * rows + row;
let field = expected.get(index).copied().unwrap_or_default();
write!(msg, "{:SEPARATOR_WIDTH$}{field:column_width$}", " ").unwrap();
}
}
Self(msg)
}
}

View file

@ -65,8 +65,9 @@ declare_clippy_lint! {
/// This can lead to confusing error messages at best and to unexpected behavior at worst.
///
/// ### Exceptions
/// Wildcard imports are allowed from modules named `prelude`. Many crates (including the standard library)
/// provide modules named "prelude" specifically designed for wildcard import.
/// Wildcard imports are allowed from modules that their name contains `prelude`. Many crates
/// (including the standard library) provide modules named "prelude" specifically designed
/// for wildcard import.
///
/// `use super::*` is allowed in test modules. This is defined as any module with "test" in the name.
///
@ -212,7 +213,9 @@ impl WildcardImports {
// Allow "...prelude::..::*" imports.
// Many crates have a prelude, and it is imported as a glob by design.
fn is_prelude_import(segments: &[PathSegment<'_>]) -> bool {
segments.iter().any(|ps| ps.ident.name == sym::prelude)
segments
.iter()
.any(|ps| ps.ident.name.as_str().contains(sym::prelude.as_str()))
}
// Allow "super::*" imports in tests.