Skip to content

Commit

Permalink
Auto merge of rust-lang#6896 - TaKO8Ki:refactor-lints-in-methods-modu…
Browse files Browse the repository at this point in the history
…le, r=phansch

Refactor lints in methods module

This PR refactors methods lints other than the lints I refactored in rust-lang/rust-clippy#6826 and moves some functions to methods/utils.rs.
Basically, I follow the instruction described in rust-lang#6680.

**For ease of review, I refactored step by step, keeping each commit small.**

closes rust-lang/rust-clippy#6886
cc: `@phansch,` `@flip1995,` `@Y-Nak`

changelog: Move lints in methods module to their own modules and some function to methods/utils.rs.
  • Loading branch information
bors committed Mar 22, 2021
2 parents aca95aa + b6a2757 commit 029777f
Show file tree
Hide file tree
Showing 27 changed files with 388 additions and 333 deletions.
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(
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

0 comments on commit 029777f

Please sign in to comment.