diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 9ef34c201e1..5880e593503 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 54230b19cb4..396395a17f7 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 00000000000..3189606aaa9 --- /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 00000000000..d8b23ae5443 --- /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 + +" +}