From 81db00a3c4519600ee1374eb131a159c384422ea Mon Sep 17 00:00:00 2001 From: Colin Delahunty <72827203+colin99d@users.noreply.github.com> Date: Fri, 20 Jan 2023 00:04:07 -0500 Subject: [PATCH] Pyupgrade: Extraneous parenthesis (#1926) --- README.md | 1 + resources/test/fixtures/pyupgrade/UP034.py | 61 ++++++ ruff.schema.json | 1 + src/checkers/tokens.rs | 12 +- src/registry.rs | 22 ++- src/rules/pyupgrade/mod.rs | 1 + .../pyupgrade/rules/extraneous_parentheses.rs | 143 ++++++++++++++ src/rules/pyupgrade/rules/mod.rs | 2 + ...les__pyupgrade__tests__UP034_UP034.py.snap | 175 ++++++++++++++++++ src/violations.rs | 14 ++ 10 files changed, 421 insertions(+), 11 deletions(-) create mode 100644 resources/test/fixtures/pyupgrade/UP034.py create mode 100644 src/rules/pyupgrade/rules/extraneous_parentheses.rs create mode 100644 src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP034_UP034.py.snap diff --git a/README.md b/README.md index 41592d5128..5edd20416a 100644 --- a/README.md +++ b/README.md @@ -728,6 +728,7 @@ For more, see [pyupgrade](https://pypi.org/project/pyupgrade/3.2.0/) on PyPI. | UP030 | format-literals | Use implicit references for positional format fields | 🛠 | | UP032 | f-string | Use f-string instead of `format` call | 🛠 | | UP033 | functools-cache | Use `@functools.cache` instead of `@functools.lru_cache(maxsize=None)` | 🛠 | +| UP034 | extraneous-parentheses | Avoid extraneous parentheses | 🛠 | ### pep8-naming (N) diff --git a/resources/test/fixtures/pyupgrade/UP034.py b/resources/test/fixtures/pyupgrade/UP034.py new file mode 100644 index 0000000000..588f6f3ac5 --- /dev/null +++ b/resources/test/fixtures/pyupgrade/UP034.py @@ -0,0 +1,61 @@ +# UP034 +print(("foo")) + +# UP034 +print(("hell((goodybe))o")) + +# UP034 +print((("foo"))) + +# UP034 +print((((1)))) + +# UP034 +print(("foo{}".format(1))) + +# UP034 +print( + ("foo{}".format(1)) +) + +# UP034 +print( + ( + "foo" + ) +) + +# UP034 +def f(): + x = int(((yield 1))) + +# UP034 +if True: + print( + ("foo{}".format(1)) + ) + +# UP034 +print((x for x in range(3))) + +# OK +print("foo") + +# OK +print((1, 2, 3)) + +# OK +print(()) + +# OK +print((1,)) + +# OK +sum((block.code for block in blocks), []) + +# OK +def f(): + x = int((yield 1)) + +# OK +sum((i for i in range(3)), []) diff --git a/ruff.schema.json b/ruff.schema.json index 429efff4e5..f68c6ddc4e 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1768,6 +1768,7 @@ "UP030", "UP032", "UP033", + "UP034", "W", "W2", "W29", diff --git a/src/checkers/tokens.rs b/src/checkers/tokens.rs index 5c414da48e..c1f2a350b0 100644 --- a/src/checkers/tokens.rs +++ b/src/checkers/tokens.rs @@ -6,7 +6,8 @@ use crate::lex::docstring_detection::StateMachine; use crate::registry::{Diagnostic, Rule}; use crate::rules::ruff::rules::Context; use crate::rules::{ - eradicate, flake8_commas, flake8_implicit_str_concat, flake8_quotes, pycodestyle, ruff, + eradicate, flake8_commas, flake8_implicit_str_concat, flake8_quotes, pycodestyle, pyupgrade, + ruff, }; use crate::settings::{flags, Settings}; use crate::source_code::Locator; @@ -45,6 +46,7 @@ pub fn check_tokens( .rules .enabled(&Rule::TrailingCommaOnBareTupleProhibited) || settings.rules.enabled(&Rule::TrailingCommaProhibited); + let enforce_extraneous_parenthesis = settings.rules.enabled(&Rule::ExtraneousParentheses); let mut state_machine = StateMachine::default(); for &(start, ref tok, end) in tokens.iter().flatten() { @@ -137,5 +139,13 @@ pub fn check_tokens( ); } + // UP034 + if enforce_extraneous_parenthesis { + diagnostics.extend( + pyupgrade::rules::extraneous_parentheses(tokens, locator, settings, autofix) + .into_iter(), + ); + } + diagnostics } diff --git a/src/registry.rs b/src/registry.rs index 190ee029ef..edd7fde6f0 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -251,7 +251,8 @@ ruff_macros::define_rule_mapping!( UP029 => violations::UnnecessaryBuiltinImport, UP030 => violations::FormatLiterals, UP032 => violations::FString, - UP033 => violations::FunctoolsCache, + UP033 => violations::FunctoolsCache, + UP034 => violations::ExtraneousParentheses, // pydocstyle D100 => violations::PublicModule, D101 => violations::PublicClass, @@ -555,20 +556,21 @@ impl Rule { | Rule::PEP3120UnnecessaryCodingComment | Rule::BlanketTypeIgnore | Rule::BlanketNOQA => &LintSource::Lines, - Rule::CommentedOutCode - | Rule::SingleLineImplicitStringConcatenation - | Rule::MultiLineImplicitStringConcatenation + Rule::AmbiguousUnicodeCharacterComment + | Rule::AmbiguousUnicodeCharacterDocstring + | Rule::AmbiguousUnicodeCharacterString + | Rule::AvoidQuoteEscape + | Rule::BadQuotesDocstring | Rule::BadQuotesInlineString | Rule::BadQuotesMultilineString - | Rule::BadQuotesDocstring - | Rule::AvoidQuoteEscape + | Rule::CommentedOutCode + | Rule::ExtraneousParentheses | Rule::InvalidEscapeSequence + | Rule::MultiLineImplicitStringConcatenation + | Rule::SingleLineImplicitStringConcatenation | Rule::TrailingCommaMissing | Rule::TrailingCommaOnBareTupleProhibited - | Rule::TrailingCommaProhibited - | Rule::AmbiguousUnicodeCharacterString - | Rule::AmbiguousUnicodeCharacterDocstring - | Rule::AmbiguousUnicodeCharacterComment => &LintSource::Tokens, + | Rule::TrailingCommaProhibited => &LintSource::Tokens, Rule::IOError => &LintSource::Io, Rule::UnsortedImports | Rule::MissingRequiredImport => &LintSource::Imports, Rule::ImplicitNamespacePackage => &LintSource::Filesystem, diff --git a/src/rules/pyupgrade/mod.rs b/src/rules/pyupgrade/mod.rs index c5a4c56428..2dceb37c14 100644 --- a/src/rules/pyupgrade/mod.rs +++ b/src/rules/pyupgrade/mod.rs @@ -54,6 +54,7 @@ mod tests { #[test_case(Rule::FormatLiterals, Path::new("UP030_1.py"); "UP030_1")] #[test_case(Rule::FString, Path::new("UP032.py"); "UP032")] #[test_case(Rule::FunctoolsCache, Path::new("UP033.py"); "UP033")] + #[test_case(Rule::ExtraneousParentheses, Path::new("UP034.py"); "UP034")] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/src/rules/pyupgrade/rules/extraneous_parentheses.rs b/src/rules/pyupgrade/rules/extraneous_parentheses.rs new file mode 100644 index 0000000000..c7ee2508f4 --- /dev/null +++ b/src/rules/pyupgrade/rules/extraneous_parentheses.rs @@ -0,0 +1,143 @@ +use rustpython_parser::lexer::{LexResult, Tok}; + +use crate::ast::types::Range; +use crate::fix::Fix; +use crate::registry::{Diagnostic, Rule}; +use crate::settings::{flags, Settings}; +use crate::source_code::Locator; +use crate::violations; + +// See: https://github.com/asottile/pyupgrade/blob/97ed6fb3cf2e650d4f762ba231c3f04c41797710/pyupgrade/_main.py#L148 +fn match_extraneous_parentheses(tokens: &[LexResult], mut i: usize) -> Option<(usize, usize)> { + i += 1; + + loop { + if i >= tokens.len() { + return None; + } + let Ok((_, tok, _)) = &tokens[i] else { + return None; + }; + match tok { + Tok::Comment(..) | Tok::NonLogicalNewline => { + i += 1; + } + Tok::Lpar => { + break; + } + _ => { + return None; + } + } + } + + // Store the location of the extraneous opening parenthesis. + let start = i; + + // Verify that we're not in a tuple or coroutine. + let mut depth = 1; + while depth > 0 { + i += 1; + if i >= tokens.len() { + return None; + } + let Ok((_, tok, _)) = &tokens[i] else { + return None; + }; + + // If we find a comma or a yield at depth 1 or 2, it's a tuple or coroutine. + if depth == 1 && matches!(tok, Tok::Comma | Tok::Yield) { + return None; + } else if matches!(tok, Tok::Lpar | Tok::Lbrace | Tok::Lsqb) { + depth += 1; + } else if matches!(tok, Tok::Rpar | Tok::Rbrace | Tok::Rsqb) { + depth -= 1; + } + } + + // Store the location of the extraneous closing parenthesis. + let end = i; + + // Verify that we're not in an empty tuple. + if (start + 1..i).all(|i| { + matches!( + tokens[i], + Ok((_, Tok::Comment(..) | Tok::NonLogicalNewline, _)) + ) + }) { + return None; + } + + // Find the next non-coding token. + i += 1; + loop { + if i >= tokens.len() { + return None; + } + let Ok((_, tok, _)) = &tokens[i] else { + return None; + }; + match tok { + Tok::Comment(..) | Tok::NonLogicalNewline => { + i += 1; + } + _ => { + break; + } + } + } + + if i >= tokens.len() { + return None; + } + let Ok((_, tok, _)) = &tokens[i] else { + return None; + }; + if matches!(tok, Tok::Rpar) { + Some((start, end)) + } else { + None + } +} + +/// UP034 +pub fn extraneous_parentheses( + tokens: &[LexResult], + locator: &Locator, + settings: &Settings, + autofix: flags::Autofix, +) -> Vec { + let mut diagnostics = vec![]; + let mut i = 0; + while i < tokens.len() { + if matches!(tokens[i], Ok((_, Tok::Lpar, _))) { + if let Some((start, end)) = match_extraneous_parentheses(tokens, i) { + i = end + 1; + let Ok((start, ..)) = &tokens[start] else { + return diagnostics; + }; + let Ok((.., end)) = &tokens[end] else { + return diagnostics; + }; + let mut diagnostic = + Diagnostic::new(violations::ExtraneousParentheses, Range::new(*start, *end)); + if matches!(autofix, flags::Autofix::Enabled) + && settings.rules.should_fix(&Rule::ExtraneousParentheses) + { + let contents = locator.slice_source_code_range(&Range::new(*start, *end)); + diagnostic.amend(Fix::replacement( + contents[1..contents.len() - 1].to_string(), + *start, + *end, + )); + } + diagnostics.push(diagnostic); + } else { + i += 1; + } + } else { + i += 1; + } + } + diagnostics +} diff --git a/src/rules/pyupgrade/rules/mod.rs b/src/rules/pyupgrade/rules/mod.rs index af4c33d93b..8920b6afa7 100644 --- a/src/rules/pyupgrade/rules/mod.rs +++ b/src/rules/pyupgrade/rules/mod.rs @@ -2,6 +2,7 @@ pub(crate) use convert_named_tuple_functional_to_class::convert_named_tuple_func pub(crate) use convert_typed_dict_functional_to_class::convert_typed_dict_functional_to_class; pub(crate) use datetime_utc_alias::datetime_utc_alias; pub(crate) use deprecated_unittest_alias::deprecated_unittest_alias; +pub(crate) use extraneous_parentheses::extraneous_parentheses; pub(crate) use f_strings::f_strings; pub(crate) use format_literals::format_literals; pub(crate) use functools_cache::functools_cache; @@ -43,6 +44,7 @@ mod convert_named_tuple_functional_to_class; mod convert_typed_dict_functional_to_class; mod datetime_utc_alias; mod deprecated_unittest_alias; +mod extraneous_parentheses; mod f_strings; mod format_literals; mod functools_cache; diff --git a/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP034_UP034.py.snap b/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP034_UP034.py.snap new file mode 100644 index 0000000000..4b82fb3021 --- /dev/null +++ b/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP034_UP034.py.snap @@ -0,0 +1,175 @@ +--- +source: src/rules/pyupgrade/mod.rs +expression: diagnostics +--- +- kind: + ExtraneousParentheses: ~ + location: + row: 2 + column: 6 + end_location: + row: 2 + column: 13 + fix: + content: "\"foo\"" + location: + row: 2 + column: 6 + end_location: + row: 2 + column: 13 + parent: ~ +- kind: + ExtraneousParentheses: ~ + location: + row: 5 + column: 6 + end_location: + row: 5 + column: 26 + fix: + content: "\"hell((goodybe))o\"" + location: + row: 5 + column: 6 + end_location: + row: 5 + column: 26 + parent: ~ +- kind: + ExtraneousParentheses: ~ + location: + row: 8 + column: 6 + end_location: + row: 8 + column: 15 + fix: + content: "(\"foo\")" + location: + row: 8 + column: 6 + end_location: + row: 8 + column: 15 + parent: ~ +- kind: + ExtraneousParentheses: ~ + location: + row: 11 + column: 6 + end_location: + row: 11 + column: 13 + fix: + content: ((1)) + location: + row: 11 + column: 6 + end_location: + row: 11 + column: 13 + parent: ~ +- kind: + ExtraneousParentheses: ~ + location: + row: 14 + column: 6 + end_location: + row: 14 + column: 25 + fix: + content: "\"foo{}\".format(1)" + location: + row: 14 + column: 6 + end_location: + row: 14 + column: 25 + parent: ~ +- kind: + ExtraneousParentheses: ~ + location: + row: 18 + column: 4 + end_location: + row: 18 + column: 23 + fix: + content: "\"foo{}\".format(1)" + location: + row: 18 + column: 4 + end_location: + row: 18 + column: 23 + parent: ~ +- kind: + ExtraneousParentheses: ~ + location: + row: 23 + column: 4 + end_location: + row: 25 + column: 5 + fix: + content: "\n \"foo\"\n " + location: + row: 23 + column: 4 + end_location: + row: 25 + column: 5 + parent: ~ +- kind: + ExtraneousParentheses: ~ + location: + row: 30 + column: 12 + end_location: + row: 30 + column: 23 + fix: + content: (yield 1) + location: + row: 30 + column: 12 + end_location: + row: 30 + column: 23 + parent: ~ +- kind: + ExtraneousParentheses: ~ + location: + row: 35 + column: 8 + end_location: + row: 35 + column: 27 + fix: + content: "\"foo{}\".format(1)" + location: + row: 35 + column: 8 + end_location: + row: 35 + column: 27 + parent: ~ +- kind: + ExtraneousParentheses: ~ + location: + row: 39 + column: 6 + end_location: + row: 39 + column: 27 + fix: + content: x for x in range(3) + location: + row: 39 + column: 6 + end_location: + row: 39 + column: 27 + parent: ~ + diff --git a/src/violations.rs b/src/violations.rs index 9b1b1f4904..cc2d51d418 100644 --- a/src/violations.rs +++ b/src/violations.rs @@ -3152,6 +3152,20 @@ impl AlwaysAutofixableViolation for FormatLiterals { } } +define_violation!( + pub struct ExtraneousParentheses; +); +impl AlwaysAutofixableViolation for ExtraneousParentheses { + #[derive_message_formats] + fn message(&self) -> String { + format!("Avoid extraneous parentheses") + } + + fn autofix_title(&self) -> String { + "Remove extraneous parentheses".to_string() + } +} + define_violation!( pub struct FString; );