use if_chain::if_chain;
use matches::matches;
use rustc::hir;
-use rustc::hir::def::{DefKind, Res};
use rustc::hir::intravisit::{self, Visitor};
use rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass};
use rustc::ty::{self, Predicate, Ty};
use crate::utils::sugg;
use crate::utils::usage::mutated_variables;
use crate::utils::{
- get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, in_macro, in_macro_or_desugar,
- is_copy, is_ctor_function, is_expn_of, iter_input_pats, last_path_segment, match_def_path, match_qpath,
- match_trait_method, match_type, match_var, method_calls, method_chain_args, remove_blocks, return_ty, same_tys,
- single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint,
- span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq,
+ get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, in_macro, is_copy,
+ is_ctor_function, is_expn_of, iter_input_pats, last_path_segment, match_def_path, match_qpath, match_trait_method,
+ match_type, match_var, method_calls, method_chain_args, remove_blocks, return_ty, same_tys, single_segment_path,
+ snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_sugg,
+ span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq,
};
use crate::utils::{paths, span_help_and_lint};
if let hir::ImplItemKind::Method(ref sig, id) = impl_item.node;
if let Some(first_arg) = iter_input_pats(&sig.decl, cx.tcx.hir().body(id)).next();
if let hir::ItemKind::Impl(_, _, _, _, None, _, _) = item.node;
+
+ let method_def_id = cx.tcx.hir().local_def_id(impl_item.hir_id);
+ let method_sig = cx.tcx.fn_sig(method_def_id);
+ let method_sig = cx.tcx.erase_late_bound_regions(&method_sig);
+
+ let first_arg_ty = &method_sig.inputs().iter().next();
+
+ // check conventions w.r.t. conversion method names and predicates
+ if let Some(first_arg_ty) = first_arg_ty;
+
then {
- let method_def_id = cx.tcx.hir().local_def_id(impl_item.hir_id);
- let method_sig = cx.tcx.fn_sig(method_def_id);
- let method_sig = cx.tcx.erase_late_bound_regions(&method_sig);
-
- let first_arg_ty = &method_sig.inputs().iter().next();
-
- // check conventions w.r.t. conversion method names and predicates
- if let Some(first_arg_ty) = first_arg_ty {
-
- if cx.access_levels.is_exported(impl_item.hir_id) {
- // check missing trait implementations
- for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS {
- if name == method_name &&
- sig.decl.inputs.len() == n_args &&
- out_type.matches(cx, &sig.decl.output) &&
- self_kind.matches(cx, ty, first_arg_ty) {
- span_lint(cx, SHOULD_IMPLEMENT_TRAIT, impl_item.span, &format!(
- "defining a method called `{}` on this type; consider implementing \
- the `{}` trait or choosing a less ambiguous name", name, trait_name));
- }
+ if cx.access_levels.is_exported(impl_item.hir_id) {
+ // check missing trait implementations
+ for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS {
+ if name == method_name &&
+ sig.decl.inputs.len() == n_args &&
+ out_type.matches(cx, &sig.decl.output) &&
+ self_kind.matches(cx, ty, first_arg_ty) {
+ span_lint(cx, SHOULD_IMPLEMENT_TRAIT, impl_item.span, &format!(
+ "defining a method called `{}` on this type; consider implementing \
+ the `{}` trait or choosing a less ambiguous name", name, trait_name));
}
}
+ }
- for &(ref conv, self_kinds) in &CONVENTIONS {
- if conv.check(&name) {
- if !self_kinds
- .iter()
- .any(|k| k.matches(cx, ty, first_arg_ty)) {
- let lint = if item.vis.node.is_pub() {
- WRONG_PUB_SELF_CONVENTION
- } else {
- WRONG_SELF_CONVENTION
- };
- span_lint(cx,
- lint,
- first_arg.pat.span,
- &format!("methods called `{}` usually take {}; consider choosing a less \
- ambiguous name",
- conv,
- &self_kinds.iter()
- .map(|k| k.description())
- .collect::<Vec<_>>()
- .join(" or ")));
- }
+ if let Some((ref conv, self_kinds)) = &CONVENTIONS
+ .iter()
+ .find(|(ref conv, _)| conv.check(&name))
+ {
+ if !self_kinds.iter().any(|k| k.matches(cx, ty, first_arg_ty)) {
+ let lint = if item.vis.node.is_pub() {
+ WRONG_PUB_SELF_CONVENTION
+ } else {
+ WRONG_SELF_CONVENTION
+ };
- // Only check the first convention to match (CONVENTIONS should be listed from most to least
- // specific)
- break;
- }
+ span_lint(
+ cx,
+ lint,
+ first_arg.pat.span,
+ &format!(
+ "methods called `{}` usually take {}; consider choosing a less \
+ ambiguous name",
+ conv,
+ &self_kinds
+ .iter()
+ .map(|k| k.description())
+ .collect::<Vec<_>>()
+ .join(" or ")
+ ),
+ );
}
}
}
let ret_ty = return_ty(cx, impl_item.hir_id);
// walk the return type and check for Self (this does not check associated types)
- for inner_type in ret_ty.walk() {
- if same_tys(cx, ty, inner_type) {
- return;
- }
+ if ret_ty.walk().any(|inner_type| same_tys(cx, ty, inner_type)) {
+ return;
}
// if return type is impl trait, check the associated types
or_has_args: bool,
span: Span,
) -> bool {
- if or_has_args {
- return false;
- }
-
- if name == "unwrap_or" {
- if let hir::ExprKind::Path(ref qpath) = fun.node {
- let path = &*last_path_segment(qpath).ident.as_str();
+ if_chain! {
+ if !or_has_args;
+ if name == "unwrap_or";
+ if let hir::ExprKind::Path(ref qpath) = fun.node;
+ let path = &*last_path_segment(qpath).ident.as_str();
+ if ["default", "new"].contains(&path);
+ let arg_ty = cx.tables.expr_ty(arg);
+ if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT);
+ if implements_trait(cx, arg_ty, default_trait_id, &[]);
- if ["default", "new"].contains(&path) {
- let arg_ty = cx.tables.expr_ty(arg);
- let default_trait_id = if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT) {
- default_trait_id
- } else {
- return false;
- };
+ then {
+ let mut applicability = Applicability::MachineApplicable;
+ span_lint_and_sugg(
+ cx,
+ OR_FUN_CALL,
+ span,
+ &format!("use of `{}` followed by a call to `{}`", name, path),
+ "try this",
+ format!(
+ "{}.unwrap_or_default()",
+ snippet_with_applicability(cx, self_expr.span, "_", &mut applicability)
+ ),
+ applicability,
+ );
- if implements_trait(cx, arg_ty, default_trait_id, &[]) {
- let mut applicability = Applicability::MachineApplicable;
- span_lint_and_sugg(
- cx,
- OR_FUN_CALL,
- span,
- &format!("use of `{}` followed by a call to `{}`", name, path),
- "try this",
- format!(
- "{}.unwrap_or_default()",
- snippet_with_applicability(cx, self_expr.span, "_", &mut applicability)
- ),
- applicability,
- );
- return true;
- }
- }
+ true
+ } else {
+ false
}
}
-
- false
}
/// Checks for `*or(foo())`.
(&paths::RESULT, true, &["or", "unwrap_or"], "else"),
];
- // early check if the name is one we care about
- if know_types.iter().all(|k| !k.2.contains(&name)) {
- return;
- }
+ if_chain! {
+ if know_types.iter().any(|k| k.2.contains(&name));
- let mut finder = FunCallFinder { cx: &cx, found: false };
- finder.visit_expr(&arg);
- if !finder.found {
- return;
- }
+ let mut finder = FunCallFinder { cx: &cx, found: false };
+ if { finder.visit_expr(&arg); finder.found };
- let self_ty = cx.tables.expr_ty(self_expr);
+ let self_ty = cx.tables.expr_ty(self_expr);
- let (fn_has_arguments, poss, suffix) = if let Some(&(_, fn_has_arguments, poss, suffix)) =
- know_types.iter().find(|&&i| match_type(cx, self_ty, i.0))
- {
- (fn_has_arguments, poss, suffix)
- } else {
- return;
- };
+ if let Some(&(_, fn_has_arguments, poss, suffix)) =
+ know_types.iter().find(|&&i| match_type(cx, self_ty, i.0));
- if !poss.contains(&name) {
- return;
- }
+ if poss.contains(&name);
- let sugg: Cow<'_, _> = match (fn_has_arguments, !or_has_args) {
- (true, _) => format!("|_| {}", snippet_with_macro_callsite(cx, arg.span, "..")).into(),
- (false, false) => format!("|| {}", snippet_with_macro_callsite(cx, arg.span, "..")).into(),
- (false, true) => snippet_with_macro_callsite(cx, fun_span, ".."),
- };
- let span_replace_word = method_span.with_hi(span.hi());
- span_lint_and_sugg(
- cx,
- OR_FUN_CALL,
- span_replace_word,
- &format!("use of `{}` followed by a function call", name),
- "try this",
- format!("{}_{}({})", name, suffix, sugg),
- Applicability::HasPlaceholders,
- );
+ then {
+ let sugg: Cow<'_, _> = match (fn_has_arguments, !or_has_args) {
+ (true, _) => format!("|_| {}", snippet_with_macro_callsite(cx, arg.span, "..")).into(),
+ (false, false) => format!("|| {}", snippet_with_macro_callsite(cx, arg.span, "..")).into(),
+ (false, true) => snippet_with_macro_callsite(cx, fun_span, ".."),
+ };
+ let span_replace_word = method_span.with_hi(span.hi());
+ span_lint_and_sugg(
+ cx,
+ OR_FUN_CALL,
+ span_replace_word,
+ &format!("use of `{}` followed by a function call", name),
+ "try this",
+ format!("{}_{}({})", name, suffix, sugg),
+ Applicability::HasPlaceholders,
+ );
+ }
+ }
}
if args.len() == 2 {
a: &hir::Expr,
applicability: &mut Applicability,
) -> Vec<String> {
- if let hir::ExprKind::AddrOf(_, ref format_arg) = a.node {
- if let hir::ExprKind::Match(ref format_arg_expr, _, _) = format_arg.node {
- if let hir::ExprKind::Tup(ref format_arg_expr_tup) = format_arg_expr.node {
- return format_arg_expr_tup
- .iter()
- .map(|a| snippet_with_applicability(cx, a.span, "..", applicability).into_owned())
- .collect();
- }
- }
- };
+ if_chain! {
+ if let hir::ExprKind::AddrOf(_, ref format_arg) = a.node;
+ if let hir::ExprKind::Match(ref format_arg_expr, _, _) = format_arg.node;
+ if let hir::ExprKind::Tup(ref format_arg_expr_tup) = format_arg_expr.node;
- unreachable!()
+ 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 {
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.node {
+ // &*x is a nop, &x.clone() is not
+ hir::ExprKind::AddrOf(..) |
+ // (*x).func() is useless, x.clone().func() can work in case func borrows mutably
+ hir::ExprKind::MethodCall(..) => return,
+ _ => {},
+ },
+ hir::Node::Stmt(stmt) => {
+ if let hir::StmtKind::Local(ref loc) = stmt.node {
+ if let hir::PatKind::Ref(..) = loc.pat.node {
+ // 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.tables.expr_ty(arg) == ty {
snip = Some(("try removing the `clone` call", format!("{}", snippet)));
} else {
- let parent = cx.tcx.hir().get_parent_node(expr.hir_id);
- match cx.tcx.hir().get(parent) {
- hir::Node::Expr(parent) => match parent.node {
- // &*x is a nop, &x.clone() is not
- hir::ExprKind::AddrOf(..) |
- // (*x).func() is useless, x.clone().func() can work in case func borrows mutably
- hir::ExprKind::MethodCall(..) => return,
- _ => {},
- },
- hir::Node::Stmt(stmt) => {
- if let hir::StmtKind::Local(ref loc) = stmt.node {
- if let hir::PatKind::Ref(..) = loc.pat.node {
- // let ref y = *x borrows x, let ref y = x.clone() does not
- return;
- }
- }
- },
- _ => {},
- }
-
let deref_count = cx
.tables
.expr_adjustments(arg)
}
}
-fn lint_cstring_as_ptr(cx: &LateContext<'_, '_>, expr: &hir::Expr, new: &hir::Expr, unwrap: &hir::Expr) {
+fn lint_cstring_as_ptr(cx: &LateContext<'_, '_>, expr: &hir::Expr, source: &hir::Expr, unwrap: &hir::Expr) {
if_chain! {
- if let hir::ExprKind::Call(ref fun, ref args) = new.node;
- if args.len() == 1;
- if let hir::ExprKind::Path(ref path) = fun.node;
- if let Res::Def(DefKind::Method, did) = cx.tables.qpath_res(path, fun.hir_id);
- if match_def_path(cx, did, &paths::CSTRING_NEW);
+ let source_type = cx.tables.expr_ty(source);
+ if let ty::Adt(def, substs) = source_type.sty;
+ if match_def_path(cx, def.did, &paths::RESULT);
+ if match_type(cx, substs.type_at(0), &paths::CSTRING);
then {
span_lint_and_then(
cx,
}
fn lint_iter_cloned_collect<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr, iter_args: &'tcx [hir::Expr]) {
- if match_type(cx, cx.tables.expr_ty(expr), &paths::VEC) {
- if let Some(slice) = derefs_to_slice(cx, &iter_args[0], cx.tables.expr_ty(&iter_args[0])) {
- if let Some(to_replace) = expr.span.trim_start(slice.span.source_callsite()) {
- span_lint_and_sugg(
- cx,
- ITER_CLONED_COLLECT,
- to_replace,
- "called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and \
- more readable",
- "try",
- ".to_vec()".to_string(),
- Applicability::MachineApplicable,
- );
- }
+ if_chain! {
+ if match_type(cx, cx.tables.expr_ty(expr), &paths::VEC);
+ if let Some(slice) = derefs_to_slice(cx, &iter_args[0], cx.tables.expr_ty(&iter_args[0]));
+ if let Some(to_replace) = expr.span.trim_start(slice.span.source_callsite());
+
+ then {
+ span_lint_and_sugg(
+ cx,
+ ITER_CLONED_COLLECT,
+ to_replace,
+ "called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and \
+ more readable",
+ "try",
+ ".to_vec()".to_string(),
+ Applicability::MachineApplicable,
+ );
}
}
}
/// lint use of `ok().expect()` for `Result`s
fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr, ok_args: &[hir::Expr]) {
- // lint if the caller of `ok()` is a `Result`
- if match_type(cx, cx.tables.expr_ty(&ok_args[0]), &paths::RESULT) {
+ if_chain! {
+ // lint if the caller of `ok()` is a `Result`
+ if match_type(cx, cx.tables.expr_ty(&ok_args[0]), &paths::RESULT);
let result_type = cx.tables.expr_ty(&ok_args[0]);
- if let Some(error_type) = get_error_type(cx, result_type) {
- if has_debug_impl(error_type, cx) {
- span_lint(
- cx,
- OK_EXPECT,
- expr.span,
- "called `ok().expect()` on a Result value. You can call `expect` directly on the `Result`",
- );
- }
+ if let Some(error_type) = get_error_type(cx, result_type);
+ if has_debug_impl(error_type, cx);
+
+ then {
+ span_lint(
+ cx,
+ OK_EXPECT,
+ expr.span,
+ "called `ok().expect()` on a Result value. You can call `expect` directly on the `Result`",
+ );
}
}
}
return;
}
- let some_inner_snip = if in_macro_or_desugar(inner_expr.span) {
+ let some_inner_snip = if inner_expr.span.from_expansion() {
snippet_with_macro_callsite(cx, inner_expr.span, "_")
} else {
snippet(cx, inner_expr.span, "_")
applicability,
);
- return true;
+ true
+ } else {
+ false
}
}
-
- false
}
/// Checks for the `CHARS_NEXT_CMP` lint with `unwrap()`.
cx: &LateContext<'_, '_>,
self_ref_ty: Ty<'_>,
) -> Option<(&'static Lint, &'static str, &'static str)> {
- if let Some(ty_name) = has_iter_method(cx, self_ref_ty) {
+ has_iter_method(cx, self_ref_ty).map(|ty_name| {
let lint = if ty_name == "array" || ty_name == "PathBuf" {
INTO_ITER_ON_ARRAY
} else {
hir::MutImmutable => "iter",
hir::MutMutable => "iter_mut",
};
- Some((lint, ty_name, method_name))
- } else {
- None
- }
+ (lint, ty_name, method_name)
+ })
}
fn lint_into_iter(cx: &LateContext<'_, '_>, expr: &hir::Expr, self_ref_ty: Ty<'_>, method_span: Span) {
/// Given a `Result<T, E>` type, return its error type (`E`).
fn get_error_type<'a>(cx: &LateContext<'_, '_>, ty: Ty<'a>) -> Option<Ty<'a>> {
- if let ty::Adt(_, substs) = ty.sty {
- if match_type(cx, ty, &paths::RESULT) {
- substs.types().nth(1)
- } else {
- None
- }
- } else {
- None
+ match ty.sty {
+ ty::Adt(_, substs) if match_type(cx, ty, &paths::RESULT) => substs.types().nth(1),
+ _ => None,
}
}
hir::Mutability::MutMutable => &paths::ASMUT_TRAIT,
};
- let trait_def_id = get_trait_def_id(cx, trait_path).expect("trait def id not found");
+ let trait_def_id = match get_trait_def_id(cx, trait_path) {
+ Some(did) => did,
+ None => return false,
+ };
implements_trait(cx, ty, trait_def_id, &[parent_ty.into()])
}