Skip to content

Commit fb79a74

Browse files
authored
Rollup merge of rust-lang#63163 - bravomikekilo:master, r=cramertj
add a pair of whitespace after remove parentheses fix [issue-63156](rust-lang#63156). add a pair of whitespace after remove parentheses.
2 parents fb1f57e + 3a95c71 commit fb79a74

7 files changed

+916
-41
lines changed

src/librustc_lint/unused.rs

+83-27
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use syntax::print::pprust;
1515
use syntax::symbol::{kw, sym};
1616
use syntax::symbol::Symbol;
1717
use syntax::util::parser;
18-
use syntax_pos::Span;
18+
use syntax_pos::{Span, BytePos};
1919

2020
use rustc::hir;
2121

@@ -353,31 +353,46 @@ declare_lint! {
353353
declare_lint_pass!(UnusedParens => [UNUSED_PARENS]);
354354

355355
impl UnusedParens {
356+
357+
fn is_expr_parens_necessary(inner: &ast::Expr, followed_by_block: bool) -> bool {
358+
followed_by_block && match inner.node {
359+
ast::ExprKind::Ret(_) | ast::ExprKind::Break(..) => true,
360+
_ => parser::contains_exterior_struct_lit(&inner),
361+
}
362+
}
363+
356364
fn check_unused_parens_expr(&self,
357-
cx: &EarlyContext<'_>,
358-
value: &ast::Expr,
359-
msg: &str,
360-
followed_by_block: bool) {
365+
cx: &EarlyContext<'_>,
366+
value: &ast::Expr,
367+
msg: &str,
368+
followed_by_block: bool,
369+
left_pos: Option<BytePos>,
370+
right_pos: Option<BytePos>) {
361371
match value.node {
362372
ast::ExprKind::Paren(ref inner) => {
363-
let necessary = followed_by_block && match inner.node {
364-
ast::ExprKind::Ret(_) | ast::ExprKind::Break(..) => true,
365-
_ => parser::contains_exterior_struct_lit(&inner),
366-
};
367-
if !necessary {
373+
if !Self::is_expr_parens_necessary(inner, followed_by_block) {
368374
let expr_text = if let Ok(snippet) = cx.sess().source_map()
369375
.span_to_snippet(value.span) {
370376
snippet
371377
} else {
372378
pprust::expr_to_string(value)
373379
};
374-
Self::remove_outer_parens(cx, value.span, &expr_text, msg);
380+
let keep_space = (
381+
left_pos.map(|s| s >= value.span.lo()).unwrap_or(false),
382+
right_pos.map(|s| s <= value.span.hi()).unwrap_or(false),
383+
);
384+
Self::remove_outer_parens(cx, value.span, &expr_text, msg, keep_space);
375385
}
376386
}
377387
ast::ExprKind::Let(_, ref expr) => {
378388
// FIXME(#60336): Properly handle `let true = (false && true)`
379389
// actually needing the parenthesis.
380-
self.check_unused_parens_expr(cx, expr, "`let` head expression", followed_by_block);
390+
self.check_unused_parens_expr(
391+
cx, expr,
392+
"`let` head expression",
393+
followed_by_block,
394+
None, None
395+
);
381396
}
382397
_ => {}
383398
}
@@ -394,11 +409,15 @@ impl UnusedParens {
394409
} else {
395410
pprust::pat_to_string(value)
396411
};
397-
Self::remove_outer_parens(cx, value.span, &pattern_text, msg);
412+
Self::remove_outer_parens(cx, value.span, &pattern_text, msg, (false, false));
398413
}
399414
}
400415

401-
fn remove_outer_parens(cx: &EarlyContext<'_>, span: Span, pattern: &str, msg: &str) {
416+
fn remove_outer_parens(cx: &EarlyContext<'_>,
417+
span: Span,
418+
pattern: &str,
419+
msg: &str,
420+
keep_space: (bool, bool)) {
402421
let span_msg = format!("unnecessary parentheses around {}", msg);
403422
let mut err = cx.struct_span_lint(UNUSED_PARENS, span, &span_msg);
404423
let mut ate_left_paren = false;
@@ -424,11 +443,27 @@ impl UnusedParens {
424443
},
425444
_ => false,
426445
}
427-
}).to_owned();
446+
});
447+
448+
let replace = {
449+
let mut replace = if keep_space.0 {
450+
let mut s = String::from(" ");
451+
s.push_str(parens_removed);
452+
s
453+
} else {
454+
String::from(parens_removed)
455+
};
456+
457+
if keep_space.1 {
458+
replace.push(' ');
459+
}
460+
replace
461+
};
462+
428463
err.span_suggestion_short(
429464
span,
430465
"remove these parentheses",
431-
parens_removed,
466+
replace,
432467
Applicability::MachineApplicable,
433468
);
434469
err.emit();
@@ -438,14 +473,35 @@ impl UnusedParens {
438473
impl EarlyLintPass for UnusedParens {
439474
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
440475
use syntax::ast::ExprKind::*;
441-
let (value, msg, followed_by_block) = match e.node {
442-
If(ref cond, ..) => (cond, "`if` condition", true),
443-
While(ref cond, ..) => (cond, "`while` condition", true),
444-
ForLoop(_, ref cond, ..) => (cond, "`for` head expression", true),
445-
Match(ref head, _) => (head, "`match` head expression", true),
446-
Ret(Some(ref value)) => (value, "`return` value", false),
447-
Assign(_, ref value) => (value, "assigned value", false),
448-
AssignOp(.., ref value) => (value, "assigned value", false),
476+
let (value, msg, followed_by_block, left_pos, right_pos) = match e.node {
477+
If(ref cond, ref block, ..) => {
478+
let left = e.span.lo() + syntax_pos::BytePos(2);
479+
let right = block.span.lo();
480+
(cond, "`if` condition", true, Some(left), Some(right))
481+
}
482+
483+
While(ref cond, ref block, ..) => {
484+
let left = e.span.lo() + syntax_pos::BytePos(5);
485+
let right = block.span.lo();
486+
(cond, "`while` condition", true, Some(left), Some(right))
487+
},
488+
489+
ForLoop(_, ref cond, ref block, ..) => {
490+
(cond, "`for` head expression", true, None, Some(block.span.lo()))
491+
}
492+
493+
Match(ref head, _) => {
494+
let left = e.span.lo() + syntax_pos::BytePos(5);
495+
(head, "`match` head expression", true, Some(left), None)
496+
}
497+
498+
Ret(Some(ref value)) => {
499+
let left = e.span.lo() + syntax_pos::BytePos(3);
500+
(value, "`return` value", false, Some(left), None)
501+
}
502+
503+
Assign(_, ref value) => (value, "assigned value", false, None, None),
504+
AssignOp(.., ref value) => (value, "assigned value", false, None, None),
449505
// either function/method call, or something this lint doesn't care about
450506
ref call_or_other => {
451507
let (args_to_check, call_kind) = match *call_or_other {
@@ -467,12 +523,12 @@ impl EarlyLintPass for UnusedParens {
467523
}
468524
let msg = format!("{} argument", call_kind);
469525
for arg in args_to_check {
470-
self.check_unused_parens_expr(cx, arg, &msg, false);
526+
self.check_unused_parens_expr(cx, arg, &msg, false, None, None);
471527
}
472528
return;
473529
}
474530
};
475-
self.check_unused_parens_expr(cx, &value, msg, followed_by_block);
531+
self.check_unused_parens_expr(cx, &value, msg, followed_by_block, left_pos, right_pos);
476532
}
477533

478534
fn check_pat(&mut self, cx: &EarlyContext<'_>, p: &ast::Pat) {
@@ -492,7 +548,7 @@ impl EarlyLintPass for UnusedParens {
492548
fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) {
493549
if let ast::StmtKind::Local(ref local) = s.node {
494550
if let Some(ref value) = local.init {
495-
self.check_unused_parens_expr(cx, &value, "assigned value", false);
551+
self.check_unused_parens_expr(cx, &value, "assigned value", false, None, None);
496552
}
497553
}
498554
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// compile-flags: --error-format pretty-json -Zunstable-options
2+
// build-pass (FIXME(62277): could be check-pass?)
3+
// run-rustfix
4+
5+
// The output for humans should just highlight the whole span without showing
6+
// the suggested replacement, but we also want to test that suggested
7+
// replacement only removes one set of parentheses, rather than naïvely
8+
// stripping away any starting or ending parenthesis characters—hence this
9+
// test of the JSON error format.
10+
11+
#![warn(unused_parens)]
12+
#![allow(unreachable_code)]
13+
14+
fn main() {
15+
// We want to suggest the properly-balanced expression `1 / (2 + 3)`, not
16+
// the malformed `1 / (2 + 3`
17+
let _a = 1 / (2 + 3);
18+
f();
19+
}
20+
21+
fn f() -> bool {
22+
loop {
23+
if (break { return true }) {
24+
}
25+
}
26+
false
27+
}

src/test/ui/lint/unused_parens_json_suggestion.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// compile-flags: --error-format pretty-json -Zunstable-options
22
// build-pass (FIXME(62277): could be check-pass?)
3+
// run-rustfix
34

45
// The output for humans should just highlight the whole span without showing
56
// the suggested replacement, but we also want to test that suggested
@@ -8,6 +9,7 @@
89
// test of the JSON error format.
910

1011
#![warn(unused_parens)]
12+
#![allow(unreachable_code)]
1113

1214
fn main() {
1315
// We want to suggest the properly-balanced expression `1 / (2 + 3)`, not

src/test/ui/lint/unused_parens_json_suggestion.stderr

+14-14
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
"spans": [
99
{
1010
"file_name": "$DIR/unused_parens_json_suggestion.rs",
11-
"byte_start": 611,
12-
"byte_end": 624,
13-
"line_start": 15,
14-
"line_end": 15,
11+
"byte_start": 654,
12+
"byte_end": 667,
13+
"line_start": 17,
14+
"line_end": 17,
1515
"column_start": 14,
1616
"column_end": 27,
1717
"is_primary": true,
@@ -36,10 +36,10 @@
3636
"spans": [
3737
{
3838
"file_name": "$DIR/unused_parens_json_suggestion.rs",
39-
"byte_start": 457,
40-
"byte_end": 470,
41-
"line_start": 10,
42-
"line_end": 10,
39+
"byte_start": 472,
40+
"byte_end": 485,
41+
"line_start": 11,
42+
"line_end": 11,
4343
"column_start": 9,
4444
"column_end": 22,
4545
"is_primary": true,
@@ -66,10 +66,10 @@
6666
"spans": [
6767
{
6868
"file_name": "$DIR/unused_parens_json_suggestion.rs",
69-
"byte_start": 611,
70-
"byte_end": 624,
71-
"line_start": 15,
72-
"line_end": 15,
69+
"byte_start": 654,
70+
"byte_end": 667,
71+
"line_start": 17,
72+
"line_end": 17,
7373
"column_start": 14,
7474
"column_end": 27,
7575
"is_primary": true,
@@ -91,13 +91,13 @@
9191
}
9292
],
9393
"rendered": "warning: unnecessary parentheses around assigned value
94-
--> $DIR/unused_parens_json_suggestion.rs:15:14
94+
--> $DIR/unused_parens_json_suggestion.rs:17:14
9595
|
9696
LL | let _a = (1 / (2 + 3));
9797
| ^^^^^^^^^^^^^ help: remove these parentheses
9898
|
9999
note: lint level defined here
100-
--> $DIR/unused_parens_json_suggestion.rs:10:9
100+
--> $DIR/unused_parens_json_suggestion.rs:11:9
101101
|
102102
LL | #![warn(unused_parens)]
103103
| ^^^^^^^^^^^^^
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// compile-flags: --error-format pretty-json -Zunstable-options
2+
// build-pass
3+
// run-rustfix
4+
5+
// The output for humans should just highlight the whole span without showing
6+
// the suggested replacement, but we also want to test that suggested
7+
// replacement only removes one set of parentheses, rather than naïvely
8+
// stripping away any starting or ending parenthesis characters—hence this
9+
// test of the JSON error format.
10+
11+
#![warn(unused_parens)]
12+
#![allow(unreachable_code)]
13+
14+
fn main() {
15+
16+
let _b = false;
17+
18+
if _b {
19+
println!("hello");
20+
}
21+
22+
f();
23+
24+
}
25+
26+
fn f() -> bool {
27+
let c = false;
28+
29+
if c {
30+
println!("next");
31+
}
32+
33+
if c {
34+
println!("prev");
35+
}
36+
37+
while false && true {
38+
if c {
39+
println!("norm");
40+
}
41+
42+
}
43+
44+
while true && false {
45+
for _ in 0 .. 3 {
46+
println!("e~")
47+
}
48+
}
49+
50+
for _ in 0 .. 3 {
51+
while true && false {
52+
println!("e~")
53+
}
54+
}
55+
56+
57+
loop {
58+
if (break { return true }) {
59+
}
60+
}
61+
false
62+
}

0 commit comments

Comments
 (0)