Add a rule to remove unnecessary parentheses in class definitions (#5032)

Closes #2409.
This commit is contained in:
Charlie Marsh 2023-06-12 14:43:06 -04:00 committed by GitHub
parent 3470dee7d4
commit 54e103fc99
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 230 additions and 40 deletions

View file

@ -0,0 +1,26 @@
# Errors
class A():
pass
class A() \
:
pass
class A \
():
pass
# OK
class A:
pass
class A(A):
pass
class A(metaclass=type):
pass

View file

@ -729,6 +729,9 @@ where
if self.enabled(Rule::UselessObjectInheritance) {
pyupgrade::rules::useless_object_inheritance(self, stmt, name, bases, keywords);
}
if self.enabled(Rule::UnnecessaryClassParentheses) {
pyupgrade::rules::unnecessary_class_parentheses(self, class_def);
}
if self.enabled(Rule::AmbiguousClassName) {
if let Some(diagnostic) = pycodestyle::rules::ambiguous_class_name(name, || {

View file

@ -414,6 +414,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pyupgrade, "036") => (RuleGroup::Unspecified, rules::pyupgrade::rules::OutdatedVersionBlock),
(Pyupgrade, "037") => (RuleGroup::Unspecified, rules::pyupgrade::rules::QuotedAnnotation),
(Pyupgrade, "038") => (RuleGroup::Unspecified, rules::pyupgrade::rules::NonPEP604Isinstance),
(Pyupgrade, "039") => (RuleGroup::Unspecified, rules::pyupgrade::rules::UnnecessaryClassParentheses),
// pydocstyle
(Pydocstyle, "100") => (RuleGroup::Unspecified, rules::pydocstyle::rules::UndocumentedPublicModule),

View file

@ -16,64 +16,65 @@ mod tests {
use crate::test::test_path;
use crate::{assert_messages, settings};
#[test_case(Rule::UselessMetaclassType, Path::new("UP001.py"))]
#[test_case(Rule::TypeOfPrimitive, Path::new("UP003.py"))]
#[test_case(Rule::UselessObjectInheritance, Path::new("UP004.py"))]
#[test_case(Rule::ConvertNamedTupleFunctionalToClass, Path::new("UP014.py"))]
#[test_case(Rule::ConvertTypedDictFunctionalToClass, Path::new("UP013.py"))]
#[test_case(Rule::DeprecatedCElementTree, Path::new("UP023.py"))]
#[test_case(Rule::DeprecatedImport, Path::new("UP035.py"))]
#[test_case(Rule::DeprecatedMockImport, Path::new("UP026.py"))]
#[test_case(Rule::DeprecatedUnittestAlias, Path::new("UP005.py"))]
#[test_case(Rule::ExtraneousParentheses, Path::new("UP034.py"))]
#[test_case(Rule::FString, Path::new("UP032_0.py"))]
#[test_case(Rule::FString, Path::new("UP032_1.py"))]
#[test_case(Rule::FString, Path::new("UP032_2.py"))]
#[test_case(Rule::FormatLiterals, Path::new("UP030_0.py"))]
#[test_case(Rule::FormatLiterals, Path::new("UP030_1.py"))]
#[test_case(Rule::FormatLiterals, Path::new("UP030_2.py"))]
#[test_case(Rule::LRUCacheWithMaxsizeNone, Path::new("UP033_0.py"))]
#[test_case(Rule::LRUCacheWithMaxsizeNone, Path::new("UP033_1.py"))]
#[test_case(Rule::LRUCacheWithoutParameters, Path::new("UP011.py"))]
#[test_case(Rule::NativeLiterals, Path::new("UP018.py"))]
#[test_case(Rule::NonPEP585Annotation, Path::new("UP006_0.py"))]
#[test_case(Rule::NonPEP585Annotation, Path::new("UP006_1.py"))]
#[test_case(Rule::NonPEP585Annotation, Path::new("UP006_2.py"))]
#[test_case(Rule::NonPEP585Annotation, Path::new("UP006_3.py"))]
#[test_case(Rule::NonPEP604Annotation, Path::new("UP007.py"))]
#[test_case(Rule::SuperCallWithParameters, Path::new("UP008.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_0.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_1.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_2.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_3.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_4.py"))]
#[test_case(Rule::UnnecessaryFutureImport, Path::new("UP010.py"))]
#[test_case(Rule::LRUCacheWithoutParameters, Path::new("UP011.py"))]
#[test_case(Rule::UnnecessaryEncodeUTF8, Path::new("UP012.py"))]
#[test_case(Rule::ConvertTypedDictFunctionalToClass, Path::new("UP013.py"))]
#[test_case(Rule::ConvertNamedTupleFunctionalToClass, Path::new("UP014.py"))]
#[test_case(Rule::RedundantOpenModes, Path::new("UP015.py"))]
#[test_case(Rule::NativeLiterals, Path::new("UP018.py"))]
#[test_case(Rule::TypingTextStrAlias, Path::new("UP019.py"))]
#[test_case(Rule::OpenAlias, Path::new("UP020.py"))]
#[test_case(Rule::ReplaceUniversalNewlines, Path::new("UP021.py"))]
#[test_case(Rule::ReplaceStdoutStderr, Path::new("UP022.py"))]
#[test_case(Rule::DeprecatedCElementTree, Path::new("UP023.py"))]
#[test_case(Rule::NonPEP604Isinstance, Path::new("UP038.py"))]
#[test_case(Rule::OSErrorAlias, Path::new("UP024_0.py"))]
#[test_case(Rule::OSErrorAlias, Path::new("UP024_1.py"))]
#[test_case(Rule::OSErrorAlias, Path::new("UP024_2.py"))]
#[test_case(Rule::OSErrorAlias, Path::new("UP024_3.py"))]
#[test_case(Rule::OSErrorAlias, Path::new("UP024_4.py"))]
#[test_case(Rule::UnicodeKindPrefix, Path::new("UP025.py"))]
#[test_case(Rule::DeprecatedMockImport, Path::new("UP026.py"))]
#[test_case(Rule::UnpackedListComprehension, Path::new("UP027.py"))]
#[test_case(Rule::YieldInForLoop, Path::new("UP028_0.py"))]
#[test_case(Rule::YieldInForLoop, Path::new("UP028_1.py"))]
#[test_case(Rule::UnnecessaryBuiltinImport, Path::new("UP029.py"))]
#[test_case(Rule::FormatLiterals, Path::new("UP030_0.py"))]
#[test_case(Rule::FormatLiterals, Path::new("UP030_1.py"))]
#[test_case(Rule::FormatLiterals, Path::new("UP030_2.py"))]
#[test_case(Rule::PrintfStringFormatting, Path::new("UP031_0.py"))]
#[test_case(Rule::PrintfStringFormatting, Path::new("UP031_1.py"))]
#[test_case(Rule::FString, Path::new("UP032_0.py"))]
#[test_case(Rule::FString, Path::new("UP032_1.py"))]
#[test_case(Rule::FString, Path::new("UP032_2.py"))]
#[test_case(Rule::LRUCacheWithMaxsizeNone, Path::new("UP033_0.py"))]
#[test_case(Rule::LRUCacheWithMaxsizeNone, Path::new("UP033_1.py"))]
#[test_case(Rule::ExtraneousParentheses, Path::new("UP034.py"))]
#[test_case(Rule::DeprecatedImport, Path::new("UP035.py"))]
#[test_case(Rule::OpenAlias, Path::new("UP020.py"))]
#[test_case(Rule::OutdatedVersionBlock, Path::new("UP036_0.py"))]
#[test_case(Rule::OutdatedVersionBlock, Path::new("UP036_1.py"))]
#[test_case(Rule::OutdatedVersionBlock, Path::new("UP036_2.py"))]
#[test_case(Rule::OutdatedVersionBlock, Path::new("UP036_3.py"))]
#[test_case(Rule::OutdatedVersionBlock, Path::new("UP036_4.py"))]
#[test_case(Rule::OutdatedVersionBlock, Path::new("UP036_5.py"))]
#[test_case(Rule::PrintfStringFormatting, Path::new("UP031_0.py"))]
#[test_case(Rule::PrintfStringFormatting, Path::new("UP031_1.py"))]
#[test_case(Rule::QuotedAnnotation, Path::new("UP037.py"))]
#[test_case(Rule::NonPEP604Isinstance, Path::new("UP038.py"))]
#[test_case(Rule::RedundantOpenModes, Path::new("UP015.py"))]
#[test_case(Rule::ReplaceStdoutStderr, Path::new("UP022.py"))]
#[test_case(Rule::ReplaceUniversalNewlines, Path::new("UP021.py"))]
#[test_case(Rule::SuperCallWithParameters, Path::new("UP008.py"))]
#[test_case(Rule::TypeOfPrimitive, Path::new("UP003.py"))]
#[test_case(Rule::TypingTextStrAlias, Path::new("UP019.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_0.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_1.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_2.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_3.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_4.py"))]
#[test_case(Rule::UnicodeKindPrefix, Path::new("UP025.py"))]
#[test_case(Rule::UnnecessaryBuiltinImport, Path::new("UP029.py"))]
#[test_case(Rule::UnnecessaryClassParentheses, Path::new("UP039.py"))]
#[test_case(Rule::UnnecessaryEncodeUTF8, Path::new("UP012.py"))]
#[test_case(Rule::UnnecessaryFutureImport, Path::new("UP010.py"))]
#[test_case(Rule::UnpackedListComprehension, Path::new("UP027.py"))]
#[test_case(Rule::UselessMetaclassType, Path::new("UP001.py"))]
#[test_case(Rule::UselessObjectInheritance, Path::new("UP004.py"))]
#[test_case(Rule::YieldInForLoop, Path::new("UP028_0.py"))]
#[test_case(Rule::YieldInForLoop, Path::new("UP028_1.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = path.to_string_lossy().to_string();
let diagnostics = test_path(

View file

@ -24,6 +24,7 @@ pub(crate) use type_of_primitive::*;
pub(crate) use typing_text_str_alias::*;
pub(crate) use unicode_kind_prefix::*;
pub(crate) use unnecessary_builtin_import::*;
pub(crate) use unnecessary_class_parentheses::*;
pub(crate) use unnecessary_coding_comment::*;
pub(crate) use unnecessary_encode_utf8::*;
pub(crate) use unnecessary_future_import::*;
@ -61,6 +62,7 @@ mod type_of_primitive;
mod typing_text_str_alias;
mod unicode_kind_prefix;
mod unnecessary_builtin_import;
mod unnecessary_class_parentheses;
mod unnecessary_coding_comment;
mod unnecessary_encode_utf8;
mod unnecessary_future_import;

View file

@ -0,0 +1,96 @@
use std::ops::Add;
use ruff_text_size::{TextRange, TextSize};
use rustpython_parser::ast::{self};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use crate::checkers::ast::Checker;
use crate::registry::AsRule;
/// ## What it does
/// Checks for class definitions that include unnecessary parentheses after
/// the class name.
///
/// ## Why is this bad?
/// If a class definition doesn't have any bases, the parentheses are
/// unnecessary.
///
/// ## Examples
/// ```python
/// class Foo():
/// ...
/// ```
///
/// Use instead:
/// ```python
/// class Foo:
/// ...
/// ```
#[violation]
pub struct UnnecessaryClassParentheses;
impl AlwaysAutofixableViolation for UnnecessaryClassParentheses {
#[derive_message_formats]
fn message(&self) -> String {
format!("Unnecessary parentheses after class definition")
}
fn autofix_title(&self) -> String {
"Remove parentheses".to_string()
}
}
/// UP039
pub(crate) fn unnecessary_class_parentheses(checker: &mut Checker, class_def: &ast::StmtClassDef) {
if !class_def.bases.is_empty() || !class_def.keywords.is_empty() {
return;
}
let contents = checker.locator.slice(class_def.range);
// Find the open and closing parentheses between the class name and the colon, if they exist.
let mut depth = 0u32;
let mut start = None;
let mut end = None;
for (i, c) in contents.char_indices() {
match c {
'(' => {
if depth == 0 {
start = Some(i);
}
depth = depth.saturating_add(1);
}
')' => {
depth = depth.saturating_sub(1);
if depth == 0 {
end = Some(i + c.len_utf8());
}
}
':' => {
if depth == 0 {
break;
}
}
_ => {}
}
}
let (Some(start), Some(end)) = (start, end) else {
return;
};
// Convert to `TextSize`.
let start = TextSize::try_from(start).unwrap();
let end = TextSize::try_from(end).unwrap();
// Add initial offset.
let start = class_def.range.start().add(start);
let end = class_def.range.start().add(end);
let mut diagnostic = Diagnostic::new(UnnecessaryClassParentheses, TextRange::new(start, end));
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::automatic(Edit::deletion(start, end)));
}
checker.diagnostics.push(diagnostic);
}

View file

@ -0,0 +1,59 @@
---
source: crates/ruff/src/rules/pyupgrade/mod.rs
---
UP039.py:2:8: UP039 [*] Unnecessary parentheses after class definition
|
1 | # Errors
2 | class A():
| ^^ UP039
3 | pass
|
= help: Remove parentheses
Fix
1 1 | # Errors
2 |-class A():
2 |+class A:
3 3 | pass
4 4 |
5 5 |
UP039.py:6:8: UP039 [*] Unnecessary parentheses after class definition
|
6 | class A() \
| ^^ UP039
7 | :
8 | pass
|
= help: Remove parentheses
Fix
3 3 | pass
4 4 |
5 5 |
6 |-class A() \
6 |+class A \
7 7 | :
8 8 | pass
9 9 |
UP039.py:12:9: UP039 [*] Unnecessary parentheses after class definition
|
11 | class A \
12 | ():
| ^^ UP039
13 | pass
|
= help: Remove parentheses
Fix
9 9 |
10 10 |
11 11 | class A \
12 |- ():
12 |+ :
13 13 | pass
14 14 |
15 15 |

1
ruff.schema.json generated
View file

@ -2562,6 +2562,7 @@
"UP036",
"UP037",
"UP038",
"UP039",
"W",
"W1",
"W19",

View file

@ -47,6 +47,7 @@ KNOWN_FORMATTING_VIOLATIONS = [
"too-few-spaces-before-inline-comment",
"trailing-comma-on-bare-tuple",
"unexpected-indentation-comment",
"unnecessary-class-parentheses",
"useless-semicolon",
"whitespace-after-open-bracket",
"whitespace-before-close-bracket",