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

New violation: Prefer TypeError for unexpected types #34

Merged
merged 3 commits into from
Nov 22, 2021
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 docs/violations/README.md
Expand Up @@ -7,6 +7,7 @@
| ----------------- | ---------------------------------------------------------- |
| [TC002](TC002.md) | Create your own exception |
| [TC003](TC003.md) | Avoid specifying long messages outside the exception class |
| [TC004](TC004.md) | Prefer `TypeError` exception for invalid type |

## `TC1xx` - General

Expand Down
31 changes: 31 additions & 0 deletions docs/violations/TC004.md
@@ -0,0 +1,31 @@
# `TC004` - Prefer `TypeError` exception for invalid type

## Why is it bad

Using semantically incorrect exceptions results in confusing diagnostic information for the user.
guilatrova marked this conversation as resolved.
Show resolved Hide resolved

The Python docs make the definition clear:

> Raised when an operation or function is applied to an object of inappropriate type. [...]
This exception may be raised by user code to indicate that an attempted operation on an object is not supported [...]
Passing arguments of the wrong type (e.g. passing a list when an int is expected) should result in a TypeError, but passing arguments with the wrong value (e.g. a number outside expected boundaries) should result in a ValueError.

[Source](https://docs.python.org/3/library/exceptions.html#TypeError)

## How it looks like

```py
if isinstance(my_var, int):
pass
else:
raise ValueError(f'{my_var} must be an int')
```

## How it should be

```py
if isinstance(my_var, int):
pass
else:
raise TypeError(f'{my_var} must be an int')
```
51 changes: 51 additions & 0 deletions src/tests/analyzers_conditional_test.py
@@ -0,0 +1,51 @@
from functools import partial

from tryceratops import analyzers
from tryceratops.violations import codes

from .analyzer_helpers import assert_violation, read_sample


def test_prefer_type_error():
tree = read_sample("conditional_prefer_type_error")
analyzer = analyzers.conditional.PreferTypeErrorAnalyzer()

assert_type_error = partial(
assert_violation, codes.PREFER_TYPE_ERROR[0], codes.PREFER_TYPE_ERROR[1]
)

violations = analyzer.check(tree, "filename")

assert len(violations) == 31

assert_type_error(12, 14, violations[0])
assert_type_error(19, 14, violations[1])
assert_type_error(30, 14, violations[2])
assert_type_error(37, 14, violations[3])
assert_type_error(44, 14, violations[4])
assert_type_error(51, 14, violations[5])
assert_type_error(58, 14, violations[6])
assert_type_error(65, 14, violations[7])
assert_type_error(72, 14, violations[8])
assert_type_error(79, 14, violations[9])
assert_type_error(86, 14, violations[10])
assert_type_error(93, 14, violations[11])
assert_type_error(100, 14, violations[12])
assert_type_error(107, 14, violations[13])
assert_type_error(114, 14, violations[14])
assert_type_error(121, 14, violations[15])
assert_type_error(128, 14, violations[16])
assert_type_error(135, 14, violations[17])
assert_type_error(142, 14, violations[18])
assert_type_error(149, 14, violations[19])
assert_type_error(156, 14, violations[20])
assert_type_error(163, 14, violations[21])
assert_type_error(170, 14, violations[22])
assert_type_error(177, 14, violations[23])
assert_type_error(184, 14, violations[24])
assert_type_error(191, 14, violations[25])
assert_type_error(198, 14, violations[26])
assert_type_error(205, 14, violations[27])
assert_type_error(212, 14, violations[28])
assert_type_error(219, 14, violations[29])
assert_type_error(226, 14, violations[30])
258 changes: 258 additions & 0 deletions src/tests/samples/violations/conditional_prefer_type_error.py
@@ -0,0 +1,258 @@
"""
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):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps parametrize all these tests using the exception types for simplicity

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:
raise MemoryError("...") # should be typeerror


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
guilatrova marked this conversation as resolved.
Show resolved Hide resolved


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
guilatrova marked this conversation as resolved.
Show resolved Hide resolved


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
guilatrova marked this conversation as resolved.
Show resolved Hide resolved


def correct_should_ignore(some_arg):
if isinstance(some_arg, int):
pass
else:
raise TypeError("...")
3 changes: 2 additions & 1 deletion src/tryceratops/analyzers/__init__.py
Expand Up @@ -2,7 +2,7 @@

from typing import TYPE_CHECKING, Set

from . import call, exception_block, try_block
from . import call, conditional, exception_block, try_block
from .base import BaseAnalyzer

if TYPE_CHECKING:
Expand All @@ -14,6 +14,7 @@
call.CallRaiseVanillaAnalyzer, # type: ignore
call.CallRaiseLongArgsAnalyzer, # type: ignore
call.CallAvoidCheckingToContinueAnalyzer, # type: ignore
conditional.PreferTypeErrorAnalyzer,
exception_block.ExceptReraiseWithoutCauseAnalyzer,
exception_block.ExceptVerboseReraiseAnalyzer,
exception_block.ExceptBroadPassAnalyzer,
Expand Down