-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of #6826 - TaKO8Ki:refactor-methods-mod, r=phansch
Refactor: arrange lints in `methods` module This PR arranges methods lints so that they can be accessed more easily. Basically, I refactored them following the instruction described in #6680. changelog: Move lints in methods module into their own modules.
- Loading branch information
Showing
51 changed files
with
2,502 additions
and
2,143 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
} | ||
}, | ||
); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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, | ||
); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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,), | ||
); | ||
} | ||
} |
Oops, something went wrong.