Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 8b04c2d

Browse files
author
Michael Wright
committed
Merge branch 'master' into lint-5734
2 parents 15244a8 + 0695f21 commit 8b04c2d

22 files changed

+468
-64
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1677,6 +1677,7 @@ Released 2018-09-13
16771677
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
16781678
[`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
16791679
[`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
1680+
[`map_err_ignore`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_err_ignore
16801681
[`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
16811682
[`map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
16821683
[`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or

‎clippy_lints/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ mod manual_async_fn;
232232
mod manual_non_exhaustive;
233233
mod manual_strip;
234234
mod map_clone;
235+
mod map_err_ignore;
235236
mod map_identity;
236237
mod map_unit_fn;
237238
mod match_on_vec_items;
@@ -629,6 +630,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
629630
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
630631
&manual_strip::MANUAL_STRIP,
631632
&map_clone::MAP_CLONE,
633+
&map_err_ignore::MAP_ERR_IGNORE,
632634
&map_identity::MAP_IDENTITY,
633635
&map_unit_fn::OPTION_MAP_UNIT_FN,
634636
&map_unit_fn::RESULT_MAP_UNIT_FN,
@@ -867,6 +869,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
867869
&utils::internal_lints::COMPILER_LINT_FUNCTIONS,
868870
&utils::internal_lints::DEFAULT_LINT,
869871
&utils::internal_lints::LINT_WITHOUT_LINT_PASS,
872+
&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
870873
&utils::internal_lints::OUTER_EXPN_EXPN_DATA,
871874
&utils::internal_lints::PRODUCE_ICE,
872875
&vec::USELESS_VEC,
@@ -922,6 +925,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
922925
store.register_late_pass(|| box implicit_saturating_sub::ImplicitSaturatingSub);
923926
store.register_late_pass(|| box methods::Methods);
924927
store.register_late_pass(|| box map_clone::MapClone);
928+
store.register_late_pass(|| box map_err_ignore::MapErrIgnore);
925929
store.register_late_pass(|| box shadow::Shadow);
926930
store.register_late_pass(|| box types::LetUnitValue);
927931
store.register_late_pass(|| box types::UnitCmp);
@@ -1112,6 +1116,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11121116
store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
11131117
store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
11141118
store.register_late_pass(|| box manual_strip::ManualStrip);
1119+
store.register_late_pass(|| box utils::internal_lints::MatchTypeOnDiagItem);
11151120

11161121
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
11171122
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1189,6 +1194,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11891194
LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP),
11901195
LintId::of(&loops::EXPLICIT_ITER_LOOP),
11911196
LintId::of(&macro_use::MACRO_USE_IMPORTS),
1197+
LintId::of(&map_err_ignore::MAP_ERR_IGNORE),
11921198
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
11931199
LintId::of(&matches::MATCH_BOOL),
11941200
LintId::of(&matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS),
@@ -1239,6 +1245,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12391245
LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS),
12401246
LintId::of(&utils::internal_lints::DEFAULT_LINT),
12411247
LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS),
1248+
LintId::of(&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM),
12421249
LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA),
12431250
LintId::of(&utils::internal_lints::PRODUCE_ICE),
12441251
]);

‎clippy_lints/src/loops.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1114,7 +1114,7 @@ fn get_vec_push<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) -> Option<(&
11141114
if let Some(self_expr) = args.get(0);
11151115
if let Some(pushed_item) = args.get(1);
11161116
// Check that the method being called is push() on a Vec
1117-
if match_type(cx, cx.typeck_results().expr_ty(self_expr), &paths::VEC);
1117+
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr), sym!(vec_type));
11181118
if path.ident.name.as_str() == "push";
11191119
then {
11201120
return Some((self_expr, pushed_item))

‎clippy_lints/src/map_err_ignore.rs

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
use crate::utils::span_lint_and_help;
2+
3+
use rustc_hir::{CaptureBy, Expr, ExprKind, PatKind};
4+
use rustc_lint::{LateContext, LateLintPass};
5+
use rustc_session::{declare_lint_pass, declare_tool_lint};
6+
7+
declare_clippy_lint! {
8+
/// **What it does:** Checks for instances of `map_err(|_| Some::Enum)`
9+
///
10+
/// **Why is this bad?** This map_err throws away the original error rather than allowing the enum to contain and report the cause of the error
11+
///
12+
/// **Known problems:** None.
13+
///
14+
/// **Example:**
15+
/// Before:
16+
/// ```rust
17+
/// use std::fmt;
18+
///
19+
/// #[derive(Debug)]
20+
/// enum Error {
21+
/// Indivisible,
22+
/// Remainder(u8),
23+
/// }
24+
///
25+
/// impl fmt::Display for Error {
26+
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
27+
/// match self {
28+
/// Error::Indivisible => write!(f, "could not divide input by three"),
29+
/// Error::Remainder(remainder) => write!(
30+
/// f,
31+
/// "input is not divisible by three, remainder = {}",
32+
/// remainder
33+
/// ),
34+
/// }
35+
/// }
36+
/// }
37+
///
38+
/// impl std::error::Error for Error {}
39+
///
40+
/// fn divisible_by_3(input: &str) -> Result<(), Error> {
41+
/// input
42+
/// .parse::<i32>()
43+
/// .map_err(|_| Error::Indivisible)
44+
/// .map(|v| v % 3)
45+
/// .and_then(|remainder| {
46+
/// if remainder == 0 {
47+
/// Ok(())
48+
/// } else {
49+
/// Err(Error::Remainder(remainder as u8))
50+
/// }
51+
/// })
52+
/// }
53+
/// ```
54+
///
55+
/// After:
56+
/// ```rust
57+
/// use std::{fmt, num::ParseIntError};
58+
///
59+
/// #[derive(Debug)]
60+
/// enum Error {
61+
/// Indivisible(ParseIntError),
62+
/// Remainder(u8),
63+
/// }
64+
///
65+
/// impl fmt::Display for Error {
66+
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
67+
/// match self {
68+
/// Error::Indivisible(_) => write!(f, "could not divide input by three"),
69+
/// Error::Remainder(remainder) => write!(
70+
/// f,
71+
/// "input is not divisible by three, remainder = {}",
72+
/// remainder
73+
/// ),
74+
/// }
75+
/// }
76+
/// }
77+
///
78+
/// impl std::error::Error for Error {
79+
/// fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
80+
/// match self {
81+
/// Error::Indivisible(source) => Some(source),
82+
/// _ => None,
83+
/// }
84+
/// }
85+
/// }
86+
///
87+
/// fn divisible_by_3(input: &str) -> Result<(), Error> {
88+
/// input
89+
/// .parse::<i32>()
90+
/// .map_err(Error::Indivisible)
91+
/// .map(|v| v % 3)
92+
/// .and_then(|remainder| {
93+
/// if remainder == 0 {
94+
/// Ok(())
95+
/// } else {
96+
/// Err(Error::Remainder(remainder as u8))
97+
/// }
98+
/// })
99+
/// }
100+
/// ```
101+
pub MAP_ERR_IGNORE,
102+
pedantic,
103+
"`map_err` should not ignore the original error"
104+
}
105+
106+
declare_lint_pass!(MapErrIgnore => [MAP_ERR_IGNORE]);
107+
108+
impl<'tcx> LateLintPass<'tcx> for MapErrIgnore {
109+
// do not try to lint if this is from a macro or desugaring
110+
fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
111+
if e.span.from_expansion() {
112+
return;
113+
}
114+
115+
// check if this is a method call (e.g. x.foo())
116+
if let ExprKind::MethodCall(ref method, _t_span, ref args, _) = e.kind {
117+
// only work if the method name is `map_err` and there are only 2 arguments (e.g. x.map_err(|_|[1]
118+
// Enum::Variant[2]))
119+
if method.ident.as_str() == "map_err" && args.len() == 2 {
120+
// make sure the first argument is a closure, and grab the CaptureRef, body_id, and body_span fields
121+
if let ExprKind::Closure(capture, _, body_id, body_span, _) = args[1].kind {
122+
// check if this is by Reference (meaning there's no move statement)
123+
if capture == CaptureBy::Ref {
124+
// Get the closure body to check the parameters and values
125+
let closure_body = cx.tcx.hir().body(body_id);
126+
// make sure there's only one parameter (`|_|`)
127+
if closure_body.params.len() == 1 {
128+
// make sure that parameter is the wild token (`_`)
129+
if let PatKind::Wild = closure_body.params[0].pat.kind {
130+
// span the area of the closure capture and warn that the
131+
// original error will be thrown away
132+
span_lint_and_help(
133+
cx,
134+
MAP_ERR_IGNORE,
135+
body_span,
136+
"`map_err(|_|...` ignores the original error",
137+
None,
138+
"Consider wrapping the error in an enum variant",
139+
);
140+
}
141+
}
142+
}
143+
}
144+
}
145+
}
146+
}
147+
}

‎clippy_lints/src/methods/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1808,7 +1808,7 @@ fn lint_or_fun_call<'tcx>(
18081808
_ => (),
18091809
}
18101810

1811-
if match_type(cx, ty, &paths::VEC) {
1811+
if is_type_diagnostic_item(cx, ty, sym!(vec_type)) {
18121812
return;
18131813
}
18141814
}

‎clippy_lints/src/misc.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,11 @@ declare_clippy_lint! {
9999
/// if y != x {} // where both are floats
100100
///
101101
/// // Good
102-
/// let error = f64::EPSILON; // Use an epsilon for comparison
102+
/// let error_margin = f64::EPSILON; // Use an epsilon for comparison
103103
/// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead.
104-
/// // let error = std::f64::EPSILON;
105-
/// if (y - 1.23f64).abs() < error { }
106-
/// if (y - x).abs() > error { }
104+
/// // let error_margin = std::f64::EPSILON;
105+
/// if (y - 1.23f64).abs() < error_margin { }
106+
/// if (y - x).abs() > error_margin { }
107107
/// ```
108108
pub FLOAT_CMP,
109109
correctness,
@@ -242,10 +242,10 @@ declare_clippy_lint! {
242242
/// if x == ONE { } // where both are floats
243243
///
244244
/// // Good
245-
/// let error = f64::EPSILON; // Use an epsilon for comparison
245+
/// let error_margin = f64::EPSILON; // Use an epsilon for comparison
246246
/// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead.
247-
/// // let error = std::f64::EPSILON;
248-
/// if (x - ONE).abs() < error { }
247+
/// // let error_margin = std::f64::EPSILON;
248+
/// if (x - ONE).abs() < error_margin { }
249249
/// ```
250250
pub FLOAT_CMP_CONST,
251251
restriction,
@@ -411,16 +411,16 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
411411
if !is_comparing_arrays {
412412
diag.span_suggestion(
413413
expr.span,
414-
"consider comparing them within some error",
414+
"consider comparing them within some margin of error",
415415
format!(
416-
"({}).abs() {} error",
416+
"({}).abs() {} error_margin",
417417
lhs - rhs,
418418
if op == BinOpKind::Eq { '<' } else { '>' }
419419
),
420420
Applicability::HasPlaceholders, // snippet
421421
);
422422
}
423-
diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error`");
423+
diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`");
424424
});
425425
} else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) {
426426
span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");

‎clippy_lints/src/option_if_let_else.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::utils;
22
use crate::utils::sugg::Sugg;
3-
use crate::utils::{match_type, paths, span_lint_and_sugg};
3+
use crate::utils::{is_type_diagnostic_item, paths, span_lint_and_sugg};
44
use if_chain::if_chain;
55

66
use rustc_errors::Applicability;
@@ -73,7 +73,7 @@ declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]);
7373
fn is_result_ok(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
7474
if let ExprKind::MethodCall(ref path, _, &[ref receiver], _) = &expr.kind {
7575
path.ident.name.to_ident_string() == "ok"
76-
&& match_type(cx, &cx.typeck_results().expr_ty(&receiver), &paths::RESULT)
76+
&& is_type_diagnostic_item(cx, &cx.typeck_results().expr_ty(&receiver), sym!(result_type))
7777
} else {
7878
false
7979
}

‎clippy_lints/src/shadow.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ declare_clippy_lint! {
2525
/// **Example:**
2626
/// ```rust
2727
/// # let x = 1;
28-
///
2928
/// // Bad
3029
/// let x = &x;
3130
///

‎clippy_lints/src/utils/diagnostics.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: impl Into<Mult
5151
///
5252
/// The `help` message can be optionally attached to a `Span`.
5353
///
54+
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
55+
///
5456
/// # Example
5557
///
5658
/// ```ignore
@@ -87,6 +89,8 @@ pub fn span_lint_and_help<'a, T: LintContext>(
8789
/// The `note` message is presented separately from the main lint message
8890
/// and is attached to a specific span:
8991
///
92+
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
93+
///
9094
/// # Example
9195
///
9296
/// ```ignore
@@ -126,6 +130,7 @@ pub fn span_lint_and_note<'a, T: LintContext>(
126130
/// Like `span_lint` but allows to add notes, help and suggestions using a closure.
127131
///
128132
/// If you need to customize your lint output a lot, use this function.
133+
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
129134
pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F)
130135
where
131136
F: for<'b> FnOnce(&mut DiagnosticBuilder<'b>),
@@ -168,6 +173,10 @@ pub fn span_lint_hir_and_then(
168173
/// In the example below, `help` is `"try"` and `sugg` is the suggested replacement `".any(|x| x >
169174
/// 2)"`.
170175
///
176+
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
177+
///
178+
/// # Example
179+
///
171180
/// ```ignore
172181
/// error: This `.fold` can be more succinctly expressed as `.any`
173182
/// --> $DIR/methods.rs:390:13

0 commit comments

Comments
 (0)