rust/clippy_lints/src/collection_is_never_read.rs
2024-06-13 12:30:48 +02:00

160 lines
6.2 KiB
Rust

use clippy_utils::diagnostics::span_lint;
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
use clippy_utils::visitors::{for_each_expr, Visitable};
use clippy_utils::{get_enclosing_block, path_to_local_id};
use core::ops::ControlFlow;
use rustc_hir::{Body, ExprKind, HirId, LangItem, LetStmt, Node, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::symbol::sym;
use rustc_span::Symbol;
declare_clippy_lint! {
/// ### What it does
/// Checks for collections that are never queried.
///
/// ### Why is this bad?
/// Putting effort into constructing a collection but then never querying it might indicate that
/// the author forgot to do whatever they intended to do with the collection. Example: Clone
/// a vector, sort it for iteration, but then mistakenly iterate the original vector
/// instead.
///
/// ### Example
/// ```no_run
/// # let samples = vec![3, 1, 2];
/// let mut sorted_samples = samples.clone();
/// sorted_samples.sort();
/// for sample in &samples { // Oops, meant to use `sorted_samples`.
/// println!("{sample}");
/// }
/// ```
/// Use instead:
/// ```no_run
/// # let samples = vec![3, 1, 2];
/// let mut sorted_samples = samples.clone();
/// sorted_samples.sort();
/// for sample in &sorted_samples {
/// println!("{sample}");
/// }
/// ```
#[clippy::version = "1.70.0"]
pub COLLECTION_IS_NEVER_READ,
nursery,
"a collection is never queried"
}
declare_lint_pass!(CollectionIsNeverRead => [COLLECTION_IS_NEVER_READ]);
// Add `String` here when it is added to diagnostic items
static COLLECTIONS: [Symbol; 9] = [
sym::BTreeMap,
sym::BTreeSet,
sym::BinaryHeap,
sym::HashMap,
sym::HashSet,
sym::LinkedList,
sym::Option,
sym::Vec,
sym::VecDeque,
];
impl<'tcx> LateLintPass<'tcx> for CollectionIsNeverRead {
fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx LetStmt<'tcx>) {
// Look for local variables whose type is a container. Search surrounding bock for read access.
if match_acceptable_type(cx, local, &COLLECTIONS)
&& let PatKind::Binding(_, local_id, _, _) = local.pat.kind
&& let Some(enclosing_block) = get_enclosing_block(cx, local.hir_id)
&& has_no_read_access(cx, local_id, enclosing_block)
{
span_lint(cx, COLLECTION_IS_NEVER_READ, local.span, "collection is never read");
}
}
}
fn match_acceptable_type(cx: &LateContext<'_>, local: &LetStmt<'_>, collections: &[Symbol]) -> bool {
let ty = cx.typeck_results().pat_ty(local.pat);
collections.iter().any(|&sym| is_type_diagnostic_item(cx, ty, sym))
// String type is a lang item but not a diagnostic item for now so we need a separate check
|| is_type_lang_item(cx, ty, LangItem::String)
}
fn has_no_read_access<'tcx, T: Visitable<'tcx>>(cx: &LateContext<'tcx>, id: HirId, block: T) -> bool {
let mut has_access = false;
let mut has_read_access = false;
// Inspect all expressions and sub-expressions in the block.
for_each_expr(cx, block, |expr| {
// Ignore expressions that are not simply `id`.
if !path_to_local_id(expr, id) {
return ControlFlow::Continue(());
}
// `id` is being accessed. Investigate if it's a read access.
has_access = true;
// `id` appearing in the left-hand side of an assignment is not a read access:
//
// id = ...; // Not reading `id`.
if let Node::Expr(parent) = cx.tcx.parent_hir_node(expr.hir_id)
&& let ExprKind::Assign(lhs, ..) = parent.kind
&& path_to_local_id(lhs, id)
{
return ControlFlow::Continue(());
}
// Look for method call with receiver `id`. It might be a non-read access:
//
// id.foo(args)
//
// Only assuming this for "official" methods defined on the type. For methods defined in extension
// traits (identified as local, based on the orphan rule), pessimistically assume that they might
// have side effects, so consider them a read.
if let Node::Expr(parent) = cx.tcx.parent_hir_node(expr.hir_id)
&& let ExprKind::MethodCall(_, receiver, args, _) = parent.kind
&& path_to_local_id(receiver, id)
&& let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(parent.hir_id)
&& !method_def_id.is_local()
{
// If this "official" method takes closures,
// it has read access if one of the closures has read access.
//
// items.retain(|item| send_item(item).is_ok());
let is_read_in_closure_arg = args.iter().any(|arg| {
if let ExprKind::Closure(closure) = arg.kind
// To keep things simple, we only check the first param to see if its read.
&& let Body { params: [param, ..], value } = cx.tcx.hir().body(closure.body)
{
!has_no_read_access(cx, param.hir_id, *value)
} else {
false
}
});
if is_read_in_closure_arg {
has_read_access = true;
return ControlFlow::Break(());
}
// The method call is a statement, so the return value is not used. That's not a read access:
//
// id.foo(args);
if let Node::Stmt(..) = cx.tcx.parent_hir_node(parent.hir_id) {
return ControlFlow::Continue(());
}
// The method call is not a statement, so its return value is used somehow but its type is the
// unit type, so this is not a real read access. Examples:
//
// let y = x.clear();
// println!("{:?}", x.clear());
if cx.typeck_results().expr_ty(parent).is_unit() {
return ControlFlow::Continue(());
}
}
// Any other access to `id` is a read access. Stop searching.
has_read_access = true;
ControlFlow::Break(())
});
// Ignore collections that have no access at all. Other lints should catch them.
has_access && !has_read_access
}