Skip to content

Commit

Permalink
Auto merge of #13065 - Jarcho:misc_small2, r=xFrednet
Browse files Browse the repository at this point in the history 
Misc refactorings part 2

A couple of theses are a bit more involved. No behaviour changes included in this set.

changelog: none
  • Loading branch information
@bors
bors committed Jul 22, 2024
2 parents 1807580 + f493d71 commit 637d39f
Show file tree
Hide file tree
Showing 13 changed files with 169 additions and 264 deletions.
48 changes: 21 additions & 27 deletions clippy_lints/src/double_parens.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,35 +40,29 @@ declare_lint_pass!(DoubleParens => [DOUBLE_PARENS]);

impl EarlyLintPass for DoubleParens {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
if expr.span.from_expansion() {
return;
}

let msg: &str = "consider removing unnecessary double parentheses";

match expr.kind {
ExprKind::Paren(ref in_paren) => match in_paren.kind {
ExprKind::Paren(_) | ExprKind::Tup(_) => {
span_lint(cx, DOUBLE_PARENS, expr.span, msg);
},
_ => {},
},
ExprKind::Call(_, ref params) => {
if params.len() == 1 {
let param = &params[0];
if let ExprKind::Paren(_) = param.kind {
span_lint(cx, DOUBLE_PARENS, param.span, msg);
}
}
let span = match &expr.kind {
ExprKind::Paren(in_paren) if matches!(in_paren.kind, ExprKind::Paren(_) | ExprKind::Tup(_)) => expr.span,
ExprKind::Call(_, params)
if let [param] = &**params
&& let ExprKind::Paren(_) = param.kind =>
{
param.span
},
ExprKind::MethodCall(ref call) => {
if let [ref arg] = call.args[..] {
if let ExprKind::Paren(_) = arg.kind {
span_lint(cx, DOUBLE_PARENS, arg.span, msg);
}
}
ExprKind::MethodCall(call)
if let [arg] = &*call.args
&& let ExprKind::Paren(_) = arg.kind =>
{
arg.span
},
_ => {},
_ => return,
};
if !expr.span.from_expansion() {
span_lint(
cx,
DOUBLE_PARENS,
span,
"consider removing unnecessary double parentheses",
);
}
}
}
39 changes: 16 additions & 23 deletions clippy_lints/src/endian_bytes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,32 +109,27 @@ impl LintKind {

impl LateLintPass<'_> for EndianBytes {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
if in_external_macro(cx.sess(), expr.span) {
return;
}

if let ExprKind::MethodCall(method_name, receiver, args, ..) = expr.kind
&& args.is_empty()
&& let ty = cx.typeck_results().expr_ty(receiver)
let (prefix, name, ty_expr) = match expr.kind {
ExprKind::MethodCall(method_name, receiver, [], ..) => (Prefix::To, method_name.ident.name, receiver),
ExprKind::Call(function, ..)
if let ExprKind::Path(qpath) = function.kind
&& let Some(def_id) = cx.qpath_res(&qpath, function.hir_id).opt_def_id()
&& let Some(function_name) = cx.get_def_path(def_id).last() =>
{
(Prefix::From, *function_name, expr)
},
_ => return,
};
if !in_external_macro(cx.sess(), expr.span)
&& let ty = cx.typeck_results().expr_ty(ty_expr)
&& ty.is_primitive_ty()
&& maybe_lint_endian_bytes(cx, expr, Prefix::To, method_name.ident.name, ty)
{
return;
}

if let ExprKind::Call(function, ..) = expr.kind
&& let ExprKind::Path(qpath) = function.kind
&& let Some(def_id) = cx.qpath_res(&qpath, function.hir_id).opt_def_id()
&& let Some(function_name) = cx.get_def_path(def_id).last()
&& let ty = cx.typeck_results().expr_ty(expr)
&& ty.is_primitive_ty()
{
maybe_lint_endian_bytes(cx, expr, Prefix::From, *function_name, ty);
maybe_lint_endian_bytes(cx, expr, prefix, name, ty);
}
}
}

fn maybe_lint_endian_bytes(cx: &LateContext<'_>, expr: &Expr<'_>, prefix: Prefix, name: Symbol, ty: Ty<'_>) -> bool {
fn maybe_lint_endian_bytes(cx: &LateContext<'_>, expr: &Expr<'_>, prefix: Prefix, name: Symbol, ty: Ty<'_>) {
let ne = LintKind::Host.as_name(prefix);
let le = LintKind::Little.as_name(prefix);
let be = LintKind::Big.as_name(prefix);
Expand All @@ -143,7 +138,7 @@ fn maybe_lint_endian_bytes(cx: &LateContext<'_>, expr: &Expr<'_>, prefix: Prefix
name if name == ne => ((&LintKind::Host), [(&LintKind::Little), (&LintKind::Big)]),
name if name == le => ((&LintKind::Little), [(&LintKind::Host), (&LintKind::Big)]),
name if name == be => ((&LintKind::Big), [(&LintKind::Host), (&LintKind::Little)]),
_ => return false,
_ => return,
};

let mut help = None;
Expand Down Expand Up @@ -208,6 +203,4 @@ fn maybe_lint_endian_bytes(cx: &LateContext<'_>, expr: &Expr<'_>, prefix: Prefix
}
},
);

true
}
95 changes: 43 additions & 52 deletions clippy_lints/src/excessive_bools.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,77 +88,67 @@ pub struct ExcessiveBools {
max_fn_params_bools: u64,
}

