Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc refactorings part 2 #13065

Merged
merged 13 commits into from
Jul 22, 2024
48 changes: 21 additions & 27 deletions clippy_lints/src/double_parens.rs
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,6 @@ pub struct ExcessiveBools {
max_fn_params_bools: u64,
}

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

impl ExcessiveBools {
#[must_use]
pub fn new(max_struct_bools: u64, max_fn_params_bools: u64) -> Self {
Expand All @@ -101,64 +95,60 @@ impl ExcessiveBools {
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
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,6 @@ impl ExtraUnusedTypeParameters {
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 @@ -266,10 +251,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 @@ -281,8 +273,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
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