Skip to content

Refactor lints in methods module #6896

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 19 commits into from
Mar 22, 2021
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
54 changes: 54 additions & 0 deletions clippy_lints/src/methods/chars_cmp.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{method_chain_args, single_segment_path};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_lint::Lint;
use rustc_middle::ty;
use rustc_span::sym;

/// Wrapper fn for `CHARS_NEXT_CMP` and `CHARS_LAST_CMP` lints.
pub(super) fn check(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if chars_last_cmp::check and chars_next_cmp::check should be merged into this because they are almost the same and depend on each other. I think separating them in this way decreases readability and understandability. This also applies to chars_cmp_with_unwrap.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you. I'm going to merge chars_last_cmp and chars_next_cmp and merge chars_next_cmp_with_unwrap and chars_last_cmp_with_unwrap.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to do it in a new PR.

cx: &LateContext<'_>,
info: &crate::methods::BinaryExprInfo<'_>,
chain_methods: &[&str],
lint: &'static Lint,
suggest: &str,
) -> bool {
if_chain! {
if let Some(args) = method_chain_args(info.chain, chain_methods);
if let hir::ExprKind::Call(ref fun, ref arg_char) = info.other.kind;
if arg_char.len() == 1;
if let hir::ExprKind::Path(ref qpath) = fun.kind;
if let Some(segment) = single_segment_path(qpath);
if segment.ident.name == sym::Some;
then {
let mut applicability = Applicability::MachineApplicable;
let self_ty = cx.typeck_results().expr_ty_adjusted(&args[0][0]).peel_refs();

if *self_ty.kind() != ty::Str {
return false;
}

span_lint_and_sugg(
cx,
lint,
info.expr.span,
&format!("you should use the `{}` method", suggest),
"like this",
format!("{}{}.{}({})",
if info.eq { "" } else { "!" },
snippet_with_applicability(cx, args[0][0].span, "..", &mut applicability),
suggest,
snippet_with_applicability(cx, arg_char[0].span, "..", &mut applicability)),
applicability,
);

return true;
}
}

false
}
44 changes: 44 additions & 0 deletions clippy_lints/src/methods/chars_cmp_with_unwrap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::method_chain_args;
use clippy_utils::source::snippet_with_applicability;
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_lint::Lint;

/// Wrapper fn for `CHARS_NEXT_CMP` and `CHARS_LAST_CMP` lints with `unwrap()`.
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
info: &crate::methods::BinaryExprInfo<'_>,
chain_methods: &[&str],
lint: &'static Lint,
suggest: &str,
) -> bool {
if_chain! {
if let Some(args) = method_chain_args(info.chain, chain_methods);
if let hir::ExprKind::Lit(ref lit) = info.other.kind;
if let ast::LitKind::Char(c) = lit.node;
then {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
lint,
info.expr.span,
&format!("you should use the `{}` method", suggest),
"like this",
format!("{}{}.{}('{}')",
if info.eq { "" } else { "!" },
snippet_with_applicability(cx, args[0][0].span, "..", &mut applicability),
suggest,
c),
applicability,
);

true
} else {
false
}
}
}
13 changes: 13 additions & 0 deletions clippy_lints/src/methods/chars_last_cmp.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use crate::methods::chars_cmp;
use rustc_lint::LateContext;

use super::CHARS_LAST_CMP;

/// Checks for the `CHARS_LAST_CMP` lint.
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, info: &crate::methods::BinaryExprInfo<'_>) -> bool {
if chars_cmp::check(cx, info, &["chars", "last"], CHARS_LAST_CMP, "ends_with") {
true
} else {
chars_cmp::check(cx, info, &["chars", "next_back"], CHARS_LAST_CMP, "ends_with")
}
}
13 changes: 13 additions & 0 deletions clippy_lints/src/methods/chars_last_cmp_with_unwrap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use crate::methods::chars_cmp_with_unwrap;
use rustc_lint::LateContext;

use super::CHARS_LAST_CMP;

/// Checks for the `CHARS_LAST_CMP` lint with `unwrap()`.
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, info: &crate::methods::BinaryExprInfo<'_>) -> bool {
if chars_cmp_with_unwrap::check(cx, info, &["chars", "last", "unwrap"], CHARS_LAST_CMP, "ends_with") {
true
} else {
chars_cmp_with_unwrap::check(cx, info, &["chars", "next_back", "unwrap"], CHARS_LAST_CMP, "ends_with")
}
}
8 changes: 8 additions & 0 deletions clippy_lints/src/methods/chars_next_cmp.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
use rustc_lint::LateContext;

