From b12c20ccc30aebb2f5c90f04e2209ef12fe8b759 Mon Sep 17 00:00:00 2001 From: bravomikekilo Date: Wed, 31 Jul 2019 22:56:12 +0800 Subject: [PATCH 1/4] add a pair of whitespace after remove parentheses --- src/librustc_lint/unused.rs | 12 ++++++++++-- .../ui/lint/unused_parens_json_suggestion.stderr | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 4cccaa942b742..9ef34c201e19a 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -424,11 +424,19 @@ impl UnusedParens { }, _ => false, } - }).to_owned(); + }); + + let replace = { + let mut replace = String::from(" "); + replace.push_str(parens_removed); + replace.push(' '); + replace + }; + err.span_suggestion_short( span, "remove these parentheses", - parens_removed, + replace, Applicability::MachineApplicable, ); err.emit(); diff --git a/src/test/ui/lint/unused_parens_json_suggestion.stderr b/src/test/ui/lint/unused_parens_json_suggestion.stderr index 396395a17f732..54230b19cb480 100644 --- a/src/test/ui/lint/unused_parens_json_suggestion.stderr +++ b/src/test/ui/lint/unused_parens_json_suggestion.stderr @@ -81,7 +81,7 @@ } ], "label": null, - "suggested_replacement": "1 / (2 + 3)", + "suggested_replacement": " 1 / (2 + 3) ", "suggestion_applicability": "MachineApplicable", "expansion": null } From e64493e2d4f5e1e60c2599b0eea165a5bde0fdc6 Mon Sep 17 00:00:00 2001 From: bravomikekilo Date: Sat, 3 Aug 2019 01:46:09 +0800 Subject: [PATCH 2/4] add new test and add conditional whitespace --- src/librustc_lint/unused.rs | 95 ++- .../lint/unused_parens_json_suggestion.stderr | 2 +- .../used_parens_remove_json_suggestion.rs | 60 ++ .../used_parens_remove_json_suggestion.stderr | 666 ++++++++++++++++++ 4 files changed, 794 insertions(+), 29 deletions(-) create mode 100644 src/test/ui/lint/used_parens_remove_json_suggestion.rs create mode 100644 src/test/ui/lint/used_parens_remove_json_suggestion.stderr diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 9ef34c201e19a..5880e59350313 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -15,7 +15,7 @@ use syntax::print::pprust; use syntax::symbol::{kw, sym}; use syntax::symbol::Symbol; use syntax::util::parser; -use syntax_pos::Span; +use syntax_pos::{Span, BytePos}; use rustc::hir; @@ -353,31 +353,41 @@ declare_lint! { declare_lint_pass!(UnusedParens => [UNUSED_PARENS]); impl UnusedParens { + + fn is_expr_parens_necessary(inner: &ast::Expr, followed_by_block: bool) -> bool { + followed_by_block && match inner.node { + ast::ExprKind::Ret(_) | ast::ExprKind::Break(..) => true, + _ => parser::contains_exterior_struct_lit(&inner), + } + } + fn check_unused_parens_expr(&self, - cx: &EarlyContext<'_>, - value: &ast::Expr, - msg: &str, - followed_by_block: bool) { + cx: &EarlyContext<'_>, + value: &ast::Expr, + msg: &str, + followed_by_block: bool, + left_pos: Option, + right_pos: Option) { match value.node { ast::ExprKind::Paren(ref inner) => { - let necessary = followed_by_block && match inner.node { - ast::ExprKind::Ret(_) | ast::ExprKind::Break(..) => true, - _ => parser::contains_exterior_struct_lit(&inner), - }; - if !necessary { + if !Self::is_expr_parens_necessary(inner, followed_by_block) { let expr_text = if let Ok(snippet) = cx.sess().source_map() .span_to_snippet(value.span) { snippet } else { pprust::expr_to_string(value) }; - Self::remove_outer_parens(cx, value.span, &expr_text, msg); + let keep_space = ( + left_pos.map(|s| s >= value.span.lo()).unwrap_or(false), + right_pos.map(|s| s <= value.span.hi()).unwrap_or(false), + ); + Self::remove_outer_parens(cx, value.span, &expr_text, msg, keep_space); } } ast::ExprKind::Let(_, ref expr) => { // FIXME(#60336): Properly handle `let true = (false && true)` // actually needing the parenthesis. - self.check_unused_parens_expr(cx, expr, "`let` head expression", followed_by_block); + self.check_unused_parens_expr(cx, expr, "`let` head expression", followed_by_block, None, None); } _ => {} } @@ -394,11 +404,11 @@ impl UnusedParens { } else { pprust::pat_to_string(value) }; - Self::remove_outer_parens(cx, value.span, &pattern_text, msg); + Self::remove_outer_parens(cx, value.span, &pattern_text, msg, (false, false)); } } - fn remove_outer_parens(cx: &EarlyContext<'_>, span: Span, pattern: &str, msg: &str) { + fn remove_outer_parens(cx: &EarlyContext<'_>, span: Span, pattern: &str, msg: &str, keep_space: (bool, bool)) { let span_msg = format!("unnecessary parentheses around {}", msg); let mut err = cx.struct_span_lint(UNUSED_PARENS, span, &span_msg); let mut ate_left_paren = false; @@ -427,9 +437,17 @@ impl UnusedParens { }); let replace = { - let mut replace = String::from(" "); - replace.push_str(parens_removed); - replace.push(' '); + let mut replace = if keep_space.0 { + let mut s = String::from(" "); + s.push_str(parens_removed); + s + } else { + String::from(parens_removed) + }; + + if keep_space.1 { + replace.push(' '); + } replace }; @@ -446,14 +464,35 @@ impl UnusedParens { impl EarlyLintPass for UnusedParens { fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { use syntax::ast::ExprKind::*; - let (value, msg, followed_by_block) = match e.node { - If(ref cond, ..) => (cond, "`if` condition", true), - While(ref cond, ..) => (cond, "`while` condition", true), - ForLoop(_, ref cond, ..) => (cond, "`for` head expression", true), - Match(ref head, _) => (head, "`match` head expression", true), - Ret(Some(ref value)) => (value, "`return` value", false), - Assign(_, ref value) => (value, "assigned value", false), - AssignOp(.., ref value) => (value, "assigned value", false), + let (value, msg, followed_by_block, left_pos, right_pos) = match e.node { + If(ref cond, ref block, ..) => { + let left = e.span.lo() + syntax_pos::BytePos(2); + let right = block.span.lo(); + (cond, "`if` condition", true, Some(left), Some(right)) + } + + While(ref cond, ref block, ..) => { + let left = e.span.lo() + syntax_pos::BytePos(5); + let right = block.span.lo(); + (cond, "`while` condition", true, Some(left), Some(right)) + }, + + ForLoop(_, ref cond, ref block, ..) => { + (cond, "`for` head expression", true, None, Some(block.span.lo())) + } + + Match(ref head, _) => { + let left = e.span.lo() + syntax_pos::BytePos(5); + (head, "`match` head expression", true, Some(left), None) + } + + Ret(Some(ref value)) => { + let left = e.span.lo() + syntax_pos::BytePos(3); + (value, "`return` value", false, Some(left), None) + } + + Assign(_, ref value) => (value, "assigned value", false, None, None), + AssignOp(.., ref value) => (value, "assigned value", false, None, None), // either function/method call, or something this lint doesn't care about ref call_or_other => { let (args_to_check, call_kind) = match *call_or_other { @@ -475,12 +514,12 @@ impl EarlyLintPass for UnusedParens { } let msg = format!("{} argument", call_kind); for arg in args_to_check { - self.check_unused_parens_expr(cx, arg, &msg, false); + self.check_unused_parens_expr(cx, arg, &msg, false, None, None); } return; } }; - self.check_unused_parens_expr(cx, &value, msg, followed_by_block); + self.check_unused_parens_expr(cx, &value, msg, followed_by_block, left_pos, right_pos); } fn check_pat(&mut self, cx: &EarlyContext<'_>, p: &ast::Pat) { @@ -500,7 +539,7 @@ impl EarlyLintPass for UnusedParens { fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) { if let ast::StmtKind::Local(ref local) = s.node { if let Some(ref value) = local.init { - self.check_unused_parens_expr(cx, &value, "assigned value", false); + self.check_unused_parens_expr(cx, &value, "assigned value", false, None, None); } } } diff --git a/src/test/ui/lint/unused_parens_json_suggestion.stderr b/src/test/ui/lint/unused_parens_json_suggestion.stderr index 54230b19cb480..396395a17f732 100644 --- a/src/test/ui/lint/unused_parens_json_suggestion.stderr +++ b/src/test/ui/lint/unused_parens_json_suggestion.stderr @@ -81,7 +81,7 @@ } ], "label": null, - "suggested_replacement": " 1 / (2 + 3) ", + "suggested_replacement": "1 / (2 + 3)", "suggestion_applicability": "MachineApplicable", "expansion": null } diff --git a/src/test/ui/lint/used_parens_remove_json_suggestion.rs b/src/test/ui/lint/used_parens_remove_json_suggestion.rs new file mode 100644 index 0000000000000..3189606aaa97f --- /dev/null +++ b/src/test/ui/lint/used_parens_remove_json_suggestion.rs @@ -0,0 +1,60 @@ +// compile-flags: --error-format pretty-json -Zunstable-options +// build-pass + +// The output for humans should just highlight the whole span without showing +// the suggested replacement, but we also want to test that suggested +// replacement only removes one set of parentheses, rather than naïvely +// stripping away any starting or ending parenthesis characters—hence this +// test of the JSON error format. + +#![warn(unused_parens)] + +fn main() { + + let _b = false; + + if (_b) { + println!("hello"); + } + + f(); + +} + +fn f() -> bool { + let c = false; + + if(c) { + println!("next"); + } + + if (c){ + println!("prev"); + } + + while (false && true){ + if (c) { + println!("norm"); + } + + } + + while(true && false) { + for i in (0 .. 3){ + println!("e~") + } + } + + for i in (0 .. 3) { + while (true && false) { + println!("e~") + } + } + + + loop { + if (break { return true }) { + } + } + false +} \ No newline at end of file diff --git a/src/test/ui/lint/used_parens_remove_json_suggestion.stderr b/src/test/ui/lint/used_parens_remove_json_suggestion.stderr new file mode 100644 index 0000000000000..d8b23ae544318 --- /dev/null +++ b/src/test/ui/lint/used_parens_remove_json_suggestion.stderr @@ -0,0 +1,666 @@ +{ + "message": "unnecessary parentheses around `if` condition", + "code": { + "code": "unused_parens", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 478, + "byte_end": 482, + "line_start": 16, + "line_end": 16, + "column_start": 8, + "column_end": 12, + "is_primary": true, + "text": [ + { + "text": " if (_b) {", + "highlight_start": 8, + "highlight_end": 12 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "lint level defined here", + "code": null, + "level": "note", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 420, + "byte_end": 433, + "line_start": 10, + "line_end": 10, + "column_start": 9, + "column_end": 22, + "is_primary": true, + "text": [ + { + "text": "#![warn(unused_parens)]", + "highlight_start": 9, + "highlight_end": 22 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [], + "rendered": null + }, + { + "message": "remove these parentheses", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 478, + "byte_end": 482, + "line_start": 16, + "line_end": 16, + "column_start": 8, + "column_end": 12, + "is_primary": true, + "text": [ + { + "text": " if (_b) {", + "highlight_start": 8, + "highlight_end": 12 + } + ], + "label": null, + "suggested_replacement": "_b", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unnecessary parentheses around `if` condition + --> $DIR/used_parens_remove_json_suggestion.rs:16:8 + | +LL | if (_b) { + | ^^^^ help: remove these parentheses + | +note: lint level defined here + --> $DIR/used_parens_remove_json_suggestion.rs:10:9 + | +LL | #![warn(unused_parens)] + | ^^^^^^^^^^^^^ + +" +} +{ + "message": "unnecessary parentheses around `if` condition", + "code": { + "code": "unused_parens", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 575, + "byte_end": 578, + "line_start": 27, + "line_end": 27, + "column_start": 7, + "column_end": 10, + "is_primary": true, + "text": [ + { + "text": " if(c) {", + "highlight_start": 7, + "highlight_end": 10 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "remove these parentheses", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 575, + "byte_end": 578, + "line_start": 27, + "line_end": 27, + "column_start": 7, + "column_end": 10, + "is_primary": true, + "text": [ + { + "text": " if(c) {", + "highlight_start": 7, + "highlight_end": 10 + } + ], + "label": null, + "suggested_replacement": " c", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unnecessary parentheses around `if` condition + --> $DIR/used_parens_remove_json_suggestion.rs:27:7 + | +LL | if(c) { + | ^^^ help: remove these parentheses + +" +} +{ + "message": "unnecessary parentheses around `if` condition", + "code": { + "code": "unused_parens", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 621, + "byte_end": 624, + "line_start": 31, + "line_end": 31, + "column_start": 8, + "column_end": 11, + "is_primary": true, + "text": [ + { + "text": " if (c){", + "highlight_start": 8, + "highlight_end": 11 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "remove these parentheses", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 621, + "byte_end": 624, + "line_start": 31, + "line_end": 31, + "column_start": 8, + "column_end": 11, + "is_primary": true, + "text": [ + { + "text": " if (c){", + "highlight_start": 8, + "highlight_end": 11 + } + ], + "label": null, + "suggested_replacement": "c ", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unnecessary parentheses around `if` condition + --> $DIR/used_parens_remove_json_suggestion.rs:31:8 + | +LL | if (c){ + | ^^^ help: remove these parentheses + +" +} +{ + "message": "unnecessary parentheses around `while` condition", + "code": { + "code": "unused_parens", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 669, + "byte_end": 684, + "line_start": 35, + "line_end": 35, + "column_start": 11, + "column_end": 26, + "is_primary": true, + "text": [ + { + "text": " while (false && true){", + "highlight_start": 11, + "highlight_end": 26 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "remove these parentheses", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 669, + "byte_end": 684, + "line_start": 35, + "line_end": 35, + "column_start": 11, + "column_end": 26, + "is_primary": true, + "text": [ + { + "text": " while (false && true){", + "highlight_start": 11, + "highlight_end": 26 + } + ], + "label": null, + "suggested_replacement": "false && true ", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unnecessary parentheses around `while` condition + --> $DIR/used_parens_remove_json_suggestion.rs:35:11 + | +LL | while (false && true){ + | ^^^^^^^^^^^^^^^ help: remove these parentheses + +" +} +{ + "message": "unnecessary parentheses around `if` condition", + "code": { + "code": "unused_parens", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 697, + "byte_end": 700, + "line_start": 36, + "line_end": 36, + "column_start": 12, + "column_end": 15, + "is_primary": true, + "text": [ + { + "text": " if (c) {", + "highlight_start": 12, + "highlight_end": 15 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "remove these parentheses", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 697, + "byte_end": 700, + "line_start": 36, + "line_end": 36, + "column_start": 12, + "column_end": 15, + "is_primary": true, + "text": [ + { + "text": " if (c) {", + "highlight_start": 12, + "highlight_end": 15 + } + ], + "label": null, + "suggested_replacement": "c", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unnecessary parentheses around `if` condition + --> $DIR/used_parens_remove_json_suggestion.rs:36:12 + | +LL | if (c) { + | ^^^ help: remove these parentheses + +" +} +{ + "message": "unnecessary parentheses around `while` condition", + "code": { + "code": "unused_parens", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 760, + "byte_end": 775, + "line_start": 42, + "line_end": 42, + "column_start": 10, + "column_end": 25, + "is_primary": true, + "text": [ + { + "text": " while(true && false) {", + "highlight_start": 10, + "highlight_end": 25 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "remove these parentheses", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 760, + "byte_end": 775, + "line_start": 42, + "line_end": 42, + "column_start": 10, + "column_end": 25, + "is_primary": true, + "text": [ + { + "text": " while(true && false) {", + "highlight_start": 10, + "highlight_end": 25 + } + ], + "label": null, + "suggested_replacement": " true && false", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unnecessary parentheses around `while` condition + --> $DIR/used_parens_remove_json_suggestion.rs:42:10 + | +LL | while(true && false) { + | ^^^^^^^^^^^^^^^ help: remove these parentheses + +" +} +{ + "message": "unnecessary parentheses around `for` head expression", + "code": { + "code": "unused_parens", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 795, + "byte_end": 803, + "line_start": 43, + "line_end": 43, + "column_start": 18, + "column_end": 26, + "is_primary": true, + "text": [ + { + "text": " for i in (0 .. 3){", + "highlight_start": 18, + "highlight_end": 26 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "remove these parentheses", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 795, + "byte_end": 803, + "line_start": 43, + "line_end": 43, + "column_start": 18, + "column_end": 26, + "is_primary": true, + "text": [ + { + "text": " for i in (0 .. 3){", + "highlight_start": 18, + "highlight_end": 26 + } + ], + "label": null, + "suggested_replacement": "0 .. 3 ", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unnecessary parentheses around `for` head expression + --> $DIR/used_parens_remove_json_suggestion.rs:43:18 + | +LL | for i in (0 .. 3){ + | ^^^^^^^^ help: remove these parentheses + +" +} +{ + "message": "unnecessary parentheses around `for` head expression", + "code": { + "code": "unused_parens", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 862, + "byte_end": 870, + "line_start": 48, + "line_end": 48, + "column_start": 14, + "column_end": 22, + "is_primary": true, + "text": [ + { + "text": " for i in (0 .. 3) {", + "highlight_start": 14, + "highlight_end": 22 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "remove these parentheses", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 862, + "byte_end": 870, + "line_start": 48, + "line_end": 48, + "column_start": 14, + "column_end": 22, + "is_primary": true, + "text": [ + { + "text": " for i in (0 .. 3) {", + "highlight_start": 14, + "highlight_end": 22 + } + ], + "label": null, + "suggested_replacement": "0 .. 3", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unnecessary parentheses around `for` head expression + --> $DIR/used_parens_remove_json_suggestion.rs:48:14 + | +LL | for i in (0 .. 3) { + | ^^^^^^^^ help: remove these parentheses + +" +} +{ + "message": "unnecessary parentheses around `while` condition", + "code": { + "code": "unused_parens", + "explanation": null + }, + "level": "warning", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 887, + "byte_end": 902, + "line_start": 49, + "line_end": 49, + "column_start": 15, + "column_end": 30, + "is_primary": true, + "text": [ + { + "text": " while (true && false) {", + "highlight_start": 15, + "highlight_end": 30 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "remove these parentheses", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "$DIR/used_parens_remove_json_suggestion.rs", + "byte_start": 887, + "byte_end": 902, + "line_start": 49, + "line_end": 49, + "column_start": 15, + "column_end": 30, + "is_primary": true, + "text": [ + { + "text": " while (true && false) {", + "highlight_start": 15, + "highlight_end": 30 + } + ], + "label": null, + "suggested_replacement": "true && false", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unnecessary parentheses around `while` condition + --> $DIR/used_parens_remove_json_suggestion.rs:49:15 + | +LL | while (true && false) { + | ^^^^^^^^^^^^^^^ help: remove these parentheses + +" +} From 4c1d892960d92005bcd7c492f777ca45f1d55cea Mon Sep 17 00:00:00 2001 From: bravomikekilo Date: Sat, 3 Aug 2019 02:17:20 +0800 Subject: [PATCH 3/4] fix tidy problem --- src/librustc_lint/unused.rs | 13 +++++++++++-- .../ui/lint/used_parens_remove_json_suggestion.rs | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 5880e59350313..634b06d9bb239 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -387,7 +387,12 @@ impl UnusedParens { ast::ExprKind::Let(_, ref expr) => { // FIXME(#60336): Properly handle `let true = (false && true)` // actually needing the parenthesis. - self.check_unused_parens_expr(cx, expr, "`let` head expression", followed_by_block, None, None); + self.check_unused_parens_expr( + cx, expr, + "`let` head expression", + followed_by_block, + None, None + ); } _ => {} } @@ -408,7 +413,11 @@ impl UnusedParens { } } - fn remove_outer_parens(cx: &EarlyContext<'_>, span: Span, pattern: &str, msg: &str, keep_space: (bool, bool)) { + fn remove_outer_parens(cx: &EarlyContext<'_>, + span: Span, + pattern: &str, + msg: &str, + keep_space: (bool, bool)) { let span_msg = format!("unnecessary parentheses around {}", msg); let mut err = cx.struct_span_lint(UNUSED_PARENS, span, &span_msg); let mut ate_left_paren = false; diff --git a/src/test/ui/lint/used_parens_remove_json_suggestion.rs b/src/test/ui/lint/used_parens_remove_json_suggestion.rs index 3189606aaa97f..3af11428b95ee 100644 --- a/src/test/ui/lint/used_parens_remove_json_suggestion.rs +++ b/src/test/ui/lint/used_parens_remove_json_suggestion.rs @@ -57,4 +57,4 @@ fn f() -> bool { } } false -} \ No newline at end of file +} From 3a95c716dcd0a846ae09725f4bdfab519c5743b8 Mon Sep 17 00:00:00 2001 From: bravomikekilo Date: Tue, 6 Aug 2019 16:00:13 +0800 Subject: [PATCH 4/4] Add rustfix test and fix test name. --- .../lint/unused_parens_json_suggestion.fixed | 27 +++ .../ui/lint/unused_parens_json_suggestion.rs | 2 + .../lint/unused_parens_json_suggestion.stderr | 28 +-- ...unused_parens_remove_json_suggestion.fixed | 62 +++++ ...> unused_parens_remove_json_suggestion.rs} | 6 +- ...used_parens_remove_json_suggestion.stderr} | 222 +++++++++--------- 6 files changed, 220 insertions(+), 127 deletions(-) create mode 100644 src/test/ui/lint/unused_parens_json_suggestion.fixed create mode 100644 src/test/ui/lint/unused_parens_remove_json_suggestion.fixed rename src/test/ui/lint/{used_parens_remove_json_suggestion.rs => unused_parens_remove_json_suggestion.rs} (91%) rename src/test/ui/lint/{used_parens_remove_json_suggestion.stderr => unused_parens_remove_json_suggestion.stderr} (75%) diff --git a/src/test/ui/lint/unused_parens_json_suggestion.fixed b/src/test/ui/lint/unused_parens_json_suggestion.fixed new file mode 100644 index 0000000000000..427407119102c --- /dev/null +++ b/src/test/ui/lint/unused_parens_json_suggestion.fixed @@ -0,0 +1,27 @@ +// compile-flags: --error-format pretty-json -Zunstable-options +// build-pass (FIXME(62277): could be check-pass?) +// run-rustfix + +// The output for humans should just highlight the whole span without showing +// the suggested replacement, but we also want to test that suggested +// replacement only removes one set of parentheses, rather than naïvely +// stripping away any starting or ending parenthesis characters—hence this +// test of the JSON error format. + +#![warn(unused_parens)] +#![allow(unreachable_code)] + +fn main() { + // We want to suggest the properly-balanced expression `1 / (2 + 3)`, not + // the malformed `1 / (2 + 3` + let _a = 1 / (2 + 3); + f(); +} + +fn f() -> bool { + loop { + if (break { return true }) { + } + } + false +} diff --git a/src/test/ui/lint/unused_parens_json_suggestion.rs b/src/test/ui/lint/unused_parens_json_suggestion.rs index 185bfacea8046..87192503986c4 100644 --- a/src/test/ui/lint/unused_parens_json_suggestion.rs +++ b/src/test/ui/lint/unused_parens_json_suggestion.rs @@ -1,5 +1,6 @@ // compile-flags: --error-format pretty-json -Zunstable-options // build-pass (FIXME(62277): could be check-pass?) +// run-rustfix // The output for humans should just highlight the whole span without showing // the suggested replacement, but we also want to test that suggested @@ -8,6 +9,7 @@ // test of the JSON error format. #![warn(unused_parens)] +#![allow(unreachable_code)] fn main() { // We want to suggest the properly-balanced expression `1 / (2 + 3)`, not diff --git a/src/test/ui/lint/unused_parens_json_suggestion.stderr b/src/test/ui/lint/unused_parens_json_suggestion.stderr index 396395a17f732..256c7555c908b 100644 --- a/src/test/ui/lint/unused_parens_json_suggestion.stderr +++ b/src/test/ui/lint/unused_parens_json_suggestion.stderr @@ -8,10 +8,10 @@ "spans": [ { "file_name": "$DIR/unused_parens_json_suggestion.rs", - "byte_start": 611, - "byte_end": 624, - "line_start": 15, - "line_end": 15, + "byte_start": 654, + "byte_end": 667, + "line_start": 17, + "line_end": 17, "column_start": 14, "column_end": 27, "is_primary": true, @@ -36,10 +36,10 @@ "spans": [ { "file_name": "$DIR/unused_parens_json_suggestion.rs", - "byte_start": 457, - "byte_end": 470, - "line_start": 10, - "line_end": 10, + "byte_start": 472, + "byte_end": 485, + "line_start": 11, + "line_end": 11, "column_start": 9, "column_end": 22, "is_primary": true, @@ -66,10 +66,10 @@ "spans": [ { "file_name": "$DIR/unused_parens_json_suggestion.rs", - "byte_start": 611, - "byte_end": 624, - "line_start": 15, - "line_end": 15, + "byte_start": 654, + "byte_end": 667, + "line_start": 17, + "line_end": 17, "column_start": 14, "column_end": 27, "is_primary": true, @@ -91,13 +91,13 @@ } ], "rendered": "warning: unnecessary parentheses around assigned value - --> $DIR/unused_parens_json_suggestion.rs:15:14 + --> $DIR/unused_parens_json_suggestion.rs:17:14 | LL | let _a = (1 / (2 + 3)); | ^^^^^^^^^^^^^ help: remove these parentheses | note: lint level defined here - --> $DIR/unused_parens_json_suggestion.rs:10:9 + --> $DIR/unused_parens_json_suggestion.rs:11:9 | LL | #![warn(unused_parens)] | ^^^^^^^^^^^^^ diff --git a/src/test/ui/lint/unused_parens_remove_json_suggestion.fixed b/src/test/ui/lint/unused_parens_remove_json_suggestion.fixed new file mode 100644 index 0000000000000..2459eb1ac5cb8 --- /dev/null +++ b/src/test/ui/lint/unused_parens_remove_json_suggestion.fixed @@ -0,0 +1,62 @@ +// compile-flags: --error-format pretty-json -Zunstable-options +// build-pass +// run-rustfix + +// The output for humans should just highlight the whole span without showing +// the suggested replacement, but we also want to test that suggested +// replacement only removes one set of parentheses, rather than naïvely +// stripping away any starting or ending parenthesis characters—hence this +// test of the JSON error format. + +#![warn(unused_parens)] +#![allow(unreachable_code)] + +fn main() { + + let _b = false; + + if _b { + println!("hello"); + } + + f(); + +} + +fn f() -> bool { + let c = false; + + if c { + println!("next"); + } + + if c { + println!("prev"); + } + + while false && true { + if c { + println!("norm"); + } + + } + + while true && false { + for _ in 0 .. 3 { + println!("e~") + } + } + + for _ in 0 .. 3 { + while true && false { + println!("e~") + } + } + + + loop { + if (break { return true }) { + } + } + false +} diff --git a/src/test/ui/lint/used_parens_remove_json_suggestion.rs b/src/test/ui/lint/unused_parens_remove_json_suggestion.rs similarity index 91% rename from src/test/ui/lint/used_parens_remove_json_suggestion.rs rename to src/test/ui/lint/unused_parens_remove_json_suggestion.rs index 3af11428b95ee..0e9869b67d590 100644 --- a/src/test/ui/lint/used_parens_remove_json_suggestion.rs +++ b/src/test/ui/lint/unused_parens_remove_json_suggestion.rs @@ -1,5 +1,6 @@ // compile-flags: --error-format pretty-json -Zunstable-options // build-pass +// run-rustfix // The output for humans should just highlight the whole span without showing // the suggested replacement, but we also want to test that suggested @@ -8,6 +9,7 @@ // test of the JSON error format. #![warn(unused_parens)] +#![allow(unreachable_code)] fn main() { @@ -40,12 +42,12 @@ fn f() -> bool { } while(true && false) { - for i in (0 .. 3){ + for _ in (0 .. 3){ println!("e~") } } - for i in (0 .. 3) { + for _ in (0 .. 3) { while (true && false) { println!("e~") } diff --git a/src/test/ui/lint/used_parens_remove_json_suggestion.stderr b/src/test/ui/lint/unused_parens_remove_json_suggestion.stderr similarity index 75% rename from src/test/ui/lint/used_parens_remove_json_suggestion.stderr rename to src/test/ui/lint/unused_parens_remove_json_suggestion.stderr index d8b23ae544318..b4eab200dd016 100644 --- a/src/test/ui/lint/used_parens_remove_json_suggestion.stderr +++ b/src/test/ui/lint/unused_parens_remove_json_suggestion.stderr @@ -7,11 +7,11 @@ "level": "warning", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 478, - "byte_end": 482, - "line_start": 16, - "line_end": 16, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 521, + "byte_end": 525, + "line_start": 18, + "line_end": 18, "column_start": 8, "column_end": 12, "is_primary": true, @@ -35,11 +35,11 @@ "level": "note", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 420, - "byte_end": 433, - "line_start": 10, - "line_end": 10, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 435, + "byte_end": 448, + "line_start": 11, + "line_end": 11, "column_start": 9, "column_end": 22, "is_primary": true, @@ -65,11 +65,11 @@ "level": "help", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 478, - "byte_end": 482, - "line_start": 16, - "line_end": 16, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 521, + "byte_end": 525, + "line_start": 18, + "line_end": 18, "column_start": 8, "column_end": 12, "is_primary": true, @@ -91,13 +91,13 @@ } ], "rendered": "warning: unnecessary parentheses around `if` condition - --> $DIR/used_parens_remove_json_suggestion.rs:16:8 + --> $DIR/unused_parens_remove_json_suggestion.rs:18:8 | LL | if (_b) { | ^^^^ help: remove these parentheses | note: lint level defined here - --> $DIR/used_parens_remove_json_suggestion.rs:10:9 + --> $DIR/unused_parens_remove_json_suggestion.rs:11:9 | LL | #![warn(unused_parens)] | ^^^^^^^^^^^^^ @@ -113,11 +113,11 @@ LL | #![warn(unused_parens)] "level": "warning", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 575, - "byte_end": 578, - "line_start": 27, - "line_end": 27, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 618, + "byte_end": 621, + "line_start": 29, + "line_end": 29, "column_start": 7, "column_end": 10, "is_primary": true, @@ -141,11 +141,11 @@ LL | #![warn(unused_parens)] "level": "help", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 575, - "byte_end": 578, - "line_start": 27, - "line_end": 27, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 618, + "byte_end": 621, + "line_start": 29, + "line_end": 29, "column_start": 7, "column_end": 10, "is_primary": true, @@ -167,7 +167,7 @@ LL | #![warn(unused_parens)] } ], "rendered": "warning: unnecessary parentheses around `if` condition - --> $DIR/used_parens_remove_json_suggestion.rs:27:7 + --> $DIR/unused_parens_remove_json_suggestion.rs:29:7 | LL | if(c) { | ^^^ help: remove these parentheses @@ -183,11 +183,11 @@ LL | if(c) { "level": "warning", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 621, - "byte_end": 624, - "line_start": 31, - "line_end": 31, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 664, + "byte_end": 667, + "line_start": 33, + "line_end": 33, "column_start": 8, "column_end": 11, "is_primary": true, @@ -211,11 +211,11 @@ LL | if(c) { "level": "help", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 621, - "byte_end": 624, - "line_start": 31, - "line_end": 31, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 664, + "byte_end": 667, + "line_start": 33, + "line_end": 33, "column_start": 8, "column_end": 11, "is_primary": true, @@ -237,7 +237,7 @@ LL | if(c) { } ], "rendered": "warning: unnecessary parentheses around `if` condition - --> $DIR/used_parens_remove_json_suggestion.rs:31:8 + --> $DIR/unused_parens_remove_json_suggestion.rs:33:8 | LL | if (c){ | ^^^ help: remove these parentheses @@ -253,11 +253,11 @@ LL | if (c){ "level": "warning", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 669, - "byte_end": 684, - "line_start": 35, - "line_end": 35, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 712, + "byte_end": 727, + "line_start": 37, + "line_end": 37, "column_start": 11, "column_end": 26, "is_primary": true, @@ -281,11 +281,11 @@ LL | if (c){ "level": "help", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 669, - "byte_end": 684, - "line_start": 35, - "line_end": 35, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 712, + "byte_end": 727, + "line_start": 37, + "line_end": 37, "column_start": 11, "column_end": 26, "is_primary": true, @@ -307,7 +307,7 @@ LL | if (c){ } ], "rendered": "warning: unnecessary parentheses around `while` condition - --> $DIR/used_parens_remove_json_suggestion.rs:35:11 + --> $DIR/unused_parens_remove_json_suggestion.rs:37:11 | LL | while (false && true){ | ^^^^^^^^^^^^^^^ help: remove these parentheses @@ -323,11 +323,11 @@ LL | while (false && true){ "level": "warning", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 697, - "byte_end": 700, - "line_start": 36, - "line_end": 36, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 740, + "byte_end": 743, + "line_start": 38, + "line_end": 38, "column_start": 12, "column_end": 15, "is_primary": true, @@ -351,11 +351,11 @@ LL | while (false && true){ "level": "help", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 697, - "byte_end": 700, - "line_start": 36, - "line_end": 36, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 740, + "byte_end": 743, + "line_start": 38, + "line_end": 38, "column_start": 12, "column_end": 15, "is_primary": true, @@ -377,7 +377,7 @@ LL | while (false && true){ } ], "rendered": "warning: unnecessary parentheses around `if` condition - --> $DIR/used_parens_remove_json_suggestion.rs:36:12 + --> $DIR/unused_parens_remove_json_suggestion.rs:38:12 | LL | if (c) { | ^^^ help: remove these parentheses @@ -393,11 +393,11 @@ LL | if (c) { "level": "warning", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 760, - "byte_end": 775, - "line_start": 42, - "line_end": 42, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 803, + "byte_end": 818, + "line_start": 44, + "line_end": 44, "column_start": 10, "column_end": 25, "is_primary": true, @@ -421,11 +421,11 @@ LL | if (c) { "level": "help", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 760, - "byte_end": 775, - "line_start": 42, - "line_end": 42, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 803, + "byte_end": 818, + "line_start": 44, + "line_end": 44, "column_start": 10, "column_end": 25, "is_primary": true, @@ -447,7 +447,7 @@ LL | if (c) { } ], "rendered": "warning: unnecessary parentheses around `while` condition - --> $DIR/used_parens_remove_json_suggestion.rs:42:10 + --> $DIR/unused_parens_remove_json_suggestion.rs:44:10 | LL | while(true && false) { | ^^^^^^^^^^^^^^^ help: remove these parentheses @@ -463,17 +463,17 @@ LL | while(true && false) { "level": "warning", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 795, - "byte_end": 803, - "line_start": 43, - "line_end": 43, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 838, + "byte_end": 846, + "line_start": 45, + "line_end": 45, "column_start": 18, "column_end": 26, "is_primary": true, "text": [ { - "text": " for i in (0 .. 3){", + "text": " for _ in (0 .. 3){", "highlight_start": 18, "highlight_end": 26 } @@ -491,17 +491,17 @@ LL | while(true && false) { "level": "help", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 795, - "byte_end": 803, - "line_start": 43, - "line_end": 43, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 838, + "byte_end": 846, + "line_start": 45, + "line_end": 45, "column_start": 18, "column_end": 26, "is_primary": true, "text": [ { - "text": " for i in (0 .. 3){", + "text": " for _ in (0 .. 3){", "highlight_start": 18, "highlight_end": 26 } @@ -517,9 +517,9 @@ LL | while(true && false) { } ], "rendered": "warning: unnecessary parentheses around `for` head expression - --> $DIR/used_parens_remove_json_suggestion.rs:43:18 + --> $DIR/unused_parens_remove_json_suggestion.rs:45:18 | -LL | for i in (0 .. 3){ +LL | for _ in (0 .. 3){ | ^^^^^^^^ help: remove these parentheses " @@ -533,17 +533,17 @@ LL | for i in (0 .. 3){ "level": "warning", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 862, - "byte_end": 870, - "line_start": 48, - "line_end": 48, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 905, + "byte_end": 913, + "line_start": 50, + "line_end": 50, "column_start": 14, "column_end": 22, "is_primary": true, "text": [ { - "text": " for i in (0 .. 3) {", + "text": " for _ in (0 .. 3) {", "highlight_start": 14, "highlight_end": 22 } @@ -561,17 +561,17 @@ LL | for i in (0 .. 3){ "level": "help", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 862, - "byte_end": 870, - "line_start": 48, - "line_end": 48, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 905, + "byte_end": 913, + "line_start": 50, + "line_end": 50, "column_start": 14, "column_end": 22, "is_primary": true, "text": [ { - "text": " for i in (0 .. 3) {", + "text": " for _ in (0 .. 3) {", "highlight_start": 14, "highlight_end": 22 } @@ -587,9 +587,9 @@ LL | for i in (0 .. 3){ } ], "rendered": "warning: unnecessary parentheses around `for` head expression - --> $DIR/used_parens_remove_json_suggestion.rs:48:14 + --> $DIR/unused_parens_remove_json_suggestion.rs:50:14 | -LL | for i in (0 .. 3) { +LL | for _ in (0 .. 3) { | ^^^^^^^^ help: remove these parentheses " @@ -603,11 +603,11 @@ LL | for i in (0 .. 3) { "level": "warning", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 887, - "byte_end": 902, - "line_start": 49, - "line_end": 49, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 930, + "byte_end": 945, + "line_start": 51, + "line_end": 51, "column_start": 15, "column_end": 30, "is_primary": true, @@ -631,11 +631,11 @@ LL | for i in (0 .. 3) { "level": "help", "spans": [ { - "file_name": "$DIR/used_parens_remove_json_suggestion.rs", - "byte_start": 887, - "byte_end": 902, - "line_start": 49, - "line_end": 49, + "file_name": "$DIR/unused_parens_remove_json_suggestion.rs", + "byte_start": 930, + "byte_end": 945, + "line_start": 51, + "line_end": 51, "column_start": 15, "column_end": 30, "is_primary": true, @@ -657,7 +657,7 @@ LL | for i in (0 .. 3) { } ], "rendered": "warning: unnecessary parentheses around `while` condition - --> $DIR/used_parens_remove_json_suggestion.rs:49:15 + --> $DIR/unused_parens_remove_json_suggestion.rs:51:15 | LL | while (true && false) { | ^^^^^^^^^^^^^^^ help: remove these parentheses