Compare commits

..

4 Commits

Author SHA1 Message Date
Charlie Marsh
e1b711d9c6 Bump version to 0.0.67 2022-10-10 12:55:04 -04:00
Charlie Marsh
35f593846e Implement B014 from flake8-bugbear (#392) 2022-10-10 12:53:42 -04:00
Charlie Marsh
c384fa513b Implement B025 from flake8-bugbear (#391) 2022-10-10 12:18:31 -04:00
Charlie Marsh
022ff64d29 Implement B011 from flake8-bugbear (#390) 2022-10-10 10:55:55 -04:00
19 changed files with 516 additions and 42 deletions

2
Cargo.lock generated
View File

@@ -1907,7 +1907,7 @@ dependencies = [
[[package]]
name = "ruff"
version = "0.0.66"
version = "0.0.67"
dependencies = [
"anyhow",
"bincode",

View File

@@ -1,6 +1,6 @@
[package]
name = "ruff"
version = "0.0.66"
version = "0.0.67"
edition = "2021"
[lib]

View File

@@ -215,7 +215,8 @@ ruff also implements some of the most popular Flake8 plugins natively, including
- [`flake8-builtins`](https://pypi.org/project/flake8-builtins/)
- [`flake8-super`](https://pypi.org/project/flake8-super/)
- [`flake8-print`](https://pypi.org/project/flake8-print/)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) (partial)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) (11/16)
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (3/32)
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (partial)
Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis Flake8:
@@ -278,6 +279,9 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
| A001 | BuiltinVariableShadowing | Variable `...` is shadowing a python builtin | | |
| A002 | BuiltinArgumentShadowing | Argument `...` is shadowing a python builtin | | |
| A003 | BuiltinAttributeShadowing | Class attribute `...` is shadowing a python builtin | | |
| B011 | DoNotAssertFalse | Do not `assert False` (`python -O` removes these calls), raise `AssertionError()` | | 🛠 |
| B014 | DuplicateHandlerException | Exception handler with duplicate exception `Exception` | | |
| B025 | DuplicateTryBlockException | try-except block with duplicate exception `Exception` | | |
| C400 | UnnecessaryGeneratorList | Unnecessary generator - rewrite as a list comprehension | | |
| C401 | UnnecessaryGeneratorSet | Unnecessary generator - rewrite as a set comprehension | | |
| C402 | UnnecessaryGeneratorDict | Unnecessary generator - rewrite as a dict comprehension | | |
@@ -295,7 +299,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
| U002 | UnnecessaryAbspath | `abspath(__file__)` is unnecessary in Python 3.9 and later | | 🛠 |
| U003 | TypeOfPrimitive | Use `str` instead of `type(...)` | | 🛠 |
| U004 | UselessObjectInheritance | Class `...` inherits from object | | 🛠 |
| U005 | NoAssertEquals | `assertEquals` is deprecated, use `assertEqual` instead | | 🛠 |
| U005 | DeprecatedUnittestAlias | `assertEquals` is deprecated, use `assertEqual` instead | | 🛠 |
| U006 | UsePEP585Annotation | Use `list` instead of `List` for type annotations | | 🛠 |
| U007 | UsePEP604Annotation | Use `X \| Y` for type annotations | | 🛠 |
| U008 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | | 🛠 |

10
resources/test/fixtures/B011.py vendored Normal file
View File

@@ -0,0 +1,10 @@
"""
Should emit:
B011 - on line 8
B011 - on line 10
"""
assert 1 != 2
assert False
assert 1 != 2, "message"
assert False, "message"

76
resources/test/fixtures/B014.py vendored Normal file
View File

@@ -0,0 +1,76 @@
"""
Should emit:
B014 - on lines 11, 17, 28, 42, 49, 56, and 74.
"""
import binascii
import re
try:
pass
except (Exception, TypeError):
# TypeError is a subclass of Exception, so it doesn't add anything
pass
try:
pass
except (OSError, OSError) as err:
# Duplicate exception types are useless
pass
class MyError(Exception):
pass
try:
pass
except (MyError, MyError):
# Detect duplicate non-builtin errors
pass
try:
pass
except (MyError, Exception) as e:
# Don't assume that we're all subclasses of Exception
pass
try:
pass
except (MyError, BaseException) as e:
# But we *can* assume that everything is a subclass of BaseException
pass
try:
pass
except (re.error, re.error):
# Duplicate exception types as attributes
pass
try:
pass
except (IOError, EnvironmentError, OSError):
# Detect if a primary exception and any its aliases are present.
#
# Since Python 3.3, IOError, EnvironmentError, WindowsError, mmap.error,
# socket.error and select.error are aliases of OSError. See PEP 3151 for
# more info.
pass
try:
pass
except (MyException, NotImplemented):
# NotImplemented is not an exception, let's not crash on it.
pass
try:
pass
except (ValueError, binascii.Error):
# binascii.Error is a subclass of ValueError.
pass

38
resources/test/fixtures/B025.py vendored Normal file
View File

@@ -0,0 +1,38 @@
"""
Should emit:
B025 - on lines 15, 22, 31
"""
import pickle
try:
a = 1
except ValueError:
a = 2
finally:
a = 3
try:
a = 1
except ValueError:
a = 2
except ValueError:
a = 2
try:
a = 1
except pickle.PickleError:
a = 2
except ValueError:
a = 2
except pickle.PickleError:
a = 2
try:
a = 1
except (ValueError, TypeError):
a = 2
except ValueError:
a = 2
except (OSError, TypeError):
a = 2

View File

@@ -9,6 +9,7 @@ use rustpython_parser::ast::{
};
use serde::{Deserialize, Serialize};
use crate::ast::helpers;
use crate::ast::types::{
Binding, BindingKind, CheckLocator, FunctionScope, Range, Scope, ScopeKind,
};
@@ -747,16 +748,6 @@ pub fn check_builtin_shadowing(
}
}
/// Returns `true` if a call is an argumented `super` invocation.
pub fn is_super_call_with_arguments(func: &Expr, args: &Vec<Expr>) -> bool {
// Check: is this a `super` call?
if let ExprKind::Name { id, .. } = &func.node {
id == "super" && !args.is_empty()
} else {
false
}
}
// flake8-comprehensions
/// Check `list(generator)` compliance.
pub fn unnecessary_generator_list(expr: &Expr, func: &Expr, args: &Vec<Expr>) -> Option<Check> {
@@ -1076,7 +1067,7 @@ pub fn check_super_args(
func: &Expr,
args: &Vec<Expr>,
) -> Option<Check> {
if !is_super_call_with_arguments(func, args) {
if !helpers::is_super_call_with_arguments(func, args) {
return None;
}

View File

@@ -4,7 +4,39 @@ use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprKind, StmtKind}
use crate::python::typing;
static DUNDER_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"__[^\s]+__").unwrap());
fn compose_call_path_inner<'a>(expr: &'a Expr, parts: &mut Vec<&'a str>) {
match &expr.node {
ExprKind::Call { func, .. } => {
compose_call_path_inner(func, parts);
}
ExprKind::Attribute { value, attr, .. } => {
compose_call_path_inner(value, parts);
parts.push(attr);
}
ExprKind::Name { id, .. } => {
parts.push(id);
}
_ => {}
}
}
pub fn compose_call_path(expr: &Expr) -> Option<String> {
let mut segments = vec![];
compose_call_path_inner(expr, &mut segments);
if segments.is_empty() {
None
} else {
Some(segments.join("."))
}
}
pub fn match_name_or_attr(expr: &Expr, target: &str) -> bool {
match &expr.node {
ExprKind::Attribute { attr, .. } => target == attr,
ExprKind::Name { id, .. } => target == id,
_ => false,
}
}
pub enum SubscriptKind {
AnnotatedSubscript,
@@ -35,21 +67,7 @@ pub fn match_annotated_subscript(expr: &Expr) -> Option<SubscriptKind> {
}
}
fn node_name(expr: &Expr) -> Option<&str> {
if let ExprKind::Name { id, .. } = &expr.node {
Some(id)
} else {
None
}
}
pub fn match_name_or_attr(expr: &Expr, target: &str) -> bool {
match &expr.node {
ExprKind::Attribute { attr, .. } => target == attr,
ExprKind::Name { id, .. } => target == id,
_ => false,
}
}
static DUNDER_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"__[^\s]+__").unwrap());
pub fn is_assignment_to_a_dunder(node: &StmtKind) -> bool {
// Check whether it's an assignment to a dunder, with or without a type annotation.
@@ -82,9 +100,7 @@ pub fn is_assignment_to_a_dunder(node: &StmtKind) -> bool {
}
/// Extract the names of all handled exceptions.
/// Note that, for now, this only matches on ExprKind::Name, and so won't catch exceptions like
/// `module.CustomException`. (But will catch all builtin exceptions.)
pub fn extract_handler_names(handlers: &[Excepthandler]) -> Vec<&str> {
pub fn extract_handler_names(handlers: &[Excepthandler]) -> Vec<String> {
let mut handler_names = vec![];
for handler in handlers {
match &handler.node {
@@ -92,11 +108,11 @@ pub fn extract_handler_names(handlers: &[Excepthandler]) -> Vec<&str> {
if let Some(type_) = type_ {
if let ExprKind::Tuple { elts, .. } = &type_.node {
for type_ in elts {
if let Some(name) = node_name(type_) {
if let Some(name) = compose_call_path(type_) {
handler_names.push(name);
}
}
} else if let Some(name) = node_name(type_) {
} else if let Some(name) = compose_call_path(type_) {
handler_names.push(name);
}
}
@@ -105,3 +121,13 @@ pub fn extract_handler_names(handlers: &[Excepthandler]) -> Vec<&str> {
}
handler_names
}
/// Returns `true` if a call is an argumented `super` invocation.
pub fn is_super_call_with_arguments(func: &Expr, args: &Vec<Expr>) -> bool {
// Check: is this a `super` call?
if let ExprKind::Name { id, .. } = &func.node {
id == "super" && !args.is_empty()
} else {
false
}
}

View File

@@ -60,7 +60,7 @@ pub struct Checker<'a> {
seen_docstring: bool,
futures_allowed: bool,
annotations_future_enabled: bool,
except_handlers: Vec<Vec<&'a str>>,
except_handlers: Vec<Vec<String>>,
}
impl<'a> Checker<'a> {
@@ -548,10 +548,13 @@ where
plugins::if_tuple(self, stmt, test);
}
}
StmtKind::Assert { test, .. } => {
StmtKind::Assert { test, msg } => {
if self.settings.enabled.contains(&CheckCode::F631) {
plugins::assert_tuple(self, stmt, test);
}
if self.settings.enabled.contains(&CheckCode::B011) {
plugins::assert_false(self, stmt, test, msg);
}
}
StmtKind::Try { handlers, .. } => {
if self.settings.enabled.contains(&CheckCode::F707) {
@@ -559,6 +562,11 @@ where
self.checks.push(check);
}
}
if self.settings.enabled.contains(&CheckCode::B014)
|| self.settings.enabled.contains(&CheckCode::B025)
{
plugins::duplicate_exceptions(self, stmt, handlers);
}
}
StmtKind::Assign { targets, value, .. } => {
if self.settings.enabled.contains(&CheckCode::E731) {
@@ -1512,7 +1520,7 @@ impl<'a> Checker<'a> {
// Avoid flagging if NameError is handled.
if let Some(handler_names) = self.except_handlers.last() {
if handler_names.contains(&"NameError") {
if handler_names.contains(&"NameError".to_string()) {
return;
}
}

View File

@@ -121,6 +121,10 @@ pub enum CheckCode {
A001,
A002,
A003,
// flake8-bugbear
B011,
B014,
B025,
// flake8-comprehensions
C400,
C401,
@@ -213,6 +217,10 @@ pub enum CheckKind {
BuiltinVariableShadowing(String),
BuiltinArgumentShadowing(String),
BuiltinAttributeShadowing(String),
// flake8-bugbear
DoNotAssertFalse,
DuplicateHandlerException(String),
DuplicateTryBlockException(String),
// flake8-comprehensions
UnnecessaryGeneratorList,
UnnecessaryGeneratorSet,
@@ -306,6 +314,10 @@ impl CheckCode {
CheckCode::A001 => CheckKind::BuiltinVariableShadowing("...".to_string()),
CheckCode::A002 => CheckKind::BuiltinArgumentShadowing("...".to_string()),
CheckCode::A003 => CheckKind::BuiltinAttributeShadowing("...".to_string()),
// flake8-bugbear
CheckCode::B011 => CheckKind::DoNotAssertFalse,
CheckCode::B014 => CheckKind::DuplicateHandlerException("Exception".to_string()),
CheckCode::B025 => CheckKind::DuplicateTryBlockException("Exception".to_string()),
// flake8-comprehensions
CheckCode::C400 => CheckKind::UnnecessaryGeneratorList,
CheckCode::C401 => CheckKind::UnnecessaryGeneratorSet,
@@ -400,6 +412,10 @@ impl CheckKind {
CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001,
CheckKind::BuiltinArgumentShadowing(_) => &CheckCode::A002,
CheckKind::BuiltinAttributeShadowing(_) => &CheckCode::A003,
// flake8-bugbear
CheckKind::DoNotAssertFalse => &CheckCode::B011,
CheckKind::DuplicateHandlerException(_) => &CheckCode::B014,
CheckKind::DuplicateTryBlockException(_) => &CheckCode::B025,
// flake8-comprehensions
CheckKind::UnnecessaryGeneratorList => &CheckCode::C400,
CheckKind::UnnecessaryGeneratorSet => &CheckCode::C401,
@@ -572,6 +588,17 @@ impl CheckKind {
CheckKind::BuiltinAttributeShadowing(name) => {
format!("Class attribute `{name}` is shadowing a python builtin")
}
// flake8-bugbear
CheckKind::DoNotAssertFalse => {
"Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`"
.to_string()
}
CheckKind::DuplicateHandlerException(name) => {
format!("Exception handler with duplicate exception `{name}`")
}
CheckKind::DuplicateTryBlockException(name) => {
format!("try-except block with duplicate exception `{name}`")
}
// flake8-comprehensions
CheckKind::UnnecessaryGeneratorList => {
"Unnecessary generator - rewrite as a list comprehension".to_string()
@@ -675,6 +702,7 @@ impl CheckKind {
matches!(
self,
CheckKind::DeprecatedUnittestAlias(_, _)
| CheckKind::DoNotAssertFalse
| CheckKind::PPrintFound
| CheckKind::PrintFound
| CheckKind::SuperCallWithParameters

View File

@@ -115,7 +115,7 @@ impl SourceGenerator {
Ok(())
}
fn unparse_stmt<U>(&mut self, ast: &Stmt<U>) -> fmt::Result {
pub fn unparse_stmt<U>(&mut self, ast: &Stmt<U>) -> fmt::Result {
macro_rules! statement {
($body:block) => {{
self.newline()?;

View File

@@ -810,6 +810,42 @@ mod tests {
Ok(())
}
#[test]
fn b011() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/B011.py"),
&settings::Settings::for_rule(CheckCode::B011),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn b014() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/B014.py"),
&settings::Settings::for_rule(CheckCode::B014),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn b025() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/B025.py"),
&settings::Settings::for_rule(CheckCode::B025),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn c400() -> Result<()> {
let mut checks = check_path(

View File

@@ -1,5 +1,7 @@
pub use assert_false::assert_false;
pub use assert_tuple::assert_tuple;
pub use deprecated_unittest_alias::deprecated_unittest_alias;
pub use duplicate_exceptions::duplicate_exceptions;
pub use if_tuple::if_tuple;
pub use invalid_print_syntax::invalid_print_syntax;
pub use print_call::print_call;
@@ -11,8 +13,10 @@ pub use use_pep604_annotation::use_pep604_annotation;
pub use useless_metaclass_type::useless_metaclass_type;
pub use useless_object_inheritance::useless_object_inheritance;
mod assert_false;
mod assert_tuple;
mod deprecated_unittest_alias;
mod duplicate_exceptions;
mod if_tuple;
mod invalid_print_syntax;
mod print_call;

View File

@@ -0,0 +1,61 @@
use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Stmt, StmtKind};
use crate::ast::types::Range;
use crate::autofix::fixer;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind, Fix};
use crate::code_gen::SourceGenerator;
fn assertion_error(msg: &Option<Box<Expr>>) -> Stmt {
Stmt::new(
Default::default(),
Default::default(),
StmtKind::Raise {
exc: Some(Box::new(Expr::new(
Default::default(),
Default::default(),
ExprKind::Call {
func: Box::new(Expr::new(
Default::default(),
Default::default(),
ExprKind::Name {
id: "AssertionError".to_string(),
ctx: ExprContext::Load,
},
)),
args: if let Some(msg) = msg {
vec![*msg.clone()]
} else {
vec![]
},
keywords: vec![],
},
))),
cause: None,
},
)
}
pub fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg: &Option<Box<Expr>>) {
if let ExprKind::Constant {
value: Constant::Bool(false),
..
} = &test.node
{
let mut check = Check::new(CheckKind::DoNotAssertFalse, Range::from_located(test));
if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) {
let mut generator = SourceGenerator::new();
if let Ok(()) = generator.unparse_stmt(&assertion_error(msg)) {
if let Ok(content) = generator.generate() {
check.amend(Fix {
content,
location: stmt.location,
end_location: stmt.end_location,
applied: false,
})
}
}
}
checker.add_check(check);
}
}

View File

@@ -0,0 +1,82 @@
use itertools::Itertools;
use std::collections::BTreeSet;
use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt};
use crate::ast::helpers;
use crate::ast::types::{CheckLocator, Range};
use crate::check_ast::Checker;
use crate::checks::{Check, CheckCode, CheckKind};
pub fn duplicate_handler_exceptions(
checker: &mut Checker,
stmt: &Stmt,
elts: &Vec<Expr>,
) -> BTreeSet<String> {
let mut seen: BTreeSet<String> = Default::default();
let mut duplicates: BTreeSet<String> = Default::default();
for type_ in elts {
if let Some(name) = helpers::compose_call_path(type_) {
if seen.contains(&name) {
duplicates.insert(name);
} else {
seen.insert(name);
}
}
}
if checker.settings.enabled.contains(&CheckCode::B014) {
// TODO(charlie): Handle "BaseException" and redundant exception aliases.
for duplicate in duplicates.into_iter().sorted() {
checker.add_check(Check::new(
CheckKind::DuplicateHandlerException(duplicate),
checker.locate_check(Range::from_located(stmt)),
));
}
}
seen
}
pub fn duplicate_exceptions(checker: &mut Checker, stmt: &Stmt, handlers: &[Excepthandler]) {
let mut seen: BTreeSet<String> = Default::default();
let mut duplicates: BTreeSet<String> = Default::default();
for handler in handlers {
match &handler.node {
ExcepthandlerKind::ExceptHandler { type_, .. } => {
if let Some(type_) = type_ {
match &type_.node {
ExprKind::Attribute { .. } | ExprKind::Name { .. } => {
if let Some(name) = helpers::compose_call_path(type_) {
if seen.contains(&name) {
duplicates.insert(name);
} else {
seen.insert(name);
}
}
}
ExprKind::Tuple { elts, .. } => {
for name in duplicate_handler_exceptions(checker, stmt, elts) {
if seen.contains(&name) {
duplicates.insert(name);
} else {
seen.insert(name);
}
}
}
_ => {}
}
}
}
}
}
if checker.settings.enabled.contains(&CheckCode::B025) {
for duplicate in duplicates.into_iter().sorted() {
checker.add_check(Check::new(
CheckKind::DuplicateTryBlockException(duplicate),
checker.locate_check(Range::from_located(stmt)),
));
}
}
}

View File

@@ -1,6 +1,6 @@
use rustpython_ast::{Expr, Stmt};
use crate::ast::checks;
use crate::ast::{checks, helpers};
use crate::autofix::{fixer, fixes};
use crate::check_ast::Checker;
@@ -12,7 +12,7 @@ pub fn super_call_with_parameters(
) {
// Only bother going through the super check at all if we're in a `super` call.
// (We check this in `check_super_args` too, so this is just an optimization.)
if checks::is_super_call_with_arguments(func, args) {
if helpers::is_super_call_with_arguments(func, args) {
let scope = checker.current_scope();
let parents: Vec<&Stmt> = checker
.parent_stack

View File

@@ -0,0 +1,37 @@
---
source: src/linter.rs
expression: checks
---
- kind: DoNotAssertFalse
location:
row: 8
column: 8
end_location:
row: 8
column: 13
fix:
content: raise AssertionError()
location:
row: 8
column: 1
end_location:
row: 8
column: 13
applied: false
- kind: DoNotAssertFalse
location:
row: 10
column: 8
end_location:
row: 10
column: 13
fix:
content: "raise AssertionError(\"message\")"
location:
row: 10
column: 1
end_location:
row: 10
column: 24
applied: false

View File

@@ -0,0 +1,32 @@
---
source: src/linter.rs
expression: checks
---
- kind:
DuplicateHandlerException: OSError
location:
row: 15
column: 1
end_location:
row: 22
column: 1
fix: ~
- kind:
DuplicateHandlerException: MyError
location:
row: 26
column: 1
end_location:
row: 33
column: 1
fix: ~
- kind:
DuplicateHandlerException: re.error
location:
row: 47
column: 1
end_location:
row: 54
column: 1
fix: ~

View File

@@ -0,0 +1,41 @@
---
source: src/linter.rs
expression: checks
---
- kind:
DuplicateTryBlockException: ValueError
location:
row: 15
column: 1
end_location:
row: 22
column: 1
fix: ~
- kind:
DuplicateTryBlockException: pickle.PickleError
location:
row: 22
column: 1
end_location:
row: 31
column: 1
fix: ~
- kind:
DuplicateTryBlockException: TypeError
location:
row: 31
column: 1
end_location:
row: 39
column: 1
fix: ~
- kind:
DuplicateTryBlockException: ValueError
location:
row: 31
column: 1
end_location:
row: 39
column: 1
fix: ~