Skip to content
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

Refactor: arrange lints in methods module #6826

Merged
merged 35 commits into from
Mar 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
f0f07ac
move expect_used, filter_next, get_unwrap, ok_expect and unwrap_used …
TaKO8Ki Mar 2, 2021
483bac2
move skip_while_next to its own module
TaKO8Ki Mar 2, 2021
110dac7
move option_as_ref_deref to its own module
TaKO8Ki Mar 2, 2021
60a0537
move suspicious_map to its own module
TaKO8Ki Mar 2, 2021
8623b33
move filetype_is_file to its own module
TaKO8Ki Mar 2, 2021
35147d4
move uninit_assumed_init to its own module
TaKO8Ki Mar 2, 2021
45ee914
move iter_cloned_collect to its own module
TaKO8Ki Mar 2, 2021
6376da7
replace `lints` and `lint` with `check`
TaKO8Ki Mar 2, 2021
9b0a42b
move zst_offset to its own module
TaKO8Ki Mar 4, 2021
db91d9c
move map_collect_result_unit to its own module
TaKO8Ki Mar 4, 2021
3fe099b
move iter_next_slice to its own module
TaKO8Ki Mar 4, 2021
5912ca9
move iter_nth, iter_nth_zero and iterator_step_by_zero to their own m…
TaKO8Ki Mar 4, 2021
b5d809a
move wrong_self_convention to its own module
TaKO8Ki Mar 4, 2021
2ade32d
move string_extend_chars and clone_on_ref_ptr to their own module
TaKO8Ki Mar 4, 2021
8006dab
move single_char_insert_string to its own module
TaKO8Ki Mar 6, 2021
805aa47
move single_char_push_string to its own module
TaKO8Ki Mar 6, 2021
0c8d143
move into_iter_on_ref and single_char_pattern to their own modules
TaKO8Ki Mar 6, 2021
2561b7e
move from_iter_insteam_of_collect to its own module
TaKO8Ki Mar 6, 2021
183daeb
move filter_map to its own module
TaKO8Ki Mar 6, 2021
805dcd1
move filter_map_map to its own module
TaKO8Ki Mar 6, 2021
2baf043
move filter_map_next to its own module
TaKO8Ki Mar 6, 2021
6d94161
move filter_flat_map to its own module
TaKO8Ki Mar 6, 2021
f430384
move filter_map_flat_map to its own module
TaKO8Ki Mar 6, 2021
37ba779
move flat_map_identity to its own module
TaKO8Ki Mar 6, 2021
24909fa
move map_flatten and search_is_some to their own modules
TaKO8Ki Mar 6, 2021
171c4c1
move iter_skip_next to its own module
TaKO8Ki Mar 6, 2021
caaba82
move clone_on_copy to its own module
TaKO8Ki Mar 6, 2021
78e572c
move useless_asref to its own module
TaKO8Ki Mar 6, 2021
bbed852
unnecessary_fold to its own module
TaKO8Ki Mar 6, 2021
5557596
move option_map_or_none to its own module
TaKO8Ki Mar 6, 2021
b0824bf
move map_unwrap_or to its own module
TaKO8Ki Mar 6, 2021
f49349b
move or_fun_call to its own module
TaKO8Ki Mar 6, 2021
c711de2
move expect_fun_call to its own module
TaKO8Ki Mar 6, 2021
99f8607
remove unused imports
TaKO8Ki Mar 11, 2021
83a9553
fix interning-defined-symbol error
TaKO8Ki Mar 11, 2021
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
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/bind_instead_of_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ pub(crate) trait BindInsteadOfMap {
}

