mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #12287 - Jarcho:issue_12250, r=llogiq

Browse Source 
Add lint `manual_inspect`

fixes #12250

A great example of a lint that sounds super simple, but has a pile of edge cases.

----

changelog: Add lint `manual_inspect`
This commit is contained in:
bors 2024-06-16 22:54:40 +00:00
commit 16efa56503
16 changed files with 964 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -5521,6 +5521,7 @@ Released 2018-09-13
[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
[`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
[`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one
[`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect
[`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
[`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check
[`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite

2
clippy_config/src/msrvs.rs
View File

@ -18,7 +18,7 @@ macro_rules! msrv_aliases {
// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,77,0 { C_STR_LITERALS }
1,76,0 { PTR_FROM_REF }
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }
1,68,0 { PATH_MAIN_SEPARATOR_STR }

4
clippy_lints/src/casts/ref_as_ptr.rs
View File

@ -22,9 +22,9 @@ pub(super) fn check<'tcx>(
if matches!(cast_from.kind(), ty::Ref(..))
&& let ty::RawPtr(_, to_mutbl) = cast_to.kind()
&& let Some(use_cx) = expr_use_ctxt(cx, expr)
&& let use_cx = expr_use_ctxt(cx, expr)
// TODO: only block the lint if `cast_expr` is a temporary
&& !matches!(use_cx.node, ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_))
&& !matches!(use_cx.use_node(cx), ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_))
{
let core_or_std = if is_no_std_crate(cx) { "core" } else { "std" };
let fn_name = match to_mutbl {

1
clippy_lints/src/declared_lints.rs
View File

@ -403,6 +403,7 @@
crate::methods::MANUAL_C_STR_LITERALS_INFO,
crate::methods::MANUAL_FILTER_MAP_INFO,
crate::methods::MANUAL_FIND_MAP_INFO,
crate::methods::MANUAL_INSPECT_INFO,
crate::methods::MANUAL_IS_VARIANT_AND_INFO,
crate::methods::MANUAL_NEXT_BACK_INFO,
crate::methods::MANUAL_OK_OR_INFO,

46
clippy_lints/src/dereference.rs
View File

@ -260,18 +260,13 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
(None, kind) => {
let expr_ty = typeck.expr_ty(expr);
let use_cx = expr_use_ctxt(cx, expr);
let adjusted_ty = match &use_cx {
Some(use_cx) => match use_cx.adjustments {
[.., a] => a.target,
_ => expr_ty,
},
_ => typeck.expr_ty_adjusted(expr),
};
let adjusted_ty = use_cx.adjustments.last().map_or(expr_ty, |a| a.target);
match (use_cx, kind) {
(Some(use_cx), RefOp::Deref) => {
match kind {
RefOp::Deref if use_cx.same_ctxt => {
let use_node = use_cx.use_node(cx);
let sub_ty = typeck.expr_ty(sub_expr);
if let ExprUseNode::FieldAccess(name) = use_cx.node
if let ExprUseNode::FieldAccess(name) = use_node
&& !use_cx.moved_before_use
&& !ty_contains_field(sub_ty, name.name)
{
@ -288,9 +283,9 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
} else if sub_ty.is_ref()
// Linting method receivers would require verifying that name lookup
// would resolve the same way. This is complicated by trait methods.
&& !use_cx.node.is_recv()
&& let Some(ty) = use_cx.node.defined_ty(cx)
&& TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return()).is_deref_stable()
&& !use_node.is_recv()
&& let Some(ty) = use_node.defined_ty(cx)
&& TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return()).is_deref_stable()
{
self.state = Some((
State::ExplicitDeref { mutability: None },
@ -301,7 +296,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
));
}
},
(_, RefOp::Method { mutbl, is_ufcs })
RefOp::Method { mutbl, is_ufcs }
if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id)
// Allow explicit deref in method chains. e.g. `foo.deref().bar()`
&& (is_ufcs || !in_postfix_position(cx, expr)) =>
@ -319,7 +314,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
},
));
},
(Some(use_cx), RefOp::AddrOf(mutability)) => {
RefOp::AddrOf(mutability) if use_cx.same_ctxt => {
// Find the number of times the borrow is auto-derefed.
let mut iter = use_cx.adjustments.iter();
let mut deref_count = 0usize;
@ -338,10 +333,11 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
};
};
let stability = use_cx.node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| {
TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return())
let use_node = use_cx.use_node(cx);
let stability = use_node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| {
TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return())
});
let can_auto_borrow = match use_cx.node {
let can_auto_borrow = match use_node {
ExprUseNode::FieldAccess(_)
if !use_cx.moved_before_use && matches!(sub_expr.kind, ExprKind::Field(..)) =>
{
@ -353,7 +349,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// deref through `ManuallyDrop<_>` will not compile.
!adjust_derefs_manually_drop(use_cx.adjustments, expr_ty)
},
ExprUseNode::Callee | ExprUseNode::FieldAccess(_) => true,
ExprUseNode::Callee | ExprUseNode::FieldAccess(_) if !use_cx.moved_before_use => true,
ExprUseNode::MethodArg(hir_id, _, 0) if !use_cx.moved_before_use => {
// Check for calls to trait methods where the trait is implemented
// on a reference.
@ -363,9 +359,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// priority.
if let Some(fn_id) = typeck.type_dependent_def_id(hir_id)
&& let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
&& let arg_ty = cx
.tcx
.erase_regions(use_cx.adjustments.last().map_or(expr_ty, |a| a.target))
&& let arg_ty = cx.tcx.erase_regions(adjusted_ty)
&& let ty::Ref(_, sub_ty, _) = *arg_ty.kind()
&& let args =
typeck.node_args_opt(hir_id).map(|args| &args[1..]).unwrap_or_default()
@ -443,7 +437,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
count: deref_count - required_refs,
msg,
stability,
for_field_access: if let ExprUseNode::FieldAccess(name) = use_cx.node
for_field_access: if let ExprUseNode::FieldAccess(name) = use_node
&& !use_cx.moved_before_use
{
Some(name.name)
@ -453,7 +447,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}),
StateData {
first_expr: expr,
adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target),
adjusted_ty,
},
));
} else if stability.is_deref_stable()
@ -465,12 +459,12 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
State::Borrow { mutability },
StateData {
first_expr: expr,
adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target),
adjusted_ty,
},
));
}
},
(None, _) | (_, RefOp::Method { .. }) => (),
_ => {},
}
},
(

233
clippy_lints/src/methods/manual_inspect.rs Normal file
View File

@ -0,0 +1,233 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{get_source_text, with_leading_whitespace, SpanRange};
use clippy_utils::ty::get_field_by_name;
use clippy_utils::visitors::{for_each_expr, for_each_expr_without_closures};
use clippy_utils::{expr_use_ctxt, is_diag_item_method, is_diag_trait_item, path_to_local_id, ExprUseNode};
use core::ops::ControlFlow;
use rustc_errors::Applicability;
use rustc_hir::{BindingMode, BorrowKind, ByRef, ClosureKind, Expr, ExprKind, Mutability, Node, PatKind};
use rustc_lint::LateContext;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
use rustc_span::{sym, BytePos, Span, Symbol, DUMMY_SP};
use super::MANUAL_INSPECT;
#[expect(clippy::too_many_lines)]
pub(crate) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &Expr<'_>, name: &str, name_span: Span, msrv: &Msrv) {
if let ExprKind::Closure(c) = arg.kind
&& matches!(c.kind, ClosureKind::Closure)
&& let typeck = cx.typeck_results()
&& let Some(fn_id) = typeck.type_dependent_def_id(expr.hir_id)
&& (is_diag_trait_item(cx, fn_id, sym::Iterator)
|| (msrv.meets(msrvs::OPTION_RESULT_INSPECT)
&& (is_diag_item_method(cx, fn_id, sym::Option) || is_diag_item_method(cx, fn_id, sym::Result))))
&& let body = cx.tcx.hir().body(c.body)
&& let [param] = body.params
&& let PatKind::Binding(BindingMode(ByRef::No, Mutability::Not), arg_id, _, None) = param.pat.kind
&& let arg_ty = typeck.node_type(arg_id)
&& let ExprKind::Block(block, _) = body.value.kind
&& let Some(final_expr) = block.expr
&& !block.stmts.is_empty()
&& path_to_local_id(final_expr, arg_id)
&& typeck.expr_adjustments(final_expr).is_empty()
{
let mut requires_copy = false;
let mut requires_deref = false;
// The number of unprocessed return expressions.
let mut ret_count = 0u32;
// The uses for which processing is delayed until after the visitor.
let mut delayed = vec![];
let ctxt = arg.span.ctxt();
let can_lint = for_each_expr_without_closures(block.stmts, |e| {
if let ExprKind::Closure(c) = e.kind {
// Nested closures don't need to treat returns specially.
let _: Option<!> = for_each_expr(cx, cx.tcx.hir().body(c.body).value, |e| {
if path_to_local_id(e, arg_id) {
let (kind, same_ctxt) = check_use(cx, e);
match (kind, same_ctxt && e.span.ctxt() == ctxt) {
(_, false) | (UseKind::Deref | UseKind::Return(..), true) => {
requires_copy = true;
requires_deref = true;
},
(UseKind::AutoBorrowed, true) => {},
(UseKind::WillAutoDeref, true) => {
requires_copy = true;
},
(kind, true) => delayed.push(kind),
}
}
ControlFlow::Continue(())
});
} else if matches!(e.kind, ExprKind::Ret(_)) {
ret_count += 1;
} else if path_to_local_id(e, arg_id) {
let (kind, same_ctxt) = check_use(cx, e);
match (kind, same_ctxt && e.span.ctxt() == ctxt) {
(UseKind::Return(..), false) => {
return ControlFlow::Break(());
},
(_, false) | (UseKind::Deref, true) => {
requires_copy = true;
requires_deref = true;
},
(UseKind::AutoBorrowed, true) => {},
(UseKind::WillAutoDeref, true) => {
requires_copy = true;
},
(kind @ UseKind::Return(_), true) => {
ret_count -= 1;
delayed.push(kind);
},
(kind, true) => delayed.push(kind),
}
}
ControlFlow::Continue(())
})
.is_none();
if ret_count != 0 {
// A return expression that didn't return the original value was found.
return;
}
let mut edits = Vec::with_capacity(delayed.len() + 3);
let mut addr_of_edits = Vec::with_capacity(delayed.len());
for x in delayed {
match x {
UseKind::Return(s) => edits.push((with_leading_whitespace(cx, s).set_span_pos(s), String::new())),
UseKind::Borrowed(s) => {
if let Some(src) = get_source_text(cx, s)
&& let Some(src) = src.as_str()
&& let trim_src = src.trim_start_matches([' ', '\t', '\n', '\r', '('])
&& trim_src.starts_with('&')
{
let range = s.into_range();
#[expect(clippy::cast_possible_truncation)]
let start = BytePos(range.start.0 + (src.len() - trim_src.len()) as u32);
addr_of_edits.push(((start..BytePos(start.0 + 1)).set_span_pos(s), String::new()));
} else {
requires_copy = true;
requires_deref = true;
}
},
UseKind::FieldAccess(name, e) => {
let Some(mut ty) = get_field_by_name(cx.tcx, arg_ty.peel_refs(), name) else {
requires_copy = true;
continue;
};
let mut prev_expr = e;
for (_, parent) in cx.tcx.hir().parent_iter(e.hir_id) {
if let Node::Expr(e) = parent {
match e.kind {
ExprKind::Field(_, name)
if let Some(fty) = get_field_by_name(cx.tcx, ty.peel_refs(), name.name) =>
{
ty = fty;
prev_expr = e;
continue;
},
ExprKind::AddrOf(BorrowKind::Ref, ..) => break,
_ if matches!(
typeck.expr_adjustments(prev_expr).first(),
Some(Adjustment {
kind: Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Not))
| Adjust::Deref(_),
..
})
) =>
{
break;
},
_ => {},
}
}
requires_copy |= !ty.is_copy_modulo_regions(cx.tcx, cx.param_env);
break;
}
},
// Already processed uses.
UseKind::AutoBorrowed | UseKind::WillAutoDeref | UseKind::Deref => {},
}
}
if can_lint
&& (!requires_copy || arg_ty.is_copy_modulo_regions(cx.tcx, cx.param_env))
// This case could be handled, but a fair bit of care would need to be taken.
&& (!requires_deref || arg_ty.is_freeze(cx.tcx, cx.param_env))
{
if requires_deref {
edits.push((param.span.shrink_to_lo(), "&".into()));
} else {
edits.extend(addr_of_edits);
}
edits.push((
name_span,
String::from(match name {
"map" => "inspect",
"map_err" => "inspect_err",
_ => return,
}),
));
edits.push((
with_leading_whitespace(cx, final_expr.span).set_span_pos(final_expr.span),
String::new(),
));
let app = if edits.iter().any(|(s, _)| s.from_expansion()) {
Applicability::MaybeIncorrect
} else {
Applicability::MachineApplicable
};
span_lint_and_then(cx, MANUAL_INSPECT, name_span, "", |diag| {
diag.multipart_suggestion("try", edits, app);
});
}
}
}
enum UseKind<'tcx> {
AutoBorrowed,
WillAutoDeref,
Deref,
Return(Span),
Borrowed(Span),
FieldAccess(Symbol, &'tcx Expr<'tcx>),
}
/// Checks how the value is used, and whether it was used in the same `SyntaxContext`.
fn check_use<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (UseKind<'tcx>, bool) {
let use_cx = expr_use_ctxt(cx, e);
if use_cx
.adjustments
.first()
.is_some_and(|a| matches!(a.kind, Adjust::Deref(_)))
{
return (UseKind::AutoBorrowed, use_cx.same_ctxt);
}
let res = match use_cx.use_node(cx) {
ExprUseNode::Return(_) => {
if let ExprKind::Ret(Some(e)) = use_cx.node.expect_expr().kind {
UseKind::Return(e.span)
} else {
return (UseKind::Return(DUMMY_SP), false);
}
},
ExprUseNode::FieldAccess(name) => UseKind::FieldAccess(name.name, use_cx.node.expect_expr()),
ExprUseNode::Callee | ExprUseNode::MethodArg(_, _, 0)
if use_cx
.adjustments
.first()
.is_some_and(|a| matches!(a.kind, Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Not)))) =>
{
UseKind::AutoBorrowed
},
ExprUseNode::Callee | ExprUseNode::MethodArg(_, _, 0) => UseKind::WillAutoDeref,
ExprUseNode::AddrOf(BorrowKind::Ref, _) => UseKind::Borrowed(use_cx.node.expect_expr().span),
_ => UseKind::Deref,
};
(res, use_cx.same_ctxt)
}

