From 19f5b853308d7504a95a5ee27d6c3adfe14d4554 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 1 Dec 2023 16:57:18 +0100 Subject: [PATCH] Add `write_and_append` lint --- clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/ineffective_open_options.rs | 95 ++++++++++++++++++++ clippy_lints/src/lib.rs | 2 + clippy_lints/src/methods/mod.rs | 2 +- 4 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/ineffective_open_options.rs diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index a9ae22e8d43..d3105ed9d18 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -215,6 +215,7 @@ crate::index_refutable_slice::INDEX_REFUTABLE_SLICE_INFO, crate::indexing_slicing::INDEXING_SLICING_INFO, crate::indexing_slicing::OUT_OF_BOUNDS_INDEXING_INFO, + crate::ineffective_open_options::INEFFECTIVE_OPEN_OPTIONS_INFO, crate::infinite_iter::INFINITE_ITER_INFO, crate::infinite_iter::MAYBE_INFINITE_ITER_INFO, crate::inherent_impl::MULTIPLE_INHERENT_IMPL_INFO, diff --git a/clippy_lints/src/ineffective_open_options.rs b/clippy_lints/src/ineffective_open_options.rs new file mode 100644 index 00000000000..955f90d4262 --- /dev/null +++ b/clippy_lints/src/ineffective_open_options.rs @@ -0,0 +1,95 @@ +use crate::methods::method_call; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::peel_blocks; +use rustc_ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_session::declare_lint_pass; +use rustc_span::{sym, BytePos, Span}; + +declare_clippy_lint! { + /// ### What it does + /// Checks if both `.write(true)` and `.append(true)` methods are called + /// on a same `OpenOptions`. + /// + /// ### Why is this bad? + /// `.append(true)` already enables `write(true)`, making this one + /// superflous. + /// + /// ### Example + /// ```no_run + /// # use std::fs::OpenOptions; + /// let _ = OpenOptions::new() + /// .write(true) + /// .append(true) + /// .create(true) + /// .open("file.json"); + /// ``` + /// Use instead: + /// ```no_run + /// # use std::fs::OpenOptions; + /// let _ = OpenOptions::new() + /// .append(true) + /// .create(true) + /// .open("file.json"); + /// ``` + #[clippy::version = "1.76.0"] + pub INEFFECTIVE_OPEN_OPTIONS, + suspicious, + "usage of both `write(true)` and `append(true)` on same `OpenOptions`" +} + +declare_lint_pass!(IneffectiveOpenOptions => [INEFFECTIVE_OPEN_OPTIONS]); + +fn index_if_arg_is_boolean(args: &[Expr<'_>], call_span: Span) -> Option { + if let [arg] = args + && let ExprKind::Lit(lit) = peel_blocks(arg).kind + && lit.node == LitKind::Bool(true) + { + // The `.` is not included in the span so we cheat a little bit to include it as well. + Some(call_span.with_lo(call_span.lo() - BytePos(1))) + } else { + None + } +} + +impl<'tcx> LateLintPass<'tcx> for IneffectiveOpenOptions { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + let Some(("open", mut receiver, [_arg], _, _)) = method_call(expr) else { + return; + }; + let receiver_ty = cx.typeck_results().expr_ty(receiver); + match receiver_ty.peel_refs().kind() { + ty::Adt(adt, _) if cx.tcx.is_diagnostic_item(sym::FsOpenOptions, adt.did()) => {}, + _ => return, + } + + let mut append = None; + let mut write = None; + + while let Some((name, recv, args, _, span)) = method_call(receiver) { + if name == "append" { + append = index_if_arg_is_boolean(args, span); + } else if name == "write" { + write = index_if_arg_is_boolean(args, span); + } + receiver = recv; + } + + if let Some(write_span) = write + && append.is_some() + { + span_lint_and_sugg( + cx, + INEFFECTIVE_OPEN_OPTIONS, + write_span, + "unnecessary use of `.write(true)` because there is `.append(true)`", + "remove `.write(true)`", + String::new(), + Applicability::MachineApplicable, + ); + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 660d3c0bdd2..e4ff7ebb0f7 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -153,6 +153,7 @@ mod inconsistent_struct_constructor; mod index_refutable_slice; mod indexing_slicing; +mod ineffective_open_options; mod infinite_iter; mod inherent_impl; mod inherent_to_string; @@ -1073,6 +1074,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(impl_hash_with_borrow_str_and_bytes::ImplHashWithBorrowStrBytes)); store.register_late_pass(|_| Box::new(repeat_vec_with_capacity::RepeatVecWithCapacity)); store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences)); + store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 25c681bb9d9..b4f60ffadd7 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3906,7 +3906,7 @@ pub fn new( ]); /// Extracts a method call name, args, and `Span` of the method name. -fn method_call<'tcx>( +pub fn method_call<'tcx>( recv: &'tcx hir::Expr<'tcx>, ) -> Option<(&'tcx str, &'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>], Span, Span)> { if let ExprKind::MethodCall(path, receiver, args, call_span) = recv.kind {