#[derive(Eq, PartialEq, Debug, Copy, Clone)]
enum Kind {
Struct,
Fn,
}

impl ExcessiveBools {
pub fn new(conf: &'static Conf) -> Self {
Self {
max_struct_bools: conf.max_struct_bools,
max_fn_params_bools: conf.max_fn_params_bools,
}
}
}

fn too_many_bools<'tcx>(&self, tys: impl Iterator<Item = &'tcx Ty<'tcx>>, kind: Kind) -> bool {
if let Ok(bools) = tys.filter(|ty| is_bool(ty)).count().try_into() {
(if Kind::Fn == kind {
self.max_fn_params_bools
} else {
self.max_struct_bools
}) < bools
} else {
false
}
}
impl_lint_pass!(ExcessiveBools => [STRUCT_EXCESSIVE_BOOLS, FN_PARAMS_EXCESSIVE_BOOLS]);

fn check_fn_sig(&self, cx: &LateContext<'_>, fn_decl: &FnDecl<'_>, span: Span) {
if !span.from_expansion() && self.too_many_bools(fn_decl.inputs.iter(), Kind::Fn) {
span_lint_and_help(
cx,
FN_PARAMS_EXCESSIVE_BOOLS,
span,
format!("more than {} bools in function parameters", self.max_fn_params_bools),
None,
"consider refactoring bools into two-variant enums",
);
}
}
fn has_n_bools<'tcx>(iter: impl Iterator<Item = &'tcx Ty<'tcx>>, mut count: u64) -> bool {
iter.filter(|ty| is_bool(ty)).any(|_| {
let (x, overflow) = count.overflowing_sub(1);
count = x;
overflow
})
}

impl_lint_pass!(ExcessiveBools => [STRUCT_EXCESSIVE_BOOLS, FN_PARAMS_EXCESSIVE_BOOLS]);
fn check_fn_decl(cx: &LateContext<'_>, decl: &FnDecl<'_>, sp: Span, max: u64) {
if has_n_bools(decl.inputs.iter(), max) && !sp.from_expansion() {
span_lint_and_help(
cx,
FN_PARAMS_EXCESSIVE_BOOLS,
sp,
format!("more than {max} bools in function parameters"),
None,
"consider refactoring bools into two-variant enums",
);
}
}

impl<'tcx> LateLintPass<'tcx> for ExcessiveBools {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if item.span.from_expansion() {
return;
}
if let ItemKind::Struct(variant_data, _) = &item.kind {
if has_repr_attr(cx, item.hir_id()) {
return;
}

if self.too_many_bools(variant_data.fields().iter().map(|field| field.ty), Kind::Struct) {
span_lint_and_help(
cx,
STRUCT_EXCESSIVE_BOOLS,
item.span,
format!("more than {} bools in a struct", self.max_struct_bools),
None,
"consider using a state machine or refactoring bools into two-variant enums",
);
}
if let ItemKind::Struct(variant_data, _) = &item.kind
&& variant_data.fields().len() as u64 > self.max_struct_bools
&& has_n_bools(
variant_data.fields().iter().map(|field| field.ty),
self.max_struct_bools,
)
&& !has_repr_attr(cx, item.hir_id())
&& !item.span.from_expansion()
{
span_lint_and_help(
cx,
STRUCT_EXCESSIVE_BOOLS,
item.span,
format!("more than {} bools in a struct", self.max_struct_bools),
None,
"consider using a state machine or refactoring bools into two-variant enums",
);
}
}

fn check_trait_item(&mut self, cx: &LateContext<'tcx>, trait_item: &'tcx TraitItem<'tcx>) {
// functions with a body are already checked by `check_fn`
if let TraitItemKind::Fn(fn_sig, TraitFn::Required(_)) = &trait_item.kind
&& fn_sig.header.abi == Abi::Rust
&& fn_sig.decl.inputs.len() as u64 > self.max_fn_params_bools
{
self.check_fn_sig(cx, fn_sig.decl, fn_sig.span);
check_fn_decl(cx, fn_sig.decl, fn_sig.span, self.max_fn_params_bools);
}
}

Expand All @@ -171,12 +161,13 @@ impl<'tcx> LateLintPass<'tcx> for ExcessiveBools {
span: Span,
def_id: LocalDefId,
) {
let hir_id = cx.tcx.local_def_id_to_hir_id(def_id);
if let Some(fn_header) = fn_kind.header()
&& fn_header.abi == Abi::Rust
&& get_parent_as_impl(cx.tcx, hir_id).map_or(true, |impl_item| impl_item.of_trait.is_none())
&& fn_decl.inputs.len() as u64 > self.max_fn_params_bools
&& get_parent_as_impl(cx.tcx, cx.tcx.local_def_id_to_hir_id(def_id))
.map_or(true, |impl_item| impl_item.of_trait.is_none())
{
self.check_fn_sig(cx, fn_decl, span);
check_fn_decl(cx, fn_decl, span, self.max_fn_params_bools);
}
}
}
30 changes: 13 additions & 17 deletions clippy_lints/src/extra_unused_type_parameters.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,6 @@ impl ExtraUnusedTypeParameters {
avoid_breaking_exported_api: conf.avoid_breaking_exported_api,
}
}

