user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Auto merge of #4823 - Areredify:must_use_res, r=flip1995

Browse source 
Add `let_underscore_must_use` lint

changelog: closes #4812 , added a new `let_underscore_must_use` lint, moved `is_must_use_ty` to utils, added `is_must_use_fn` util function
This commit is contained in:
bors 2019-12-23 06:16:37 +00:00
commit b38b026a98
9 changed files with 331 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

49
clippy_lints/src/functions.rs
View file

@ -1,6 +1,7 @@
use crate::utils::{
attrs::is_proc_macro, iter_input_pats, match_def_path, qpath_res, return_ty, snippet, snippet_opt,
span_help_and_lint, span_lint, span_lint_and_then, trait_ref_of_method, type_is_unsafe_function,
attrs::is_proc_macro, is_must_use_ty, iter_input_pats, match_def_path, must_use_attr, qpath_res, return_ty,
snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_then, trait_ref_of_method,
type_is_unsafe_function,
};
use matches::matches;
use rustc::hir::{self, def::Res, def_id::DefId, intravisit};
@ -466,15 +467,6 @@ fn check_must_use_candidate<'a, 'tcx>(
});
}
fn must_use_attr(attrs: &[Attribute]) -> Option<&Attribute> {
attrs.iter().find(|attr| {
attr.ident().map_or(false, |ident| {
let ident: &str = &ident.as_str();
"must_use" == ident
})
})
}
fn returns_unit(decl: &hir::FnDecl) -> bool {
match decl.output {
hir::FunctionRetTy::DefaultReturn(_) => true,
@ -486,41 +478,6 @@ fn returns_unit(decl: &hir::FnDecl) -> bool {
}
}
fn is_must_use_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool {
use ty::TyKind::*;
match ty.kind {
Adt(ref adt, _) => must_use_attr(&cx.tcx.get_attrs(adt.did)).is_some(),
Foreign(ref did) => must_use_attr(&cx.tcx.get_attrs(*did)).is_some(),
Slice(ref ty) | Array(ref ty, _) | RawPtr(ty::TypeAndMut { ref ty, .. }) | Ref(_, ref ty, _) => {
// for the Array case we don't need to care for the len == 0 case
// because we don't want to lint functions returning empty arrays
is_must_use_ty(cx, *ty)
},
Tuple(ref substs) => substs.types().any(|ty| is_must_use_ty(cx, ty)),
Opaque(ref def_id, _) => {
for (predicate, _) in cx.tcx.predicates_of(*def_id).predicates {
if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate {
if must_use_attr(&cx.tcx.get_attrs(poly_trait_predicate.skip_binder().trait_ref.def_id)).is_some() {
return true;
}
}
}
false
},
Dynamic(binder, _) => {
for predicate in binder.skip_binder().iter() {
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate {
if must_use_attr(&cx.tcx.get_attrs(trait_ref.def_id)).is_some() {
return true;
}
}
}
false
},
_ => false,
}
}
fn has_mutable_arg(cx: &LateContext<'_, '_>, body: &hir::Body<'_>) -> bool {
let mut tys = FxHashSet::default();
body.params.iter().any(|param| is_mutable_pat(cx, &param.pat, &mut tys))

62
clippy_lints/src/let_underscore.rs Normal file
View file

@ -0,0 +1,62 @@
use if_chain::if_chain;
use rustc::declare_lint_pass;
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc_session::declare_tool_lint;
use crate::utils::{is_must_use_func_call, is_must_use_ty, span_help_and_lint};
declare_clippy_lint! {
/// **What it does:** Checks for `let _ = <expr>`
/// where expr is #[must_use]
///
/// **Why is this bad?** It's better to explicitly
/// handle the value of a #[must_use] expr
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn f() -> Result<u32, u32> {
/// Ok(0)
/// }
///
/// let _ = f();
/// // is_ok() is marked #[must_use]
/// let _ = f().is_ok();
/// ```
pub LET_UNDERSCORE_MUST_USE,
restriction,
"non-binding let on a #[must_use] expression"
}
declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore {
fn check_stmt(&mut self, cx: &LateContext<'_, '_>, stmt: &Stmt) {
if_chain! {
if let StmtKind::Local(ref local) = stmt.kind;
if let PatKind::Wild = local.pat.kind;
if let Some(ref init) = local.init;
then {
if is_must_use_ty(cx, cx.tables.expr_ty(init)) {
span_help_and_lint(
cx,
LET_UNDERSCORE_MUST_USE,
stmt.span,
"non-binding let on an expression with #[must_use] type",
"consider explicitly using expression value"
)
} else if is_must_use_func_call(cx, init) {
span_help_and_lint(
cx,
LET_UNDERSCORE_MUST_USE,
stmt.span,
"non-binding let on a result of a #[must_use] function",
"consider explicitly using function result"
)
}
}
}
}
}

4
clippy_lints/src/lib.rs
View file

@ -220,6 +220,7 @@ pub mod large_enum_variant;
pub mod large_stack_arrays;
pub mod len_zero;
pub mod let_if_seq;
pub mod let_underscore;
pub mod lifetimes;
pub mod literal_representation;
pub mod loops;
@ -555,6 +556,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
&len_zero::LEN_WITHOUT_IS_EMPTY,
&len_zero::LEN_ZERO,
&let_if_seq::USELESS_LET_IF_SEQ,
&let_underscore::LET_UNDERSCORE_MUST_USE,
&lifetimes::EXTRA_UNUSED_LIFETIMES,
&lifetimes::NEEDLESS_LIFETIMES,
&literal_representation::DECIMAL_LITERAL_REPRESENTATION,
@ -970,6 +972,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold));
store.register_early_pass(|| box as_conversions::AsConversions);
store.register_early_pass(|| box utils::internal_lints::ProduceIce);
store.register_late_pass(|| box let_underscore::LetUnderscore);
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@ -982,6 +985,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&indexing_slicing::INDEXING_SLICING),
LintId::of(&inherent_impl::MULTIPLE_INHERENT_IMPL),
LintId::of(&integer_division::INTEGER_DIVISION),
LintId::of(&let_underscore::LET_UNDERSCORE_MUST_USE),
LintId::of(&literal_representation::DECIMAL_LITERAL_REPRESENTATION),
LintId::of(&matches::WILDCARD_ENUM_MATCH_ARM),
LintId::of(&mem_forget::MEM_FORGET),

