From 0307ff020ce40fc025aa81dc09e0e50e5e34849f Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Mon, 8 Apr 2019 22:06:02 +0200 Subject: [PATCH 1/4] Fix ICE in decimal_literal_representation lint Handling the integer parsing properly instead of just unwrapping. Note that the test is not catching the ICE because plain UI tests [currently hide ICEs][compiletest_issue]. Once that issue is fixed, this test would fail properly again. [compiletest_issue]: https://github.com/laumann/compiletest-rs/issues/169 --- clippy_lints/src/literal_representation.rs | 22 +++++++++++++--------- tests/ui/crashes/ice-3891.rs | 3 +++ tests/ui/crashes/ice-3891.stderr | 10 ++++++++++ 3 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 tests/ui/crashes/ice-3891.rs create mode 100644 tests/ui/crashes/ice-3891.stderr diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 5c2ca4b1c935..3ea818d3db60 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -529,19 +529,23 @@ impl LiteralRepresentation { then { let digit_info = DigitInfo::new(&src, false); if digit_info.radix == Radix::Decimal { - let val = digit_info.digits + if let Ok(val) = digit_info.digits .chars() .filter(|&c| c != '_') .collect::() - .parse::().unwrap(); - if val < u128::from(self.threshold) { - return + .parse::() { + if val < u128::from(self.threshold) { + return + } + 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 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) - }); + else { + return + }; } } } diff --git a/tests/ui/crashes/ice-3891.rs b/tests/ui/crashes/ice-3891.rs new file mode 100644 index 000000000000..05c5134c8457 --- /dev/null +++ b/tests/ui/crashes/ice-3891.rs @@ -0,0 +1,3 @@ +fn main() { + 1x; +} diff --git a/tests/ui/crashes/ice-3891.stderr b/tests/ui/crashes/ice-3891.stderr new file mode 100644 index 000000000000..16aedbd98dec --- /dev/null +++ b/tests/ui/crashes/ice-3891.stderr @@ -0,0 +1,10 @@ +error: invalid suffix `x` for numeric literal + --> $DIR/ice-3891.rs:2:5 + | +LL | 1x; + | ^^ invalid suffix `x` + | + = help: the suffix must be one of the integral types (`u32`, `isize`, etc) + +error: aborting due to previous error + From 0eb7596fdcda78543af05eefe1b94cdd85ba8ee5 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Tue, 9 Apr 2019 20:53:38 +0200 Subject: [PATCH 2/4] Exclude ice-3891.rs from rustfmt run Because the code triggers a rustc parse error which makes rustfmt fail. --- ci/base-tests.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ci/base-tests.sh b/ci/base-tests.sh index fbb5d1cba48d..64214e3be865 100755 --- a/ci/base-tests.sh +++ b/ci/base-tests.sh @@ -59,7 +59,9 @@ rustup override set nightly # avoid loop spam and allow cmds with exit status != 0 set +ex -for file in `find tests | grep "\.rs$"` ; do +# Excluding `ice-3891.rs` because the code triggers a rustc parse error which +# makes rustfmt fail. +for file in `find tests -not -path "tests/ui/crashes/ice-3891.rs" | grep "\.rs$"` ; do rustfmt ${file} --check if [ $? -ne 0 ]; then echo "${file} needs reformatting!" From 1fd2451b901f26b3197b9c326ae8127f0788ef26 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 10 Apr 2019 07:50:34 +0200 Subject: [PATCH 3/4] Code formatting/cleanup --- clippy_lints/src/literal_representation.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 3ea818d3db60..6991be7d7909 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -538,13 +538,10 @@ impl LiteralRepresentation { return } let hex = format!("{:#X}", val); - let digit_info = DigitInfo::new(&hex[..], false); + 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) }); - } - else { - return }; } } From ab6b949224a4704641e7c9d24163b9b99d3b47ea Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 10 Apr 2019 21:05:56 +0200 Subject: [PATCH 4/4] Refactor check_lit method --- clippy_lints/src/literal_representation.rs | 30 ++++++++++------------ 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 6991be7d7909..e97845314f9d 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -526,24 +526,20 @@ impl LiteralRepresentation { if let Some(src) = snippet_opt(cx, lit.span); if let Some(firstch) = src.chars().next(); if char::to_digit(firstch, 10).is_some(); + let digit_info = DigitInfo::new(&src, false); + if digit_info.radix == Radix::Decimal; + if let Ok(val) = digit_info.digits + .chars() + .filter(|&c| c != '_') + .collect::() + .parse::(); + if val >= u128::from(self.threshold); then { - let digit_info = DigitInfo::new(&src, false); - if digit_info.radix == Radix::Decimal { - if let Ok(val) = digit_info.digits - .chars() - .filter(|&c| c != '_') - .collect::() - .parse::() { - if val < u128::from(self.threshold) { - return - } - 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 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) + }); } } }