24
clippy_lints/src/methods/mod.rs
View File

@ -53,6 +53,7 @@
mod iterator_step_by_zero;
mod join_absolute_paths;
mod manual_c_str_literals;
mod manual_inspect;
mod manual_is_variant_and;
mod manual_next_back;
mod manual_ok_or;
@ -4079,6 +4080,27 @@
"is_ascii() called on a char iterator"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for uses of `map` which return the original item.
///
/// ### Why is this bad?
/// `inspect` is both clearer in intent and shorter.
///
/// ### Example
/// ```no_run
/// let x = Some(0).map(|x| { println!("{x}"); x });
/// ```
/// Use instead:
/// ```no_run
/// let x = Some(0).inspect(|x| println!("{x}"));
/// ```
#[clippy::version = "1.78.0"]
pub MANUAL_INSPECT,
complexity,
"use of `map` returning the original item"
}
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
@ -4244,6 +4266,7 @@ pub fn new(
MANUAL_C_STR_LITERALS,
UNNECESSARY_GET_THEN_CHECK,
NEEDLESS_CHARACTER_ITERATION,
MANUAL_INSPECT,
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -4747,6 +4770,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}
}
map_identity::check(cx, expr, recv, m_arg, name, span);
manual_inspect::check(cx, expr, m_arg, name, span, &self.msrv);
},
("map_or", [def, map]) => {
option_map_or_none::check(cx, expr, recv, def, map);

8
clippy_lints/src/needless_borrows_for_generic_args.rs
View File

@ -80,11 +80,13 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowsForGenericArgs<'tcx> {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if matches!(expr.kind, ExprKind::AddrOf(..))
&& !expr.span.from_expansion()
&& let Some(use_cx) = expr_use_ctxt(cx, expr)
&& let use_cx = expr_use_ctxt(cx, expr)
&& use_cx.same_ctxt
&& !use_cx.is_ty_unified
&& let Some(DefinedTy::Mir(ty)) = use_cx.node.defined_ty(cx)
&& let use_node = use_cx.use_node(cx)
&& let Some(DefinedTy::Mir(ty)) = use_node.defined_ty(cx)
&& let ty::Param(ty) = *ty.value.skip_binder().kind()
&& let Some((hir_id, fn_id, i)) = match use_cx.node {
&& let Some((hir_id, fn_id, i)) = match use_node {
ExprUseNode::MethodArg(_, _, 0) => None,
ExprUseNode::MethodArg(hir_id, None, i) => cx
.typeck_results()

164
clippy_utils/src/lib.rs
View File

@ -1,6 +1,7 @@
#![feature(array_chunks)]
#![feature(box_patterns)]
#![feature(control_flow_enum)]
#![feature(exhaustive_patterns)]
#![feature(if_let_guard)]
#![feature(let_chains)]
#![feature(lint_reasons)]
@ -2664,13 +2665,80 @@ pub enum DefinedTy<'tcx> {
/// The context an expressions value is used in.
pub struct ExprUseCtxt<'tcx> {
/// The parent node which consumes the value.
pub node: ExprUseNode<'tcx>,
pub node: Node<'tcx>,
/// The child id of the node the value came from.
pub child_id: HirId,
/// Any adjustments applied to the type.
pub adjustments: &'tcx [Adjustment<'tcx>],
/// Whether or not the type must unify with another code path.
/// Whether the type must unify with another code path.
pub is_ty_unified: bool,
/// Whether or not the value will be moved before it's used.
/// Whether the value will be moved before it's used.
pub moved_before_use: bool,
/// Whether the use site has the same `SyntaxContext` as the value.
pub same_ctxt: bool,
}
impl<'tcx> ExprUseCtxt<'tcx> {
pub fn use_node(&self, cx: &LateContext<'tcx>) -> ExprUseNode<'tcx> {
match self.node {
Node::LetStmt(l) => ExprUseNode::LetStmt(l),
Node::ExprField(field) => ExprUseNode::Field(field),
Node::Item(&Item {
kind: ItemKind::Static(..) | ItemKind::Const(..),
owner_id,
..
})
| Node::TraitItem(&TraitItem {
kind: TraitItemKind::Const(..),
owner_id,
..
})
| Node::ImplItem(&ImplItem {
kind: ImplItemKind::Const(..),
owner_id,
..
}) => ExprUseNode::ConstStatic(owner_id),
Node::Item(&Item {
kind: ItemKind::Fn(..),
owner_id,
..
})
| Node::TraitItem(&TraitItem {
kind: TraitItemKind::Fn(..),
owner_id,
..
})
| Node::ImplItem(&ImplItem {
kind: ImplItemKind::Fn(..),
owner_id,
..
}) => ExprUseNode::Return(owner_id),
Node::Expr(use_expr) => match use_expr.kind {
ExprKind::Ret(_) => ExprUseNode::Return(OwnerId {
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
}),
ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == self.child_id) {
Some(i) => ExprUseNode::FnArg(func, i),
None => ExprUseNode::Callee,
},
ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg(
use_expr.hir_id,
name.args,
args.iter()
.position(|arg| arg.hir_id == self.child_id)
.map_or(0, |i| i + 1),
),
ExprKind::Field(_, name) => ExprUseNode::FieldAccess(name),
ExprKind::AddrOf(kind, mutbl, _) => ExprUseNode::AddrOf(kind, mutbl),
_ => ExprUseNode::Other,
},
_ => ExprUseNode::Other,
}
}
}
/// The node which consumes a value.
@ -2691,7 +2759,8 @@ pub enum ExprUseNode<'tcx> {
Callee,
/// Access of a field.
FieldAccess(Ident),
Expr,
/// Borrow expression.
AddrOf(ast::BorrowKind, Mutability),
Other,
}
impl<'tcx> ExprUseNode<'tcx> {
@ -2768,26 +2837,25 @@ pub fn defined_ty(&self, cx: &LateContext<'tcx>) -> Option<DefinedTy<'tcx>> {
let sig = cx.tcx.fn_sig(id).skip_binder();
Some(DefinedTy::Mir(cx.tcx.param_env(id).and(sig.input(i))))
},
Self::LetStmt(_) | Self::FieldAccess(..) | Self::Callee | Self::Expr | Self::Other => None,
Self::LetStmt(_) | Self::FieldAccess(..) | Self::Callee | Self::Other | Self::AddrOf(..) => None,
}
}
}
/// Gets the context an expression's value is used in.
pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Option<ExprUseCtxt<'tcx>> {
pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> ExprUseCtxt<'tcx> {
let mut adjustments = [].as_slice();
let mut is_ty_unified = false;
let mut moved_before_use = false;
let mut same_ctxt = true;
let ctxt = e.span.ctxt();
walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| {
let node = walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| -> ControlFlow<!> {
if adjustments.is_empty()
&& let Node::Expr(e) = cx.tcx.hir_node(child_id)
{
adjustments = cx.typeck_results().expr_adjustments(e);
}
if cx.tcx.hir().span(parent_id).ctxt() != ctxt {
return ControlFlow::Break(());
}
same_ctxt &= cx.tcx.hir().span(parent_id).ctxt() == ctxt;
if let Node::Expr(e) = parent {
match e.kind {
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => {
@ -2803,71 +2871,25 @@ pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Optio
}
}
ControlFlow::Continue(())
})?
.continue_value()
.map(|(use_node, child_id)| {
let node = match use_node {
Node::LetStmt(l) => ExprUseNode::LetStmt(l),
Node::ExprField(field) => ExprUseNode::Field(field),
Node::Item(&Item {
kind: ItemKind::Static(..) | ItemKind::Const(..),
owner_id,
..
})
| Node::TraitItem(&TraitItem {
kind: TraitItemKind::Const(..),
owner_id,
..
})
| Node::ImplItem(&ImplItem {
kind: ImplItemKind::Const(..),
owner_id,
..
}) => ExprUseNode::ConstStatic(owner_id),
Node::Item(&Item {
kind: ItemKind::Fn(..),
owner_id,
..
})
| Node::TraitItem(&TraitItem {
kind: TraitItemKind::Fn(..),
owner_id,
..
})
| Node::ImplItem(&ImplItem {
kind: ImplItemKind::Fn(..),
owner_id,
..
}) => ExprUseNode::Return(owner_id),
Node::Expr(use_expr) => match use_expr.kind {
ExprKind::Ret(_) => ExprUseNode::Return(OwnerId {
def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
}),
ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == child_id) {
Some(i) => ExprUseNode::FnArg(func, i),
None => ExprUseNode::Callee,
},
ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg(
use_expr.hir_id,
name.args,
args.iter().position(|arg| arg.hir_id == child_id).map_or(0, |i| i + 1),
),
ExprKind::Field(child, name) if child.hir_id == e.hir_id => ExprUseNode::FieldAccess(name),
_ => ExprUseNode::Expr,
},
_ => ExprUseNode::Other,
};
ExprUseCtxt {
});
match node {
Some(ControlFlow::Continue((node, child_id))) => ExprUseCtxt {
node,
child_id,
adjustments,
is_ty_unified,
moved_before_use,
}
})
same_ctxt,
},
None => ExprUseCtxt {
node: Node::Crate(cx.tcx.hir().root_module()),
child_id: HirId::INVALID,
adjustments: &[],
is_ty_unified: true,
moved_before_use: true,
same_ctxt: false,
},
}
}
/// Tokenizes the input while keeping the text associated with each token.

