feat: implementation for TRY004 (#2066)

See: #2056.
This commit is contained in:
Simon Brugman
2023-01-21 20:58:59 +01:00
committed by GitHub
parent b1bda0de82
commit 67de8ac85e
9 changed files with 1058 additions and 0 deletions

View File

@@ -1194,6 +1194,7 @@ For more, see [tryceratops](https://pypi.org/project/tryceratops/1.1.0/) on PyPI
| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| TRY004 | prefer-type-error | Prefer `TypeError` exception for invalid type | |
| TRY300 | try-consider-else | Consider `else` block | |
### Ruff-specific rules (RUF)

View File

@@ -0,0 +1,296 @@
"""
Violation:
Prefer TypeError when relevant.
"""
def incorrect_basic(some_arg):
if isinstance(some_arg, int):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_multiple_type_check(some_arg):
if isinstance(some_arg, (int, str)):
pass
else:
raise Exception("...") # should be typeerror
class MyClass:
pass
def incorrect_with_issubclass(some_arg):
if issubclass(some_arg, MyClass):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_with_callable(some_arg):
if callable(some_arg):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_ArithmeticError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise ArithmeticError("...") # should be typeerror
def incorrect_AssertionError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise AssertionError("...") # should be typeerror
def incorrect_AttributeError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise AttributeError("...") # should be typeerror
def incorrect_BufferError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise BufferError # should be typeerror
def incorrect_EOFError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise EOFError("...") # should be typeerror
def incorrect_ImportError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise ImportError("...") # should be typeerror
def incorrect_LookupError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise LookupError("...") # should be typeerror
def incorrect_MemoryError(some_arg):
if isinstance(some_arg, int):
pass
else:
# should be typeerror
# not multiline is on purpose for fix
raise MemoryError(
"..."
)
def incorrect_NameError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise NameError("...") # should be typeerror
def incorrect_ReferenceError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise ReferenceError("...") # should be typeerror
def incorrect_RuntimeError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise RuntimeError("...") # should be typeerror
def incorrect_SyntaxError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise SyntaxError("...") # should be typeerror
def incorrect_SystemError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise SystemError("...") # should be typeerror
def incorrect_ValueError(some_arg):
if isinstance(some_arg, int):
pass
else:
raise ValueError("...") # should be typeerror
def incorrect_not_operator_isinstance(some_arg):
if not isinstance(some_arg):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_and_operator_isinstance(arg1, arg2):
if isinstance(some_arg) and isinstance(arg2):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_or_operator_isinstance(arg1, arg2):
if isinstance(some_arg) or isinstance(arg2):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_multiple_operators_isinstance(arg1, arg2, arg3):
if not isinstance(arg1) and isinstance(arg2) or isinstance(arg3):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_not_operator_callable(some_arg):
if not callable(some_arg):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_and_operator_callable(arg1, arg2):
if callable(some_arg) and callable(arg2):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_or_operator_callable(arg1, arg2):
if callable(some_arg) or callable(arg2):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_multiple_operators_callable(arg1, arg2, arg3):
if not callable(arg1) and callable(arg2) or callable(arg3):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_not_operator_issubclass(some_arg):
if not issubclass(some_arg):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_and_operator_issubclass(arg1, arg2):
if issubclass(some_arg) and issubclass(arg2):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_or_operator_issubclass(arg1, arg2):
if issubclass(some_arg) or issubclass(arg2):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_multiple_operators_issubclass(arg1, arg2, arg3):
if not issubclass(arg1) and issubclass(arg2) or issubclass(arg3):
pass
else:
raise Exception("...") # should be typeerror
def incorrect_multi_conditional(arg1, arg2):
if isinstance(arg1, int):
pass
elif isinstance(arg2, int):
raise Exception("...") # should be typeerror
class MyCustomTypeValidation(Exception):
pass
def correct_custom_exception(some_arg):
if isinstance(some_arg, int):
pass
else:
raise MyCustomTypeValidation("...") # that's correct, because it's not vanilla
def correct_complex_conditional(val):
if val is not None and (not isinstance(val, int) or val < 0):
raise ValueError(...) # fine if this is not a TypeError
def correct_multi_conditional(some_arg):
if some_arg == 3:
pass
elif isinstance(some_arg, int):
pass
else:
raise Exception("...") # fine if this is not a TypeError
def correct_should_ignore(some_arg):
if isinstance(some_arg, int):
pass
else:
raise TypeError("...")
def check_body(some_args):
if isinstance(some_args, int):
raise ValueError("...") # should be typeerror
def check_body_correct(some_args):
if isinstance(some_args, int):
raise TypeError("...") # correct
def multiple_elifs(some_args):
if not isinstance(some_args, int):
raise ValueError("...") # should be typerror
elif some_args < 3:
raise ValueError("...") # this is ok
elif some_args > 10:
raise ValueError("...") # this is ok if we don't simplify
else:
pass
def multiple_ifs(some_args):
if not isinstance(some_args, int):
raise ValueError("...") # should be typerror
else:
if some_args < 3:
raise ValueError("...") # this is ok
else:
if some_args > 10:
raise ValueError("...") # this is ok if we don't simplify
else:
pass

View File

@@ -1748,6 +1748,9 @@
"TID251",
"TID252",
"TRY",
"TRY0",
"TRY00",
"TRY004",
"TRY3",
"TRY30",
"TRY300",

View File

@@ -1391,6 +1391,15 @@ where
self.current_stmt_parent().map(std::convert::Into::into),
);
}
if self.settings.rules.enabled(&Rule::PreferTypeError) {
tryceratops::rules::prefer_type_error(
self,
body,
test,
orelse,
self.current_stmt_parent().map(Into::into),
);
}
}
StmtKind::Assert { test, msg } => {
if self.settings.rules.enabled(&Rule::AssertTuple) {

View File

@@ -428,6 +428,7 @@ ruff_macros::define_rule_mapping!(
// flake8-type-checking
TYP005 => rules::flake8_type_checking::rules::EmptyTypeCheckingBlock,
// tryceratops
TRY004 => rules::tryceratops::rules::PreferTypeError,
TRY300 => rules::tryceratops::rules::TryConsiderElse,
// Ruff
RUF001 => violations::AmbiguousUnicodeCharacterString,

View File

@@ -13,6 +13,7 @@ mod tests {
use crate::registry::Rule;
use crate::settings;
#[test_case(Rule::PreferTypeError, Path::new("TRY004.py"); "TRY004")]
#[test_case(Rule::TryConsiderElse, Path::new("TRY300.py"); "TRY300")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());

View File

@@ -1,3 +1,5 @@
pub use prefer_type_error::{prefer_type_error, PreferTypeError};
pub use try_consider_else::{try_consider_else, TryConsiderElse};
mod prefer_type_error;
mod try_consider_else;

View File

@@ -0,0 +1,162 @@
use ruff_macros::derive_message_formats;
use rustpython_ast::{Expr, ExprKind, Stmt, StmtKind};
use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::define_violation;
use crate::fix::Fix;
use crate::registry::Diagnostic;
use crate::violation::Violation;
define_violation!(
pub struct PreferTypeError;
);
impl Violation for PreferTypeError {
#[derive_message_formats]
fn message(&self) -> String {
format!("Prefer `TypeError` exception for invalid type")
}
}
/// Returns `true` if an [`Expr`] is a call to check types.
fn check_type_check_call(checker: &mut Checker, call: &Expr) -> bool {
checker.resolve_call_path(call).map_or(false, |call_path| {
call_path.as_slice() == ["", "isinstance"]
|| call_path.as_slice() == ["", "issubclass"]
|| call_path.as_slice() == ["", "callable"]
})
}
/// Returns `true` if an [`Expr`] is a test to check types (e.g. via isinstance)
fn check_type_check_test(checker: &mut Checker, test: &Expr) -> bool {
match &test.node {
ExprKind::BoolOp { values, .. } => values
.iter()
.all(|expr| check_type_check_test(checker, expr)),
ExprKind::UnaryOp { operand, .. } => check_type_check_test(checker, operand),
ExprKind::Call { .. } => check_type_check_call(checker, test),
_ => false,
}
}
/// Returns the [`Expr`] representing the name of the exception.
fn match_name(exc: &Expr) -> Option<&Expr> {
match &exc.node {
ExprKind::Name { .. } => Some(exc),
ExprKind::Call { func, .. } => {
if let ExprKind::Name { .. } = &func.node {
Some(func)
} else {
None
}
}
_ => None,
}
}
/// Returns `true` if `exc` is a reference to a builtin exception.
fn is_builtin_exception(checker: &mut Checker, exc: &Expr) -> bool {
return checker.resolve_call_path(exc).map_or(false, |call_path| {
[
"ArithmeticError",
"AssertionError",
"AttributeError",
"BufferError",
"EOFError",
"Exception",
"ImportError",
"LookupError",
"MemoryError",
"NameError",
"ReferenceError",
"RuntimeError",
"SyntaxError",
"SystemError",
"ValueError",
]
.iter()
.any(|target| call_path.as_slice() == ["", target])
});
}
/// Returns `true` if an [`Expr`] is a reference to a builtin exception.
fn check_raise_type(checker: &mut Checker, exc: &Expr) -> bool {
match &exc.node {
ExprKind::Name { .. } => is_builtin_exception(checker, exc),
ExprKind::Call { func, .. } => {
if let ExprKind::Name { .. } = &func.node {
is_builtin_exception(checker, func)
} else {
false
}
}
_ => false,
}
}
fn check_raise(checker: &mut Checker, exc: &Expr, item: &Stmt) {
if check_raise_type(checker, exc) {
let mut diagnostic = Diagnostic::new(PreferTypeError, Range::from_located(item));
if checker.patch(diagnostic.kind.rule()) {
if let Some(name) = match_name(exc) {
diagnostic.amend(Fix::replacement(
"TypeError".to_string(),
name.location,
name.end_location.unwrap(),
));
}
}
checker.diagnostics.push(diagnostic);
}
}
/// Search the body of an if-condition for raises.
fn check_body(checker: &mut Checker, func: &[Stmt]) {
for item in func {
if let StmtKind::Raise { exc: Some(exc), .. } = &item.node {
check_raise(checker, exc, item);
}
}
}
/// Search the orelse of an if-condition for raises.
fn check_orelse(checker: &mut Checker, func: &[Stmt]) {
for item in func {
match &item.node {
StmtKind::If { test, .. } => {
if !check_type_check_test(checker, test) {
return;
}
}
StmtKind::Raise { exc: Some(exc), .. } => {
check_raise(checker, exc, item);
}
_ => {}
}
}
}
/// TRY004
pub fn prefer_type_error(
checker: &mut Checker,
body: &[Stmt],
test: &Expr,
orelse: &[Stmt],
parent: Option<&Stmt>,
) {
if let Some(parent) = parent {
if let StmtKind::If { test, .. } = &parent.node {
if !check_type_check_test(checker, test) {
return;
}
}
}
// Only consider the body when the `if` condition is all type-related
if check_type_check_test(checker, test) {
check_body(checker, body);
check_orelse(checker, orelse);
}
}

View File

@@ -0,0 +1,583 @@
---
source: src/rules/tryceratops/mod.rs
expression: diagnostics
---
- kind:
PreferTypeError: ~
location:
row: 12
column: 8
end_location:
row: 12
column: 30
fix:
content: TypeError
location:
row: 12
column: 14
end_location:
row: 12
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 19
column: 8
end_location:
row: 19
column: 30
fix:
content: TypeError
location:
row: 19
column: 14
end_location:
row: 19
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 30
column: 8
end_location:
row: 30
column: 30
fix:
content: TypeError
location:
row: 30
column: 14
end_location:
row: 30
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 37
column: 8
end_location:
row: 37
column: 30
fix:
content: TypeError
location:
row: 37
column: 14
end_location:
row: 37
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 44
column: 8
end_location:
row: 44
column: 36
fix:
content: TypeError
location:
row: 44
column: 14
end_location:
row: 44
column: 29
parent: ~
- kind:
PreferTypeError: ~
location:
row: 51
column: 8
end_location:
row: 51
column: 35
fix:
content: TypeError
location:
row: 51
column: 14
end_location:
row: 51
column: 28
parent: ~
- kind:
PreferTypeError: ~
location:
row: 58
column: 8
end_location:
row: 58
column: 35
fix:
content: TypeError
location:
row: 58
column: 14
end_location:
row: 58
column: 28
parent: ~
- kind:
PreferTypeError: ~
location:
row: 65
column: 8
end_location:
row: 65
column: 25
fix:
content: TypeError
location:
row: 65
column: 14
end_location:
row: 65
column: 25
parent: ~
- kind:
PreferTypeError: ~
location:
row: 72
column: 8
end_location:
row: 72
column: 29
fix:
content: TypeError
location:
row: 72
column: 14
end_location:
row: 72
column: 22
parent: ~
- kind:
PreferTypeError: ~
location:
row: 79
column: 8
end_location:
row: 79
column: 32
fix:
content: TypeError
location:
row: 79
column: 14
end_location:
row: 79
column: 25
parent: ~
- kind:
PreferTypeError: ~
location:
row: 86
column: 8
end_location:
row: 86
column: 32
fix:
content: TypeError
location:
row: 86
column: 14
end_location:
row: 86
column: 25
parent: ~
- kind:
PreferTypeError: ~
location:
row: 95
column: 8
end_location:
row: 97
column: 9
fix:
content: TypeError
location:
row: 95
column: 14
end_location:
row: 95
column: 25
parent: ~
- kind:
PreferTypeError: ~
location:
row: 104
column: 8
end_location:
row: 104
column: 30
fix:
content: TypeError
location:
row: 104
column: 14
end_location:
row: 104
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 111
column: 8
end_location:
row: 111
column: 35
fix:
content: TypeError
location:
row: 111
column: 14
end_location:
row: 111
column: 28
parent: ~
- kind:
PreferTypeError: ~
location:
row: 118
column: 8
end_location:
row: 118
column: 33
fix:
content: TypeError
location:
row: 118
column: 14
end_location:
row: 118
column: 26
parent: ~
- kind:
PreferTypeError: ~
location:
row: 125
column: 8
end_location:
row: 125
column: 32
fix:
content: TypeError
location:
row: 125
column: 14
end_location:
row: 125
column: 25
parent: ~
- kind:
PreferTypeError: ~
location:
row: 132
column: 8
end_location:
row: 132
column: 32
fix:
content: TypeError
location:
row: 132
column: 14
end_location:
row: 132
column: 25
parent: ~
- kind:
PreferTypeError: ~
location:
row: 139
column: 8
end_location:
row: 139
column: 31
fix:
content: TypeError
location:
row: 139
column: 14
end_location:
row: 139
column: 24
parent: ~
- kind:
PreferTypeError: ~
location:
row: 146
column: 8
end_location:
row: 146
column: 30
fix:
content: TypeError
location:
row: 146
column: 14
end_location:
row: 146
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 153
column: 8
end_location:
row: 153
column: 30
fix:
content: TypeError
location:
row: 153
column: 14
end_location:
row: 153
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 160
column: 8
end_location:
row: 160
column: 30
fix:
content: TypeError
location:
row: 160
column: 14
end_location:
row: 160
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 167
column: 8
end_location:
row: 167
column: 30
fix:
content: TypeError
location:
row: 167
column: 14
end_location:
row: 167
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 174
column: 8
end_location:
row: 174
column: 30
fix:
content: TypeError
location:
row: 174
column: 14
end_location:
row: 174
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 181
column: 8
end_location:
row: 181
column: 30
fix:
content: TypeError
location:
row: 181
column: 14
end_location:
row: 181
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 188
column: 8
end_location:
row: 188
column: 30
fix:
content: TypeError
location:
row: 188
column: 14
end_location:
row: 188
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 195
column: 8
end_location:
row: 195
column: 30
fix:
content: TypeError
location:
row: 195
column: 14
end_location:
row: 195
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 202
column: 8
end_location:
row: 202
column: 30
fix:
content: TypeError
location:
row: 202
column: 14
end_location:
row: 202
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 209
column: 8
end_location:
row: 209
column: 30
fix:
content: TypeError
location:
row: 209
column: 14
end_location:
row: 209
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 216
column: 8
end_location:
row: 216
column: 30
fix:
content: TypeError
location:
row: 216
column: 14
end_location:
row: 216
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 223
column: 8
end_location:
row: 223
column: 30
fix:
content: TypeError
location:
row: 223
column: 14
end_location:
row: 223
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 230
column: 8
end_location:
row: 230
column: 30
fix:
content: TypeError
location:
row: 230
column: 14
end_location:
row: 230
column: 23
parent: ~
- kind:
PreferTypeError: ~
location:
row: 267
column: 8
end_location:
row: 267
column: 31
fix:
content: TypeError
location:
row: 267
column: 14
end_location:
row: 267
column: 24
parent: ~
- kind:
PreferTypeError: ~
location:
row: 277
column: 8
end_location:
row: 277
column: 31
fix:
content: TypeError
location:
row: 277
column: 14
end_location:
row: 277
column: 24
parent: ~
- kind:
PreferTypeError: ~
location:
row: 288
column: 8
end_location:
row: 288
column: 31
fix:
content: TypeError
location:
row: 288
column: 14
end_location:
row: 288
column: 24
parent: ~