Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implementation for TRY004 #2066

Merged
merged 4 commits into from
Jan 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
296 changes: 296 additions & 0 deletions resources/test/fixtures/tryceratops/TRY004.py
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1748,6 +1748,9 @@
"TID251",
"TID252",
"TRY",
"TRY0",
"TRY00",
"TRY004",
"TRY3",
"TRY30",
"TRY300",
Expand Down
9 changes: 9 additions & 0 deletions src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/rules/tryceratops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 2 additions & 0 deletions src/rules/tryceratops/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -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;