diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/consts.rs | 8 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 5 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 4 | ||||
| -rw-r--r-- | clippy_lints/src/needless_pass_by_value.rs | 3 | ||||
| -rw-r--r-- | clippy_lints/src/unit_return_expecting_ord.rs | 177 | ||||
| -rw-r--r-- | src/lintlist/mod.rs | 7 | ||||
| -rw-r--r-- | tests/compile-test.rs | 3 | ||||
| -rw-r--r-- | tests/ui/iter_nth_zero.stderr | 12 | ||||
| -rw-r--r-- | tests/ui/needless_range_loop2.rs | 10 | ||||
| -rw-r--r-- | tests/ui/unit_return_expecting_ord.rs | 36 | ||||
| -rw-r--r-- | tests/ui/unit_return_expecting_ord.stderr | 39 | ||||
| -rwxr-xr-x | util/dev | 7 |
13 files changed, 291 insertions, 21 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d08b44ba40..1c927b5f83a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1679,6 +1679,7 @@ Released 2018-09-13 [`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init [`unit_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg [`unit_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_cmp +[`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord [`unknown_clippy_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints [`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast [`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index e6ef54f528f..5917968c973 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -332,7 +332,13 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { let result = self .lcx .tcx - .const_eval_resolve(self.param_env, ty::WithOptConstParam::unknown(def_id), substs, None, None) + .const_eval_resolve( + self.param_env, + ty::WithOptConstParam::unknown(def_id), + substs, + None, + None, + ) .ok() .map(|val| rustc_middle::ty::Const::from_value(self.lcx.tcx, val, ty))?; let result = miri_to_const(&result); diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 32e79317f82..7a4ca3902b3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -300,6 +300,7 @@ mod trivially_copy_pass_by_ref; mod try_err; mod types; mod unicode; +mod unit_return_expecting_ord; mod unnamed_address; mod unnecessary_sort_by; mod unnested_or_patterns; @@ -826,6 +827,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &unicode::NON_ASCII_LITERAL, &unicode::UNICODE_NOT_NFC, &unicode::ZERO_WIDTH_SPACE, + &unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD, &unnamed_address::FN_ADDRESS_COMPARISONS, &unnamed_address::VTABLE_ADDRESS_COMPARISONS, &unnecessary_sort_by::UNNECESSARY_SORT_BY, @@ -891,6 +893,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box attrs::Attributes); store.register_late_pass(|| box blocks_in_if_conditions::BlocksInIfConditions); store.register_late_pass(|| box unicode::Unicode); + store.register_late_pass(|| box unit_return_expecting_ord::UnitReturnExpectingOrd); store.register_late_pass(|| box strings::StringAdd); store.register_late_pass(|| box implicit_return::ImplicitReturn); store.register_late_pass(|| box implicit_saturating_sub::ImplicitSaturatingSub); @@ -1436,6 +1439,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), LintId::of(&unicode::ZERO_WIDTH_SPACE), + LintId::of(&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD), LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS), LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS), LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), @@ -1692,6 +1696,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::CAST_REF_TO_MUT), LintId::of(&types::UNIT_CMP), LintId::of(&unicode::ZERO_WIDTH_SPACE), + LintId::of(&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD), LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS), LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS), LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 4c595029ff7..69e985cc0a4 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2348,8 +2348,8 @@ fn lint_iter_nth_zero<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, nth_ar cx, ITER_NTH_ZERO, expr.span, - "called `.nth(0)` on a `std::iter::Iterator`", - "try calling", + "called `.nth(0)` on a `std::iter::Iterator`, when `.next()` is equivalent", + "try calling `.next()` instead of `.nth(0)`", format!("{}.next()", snippet_with_applicability(cx, nth_args[0].span, "..", &mut applicability)), applicability, ); diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index 29e5d4d1664..1d756477be8 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -40,9 +40,8 @@ declare_clippy_lint! { /// assert_eq!(v.len(), 42); /// } /// ``` - /// + /// should be /// ```rust - /// // should be /// fn foo(v: &[i32]) { /// assert_eq!(v.len(), 42); /// } diff --git a/clippy_lints/src/unit_return_expecting_ord.rs b/clippy_lints/src/unit_return_expecting_ord.rs new file mode 100644 index 00000000000..fceb885516b --- /dev/null +++ b/clippy_lints/src/unit_return_expecting_ord.rs @@ -0,0 +1,177 @@ +use crate::utils::{get_trait_def_id, paths, span_lint, span_lint_and_help}; +use if_chain::if_chain; +use rustc_hir::def_id::DefId; +use rustc_hir::{Expr, ExprKind, StmtKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_middle::ty::{GenericPredicates, PredicateKind, ProjectionPredicate, TraitPredicate}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::{BytePos, Span}; + +declare_clippy_lint! { + /// **What it does:** Checks for functions that expect closures of type + /// Fn(...) -> Ord where the implemented closure returns the unit type. + /// The lint also suggests to remove the semi-colon at the end of the statement if present. + /// + /// **Why is this bad?** Likely, returning the unit type is unintentional, and + /// could simply be caused by an extra semi-colon. Since () implements Ord + /// it doesn't cause a compilation error. + /// This is the same reasoning behind the unit_cmp lint. + /// + /// **Known problems:** If returning unit is intentional, then there is no + /// way of specifying this without triggering needless_return lint + /// + /// **Example:** + /// + /// ```rust + /// let mut twins = vec!((1,1), (2,2)); + /// twins.sort_by_key(|x| { x.1; }); + /// ``` + pub UNIT_RETURN_EXPECTING_ORD, + correctness, + "fn arguments of type Fn(...) -> Ord returning the unit type ()." +} + +declare_lint_pass!(UnitReturnExpectingOrd => [UNIT_RETURN_EXPECTING_ORD]); + +fn get_trait_predicates_for_trait_id<'tcx>( + cx: &LateContext<'tcx>, + generics: GenericPredicates<'tcx>, + trait_id: Option<DefId>, +) -> Vec<TraitPredicate<'tcx>> { + let mut preds = Vec::new(); + for (pred, _) in generics.predicates { + if_chain! { + if let PredicateKind::Trait(poly_trait_pred, _) = pred.kind(); + let trait_pred = cx.tcx.erase_late_bound_regions(&poly_trait_pred); + if let Some(trait_def_id) = trait_id; + if trait_def_id == trait_pred.trait_ref.def_id; + then { + preds.push(trait_pred); + } + } + } + preds +} + +fn get_projection_pred<'tcx>( + cx: &LateContext<'tcx>, + generics: GenericPredicates<'tcx>, + pred: TraitPredicate<'tcx>, +) -> Option<ProjectionPredicate<'tcx>> { + generics.predicates.iter().find_map(|(proj_pred, _)| { + if let PredicateKind::Projection(proj_pred) = proj_pred.kind() { + let projection_pred = cx.tcx.erase_late_bound_regions(proj_pred); + if projection_pred.projection_ty.substs == pred.trait_ref.substs { + return Some(projection_pred); + } + } + None + }) +} + +fn get_args_to_check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Vec<(usize, String)> { + let mut args_to_check = Vec::new(); + if let Some(def_id) = cx.tables().type_dependent_def_id(expr.hir_id) { + let fn_sig = cx.tcx.fn_sig(def_id); + let generics = cx.tcx.predicates_of(def_id); + let fn_mut_preds = get_trait_predicates_for_trait_id(cx, generics, cx.tcx.lang_items().fn_mut_trait()); + let ord_preds = get_trait_predicates_for_trait_id(cx, generics, get_trait_def_id(cx, &paths::ORD)); + let partial_ord_preds = + get_trait_predicates_for_trait_id(cx, generics, cx.tcx.lang_items().partial_ord_trait()); + // Trying to call erase_late_bound_regions on fn_sig.inputs() gives the following error + // The trait `rustc::ty::TypeFoldable<'_>` is not implemented for `&[&rustc::ty::TyS<'_>]` + let inputs_output = cx.tcx.erase_late_bound_regions(&fn_sig.inputs_and_output()); + inputs_output + .iter() + .rev() + .skip(1) + .rev() + .enumerate() + .for_each(|(i, inp)| { + for trait_pred in &fn_mut_preds { + if_chain! { + if trait_pred.self_ty() == inp; + if let Some(return_ty_pred) = get_projection_pred(cx, generics, *trait_pred); + then { + if ord_preds.iter().any(|ord| ord.self_ty() == return_ty_pred.ty) { + args_to_check.push((i, "Ord".to_string())); + } else if partial_ord_preds.iter().any(|pord| pord.self_ty() == return_ty_pred.ty) { + args_to_check.push((i, "PartialOrd".to_string())); + } + } + } + } + }); + } + args_to_check +} + +fn check_arg<'tcx>(cx: &LateContext<'tcx>, arg: &'tcx Expr<'tcx>) -> Option<(Span, Option<Span>)> { + if_chain! { + if let ExprKind::Closure(_, _fn_decl, body_id, span, _) = arg.kind; + if let ty::Closure(_def_id, substs) = &cx.tables().node_type(arg.hir_id).kind; + let ret_ty = substs.as_closure().sig().output(); + let ty = cx.tcx.erase_late_bound_regions(&ret_ty); + if ty.is_unit(); + then { + if_chain! { + let body = cx.tcx.hir().body(body_id); + if let ExprKind::Block(block, _) = body.value.kind; + if block.expr.is_none(); + if let Some(stmt) = block.stmts.last(); + if let StmtKind::Semi(_) = stmt.kind; + then { + let data = stmt.span.data(); + // Make a span out of the semicolon for the help message + Some((span, Some(Span::new(data.hi-BytePos(1), data.hi, data.ctxt)))) + } else { + Some((span, None)) + } + } + } else { + None + } + } +} + +impl<'tcx> LateLintPass<'tcx> for UnitReturnExpectingOrd { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if let ExprKind::MethodCall(_, _, ref args, _) = expr.kind { + let arg_indices = get_args_to_check(cx, expr); + for (i, trait_name) in arg_indices { + if i < args.len() { + match check_arg(cx, &args[i]) { + Some((span, None)) => { + span_lint( + cx, + UNIT_RETURN_EXPECTING_ORD, + span, + &format!( + "this closure returns \ + the unit type which also implements {}", + trait_name + ), + ); + }, + Some((span, Some(last_semi))) => { + span_lint_and_help( + cx, + UNIT_RETURN_EXPECTING_ORD, + span, + &format!( + "this closure returns \ + the unit type which also implements {}", + trait_name + ), + Some(last_semi), + &"probably caused by this trailing semicolon".to_string(), + ); + }, + None => {}, + } + } + } + } + } +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index b89a8712862..96b004904aa 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -2293,6 +2293,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![ module: "types", }, Lint { + name: "unit_return_expecting_ord", + group: "correctness", + desc: "fn arguments of type Fn(...) -> Ord returning the unit type ().", + deprecation: None, + module: "unit_return_expecting_ord", + }, + Lint { name: "unknown_clippy_lints", group: "style", desc: "unknown_lints for scoped Clippy lints", diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 26a47d23706..eb6d495acbe 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -147,9 +147,6 @@ fn run_ui_toml(config: &mut compiletest::Config) { } fn run_ui_cargo(config: &mut compiletest::Config) { - if cargo::is_rustc_test_suite() { - return; - } fn run_tests( config: &compiletest::Config, filter: &Option<String>, diff --git a/tests/ui/iter_nth_zero.stderr b/tests/ui/iter_nth_zero.stderr index 2b20a4ceb4a..29c56f3a94f 100644 --- a/tests/ui/iter_nth_zero.stderr +++ b/tests/ui/iter_nth_zero.stderr @@ -1,22 +1,22 @@ -error: called `.nth(0)` on a `std::iter::Iterator` +error: called `.nth(0)` on a `std::iter::Iterator`, when `.next()` is equivalent --> $DIR/iter_nth_zero.rs:20:14 | LL | let _x = s.iter().nth(0); - | ^^^^^^^^^^^^^^^ help: try calling: `s.iter().next()` + | ^^^^^^^^^^^^^^^ help: try calling `.next()` instead of `.nth(0)`: `s.iter().next()` | = note: `-D clippy::iter-nth-zero` implied by `-D warnings` -error: called `.nth(0)` on a `std::iter::Iterator` +error: called `.nth(0)` on a `std::iter::Iterator`, when `.next()` is equivalent --> $DIR/iter_nth_zero.rs:25:14 | LL | let _y = iter.nth(0); - | ^^^^^^^^^^^ help: try calling: `iter.next()` + | ^^^^^^^^^^^ help: try calling `.next()` instead of `.nth(0)`: `iter.next()` -error: called `.nth(0)` on a `std::iter::Iterator` +error: called `.nth(0)` on a `std::iter::Iterator`, when `.next()` is equivalent --> $DIR/iter_nth_zero.rs:30:22 | LL | let _unwrapped = iter2.nth(0).unwrap(); - | ^^^^^^^^^^^^ help: try calling: `iter2.next()` + | ^^^^^^^^^^^^ help: try calling `.next()` instead of `.nth(0)`: `iter2.next()` error: aborting due to 3 previous errors diff --git a/tests/ui/needless_range_loop2.rs b/tests/ui/needless_range_loop2.rs index 2ed1b09bece..a82b1159161 100644 --- a/tests/ui/needless_range_loop2.rs +++ b/tests/ui/needless_range_loop2.rs @@ -83,3 +83,13 @@ fn main() { println!("{}", arr[i]); } } + +mod issue2277 { + pub fn example(list: &[[f64; 3]]) { + let mut x: [f64; 3] = [10.; 3]; + + for i in 0..3 { + x[i] = list.iter().map(|item| item[i]).sum::<f64>(); + } + } +} diff --git a/tests/ui/unit_return_expecting_ord.rs b/tests/ui/unit_return_expecting_ord.rs new file mode 100644 index 00000000000..bdb4710cc69 --- /dev/null +++ b/tests/ui/unit_return_expecting_ord.rs @@ -0,0 +1,36 @@ +#![warn(clippy::unit_return_expecting_ord)] +#![allow(clippy::needless_return)] +#![allow(clippy::unused_unit)] +#![feature(is_sorted)] + +struct Struct { + field: isize, +} + +fn double(i: isize) -> isize { + i * 2 +} + +fn unit(_i: isize) {} + +fn main() { + let mut structs = vec![Struct { field: 2 }, Struct { field: 1 }]; + structs.sort_by_key(|s| { + double(s.field); + }); + structs.sort_by_key(|s| double(s.field)); + structs.is_sorted_by_key(|s| { + double(s.field); + }); + structs.is_sorted_by_key(|s| { + if s.field > 0 { + () + } else { + return (); + } + }); + structs.sort_by_key(|s| { + return double(s.field); + }); + structs.sort_by_key(|s| unit(s.field)); +} diff --git a/tests/ui/unit_return_expecting_ord.stderr b/tests/ui/unit_return_expecting_ord.stderr new file mode 100644 index 00000000000..e63d5874609 --- /dev/null +++ b/tests/ui/unit_return_expecting_ord.stderr @@ -0,0 +1,39 @@ +error: this closure returns the unit type which also implements Ord + --> $DIR/unit_return_expecting_ord.rs:18:25 + | +LL | structs.sort_by_key(|s| { + | ^^^ + | + = note: `-D clippy::unit-return-expecting-ord` implied by `-D warnings` +help: probably caused by this trailing semicolon + --> $DIR/unit_return_expecting_ord.rs:19:24 + | +LL | double(s.field); + | ^ + +error: this closure returns the unit type which also implements PartialOrd + --> $DIR/unit_return_expecting_ord.rs:22:30 + | +LL | structs.is_sorted_by_key(|s| { + | ^^^ + | +help: probably caused by this trailing semicolon + --> $DIR/unit_return_expecting_ord.rs:23:24 + | +LL | double(s.field); + | ^ + +error: this closure returns the unit type which also implements PartialOrd + --> $DIR/unit_return_expecting_ord.rs:25:30 + | +LL | structs.is_sorted_by_key(|s| { + | ^^^ + +error: this closure returns the unit type which also implements Ord + --> $DIR/unit_return_expecting_ord.rs:35:25 + | +LL | structs.sort_by_key(|s| unit(s.field)); + | ^^^ + +error: aborting due to 4 previous errors + diff --git a/util/dev b/util/dev deleted file mode 100755 index 319de217e0d..00000000000 --- a/util/dev +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/sh -CARGO_TARGET_DIR=$(pwd)/target/ -export CARGO_TARGET_DIR - -echo 'Deprecated! `util/dev` usage is deprecated, please use `cargo dev` instead.' - -cd clippy_dev && cargo run -- "$@" |