/// Don't lint external macros or functions with empty bodies. Also, don't lint exported items
/// if the `avoid_breaking_exported_api` config option is set.
fn is_empty_exported_or_macro(
&self,
cx: &LateContext<'_>,
span: Span,
def_id: LocalDefId,
body_id: BodyId,
) -> bool {
let body = cx.tcx.hir().body(body_id).value;
let fn_empty = matches!(&body.kind, ExprKind::Block(blk, None) if blk.stmts.is_empty() && blk.expr.is_none());
let is_exported = cx.effective_visibilities.is_exported(def_id);
in_external_macro(cx.sess(), span) || fn_empty || (is_exported && self.avoid_breaking_exported_api)
}
}

impl_lint_pass!(ExtraUnusedTypeParameters => [EXTRA_UNUSED_TYPE_PARAMETERS]);
Expand Down Expand Up @@ -267,10 +252,17 @@ impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {
}
}

fn is_empty_body(cx: &LateContext<'_>, body: BodyId) -> bool {
matches!(cx.tcx.hir().body(body).value.kind, ExprKind::Block(b, _) if b.stmts.is_empty() && b.expr.is_none())
}

impl<'tcx> LateLintPass<'tcx> for ExtraUnusedTypeParameters {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if let ItemKind::Fn(_, generics, body_id) = item.kind
&& !self.is_empty_exported_or_macro(cx, item.span, item.owner_id.def_id, body_id)
&& !generics.params.is_empty()
&& !is_empty_body(cx, body_id)
&& (!self.avoid_breaking_exported_api || !cx.effective_visibilities.is_exported(item.owner_id.def_id))
&& !in_external_macro(cx.sess(), item.span)
&& !is_from_proc_macro(cx, item)
{
let mut walker = TypeWalker::new(cx, generics);
Expand All @@ -282,8 +274,12 @@ impl<'tcx> LateLintPass<'tcx> for ExtraUnusedTypeParameters {
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) {
// Only lint on inherent methods, not trait methods.
if let ImplItemKind::Fn(.., body_id) = item.kind
&& !item.generics.params.is_empty()
&& trait_ref_of_method(cx, item.owner_id.def_id).is_none()
&& !self.is_empty_exported_or_macro(cx, item.span, item.owner_id.def_id, body_id)
&& !is_empty_body(cx, body_id)
&& (!self.avoid_breaking_exported_api || !cx.effective_visibilities.is_exported(item.owner_id.def_id))
&& !in_external_macro(cx.sess(), item.span)
&& !is_from_proc_macro(cx, item)
{
let mut walker = TypeWalker::new(cx, item.generics);
walk_impl_item(&mut walker, item);
Expand Down
14 changes: 5 additions & 9 deletions clippy_lints/src/future_not_send.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,11 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend {
let ret_ty = return_ty(cx, cx.tcx.local_def_id_to_hir_id(fn_def_id).expect_owner());
if let ty::Alias(ty::Opaque, AliasTy { def_id, args, .. }) = *ret_ty.kind() {
let preds = cx.tcx.explicit_item_super_predicates(def_id);
let mut is_future = false;
for (p, _span) in preds.iter_instantiated_copied(cx.tcx, args) {
if let Some(trait_pred) = p.as_trait_clause() {
if Some(trait_pred.skip_binder().trait_ref.def_id) == cx.tcx.lang_items().future_trait() {
is_future = true;
break;
}
}
}
let is_future = preds.iter_instantiated_copied(cx.tcx, args).any(|(p, _)| {
p.as_trait_clause().is_some_and(|trait_pred| {
Some(trait_pred.skip_binder().trait_ref.def_id) == cx.tcx.lang_items().future_trait()
})
});
if is_future {
let send_trait = cx.tcx.get_diagnostic_item(sym::Send).unwrap();
let span = decl.output.span();
Expand Down
Loading

0 comments on commit 637d39f

Please sign in to comment.