diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index fa981ce78d9..f60ed90cd7c 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -90,6 +90,7 @@ fn check_hash_peq<'a, 'tcx>( if_chain! { if match_path(&trait_ref.path, &paths::HASH); if let Some(peq_trait_def_id) = cx.tcx.lang_items().eq_trait(); + if !&trait_ref.trait_def_id().is_local(); then { // Look for the PartialEq implementations for `ty` cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| { diff --git a/clippy_lints/src/excessive_precision.rs b/clippy_lints/src/excessive_precision.rs index 8027a736c6b..40dbaa17732 100644 --- a/clippy_lints/src/excessive_precision.rs +++ b/clippy_lints/src/excessive_precision.rs @@ -1,4 +1,5 @@ use crate::utils::span_lint_and_sugg; +use crate::utils::sugg::format_numeric_literal; use if_chain::if_chain; use rustc::hir; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; @@ -86,8 +87,7 @@ impl ExcessivePrecision { if sym_str == s { None } else { - let di = super::literal_representation::DigitInfo::new(&s, true); - Some(di.grouping_hint()) + Some(format_numeric_literal(&s, None, true)) } } else { None diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 8ddd54bb7a3..8aa0e872968 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -114,7 +114,7 @@ pub(super) enum Radix { impl Radix { /// Returns a reasonable digit group size for this radix. #[must_use] - crate fn suggest_grouping(&self) -> usize { + fn suggest_grouping(&self) -> usize { match *self { Self::Binary | Self::Hexadecimal => 4, Self::Octal | Self::Decimal => 3, @@ -122,23 +122,43 @@ impl Radix { } } -#[derive(Debug)] -pub(super) struct DigitInfo<'a> { - /// Characters of a literal between the radix prefix and type suffix. - crate digits: &'a str, - /// Which radix the literal was represented in. - crate radix: Radix, - /// The radix prefix, if present. - crate prefix: Option<&'a str>, - /// The type suffix, including preceding underscore if present. - crate suffix: Option<&'a str>, - /// True for floating-point literals. - crate float: bool, +/// A helper method to format numeric literals with digit grouping. +/// `lit` must be a valid numeric literal without suffix. +pub fn format_numeric_literal(lit: &str, type_suffix: Option<&str>, float: bool) -> String { + NumericLiteral::new(lit, type_suffix, float).format() } -impl<'a> DigitInfo<'a> { +#[derive(Debug)] +pub(super) struct NumericLiteral<'a> { + /// Which radix the literal was represented in. + radix: Radix, + /// The radix prefix, if present. + prefix: Option<&'a str>, + + /// The integer part of the number. + integer: &'a str, + /// The fraction part of the number. + fraction: Option<&'a str>, + /// The character used as exponent seperator (b'e' or b'E') and the exponent part. + exponent: Option<(char, &'a str)>, + + /// The type suffix, including preceding underscore if present. + suffix: Option<&'a str>, +} + +impl<'a> NumericLiteral<'a> { + fn from_lit(src: &'a str, lit: &Lit) -> Option> { + if lit.kind.is_numeric() && src.chars().next().map_or(false, |c| c.is_digit(10)) { + let (unsuffixed, suffix) = split_suffix(&src, &lit.kind); + let float = if let LitKind::Float(..) = lit.kind { true } else { false }; + Some(NumericLiteral::new(unsuffixed, suffix, float)) + } else { + None + } + } + #[must_use] - crate fn new(lit: &'a str, float: bool) -> Self { + fn new(lit: &'a str, suffix: Option<&'a str>, float: bool) -> Self { // Determine delimiter for radix prefix, if present, and radix. let radix = if lit.starts_with("0x") { Radix::Hexadecimal @@ -151,131 +171,156 @@ impl<'a> DigitInfo<'a> { }; // Grab part of the literal after prefix, if present. - let (prefix, sans_prefix) = if let Radix::Decimal = radix { + let (prefix, mut sans_prefix) = if let Radix::Decimal = radix { (None, lit) } else { let (p, s) = lit.split_at(2); (Some(p), s) }; - let len = sans_prefix.len(); - let mut last_d = '\0'; - for (d_idx, d) in sans_prefix.char_indices() { - let suffix_start = if last_d == '_' { d_idx - 1 } else { d_idx }; - if float - && (d == 'f' - || is_possible_float_suffix_index(&sans_prefix, suffix_start, len) - || ((d == 'E' || d == 'e') && !has_possible_float_suffix(&sans_prefix))) - || !float && (d == 'i' || d == 'u' || is_possible_suffix_index(&sans_prefix, suffix_start, len)) - { - let (digits, suffix) = sans_prefix.split_at(suffix_start); - return Self { - digits, - radix, - prefix, - suffix: Some(suffix), - float, - }; - } - last_d = d + if suffix.is_some() && sans_prefix.ends_with('_') { + // The '_' before the suffix isn't part of the digits + sans_prefix = &sans_prefix[..sans_prefix.len() - 1]; } - // No suffix found + let (integer, fraction, exponent) = Self::split_digit_parts(sans_prefix, float); + Self { - digits: sans_prefix, radix, prefix, - suffix: None, - float, + integer, + fraction, + exponent, + suffix, } } + fn split_digit_parts(digits: &str, float: bool) -> (&str, Option<&str>, Option<(char, &str)>) { + let mut integer = digits; + let mut fraction = None; + let mut exponent = None; + + if float { + for (i, c) in digits.char_indices() { + match c { + '.' => { + integer = &digits[..i]; + fraction = Some(&digits[i + 1..]); + }, + 'e' | 'E' => { + if integer.len() > i { + integer = &digits[..i]; + } else { + fraction = Some(&digits[integer.len() + 1..i]); + }; + exponent = Some((c, &digits[i + 1..])); + break; + }, + _ => {}, + } + } + } + + (integer, fraction, exponent) + } + /// Returns literal formatted in a sensible way. - crate fn grouping_hint(&self) -> String { + fn format(&self) -> String { + let mut output = String::new(); + + if let Some(prefix) = self.prefix { + output.push_str(prefix); + } + let group_size = self.radix.suggest_grouping(); - if self.digits.contains('.') { - let mut parts = self.digits.split('.'); - let int_part_hint = parts - .next() - .expect("split always returns at least one element") - .chars() - .rev() - .filter(|&c| c != '_') - .collect::>() - .chunks(group_size) - .map(|chunk| chunk.iter().rev().collect()) - .rev() - .collect::>() - .join("_"); - let frac_part_hint = parts - .next() - .expect("already checked that there is a `.`") - .chars() - .filter(|&c| c != '_') - .collect::>() - .chunks(group_size) - .map(|chunk| chunk.iter().collect()) - .collect::>() - .join("_"); - let suffix_hint = match self.suffix { - Some(suffix) if is_mistyped_float_suffix(suffix) => format!("_f{}", &suffix[1..]), - Some(suffix) => suffix.to_string(), - None => String::new(), - }; - format!("{}.{}{}", int_part_hint, frac_part_hint, suffix_hint) - } else if self.float && (self.digits.contains('E') || self.digits.contains('e')) { - let which_e = if self.digits.contains('E') { 'E' } else { 'e' }; - let parts: Vec<&str> = self.digits.split(which_e).collect(); - let filtered_digits_vec_0 = parts[0].chars().filter(|&c| c != '_').rev().collect::>(); - let filtered_digits_vec_1 = parts[1].chars().filter(|&c| c != '_').rev().collect::>(); - let before_e_hint = filtered_digits_vec_0 - .chunks(group_size) - .map(|chunk| chunk.iter().rev().collect()) - .rev() - .collect::>() - .join("_"); - let after_e_hint = filtered_digits_vec_1 - .chunks(group_size) - .map(|chunk| chunk.iter().rev().collect()) - .rev() - .collect::>() - .join("_"); - let suffix_hint = match self.suffix { - Some(suffix) if is_mistyped_float_suffix(suffix) => format!("_f{}", &suffix[1..]), - Some(suffix) => suffix.to_string(), - None => String::new(), - }; - format!( - "{}{}{}{}{}", - self.prefix.unwrap_or(""), - before_e_hint, - which_e, - after_e_hint, - suffix_hint - ) - } else { - let filtered_digits_vec = self.digits.chars().filter(|&c| c != '_').rev().collect::>(); - let mut hint = filtered_digits_vec - .chunks(group_size) - .map(|chunk| chunk.iter().rev().collect()) - .rev() - .collect::>() - .join("_"); - // Forces hexadecimal values to be grouped by 4 being filled with zeroes (e.g 0x00ab_cdef) - let nb_digits_to_fill = filtered_digits_vec.len() % 4; - if self.radix == Radix::Hexadecimal && nb_digits_to_fill != 0 { - hint = format!("{:0>4}{}", &hint[..nb_digits_to_fill], &hint[nb_digits_to_fill..]); + + Self::group_digits( + &mut output, + self.integer, + group_size, + true, + self.radix == Radix::Hexadecimal, + ); + + if let Some(fraction) = self.fraction { + output.push('.'); + Self::group_digits(&mut output, fraction, group_size, false, false); + } + + if let Some((separator, exponent)) = self.exponent { + output.push(separator); + Self::group_digits(&mut output, exponent, group_size, true, false); + } + + if let Some(suffix) = self.suffix { + output.push('_'); + output.push_str(suffix); + } + + output + } + + fn group_digits(output: &mut String, input: &str, group_size: usize, partial_group_first: bool, pad: bool) { + debug_assert!(group_size > 0); + + let mut digits = input.chars().filter(|&c| c != '_'); + + let first_group_size; + + if partial_group_first { + first_group_size = (digits.clone().count() - 1) % group_size + 1; + if pad { + for _ in 0..group_size - first_group_size { + output.push('0'); + } } - let suffix_hint = match self.suffix { - Some(suffix) if is_mistyped_suffix(suffix) => format!("_i{}", &suffix[1..]), - Some(suffix) => suffix.to_string(), - None => String::new(), - }; - format!("{}{}{}", self.prefix.unwrap_or(""), hint, suffix_hint) + } else { + first_group_size = group_size; + } + + for _ in 0..first_group_size { + if let Some(digit) = digits.next() { + output.push(digit); + } + } + + for (c, i) in digits.zip((0..group_size).cycle()) { + if i == 0 { + output.push('_'); + } + output.push(c); } } } +fn split_suffix<'a>(src: &'a str, lit_kind: &LitKind) -> (&'a str, Option<&'a str>) { + debug_assert!(lit_kind.is_numeric()); + if let Some(suffix_length) = lit_suffix_length(lit_kind) { + let (unsuffixed, suffix) = src.split_at(src.len() - suffix_length); + (unsuffixed, Some(suffix)) + } else { + (src, None) + } +} + +fn lit_suffix_length(lit_kind: &LitKind) -> Option { + debug_assert!(lit_kind.is_numeric()); + let suffix = match lit_kind { + LitKind::Int(_, int_lit_kind) => match int_lit_kind { + LitIntType::Signed(int_ty) => Some(int_ty.name_str()), + LitIntType::Unsigned(uint_ty) => Some(uint_ty.name_str()), + LitIntType::Unsuffixed => None, + }, + LitKind::Float(_, float_lit_kind) => match float_lit_kind { + LitFloatType::Suffixed(float_ty) => Some(float_ty.name_str()), + LitFloatType::Unsuffixed => None, + }, + _ => None, + }; + + suffix.map(str::len) +} + enum WarningType { UnreadableLiteral, InconsistentDigitGrouping, @@ -285,7 +330,7 @@ enum WarningType { } impl WarningType { - crate fn display(&self, grouping_hint: &str, cx: &EarlyContext<'_>, span: syntax_pos::Span) { + fn display(&self, suggested_format: String, cx: &EarlyContext<'_>, span: syntax_pos::Span) { match self { Self::MistypedLiteralSuffix => span_lint_and_sugg( cx, @@ -293,7 +338,7 @@ impl WarningType { span, "mistyped literal suffix", "did you mean to write", - grouping_hint.to_string(), + suggested_format, Applicability::MaybeIncorrect, ), Self::UnreadableLiteral => span_lint_and_sugg( @@ -302,7 +347,7 @@ impl WarningType { span, "long literal lacking separators", "consider", - grouping_hint.to_owned(), + suggested_format, Applicability::MachineApplicable, ), Self::LargeDigitGroups => span_lint_and_sugg( @@ -311,7 +356,7 @@ impl WarningType { span, "digit groups should be smaller", "consider", - grouping_hint.to_owned(), + suggested_format, Applicability::MachineApplicable, ), Self::InconsistentDigitGrouping => span_lint_and_sugg( @@ -320,7 +365,7 @@ impl WarningType { span, "digits grouped inconsistently by underscores", "consider", - grouping_hint.to_owned(), + suggested_format, Applicability::MachineApplicable, ), Self::DecimalRepresentation => span_lint_and_sugg( @@ -329,7 +374,7 @@ impl WarningType { span, "integer literal has a better hexadecimal representation", "consider", - grouping_hint.to_owned(), + suggested_format, Applicability::MachineApplicable, ), }; @@ -357,67 +402,81 @@ impl EarlyLintPass for LiteralDigitGrouping { impl LiteralDigitGrouping { fn check_lit(cx: &EarlyContext<'_>, lit: &Lit) { - let in_macro = in_macro(lit.span); - match lit.kind { - LitKind::Int(..) => { - // Lint integral literals. - if_chain! { - if let Some(src) = snippet_opt(cx, lit.span); - if let Some(firstch) = src.chars().next(); - if char::is_digit(firstch, 10); - then { - let digit_info = DigitInfo::new(&src, false); - let _ = Self::do_lint(digit_info.digits, digit_info.suffix, in_macro).map_err(|warning_type| { - warning_type.display(&digit_info.grouping_hint(), cx, lit.span) - }); - } + if_chain! { + if let Some(src) = snippet_opt(cx, lit.span); + if let Some(mut num_lit) = NumericLiteral::from_lit(&src, &lit); + then { + if !Self::check_for_mistyped_suffix(cx, lit.span, &mut num_lit) { + return; } - }, - LitKind::Float(..) => { - // Lint floating-point literals. - if_chain! { - if let Some(src) = snippet_opt(cx, lit.span); - if let Some(firstch) = src.chars().next(); - if char::is_digit(firstch, 10); - then { - let digit_info = DigitInfo::new(&src, true); - // Separate digits into integral and fractional parts. - let parts: Vec<&str> = digit_info - .digits - .split_terminator('.') - .collect(); - // Lint integral and fractional parts separately, and then check consistency of digit - // groups if both pass. - let _ = Self::do_lint(parts[0], digit_info.suffix, in_macro) - .map(|integral_group_size| { - if parts.len() > 1 { - // Lint the fractional part of literal just like integral part, but reversed. - let fractional_part = &parts[1].chars().rev().collect::(); - let _ = Self::do_lint(fractional_part, None, in_macro) - .map(|fractional_group_size| { - let consistent = Self::parts_consistent(integral_group_size, - fractional_group_size, - parts[0].len(), - parts[1].len()); - if !consistent { - WarningType::InconsistentDigitGrouping.display( - &digit_info.grouping_hint(), - cx, - lit.span, - ); - } - }) - .map_err(|warning_type| warning_type.display(&digit_info.grouping_hint(), - cx, - lit.span)); - } - }) - .map_err(|warning_type| warning_type.display(&digit_info.grouping_hint(), cx, lit.span)); + let result = (|| { + + let integral_group_size = Self::get_group_size(num_lit.integer.split('_'))?; + if let Some(fraction) = num_lit.fraction { + let fractional_group_size = Self::get_group_size(fraction.rsplit('_'))?; + + let consistent = Self::parts_consistent(integral_group_size, + fractional_group_size, + num_lit.integer.len(), + fraction.len()); + if !consistent { + return Err(WarningType::InconsistentDigitGrouping); + }; + } + Ok(()) + })(); + + + if let Err(warning_type) = result { + let should_warn = match warning_type { + | WarningType::UnreadableLiteral + | WarningType::InconsistentDigitGrouping + | WarningType::LargeDigitGroups => { + !in_macro(lit.span) + } + WarningType::DecimalRepresentation | WarningType::MistypedLiteralSuffix => { + true + } + }; + if should_warn { + warning_type.display(num_lit.format(), cx, lit.span) } } - }, - _ => (), + } + } + } + + // Returns `false` if the check fails + fn check_for_mistyped_suffix( + cx: &EarlyContext<'_>, + span: syntax_pos::Span, + num_lit: &mut NumericLiteral<'_>, + ) -> bool { + if num_lit.suffix.is_some() { + return true; + } + + let (part, mistyped_suffixes, missing_char) = if let Some((_, exponent)) = &mut num_lit.exponent { + (exponent, &["32", "64"][..], 'f') + } else if let Some(fraction) = &mut num_lit.fraction { + (fraction, &["32", "64"][..], 'f') + } else { + (&mut num_lit.integer, &["8", "16", "32", "64"][..], 'i') + }; + + let mut split = part.rsplit('_'); + let last_group = split.next().expect("At least one group"); + if split.next().is_some() && mistyped_suffixes.contains(&last_group) { + *part = &part[..part.len() - last_group.len()]; + let mut sugg = num_lit.format(); + sugg.push('_'); + sugg.push(missing_char); + sugg.push_str(last_group); + WarningType::MistypedLiteralSuffix.display(sugg, cx, span); + false + } else { + true } } @@ -425,58 +484,43 @@ impl LiteralDigitGrouping { /// parts, and the length /// of both parts, determine if the digits have been grouped consistently. #[must_use] - fn parts_consistent(int_group_size: usize, frac_group_size: usize, int_size: usize, frac_size: usize) -> bool { + fn parts_consistent( + int_group_size: Option, + frac_group_size: Option, + int_size: usize, + frac_size: usize, + ) -> bool { match (int_group_size, frac_group_size) { // No groups on either side of decimal point - trivially consistent. - (0, 0) => true, + (None, None) => true, // Integral part has grouped digits, fractional part does not. - (_, 0) => frac_size <= int_group_size, + (Some(int_group_size), None) => frac_size <= int_group_size, // Fractional part has grouped digits, integral part does not. - (0, _) => int_size <= frac_group_size, + (None, Some(frac_group_size)) => int_size <= frac_group_size, // Both parts have grouped digits. Groups should be the same size. - (_, _) => int_group_size == frac_group_size, + (Some(int_group_size), Some(frac_group_size)) => int_group_size == frac_group_size, } } - /// Performs lint on `digits` (no decimal point) and returns the group - /// size on success or `WarningType` when emitting a warning. - fn do_lint(digits: &str, suffix: Option<&str>, in_macro: bool) -> Result { - if let Some(suffix) = suffix { - if is_mistyped_suffix(suffix) { - return Err(WarningType::MistypedLiteralSuffix); - } - } - // Grab underscore indices with respect to the units digit. - let underscore_positions: Vec = digits - .chars() - .rev() - .enumerate() - .filter_map(|(idx, digit)| if digit == '_' { Some(idx) } else { None }) - .collect(); + /// Returns the size of the digit groups (or None if ungrouped) if successful, + /// otherwise returns a `WarningType` for linting. + fn get_group_size<'a>(groups: impl Iterator) -> Result, WarningType> { + let mut groups = groups.map(str::len); - if underscore_positions.is_empty() { - // Check if literal needs underscores. - if !in_macro && digits.len() > 5 { - Err(WarningType::UnreadableLiteral) + let first = groups.next().expect("At least one group"); + + if let Some(second) = groups.next() { + if !groups.all(|x| x == second) || first > second { + Err(WarningType::InconsistentDigitGrouping) + } else if second > 4 { + Err(WarningType::LargeDigitGroups) } else { - Ok(0) + Ok(Some(second)) } + } else if first > 5 { + Err(WarningType::UnreadableLiteral) } else { - // Check consistency and the sizes of the groups. - let group_size = underscore_positions[0]; - let consistent = underscore_positions - .windows(2) - .all(|ps| ps[1] - ps[0] == group_size + 1) - // number of digits to the left of the last group cannot be bigger than group size. - && (digits.len() - underscore_positions.last() - .expect("there's at least one element") <= group_size + 1); - - if !consistent { - return Err(WarningType::InconsistentDigitGrouping); - } else if group_size > 4 { - return Err(WarningType::LargeDigitGroups); - } - Ok(group_size) + Ok(None) } } } @@ -511,16 +555,14 @@ impl DecimalLiteralRepresentation { if_chain! { if let LitKind::Int(val, _) = lit.kind; if let Some(src) = snippet_opt(cx, lit.span); - if let Some(firstch) = src.chars().next(); - if char::is_digit(firstch, 10); - let digit_info = DigitInfo::new(&src, false); - if digit_info.radix == Radix::Decimal; + if let Some(num_lit) = NumericLiteral::from_lit(&src, &lit); + if num_lit.radix == Radix::Decimal; if val >= u128::from(self.threshold); then { let hex = format!("{:#X}", val); - let digit_info = DigitInfo::new(&hex, false); - let _ = Self::do_lint(digit_info.digits).map_err(|warning_type| { - warning_type.display(&digit_info.grouping_hint(), cx, lit.span) + let num_lit = NumericLiteral::new(&hex, None, false); + let _ = Self::do_lint(num_lit.integer).map_err(|warning_type| { + warning_type.display(num_lit.format(), cx, lit.span) }); } } @@ -571,28 +613,3 @@ impl DecimalLiteralRepresentation { Ok(()) } } - -#[must_use] -fn is_mistyped_suffix(suffix: &str) -> bool { - ["_8", "_16", "_32", "_64"].contains(&suffix) -} - -#[must_use] -fn is_possible_suffix_index(lit: &str, idx: usize, len: usize) -> bool { - ((len > 3 && idx == len - 3) || (len > 2 && idx == len - 2)) && is_mistyped_suffix(lit.split_at(idx).1) -} - -#[must_use] -fn is_mistyped_float_suffix(suffix: &str) -> bool { - ["_32", "_64"].contains(&suffix) -} - -#[must_use] -fn is_possible_float_suffix_index(lit: &str, idx: usize, len: usize) -> bool { - (len > 3 && idx == len - 3) && is_mistyped_float_suffix(lit.split_at(idx).1) -} - -#[must_use] -fn has_possible_float_suffix(lit: &str) -> bool { - lit.ends_with("_32") || lit.ends_with("_64") -} diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 31d97d790cb..2a53c239649 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -2367,17 +2367,57 @@ fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, e return; }; let mutable_static_in_cond = var_visitor.def_ids.iter().any(|(_, v)| *v); + + let mut has_break_or_return_visitor = HasBreakOrReturnVisitor { + has_break_or_return: false, + }; + has_break_or_return_visitor.visit_expr(expr); + let has_break_or_return = has_break_or_return_visitor.has_break_or_return; + if no_cond_variable_mutated && !mutable_static_in_cond { - span_lint( + span_lint_and_then( cx, WHILE_IMMUTABLE_CONDITION, cond.span, - "Variable in the condition are not mutated in the loop body. \ - This either leads to an infinite or to a never running loop.", + "variables in the condition are not mutated in the loop body", + |db| { + db.note("this may lead to an infinite or to a never running loop"); + + if has_break_or_return { + db.note("this loop contains `return`s or `break`s"); + db.help("rewrite it as `if cond { loop { } }`"); + } + }, ); } } +struct HasBreakOrReturnVisitor { + has_break_or_return: bool, +} + +impl<'a, 'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor { + fn visit_expr(&mut self, expr: &'tcx Expr) { + if self.has_break_or_return { + return; + } + + match expr.kind { + ExprKind::Ret(_) | ExprKind::Break(_, _) => { + self.has_break_or_return = true; + return; + }, + _ => {}, + } + + walk_expr(self, expr); + } + + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + NestedVisitorMap::None + } +} + /// Collects the set of variables in an expression /// Stops analysis if a function call is found /// Note: In some cases such as `self`, there are no mutable annotation, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index f20abeff065..78188527cba 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2424,7 +2424,7 @@ fn lint_find_map<'a, 'tcx>( } } -/// lint use of `filter().map()` for `Iterators` +/// lint use of `filter_map().map()` for `Iterators` fn lint_filter_map_map<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index fca70fe8dbd..a3d1193052e 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -154,7 +154,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringLitAsBytes { if let LitKind::Str(ref lit_content, style) = lit.node { let callsite = snippet(cx, args[0].span.source_callsite(), r#""foo""#); let expanded = if let StrStyle::Raw(n) = style { - let term = (0..n).map(|_| '#').collect::(); + let term = "#".repeat(usize::from(n)); format!("r{0}\"{1}\"{0}", term, lit_content.as_str()) } else { format!("\"{}\"", lit_content.as_str()) diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index ff874179cff..228fda9eec0 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -18,6 +18,8 @@ use syntax::token; use syntax::util::parser::AssocOp; use syntax_pos::{BytePos, Pos}; +pub use crate::literal_representation::format_numeric_literal; + /// A helper type to build suggestion correctly handling parenthesis. pub enum Sugg<'a> { /// An expression that never needs parenthesis such as `1337` or `[0; 42]`. diff --git a/tests/ui/derive_hash_xor_eq.rs b/tests/ui/derive_hash_xor_eq.rs index c0be787f5e4..10abe22d53e 100644 --- a/tests/ui/derive_hash_xor_eq.rs +++ b/tests/ui/derive_hash_xor_eq.rs @@ -1,5 +1,3 @@ -use std::hash::{Hash, Hasher}; - #[derive(PartialEq, Hash)] struct Foo; @@ -30,8 +28,27 @@ impl PartialEq for Baz { #[derive(PartialEq)] struct Bah; -impl Hash for Bah { - fn hash(&self, _: &mut H) {} +impl std::hash::Hash for Bah { + fn hash(&self, _: &mut H) {} +} + +#[derive(PartialEq)] +struct Foo2; + +trait Hash {} + +// We don't want to lint on user-defined traits called `Hash` +impl Hash for Foo2 {} + +mod use_hash { + use std::hash::{Hash, Hasher}; + + #[derive(PartialEq)] + struct Foo3; + + impl Hash for Foo3 { + fn hash(&self, _: &mut H) {} + } } fn main() {} diff --git a/tests/ui/derive_hash_xor_eq.stderr b/tests/ui/derive_hash_xor_eq.stderr index c3d451aaed6..10c38bb42e3 100644 --- a/tests/ui/derive_hash_xor_eq.stderr +++ b/tests/ui/derive_hash_xor_eq.stderr @@ -1,12 +1,12 @@ error: you are deriving `Hash` but have implemented `PartialEq` explicitly - --> $DIR/derive_hash_xor_eq.rs:12:10 + --> $DIR/derive_hash_xor_eq.rs:10:10 | LL | #[derive(Hash)] | ^^^^ | = note: `#[deny(clippy::derive_hash_xor_eq)]` on by default note: `PartialEq` implemented here - --> $DIR/derive_hash_xor_eq.rs:15:1 + --> $DIR/derive_hash_xor_eq.rs:13:1 | LL | / impl PartialEq for Bar { LL | | fn eq(&self, _: &Bar) -> bool { @@ -16,13 +16,13 @@ LL | | } | |_^ error: you are deriving `Hash` but have implemented `PartialEq` explicitly - --> $DIR/derive_hash_xor_eq.rs:21:10 + --> $DIR/derive_hash_xor_eq.rs:19:10 | LL | #[derive(Hash)] | ^^^^ | note: `PartialEq` implemented here - --> $DIR/derive_hash_xor_eq.rs:24:1 + --> $DIR/derive_hash_xor_eq.rs:22:1 | LL | / impl PartialEq for Baz { LL | | fn eq(&self, _: &Baz) -> bool { @@ -32,18 +32,32 @@ LL | | } | |_^ error: you are implementing `Hash` explicitly but have derived `PartialEq` - --> $DIR/derive_hash_xor_eq.rs:33:1 + --> $DIR/derive_hash_xor_eq.rs:31:1 | -LL | / impl Hash for Bah { -LL | | fn hash(&self, _: &mut H) {} +LL | / impl std::hash::Hash for Bah { +LL | | fn hash(&self, _: &mut H) {} LL | | } | |_^ | note: `PartialEq` implemented here - --> $DIR/derive_hash_xor_eq.rs:30:10 + --> $DIR/derive_hash_xor_eq.rs:28:10 | LL | #[derive(PartialEq)] | ^^^^^^^^^ -error: aborting due to 3 previous errors +error: you are implementing `Hash` explicitly but have derived `PartialEq` + --> $DIR/derive_hash_xor_eq.rs:49:5 + | +LL | / impl Hash for Foo3 { +LL | | fn hash(&self, _: &mut H) {} +LL | | } + | |_____^ + | +note: `PartialEq` implemented here + --> $DIR/derive_hash_xor_eq.rs:46:14 + | +LL | #[derive(PartialEq)] + | ^^^^^^^^^ + +error: aborting due to 4 previous errors diff --git a/tests/ui/inconsistent_digit_grouping.fixed b/tests/ui/inconsistent_digit_grouping.fixed index f25be70737b..f10673adfb2 100644 --- a/tests/ui/inconsistent_digit_grouping.fixed +++ b/tests/ui/inconsistent_digit_grouping.fixed @@ -2,6 +2,17 @@ #[warn(clippy::inconsistent_digit_grouping)] #[allow(unused_variables, clippy::excessive_precision)] fn main() { + macro_rules! mac1 { + () => { + 1_23_456 + }; + } + macro_rules! mac2 { + () => { + 1_234.5678_f32 + }; + } + let good = ( 123, 1_234, @@ -12,4 +23,17 @@ fn main() { 1.123_456_7_f32, ); let bad = (123_456, 12_345_678, 1_234_567, 1_234.567_8_f32, 1.234_567_8_f32); + + // Test padding + let _ = 0x0010_0000; + let _ = 0x0100_0000; + let _ = 0x1000_0000; + let _ = 0x0001_0000_0000_u64; + + // Test suggestion when fraction has no digits + let _: f32 = 123_456.; + + // Ignore literals in macros + let _ = mac1!(); + let _ = mac2!(); } diff --git a/tests/ui/inconsistent_digit_grouping.rs b/tests/ui/inconsistent_digit_grouping.rs index 206fac8d3e3..b97df0865ee 100644 --- a/tests/ui/inconsistent_digit_grouping.rs +++ b/tests/ui/inconsistent_digit_grouping.rs @@ -2,6 +2,17 @@ #[warn(clippy::inconsistent_digit_grouping)] #[allow(unused_variables, clippy::excessive_precision)] fn main() { + macro_rules! mac1 { + () => { + 1_23_456 + }; + } + macro_rules! mac2 { + () => { + 1_234.5678_f32 + }; + } + let good = ( 123, 1_234, @@ -12,4 +23,17 @@ fn main() { 1.123_456_7_f32, ); let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); + + // Test padding + let _ = 0x100000; + let _ = 0x1000000; + let _ = 0x10000000; + let _ = 0x100000000_u64; + + // Test suggestion when fraction has no digits + let _: f32 = 1_23_456.; + + // Ignore literals in macros + let _ = mac1!(); + let _ = mac2!(); } diff --git a/tests/ui/inconsistent_digit_grouping.stderr b/tests/ui/inconsistent_digit_grouping.stderr index 9fc1f424dc6..37211efcab5 100644 --- a/tests/ui/inconsistent_digit_grouping.stderr +++ b/tests/ui/inconsistent_digit_grouping.stderr @@ -1,5 +1,5 @@ error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:14:16 + --> $DIR/inconsistent_digit_grouping.rs:25:16 | LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); | ^^^^^^^^ help: consider: `123_456` @@ -7,28 +7,60 @@ LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f = note: `-D clippy::inconsistent-digit-grouping` implied by `-D warnings` error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:14:26 + --> $DIR/inconsistent_digit_grouping.rs:25:26 | LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); | ^^^^^^^^^^ help: consider: `12_345_678` error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:14:38 + --> $DIR/inconsistent_digit_grouping.rs:25:38 | LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); | ^^^^^^^^ help: consider: `1_234_567` error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:14:48 + --> $DIR/inconsistent_digit_grouping.rs:25:48 | LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); | ^^^^^^^^^^^^^^ help: consider: `1_234.567_8_f32` error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:14:64 + --> $DIR/inconsistent_digit_grouping.rs:25:64 | LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); | ^^^^^^^^^^^^^^ help: consider: `1.234_567_8_f32` -error: aborting due to 5 previous errors +error: long literal lacking separators + --> $DIR/inconsistent_digit_grouping.rs:28:13 + | +LL | let _ = 0x100000; + | ^^^^^^^^ help: consider: `0x0010_0000` + | + = note: `-D clippy::unreadable-literal` implied by `-D warnings` + +error: long literal lacking separators + --> $DIR/inconsistent_digit_grouping.rs:29:13 + | +LL | let _ = 0x1000000; + | ^^^^^^^^^ help: consider: `0x0100_0000` + +error: long literal lacking separators + --> $DIR/inconsistent_digit_grouping.rs:30:13 + | +LL | let _ = 0x10000000; + | ^^^^^^^^^^ help: consider: `0x1000_0000` + +error: long literal lacking separators + --> $DIR/inconsistent_digit_grouping.rs:31:13 + | +LL | let _ = 0x100000000_u64; + | ^^^^^^^^^^^^^^^ help: consider: `0x0001_0000_0000_u64` + +error: digits grouped inconsistently by underscores + --> $DIR/inconsistent_digit_grouping.rs:34:18 + | +LL | let _: f32 = 1_23_456.; + | ^^^^^^^^^ help: consider: `123_456.` + +error: aborting due to 10 previous errors diff --git a/tests/ui/infinite_loop.rs b/tests/ui/infinite_loop.rs index 4df218aa4f3..09f47adc46e 100644 --- a/tests/ui/infinite_loop.rs +++ b/tests/ui/infinite_loop.rs @@ -177,6 +177,23 @@ impl Counter { } } +fn while_loop_with_break_and_return() { + let y = 0; + while y < 10 { + if y == 0 { + break; + } + println!("KO - loop contains break"); + } + + while y < 10 { + if y == 0 { + return; + } + println!("KO - loop contains return"); + } +} + fn main() { immutable_condition(); unused_var(); @@ -186,4 +203,6 @@ fn main() { let mut c = Counter { count: 0 }; c.inc_n(5); c.print_n(2); + + while_loop_with_break_and_return(); } diff --git a/tests/ui/infinite_loop.stderr b/tests/ui/infinite_loop.stderr index 6f0332fa8c4..2736753c14b 100644 --- a/tests/ui/infinite_loop.stderr +++ b/tests/ui/infinite_loop.stderr @@ -1,58 +1,95 @@ -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:23:11 | LL | while y < 10 { | ^^^^^^ | = note: `#[deny(clippy::while_immutable_condition)]` on by default + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:28:11 | LL | while y < 10 && x < 3 { | ^^^^^^^^^^^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:35:11 | LL | while !cond { | ^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:79:11 | LL | while i < 3 { | ^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:84:11 | LL | while i < 3 && j > 0 { | ^^^^^^^^^^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:88:11 | LL | while i < 3 { | ^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:103:11 | LL | while i < 3 { | ^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:108:11 | LL | while i < 3 { | ^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:174:15 | LL | while self.count < n { | ^^^^^^^^^^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: aborting due to 9 previous errors +error: variables in the condition are not mutated in the loop body + --> $DIR/infinite_loop.rs:182:11 + | +LL | while y < 10 { + | ^^^^^^ + | + = note: this may lead to an infinite or to a never running loop + = note: this loop contains `return`s or `break`s + = help: rewrite it as `if cond { loop { } }` + +error: variables in the condition are not mutated in the loop body + --> $DIR/infinite_loop.rs:189:11 + | +LL | while y < 10 { + | ^^^^^^ + | + = note: this may lead to an infinite or to a never running loop + = note: this loop contains `return`s or `break`s + = help: rewrite it as `if cond { loop { } }` + +error: aborting due to 11 previous errors diff --git a/tests/ui/large_digit_groups.fixed b/tests/ui/large_digit_groups.fixed index cf8b36a499b..02daa22bb36 100644 --- a/tests/ui/large_digit_groups.fixed +++ b/tests/ui/large_digit_groups.fixed @@ -2,6 +2,12 @@ #[warn(clippy::large_digit_groups)] #[allow(unused_variables)] fn main() { + macro_rules! mac { + () => { + 0b1_10110_i64 + }; + } + let good = ( 0b1011_i64, 0o1_234_u32, @@ -20,4 +26,7 @@ fn main() { 123_456.123_45_f64, 123_456.123_456_f64, ); + + // Ignore literals in macros + let _ = mac!(); } diff --git a/tests/ui/large_digit_groups.rs b/tests/ui/large_digit_groups.rs index 5b9aa8c58d8..c1bb78c9d83 100644 --- a/tests/ui/large_digit_groups.rs +++ b/tests/ui/large_digit_groups.rs @@ -2,6 +2,12 @@ #[warn(clippy::large_digit_groups)] #[allow(unused_variables)] fn main() { + macro_rules! mac { + () => { + 0b1_10110_i64 + }; + } + let good = ( 0b1011_i64, 0o1_234_u32, @@ -20,4 +26,7 @@ fn main() { 1_23456.12345_f64, 1_23456.12345_6_f64, ); + + // Ignore literals in macros + let _ = mac!(); } diff --git a/tests/ui/large_digit_groups.stderr b/tests/ui/large_digit_groups.stderr index 4b5d0bd1a9f..ba8ea6b53e7 100644 --- a/tests/ui/large_digit_groups.stderr +++ b/tests/ui/large_digit_groups.stderr @@ -1,5 +1,5 @@ error: digit groups should be smaller - --> $DIR/large_digit_groups.rs:16:9 + --> $DIR/large_digit_groups.rs:22:9 | LL | 0b1_10110_i64, | ^^^^^^^^^^^^^ help: consider: `0b11_0110_i64` @@ -7,31 +7,31 @@ LL | 0b1_10110_i64, = note: `-D clippy::large-digit-groups` implied by `-D warnings` error: digit groups should be smaller - --> $DIR/large_digit_groups.rs:17:9 + --> $DIR/large_digit_groups.rs:23:9 | LL | 0x1_23456_78901_usize, | ^^^^^^^^^^^^^^^^^^^^^ help: consider: `0x0123_4567_8901_usize` error: digit groups should be smaller - --> $DIR/large_digit_groups.rs:18:9 + --> $DIR/large_digit_groups.rs:24:9 | LL | 1_23456_f32, | ^^^^^^^^^^^ help: consider: `123_456_f32` error: digit groups should be smaller - --> $DIR/large_digit_groups.rs:19:9 + --> $DIR/large_digit_groups.rs:25:9 | LL | 1_23456.12_f32, | ^^^^^^^^^^^^^^ help: consider: `123_456.12_f32` error: digit groups should be smaller - --> $DIR/large_digit_groups.rs:20:9 + --> $DIR/large_digit_groups.rs:26:9 | LL | 1_23456.12345_f64, | ^^^^^^^^^^^^^^^^^ help: consider: `123_456.123_45_f64` error: digit groups should be smaller - --> $DIR/large_digit_groups.rs:21:9 + --> $DIR/large_digit_groups.rs:27:9 | LL | 1_23456.12345_6_f64, | ^^^^^^^^^^^^^^^^^^^ help: consider: `123_456.123_456_f64` diff --git a/tests/ui/mistyped_literal_suffix.fixed b/tests/ui/mistyped_literal_suffix.fixed index 531e44a781c..baee7735730 100644 --- a/tests/ui/mistyped_literal_suffix.fixed +++ b/tests/ui/mistyped_literal_suffix.fixed @@ -19,4 +19,6 @@ fn main() { #[allow(overflowing_literals)] let fail28 = 241_251_235E723_f64; let fail29 = 42_279.911_f32; + + let _ = 1.123_45E1_f32; } diff --git a/tests/ui/mistyped_literal_suffix.rs b/tests/ui/mistyped_literal_suffix.rs index d67c842b4af..6de447f4021 100644 --- a/tests/ui/mistyped_literal_suffix.rs +++ b/tests/ui/mistyped_literal_suffix.rs @@ -19,4 +19,6 @@ fn main() { #[allow(overflowing_literals)] let fail28 = 241251235E723_64; let fail29 = 42279.911_32; + + let _ = 1.12345E1_32; } diff --git a/tests/ui/mistyped_literal_suffix.stderr b/tests/ui/mistyped_literal_suffix.stderr index 17c8b323027..48a7ae90494 100644 --- a/tests/ui/mistyped_literal_suffix.stderr +++ b/tests/ui/mistyped_literal_suffix.stderr @@ -72,5 +72,11 @@ error: mistyped literal suffix LL | let fail29 = 42279.911_32; | ^^^^^^^^^^^^ help: did you mean to write: `42_279.911_f32` -error: aborting due to 12 previous errors +error: mistyped literal suffix + --> $DIR/mistyped_literal_suffix.rs:23:13 + | +LL | let _ = 1.12345E1_32; + | ^^^^^^^^^^^^ help: did you mean to write: `1.123_45E1_f32` + +error: aborting due to 13 previous errors