use super::CHARS_NEXT_CMP;

/// Checks for the `CHARS_NEXT_CMP` lint.
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, info: &crate::methods::BinaryExprInfo<'_>) -> bool {
crate::methods::chars_cmp::check(cx, info, &["chars", "next"], CHARS_NEXT_CMP, "starts_with")
}
8 changes: 8 additions & 0 deletions clippy_lints/src/methods/chars_next_cmp_with_unwrap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
use rustc_lint::LateContext;

use super::CHARS_NEXT_CMP;

/// Checks for the `CHARS_NEXT_CMP` lint with `unwrap()`.
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, info: &crate::methods::BinaryExprInfo<'_>) -> bool {
crate::methods::chars_cmp_with_unwrap::check(cx, info, &["chars", "next", "unwrap"], CHARS_NEXT_CMP, "starts_with")
}
10 changes: 8 additions & 2 deletions clippy_lints/src/methods/clone_on_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,20 @@ use clippy_utils::ty::is_copy;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty;
use rustc_span::symbol::{sym, Symbol};
use std::iter;

use super::CLONE_DOUBLE_REF;
use super::CLONE_ON_COPY;

/// Checks for the `CLONE_ON_COPY` lint.
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, arg_ty: Ty<'_>) {
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_name: Symbol, args: &[hir::Expr<'_>]) {
if !(args.len() == 1 && method_name == sym::clone) {
return;
}
let arg = &args[0];
let arg_ty = cx.typeck_results().expr_ty_adjusted(&args[0]);
let ty = cx.typeck_results().expr_ty(expr);
if let ty::Ref(_, inner, _) = arg_ty.kind() {
if let ty::Ref(_, innermost, _) = inner.kind() {
Expand Down
8 changes: 6 additions & 2 deletions clippy_lints/src/methods/clone_on_ref_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@ use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::symbol::sym;
use rustc_span::symbol::{sym, Symbol};

use super::CLONE_ON_REF_PTR;

pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>) {
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_name: Symbol, args: &[hir::Expr<'_>]) {
if !(args.len() == 1 && method_name == sym::clone) {
return;
}
let arg = &args[0];
let obj_ty = cx.typeck_results().expr_ty(arg).peel_refs();

if let ty::Adt(_, subst) = obj_ty.kind() {
Expand Down
7 changes: 1 addition & 6 deletions clippy_lints/src/methods/filter_flat_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@ use rustc_span::sym;
use super::FILTER_MAP;

/// lint use of `filter().flat_map()` for `Iterators`
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
_filter_args: &'tcx [hir::Expr<'_>],
_map_args: &'tcx [hir::Expr<'_>],
) {
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
// lint if caller of `.filter().flat_map()` is an Iterator
if is_trait_method(cx, expr, sym::Iterator) {
let msg = "called `filter(..).flat_map(..)` on an `Iterator`";
Expand Down
7 changes: 1 addition & 6 deletions clippy_lints/src/methods/filter_map_flat_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@ use rustc_span::sym;
use super::FILTER_MAP;

/// lint use of `filter_map().flat_map()` for `Iterators`
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
_filter_args: &'tcx [hir::Expr<'_>],
_map_args: &'tcx [hir::Expr<'_>],
) {
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
// lint if caller of `.filter_map().flat_map()` is an Iterator
if is_trait_method(cx, expr, sym::Iterator) {
let msg = "called `filter_map(..).flat_map(..)` on an `Iterator`";
Expand Down
7 changes: 1 addition & 6 deletions clippy_lints/src/methods/filter_map_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@ use rustc_span::sym;
use super::FILTER_MAP;

/// lint use of `filter_map().map()` for `Iterators`
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
_filter_args: &'tcx [hir::Expr<'_>],
_map_args: &'tcx [hir::Expr<'_>],
) {
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
// lint if caller of `.filter_map().map()` is an Iterator
if is_trait_method(cx, expr, sym::Iterator) {
let msg = "called `filter_map(..).map(..)` on an `Iterator`";
Expand Down
12 changes: 7 additions & 5 deletions clippy_lints/src/methods/from_iter_instead_of_collect.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::ty::implements_trait;
use clippy_utils::{get_trait_def_id, paths, sugg};
use clippy_utils::{get_trait_def_id, match_qpath, paths, sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::ExprKind;
use rustc_lint::{LateContext, LintContext};
use rustc_middle::ty::Ty;
use rustc_span::sym;

use super::FROM_ITER_INSTEAD_OF_COLLECT;

pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
let ty = cx.typeck_results().expr_ty(expr);
let arg_ty = cx.typeck_results().expr_ty(&args[0]);

pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>], func_kind: &ExprKind<'_>) {
if_chain! {
if let hir::ExprKind::Path(path) = func_kind;
if match_qpath(path, &["from_iter"]);
let ty = cx.typeck_results().expr_ty(expr);
let arg_ty = cx.typeck_results().expr_ty(&args[0]);
if let Some(from_iter_id) = get_trait_def_id(cx, &paths::FROM_ITERATOR);
if let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator);

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/get_unwrap.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::methods::derefs_to_slice;
use super::utils::derefs_to_slice;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::{is_type_diagnostic_item, match_type};
Expand Down
11 changes: 7 additions & 4 deletions clippy_lints/src/methods/inefficient_to_string.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use super::INEFFICIENT_TO_STRING;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::{is_type_diagnostic_item, walk_ptrs_ty_depth};
Expand All @@ -8,14 +7,18 @@ use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_span::sym;
use rustc_span::symbol::{sym, Symbol};

use super::INEFFICIENT_TO_STRING;

/// Checks for the `INEFFICIENT_TO_STRING` lint
pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, arg_ty: Ty<'tcx>) {
pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, method_name: Symbol, args: &[hir::Expr<'_>]) {
if_chain! {
if args.len() == 1 && method_name == sym!(to_string);
if let Some(to_string_meth_did) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if match_def_path(cx, to_string_meth_did, &paths::TO_STRING_METHOD);
if let Some(substs) = cx.typeck_results().node_substs_opt(expr.hir_id);
let arg_ty = cx.typeck_results().expr_ty_adjusted(&args[0]);
let self_ty = substs.type_at(0);
let (deref_self_ty, deref_count) = walk_ptrs_ty_depth(self_ty);
if deref_count >= 1;
Expand All @@ -32,7 +35,7 @@ pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, arg: &hir::Expr
self_ty, deref_self_ty
));
let mut applicability = Applicability::MachineApplicable;
let arg_snippet = snippet_with_applicability(cx, arg.span, "..", &mut applicability);
let arg_snippet = snippet_with_applicability(cx, args[0].span, "..", &mut applicability);
diag.span_suggestion(
expr.span,
"try dereferencing the receiver",
Expand Down
49 changes: 30 additions & 19 deletions clippy_lints/src/methods/into_iter_on_ref.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,43 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_trait_method;
use clippy_utils::ty::has_iter_method;
use clippy_utils::{match_trait_method, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_span::source_map::Span;
use rustc_span::symbol::Symbol;
use rustc_span::symbol::{sym, Symbol};

use super::INTO_ITER_ON_REF;

pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, self_ref_ty: Ty<'_>, method_span: Span) {
if !match_trait_method(cx, expr, &paths::INTO_ITERATOR) {
return;
}
if let Some((kind, method_name)) = ty_has_iter_method(cx, self_ref_ty) {
span_lint_and_sugg(
cx,
INTO_ITER_ON_REF,
method_span,
&format!(
"this `.into_iter()` call is equivalent to `.{}()` and will not consume the `{}`",
method_name, kind,
),
"call directly",
method_name.to_string(),
Applicability::MachineApplicable,
);
pub(super) fn check(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
method_span: Span,
method_name: Symbol,
args: &[hir::Expr<'_>],
) {
let self_ty = cx.typeck_results().expr_ty_adjusted(&args[0]);
if_chain! {
if let ty::Ref(..) = self_ty.kind();
if method_name == sym::into_iter;
if is_trait_method(cx, expr, sym::IntoIterator);
if let Some((kind, method_name)) = ty_has_iter_method(cx, self_ty);
then {
span_lint_and_sugg(
cx,
INTO_ITER_ON_REF,
method_span,
&format!(
"this `.into_iter()` call is equivalent to `.{}()` and will not consume the `{}`",
method_name, kind,
),
"call directly",
method_name.to_string(),
Applicability::MachineApplicable,
);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/iter_cloned_collect.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::methods::derefs_to_slice;
use crate::methods::utils::derefs_to_slice;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::ty::is_type_diagnostic_item;
use if_chain::if_chain;
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/iter_count.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::methods::derefs_to_slice;
use super::utils::derefs_to_slice;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::paths;
use clippy_utils::source::snippet_with_applicability;
Expand Down
Loading