Handle decorators in class-parenthesis-modifying rules (#5034)

## Summary

A few of our rules look at the parentheses that follow a class
definition (e.g., `class Foo(object):`) and attempt to modify those
parentheses. Neither of those rules were behaving properly in the
presence of decorators, which were recently added to the statement
range.

## Test Plan

`cargo test` with a variety of new fixture tests.
This commit is contained in:
Charlie Marsh 2023-06-12 15:19:59 -04:00 committed by GitHub
parent 6d861743c8
commit 4080f36850
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 197 additions and 91 deletions

View file

@ -134,6 +134,19 @@ class A(
... ...
class A(object, object):
...
@decorator()
class A(object):
...
@decorator() # class A(object):
class A(object):
...
object = A object = A

View file

@ -13,6 +13,14 @@ class A \
pass pass
@decorator()
class A():
pass
@decorator
class A():
pass
# OK # OK
class A: class A:
pass pass
@ -24,3 +32,12 @@ class A(A):
class A(metaclass=type): class A(metaclass=type):
pass pass
@decorator()
class A:
pass
@decorator
class A:
pass

View file

@ -727,10 +727,10 @@ where
pylint::rules::global_statement(self, name); pylint::rules::global_statement(self, name);
} }
if self.enabled(Rule::UselessObjectInheritance) { if self.enabled(Rule::UselessObjectInheritance) {
pyupgrade::rules::useless_object_inheritance(self, stmt, name, bases, keywords); pyupgrade::rules::useless_object_inheritance(self, class_def, stmt);
} }
if self.enabled(Rule::UnnecessaryClassParentheses) { if self.enabled(Rule::UnnecessaryClassParentheses) {
pyupgrade::rules::unnecessary_class_parentheses(self, class_def); pyupgrade::rules::unnecessary_class_parentheses(self, class_def, stmt);
} }
if self.enabled(Rule::AmbiguousClassName) { if self.enabled(Rule::AmbiguousClassName) {

View file

@ -1,10 +1,11 @@
use std::ops::Add; use std::ops::Add;
use ruff_text_size::{TextRange, TextSize}; use ruff_text_size::{TextRange, TextSize};
use rustpython_parser::ast::{self}; use rustpython_parser::ast::{self, Stmt};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::identifier_range;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::registry::AsRule; use crate::registry::AsRule;
@ -43,12 +44,17 @@ impl AlwaysAutofixableViolation for UnnecessaryClassParentheses {
} }
/// UP039 /// UP039
pub(crate) fn unnecessary_class_parentheses(checker: &mut Checker, class_def: &ast::StmtClassDef) { pub(crate) fn unnecessary_class_parentheses(
checker: &mut Checker,
class_def: &ast::StmtClassDef,
stmt: &Stmt,
) {
if !class_def.bases.is_empty() || !class_def.keywords.is_empty() { if !class_def.bases.is_empty() || !class_def.keywords.is_empty() {
return; return;
} }
let contents = checker.locator.slice(class_def.range); let offset = identifier_range(stmt, checker.locator).start();
let contents = checker.locator.after(offset);
// Find the open and closing parentheses between the class name and the colon, if they exist. // Find the open and closing parentheses between the class name and the colon, if they exist.
let mut depth = 0u32; let mut depth = 0u32;
@ -85,8 +91,8 @@ pub(crate) fn unnecessary_class_parentheses(checker: &mut Checker, class_def: &a
let end = TextSize::try_from(end).unwrap(); let end = TextSize::try_from(end).unwrap();
// Add initial offset. // Add initial offset.
let start = class_def.range.start().add(start); let start = offset.add(start);
let end = class_def.range.start().add(end); let end = offset.add(end);
let mut diagnostic = Diagnostic::new(UnnecessaryClassParentheses, TextRange::new(start, end)); let mut diagnostic = Diagnostic::new(UnnecessaryClassParentheses, TextRange::new(start, end));
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {

View file

@ -1,9 +1,8 @@
use rustpython_parser::ast::{self, Expr, Keyword, Ranged, Stmt}; use rustpython_parser::ast::{self, Expr, Ranged, Stmt};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::binding::{Binding, BindingKind, Bindings}; use ruff_python_ast::helpers::identifier_range;
use ruff_python_semantic::scope::Scope;
use crate::autofix::edits::remove_argument; use crate::autofix::edits::remove_argument;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -26,62 +25,40 @@ impl AlwaysAutofixableViolation for UselessObjectInheritance {
} }
} }
fn rule(name: &str, bases: &[Expr], scope: &Scope, bindings: &Bindings) -> Option<Diagnostic> { /// UP004
for expr in bases { pub(crate) fn useless_object_inheritance(
checker: &mut Checker,
class_def: &ast::StmtClassDef,
stmt: &Stmt,
) {
for expr in &class_def.bases {
let Expr::Name(ast::ExprName { id, .. }) = expr else { let Expr::Name(ast::ExprName { id, .. }) = expr else {
continue; continue;
}; };
if id != "object" { if id != "object" {
continue; continue;
} }
if !matches!( if !checker.semantic_model().is_builtin("object") {
scope
.get(id.as_str())
.map(|binding_id| &bindings[binding_id]),
None | Some(Binding {
kind: BindingKind::Builtin,
..
})
) {
continue; continue;
} }
return Some(Diagnostic::new(
let mut diagnostic = Diagnostic::new(
UselessObjectInheritance { UselessObjectInheritance {
name: name.to_string(), name: class_def.name.to_string(),
}, },
expr.range(), expr.range(),
)); );
}
None
}
/// UP004
pub(crate) fn useless_object_inheritance(
checker: &mut Checker,
stmt: &Stmt,
name: &str,
bases: &[Expr],
keywords: &[Keyword],
) {
if let Some(mut diagnostic) = rule(
name,
bases,
checker.semantic_model().scope(),
&checker.semantic_model().bindings,
) {
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
let expr_range = diagnostic.range(); diagnostic.try_set_fix(|| {
#[allow(deprecated)] let edit = remove_argument(
diagnostic.try_set_fix_from_edit(|| {
remove_argument(
checker.locator, checker.locator,
stmt.start(), identifier_range(stmt, checker.locator).start(),
expr_range, expr.range(),
bases, &class_def.bases,
keywords, &class_def.keywords,
true, true,
) )?;
Ok(Fix::automatic(edit))
}); });
} }
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);

View file

@ -9,7 +9,7 @@ UP004.py:5:9: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
2 2 | ... 2 2 | ...
3 3 | 3 3 |
4 4 | 4 4 |
@ -29,7 +29,7 @@ UP004.py:10:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
6 6 | ... 6 6 | ...
7 7 | 7 7 |
8 8 | 8 8 |
@ -51,7 +51,7 @@ UP004.py:16:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
12 12 | ... 12 12 | ...
13 13 | 13 13 |
14 14 | 14 14 |
@ -75,7 +75,7 @@ UP004.py:24:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
19 19 | ... 19 19 | ...
20 20 | 20 20 |
21 21 | 21 21 |
@ -99,7 +99,7 @@ UP004.py:31:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
26 26 | ... 26 26 | ...
27 27 | 27 27 |
28 28 | 28 28 |
@ -122,7 +122,7 @@ UP004.py:37:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
33 33 | ... 33 33 | ...
34 34 | 34 34 |
35 35 | 35 35 |
@ -146,7 +146,7 @@ UP004.py:45:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
40 40 | ... 40 40 | ...
41 41 | 41 41 |
42 42 | 42 42 |
@ -171,7 +171,7 @@ UP004.py:53:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
48 48 | ... 48 48 | ...
49 49 | 49 49 |
50 50 | 50 50 |
@ -196,7 +196,7 @@ UP004.py:61:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
56 56 | ... 56 56 | ...
57 57 | 57 57 |
58 58 | 58 58 |
@ -221,7 +221,7 @@ UP004.py:69:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
64 64 | ... 64 64 | ...
65 65 | 65 65 |
66 66 | 66 66 |
@ -243,7 +243,7 @@ UP004.py:75:12: UP004 [*] Class `B` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
72 72 | ... 72 72 | ...
73 73 | 73 73 |
74 74 | 74 74 |
@ -261,7 +261,7 @@ UP004.py:79:9: UP004 [*] Class `B` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
76 76 | ... 76 76 | ...
77 77 | 77 77 |
78 78 | 78 78 |
@ -281,7 +281,7 @@ UP004.py:84:5: UP004 [*] Class `B` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
81 81 | 81 81 |
82 82 | 82 82 |
83 83 | class B( 83 83 | class B(
@ -301,7 +301,7 @@ UP004.py:92:5: UP004 [*] Class `B` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
89 89 | 89 89 |
90 90 | class B( 90 90 | class B(
91 91 | A, 91 91 | A,
@ -320,7 +320,7 @@ UP004.py:98:5: UP004 [*] Class `B` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
95 95 | 95 95 |
96 96 | 96 96 |
97 97 | class B( 97 97 | class B(
@ -340,7 +340,7 @@ UP004.py:108:5: UP004 [*] Class `B` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
105 105 | class B( 105 105 | class B(
106 106 | # Comment on A. 106 106 | # Comment on A.
107 107 | A, 107 107 | A,
@ -349,25 +349,6 @@ UP004.py:108:5: UP004 [*] Class `B` inherits from `object`
110 109 | ... 110 109 | ...
111 110 | 111 110 |
UP004.py:114:13: UP004 [*] Class `A` inherits from `object`
|
113 | def f():
114 | class A(object):
| ^^^^^^ UP004
115 | ...
|
= help: Remove `object` inheritance
Suggested fix
111 111 |
112 112 |
113 113 | def f():
114 |- class A(object):
114 |+ class A:
115 115 | ...
116 116 |
117 117 |
UP004.py:119:5: UP004 [*] Class `A` inherits from `object` UP004.py:119:5: UP004 [*] Class `A` inherits from `object`
| |
118 | class A( 118 | class A(
@ -378,7 +359,7 @@ UP004.py:119:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
115 115 | ... 115 115 | ...
116 116 | 116 116 |
117 117 | 117 117 |
@ -400,7 +381,7 @@ UP004.py:125:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
121 121 | ... 121 121 | ...
122 122 | 122 122 |
123 123 | 123 123 |
@ -422,7 +403,7 @@ UP004.py:131:5: UP004 [*] Class `A` inherits from `object`
| |
= help: Remove `object` inheritance = help: Remove `object` inheritance
Suggested fix Fix
127 127 | ... 127 127 | ...
128 128 | 128 128 |
129 129 | 129 129 |
@ -435,4 +416,78 @@ UP004.py:131:5: UP004 [*] Class `A` inherits from `object`
135 132 | 135 132 |
136 133 | 136 133 |
UP004.py:137:9: UP004 [*] Class `A` inherits from `object`
|
137 | class A(object, object):
| ^^^^^^ UP004
138 | ...
|
= help: Remove `object` inheritance
Fix
134 134 | ...
135 135 |
136 136 |
137 |-class A(object, object):
137 |+class A(object):
138 138 | ...
139 139 |
140 140 |
UP004.py:137:17: UP004 [*] Class `A` inherits from `object`
|
137 | class A(object, object):
| ^^^^^^ UP004
138 | ...
|
= help: Remove `object` inheritance
Fix
134 134 | ...
135 135 |
136 136 |
137 |-class A(object, object):
137 |+class A(object):
138 138 | ...
139 139 |
140 140 |
UP004.py:142:9: UP004 [*] Class `A` inherits from `object`
|
141 | @decorator()
142 | class A(object):
| ^^^^^^ UP004
143 | ...
|
= help: Remove `object` inheritance
Fix
139 139 |
140 140 |
141 141 | @decorator()
142 |-class A(object):
142 |+class A:
143 143 | ...
144 144 |
145 145 | @decorator() # class A(object):
UP004.py:146:9: UP004 [*] Class `A` inherits from `object`
|
145 | @decorator() # class A(object):
146 | class A(object):
| ^^^^^^ UP004
147 | ...
|
= help: Remove `object` inheritance
Fix
143 143 | ...
144 144 |
145 145 | @decorator() # class A(object):
146 |-class A(object):
146 |+class A:
147 147 | ...
148 148 |
149 149 |

View file

@ -56,4 +56,42 @@ UP039.py:12:9: UP039 [*] Unnecessary parentheses after class definition
14 14 | 14 14 |
15 15 | 15 15 |
UP039.py:17:8: UP039 [*] Unnecessary parentheses after class definition
|
16 | @decorator()
17 | class A():
| ^^ UP039
18 | pass
|
= help: Remove parentheses
Fix
14 14 |
15 15 |
16 16 | @decorator()
17 |-class A():
17 |+class A:
18 18 | pass
19 19 |
20 20 | @decorator
UP039.py:21:8: UP039 [*] Unnecessary parentheses after class definition
|
20 | @decorator
21 | class A():
| ^^ UP039
22 | pass
|
= help: Remove parentheses
Fix
18 18 | pass
19 19 |
20 20 | @decorator
21 |-class A():
21 |+class A:
22 22 | pass
23 23 |
24 24 | # OK