Skip to content

initial clippy::ref_patterns implementation #10736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ mod redundant_pub_crate;
mod redundant_slicing;
mod redundant_static_lifetimes;
mod ref_option_ref;
mod ref_patterns;
mod reference;
mod regex;
mod return_self_not_must_use;
Expand Down Expand Up @@ -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`
}
Expand Down
11 changes: 10 additions & 1 deletion clippy_lints/src/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ use rustc_span::source_map::{ExpnKind, Span};

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
Expand Down Expand Up @@ -162,6 +165,10 @@ impl<'tcx> LateLintPass<'tcx> for LintPass {
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,
Expand All @@ -180,6 +187,8 @@ impl<'tcx> LateLintPass<'tcx> for LintPass {
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;
Expand Down
44 changes: 44 additions & 0 deletions clippy_lints/src/ref_patterns.rs
Original file line number Diff line number Diff line change
@@ -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",
);
}
}
}
19 changes: 19 additions & 0 deletions tests/ui/ref_patterns.rs
Original file line number Diff line number Diff line change
@@ -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() {}
27 changes: 27 additions & 0 deletions tests/ui/ref_patterns.stderr
Original file line number Diff line number Diff line change
@@ -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