Skip to content

Commit

Permalink
Merge branch 'master' into lint-5734
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Wright committed Sep 15, 2020
2 parents 15244a8 + 0695f21 commit 8b04c2d
Show file tree
Hide file tree
Showing 22 changed files with 468 additions and 64 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -1677,6 +1677,7 @@ Released 2018-09-13
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
[`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
[`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
[`map_err_ignore`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_err_ignore
[`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
[`map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
[`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or
Expand Down
7 changes: 7 additions & 0 deletions clippy_lints/src/lib.rs
Expand Up @@ -232,6 +232,7 @@ mod manual_async_fn;
mod manual_non_exhaustive;
mod manual_strip;
mod map_clone;
mod map_err_ignore;
mod map_identity;
mod map_unit_fn;
mod match_on_vec_items;
Expand Down Expand Up @@ -629,6 +630,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
&manual_strip::MANUAL_STRIP,
&map_clone::MAP_CLONE,
&map_err_ignore::MAP_ERR_IGNORE,
&map_identity::MAP_IDENTITY,
&map_unit_fn::OPTION_MAP_UNIT_FN,
&map_unit_fn::RESULT_MAP_UNIT_FN,
Expand Down Expand Up @@ -867,6 +869,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&utils::internal_lints::COMPILER_LINT_FUNCTIONS,
&utils::internal_lints::DEFAULT_LINT,
&utils::internal_lints::LINT_WITHOUT_LINT_PASS,
&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
&utils::internal_lints::OUTER_EXPN_EXPN_DATA,
&utils::internal_lints::PRODUCE_ICE,
&vec::USELESS_VEC,
Expand Down Expand Up @@ -922,6 +925,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box implicit_saturating_sub::ImplicitSaturatingSub);
store.register_late_pass(|| box methods::Methods);
store.register_late_pass(|| box map_clone::MapClone);
store.register_late_pass(|| box map_err_ignore::MapErrIgnore);
store.register_late_pass(|| box shadow::Shadow);
store.register_late_pass(|| box types::LetUnitValue);
store.register_late_pass(|| box types::UnitCmp);
Expand Down Expand Up @@ -1112,6 +1116,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
store.register_late_pass(|| box manual_strip::ManualStrip);
store.register_late_pass(|| box utils::internal_lints::MatchTypeOnDiagItem);

store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
Expand Down Expand Up @@ -1189,6 +1194,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP),
LintId::of(&loops::EXPLICIT_ITER_LOOP),
LintId::of(&macro_use::MACRO_USE_IMPORTS),
LintId::of(&map_err_ignore::MAP_ERR_IGNORE),
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
LintId::of(&matches::MATCH_BOOL),
LintId::of(&matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS),
Expand Down Expand Up @@ -1239,6 +1245,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS),
LintId::of(&utils::internal_lints::DEFAULT_LINT),
LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS),
LintId::of(&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM),
LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA),
LintId::of(&utils::internal_lints::PRODUCE_ICE),
]);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/loops.rs
Expand Up @@ -1114,7 +1114,7 @@ fn get_vec_push<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) -> Option<(&
if let Some(self_expr) = args.get(0);
if let Some(pushed_item) = args.get(1);
// Check that the method being called is push() on a Vec
if match_type(cx, cx.typeck_results().expr_ty(self_expr), &paths::VEC);
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr), sym!(vec_type));
if path.ident.name.as_str() == "push";
then {
return Some((self_expr, pushed_item))
Expand Down
147 changes: 147 additions & 0 deletions clippy_lints/src/map_err_ignore.rs
@@ -0,0 +1,147 @@
use crate::utils::span_lint_and_help;

use rustc_hir::{CaptureBy, Expr, ExprKind, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// **What it does:** Checks for instances of `map_err(|_| Some::Enum)`
///
/// **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
///
/// **Known problems:** None.
///
/// **Example:**
/// Before:
/// ```rust
/// use std::fmt;
///
/// #[derive(Debug)]
/// enum Error {
/// Indivisible,
/// Remainder(u8),
/// }
///
/// impl fmt::Display for Error {
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// match self {
/// Error::Indivisible => write!(f, "could not divide input by three"),
/// Error::Remainder(remainder) => write!(
/// f,
/// "input is not divisible by three, remainder = {}",
/// remainder
/// ),
/// }
/// }
/// }
///
/// impl std::error::Error for Error {}
///
/// fn divisible_by_3(input: &str) -> Result<(), Error> {
/// input
/// .parse::<i32>()
/// .map_err(|_| Error::Indivisible)
/// .map(|v| v % 3)
/// .and_then(|remainder| {
/// if remainder == 0 {
/// Ok(())
/// } else {
/// Err(Error::Remainder(remainder as u8))
/// }
/// })
/// }
/// ```
///
/// After:
/// ```rust
/// use std::{fmt, num::ParseIntError};
///
/// #[derive(Debug)]
/// enum Error {
/// Indivisible(ParseIntError),
/// Remainder(u8),
/// }
///
/// impl fmt::Display for Error {
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// match self {
/// Error::Indivisible(_) => write!(f, "could not divide input by three"),
/// Error::Remainder(remainder) => write!(
/// f,
/// "input is not divisible by three, remainder = {}",
/// remainder
/// ),
/// }
/// }
/// }
///
/// impl std::error::Error for Error {
/// fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
/// match self {
/// Error::Indivisible(source) => Some(source),
/// _ => None,
/// }
/// }
/// }
///
/// fn divisible_by_3(input: &str) -> Result<(), Error> {
/// input
/// .parse::<i32>()
/// .map_err(Error::Indivisible)
/// .map(|v| v % 3)
/// .and_then(|remainder| {
/// if remainder == 0 {
/// Ok(())
/// } else {
/// Err(Error::Remainder(remainder as u8))
/// }
/// })
/// }
/// ```
pub MAP_ERR_IGNORE,
pedantic,
"`map_err` should not ignore the original error"
}

declare_lint_pass!(MapErrIgnore => [MAP_ERR_IGNORE]);

impl<'tcx> LateLintPass<'tcx> for MapErrIgnore {
// do not try to lint if this is from a macro or desugaring
fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
if e.span.from_expansion() {
return;
}

// check if this is a method call (e.g. x.foo())
if let ExprKind::MethodCall(ref method, _t_span, ref args, _) = e.kind {
// only work if the method name is `map_err` and there are only 2 arguments (e.g. x.map_err(|_|[1]
// Enum::Variant[2]))
if method.ident.as_str() == "map_err" && args.len() == 2 {
// make sure the first argument is a closure, and grab the CaptureRef, body_id, and body_span fields
if let ExprKind::Closure(capture, _, body_id, body_span, _) = args[1].kind {
// check if this is by Reference (meaning there's no move statement)
if capture == CaptureBy::Ref {
// Get the closure body to check the parameters and values
let closure_body = cx.tcx.hir().body(body_id);
// make sure there's only one parameter (`|_|`)
if closure_body.params.len() == 1 {
// make sure that parameter is the wild token (`_`)
if let PatKind::Wild = closure_body.params[0].pat.kind {
// span the area of the closure capture and warn that the
// original error will be thrown away
span_lint_and_help(
cx,
MAP_ERR_IGNORE,
body_span,
"`map_err(|_|...` ignores the original error",
None,
"Consider wrapping the error in an enum variant",
);
}
}
}
}
}
}
}
}
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/mod.rs
Expand Up @@ -1808,7 +1808,7 @@ fn lint_or_fun_call<'tcx>(
_ => (),
}

if match_type(cx, ty, &paths::VEC) {
if is_type_diagnostic_item(cx, ty, sym!(vec_type)) {
return;
}
}
Expand Down
20 changes: 10 additions & 10 deletions clippy_lints/src/misc.rs
Expand Up @@ -99,11 +99,11 @@ declare_clippy_lint! {
/// if y != x {} // where both are floats
///
/// // Good
/// let error = f64::EPSILON; // Use an epsilon for comparison
/// let error_margin = f64::EPSILON; // Use an epsilon for comparison
/// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead.
/// // let error = std::f64::EPSILON;
/// if (y - 1.23f64).abs() < error { }
/// if (y - x).abs() > error { }
/// // let error_margin = std::f64::EPSILON;
/// if (y - 1.23f64).abs() < error_margin { }
/// if (y - x).abs() > error_margin { }
/// ```
pub FLOAT_CMP,
correctness,
Expand Down Expand Up @@ -242,10 +242,10 @@ declare_clippy_lint! {
/// if x == ONE { } // where both are floats
///
/// // Good
/// let error = f64::EPSILON; // Use an epsilon for comparison
/// let error_margin = f64::EPSILON; // Use an epsilon for comparison
/// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead.
/// // let error = std::f64::EPSILON;
/// if (x - ONE).abs() < error { }
/// // let error_margin = std::f64::EPSILON;
/// if (x - ONE).abs() < error_margin { }
/// ```
pub FLOAT_CMP_CONST,
restriction,
Expand Down Expand Up @@ -411,16 +411,16 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
if !is_comparing_arrays {
diag.span_suggestion(
expr.span,
"consider comparing them within some error",
"consider comparing them within some margin of error",
format!(
"({}).abs() {} error",
"({}).abs() {} error_margin",
lhs - rhs,
if op == BinOpKind::Eq { '<' } else { '>' }
),
Applicability::HasPlaceholders, // snippet
);
}
diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error`");
diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`");
});
} else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) {
span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/option_if_let_else.rs
@@ -1,6 +1,6 @@
use crate::utils;
use crate::utils::sugg::Sugg;
use crate::utils::{match_type, paths, span_lint_and_sugg};
use crate::utils::{is_type_diagnostic_item, paths, span_lint_and_sugg};
use if_chain::if_chain;

use rustc_errors::Applicability;
Expand Down Expand Up @@ -73,7 +73,7 @@ declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]);
fn is_result_ok(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
if let ExprKind::MethodCall(ref path, _, &[ref receiver], _) = &expr.kind {
path.ident.name.to_ident_string() == "ok"
&& match_type(cx, &cx.typeck_results().expr_ty(&receiver), &paths::RESULT)
&& is_type_diagnostic_item(cx, &cx.typeck_results().expr_ty(&receiver), sym!(result_type))
} else {
false
}
Expand Down
1 change: 0 additions & 1 deletion clippy_lints/src/shadow.rs
Expand Up @@ -25,7 +25,6 @@ declare_clippy_lint! {
/// **Example:**
/// ```rust
/// # let x = 1;
///
/// // Bad
/// let x = &x;
///
Expand Down
9 changes: 9 additions & 0 deletions clippy_lints/src/utils/diagnostics.rs
Expand Up @@ -51,6 +51,8 @@ pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: impl Into<Mult
///
/// The `help` message can be optionally attached to a `Span`.
///
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
///
/// # Example
///
/// ```ignore
Expand Down Expand Up @@ -87,6 +89,8 @@ pub fn span_lint_and_help<'a, T: LintContext>(
/// The `note` message is presented separately from the main lint message
/// and is attached to a specific span:
///
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
///
/// # Example
///
/// ```ignore
Expand Down Expand Up @@ -126,6 +130,7 @@ pub fn span_lint_and_note<'a, T: LintContext>(
/// Like `span_lint` but allows to add notes, help and suggestions using a closure.
///
/// If you need to customize your lint output a lot, use this function.
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F)
where
F: for<'b> FnOnce(&mut DiagnosticBuilder<'b>),
Expand Down Expand Up @@ -168,6 +173,10 @@ pub fn span_lint_hir_and_then(
/// In the example below, `help` is `"try"` and `sugg` is the suggested replacement `".any(|x| x >
/// 2)"`.
///
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
///
/// # Example
///
/// ```ignore
/// error: This `.fold` can be more succinctly expressed as `.any`
/// --> $DIR/methods.rs:390:13
Expand Down

0 comments on commit 8b04c2d

Please sign in to comment.