70
clippy_lints/src/utils/mod.rs
View file

@ -41,7 +41,7 @@ use rustc::ty::{
};
use rustc_errors::Applicability;
use smallvec::SmallVec;
use syntax::ast::{self, LitKind};
use syntax::ast::{self, Attribute, LitKind};
use syntax::attr;
use syntax::source_map::{Span, DUMMY_SP};
use syntax::symbol::{kw, Symbol};
@ -1233,3 +1233,71 @@ pub fn parent_node_is_if_expr<'a, 'b>(expr: &Expr, cx: &LateContext<'a, 'b>) ->
_ => false,
}
}
pub fn must_use_attr(attrs: &[Attribute]) -> Option<&Attribute> {
attrs.iter().find(|attr| {
attr.ident().map_or(false, |ident| {
let ident: &str = &ident.as_str();
"must_use" == ident
})
})
}
// Returns whether the type has #[must_use] attribute
pub fn is_must_use_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool {
use ty::TyKind::*;
match ty.kind {
Adt(ref adt, _) => must_use_attr(&cx.tcx.get_attrs(adt.did)).is_some(),
Foreign(ref did) => must_use_attr(&cx.tcx.get_attrs(*did)).is_some(),
Slice(ref ty) | Array(ref ty, _) | RawPtr(ty::TypeAndMut { ref ty, .. }) | Ref(_, ref ty, _) => {
// for the Array case we don't need to care for the len == 0 case
// because we don't want to lint functions returning empty arrays
is_must_use_ty(cx, *ty)
},
Tuple(ref substs) => substs.types().any(|ty| is_must_use_ty(cx, ty)),
Opaque(ref def_id, _) => {
for (predicate, _) in cx.tcx.predicates_of(*def_id).predicates {
if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate {
if must_use_attr(&cx.tcx.get_attrs(poly_trait_predicate.skip_binder().trait_ref.def_id)).is_some() {
return true;
}
}
}
false
},
Dynamic(binder, _) => {
for predicate in binder.skip_binder().iter() {
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate {
if must_use_attr(&cx.tcx.get_attrs(trait_ref.def_id)).is_some() {
return true;
}
}
}
false
},
_ => false,
}
}
// check if expr is calling method or function with #[must_use] attribyte
pub fn is_must_use_func_call(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
let did = match expr.kind {
ExprKind::Call(ref path, _) => if_chain! {
if let ExprKind::Path(ref qpath) = path.kind;
if let def::Res::Def(_, did) = cx.tables.qpath_res(qpath, path.hir_id);
then {
Some(did)
} else {
None
}
},
ExprKind::MethodCall(_, _, _) => cx.tables.type_dependent_def_id(expr.hir_id),
_ => None,
};
if let Some(did) = did {
must_use_attr(&cx.tcx.get_attrs(did)).is_some()
} else {
false
}
}