Skip to content

Commit d9bf74e

Browse files
flip1995flip1995
committed
Merge remote-tracking branch 'upstream/master' into rustup
2 parents bf3104e + 84cdce0 commit d9bf74e

13 files changed

+291
-21
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,6 +1679,7 @@ Released 2018-09-13
16791679
[`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init
16801680
[`unit_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
16811681
[`unit_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_cmp
1682+
[`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord
16821683
[`unknown_clippy_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints
16831684
[`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
16841685
[`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map

‎clippy_lints/src/consts.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,13 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
332332
let result = self
333333
.lcx
334334
.tcx
335-
.const_eval_resolve(self.param_env, ty::WithOptConstParam::unknown(def_id), substs, None, None)
335+
.const_eval_resolve(
336+
self.param_env,
337+
ty::WithOptConstParam::unknown(def_id),
338+
substs,
339+
None,
340+
None,
341+
)
336342
.ok()
337343
.map(|val| rustc_middle::ty::Const::from_value(self.lcx.tcx, val, ty))?;
338344
let result = miri_to_const(&result);

‎clippy_lints/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ mod trivially_copy_pass_by_ref;
300300
mod try_err;
301301
mod types;
302302
mod unicode;
303+
mod unit_return_expecting_ord;
303304
mod unnamed_address;
304305
mod unnecessary_sort_by;
305306
mod unnested_or_patterns;
@@ -826,6 +827,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
826827
&unicode::NON_ASCII_LITERAL,
827828
&unicode::UNICODE_NOT_NFC,
828829
&unicode::ZERO_WIDTH_SPACE,
830+
&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD,
829831
&unnamed_address::FN_ADDRESS_COMPARISONS,
830832
&unnamed_address::VTABLE_ADDRESS_COMPARISONS,
831833
&unnecessary_sort_by::UNNECESSARY_SORT_BY,
@@ -891,6 +893,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
891893
store.register_late_pass(|| box attrs::Attributes);
892894
store.register_late_pass(|| box blocks_in_if_conditions::BlocksInIfConditions);
893895
store.register_late_pass(|| box unicode::Unicode);
896+
store.register_late_pass(|| box unit_return_expecting_ord::UnitReturnExpectingOrd);
894897
store.register_late_pass(|| box strings::StringAdd);
895898
store.register_late_pass(|| box implicit_return::ImplicitReturn);
896899
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:
14361439
LintId::of(&types::UNNECESSARY_CAST),
14371440
LintId::of(&types::VEC_BOX),
14381441
LintId::of(&unicode::ZERO_WIDTH_SPACE),
1442+
LintId::of(&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD),
14391443
LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS),
14401444
LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS),
14411445
LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY),
@@ -1692,6 +1696,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16921696
LintId::of(&types::CAST_REF_TO_MUT),
16931697
LintId::of(&types::UNIT_CMP),
16941698
LintId::of(&unicode::ZERO_WIDTH_SPACE),
1699+
LintId::of(&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD),
16951700
LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS),
16961701
LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS),
16971702
LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT),

‎clippy_lints/src/methods/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2348,8 +2348,8 @@ fn lint_iter_nth_zero<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, nth_ar
23482348
cx,
23492349
ITER_NTH_ZERO,
23502350
expr.span,
2351-
"called `.nth(0)` on a `std::iter::Iterator`",
2352-
"try calling",
2351+
"called `.nth(0)` on a `std::iter::Iterator`, when `.next()` is equivalent",
2352+
"try calling `.next()` instead of `.nth(0)`",
23532353
format!("{}.next()", snippet_with_applicability(cx, nth_args[0].span, "..", &mut applicability)),
23542354
applicability,
23552355
);

‎clippy_lints/src/needless_pass_by_value.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,8 @@ declare_clippy_lint! {
4040
/// assert_eq!(v.len(), 42);
4141
/// }
4242
/// ```
43-
///
43+
/// should be
4444
/// ```rust
45-
/// // should be
4645
/// fn foo(v: &[i32]) {
4746
/// assert_eq!(v.len(), 42);
4847
/// }

‎clippy_lints/src/unit_return_expecting_ord.rs

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
use crate::utils::{get_trait_def_id, paths, span_lint, span_lint_and_help};
2+
use if_chain::if_chain;
3+
use rustc_hir::def_id::DefId;
4+
use rustc_hir::{Expr, ExprKind, StmtKind};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_middle::ty;
7+
use rustc_middle::ty::{GenericPredicates, PredicateKind, ProjectionPredicate, TraitPredicate};
8+
use rustc_session::{declare_lint_pass, declare_tool_lint};
9+
use rustc_span::{BytePos, Span};
10+
11+
declare_clippy_lint! {
12+
/// **What it does:** Checks for functions that expect closures of type
13+
/// Fn(...) -> Ord where the implemented closure returns the unit type.
14+
/// The lint also suggests to remove the semi-colon at the end of the statement if present.
15+
///
16+
/// **Why is this bad?** Likely, returning the unit type is unintentional, and
17+
/// could simply be caused by an extra semi-colon. Since () implements Ord
18+
/// it doesn't cause a compilation error.
19+
/// This is the same reasoning behind the unit_cmp lint.
20+
///
21+
/// **Known problems:** If returning unit is intentional, then there is no
22+
/// way of specifying this without triggering needless_return lint
23+
///
24+
/// **Example:**
25+
///
26+
/// ```rust
27+
/// let mut twins = vec!((1,1), (2,2));
28+
/// twins.sort_by_key(|x| { x.1; });
29+
/// ```
30+
pub UNIT_RETURN_EXPECTING_ORD,
31+
correctness,
32+
"fn arguments of type Fn(...) -> Ord returning the unit type ()."
33+
}
34+
35+
declare_lint_pass!(UnitReturnExpectingOrd => [UNIT_RETURN_EXPECTING_ORD]);
36+
37+
fn get_trait_predicates_for_trait_id<'tcx>(
38+
cx: &LateContext<'tcx>,
39+
generics: GenericPredicates<'tcx>,
40+
trait_id: Option<DefId>,
41+
) -> Vec<TraitPredicate<'tcx>> {
42+
let mut preds = Vec::new();
43+
for (pred, _) in generics.predicates {
44+
if_chain! {
45+
if let PredicateKind::Trait(poly_trait_pred, _) = pred.kind();
46+
let trait_pred = cx.tcx.erase_late_bound_regions(&poly_trait_pred);
47+
if let Some(trait_def_id) = trait_id;
48+
if trait_def_id == trait_pred.trait_ref.def_id;
49+
then {
50+
preds.push(trait_pred);
51+
}
52+
}
53+
}
54+
preds
55+
}
56+
57+
fn get_projection_pred<'tcx>(
58+
cx: &LateContext<'tcx>,
59+
generics: GenericPredicates<'tcx>,
60+
pred: TraitPredicate<'tcx>,
61+
) -> Option<ProjectionPredicate<'tcx>> {
62+
generics.predicates.iter().find_map(|(proj_pred, _)| {
63+
if let PredicateKind::Projection(proj_pred) = proj_pred.kind() {
64+
let projection_pred = cx.tcx.erase_late_bound_regions(proj_pred);
65+
if projection_pred.projection_ty.substs == pred.trait_ref.substs {
66+
return Some(projection_pred);
67+
}
68+
}
69+
None
70+
})
71+
}
72+
73+
fn get_args_to_check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Vec<(usize, String)> {
74+
let mut args_to_check = Vec::new();
75+
if let Some(def_id) = cx.tables().type_dependent_def_id(expr.hir_id) {
76+
let fn_sig = cx.tcx.fn_sig(def_id);
77+
let generics = cx.tcx.predicates_of(def_id);
78+
let fn_mut_preds = get_trait_predicates_for_trait_id(cx, generics, cx.tcx.lang_items().fn_mut_trait());
79+
let ord_preds = get_trait_predicates_for_trait_id(cx, generics, get_trait_def_id(cx, &paths::ORD));
80+
let partial_ord_preds =
81+
get_trait_predicates_for_trait_id(cx, generics, cx.tcx.lang_items().partial_ord_trait());
82+
// Trying to call erase_late_bound_regions on fn_sig.inputs() gives the following error
83+
// The trait `rustc::ty::TypeFoldable<'_>` is not implemented for `&[&rustc::ty::TyS<'_>]`
84+
let inputs_output = cx.tcx.erase_late_bound_regions(&fn_sig.inputs_and_output());
85+
inputs_output
86+
.iter()
87+
.rev()
88+
.skip(1)
89+
.rev()
90+
.enumerate()
91+
.for_each(|(i, inp)| {
92+
for trait_pred in &fn_mut_preds {
93+
if_chain! {
94+
if trait_pred.self_ty() == inp;
95+
if let Some(return_ty_pred) = get_projection_pred(cx, generics, *trait_pred);
96+
then {
97+
if ord_preds.iter().any(|ord| ord.self_ty() == return_ty_pred.ty) {
98+
args_to_check.push((i, "Ord".to_string()));
99+
} else if partial_ord_preds.iter().any(|pord| pord.self_ty() == return_ty_pred.ty) {
100+
args_to_check.push((i, "PartialOrd".to_string()));
101+
}
102+
}
103+
}
104+
}
105+
});
106+
}
107+
args_to_check
108+
}
109+
110+
fn check_arg<'tcx>(cx: &LateContext<'tcx>, arg: &'tcx Expr<'tcx>) -> Option<(Span, Option<Span>)> {
111+
if_chain! {
112+
if let ExprKind::Closure(_, _fn_decl, body_id, span, _) = arg.kind;
113+
if let ty::Closure(_def_id, substs) = &cx.tables().node_type(arg.hir_id).kind;
114+
let ret_ty = substs.as_closure().sig().output();
115+
let ty = cx.tcx.erase_late_bound_regions(&ret_ty);
116+
if ty.is_unit();
117+
then {
118+
if_chain! {
119+
let body = cx.tcx.hir().body(body_id);
120+
if let ExprKind::Block(block, _) = body.value.kind;
121+
if block.expr.is_none();
122+
if let Some(stmt) = block.stmts.last();
123+
if let StmtKind::Semi(_) = stmt.kind;
124+
then {
125+
let data = stmt.span.data();
126+
// Make a span out of the semicolon for the help message
127+
Some((span, Some(Span::new(data.hi-BytePos(1), data.hi, data.ctxt))))
128+
} else {
129+
Some((span, None))
130+
}
131+
}
132+
} else {
133+
None
134+
}
135+
}
136+
}
137+
138+
impl<'tcx> LateLintPass<'tcx> for UnitReturnExpectingOrd {
139+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
140+
if let ExprKind::MethodCall(_, _, ref args, _) = expr.kind {
141+
let arg_indices = get_args_to_check(cx, expr);
142+
for (i, trait_name) in arg_indices {
143+
if i < args.len() {
144+
match check_arg(cx, &args[i]) {
145+
Some((span, None)) => {
146+
span_lint(
147+
cx,
148+
UNIT_RETURN_EXPECTING_ORD,
149+
span,
150+
&format!(
151+
"this closure returns \
152+
the unit type which also implements {}",
153+
trait_name
154+
),
155+
);
156+
},
157+
Some((span, Some(last_semi))) => {
158+
span_lint_and_help(
159+
cx,
160+
UNIT_RETURN_EXPECTING_ORD,
161+
span,
162+
&format!(
163+
"this closure returns \
164+
the unit type which also implements {}",
165+
trait_name
166+
),
167+
Some(last_semi),
168+
&"probably caused by this trailing semicolon".to_string(),
169+
);
170+
},
171+
None => {},
172+
}
173+
}
174+
}
175+
}
176+
}
177+
}

