Merge pull request #1556 from sinkuu/take_by_value
Lint needless take-by-value
This commit is contained in:
commit
27aa309b3d
@ -381,6 +381,7 @@ All notable changes to this project will be documented in this file.
|
||||
[`needless_bool`]: https://github.com/Manishearth/rust-clippy/wiki#needless_bool
|
||||
[`needless_borrow`]: https://github.com/Manishearth/rust-clippy/wiki#needless_borrow
|
||||
[`needless_lifetimes`]: https://github.com/Manishearth/rust-clippy/wiki#needless_lifetimes
|
||||
[`needless_pass_by_value`]: https://github.com/Manishearth/rust-clippy/wiki#needless_pass_by_value
|
||||
[`needless_range_loop`]: https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop
|
||||
[`needless_return`]: https://github.com/Manishearth/rust-clippy/wiki#needless_return
|
||||
[`needless_update`]: https://github.com/Manishearth/rust-clippy/wiki#needless_update
|
||||
|
@ -180,7 +180,7 @@ transparently:
|
||||
|
||||
## Lints
|
||||
|
||||
There are 191 lints included in this crate:
|
||||
There are 192 lints included in this crate:
|
||||
|
||||
name | default | triggers on
|
||||
-----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
|
||||
@ -287,6 +287,7 @@ name
|
||||
[needless_bool](https://github.com/Manishearth/rust-clippy/wiki#needless_bool) | warn | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`
|
||||
[needless_borrow](https://github.com/Manishearth/rust-clippy/wiki#needless_borrow) | warn | taking a reference that is going to be automatically dereferenced
|
||||
[needless_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#needless_lifetimes) | warn | using explicit lifetimes for references in function arguments when elision rules would allow omitting them
|
||||
[needless_pass_by_value](https://github.com/Manishearth/rust-clippy/wiki#needless_pass_by_value) | warn | functions taking arguments by value, but not consuming them in its body
|
||||
[needless_range_loop](https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop) | warn | for-looping over a range of indices where an iterator over items would do
|
||||
[needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return) | warn | using a return statement like `return expr;` where an expression would suffice
|
||||
[needless_update](https://github.com/Manishearth/rust-clippy/wiki#needless_update) | warn | using `Foo { ..base }` when there are no missing fields
|
||||
|
@ -83,7 +83,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIndexing {
|
||||
.map(|end| constcx.eval(end, ExprTypeChecked))
|
||||
.map(|v| v.ok());
|
||||
|
||||
if let Some((start, end)) = to_const_range(start, end, range.limits, size) {
|
||||
if let Some((start, end)) = to_const_range(&start, &end, range.limits, size) {
|
||||
if start > size || end > size {
|
||||
utils::span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span, "range is out of bounds");
|
||||
}
|
||||
@ -109,18 +109,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIndexing {
|
||||
|
||||
/// Returns an option containing a tuple with the start and end (exclusive) of the range.
|
||||
fn to_const_range(
|
||||
start: Option<Option<ConstVal>>,
|
||||
end: Option<Option<ConstVal>>,
|
||||
start: &Option<Option<ConstVal>>,
|
||||
end: &Option<Option<ConstVal>>,
|
||||
limits: RangeLimits,
|
||||
array_size: ConstInt
|
||||
) -> Option<(ConstInt, ConstInt)> {
|
||||
let start = match start {
|
||||
let start = match *start {
|
||||
Some(Some(ConstVal::Integral(x))) => x,
|
||||
Some(_) => return None,
|
||||
None => ConstInt::Infer(0),
|
||||
};
|
||||
|
||||
let end = match end {
|
||||
let end = match *end {
|
||||
Some(Some(ConstVal::Integral(x))) => {
|
||||
if limits == RangeLimits::Closed {
|
||||
(x + ConstInt::Infer(1)).expect("such a big array is not realistic")
|
||||
|
@ -203,9 +203,9 @@ pub fn lit_to_constant(lit: &LitKind) -> Constant {
|
||||
}
|
||||
}
|
||||
|
||||
fn constant_not(o: Constant) -> Option<Constant> {
|
||||
fn constant_not(o: &Constant) -> Option<Constant> {
|
||||
use self::Constant::*;
|
||||
match o {
|
||||
match *o {
|
||||
Bool(b) => Some(Bool(!b)),
|
||||
Int(value) => (!value).ok().map(Int),
|
||||
_ => None,
|
||||
@ -216,12 +216,12 @@ fn constant_negate(o: Constant) -> Option<Constant> {
|
||||
use self::Constant::*;
|
||||
match o {
|
||||
Int(value) => (-value).ok().map(Int),
|
||||
Float(is, ty) => Some(Float(neg_float_str(is), ty)),
|
||||
Float(is, ty) => Some(Float(neg_float_str(&is), ty)),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn neg_float_str(s: String) -> String {
|
||||
fn neg_float_str(s: &str) -> String {
|
||||
if s.starts_with('-') {
|
||||
s[1..].to_owned()
|
||||
} else {
|
||||
@ -271,7 +271,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
|
||||
},
|
||||
ExprUnary(op, ref operand) => {
|
||||
self.expr(operand).and_then(|o| match op {
|
||||
UnNot => constant_not(o),
|
||||
UnNot => constant_not(&o),
|
||||
UnNeg => constant_negate(o),
|
||||
UnDeref => Some(o),
|
||||
})
|
||||
|
@ -107,6 +107,7 @@ pub mod mut_reference;
|
||||
pub mod mutex_atomic;
|
||||
pub mod needless_bool;
|
||||
pub mod needless_borrow;
|
||||
pub mod needless_pass_by_value;
|
||||
pub mod needless_update;
|
||||
pub mod neg_multiply;
|
||||
pub mod new_without_default;
|
||||
@ -299,6 +300,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
|
||||
reg.register_late_lint_pass(box unused_io_amount::UnusedIoAmount);
|
||||
reg.register_late_lint_pass(box large_enum_variant::LargeEnumVariant::new(conf.enum_variant_size_threshold));
|
||||
reg.register_late_lint_pass(box should_assert_eq::ShouldAssertEq);
|
||||
reg.register_late_lint_pass(box needless_pass_by_value::NeedlessPassByValue);
|
||||
|
||||
reg.register_lint_group("clippy_restrictions", vec![
|
||||
arithmetic::FLOAT_ARITHMETIC,
|
||||
@ -455,6 +457,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
|
||||
needless_bool::BOOL_COMPARISON,
|
||||
needless_bool::NEEDLESS_BOOL,
|
||||
needless_borrow::NEEDLESS_BORROW,
|
||||
needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
|
||||
needless_update::NEEDLESS_UPDATE,
|
||||
neg_multiply::NEG_MULTIPLY,
|
||||
new_without_default::NEW_WITHOUT_DEFAULT,
|
||||
|
@ -549,8 +549,8 @@ fn check_for_loop_range<'a, 'tcx>(
|
||||
|db| {
|
||||
multispan_sugg(db,
|
||||
"consider using an iterator".to_string(),
|
||||
&[(pat.span, &format!("({}, <item>)", ident.node)),
|
||||
(arg.span, &format!("{}.iter().enumerate(){}{}", indexed, take, skip))]);
|
||||
vec![(pat.span, format!("({}, <item>)", ident.node)),
|
||||
(arg.span, format!("{}.iter().enumerate(){}{}", indexed, take, skip))]);
|
||||
});
|
||||
} else {
|
||||
let repl = if starts_at_zero && take.is_empty() {
|
||||
@ -568,7 +568,7 @@ fn check_for_loop_range<'a, 'tcx>(
|
||||
|db| {
|
||||
multispan_sugg(db,
|
||||
"consider using an iterator".to_string(),
|
||||
&[(pat.span, "<item>"), (arg.span, &repl)]);
|
||||
vec![(pat.span, "<item>".to_string()), (arg.span, repl)]);
|
||||
});
|
||||
}
|
||||
}
|
||||
@ -816,8 +816,8 @@ fn check_for_loop_over_map_kv<'a, 'tcx>(
|
||||
let map = sugg::Sugg::hir(cx, arg, "map");
|
||||
multispan_sugg(db,
|
||||
"use the corresponding method".into(),
|
||||
&[(pat_span, &snippet(cx, new_pat_span, kind)),
|
||||
(arg_span, &format!("{}.{}s{}()", map.maybe_par(), kind, mutbl))]);
|
||||
vec![(pat_span, snippet(cx, new_pat_span, kind).into_owned()),
|
||||
(arg_span, format!("{}.{}s{}()", map.maybe_par(), kind, mutbl))]);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
@ -395,7 +395,7 @@ fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: Match
|
||||
"you don't need to add `&` to both the expression and the patterns",
|
||||
|db| {
|
||||
let inner = Sugg::hir(cx, inner, "..");
|
||||
let template = match_template(expr.span, source, inner);
|
||||
let template = match_template(expr.span, source, &inner);
|
||||
db.span_suggestion(expr.span, "try", template);
|
||||
});
|
||||
} else {
|
||||
@ -405,7 +405,7 @@ fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: Match
|
||||
"you don't need to add `&` to all patterns",
|
||||
|db| {
|
||||
let ex = Sugg::hir(cx, ex, "..");
|
||||
let template = match_template(expr.span, source, ex.deref());
|
||||
let template = match_template(expr.span, source, &ex.deref());
|
||||
db.span_suggestion(expr.span,
|
||||
"instead of prefixing all patterns with `&`, you can dereference the expression",
|
||||
template);
|
||||
@ -509,7 +509,7 @@ fn has_only_ref_pats(arms: &[Arm]) -> bool {
|
||||
mapped.map_or(false, |v| v.iter().any(|el| *el))
|
||||
}
|
||||
|
||||
fn match_template(span: Span, source: MatchSource, expr: Sugg) -> String {
|
||||
fn match_template(span: Span, source: MatchSource, expr: &Sugg) -> String {
|
||||
match source {
|
||||
MatchSource::Normal => format!("match {} {{ .. }}", expr),
|
||||
MatchSource::IfLetDesugar { .. } => format!("if let .. = {} {{ .. }}", expr),
|
||||
|
297
clippy_lints/src/needless_pass_by_value.rs
Normal file
297
clippy_lints/src/needless_pass_by_value.rs
Normal file
@ -0,0 +1,297 @@
|
||||
use rustc::hir::*;
|
||||
use rustc::hir::intravisit::FnKind;
|
||||
use rustc::hir::def_id::DefId;
|
||||
use rustc::lint::*;
|
||||
use rustc::ty::{self, TypeFoldable};
|
||||
use rustc::traits;
|
||||
use rustc::middle::expr_use_visitor as euv;
|
||||
use rustc::middle::mem_categorization as mc;
|
||||
use syntax::ast::NodeId;
|
||||
use syntax_pos::Span;
|
||||
use syntax::errors::DiagnosticBuilder;
|
||||
use utils::{in_macro, is_self, is_copy, implements_trait, get_trait_def_id, match_type, snippet, span_lint_and_then,
|
||||
multispan_sugg, paths};
|
||||
use std::collections::{HashSet, HashMap};
|
||||
|
||||
/// **What it does:** Checks for functions taking arguments by value, but not consuming them in its
|
||||
/// body.
|
||||
///
|
||||
/// **Why is this bad?** Taking arguments by reference is more flexible and can sometimes avoid
|
||||
/// unnecessary allocations.
|
||||
///
|
||||
/// **Known problems:** Hopefully none.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// fn foo(v: Vec<i32>) {
|
||||
/// assert_eq!(v.len(), 42);
|
||||
/// }
|
||||
/// ```
|
||||
declare_lint! {
|
||||
pub NEEDLESS_PASS_BY_VALUE,
|
||||
Warn,
|
||||
"functions taking arguments by value, but not consuming them in its body"
|
||||
}
|
||||
|
||||
pub struct NeedlessPassByValue;
|
||||
|
||||
impl LintPass for NeedlessPassByValue {
|
||||
fn get_lints(&self) -> LintArray {
|
||||
lint_array![NEEDLESS_PASS_BY_VALUE]
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
|
||||
fn check_fn(
|
||||
&mut self,
|
||||
cx: &LateContext<'a, 'tcx>,
|
||||
kind: FnKind<'tcx>,
|
||||
decl: &'tcx FnDecl,
|
||||
body: &'tcx Body,
|
||||
span: Span,
|
||||
node_id: NodeId
|
||||
) {
|
||||
if in_macro(cx, span) {
|
||||
return;
|
||||
}
|
||||
|
||||
if !matches!(kind, FnKind::ItemFn(..)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Allows these to be passed by value.
|
||||
let fn_trait = cx.tcx.lang_items.fn_trait().expect("failed to find `Fn` trait");
|
||||
let asref_trait = get_trait_def_id(cx, &paths::ASREF_TRAIT).expect("failed to find `AsRef` trait");
|
||||
let borrow_trait = get_trait_def_id(cx, &paths::BORROW_TRAIT).expect("failed to find `Borrow` trait");
|
||||
|
||||
let preds: Vec<ty::Predicate> = {
|
||||
let parameter_env = ty::ParameterEnvironment::for_item(cx.tcx, node_id);
|
||||
traits::elaborate_predicates(cx.tcx, parameter_env.caller_bounds.clone())
|
||||
.filter(|p| !p.is_global())
|
||||
.collect()
|
||||
};
|
||||
|
||||
// Collect moved variables and spans which will need dereferencings from the function body.
|
||||
let MovedVariablesCtxt { moved_vars, spans_need_deref, .. } = {
|
||||
let mut ctx = MovedVariablesCtxt::new(cx);
|
||||
let infcx = cx.tcx.borrowck_fake_infer_ctxt(body.id());
|
||||
{
|
||||
let mut v = euv::ExprUseVisitor::new(&mut ctx, &infcx);
|
||||
v.consume_body(body);
|
||||
}
|
||||
ctx
|
||||
};
|
||||
|
||||
let fn_def_id = cx.tcx.hir.local_def_id(node_id);
|
||||
let param_env = ty::ParameterEnvironment::for_item(cx.tcx, node_id);
|
||||
let fn_sig = cx.tcx.item_type(fn_def_id).fn_sig();
|
||||
let fn_sig = cx.tcx.liberate_late_bound_regions(param_env.free_id_outlive, fn_sig);
|
||||
|
||||
for ((input, ty), arg) in decl.inputs.iter().zip(fn_sig.inputs()).zip(&body.arguments) {
|
||||
|
||||
// Determines whether `ty` implements `Borrow<U>` (U != ty) specifically.
|
||||
// This is needed due to the `Borrow<T> for T` blanket impl.
|
||||
let implements_borrow_trait = preds.iter()
|
||||
.filter_map(|pred| if let ty::Predicate::Trait(ref poly_trait_ref) = *pred {
|
||||
Some(poly_trait_ref.skip_binder())
|
||||
} else {
|
||||
None
|
||||
})
|
||||
.filter(|tpred| tpred.def_id() == borrow_trait && &tpred.self_ty() == ty)
|
||||
.any(|tpred| &tpred.input_types().nth(1).expect("Borrow trait must have an parameter") != ty);
|
||||
|
||||
if_let_chain! {[
|
||||
!is_self(arg),
|
||||
!ty.is_mutable_pointer(),
|
||||
!is_copy(cx, ty, node_id),
|
||||
!implements_trait(cx, ty, fn_trait, &[], Some(node_id)),
|
||||
!implements_trait(cx, ty, asref_trait, &[], Some(node_id)),
|
||||
!implements_borrow_trait,
|
||||
|
||||
let PatKind::Binding(mode, defid, ..) = arg.pat.node,
|
||||
!moved_vars.contains(&defid),
|
||||
], {
|
||||
// Note: `toplevel_ref_arg` warns if `BindByRef`
|
||||
let m = match mode {
|
||||
BindingMode::BindByRef(m) | BindingMode::BindByValue(m) => m,
|
||||
};
|
||||
if m == Mutability::MutMutable {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Suggestion logic
|
||||
let sugg = |db: &mut DiagnosticBuilder| {
|
||||
let deref_span = spans_need_deref.get(&defid);
|
||||
if_let_chain! {[
|
||||
match_type(cx, ty, &paths::VEC),
|
||||
let TyPath(QPath::Resolved(_, ref path)) = input.node,
|
||||
let Some(elem_ty) = path.segments.iter()
|
||||
.find(|seg| &*seg.name.as_str() == "Vec")
|
||||
.map(|ps| ps.parameters.types()[0]),
|
||||
], {
|
||||
let slice_ty = format!("&[{}]", snippet(cx, elem_ty.span, "_"));
|
||||
db.span_suggestion(input.span,
|
||||
&format!("consider changing the type to `{}`", slice_ty),
|
||||
slice_ty);
|
||||
assert!(deref_span.is_none());
|
||||
return; // `Vec` and `String` cannot be destructured - no need for `*` suggestion
|
||||
}}
|
||||
|
||||
if match_type(cx, ty, &paths::STRING) {
|
||||
db.span_suggestion(input.span,
|
||||
"consider changing the type to `&str`",
|
||||
"&str".to_string());
|
||||
assert!(deref_span.is_none());
|
||||
return;
|
||||
}
|
||||
|
||||
let mut spans = vec![(input.span, format!("&{}", snippet(cx, input.span, "_")))];
|
||||
|
||||
// Suggests adding `*` to dereference the added reference.
|
||||
if let Some(deref_span) = deref_span {
|
||||
spans.extend(deref_span.iter().cloned()
|
||||
.map(|span| (span, format!("*{}", snippet(cx, span, "<expr>")))));
|
||||
spans.sort_by_key(|&(span, _)| span);
|
||||
}
|
||||
multispan_sugg(db, "consider taking a reference instead".to_string(), spans);
|
||||
};
|
||||
|
||||
span_lint_and_then(cx,
|
||||
NEEDLESS_PASS_BY_VALUE,
|
||||
input.span,
|
||||
"this argument is passed by value, but not consumed in the function body",
|
||||
sugg);
|
||||
}}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
struct MovedVariablesCtxt<'a, 'tcx: 'a> {
|
||||
cx: &'a LateContext<'a, 'tcx>,
|
||||
moved_vars: HashSet<DefId>,
|
||||
/// Spans which need to be prefixed with `*` for dereferencing the suggested additional
|
||||
/// reference.
|
||||
spans_need_deref: HashMap<DefId, HashSet<Span>>,
|
||||
}
|
||||
|
||||
impl<'a, 'tcx: 'a> MovedVariablesCtxt<'a, 'tcx> {
|
||||
fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
|
||||
MovedVariablesCtxt {
|
||||
cx: cx,
|
||||
moved_vars: HashSet::new(),
|
||||
spans_need_deref: HashMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn move_common(&mut self, _consume_id: NodeId, _span: Span, cmt: mc::cmt<'tcx>) {
|
||||
let cmt = unwrap_downcast_or_interior(cmt);
|
||||
|
||||
if_let_chain! {[
|
||||
let mc::Categorization::Local(vid) = cmt.cat,
|
||||
let Some(def_id) = self.cx.tcx.hir.opt_local_def_id(vid),
|
||||
], {
|
||||
self.moved_vars.insert(def_id);
|
||||
}}
|
||||
}
|
||||
|
||||
fn non_moving_pat(&mut self, matched_pat: &Pat, cmt: mc::cmt<'tcx>) {
|
||||
let cmt = unwrap_downcast_or_interior(cmt);
|
||||
|
||||
if_let_chain! {[
|
||||
let mc::Categorization::Local(vid) = cmt.cat,
|
||||
let Some(def_id) = self.cx.tcx.hir.opt_local_def_id(vid),
|
||||
], {
|
||||
let mut id = matched_pat.id;
|
||||
loop {
|
||||
let parent = self.cx.tcx.hir.get_parent_node(id);
|
||||
if id == parent {
|
||||
// no parent
|
||||
return;
|
||||
}
|
||||
id = parent;
|
||||
|
||||
if let Some(node) = self.cx.tcx.hir.find(id) {
|
||||
match node {
|
||||
map::Node::NodeExpr(e) => {
|
||||
// `match` and `if let`
|
||||
if let ExprMatch(ref c, ..) = e.node {
|
||||
self.spans_need_deref
|
||||
.entry(def_id)
|
||||
.or_insert_with(HashSet::new)
|
||||
.insert(c.span);
|
||||
}
|
||||
}
|
||||
|
||||
map::Node::NodeStmt(s) => {
|
||||
// `let <pat> = x;`
|
||||
if_let_chain! {[
|
||||
let StmtDecl(ref decl, _) = s.node,
|
||||
let DeclLocal(ref local) = decl.node,
|
||||
], {
|
||||
self.spans_need_deref
|
||||
.entry(def_id)
|
||||
.or_insert_with(HashSet::new)
|
||||
.insert(local.init
|
||||
.as_ref()
|
||||
.map(|e| e.span)
|
||||
.expect("`let` stmt without init aren't caught by match_pat"));
|
||||
}}
|
||||
}
|
||||
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
}}
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a, 'tcx: 'a> euv::Delegate<'tcx> for MovedVariablesCtxt<'a, 'tcx> {
|
||||
fn consume(&mut self, consume_id: NodeId, consume_span: Span, cmt: mc::cmt<'tcx>, mode: euv::ConsumeMode) {
|
||||
if let euv::ConsumeMode::Move(_) = mode {
|
||||
self.move_common(consume_id, consume_span, cmt);
|
||||
}
|
||||
}
|
||||
|
||||
fn matched_pat(&mut self, matched_pat: &Pat, cmt: mc::cmt<'tcx>, mode: euv::MatchMode) {
|
||||
if let euv::MatchMode::MovingMatch = mode {
|
||||
self.move_common(matched_pat.id, matched_pat.span, cmt);
|
||||
} else {
|
||||
self.non_moving_pat(matched_pat, cmt);
|
||||
}
|
||||
}
|
||||
|
||||
fn consume_pat(&mut self, consume_pat: &Pat, cmt: mc::cmt<'tcx>, mode: euv::ConsumeMode) {
|
||||
if let euv::ConsumeMode::Move(_) = mode {
|
||||
self.move_common(consume_pat.id, consume_pat.span, cmt);
|
||||
}
|
||||
}
|
||||
|
||||
fn borrow(
|
||||
&mut self,
|
||||
_: NodeId,
|
||||
_: Span,
|
||||
_: mc::cmt<'tcx>,
|
||||
_: &'tcx ty::Region,
|
||||
_: ty::BorrowKind,
|
||||
_: euv::LoanCause
|
||||
) {
|
||||
}
|
||||
|
||||
fn mutate(&mut self, _: NodeId, _: Span, _: mc::cmt<'tcx>, _: euv::MutateMode) {}
|
||||
|
||||
fn decl_without_init(&mut self, _: NodeId, _: Span) {}
|
||||
}
|
||||
|
||||
|
||||
fn unwrap_downcast_or_interior(mut cmt: mc::cmt) -> mc::cmt {
|
||||
loop {
|
||||
match cmt.cat.clone() {
|
||||
mc::Categorization::Downcast(c, _) |
|
||||
mc::Categorization::Interior(c, _) => {
|
||||
cmt = c;
|
||||
},
|
||||
_ => return cmt,
|
||||
}
|
||||
}
|
||||
}
|
@ -573,10 +573,10 @@ pub fn span_lint_and_then<'a, 'tcx: 'a, T: LintContext<'tcx>, F>(
|
||||
///
|
||||
/// Note: in the JSON format (used by `compiletest_rs`), the help message will appear once per
|
||||
/// replacement. In human-readable format though, it only appears once before the whole suggestion.
|
||||
pub fn multispan_sugg(db: &mut DiagnosticBuilder, help_msg: String, sugg: &[(Span, &str)]) {
|
||||
pub fn multispan_sugg(db: &mut DiagnosticBuilder, help_msg: String, sugg: Vec<(Span, String)>) {
|
||||
let sugg = rustc_errors::RenderSpan::Suggestion(rustc_errors::CodeSuggestion {
|
||||
msp: MultiSpan::from_spans(sugg.iter().map(|&(span, _)| span).collect()),
|
||||
substitutes: sugg.iter().map(|&(_, subs)| subs.to_owned()).collect(),
|
||||
substitutes: sugg.into_iter().map(|(_, subs)| subs).collect(),
|
||||
});
|
||||
|
||||
let sub = rustc_errors::SubDiagnostic {
|
||||
|
@ -1,7 +1,9 @@
|
||||
//! This module contains paths to types and functions Clippy needs to know about.
|
||||
|
||||
pub const ASREF_TRAIT: [&'static str; 3] = ["core", "convert", "AsRef"];
|
||||
pub const BEGIN_PANIC: [&'static str; 3] = ["std", "panicking", "begin_panic"];
|
||||
pub const BINARY_HEAP: [&'static str; 3] = ["collections", "binary_heap", "BinaryHeap"];
|
||||
pub const BORROW_TRAIT: [&'static str; 3] = ["core", "borrow", "Borrow"];
|
||||
pub const BOX: [&'static str; 3] = ["std", "boxed", "Box"];
|
||||
pub const BOX_NEW: [&'static str; 4] = ["std", "boxed", "Box", "new"];
|
||||
pub const BTREEMAP: [&'static str; 4] = ["collections", "btree", "map", "BTreeMap"];
|
||||
|
@ -301,7 +301,7 @@ pub fn make_binop(op: ast::BinOpKind, lhs: &Sugg, rhs: &Sugg) -> Sugg<'static> {
|
||||
make_assoc(AssocOp::from_ast_binop(op), lhs, rhs)
|
||||
}
|
||||
|
||||
#[derive(PartialEq, Eq)]
|
||||
#[derive(PartialEq, Eq, Clone, Copy)]
|
||||
/// Operator associativity.
|
||||
enum Associativity {
|
||||
/// The operator is both left-associative and right-associative.
|
||||
|
@ -4,7 +4,7 @@
|
||||
#![allow(warnings)]
|
||||
|
||||
// this should compile in a reasonable amount of time
|
||||
fn rust_type_id(name: String) {
|
||||
fn rust_type_id(name: &str) {
|
||||
if "bool" == &name[..] || "uint" == &name[..] || "u8" == &name[..] || "u16" == &name[..] ||
|
||||
"u32" == &name[..] || "f32" == &name[..] || "f64" == &name[..] || "i8" == &name[..] ||
|
||||
"i16" == &name[..] || "i32" == &name[..] ||
|
||||
|
@ -2,7 +2,7 @@
|
||||
#![plugin(clippy)]
|
||||
|
||||
#![deny(absurd_extreme_comparisons)]
|
||||
#![allow(unused, eq_op, no_effect, unnecessary_operation)]
|
||||
#![allow(unused, eq_op, no_effect, unnecessary_operation, needless_pass_by_value)]
|
||||
|
||||
fn main() {
|
||||
const Z: u32 = 0;
|
||||
|
@ -2,7 +2,7 @@
|
||||
#![plugin(clippy)]
|
||||
|
||||
#![deny(clippy)]
|
||||
#![allow(boxed_local)]
|
||||
#![allow(boxed_local, needless_pass_by_value)]
|
||||
#![allow(blacklisted_name)]
|
||||
|
||||
macro_rules! boxit {
|
||||
|
@ -1,7 +1,7 @@
|
||||
#![feature(plugin)]
|
||||
#![plugin(clippy)]
|
||||
#![deny(clippy)]
|
||||
#![allow(unused)]
|
||||
#![allow(unused, needless_pass_by_value)]
|
||||
#![feature(associated_consts, associated_type_defaults)]
|
||||
|
||||
type Alias = Vec<Vec<Box<(u32, u32, u32, u32)>>>; // no warning here
|
||||
|
@ -4,7 +4,7 @@
|
||||
|
||||
#![plugin(clippy)]
|
||||
#![deny(clippy)]
|
||||
#![allow(dead_code)]
|
||||
#![allow(dead_code, needless_pass_by_value)]
|
||||
|
||||
extern crate collections;
|
||||
use collections::linked_list::LinkedList;
|
||||
|
@ -2,7 +2,7 @@
|
||||
#![plugin(clippy)]
|
||||
|
||||
#![deny(drop_ref, forget_ref)]
|
||||
#![allow(toplevel_ref_arg, similar_names)]
|
||||
#![allow(toplevel_ref_arg, similar_names, needless_pass_by_value)]
|
||||
|
||||
use std::mem::{drop, forget};
|
||||
|
||||
|
@ -1,6 +1,6 @@
|
||||
#![feature(plugin)]
|
||||
#![plugin(clippy)]
|
||||
#![allow(unused)]
|
||||
#![allow(unused, needless_pass_by_value)]
|
||||
|
||||
#![deny(map_entry)]
|
||||
|
||||
|
@ -1,6 +1,6 @@
|
||||
#![feature(plugin)]
|
||||
#![plugin(clippy)]
|
||||
#![allow(unknown_lints, unused, no_effect, redundant_closure_call, many_single_char_names)]
|
||||
#![allow(unknown_lints, unused, no_effect, redundant_closure_call, many_single_char_names, needless_pass_by_value)]
|
||||
#![deny(redundant_closure)]
|
||||
|
||||
fn main() {
|
||||
|
@ -2,7 +2,7 @@
|
||||
#![plugin(clippy)]
|
||||
|
||||
#![deny(needless_lifetimes, unused_lifetimes)]
|
||||
#![allow(dead_code)]
|
||||
#![allow(dead_code, needless_pass_by_value)]
|
||||
|
||||
fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) { }
|
||||
|
||||
|
62
tests/ui/needless_pass_by_value.rs
Normal file
62
tests/ui/needless_pass_by_value.rs
Normal file
@ -0,0 +1,62 @@
|
||||
#![feature(plugin)]
|
||||
#![plugin(clippy)]
|
||||
|
||||
#![deny(needless_pass_by_value)]
|
||||
#![allow(dead_code, single_match, if_let_redundant_pattern_matching, many_single_char_names)]
|
||||
|
||||
// `v` should be warned
|
||||
// `w`, `x` and `y` are allowed (moved or mutated)
|
||||
fn foo<T: Default>(v: Vec<T>, w: Vec<T>, mut x: Vec<T>, y: Vec<T>) -> Vec<T> {
|
||||
assert_eq!(v.len(), 42);
|
||||
|
||||
consume(w);
|
||||
|
||||
x.push(T::default());
|
||||
|
||||
y
|
||||
}
|
||||
|
||||
fn consume<T>(_: T) {}
|
||||
|
||||
struct Wrapper(String);
|
||||
|
||||
fn bar(x: String, y: Wrapper) {
|
||||
assert_eq!(x.len(), 42);
|
||||
assert_eq!(y.0.len(), 42);
|
||||
}
|
||||
|
||||
// U implements `Borrow<U>`, but should be warned correctly
|
||||
fn test_borrow_trait<T: std::borrow::Borrow<str>, U>(t: T, u: U) {
|
||||
println!("{}", t.borrow());
|
||||
consume(&u);
|
||||
}
|
||||
|
||||
// ok
|
||||
fn test_fn<F: Fn(i32) -> i32>(f: F) {
|
||||
f(1);
|
||||
}
|
||||
|
||||
// x should be warned, but y is ok
|
||||
fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
|
||||
match x {
|
||||
Some(Some(_)) => 1, // not moved
|
||||
_ => 0,
|
||||
};
|
||||
|
||||
match y {
|
||||
Some(Some(s)) => consume(s), // moved
|
||||
_ => (),
|
||||
};
|
||||
}
|
||||
|
||||
// x and y should be warned, but z is ok
|
||||
fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
|
||||
let Wrapper(s) = z; // moved
|
||||
let Wrapper(ref t) = y; // not moved
|
||||
let Wrapper(_) = y; // still not moved
|
||||
|
||||
assert_eq!(x.0.len(), s.len());
|
||||
println!("{}", t);
|
||||
}
|
||||
|
||||
fn main() {}
|
74
tests/ui/needless_pass_by_value.stderr
Normal file
74
tests/ui/needless_pass_by_value.stderr
Normal file
@ -0,0 +1,74 @@
|
||||
error: this argument is passed by value, but not consumed in the function body
|
||||
--> $DIR/needless_pass_by_value.rs:9:23
|
||||
|
|
||||
9 | fn foo<T: Default>(v: Vec<T>, w: Vec<T>, mut x: Vec<T>, y: Vec<T>) -> Vec<T> {
|
||||
| ^^^^^^
|
||||
|
|
||||
note: lint level defined here
|
||||
--> $DIR/needless_pass_by_value.rs:4:9
|
||||
|
|
||||
4 | #![deny(needless_pass_by_value)]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
help: consider changing the type to `&[T]`
|
||||
| fn foo<T: Default>(v: &[T], w: Vec<T>, mut x: Vec<T>, y: Vec<T>) -> Vec<T> {
|
||||
|
||||
error: this argument is passed by value, but not consumed in the function body
|
||||
--> $DIR/needless_pass_by_value.rs:23:11
|
||||
|
|
||||
23 | fn bar(x: String, y: Wrapper) {
|
||||
| ^^^^^^
|
||||
|
|
||||
help: consider changing the type to `&str`
|
||||
| fn bar(x: &str, y: Wrapper) {
|
||||
|
||||
error: this argument is passed by value, but not consumed in the function body
|
||||
--> $DIR/needless_pass_by_value.rs:23:22
|
||||
|
|
||||
23 | fn bar(x: String, y: Wrapper) {
|
||||
| ^^^^^^^
|
||||
|
|
||||
help: consider taking a reference instead
|
||||
| fn bar(x: String, y: &Wrapper) {
|
||||
|
||||
error: this argument is passed by value, but not consumed in the function body
|
||||
--> $DIR/needless_pass_by_value.rs:29:63
|
||||
|
|
||||
29 | fn test_borrow_trait<T: std::borrow::Borrow<str>, U>(t: T, u: U) {
|
||||
| ^
|
||||
|
|
||||
help: consider taking a reference instead
|
||||
| fn test_borrow_trait<T: std::borrow::Borrow<str>, U>(t: T, u: &U) {
|
||||
|
||||
error: this argument is passed by value, but not consumed in the function body
|
||||
--> $DIR/needless_pass_by_value.rs:40:18
|
||||
|
|
||||
40 | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: consider taking a reference instead
|
||||
| fn test_match(x: &Option<Option<String>>, y: Option<Option<String>>) {
|
||||
| match *x {
|
||||
|
||||
error: this argument is passed by value, but not consumed in the function body
|
||||
--> $DIR/needless_pass_by_value.rs:53:24
|
||||
|
|
||||
53 | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
|
||||
| ^^^^^^^
|
||||
|
|
||||
help: consider taking a reference instead
|
||||
| fn test_destructure(x: &Wrapper, y: Wrapper, z: Wrapper) {
|
||||
|
||||
error: this argument is passed by value, but not consumed in the function body
|
||||
--> $DIR/needless_pass_by_value.rs:53:36
|
||||
|
|
||||
53 | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
|
||||
| ^^^^^^^
|
||||
|
|
||||
help: consider taking a reference instead
|
||||
| fn test_destructure(x: Wrapper, y: &Wrapper, z: Wrapper) {
|
||||
| let Wrapper(s) = z; // moved
|
||||
| let Wrapper(ref t) = *y; // not moved
|
||||
| let Wrapper(_) = *y; // still not moved
|
||||
|
||||
error: aborting due to 7 previous errors
|
||||
|
@ -1,6 +1,7 @@
|
||||
#![feature(plugin)]
|
||||
#![plugin(clippy)]
|
||||
|
||||
#![allow(needless_pass_by_value)]
|
||||
#![deny(should_assert_eq)]
|
||||
|
||||
#[derive(PartialEq, Eq)]
|
||||
|
@ -1,28 +1,28 @@
|
||||
error: use `assert_eq` for better reporting
|
||||
--> $DIR/should_assert_eq.rs:13:5
|
||||
--> $DIR/should_assert_eq.rs:14:5
|
||||
|
|
||||
13 | assert!(1 == 2);
|
||||
14 | assert!(1 == 2);
|
||||
| ^^^^^^^^^^^^^^^^
|
||||
|
|
||||
note: lint level defined here
|
||||
--> $DIR/should_assert_eq.rs:4:9
|
||||
--> $DIR/should_assert_eq.rs:5:9
|
||||
|
|
||||
4 | #![deny(should_assert_eq)]
|
||||
5 | #![deny(should_assert_eq)]
|
||||
| ^^^^^^^^^^^^^^^^
|
||||
= note: this error originates in a macro outside of the current crate
|
||||
|
||||
error: use `assert_eq` for better reporting
|
||||
--> $DIR/should_assert_eq.rs:14:5
|
||||
--> $DIR/should_assert_eq.rs:15:5
|
||||
|
|
||||
14 | assert!(Debug(1) == Debug(2));
|
||||
15 | assert!(Debug(1) == Debug(2));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: this error originates in a macro outside of the current crate
|
||||
|
||||
error: use `assert_eq` for better reporting
|
||||
--> $DIR/should_assert_eq.rs:21:5
|
||||
--> $DIR/should_assert_eq.rs:22:5
|
||||
|
|
||||
21 | assert!(x == y);
|
||||
22 | assert!(x == y);
|
||||
| ^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: this error originates in a macro outside of the current crate
|
||||
|
@ -1,6 +1,6 @@
|
||||
#![feature(plugin)]
|
||||
#![plugin(clippy)]
|
||||
#![allow(unused, dead_code, needless_lifetimes)]
|
||||
#![allow(unused, dead_code, needless_lifetimes, needless_pass_by_value)]
|
||||
#![deny(unused_lifetimes)]
|
||||
|
||||
fn empty() {
|
||||
|
Loading…
Reference in New Issue
Block a user