Implement __metaclass__ = type removal (#324)

This commit is contained in:
Charlie Marsh 2022-10-04 14:31:52 -04:00 committed by GitHub
parent 4e6ae33a3a
commit fdb32330a9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 165 additions and 26 deletions

View file

@ -213,6 +213,7 @@ ruff also implements some of the most popular Flake8 plugins natively, including
- [`flake8-builtins`](https://pypi.org/project/flake8-builtins/) - [`flake8-builtins`](https://pypi.org/project/flake8-builtins/)
- [`flake8-super`](https://pypi.org/project/flake8-super/) - [`flake8-super`](https://pypi.org/project/flake8-super/)
- [`flake8-print`](https://pypi.org/project/flake8-print/) - [`flake8-print`](https://pypi.org/project/flake8-print/)
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (partial)
Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis Flake8: Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis Flake8:
@ -276,6 +277,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
| SPR001 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | | 🛠 | | SPR001 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | | 🛠 |
| T201 | PrintFound | `print` found | | 🛠 | | T201 | PrintFound | `print` found | | 🛠 |
| T203 | PPrintFound | `pprint` found | | 🛠 | | T203 | PPrintFound | `pprint` found | | 🛠 |
| U001 | UselessMetaclassType | `__metaclass__ = type` is implied | | 🛠 |
| R001 | UselessObjectInheritance | Class `...` inherits from object | | 🛠 | | R001 | UselessObjectInheritance | Class `...` inherits from object | | 🛠 |
| R002 | NoAssertEquals | `assertEquals` is deprecated, use `assertEqual` instead | | 🛠 | | R002 | NoAssertEquals | `assertEquals` is deprecated, use `assertEqual` instead | | 🛠 |
| M001 | UnusedNOQA | Unused `noqa` directive | | 🛠 | | M001 | UnusedNOQA | Unused `noqa` directive | | 🛠 |

13
resources/test/fixtures/U001.py vendored Normal file
View file

@ -0,0 +1,13 @@
class A:
__metaclass__ = type
class B:
__metaclass__ = type
def __init__(self) -> None:
pass
class C(metaclass=type):
pass

View file

@ -116,6 +116,26 @@ pub fn check_do_not_assign_lambda(value: &Expr, location: Range) -> Option<Check
} }
} }
/// Check UselessMetaclassType compliance.
pub fn check_useless_metaclass_type(
targets: &Vec<Expr>,
value: &Expr,
location: Range,
) -> Option<Check> {
if targets.len() == 1 {
if let ExprKind::Name { id, .. } = targets.first().map(|expr| &expr.node).unwrap() {
if id == "__metaclass__" {
if let ExprKind::Name { id, .. } = &value.node {
if id == "type" {
return Some(Check::new(CheckKind::UselessMetaclassType, location));
}
}
}
}
}
None
}
fn is_ambiguous_name(name: &str) -> bool { fn is_ambiguous_name(name: &str) -> bool {
name == "l" || name == "I" || name == "O" name == "l" || name == "I" || name == "O"
} }

View file