‎src/lintlist/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2292,6 +2292,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
22922292
deprecation: None,
22932293
module: "types",
22942294
},
2295+
Lint {
2296+
name: "unit_return_expecting_ord",
2297+
group: "correctness",
2298+
desc: "fn arguments of type Fn(...) -> Ord returning the unit type ().",
2299+
deprecation: None,
2300+
module: "unit_return_expecting_ord",
2301+
},
22952302
Lint {
22962303
name: "unknown_clippy_lints",
22972304
group: "style",

‎tests/compile-test.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,6 @@ fn run_ui_toml(config: &mut compiletest::Config) {
147147
}
148148

149149
fn run_ui_cargo(config: &mut compiletest::Config) {
150-
if cargo::is_rustc_test_suite() {
151-
return;
152-
}
153150
fn run_tests(
154151
config: &compiletest::Config,
155152
filter: &Option<String>,

‎tests/ui/iter_nth_zero.stderr

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
1-
error: called `.nth(0)` on a `std::iter::Iterator`
1+
error: called `.nth(0)` on a `std::iter::Iterator`, when `.next()` is equivalent
22
--> $DIR/iter_nth_zero.rs:20:14
33
|
44
LL | let _x = s.iter().nth(0);
5-
| ^^^^^^^^^^^^^^^ help: try calling: `s.iter().next()`
5+
| ^^^^^^^^^^^^^^^ help: try calling `.next()` instead of `.nth(0)`: `s.iter().next()`
66
|
77
= note: `-D clippy::iter-nth-zero` implied by `-D warnings`
88

9-
error: called `.nth(0)` on a `std::iter::Iterator`
9+
error: called `.nth(0)` on a `std::iter::Iterator`, when `.next()` is equivalent
1010
--> $DIR/iter_nth_zero.rs:25:14
1111
|
1212
LL | let _y = iter.nth(0);
13-
| ^^^^^^^^^^^ help: try calling: `iter.next()`
13+
| ^^^^^^^^^^^ help: try calling `.next()` instead of `.nth(0)`: `iter.next()`
1414

15-
error: called `.nth(0)` on a `std::iter::Iterator`
15+
error: called `.nth(0)` on a `std::iter::Iterator`, when `.next()` is equivalent
1616
--> $DIR/iter_nth_zero.rs:30:22
1717
|
1818
LL | let _unwrapped = iter2.nth(0).unwrap();
19-
| ^^^^^^^^^^^^ help: try calling: `iter2.next()`
19+
| ^^^^^^^^^^^^ help: try calling `.next()` instead of `.nth(0)`: `iter2.next()`
2020

2121
error: aborting due to 3 previous errors
2222

‎tests/ui/needless_range_loop2.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,13 @@ fn main() {
8383
println!("{}", arr[i]);
8484
}
8585
}
86+
87+
mod issue2277 {
88+
pub fn example(list: &[[f64; 3]]) {
89+
let mut x: [f64; 3] = [10.; 3];
90+
91+
for i in 0..3 {
92+
x[i] = list.iter().map(|item| item[i]).sum::<f64>();
93+
}
94+
}
95+
}

‎tests/ui/unit_return_expecting_ord.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
#![warn(clippy::unit_return_expecting_ord)]
2+
#![allow(clippy::needless_return)]
3+
#![allow(clippy::unused_unit)]
4+
#![feature(is_sorted)]
5+
6+
struct Struct {
7+
field: isize,
8+
}
9+
10+
fn double(i: isize) -> isize {
11+
i * 2
12+
}
13+
14+
fn unit(_i: isize) {}
15+
16+
fn main() {
17+
let mut structs = vec![Struct { field: 2 }, Struct { field: 1 }];
18+
structs.sort_by_key(|s| {
19+
double(s.field);
20+
});
21+
structs.sort_by_key(|s| double(s.field));
22+
structs.is_sorted_by_key(|s| {
23+
double(s.field);
24+
});
25+
structs.is_sorted_by_key(|s| {
26+
if s.field > 0 {
27+
()
28+
} else {
29+
return ();
30+
}
31+
});
32+
structs.sort_by_key(|s| {
33+
return double(s.field);
34+
});
35+
structs.sort_by_key(|s| unit(s.field));
36+
}

0 commit comments

Comments
 (0)