Skip to content

Commit

Permalink
ENH: Add SIM199 to detect dataclasses (#44)
Browse files Browse the repository at this point in the history
Closes #37
  • Loading branch information
MartinThoma authored Feb 28, 2021
1 parent ae8130f commit 9f74bde
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 9 deletions.
53 changes: 44 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,21 @@ Python-specific rules:

* `SIM101`: Multiple isinstance-calls which can be merged into a single call by
using a tuple as a second argument ([example](#SIM101))
* `SIM102`: Use a single if-statement instead of nested if-statements ([example](#SIM102))
* [`SIM103`](https://github.com/MartinThoma/flake8-simplify/issues/3): Return the boolean condition directly ([example](#SIM103))
* [`SIM104`](https://github.com/MartinThoma/flake8-simplify/issues/4): Use 'yield from iterable' (introduced in Python 3.3, see [PEP 380](https://docs.python.org/3/whatsnew/3.3.html#pep-380)) ([example](#SIM104))
* [`SIM104`](https://github.com/MartinThoma/flake8-simplify/issues/4) ![](https://shields.io/badge/-legacyfix-inactive): Use 'yield from iterable' (introduced in Python 3.3, see [PEP 380](https://docs.python.org/3/whatsnew/3.3.html#pep-380)) ([example](#SIM104))
* [`SIM105`](https://github.com/MartinThoma/flake8-simplify/issues/5): Use ['contextlib.suppress(...)'](https://docs.python.org/3/library/contextlib.html#contextlib.suppress) instead of try-except-pass ([example](#SIM105))
* [`SIM106`](https://github.com/MartinThoma/flake8-simplify/issues/8): Handle error-cases first ([example](#SIM106))
* [`SIM107`](https://github.com/MartinThoma/flake8-simplify/issues/9): Don't use `return` in try/except and finally ([example](#SIM107))
* [`SIM108`](https://github.com/MartinThoma/flake8-simplify/issues/12): Use the ternary operator if it's reasonable ([example](#SIM108))
* [`SIM109`](https://github.com/MartinThoma/flake8-simplify/issues/11): Use a list to compare a value against multiple values ([example](#SIM109))
* [`SIM110`](https://github.com/MartinThoma/flake8-simplify/issues/15): Use [any(...)](https://docs.python.org/3/library/functions.html#any) ([example](#SIM110))
* [`SIM111`](https://github.com/MartinThoma/flake8-simplify/issues/15): Use [all(...)](https://docs.python.org/3/library/functions.html#all) ([example](#SIM111))
* [`SIM112`](https://github.com/MartinThoma/flake8-simplify/issues/19): Use CAPITAL environment variables ([example](#SIM112))
* [`SIM113`](https://github.com/MartinThoma/flake8-simplify/issues/18): Use enumerate instead of manually incrementing a counter ([example](#SIM113))
* [`SIM114`](https://github.com/MartinThoma/flake8-simplify/issues/10): Combine conditions via a logical or to prevent duplicating code ([example](#SIM114))
* [`SIM115`](https://github.com/MartinThoma/flake8-simplify/issues/17): Use context handler for opening files ([example](#SIM115))
* [`SIM116`](https://github.com/MartinThoma/flake8-simplify/issues/31): Use a dictionary instead of many if/else equality checks ([example](#SIM116))
* [`SIM117`](https://github.com/MartinThoma/flake8-simplify/issues/35): Merge with-statements that use the same scope ([example](#SIM117))
* [`SIM118`](https://github.com/MartinThoma/flake8-simplify/issues/40): Use 'key in dict' instead of 'key in dict.keys()' ([example](#SIM118))
* [`SIM119`](https://github.com/MartinThoma/flake8-simplify/issues/37) ![](https://shields.io/badge/-legacyfix-inactive): Use dataclasses for data containers ([example](#SIM119))
* `SIM120` ![](https://shields.io/badge/-legacyfix-inactive): Use 'class FooBar:' instead of 'class FooBar(object):' ([example](#SIM120))

Comparations:

Expand All @@ -71,12 +69,18 @@ Comparations:
* [`SIM223`](https://github.com/MartinThoma/flake8-simplify/issues/6): Use 'False' instead of '... and False' ([example](#SIM223))
* [`SIM300`](https://github.com/MartinThoma/flake8-simplify/issues/16): Use 'age == 42' instead of '42 == age' ([example](#SIM300))

The `SIM201` - `SIM208` rules have one good reason to be ignored: When you are
checking an error condition:
General Code Style:

* `SIM102`: Use a single if-statement instead of nested if-statements ([example](#SIM102))
* [`SIM103`](https://github.com/MartinThoma/flake8-simplify/issues/3): Return the boolean condition directly ([example](#SIM103))
* [`SIM106`](https://github.com/MartinThoma/flake8-simplify/issues/8): Handle error-cases first ([example](#SIM106))
* [`SIM112`](https://github.com/MartinThoma/flake8-simplify/issues/19): Use CAPITAL environment variables ([example](#SIM112))

You might have good reasons to ignore some rules. To do that, use the standard Flake8 configuration. For example, within the `setup.cfg` file:

```python
if not correct_condition:
handle_error() # e.g. raise Exception
[flake8]
ignore = SIM106, SIM119
```


Expand Down Expand Up @@ -283,6 +287,37 @@ key in dict

Thank you for pointing this one out, [Aaron Gokaslan](https://github.com/Skylion007)!

### SIM119

Dataclasses were introduced with [PEP 557](https://www.python.org/dev/peps/pep-0557/)
in Python 3.7. The main reason not to use dataclasses is to support legacy Python versions.

Dataclasses create a lot of the boilerplate code for you:

* `__init__`
* `__eq__`
* `__hash__`
* `__str__`
* `__repr__`

A lot of projects use them:

* [black](https://github.com/psf/black/blob/master/src/black/__init__.py#L1472)

### SIM120

```
# Bad
class FooBar(object):
...
# Good
class FooBar:
...
```

Both notations are equivalent in Python 3, but the second one is shorter.

### SIM201

```python
Expand Down
91 changes: 91 additions & 0 deletions flake8_simplify.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def __init__(self, orig: ast.Call) -> None:
# Core Library
import importlib.metadata as importlib_metadata

# Python-Specific
SIM101 = (
"SIM101 Multiple isinstance-calls which can be merged into a single "
"call for variable '{var}'"
Expand Down Expand Up @@ -75,6 +76,12 @@ def __init__(self, orig: ast.Call) -> None:
)
SIM117 = "SIM117 Use '{merged_with}' instead of multiple with statements"
SIM118 = "SIM118 Use '{el} in {dict}' instead of '{el} in {dict}.keys()'"
SIM119 = "SIM119 Use a dataclass for 'class {classname}'"
SIM120 = (
"SIM120 Use 'class {classname}:' instead of 'class {classname}(object):'"
)

# Comparations
SIM201 = "SIM201 Use '{left} != {right}' instead of 'not {left} == {right}'"
SIM202 = "SIM202 Use '{left} == {right}' instead of 'not {left} != {right}'"
SIM203 = "SIM203 Use '{a} not in {b}' instead of 'not {a} in {b}'"
Expand Down Expand Up @@ -1021,6 +1028,85 @@ def _get_sim118(node: ast.Compare) -> List[Tuple[int, int, str]]:
return errors


def _get_sim119(node: ast.ClassDef) -> List[Tuple[int, int, str]]:
"""
Get a list of all classes that should be dataclasses"
ClassDef(
name='Person',
bases=[],
keywords=[],
body=[
AnnAssign(
target=Name(id='first_name', ctx=Store()),
annotation=Name(id='str', ctx=Load()),
value=None,
simple=1,
),
AnnAssign(
target=Name(id='last_name', ctx=Store()),
annotation=Name(id='str', ctx=Load()),
value=None,
simple=1,
),
AnnAssign(
target=Name(id='birthdate', ctx=Store()),
annotation=Name(id='date', ctx=Load()),
value=None,
simple=1,
),
],
decorator_list=[Name(id='dataclass', ctx=Load())],
)
"""
errors: List[Tuple[int, int, str]] = []

if not (len(node.decorator_list) == 0 and len(node.bases) == 0):
return errors

dataclass_functions = [
"__init__",
"__eq__",
"__hash__",
"__repr__",
"__str__",
]
has_only_constructur_function = True
for body_el in node.body:
if (
isinstance(body_el, ast.FunctionDef)
and body_el.name not in dataclass_functions
):
has_only_constructur_function = False
break

if not has_only_constructur_function:
return errors

errors.append(
(node.lineno, node.col_offset, SIM119.format(classname=node.name))
)

return errors


def _get_sim120(node: ast.ClassDef) -> List[Tuple[int, int, str]]:
"""
Get a list of all classes that inherit from object.
"""
errors: List[Tuple[int, int, str]] = []
if not (
len(node.bases) == 1
and isinstance(node.bases[0], ast.Name)
and node.bases[0].id == "object"
):
return errors
errors.append(
(node.lineno, node.col_offset, SIM120.format(classname=node.name))
)
return errors


def get_if_body_pairs(node: ast.If) -> List[Tuple[ast.expr, List[ast.stmt]]]:
pairs = [(node.test, node.body)]
orelse = node.orelse
Expand Down Expand Up @@ -1558,6 +1644,11 @@ def visit_Compare(self, node: ast.Compare) -> None:
self.errors += _get_sim300(node)
self.generic_visit(node)

def visit_ClassDef(self, node: ast.ClassDef) -> None:
self.errors += _get_sim119(node)
self.errors += _get_sim120(node)
self.generic_visit(node)


class Plugin:
name = __name__
Expand Down
22 changes: 22 additions & 0 deletions tests/test_simplify.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,28 @@ def test_sim118():
}


def test_sim119():
results = _results(
"""
class FooBar:
def __init__(self, a, b):
self.a = a
self.b = b
def __str__(self):
return "FooBar"
"""
)
assert results == {"2:0 SIM119 Use a dataclass for 'class FooBar'"}


def test_sim120():
results = _results("class FooBar(object): pass")
assert results == {
"1:0 SIM120 Use 'class FooBar:' instead of 'class FooBar(object):'"
}


def test_get_if_body_pairs():
ret = ast.parse(
"""if a == b:
Expand Down

0 comments on commit 9f74bde

Please sign in to comment.