27
clippy_utils/src/source.rs
View File

@ -14,8 +14,17 @@
use std::ops::Range;
/// A type which can be converted to the range portion of a `Span`.
pub trait SpanRange {
pub trait SpanRange: Sized {
fn into_range(self) -> Range<BytePos>;
fn set_span_pos(self, sp: Span) -> Span {
let range = self.into_range();
SpanData {
lo: range.start,
hi: range.end,
..sp.data()
}
.span()
}
}
impl SpanRange for Span {
fn into_range(self) -> Range<BytePos> {
@ -61,6 +70,22 @@ fn f(sm: &SourceMap, sp: Range<BytePos>) -> Option<SourceFileRange> {
f(cx.sess().source_map(), sp.into_range())
}
pub fn with_leading_whitespace(cx: &impl LintContext, sp: impl SpanRange) -> Range<BytePos> {
#[expect(clippy::needless_pass_by_value, clippy::cast_possible_truncation)]
fn f(src: SourceFileRange, sp: Range<BytePos>) -> Range<BytePos> {
let Some(text) = &src.sf.src else {
return sp;
};
let len = src.range.start - text[..src.range.start].trim_end().len();
BytePos(sp.start.0 - len as u32)..sp.end
}
let sp = sp.into_range();
match get_source_text(cx, sp.clone()) {
Some(src) => f(src, sp),
None => sp,
}
}
/// Like `snippet_block`, but add braces if the expr is not an `ExprKind::Block`.
pub fn expr_block<T: LintContext>(
cx: &T,

14
clippy_utils/src/ty.rs
View File

@ -1349,3 +1349,17 @@ pub fn get_adt_inherent_method<'a>(cx: &'a LateContext<'_>, ty: Ty<'_>, method_n
None
}
}
/// Get's the type of a field by name.
pub fn get_field_by_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, name: Symbol) -> Option<Ty<'tcx>> {
match *ty.kind() {
ty::Adt(def, args) if def.is_union() || def.is_struct() => def
.non_enum_variant()
.fields
.iter()
.find(|f| f.name == name)
.map(|f| f.ty(tcx, args)),
ty::Tuple(args) => name.as_str().parse::<usize>().ok().and_then(|i| args.get(i).copied()),
_ => None,
}
}