/// Lint use of `_.and_then(|x| Some(y))` for `Option`s
fn lint(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) -> bool {
fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) -> bool {
if !match_type(cx, cx.typeck_results().expr_ty(&args[0]), Self::TYPE_QPATH) {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/bytes_nth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_span::sym;

use super::BYTES_NTH;

pub(super) fn lints<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>]) {
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>]) {
if_chain! {
if let ExprKind::MethodCall(_, _, ref args, _) = expr.kind;
let ty = cx.typeck_results().expr_ty(&iter_args[0]).peel_refs();
Expand Down
109 changes: 109 additions & 0 deletions clippy_lints/src/methods/clone_on_copy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
use crate::utils::{is_copy, span_lint_and_then, sugg};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
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<'_>) {
let ty = cx.typeck_results().expr_ty(expr);
if let ty::Ref(_, inner, _) = arg_ty.kind() {
if let ty::Ref(_, innermost, _) = inner.kind() {
span_lint_and_then(
cx,
CLONE_DOUBLE_REF,
expr.span,
&format!(
"using `clone` on a double-reference; \
this will copy the reference of type `{}` instead of cloning the inner type",
ty
),
|diag| {
if let Some(snip) = sugg::Sugg::hir_opt(cx, arg) {
let mut ty = innermost;
let mut n = 0;
while let ty::Ref(_, inner, _) = ty.kind() {
ty = inner;
n += 1;
}
let refs: String = iter::repeat('&').take(n + 1).collect();
let derefs: String = iter::repeat('*').take(n).collect();
let explicit = format!("<{}{}>::clone({})", refs, ty, snip);
diag.span_suggestion(
expr.span,
"try dereferencing it",
format!("{}({}{}).clone()", refs, derefs, snip.deref()),
Applicability::MaybeIncorrect,
);
diag.span_suggestion(
expr.span,
"or try being explicit if you are sure, that you want to clone a reference",
explicit,
Applicability::MaybeIncorrect,
);
}
},
);
return; // don't report clone_on_copy
}
}

if is_copy(cx, ty) {
let snip;
if let Some(snippet) = sugg::Sugg::hir_opt(cx, arg) {
let parent = cx.tcx.hir().get_parent_node(expr.hir_id);
match &cx.tcx.hir().get(parent) {
hir::Node::Expr(parent) => match parent.kind {
// &*x is a nop, &x.clone() is not
hir::ExprKind::AddrOf(..) => return,
// (*x).func() is useless, x.clone().func() can work in case func borrows mutably
hir::ExprKind::MethodCall(_, _, parent_args, _) if expr.hir_id == parent_args[0].hir_id => {
return;
},

_ => {},
},
hir::Node::Stmt(stmt) => {
if let hir::StmtKind::Local(ref loc) = stmt.kind {
if let hir::PatKind::Ref(..) = loc.pat.kind {
// let ref y = *x borrows x, let ref y = x.clone() does not
return;
}
}
},
_ => {},
}

// x.clone() might have dereferenced x, possibly through Deref impls
if cx.typeck_results().expr_ty(arg) == ty {
snip = Some(("try removing the `clone` call", format!("{}", snippet)));
} else {
let deref_count = cx
.typeck_results()
.expr_adjustments(arg)
.iter()
.filter(|adj| matches!(adj.kind, ty::adjustment::Adjust::Deref(_)))
.count();
let derefs: String = iter::repeat('*').take(deref_count).collect();
snip = Some(("try dereferencing it", format!("{}{}", derefs, snippet)));
}
} else {
snip = None;
}
span_lint_and_then(
cx,
CLONE_ON_COPY,
expr.span,
&format!("using `clone` on type `{}` which implements the `Copy` trait", ty),
|diag| {
if let Some((text, snip)) = snip {
diag.span_suggestion(expr.span, text, snip, Applicability::MachineApplicable);
}
},
);
}
}
36 changes: 36 additions & 0 deletions clippy_lints/src/methods/clone_on_ref_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use crate::utils::{is_type_diagnostic_item, match_type, paths, snippet_with_macro_callsite, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::symbol::sym;

use super::CLONE_ON_REF_PTR;

pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>) {
let obj_ty = cx.typeck_results().expr_ty(arg).peel_refs();

if let ty::Adt(_, subst) = obj_ty.kind() {
let caller_type = if is_type_diagnostic_item(cx, obj_ty, sym::Rc) {
"Rc"
} else if is_type_diagnostic_item(cx, obj_ty, sym::Arc) {
"Arc"
} else if match_type(cx, obj_ty, &paths::WEAK_RC) || match_type(cx, obj_ty, &paths::WEAK_ARC) {
"Weak"
} else {
return;
};

let snippet = snippet_with_macro_callsite(cx, arg.span, "..");

span_lint_and_sugg(
cx,
CLONE_ON_REF_PTR,
expr.span,
"using `.clone()` on a ref-counted pointer",
"try this",
format!("{}::<{}>::clone(&{})", caller_type, subst.type_at(0), snippet),
Applicability::Unspecified, // Sometimes unnecessary ::<_> after Rc/Arc/Weak
);
}
}
199 changes: 199 additions & 0 deletions clippy_lints/src/methods/expect_fun_call.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
use crate::utils::{is_expn_of, is_type_diagnostic_item, snippet, snippet_with_applicability, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::source_map::Span;
use rustc_span::symbol::sym;
use std::borrow::Cow;

use super::EXPECT_FUN_CALL;

/// Checks for the `EXPECT_FUN_CALL` lint.
#[allow(clippy::too_many_lines)]
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_span: Span, name: &str, args: &[hir::Expr<'_>]) {
// Strip `&`, `as_ref()` and `as_str()` off `arg` until we're left with either a `String` or
// `&str`
fn get_arg_root<'a>(cx: &LateContext<'_>, arg: &'a hir::Expr<'a>) -> &'a hir::Expr<'a> {
let mut arg_root = arg;
loop {
arg_root = match &arg_root.kind {
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, expr) => expr,
hir::ExprKind::MethodCall(method_name, _, call_args, _) => {
if call_args.len() == 1
&& (method_name.ident.name == sym::as_str || method_name.ident.name == sym!(as_ref))
&& {
let arg_type = cx.typeck_results().expr_ty(&call_args[0]);
let base_type = arg_type.peel_refs();
*base_type.kind() == ty::Str || is_type_diagnostic_item(cx, base_type, sym::string_type)
}
{
&call_args[0]
} else {
break;
}
},
_ => break,
};
}
arg_root
}

// Only `&'static str` or `String` can be used directly in the `panic!`. Other types should be
// converted to string.
fn requires_to_string(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool {
let arg_ty = cx.typeck_results().expr_ty(arg);
if is_type_diagnostic_item(cx, arg_ty, sym::string_type) {
return false;
}
if let ty::Ref(_, ty, ..) = arg_ty.kind() {
if *ty.kind() == ty::Str && can_be_static_str(cx, arg) {
return false;
}
};
true
}

// Check if an expression could have type `&'static str`, knowing that it
// has type `&str` for some lifetime.
fn can_be_static_str(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool {
match arg.kind {
hir::ExprKind::Lit(_) => true,
hir::ExprKind::Call(fun, _) => {
if let hir::ExprKind::Path(ref p) = fun.kind {
match cx.qpath_res(p, fun.hir_id) {
hir::def::Res::Def(hir::def::DefKind::Fn | hir::def::DefKind::AssocFn, def_id) => matches!(
cx.tcx.fn_sig(def_id).output().skip_binder().kind(),
ty::Ref(ty::ReStatic, ..)
),
_ => false,
}
} else {
false
}
},
hir::ExprKind::MethodCall(..) => {
cx.typeck_results()
.type_dependent_def_id(arg.hir_id)
.map_or(false, |method_id| {
matches!(
cx.tcx.fn_sig(method_id).output().skip_binder().kind(),
ty::Ref(ty::ReStatic, ..)
)
})
},
hir::ExprKind::Path(ref p) => matches!(
cx.qpath_res(p, arg.hir_id),
hir::def::Res::Def(hir::def::DefKind::Const | hir::def::DefKind::Static, _)
),
_ => false,
}
}

fn generate_format_arg_snippet(
cx: &LateContext<'_>,
a: &hir::Expr<'_>,
applicability: &mut Applicability,
) -> Vec<String> {
if_chain! {
if let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, ref format_arg) = a.kind;
if let hir::ExprKind::Match(ref format_arg_expr, _, _) = format_arg.kind;
if let hir::ExprKind::Tup(ref format_arg_expr_tup) = format_arg_expr.kind;

then {
format_arg_expr_tup
.iter()
.map(|a| snippet_with_applicability(cx, a.span, "..", applicability).into_owned())
.collect()
} else {
unreachable!()
}
}
}

fn is_call(node: &hir::ExprKind<'_>) -> bool {
match node {
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, expr) => {
is_call(&expr.kind)
},
hir::ExprKind::Call(..)
| hir::ExprKind::MethodCall(..)
// These variants are debatable or require further examination
| hir::ExprKind::If(..)
| hir::ExprKind::Match(..)
| hir::ExprKind::Block{ .. } => true,
_ => false,
}
}

if args.len() != 2 || name != "expect" || !is_call(&args[1].kind) {
return;
}

let receiver_type = cx.typeck_results().expr_ty_adjusted(&args[0]);
let closure_args = if is_type_diagnostic_item(cx, receiver_type, sym::option_type) {
"||"
} else if is_type_diagnostic_item(cx, receiver_type, sym::result_type) {
"|_|"
} else {
return;
};

let arg_root = get_arg_root(cx, &args[1]);

let span_replace_word = method_span.with_hi(expr.span.hi());

let mut applicability = Applicability::MachineApplicable;

//Special handling for `format!` as arg_root
if_chain! {
if let hir::ExprKind::Block(block, None) = &arg_root.kind;
if block.stmts.len() == 1;
if let hir::StmtKind::Local(local) = &block.stmts[0].kind;
if let Some(arg_root) = &local.init;
if let hir::ExprKind::Call(ref inner_fun, ref inner_args) = arg_root.kind;
if is_expn_of(inner_fun.span, "format").is_some() && inner_args.len() == 1;
if let hir::ExprKind::Call(_, format_args) = &inner_args[0].kind;
then {
let fmt_spec = &format_args[0];
let fmt_args = &format_args[1];

let mut args = vec![snippet(cx, fmt_spec.span, "..").into_owned()];

args.extend(generate_format_arg_snippet(cx, fmt_args, &mut applicability));

let sugg = args.join(", ");

span_lint_and_sugg(
cx,
EXPECT_FUN_CALL,
span_replace_word,
&format!("use of `{}` followed by a function call", name),
"try this",
format!("unwrap_or_else({} panic!({}))", closure_args, sugg),
applicability,
);

return;
}
}

let mut arg_root_snippet: Cow<'_, _> = snippet_with_applicability(cx, arg_root.span, "..", &mut applicability);
if requires_to_string(cx, arg_root) {
arg_root_snippet.to_mut().push_str(".to_string()");
}

span_lint_and_sugg(
cx,
EXPECT_FUN_CALL,
span_replace_word,
&format!("use of `{}` followed by a function call", name),
"try this",
format!(
"unwrap_or_else({} {{ panic!(\"{{}}\", {}) }})",
closure_args, arg_root_snippet
),
applicability,
);
}
30 changes: 30 additions & 0 deletions clippy_lints/src/methods/expect_used.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use crate::utils::{is_type_diagnostic_item, span_lint_and_help};
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::sym;

use super::EXPECT_USED;

/// lint use of `expect()` for `Option`s and `Result`s
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, expect_args: &[hir::Expr<'_>]) {
let obj_ty = cx.typeck_results().expr_ty(&expect_args[0]).peel_refs();

let mess = if is_type_diagnostic_item(cx, obj_ty, sym::option_type) {
Some((EXPECT_USED, "an Option", "None"))
} else if is_type_diagnostic_item(cx, obj_ty, sym::result_type) {
Some((EXPECT_USED, "a Result", "Err"))
} else {
None
};

if let Some((lint, kind, none_value)) = mess {
span_lint_and_help(
cx,
lint,
expr.span,
&format!("used `expect()` on `{}` value", kind,),
None,
&format!("if this value is an `{}`, it will panic", none_value,),
);
}
}
Loading