Switch to for_each_expr in some lints

This commit is contained in:
Alex Macleod 2024-05-19 15:08:36 +00:00
parent e6040437ef
commit 68e7356b7e
9 changed files with 77 additions and 17 deletions

View File

@ -3,7 +3,7 @@ use std::ops::ControlFlow;
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_path_lang_item; use clippy_utils::is_path_lang_item;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::{for_each_expr_without_closures, Visitable}; use clippy_utils::visitors::{for_each_expr, Visitable};
use rustc_ast::LitKind; use rustc_ast::LitKind;
use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::fx::FxHashSet;
use rustc_hir::def::{DefKind, Res}; use rustc_hir::def::{DefKind, Res};
@ -110,7 +110,7 @@ fn should_lint<'tcx>(
// Is there a call to `DebugStruct::debug_struct`? Do lint if there is. // Is there a call to `DebugStruct::debug_struct`? Do lint if there is.
let mut has_debug_struct = false; let mut has_debug_struct = false;
for_each_expr_without_closures(block, |expr| { for_each_expr(cx, block, |expr| {
if let ExprKind::MethodCall(path, recv, ..) = &expr.kind { if let ExprKind::MethodCall(path, recv, ..) = &expr.kind {
let recv_ty = typeck_results.expr_ty(recv).peel_refs(); let recv_ty = typeck_results.expr_ty(recv).peel_refs();
@ -165,7 +165,7 @@ fn check_struct<'tcx>(
let mut has_direct_field_access = false; let mut has_direct_field_access = false;
let mut field_accesses = FxHashSet::default(); let mut field_accesses = FxHashSet::default();
for_each_expr_without_closures(block, |expr| { for_each_expr(cx, block, |expr| {
if let ExprKind::Field(target, ident) = expr.kind if let ExprKind::Field(target, ident) = expr.kind
&& let target_ty = typeck_results.expr_ty_adjusted(target).peel_refs() && let target_ty = typeck_results.expr_ty_adjusted(target).peel_refs()
&& target_ty == self_ty && target_ty == self_ty

View File

@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::root_macro_call_first_node; use clippy_utils::macros::root_macro_call_first_node;
use clippy_utils::return_ty; use clippy_utils::return_ty;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::{for_each_expr_without_closures, Descend}; use clippy_utils::visitors::{for_each_expr, Descend};
use core::ops::ControlFlow; use core::ops::ControlFlow;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::intravisit::FnKind; use rustc_hir::intravisit::FnKind;
@ -64,7 +64,7 @@ impl<'tcx> LateLintPass<'tcx> for PanicInResultFn {
fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) { fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) {
let mut panics = Vec::new(); let mut panics = Vec::new();
let _: Option<!> = for_each_expr_without_closures(body.value, |e| { let _: Option<!> = for_each_expr(cx, body.value, |e| {
let Some(macro_call) = root_macro_call_first_node(cx, e) else { let Some(macro_call) = root_macro_call_first_node(cx, e) else {
return ControlFlow::Continue(Descend::Yes); return ControlFlow::Continue(Descend::Yes);
}; };

View File

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::for_each_expr_without_closures; use clippy_utils::visitors::for_each_expr;
use clippy_utils::{method_chain_args, return_ty}; use clippy_utils::{method_chain_args, return_ty};
use core::ops::ControlFlow; use core::ops::ControlFlow;
use rustc_hir as hir; use rustc_hir as hir;
@ -75,7 +75,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tc
let body = cx.tcx.hir().body(body_id); let body = cx.tcx.hir().body(body_id);
let typeck = cx.tcx.typeck(impl_item.owner_id.def_id); let typeck = cx.tcx.typeck(impl_item.owner_id.def_id);
let mut result = Vec::new(); let mut result = Vec::new();
let _: Option<!> = for_each_expr_without_closures(body.value, |e| { let _: Option<!> = for_each_expr(cx, body.value, |e| {
// check for `expect` // check for `expect`
if let Some(arglists) = method_chain_args(e, &["expect"]) { if let Some(arglists) = method_chain_args(e, &["expect"]) {
let receiver_ty = typeck.expr_ty(arglists[0].0).peel_refs(); let receiver_ty = typeck.expr_ty(arglists[0].0).peel_refs();

View File

@ -4,6 +4,7 @@
use std::fmt; use std::fmt;
use std::marker::PhantomData; use std::marker::PhantomData;
use std::ops::Deref; use std::ops::Deref;
use std::thread::LocalKey;
struct NamedStruct1Ignored { struct NamedStruct1Ignored {
data: u8, data: u8,
@ -191,4 +192,21 @@ impl fmt::Debug for WithPD {
} }
} }
struct InClosure {
a: u8,
b: String,
}
impl fmt::Debug for InClosure {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut d = f.debug_struct("InClosure");
d.field("a", &self.a);
let mut c = || {
d.field("b", &self.b);
};
c();
d.finish()
}
}
fn main() {} fn main() {}

View File

@ -1,5 +1,5 @@
error: manual `Debug` impl does not include all fields error: manual `Debug` impl does not include all fields
--> tests/ui/missing_fields_in_debug.rs:13:1 --> tests/ui/missing_fields_in_debug.rs:14:1
| |
LL | / impl fmt::Debug for NamedStruct1Ignored { LL | / impl fmt::Debug for NamedStruct1Ignored {
LL | | LL | |
@ -11,7 +11,7 @@ LL | | }
| |_^ | |_^
| |
note: this field is unused note: this field is unused
--> tests/ui/missing_fields_in_debug.rs:10:5 --> tests/ui/missing_fields_in_debug.rs:11:5
| |
LL | hidden: u32, LL | hidden: u32,
| ^^^^^^^^^^^ | ^^^^^^^^^^^
@ -21,7 +21,7 @@ LL | hidden: u32,
= help: to override `-D warnings` add `#[allow(clippy::missing_fields_in_debug)]` = help: to override `-D warnings` add `#[allow(clippy::missing_fields_in_debug)]`
error: manual `Debug` impl does not include all fields error: manual `Debug` impl does not include all fields
--> tests/ui/missing_fields_in_debug.rs:32:1 --> tests/ui/missing_fields_in_debug.rs:33:1
| |
LL | / impl fmt::Debug for NamedStructMultipleIgnored { LL | / impl fmt::Debug for NamedStructMultipleIgnored {
LL | | LL | |
@ -33,17 +33,17 @@ LL | | }
| |_^ | |_^
| |
note: this field is unused note: this field is unused
--> tests/ui/missing_fields_in_debug.rs:26:5 --> tests/ui/missing_fields_in_debug.rs:27:5
| |
LL | hidden: u32, LL | hidden: u32,
| ^^^^^^^^^^^ | ^^^^^^^^^^^
note: this field is unused note: this field is unused
--> tests/ui/missing_fields_in_debug.rs:27:5 --> tests/ui/missing_fields_in_debug.rs:28:5
| |
LL | hidden2: String, LL | hidden2: String,
| ^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^
note: this field is unused note: this field is unused
--> tests/ui/missing_fields_in_debug.rs:29:5 --> tests/ui/missing_fields_in_debug.rs:30:5
| |
LL | hidden4: ((((u8), u16), u32), u64), LL | hidden4: ((((u8), u16), u32), u64),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -51,7 +51,7 @@ LL | hidden4: ((((u8), u16), u32), u64),
= help: consider calling `.finish_non_exhaustive()` if you intend to ignore fields = help: consider calling `.finish_non_exhaustive()` if you intend to ignore fields
error: manual `Debug` impl does not include all fields error: manual `Debug` impl does not include all fields
--> tests/ui/missing_fields_in_debug.rs:94:1 --> tests/ui/missing_fields_in_debug.rs:95:1
| |
LL | / impl fmt::Debug for MultiExprDebugImpl { LL | / impl fmt::Debug for MultiExprDebugImpl {
LL | | LL | |
@ -63,7 +63,7 @@ LL | | }
| |_^ | |_^
| |
note: this field is unused note: this field is unused
--> tests/ui/missing_fields_in_debug.rs:90:5 --> tests/ui/missing_fields_in_debug.rs:91:5
| |
LL | b: String, LL | b: String,
| ^^^^^^^^^ | ^^^^^^^^^

View File

@ -56,6 +56,11 @@ fn function_result_with_panic() -> Result<bool, String> // should emit lint
panic!("error"); panic!("error");
} }
fn in_closure() -> Result<bool, String> {
let c = || panic!();
c()
}
fn todo() { fn todo() {
println!("something"); println!("something");
} }

View File

@ -34,5 +34,21 @@ note: return Err() instead of panicking
LL | panic!("error"); LL | panic!("error");
| ^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^
error: aborting due to 2 previous errors error: used `panic!()` or assertion in a function that returns `Result`
--> tests/ui/panic_in_result_fn.rs:59:1
|
LL | / fn in_closure() -> Result<bool, String> {
LL | | let c = || panic!();
LL | | c()
LL | | }
| |_^
|
= help: `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> tests/ui/panic_in_result_fn.rs:60:16
|
LL | let c = || panic!();
| ^^^^^^^^
error: aborting due to 3 previous errors

View File

@ -38,6 +38,11 @@ impl A {
} }
None None
} }
fn in_closure(a: Option<bool>) -> Option<bool> {
let c = || a.unwrap();
Some(c())
}
} }
fn main() { fn main() {

View File

@ -38,5 +38,21 @@ note: potential non-recoverable error(s)
LL | let i = i_str.parse::<i32>().expect("not a number"); LL | let i = i_str.parse::<i32>().expect("not a number");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 2 previous errors error: used unwrap or expect in a function that returns result or option
--> tests/ui/unwrap_in_result.rs:42:5
|
LL | / fn in_closure(a: Option<bool>) -> Option<bool> {
LL | | let c = || a.unwrap();
LL | | Some(c())
LL | | }
| |_____^
|
= help: unwrap and expect should not be used in a function that returns result or option
note: potential non-recoverable error(s)
--> tests/ui/unwrap_in_result.rs:43:20
|
LL | let c = || a.unwrap();
| ^^^^^^^^^^
error: aborting due to 3 previous errors