1
tests/ui/copy_iterator.rs
View File

@ -1,4 +1,5 @@
#![warn(clippy::copy_iterator)]
#![allow(clippy::manual_inspect)]
#[derive(Copy, Clone)]
struct Countdown(u8);

2
tests/ui/copy_iterator.stderr
View File

@ -1,5 +1,5 @@
error: you are implementing `Iterator` on a `Copy` type
--> tests/ui/copy_iterator.rs:6:1
--> tests/ui/copy_iterator.rs:7:1
|
LL | / impl Iterator for Countdown {
LL | |

174
tests/ui/manual_inspect.fixed Normal file
View File

@ -0,0 +1,174 @@
#![warn(clippy::manual_inspect)]
#![allow(clippy::no_effect, clippy::op_ref)]
fn main() {
let _ = Some(0).inspect(|&x| {
println!("{}", x);
});
let _ = Some(0).inspect(|&x| {
println!("{x}");
});
let _ = Some(0).inspect(|&x| {
println!("{}", x * 5 + 1);
});
let _ = Some(0).inspect(|&x| {
if x == 0 {
panic!();
}
});
let _ = Some(0).inspect(|&x| {
if &x == &0 {
let _y = x;
panic!();
}
});
let _ = Some(0).map(|x| {
let y = x + 1;
if y > 5 {
return y;
}
x
});
{
#[derive(PartialEq)]
struct Foo(i32);
let _ = Some(Foo(0)).map(|x| {
if x == Foo(0) {
panic!();
}
x
});
let _ = Some(Foo(0)).map(|x| {
if &x == &Foo(0) {
let _y = x;
panic!();
}
x
});
}
{
macro_rules! maybe_ret {
($e:expr) => {
if $e == 0 {
return $e;
}
};
}
let _ = Some(0).map(|x| {
maybe_ret!(x);
x
});
}
let _ = Some((String::new(), 0u32)).inspect(|x| {
if x.1 == 0 {
let _x = x.1;
panic!();
}
});
let _ = Some((String::new(), 0u32)).map(|x| {
if x.1 == 0 {
let _x = x.0;
panic!();
}
x
});
let _ = Some(String::new()).map(|x| {
if x.is_empty() {
let _ = || {
let _x = x;
};
panic!();
}
x
});
let _ = Some(String::new()).inspect(|x| {
if x.is_empty() {
let _ = || {
let _x = x;
};
return;
}
println!("test");
});
let _ = Some(0).inspect(|&x| {
if x == 0 {
let _ = || {
let _x = x;
};
panic!();
}
});
{
use core::cell::Cell;
#[derive(Debug)]
struct Cell2(core::cell::Cell<u32>);
let _ = Some(Cell2(Cell::new(0u32))).inspect(|x| {
x.0.set(1);
});
let _ = Some(Cell2(Cell::new(0u32))).map(|x| {
let y = &x;
if x.0.get() == 0 {
y.0.set(1)
} else {
println!("{x:?}");
}
x
});
}
let _: Result<_, ()> = Ok(0).inspect(|&x| {
println!("{}", x);
});
let _: Result<(), _> = Err(0).inspect_err(|&x| {
println!("{}", x);
});
let _ = [0]
.into_iter()
.inspect(|&x| {
println!("{}", x);
})
.count();
{
struct S<T>(T);
impl<T> S<T> {
fn map<U>(self, f: impl FnOnce(T) -> U) -> S<U> {
S(f(self.0))
}
fn map_err<U>(self, f: impl FnOnce(T) -> U) -> S<U> {
S(f(self.0))
}
}
let _ = S(0).map(|x| {
println!("{}", x);
x
});
let _ = S(0).map_err(|x| {
println!("{}", x);
x
});
}
}

186
tests/ui/manual_inspect.rs Normal file
View File

@ -0,0 +1,186 @@
#![warn(clippy::manual_inspect)]
#![allow(clippy::no_effect, clippy::op_ref)]
fn main() {
let _ = Some(0).map(|x| {
println!("{}", x);
x
});
let _ = Some(0).map(|x| {
println!("{x}");
x
});
let _ = Some(0).map(|x| {
println!("{}", x * 5 + 1);
x
});
let _ = Some(0).map(|x| {
if x == 0 {
panic!();
}
x
});
let _ = Some(0).map(|x| {
if &x == &0 {
let _y = x;
panic!();
}
x
});
let _ = Some(0).map(|x| {
let y = x + 1;
if y > 5 {
return y;
}
x
});
{
#[derive(PartialEq)]
struct Foo(i32);
let _ = Some(Foo(0)).map(|x| {
if x == Foo(0) {
panic!();
}
x
});
let _ = Some(Foo(0)).map(|x| {
if &x == &Foo(0) {
let _y = x;
panic!();
}
x
});
}
{
macro_rules! maybe_ret {
($e:expr) => {
if $e == 0 {
return $e;
}
};
}
let _ = Some(0).map(|x| {
maybe_ret!(x);
x
});
}
let _ = Some((String::new(), 0u32)).map(|x| {
if x.1 == 0 {
let _x = x.1;
panic!();
}
x
});
let _ = Some((String::new(), 0u32)).map(|x| {
if x.1 == 0 {
let _x = x.0;
panic!();
}
x
});
let _ = Some(String::new()).map(|x| {
if x.is_empty() {
let _ = || {
let _x = x;
};
panic!();
}
x
});
let _ = Some(String::new()).map(|x| {
if x.is_empty() {
let _ = || {
let _x = &x;
};
return x;
}
println!("test");
x
});
let _ = Some(0).map(|x| {
if x == 0 {
let _ = || {
let _x = x;
};
panic!();
}
x
});
{
use core::cell::Cell;
#[derive(Debug)]
struct Cell2(core::cell::Cell<u32>);
let _ = Some(Cell2(Cell::new(0u32))).map(|x| {
x.0.set(1);
x
});
let _ = Some(Cell2(Cell::new(0u32))).map(|x| {
let y = &x;
if x.0.get() == 0 {
y.0.set(1)
} else {
println!("{x:?}");
}
x
});
}
let _: Result<_, ()> = Ok(0).map(|x| {
println!("{}", x);
x
});
let _: Result<(), _> = Err(0).map_err(|x| {
println!("{}", x);
x
});
let _ = [0]
.into_iter()
.map(|x| {
println!("{}", x);
x
})
.count();
{
struct S<T>(T);
impl<T> S<T> {
fn map<U>(self, f: impl FnOnce(T) -> U) -> S<U> {
S(f(self.0))
}
fn map_err<U>(self, f: impl FnOnce(T) -> U) -> S<U> {
S(f(self.0))
}
}
let _ = S(0).map(|x| {
println!("{}", x);
x
});
let _ = S(0).map_err(|x| {
println!("{}", x);
x
});
}
}

182
tests/ui/manual_inspect.stderr Normal file
View File

@ -0,0 +1,182 @@
error:
--> tests/ui/manual_inspect.rs:5:21
|
LL | let _ = Some(0).map(|x| {
| ^^^
|
= note: `-D clippy::manual-inspect` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_inspect)]`
help: try
|
LL ~ let _ = Some(0).inspect(|&x| {
LL ~ println!("{}", x);
|
error:
--> tests/ui/manual_inspect.rs:10:21
|
LL | let _ = Some(0).map(|x| {
| ^^^
|
help: try
|
LL ~ let _ = Some(0).inspect(|&x| {
LL ~ println!("{x}");
|
error:
--> tests/ui/manual_inspect.rs:15:21
|
LL | let _ = Some(0).map(|x| {
| ^^^
|
help: try
|
LL ~ let _ = Some(0).inspect(|&x| {
LL ~ println!("{}", x * 5 + 1);
|
error:
--> tests/ui/manual_inspect.rs:20:21
|
LL | let _ = Some(0).map(|x| {
| ^^^
|
help: try
|
LL ~ let _ = Some(0).inspect(|&x| {
LL | if x == 0 {
LL | panic!();
LL ~ }
|
error:
--> tests/ui/manual_inspect.rs:27:21
|
LL | let _ = Some(0).map(|x| {
| ^^^
|
help: try
|
LL ~ let _ = Some(0).inspect(|&x| {
LL | if &x == &0 {
LL | let _y = x;
LL | panic!();
LL ~ }
|
error:
--> tests/ui/manual_inspect.rs:78:41
|
LL | let _ = Some((String::new(), 0u32)).map(|x| {
| ^^^
|
help: try
|
LL ~ let _ = Some((String::new(), 0u32)).inspect(|x| {
LL | if x.1 == 0 {
LL | let _x = x.1;
LL | panic!();
LL ~ }
|
error:
--> tests/ui/manual_inspect.rs:104:33
|
LL | let _ = Some(String::new()).map(|x| {
| ^^^
|
help: try
|
LL ~ let _ = Some(String::new()).inspect(|x| {
LL | if x.is_empty() {
LL | let _ = || {
LL ~ let _x = x;
LL | };
LL ~ return;
LL | }
LL ~ println!("test");
|
error:
--> tests/ui/manual_inspect.rs:115:21
|
LL | let _ = Some(0).map(|x| {
| ^^^
|
help: try
|
LL ~ let _ = Some(0).inspect(|&x| {
LL | if x == 0 {
...
LL | panic!();
LL ~ }
|
error:
--> tests/ui/manual_inspect.rs:130:46
|
LL | let _ = Some(Cell2(Cell::new(0u32))).map(|x| {
| ^^^
|
help: try
|
LL ~ let _ = Some(Cell2(Cell::new(0u32))).inspect(|x| {
LL ~ x.0.set(1);
|
error:
--> tests/ui/manual_inspect.rs:146:34
|
LL | let _: Result<_, ()> = Ok(0).map(|x| {
| ^^^
|
help: try
|
LL ~ let _: Result<_, ()> = Ok(0).inspect(|&x| {
LL ~ println!("{}", x);
|
error:
--> tests/ui/manual_inspect.rs:151:35
|
LL | let _: Result<(), _> = Err(0).map_err(|x| {
| ^^^^^^^
|
help: try
|
LL ~ let _: Result<(), _> = Err(0).inspect_err(|&x| {
LL ~ println!("{}", x);
|
error: this call to `map()` won't have an effect on the call to `count()`
--> tests/ui/manual_inspect.rs:156:13
|
LL | let _ = [0]
| _____________^
LL | | .into_iter()
LL | | .map(|x| {
LL | | println!("{}", x);
LL | | x
LL | | })
LL | | .count();
| |________________^
|
= help: make sure you did not confuse `map` with `filter`, `for_each` or `inspect`
= note: `-D clippy::suspicious-map` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::suspicious_map)]`
error:
--> tests/ui/manual_inspect.rs:158:10
|
LL | .map(|x| {
| ^^^
|
help: try
|
LL ~ .inspect(|&x| {
LL ~ println!("{}", x);
|
error: aborting due to 13 previous errors