Rollup merge of #112316 - clubby789:unused-parens-space, r=compiler-errors

Ensure space is inserted after keyword in `unused_delims`

Fixes #112276
This commit is contained in:
Matthias Krüger 2023-06-05 23:47:59 +02:00 committed by GitHub
commit 0ff5a6ee57
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 131 additions and 30 deletions

View File

@ -556,6 +556,7 @@ trait UnusedDelimLint {
followed_by_block: bool, followed_by_block: bool,
left_pos: Option<BytePos>, left_pos: Option<BytePos>,
right_pos: Option<BytePos>, right_pos: Option<BytePos>,
is_kw: bool,
); );
fn is_expr_delims_necessary( fn is_expr_delims_necessary(
@ -624,6 +625,7 @@ trait UnusedDelimLint {
ctx: UnusedDelimsCtx, ctx: UnusedDelimsCtx,
left_pos: Option<BytePos>, left_pos: Option<BytePos>,
right_pos: Option<BytePos>, right_pos: Option<BytePos>,
is_kw: bool,
) { ) {
// If `value` has `ExprKind::Err`, unused delim lint can be broken. // If `value` has `ExprKind::Err`, unused delim lint can be broken.
// For example, the following code caused ICE. // For example, the following code caused ICE.
@ -667,7 +669,7 @@ trait UnusedDelimLint {
left_pos.is_some_and(|s| s >= value.span.lo()), left_pos.is_some_and(|s| s >= value.span.lo()),
right_pos.is_some_and(|s| s <= value.span.hi()), right_pos.is_some_and(|s| s <= value.span.hi()),
); );
self.emit_unused_delims(cx, value.span, spans, ctx.into(), keep_space); self.emit_unused_delims(cx, value.span, spans, ctx.into(), keep_space, is_kw);
} }
fn emit_unused_delims( fn emit_unused_delims(
@ -677,6 +679,7 @@ trait UnusedDelimLint {
spans: Option<(Span, Span)>, spans: Option<(Span, Span)>,
msg: &str, msg: &str,
keep_space: (bool, bool), keep_space: (bool, bool),
is_kw: bool,
) { ) {
let primary_span = if let Some((lo, hi)) = spans { let primary_span = if let Some((lo, hi)) = spans {
if hi.is_empty() { if hi.is_empty() {
@ -690,7 +693,7 @@ trait UnusedDelimLint {
let suggestion = spans.map(|(lo, hi)| { let suggestion = spans.map(|(lo, hi)| {
let sm = cx.sess().source_map(); let sm = cx.sess().source_map();
let lo_replace = let lo_replace =
if keep_space.0 && if (keep_space.0 || is_kw) &&
let Ok(snip) = sm.span_to_prev_source(lo) && !snip.ends_with(' ') { let Ok(snip) = sm.span_to_prev_source(lo) && !snip.ends_with(' ') {
" " " "
} else { } else {
@ -720,7 +723,7 @@ trait UnusedDelimLint {
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
use rustc_ast::ExprKind::*; use rustc_ast::ExprKind::*;
let (value, ctx, followed_by_block, left_pos, right_pos) = match e.kind { let (value, ctx, followed_by_block, left_pos, right_pos, is_kw) = match e.kind {
// Do not lint `unused_braces` in `if let` expressions. // Do not lint `unused_braces` in `if let` expressions.
If(ref cond, ref block, _) If(ref cond, ref block, _)
if !matches!(cond.kind, Let(_, _, _)) if !matches!(cond.kind, Let(_, _, _))
@ -728,7 +731,7 @@ trait UnusedDelimLint {
{ {
let left = e.span.lo() + rustc_span::BytePos(2); let left = e.span.lo() + rustc_span::BytePos(2);
let right = block.span.lo(); let right = block.span.lo();
(cond, UnusedDelimsCtx::IfCond, true, Some(left), Some(right)) (cond, UnusedDelimsCtx::IfCond, true, Some(left), Some(right), true)
} }
// Do not lint `unused_braces` in `while let` expressions. // Do not lint `unused_braces` in `while let` expressions.
@ -738,27 +741,27 @@ trait UnusedDelimLint {
{ {
let left = e.span.lo() + rustc_span::BytePos(5); let left = e.span.lo() + rustc_span::BytePos(5);
let right = block.span.lo(); let right = block.span.lo();
(cond, UnusedDelimsCtx::WhileCond, true, Some(left), Some(right)) (cond, UnusedDelimsCtx::WhileCond, true, Some(left), Some(right), true)
} }
ForLoop(_, ref cond, ref block, ..) => { ForLoop(_, ref cond, ref block, ..) => {
(cond, UnusedDelimsCtx::ForIterExpr, true, None, Some(block.span.lo())) (cond, UnusedDelimsCtx::ForIterExpr, true, None, Some(block.span.lo()), true)
} }
Match(ref head, _) if Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX => { Match(ref head, _) if Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX => {
let left = e.span.lo() + rustc_span::BytePos(5); let left = e.span.lo() + rustc_span::BytePos(5);
(head, UnusedDelimsCtx::MatchScrutineeExpr, true, Some(left), None) (head, UnusedDelimsCtx::MatchScrutineeExpr, true, Some(left), None, true)
} }
Ret(Some(ref value)) => { Ret(Some(ref value)) => {
let left = e.span.lo() + rustc_span::BytePos(3); let left = e.span.lo() + rustc_span::BytePos(3);
(value, UnusedDelimsCtx::ReturnValue, false, Some(left), None) (value, UnusedDelimsCtx::ReturnValue, false, Some(left), None, true)
} }
Index(_, ref value) => (value, UnusedDelimsCtx::IndexExpr, false, None, None), Index(_, ref value) => (value, UnusedDelimsCtx::IndexExpr, false, None, None, false),
Assign(_, ref value, _) | AssignOp(.., ref value) => { Assign(_, ref value, _) | AssignOp(.., ref value) => {
(value, UnusedDelimsCtx::AssignedValue, false, None, None) (value, UnusedDelimsCtx::AssignedValue, false, None, None, false)
} }
// either function/method call, or something this lint doesn't care about // either function/method call, or something this lint doesn't care about
ref call_or_other => { ref call_or_other => {
@ -778,12 +781,20 @@ trait UnusedDelimLint {
return; return;
} }
for arg in args_to_check { for arg in args_to_check {
self.check_unused_delims_expr(cx, arg, ctx, false, None, None); self.check_unused_delims_expr(cx, arg, ctx, false, None, None, false);
} }
return; return;
} }
}; };
self.check_unused_delims_expr(cx, &value, ctx, followed_by_block, left_pos, right_pos); self.check_unused_delims_expr(
cx,
&value,
ctx,
followed_by_block,
left_pos,
right_pos,
is_kw,
);
} }
fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) { fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) {
@ -794,7 +805,7 @@ trait UnusedDelimLint {
None => UnusedDelimsCtx::AssignedValue, None => UnusedDelimsCtx::AssignedValue,
Some(_) => UnusedDelimsCtx::AssignedValueLetElse, Some(_) => UnusedDelimsCtx::AssignedValueLetElse,
}; };
self.check_unused_delims_expr(cx, init, ctx, false, None, None); self.check_unused_delims_expr(cx, init, ctx, false, None, None, false);
} }
} }
StmtKind::Expr(ref expr) => { StmtKind::Expr(ref expr) => {
@ -805,6 +816,7 @@ trait UnusedDelimLint {
false, false,
None, None,
None, None,
false,
); );
} }
_ => {} _ => {}
@ -824,6 +836,7 @@ trait UnusedDelimLint {
false, false,
None, None,
None, None,
false,
); );
} }
} }
@ -879,6 +892,7 @@ impl UnusedDelimLint for UnusedParens {
followed_by_block: bool, followed_by_block: bool,
left_pos: Option<BytePos>, left_pos: Option<BytePos>,
right_pos: Option<BytePos>, right_pos: Option<BytePos>,
is_kw: bool,
) { ) {
match value.kind { match value.kind {
ast::ExprKind::Paren(ref inner) => { ast::ExprKind::Paren(ref inner) => {
@ -893,7 +907,7 @@ impl UnusedDelimLint for UnusedParens {
_, _,
) if node.lazy())) ) if node.lazy()))
{ {
self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos) self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos, is_kw)
} }
} }
ast::ExprKind::Let(_, ref expr, _) => { ast::ExprKind::Let(_, ref expr, _) => {
@ -904,6 +918,7 @@ impl UnusedDelimLint for UnusedParens {
followed_by_block, followed_by_block,
None, None,
None, None,
false,
); );
} }
_ => {} _ => {}
@ -942,7 +957,7 @@ impl UnusedParens {
.span .span
.find_ancestor_inside(value.span) .find_ancestor_inside(value.span)
.map(|inner| (value.span.with_hi(inner.lo()), value.span.with_lo(inner.hi()))); .map(|inner| (value.span.with_hi(inner.lo()), value.span.with_lo(inner.hi())));
self.emit_unused_delims(cx, value.span, spans, "pattern", keep_space); self.emit_unused_delims(cx, value.span, spans, "pattern", keep_space, false);
} }
} }
} }
@ -967,6 +982,7 @@ impl EarlyLintPass for UnusedParens {
true, true,
None, None,
None, None,
true,
); );
for stmt in &block.stmts { for stmt in &block.stmts {
<Self as UnusedDelimLint>::check_stmt(self, cx, stmt); <Self as UnusedDelimLint>::check_stmt(self, cx, stmt);
@ -985,6 +1001,7 @@ impl EarlyLintPass for UnusedParens {
false, false,
None, None,
None, None,
true,
); );
} }
} }
@ -1043,6 +1060,7 @@ impl EarlyLintPass for UnusedParens {
false, false,
None, None,
None, None,
false,
); );
} }
ast::TyKind::Paren(r) => { ast::TyKind::Paren(r) => {
@ -1057,7 +1075,7 @@ impl EarlyLintPass for UnusedParens {
.find_ancestor_inside(ty.span) .find_ancestor_inside(ty.span)
.map(|r| (ty.span.with_hi(r.lo()), ty.span.with_lo(r.hi()))); .map(|r| (ty.span.with_hi(r.lo()), ty.span.with_lo(r.hi())));
self.emit_unused_delims(cx, ty.span, spans, "type", (false, false)); self.emit_unused_delims(cx, ty.span, spans, "type", (false, false), false);
} }
} }
self.with_self_ty_parens = false; self.with_self_ty_parens = false;
@ -1130,6 +1148,7 @@ impl UnusedDelimLint for UnusedBraces {
followed_by_block: bool, followed_by_block: bool,
left_pos: Option<BytePos>, left_pos: Option<BytePos>,
right_pos: Option<BytePos>, right_pos: Option<BytePos>,
is_kw: bool,
) { ) {
match value.kind { match value.kind {
ast::ExprKind::Block(ref inner, None) ast::ExprKind::Block(ref inner, None)
@ -1170,7 +1189,7 @@ impl UnusedDelimLint for UnusedBraces {
&& !value.span.from_expansion() && !value.span.from_expansion()
&& !inner.span.from_expansion() && !inner.span.from_expansion()
{ {
self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos) self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos, is_kw)
} }
} }
} }
@ -1183,6 +1202,7 @@ impl UnusedDelimLint for UnusedBraces {
followed_by_block, followed_by_block,
None, None,
None, None,
false,
); );
} }
_ => {} _ => {}
@ -1207,6 +1227,7 @@ impl EarlyLintPass for UnusedBraces {
false, false,
None, None,
None, None,
false,
); );
} }
} }
@ -1220,6 +1241,7 @@ impl EarlyLintPass for UnusedBraces {
false, false,
None, None,
None, None,
false,
); );
} }
} }
@ -1233,6 +1255,7 @@ impl EarlyLintPass for UnusedBraces {
false, false,
None, None,
None, None,
false,
); );
} }
} }
@ -1247,6 +1270,7 @@ impl EarlyLintPass for UnusedBraces {
false, false,
None, None,
None, None,
false,
); );
} }
@ -1258,6 +1282,7 @@ impl EarlyLintPass for UnusedBraces {
false, false,
None, None,
None, None,
false,
); );
} }