@ -19,7 +19,6 @@ use crate::ast::types::{
}; };
use crate::ast::visitor::{walk_excepthandler, Visitor}; use crate::ast::visitor::{walk_excepthandler, Visitor};
use crate::ast::{checks, operations, visitor}; use crate::ast::{checks, operations, visitor};
use crate::autofix::fixes::remove_stmt;
use crate::autofix::{fixer, fixes}; use crate::autofix::{fixer, fixes};
use crate::checks::{Check, CheckCode, CheckKind}; use crate::checks::{Check, CheckCode, CheckKind};
use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS}; use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS};
@ -622,7 +621,7 @@ where
} }
} }
} }
StmtKind::Assign { value, .. } => { StmtKind::Assign { targets, value, .. } => {
if self.settings.select.contains(&CheckCode::E731) { if self.settings.select.contains(&CheckCode::E731) {
if let Some(check) = checks::check_do_not_assign_lambda( if let Some(check) = checks::check_do_not_assign_lambda(
value, value,
@ -631,6 +630,37 @@ where
self.checks.push(check); self.checks.push(check);
} }
} }
if self.settings.select.contains(&CheckCode::U001) {
if let Some(mut check) = checks::check_useless_metaclass_type(
targets,
value,
self.locate_check(Range::from_located(stmt)),
) {
if matches!(self.autofix, fixer::Mode::Generate | fixer::Mode::Apply) {
let context = self.binding_context();
let deleted: Vec<&Stmt> = self
.deletions
.iter()
.map(|index| self.parents[*index])
.collect();
match fixes::remove_stmt(
self.parents[context.defined_by],
context.defined_in.map(|index| self.parents[index]),
&deleted,
) {
Ok(fix) => {
if fix.content.is_empty() || fix.content == "pass" {
self.deletions.insert(context.defined_by);
}
check.amend(fix)
}
Err(e) => error!("Failed to fix unused imports: {}", e),
}
}
self.checks.push(check);
}
}
} }
StmtKind::AnnAssign { value, .. } => { StmtKind::AnnAssign { value, .. } => {
if self.settings.select.contains(&CheckCode::E731) { if self.settings.select.contains(&CheckCode::E731) {
@ -797,7 +827,7 @@ where
.map(|index| self.parents[*index]) .map(|index| self.parents[*index])
.collect(); .collect();
match remove_stmt( match fixes::remove_stmt(
self.parents[context.defined_by], self.parents[context.defined_by],
context.defined_in.map(|index| self.parents[index]), context.defined_in.map(|index| self.parents[index]),
&deleted, &deleted,

View file

@ -53,7 +53,7 @@ pub const DEFAULT_CHECK_CODES: [CheckCode; 42] = [
CheckCode::F901, CheckCode::F901,
]; ];
pub const ALL_CHECK_CODES: [CheckCode; 51] = [ pub const ALL_CHECK_CODES: [CheckCode; 52] = [
// pycodestyle // pycodestyle
CheckCode::E402, CheckCode::E402,
CheckCode::E501, CheckCode::E501,
@ -107,11 +107,13 @@ pub const ALL_CHECK_CODES: [CheckCode; 51] = [
// flake8-print // flake8-print
CheckCode::T201, CheckCode::T201,
CheckCode::T203, CheckCode::T203,
// Meta // pyupgrade
CheckCode::M001, CheckCode::U001,
// Refactor // Refactor
CheckCode::R001, CheckCode::R001,
CheckCode::R002, CheckCode::R002,
// Meta
CheckCode::M001,
]; ];
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, Hash, PartialOrd, Ord)] #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, Hash, PartialOrd, Ord)]
@ -169,6 +171,8 @@ pub enum CheckCode {
// flake8-print // flake8-print
T201, T201,
T203, T203,
// pyupgrade
U001,
// Refactor // Refactor
R001, R001,
R002, R002,
@ -231,6 +235,8 @@ impl FromStr for CheckCode {
"A003" => Ok(CheckCode::A003), "A003" => Ok(CheckCode::A003),
// flake8-super // flake8-super
"SPR001" => Ok(CheckCode::SPR001), "SPR001" => Ok(CheckCode::SPR001),
// pyupgrade
"U001" => Ok(CheckCode::U001),
// Refactor // Refactor
"R001" => Ok(CheckCode::R001), "R001" => Ok(CheckCode::R001),
"R002" => Ok(CheckCode::R002), "R002" => Ok(CheckCode::R002),
@ -297,6 +303,8 @@ impl CheckCode {
// flake8-print // flake8-print
CheckCode::T201 => "T201", CheckCode::T201 => "T201",
CheckCode::T203 => "T203", CheckCode::T203 => "T203",
// pyupgrade
CheckCode::U001 => "U001",
// Refactor // Refactor
CheckCode::R001 => "R001", CheckCode::R001 => "R001",
CheckCode::R002 => "R002", CheckCode::R002 => "R002",
@ -372,6 +380,8 @@ impl CheckCode {
// flake8-print // flake8-print
CheckCode::T201 => CheckKind::PrintFound, CheckCode::T201 => CheckKind::PrintFound,
CheckCode::T203 => CheckKind::PPrintFound, CheckCode::T203 => CheckKind::PPrintFound,
// pyupgrade
CheckCode::U001 => CheckKind::UselessMetaclassType,
// Refactor // Refactor
CheckCode::R001 => CheckKind::UselessObjectInheritance("...".to_string()), CheckCode::R001 => CheckKind::UselessObjectInheritance("...".to_string()),
CheckCode::R002 => CheckKind::NoAssertEquals, CheckCode::R002 => CheckKind::NoAssertEquals,
@ -439,6 +449,7 @@ pub enum CheckKind {
UnusedImport(String), UnusedImport(String),
UnusedNOQA(Option<String>), UnusedNOQA(Option<String>),
UnusedVariable(String), UnusedVariable(String),
UselessMetaclassType,
UselessObjectInheritance(String), UselessObjectInheritance(String),
YieldOutsideFunction, YieldOutsideFunction,
// flake8-builtin // flake8-builtin
@ -483,7 +494,6 @@ impl CheckKind {
CheckKind::ModuleImportNotAtTopOfFile => "ModuleImportNotAtTopOfFile", CheckKind::ModuleImportNotAtTopOfFile => "ModuleImportNotAtTopOfFile",
CheckKind::MultiValueRepeatedKeyLiteral => "MultiValueRepeatedKeyLiteral", CheckKind::MultiValueRepeatedKeyLiteral => "MultiValueRepeatedKeyLiteral",
CheckKind::MultiValueRepeatedKeyVariable(_) => "MultiValueRepeatedKeyVariable", CheckKind::MultiValueRepeatedKeyVariable(_) => "MultiValueRepeatedKeyVariable",
CheckKind::NoAssertEquals => "NoAssertEquals",
CheckKind::NoneComparison(_) => "NoneComparison", CheckKind::NoneComparison(_) => "NoneComparison",
CheckKind::NotInTest => "NotInTest", CheckKind::NotInTest => "NotInTest",
CheckKind::NotIsTest => "NotIsTest", CheckKind::NotIsTest => "NotIsTest",
@ -497,9 +507,7 @@ impl CheckKind {
CheckKind::UndefinedLocal(_) => "UndefinedLocal", CheckKind::UndefinedLocal(_) => "UndefinedLocal",
CheckKind::UndefinedName(_) => "UndefinedName", CheckKind::UndefinedName(_) => "UndefinedName",
CheckKind::UnusedImport(_) => "UnusedImport", CheckKind::UnusedImport(_) => "UnusedImport",
CheckKind::UnusedNOQA(_) => "UnusedNOQA",
CheckKind::UnusedVariable(_) => "UnusedVariable", CheckKind::UnusedVariable(_) => "UnusedVariable",
CheckKind::UselessObjectInheritance(_) => "UselessObjectInheritance",
CheckKind::YieldOutsideFunction => "YieldOutsideFunction", CheckKind::YieldOutsideFunction => "YieldOutsideFunction",
// flake8-builtins // flake8-builtins
CheckKind::BuiltinVariableShadowing(_) => "BuiltinVariableShadowing", CheckKind::BuiltinVariableShadowing(_) => "BuiltinVariableShadowing",
@ -510,6 +518,13 @@ impl CheckKind {
// flake8-print // flake8-print
CheckKind::PrintFound => "PrintFound", CheckKind::PrintFound => "PrintFound",
CheckKind::PPrintFound => "PPrintFound", CheckKind::PPrintFound => "PPrintFound",
// pyupgrade
CheckKind::UselessMetaclassType => "UselessMetaclassType",
// Refactor
CheckKind::NoAssertEquals => "NoAssertEquals",
CheckKind::UselessObjectInheritance(_) => "UselessObjectInheritance",
// Meta
CheckKind::UnusedNOQA(_) => "UnusedNOQA",
} }
} }
@ -542,7 +557,6 @@ impl CheckKind {
CheckKind::ModuleImportNotAtTopOfFile => &CheckCode::E402, CheckKind::ModuleImportNotAtTopOfFile => &CheckCode::E402,
CheckKind::MultiValueRepeatedKeyLiteral => &CheckCode::F601, CheckKind::MultiValueRepeatedKeyLiteral => &CheckCode::F601,
CheckKind::MultiValueRepeatedKeyVariable(_) => &CheckCode::F602, CheckKind::MultiValueRepeatedKeyVariable(_) => &CheckCode::F602,
CheckKind::NoAssertEquals => &CheckCode::R002,
CheckKind::NoneComparison(_) => &CheckCode::E711, CheckKind::NoneComparison(_) => &CheckCode::E711,
CheckKind::NotInTest => &CheckCode::E713, CheckKind::NotInTest => &CheckCode::E713,
CheckKind::NotIsTest => &CheckCode::E714, CheckKind::NotIsTest => &CheckCode::E714,
@ -557,9 +571,7 @@ impl CheckKind {
CheckKind::UndefinedLocal(_) => &CheckCode::F823, CheckKind::UndefinedLocal(_) => &CheckCode::F823,
CheckKind::UndefinedName(_) => &CheckCode::F821, CheckKind::UndefinedName(_) => &CheckCode::F821,
CheckKind::UnusedImport(_) => &CheckCode::F401, CheckKind::UnusedImport(_) => &CheckCode::F401,
CheckKind::UnusedNOQA(_) => &CheckCode::M001,
CheckKind::UnusedVariable(_) => &CheckCode::F841, CheckKind::UnusedVariable(_) => &CheckCode::F841,
CheckKind::UselessObjectInheritance(_) => &CheckCode::R001,
CheckKind::YieldOutsideFunction => &CheckCode::F704, CheckKind::YieldOutsideFunction => &CheckCode::F704,
// flake8-builtins // flake8-builtins
CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001, CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001,
@ -570,6 +582,13 @@ impl CheckKind {
// flake8-print // flake8-print
CheckKind::PrintFound => &CheckCode::T201, CheckKind::PrintFound => &CheckCode::T201,
CheckKind::PPrintFound => &CheckCode::T203, CheckKind::PPrintFound => &CheckCode::T203,
// pyupgrade
CheckKind::UselessMetaclassType => &CheckCode::U001,
// Refactor
CheckKind::NoAssertEquals => &CheckCode::R002,
CheckKind::UselessObjectInheritance(_) => &CheckCode::R001,
// Meta
CheckKind::UnusedNOQA(_) => &CheckCode::M001,
} }
} }
@ -646,9 +665,6 @@ impl CheckKind {
CheckKind::MultiValueRepeatedKeyVariable(name) => { CheckKind::MultiValueRepeatedKeyVariable(name) => {
format!("Dictionary key `{name}` repeated") format!("Dictionary key `{name}` repeated")
} }
CheckKind::NoAssertEquals => {
"`assertEquals` is deprecated, use `assertEqual` instead".to_string()
}
CheckKind::NoneComparison(op) => match op { CheckKind::NoneComparison(op) => match op {
RejectedCmpop::Eq => "Comparison to `None` should be `cond is None`".to_string(), RejectedCmpop::Eq => "Comparison to `None` should be `cond is None`".to_string(),
RejectedCmpop::NotEq => { RejectedCmpop::NotEq => {
@ -700,16 +716,10 @@ impl CheckKind {
CheckKind::UnusedVariable(name) => { CheckKind::UnusedVariable(name) => {
format!("Local variable `{name}` is assigned to but never used") format!("Local variable `{name}` is assigned to but never used")
} }
CheckKind::UselessObjectInheritance(name) => {
format!("Class `{name}` inherits from object")
}
CheckKind::YieldOutsideFunction => { CheckKind::YieldOutsideFunction => {
"`yield` or `yield from` statement outside of a function/method".to_string() "`yield` or `yield from` statement outside of a function/method".to_string()
} }
CheckKind::UnusedNOQA(code) => match code {
None => "Unused `noqa` directive".to_string(),
Some(code) => format!("Unused `noqa` directive for: {code}"),
},
// flake8-builtins // flake8-builtins
CheckKind::BuiltinVariableShadowing(name) => { CheckKind::BuiltinVariableShadowing(name) => {
format!("Variable `{name}` is shadowing a python builtin") format!("Variable `{name}` is shadowing a python builtin")
@ -727,6 +737,20 @@ impl CheckKind {
// flake8-print // flake8-print
CheckKind::PrintFound => "`print` found".to_string(), CheckKind::PrintFound => "`print` found".to_string(),
CheckKind::PPrintFound => "`pprint` found".to_string(), CheckKind::PPrintFound => "`pprint` found".to_string(),
// pyupgrade
CheckKind::UselessMetaclassType => "`__metaclass__ = type` is implied".to_string(),
// Refactor
CheckKind::NoAssertEquals => {
"`assertEquals` is deprecated, use `assertEqual` instead".to_string()
}
CheckKind::UselessObjectInheritance(name) => {
format!("Class `{name}` inherits from object")
}
// Meta
CheckKind::UnusedNOQA(code) => match code {
None => "Unused `noqa` directive".to_string(),
Some(code) => format!("Unused `noqa` directive for: {code}"),
},
} }
} }
@ -735,12 +759,13 @@ impl CheckKind {
matches!( matches!(
self, self,
CheckKind::NoAssertEquals CheckKind::NoAssertEquals
| CheckKind::UselessObjectInheritance(_) | CheckKind::PPrintFound
| CheckKind::UnusedNOQA(_) | CheckKind::PrintFound
| CheckKind::SuperCallWithParameters | CheckKind::SuperCallWithParameters
| CheckKind::UnusedImport(_) | CheckKind::UnusedImport(_)
| CheckKind::PrintFound | CheckKind::UnusedNOQA(_)
| CheckKind::PPrintFound | CheckKind::UselessMetaclassType
| CheckKind::UselessObjectInheritance(_)
) )
} }
} }

View file

@ -803,4 +803,16 @@ mod tests {
insta::assert_yaml_snapshot!(checks); insta::assert_yaml_snapshot!(checks);
Ok(()) Ok(())
} }
#[test]
fn u001() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/U001.py"),
&settings::Settings::for_rule(CheckCode::U001),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
} }

View file

@ -0,0 +1,37 @@
---
source: src/linter.rs
expression: checks
---
- kind: UselessMetaclassType
location:
row: 2
column: 5
end_location:
row: 2
column: 25
fix:
content: pass
location:
row: 2
column: 5
end_location:
row: 2
column: 25
applied: false
- kind: UselessMetaclassType
location:
row: 6
column: 5
end_location:
row: 6
column: 25
fix:
content: ""
location:
row: 6
column: 1
end_location:
row: 7
column: 1
applied: false