From a8f77e12fca0072d08745feb8c32900c5a46967a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 15 Jan 2018 13:01:05 -0800 Subject: [PATCH 1/4] Include space in suggestion `mut` in bindings --- src/librustc_borrowck/borrowck/unused.rs | 2 +- src/test/ui/lint/suggestions.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_borrowck/borrowck/unused.rs b/src/librustc_borrowck/borrowck/unused.rs index ddee122d0a6bd..f00a1c24bc350 100644 --- a/src/librustc_borrowck/borrowck/unused.rs +++ b/src/librustc_borrowck/borrowck/unused.rs @@ -77,7 +77,7 @@ impl<'a, 'tcx> UnusedMutCx<'a, 'tcx> { continue } - let mut_span = tcx.sess.codemap().span_until_char(ids[0].2, ' '); + let mut_span = tcx.sess.codemap().span_through_char(ids[0].2, ' '); // Ok, every name wasn't used mutably, so issue a warning that this // didn't need to be mutable. diff --git a/src/test/ui/lint/suggestions.stderr b/src/test/ui/lint/suggestions.stderr index 8b30f552d3771..d04884d29ccac 100644 --- a/src/test/ui/lint/suggestions.stderr +++ b/src/test/ui/lint/suggestions.stderr @@ -22,7 +22,7 @@ warning: variable does not need to be mutable --> $DIR/suggestions.rs:46:13 | 46 | let mut a = (1); // should suggest no `mut`, no parens - | ---^^ + | ----^ | | | help: remove this `mut` | From d0bd090efb574cda08647ac07e1870cdafe4ac7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 23 Jan 2018 10:54:57 -0800 Subject: [PATCH 2/4] Consider all whitespace when preparing span --- src/librustc_borrowck/borrowck/unused.rs | 2 +- src/libsyntax/codemap.rs | 28 +++++++++- src/test/ui/lint/suggestions.rs | 8 ++- src/test/ui/lint/suggestions.stderr | 66 ++++++++++++++---------- 4 files changed, 73 insertions(+), 31 deletions(-) diff --git a/src/librustc_borrowck/borrowck/unused.rs b/src/librustc_borrowck/borrowck/unused.rs index f00a1c24bc350..7bcd8a185453b 100644 --- a/src/librustc_borrowck/borrowck/unused.rs +++ b/src/librustc_borrowck/borrowck/unused.rs @@ -77,7 +77,7 @@ impl<'a, 'tcx> UnusedMutCx<'a, 'tcx> { continue } - let mut_span = tcx.sess.codemap().span_through_char(ids[0].2, ' '); + let mut_span = tcx.sess.codemap().span_until_non_whitespace(ids[0].2); // Ok, every name wasn't used mutably, so issue a warning that this // didn't need to be mutable. diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index a58a61c36361b..c9b524a0c8973 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -592,8 +592,32 @@ impl CodeMap { } } - /// Given a `Span`, try to get a shorter span ending just after the first - /// occurrence of `char` `c`. + /// Given a `Span`, get a new `Span` covering the first token and all its trailing whitespace or + /// the original `Span`. + /// + /// If `sp` points to `"let mut x"`, then a span pointing at `"let "` will be returned. + pub fn span_until_non_whitespace(&self, sp: Span) -> Span { + if let Ok(snippet) = self.span_to_snippet(sp) { + let mut offset = 0; + let mut pos = 0; + // get the bytes width of all the non-whitespace characters + for (i, c) in snippet.chars().take_while(|c| !c.is_whitespace()).enumerate() { + offset += c.len_utf8(); + pos = i + 1; + } + // get the bytes width of all the whitespace characters after that + for c in snippet[pos..].chars().take_while(|c| c.is_whitespace()) { + offset += c.len_utf8(); + } + if offset != 0 { + return sp.with_hi(BytePos(sp.lo().0 + offset as u32)); + } + } + sp + } + + /// Given a `Span`, try to get a shorter span ending just after the first occurrence of `char` + /// `c`. pub fn span_through_char(&self, sp: Span, c: char) -> Span { if let Ok(snippet) = self.span_to_snippet(sp) { if let Some(offset) = snippet.find(c) { diff --git a/src/test/ui/lint/suggestions.rs b/src/test/ui/lint/suggestions.rs index dfcaede1402da..e35675eacd835 100644 --- a/src/test/ui/lint/suggestions.rs +++ b/src/test/ui/lint/suggestions.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// ignore-tidy-tab + #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issue #43896 #![feature(no_debug)] @@ -46,11 +48,15 @@ fn main() { let mut a = (1); // should suggest no `mut`, no parens //~^ WARN does not need to be mutable //~| WARN unnecessary parentheses + // the line after `mut` has a `\t` at the beginning, this is on purpose + let mut + b = 1; + //~^^ WARN does not need to be mutable let d = Equinox { warp_factor: 9.975 }; match d { Equinox { warp_factor: warp_factor } => {} // should suggest shorthand //~^ WARN this pattern is redundant } - println!("{}", a); + println!("{} {}", a, b); } } diff --git a/src/test/ui/lint/suggestions.stderr b/src/test/ui/lint/suggestions.stderr index d04884d29ccac..90d6bd312e419 100644 --- a/src/test/ui/lint/suggestions.stderr +++ b/src/test/ui/lint/suggestions.stderr @@ -1,41 +1,53 @@ warning: unnecessary parentheses around assigned value - --> $DIR/suggestions.rs:46:21 + --> $DIR/suggestions.rs:48:21 | -46 | let mut a = (1); // should suggest no `mut`, no parens +48 | let mut a = (1); // should suggest no `mut`, no parens | ^^^ help: remove these parentheses | note: lint level defined here - --> $DIR/suggestions.rs:11:21 + --> $DIR/suggestions.rs:13:21 | -11 | #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issue #43896 +13 | #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issue #43896 | ^^^^^^^^^^^^^ warning: use of deprecated attribute `no_debug`: the `#[no_debug]` attribute was an experimental feature that has been deprecated due to lack of demand. See https://github.com/rust-lang/rust/issues/29721 - --> $DIR/suggestions.rs:41:1 + --> $DIR/suggestions.rs:43:1 | -41 | #[no_debug] // should suggest removal of deprecated attribute +43 | #[no_debug] // should suggest removal of deprecated attribute | ^^^^^^^^^^^ help: remove this attribute | = note: #[warn(deprecated)] on by default warning: variable does not need to be mutable - --> $DIR/suggestions.rs:46:13 + --> $DIR/suggestions.rs:48:13 | -46 | let mut a = (1); // should suggest no `mut`, no parens +48 | let mut a = (1); // should suggest no `mut`, no parens | ----^ | | | help: remove this `mut` | note: lint level defined here - --> $DIR/suggestions.rs:11:9 + --> $DIR/suggestions.rs:13:9 | -11 | #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issue #43896 +13 | #![warn(unused_mut, unused_parens)] // UI tests pass `-A unused`—see Issue #43896 | ^^^^^^^^^^ +warning: variable does not need to be mutable + --> $DIR/suggestions.rs:52:13 + | +52 | let mut + | _____________^ + | |_____________| + | || +53 | || b = 1; + | ||____________-^ + | |____________| + | help: remove this `mut` + warning: static is marked #[no_mangle], but not exported - --> $DIR/suggestions.rs:14:14 + --> $DIR/suggestions.rs:16:14 | -14 | #[no_mangle] static SHENZHOU: usize = 1; // should suggest `pub` +16 | #[no_mangle] static SHENZHOU: usize = 1; // should suggest `pub` | -^^^^^^^^^^^^^^^^^^^^^^^^^^ | | | help: try making it public: `pub` @@ -43,9 +55,9 @@ warning: static is marked #[no_mangle], but not exported = note: #[warn(private_no_mangle_statics)] on by default error: const items should never be #[no_mangle] - --> $DIR/suggestions.rs:16:14 + --> $DIR/suggestions.rs:18:14 | -16 | #[no_mangle] const DISCOVERY: usize = 1; // should suggest `pub static` rather than `const` +18 | #[no_mangle] const DISCOVERY: usize = 1; // should suggest `pub static` rather than `const` | -----^^^^^^^^^^^^^^^^^^^^^^ | | | help: try a static value: `pub static` @@ -53,19 +65,19 @@ error: const items should never be #[no_mangle] = note: #[deny(no_mangle_const_items)] on by default warning: functions generic over types must be mangled - --> $DIR/suggestions.rs:20:1 + --> $DIR/suggestions.rs:22:1 | -19 | #[no_mangle] // should suggest removal (generics can't be no-mangle) +21 | #[no_mangle] // should suggest removal (generics can't be no-mangle) | ------------ help: remove this attribute -20 | pub fn defiant(_t: T) {} +22 | pub fn defiant(_t: T) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: #[warn(no_mangle_generic_items)] on by default warning: function is marked #[no_mangle], but not exported - --> $DIR/suggestions.rs:24:1 + --> $DIR/suggestions.rs:26:1 | -24 | fn rio_grande() {} // should suggest `pub` +26 | fn rio_grande() {} // should suggest `pub` | -^^^^^^^^^^^^^^^^^ | | | help: try making it public: `pub` @@ -73,29 +85,29 @@ warning: function is marked #[no_mangle], but not exported = note: #[warn(private_no_mangle_fns)] on by default warning: static is marked #[no_mangle], but not exported - --> $DIR/suggestions.rs:31:18 + --> $DIR/suggestions.rs:33:18 | -31 | #[no_mangle] pub static DAUNTLESS: bool = true; +33 | #[no_mangle] pub static DAUNTLESS: bool = true; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: function is marked #[no_mangle], but not exported - --> $DIR/suggestions.rs:33:18 + --> $DIR/suggestions.rs:35:18 | -33 | #[no_mangle] pub fn val_jean() {} +35 | #[no_mangle] pub fn val_jean() {} | ^^^^^^^^^^^^^^^^^^^^ warning: denote infinite loops with `loop { ... }` - --> $DIR/suggestions.rs:44:5 + --> $DIR/suggestions.rs:46:5 | -44 | while true { // should suggest `loop` +46 | while true { // should suggest `loop` | ^^^^^^^^^^ help: use `loop` | = note: #[warn(while_true)] on by default warning: the `warp_factor:` in this pattern is redundant - --> $DIR/suggestions.rs:51:23 + --> $DIR/suggestions.rs:57:23 | -51 | Equinox { warp_factor: warp_factor } => {} // should suggest shorthand +57 | Equinox { warp_factor: warp_factor } => {} // should suggest shorthand | ------------^^^^^^^^^^^^ | | | help: remove this From fa7767e1ea825bb4cbadbb05c0bfa8ff8a9100e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 23 Jan 2018 13:53:08 -0800 Subject: [PATCH 3/4] review comment --- src/libsyntax/codemap.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index c9b524a0c8973..fbd9088797e22 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -598,18 +598,16 @@ impl CodeMap { /// If `sp` points to `"let mut x"`, then a span pointing at `"let "` will be returned. pub fn span_until_non_whitespace(&self, sp: Span) -> Span { if let Ok(snippet) = self.span_to_snippet(sp) { - let mut offset = 0; - let mut pos = 0; + let mut offset = 1; // get the bytes width of all the non-whitespace characters - for (i, c) in snippet.chars().take_while(|c| !c.is_whitespace()).enumerate() { + for c in snippet.chars().take_while(|c| !c.is_whitespace()) { offset += c.len_utf8(); - pos = i + 1; } // get the bytes width of all the whitespace characters after that - for c in snippet[pos..].chars().take_while(|c| c.is_whitespace()) { + for c in snippet[offset..].chars().take_while(|c| c.is_whitespace()) { offset += c.len_utf8(); } - if offset != 0 { + if offset > 1 { return sp.with_hi(BytePos(sp.lo().0 + offset as u32)); } } From df412ce2083308cbe7c21ce2fc5622bfa8518993 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 1 Feb 2018 12:02:22 -0800 Subject: [PATCH 4/4] Change offset to `0` --- src/libsyntax/codemap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index fbd9088797e22..e980d6e67932a 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -598,7 +598,7 @@ impl CodeMap { /// If `sp` points to `"let mut x"`, then a span pointing at `"let "` will be returned. pub fn span_until_non_whitespace(&self, sp: Span) -> Span { if let Ok(snippet) = self.span_to_snippet(sp) { - let mut offset = 1; + let mut offset = 0; // get the bytes width of all the non-whitespace characters for c in snippet.chars().take_while(|c| !c.is_whitespace()) { offset += c.len_utf8();