From 56e8e1a27da1a204a5ed6ace2132c09abe49178b Mon Sep 17 00:00:00 2001 From: NotAPenguin Date: Mon, 8 May 2023 13:20:33 +0200 Subject: [PATCH] new lint: clippy::ref_patterns --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 ++ clippy_lints/src/misc.rs | 11 +++++++- clippy_lints/src/ref_patterns.rs | 44 ++++++++++++++++++++++++++++++ tests/ui/ref_patterns.rs | 19 +++++++++++++ tests/ui/ref_patterns.stderr | 27 ++++++++++++++++++ 7 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/ref_patterns.rs create mode 100644 tests/ui/ref_patterns.rs create mode 100644 tests/ui/ref_patterns.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index ebf5b58a586..85d451a87d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4978,6 +4978,7 @@ Released 2018-09-13 [`ref_binding_to_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_binding_to_reference [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref [`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref +[`ref_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_patterns [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro [`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 79d0f6f3607..6614b99713a 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -539,6 +539,7 @@ crate::redundant_slicing::REDUNDANT_SLICING_INFO, crate::redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES_INFO, crate::ref_option_ref::REF_OPTION_REF_INFO, + crate::ref_patterns::REF_PATTERNS_INFO, crate::reference::DEREF_ADDROF_INFO, crate::regex::INVALID_REGEX_INFO, crate::regex::TRIVIAL_REGEX_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 3517842a01e..766a701fad8 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -266,6 +266,7 @@ mod redundant_slicing; mod redundant_static_lifetimes; mod ref_option_ref; +mod ref_patterns; mod reference; mod regex; mod return_self_not_must_use; @@ -971,6 +972,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation)); store.register_early_pass(|| Box::new(suspicious_doc_comments::SuspiciousDocComments)); store.register_late_pass(|_| Box::new(items_after_test_module::ItemsAfterTestModule)); + store.register_early_pass(|| Box::new(ref_patterns::RefPatterns)); store.register_late_pass(|_| Box::new(default_constructed_unit_structs::DefaultConstructedUnitStructs)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 3752b9a946f..303f0125690 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -16,9 +16,12 @@ use clippy_utils::sugg::Sugg; use clippy_utils::{ - get_parent_expr, in_constant, is_integer_literal, is_no_std_crate, iter_input_pats, last_path_segment, SpanlessEq, + get_parent_expr, in_constant, is_integer_literal, is_lint_allowed, is_no_std_crate, iter_input_pats, + last_path_segment, SpanlessEq, }; +use crate::ref_patterns::REF_PATTERNS; + declare_clippy_lint! { /// ### What it does /// Checks for function arguments and let bindings denoted as @@ -162,6 +165,10 @@ fn check_fn( return; } for arg in iter_input_pats(decl, body) { + // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue. + if !is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id) { + return; + } if let PatKind::Binding(BindingAnnotation(ByRef::Yes, _), ..) = arg.pat.kind { span_lint( cx, @@ -180,6 +187,8 @@ fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { if let StmtKind::Local(local) = stmt.kind; if let PatKind::Binding(BindingAnnotation(ByRef::Yes, mutabl), .., name, None) = local.pat.kind; if let Some(init) = local.init; + // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue. + if is_lint_allowed(cx, REF_PATTERNS, local.pat.hir_id); then { let ctxt = local.span.ctxt(); let mut app = Applicability::MachineApplicable; diff --git a/clippy_lints/src/ref_patterns.rs b/clippy_lints/src/ref_patterns.rs new file mode 100644 index 00000000000..b1530eed1c1 --- /dev/null +++ b/clippy_lints/src/ref_patterns.rs @@ -0,0 +1,44 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use rustc_ast::ast::{BindingAnnotation, Pat, PatKind}; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for usages of the `ref` keyword. + /// ### Why is this bad? + /// The `ref` keyword can be confusing for people unfamiliar with it, and often + /// it is more concise to use `&` instead. + /// ### Example + /// ```rust + /// let opt = Some(5); + /// if let Some(ref foo) = opt {} + /// ``` + /// Use instead: + /// ```rust + /// let opt = Some(5); + /// if let Some(foo) = &opt {} + /// ``` + #[clippy::version = "1.71.0"] + pub REF_PATTERNS, + restriction, + "use of a ref pattern, e.g. Some(ref value)" +} +declare_lint_pass!(RefPatterns => [REF_PATTERNS]); + +impl EarlyLintPass for RefPatterns { + fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat) { + if let PatKind::Ident(BindingAnnotation::REF, _, _) = pat.kind + && !pat.span.from_expansion() + { + span_lint_and_help( + cx, + REF_PATTERNS, + pat.span, + "usage of ref pattern", + None, + "consider using `&` for clarity instead", + ); + } + } +} diff --git a/tests/ui/ref_patterns.rs b/tests/ui/ref_patterns.rs new file mode 100644 index 00000000000..c51e0bc76ef --- /dev/null +++ b/tests/ui/ref_patterns.rs @@ -0,0 +1,19 @@ +#![allow(unused)] +#![warn(clippy::ref_patterns)] + +fn use_in_pattern() { + let opt = Some(5); + match opt { + None => {}, + Some(ref opt) => {}, + } +} + +fn use_in_binding() { + let x = 5; + let ref y = x; +} + +fn use_in_parameter(ref x: i32) {} + +fn main() {} diff --git a/tests/ui/ref_patterns.stderr b/tests/ui/ref_patterns.stderr new file mode 100644 index 00000000000..aa007782683 --- /dev/null +++ b/tests/ui/ref_patterns.stderr @@ -0,0 +1,27 @@ +error: usage of ref pattern + --> $DIR/ref_patterns.rs:8:14 + | +LL | Some(ref opt) => {}, + | ^^^^^^^ + | + = help: consider using `&` for clarity instead + = note: `-D clippy::ref-patterns` implied by `-D warnings` + +error: usage of ref pattern + --> $DIR/ref_patterns.rs:14:9 + | +LL | let ref y = x; + | ^^^^^ + | + = help: consider using `&` for clarity instead + +error: usage of ref pattern + --> $DIR/ref_patterns.rs:17:21 + | +LL | fn use_in_parameter(ref x: i32) {} + | ^^^^^ + | + = help: consider using `&` for clarity instead + +error: aborting due to 3 previous errors +