Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clippy_utils: fix needless parenthesis output from sugg::Sugg::maybe_par #7013

Merged
merged 1 commit into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions clippy_utils/src/sugg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,17 +267,44 @@ impl<'a> Sugg<'a> {
Sugg::NonParen(..) => self,
// `(x)` and `(x).y()` both don't need additional parens.
Sugg::MaybeParen(sugg) => {
if sugg.starts_with('(') && sugg.ends_with(')') {
if has_enclosing_paren(&sugg) {
Sugg::MaybeParen(sugg)
} else {
Sugg::NonParen(format!("({})", sugg).into())
}
},
Sugg::BinOp(_, sugg) => Sugg::NonParen(format!("({})", sugg).into()),
Sugg::BinOp(_, sugg) => {
if has_enclosing_paren(&sugg) {
Sugg::NonParen(sugg)
} else {
Sugg::NonParen(format!("({})", sugg).into())
}
},
}
}
}

/// Return `true` if `sugg` is enclosed in parenthesis.
fn has_enclosing_paren(sugg: impl AsRef<str>) -> bool {
let mut chars = sugg.as_ref().chars();
if let Some('(') = chars.next() {
let mut depth = 1;
while let Some(c) = chars.next() {
if c == '(' {
depth += 1;
} else if c == ')' {
depth -= 1;
}
if depth == 0 {
break;
}
}
chars.next().is_none()
} else {
false
}
}

// Copied from the rust standart library, and then edited
macro_rules! forward_binop_impls_to_ref {
(impl $imp:ident, $method:ident for $t:ty, type Output = $o:ty) => {
Expand Down Expand Up @@ -668,6 +695,8 @@ impl<T: LintContext> DiagnosticBuilderExt<T> for rustc_errors::DiagnosticBuilder
#[cfg(test)]
mod test {
use super::Sugg;

use rustc_ast::util::parser::AssocOp;
use std::borrow::Cow;

const SUGGESTION: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("function_call()"));
Expand All @@ -681,4 +710,13 @@ mod test {
fn blockify_transforms_sugg_into_a_block() {
assert_eq!("{ function_call() }", SUGGESTION.blockify().to_string());
}

#[test]
fn binop_maybe_par() {
let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1)".into());
assert_eq!("(1 + 1)", sugg.maybe_par().to_string());

let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1) + (1 + 1)".into());
assert_eq!("((1 + 1) + (1 + 1))", sugg.maybe_par().to_string());
}
}
2 changes: 1 addition & 1 deletion tests/ui/floating_point_log.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn check_ln1p() {
let _ = (x / 2.0).ln_1p();
let _ = x.powi(3).ln_1p();
let _ = (x.powi(3) / 2.0).ln_1p();
let _ = ((std::f32::consts::E - 1.0)).ln_1p();
let _ = (std::f32::consts::E - 1.0).ln_1p();
let _ = x.ln_1p();
let _ = x.powi(3).ln_1p();
let _ = (x + 2.0).ln_1p();
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/floating_point_log.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ error: ln(1 + x) can be computed more accurately
--> $DIR/floating_point_log.rs:30:13
|
LL | let _ = (1.0 + (std::f32::consts::E - 1.0)).ln();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `((std::f32::consts::E - 1.0)).ln_1p()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(std::f32::consts::E - 1.0).ln_1p()`

error: ln(1 + x) can be computed more accurately
--> $DIR/floating_point_log.rs:31:13
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/from_str_radix_10.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ error: this call to `from_str_radix` can be replaced with a call to `str::parse`
--> $DIR/from_str_radix_10.rs:32:5
|
LL | u16::from_str_radix(&("10".to_owned() + "5"), 10)?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(("10".to_owned() + "5")).parse::<u16>()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("10".to_owned() + "5").parse::<u16>()`

error: this call to `from_str_radix` can be replaced with a call to `str::parse`
--> $DIR/from_str_radix_10.rs:33:5
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/manual_memcpy/with_loop_counters.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ LL | / for i in 3..(3 + src.len()) {
LL | | dst[i] = src[count];
LL | | count += 1;
LL | | }
| |_____^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)]);`
| |_____^ help: try replacing the loop by: `dst[3..(3 + src.len())].clone_from_slice(&src[..((3 + src.len()) - 3)]);`

error: it looks like you're manually copying between slices
--> $DIR/with_loop_counters.rs:35:5
Expand Down