View File

@ -35,6 +35,14 @@ pub fn passes_unused_parens_lint() -> &'static (dyn Trait) {
panic!() panic!()
} }
pub fn parens_with_keyword(e: &[()]) -> i32 {
if true {} //~ ERROR unnecessary parentheses around `if`
while true {} //~ ERROR unnecessary parentheses around `while`
for _ in e {} //~ ERROR unnecessary parentheses around `for`
match 1 { _ => ()} //~ ERROR unnecessary parentheses around `match`
return 1; //~ ERROR unnecessary parentheses around `return` value
}
macro_rules! baz { macro_rules! baz {
($($foo:expr),+) => { ($($foo:expr),+) => {
($($foo),*) ($($foo),*)

View File

@ -35,6 +35,14 @@ pub fn passes_unused_parens_lint() -> &'static (dyn Trait) {
panic!() panic!()
} }
pub fn parens_with_keyword(e: &[()]) -> i32 {
if(true) {} //~ ERROR unnecessary parentheses around `if`
while(true) {} //~ ERROR unnecessary parentheses around `while`
for _ in(e) {} //~ ERROR unnecessary parentheses around `for`
match(1) { _ => ()} //~ ERROR unnecessary parentheses around `match`
return(1); //~ ERROR unnecessary parentheses around `return` value
}
macro_rules! baz { macro_rules! baz {
($($foo:expr),+) => { ($($foo:expr),+) => {
($($foo),*) ($($foo),*)

View File

@ -63,8 +63,68 @@ LL - (5)
LL + 5 LL + 5
| |
error: unnecessary parentheses around `if` condition
--> $DIR/lint-unnecessary-parens.rs:39:7
|
LL | if(true) {}
| ^ ^
|
help: remove these parentheses
|
LL - if(true) {}
LL + if true {}
|
error: unnecessary parentheses around `while` condition
--> $DIR/lint-unnecessary-parens.rs:40:10
|
LL | while(true) {}
| ^ ^
|
help: remove these parentheses
|
LL - while(true) {}
LL + while true {}
|
error: unnecessary parentheses around `for` iterator expression
--> $DIR/lint-unnecessary-parens.rs:41:13
|
LL | for _ in(e) {}
| ^ ^
|
help: remove these parentheses
|
LL - for _ in(e) {}
LL + for _ in e {}
|
error: unnecessary parentheses around `match` scrutinee expression
--> $DIR/lint-unnecessary-parens.rs:42:10
|
LL | match(1) { _ => ()}
| ^ ^
|
help: remove these parentheses
|
LL - match(1) { _ => ()}
LL + match 1 { _ => ()}
|
error: unnecessary parentheses around `return` value
--> $DIR/lint-unnecessary-parens.rs:43:11
|
LL | return(1);
| ^ ^
|
help: remove these parentheses
|
LL - return(1);
LL + return 1;
|
error: unnecessary parentheses around assigned value error: unnecessary parentheses around assigned value
--> $DIR/lint-unnecessary-parens.rs:44:31 --> $DIR/lint-unnecessary-parens.rs:52:31
| |
LL | pub const CONST_ITEM: usize = (10); LL | pub const CONST_ITEM: usize = (10);
| ^ ^ | ^ ^
@ -76,7 +136,7 @@ LL + pub const CONST_ITEM: usize = 10;
| |
error: unnecessary parentheses around assigned value error: unnecessary parentheses around assigned value
--> $DIR/lint-unnecessary-parens.rs:45:33 --> $DIR/lint-unnecessary-parens.rs:53:33
| |
LL | pub static STATIC_ITEM: usize = (10); LL | pub static STATIC_ITEM: usize = (10);
| ^ ^ | ^ ^
@ -88,7 +148,7 @@ LL + pub static STATIC_ITEM: usize = 10;
| |
error: unnecessary parentheses around function argument error: unnecessary parentheses around function argument
--> $DIR/lint-unnecessary-parens.rs:49:9 --> $DIR/lint-unnecessary-parens.rs:57:9
| |
LL | bar((true)); LL | bar((true));
| ^ ^ | ^ ^
@ -100,7 +160,7 @@ LL + bar(true);
| |
error: unnecessary parentheses around `if` condition error: unnecessary parentheses around `if` condition
--> $DIR/lint-unnecessary-parens.rs:51:8 --> $DIR/lint-unnecessary-parens.rs:59:8
| |
LL | if (true) {} LL | if (true) {}
| ^ ^ | ^ ^
@ -112,7 +172,7 @@ LL + if true {}
| |
error: unnecessary parentheses around `while` condition error: unnecessary parentheses around `while` condition
--> $DIR/lint-unnecessary-parens.rs:52:11 --> $DIR/lint-unnecessary-parens.rs:60:11
| |
LL | while (true) {} LL | while (true) {}
| ^ ^ | ^ ^
@ -124,7 +184,7 @@ LL + while true {}
| |
error: unnecessary parentheses around `match` scrutinee expression error: unnecessary parentheses around `match` scrutinee expression
--> $DIR/lint-unnecessary-parens.rs:53:11 --> $DIR/lint-unnecessary-parens.rs:61:11
| |
LL | match (true) { LL | match (true) {
| ^ ^ | ^ ^
@ -136,7 +196,7 @@ LL + match true {
| |
error: unnecessary parentheses around `let` scrutinee expression error: unnecessary parentheses around `let` scrutinee expression
--> $DIR/lint-unnecessary-parens.rs:56:16 --> $DIR/lint-unnecessary-parens.rs:64:16
| |
LL | if let 1 = (1) {} LL | if let 1 = (1) {}
| ^ ^ | ^ ^
@ -148,7 +208,7 @@ LL + if let 1 = 1 {}
| |
error: unnecessary parentheses around `let` scrutinee expression error: unnecessary parentheses around `let` scrutinee expression
--> $DIR/lint-unnecessary-parens.rs:57:19 --> $DIR/lint-unnecessary-parens.rs:65:19
| |
LL | while let 1 = (2) {} LL | while let 1 = (2) {}
| ^ ^ | ^ ^
@ -160,7 +220,7 @@ LL + while let 1 = 2 {}
| |
error: unnecessary parentheses around method argument error: unnecessary parentheses around method argument
--> $DIR/lint-unnecessary-parens.rs:73:24 --> $DIR/lint-unnecessary-parens.rs:81:24
| |
LL | X { y: false }.foo((true)); LL | X { y: false }.foo((true));
| ^ ^ | ^ ^
@ -172,7 +232,7 @@ LL + X { y: false }.foo(true);
| |
error: unnecessary parentheses around assigned value error: unnecessary parentheses around assigned value
--> $DIR/lint-unnecessary-parens.rs:75:18 --> $DIR/lint-unnecessary-parens.rs:83:18
| |
LL | let mut _a = (0); LL | let mut _a = (0);
| ^ ^ | ^ ^
@ -184,7 +244,7 @@ LL + let mut _a = 0;
| |
error: unnecessary parentheses around assigned value error: unnecessary parentheses around assigned value
--> $DIR/lint-unnecessary-parens.rs:76:10 --> $DIR/lint-unnecessary-parens.rs:84:10
| |
LL | _a = (0); LL | _a = (0);
| ^ ^ | ^ ^
@ -196,7 +256,7 @@ LL + _a = 0;
| |
error: unnecessary parentheses around assigned value error: unnecessary parentheses around assigned value
--> $DIR/lint-unnecessary-parens.rs:77:11 --> $DIR/lint-unnecessary-parens.rs:85:11
| |
LL | _a += (1); LL | _a += (1);
| ^ ^ | ^ ^
@ -207,5 +267,5 @@ LL - _a += (1);
LL + _a += 1; LL + _a += 1;
| |
error: aborting due to 17 previous errors error: aborting due to 22 previous errors