Skip to content

Commit

Permalink
SIM119: Removed due to false-positives (#123)
Browse files Browse the repository at this point in the history
  • Loading branch information
MartinThoma committed Mar 28, 2022
1 parent bef1b2c commit 91f371f
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 199 deletions.
18 changes: 1 addition & 17 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Python-specific rules:
* [`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))
* [`SIM119`](https://github.com/MartinThoma/flake8-simplify/issues/37) ![](https://shields.io/badge/-legacyfix-inactive): Use dataclasses for data containers ([example](#SIM119))
* `SIM119`: ![](https://img.shields.io/badge/-removed-lightgrey) Moved to [flake8-scream](https://github.com/MartinThoma/flake8-scream) due to [issue 63](https://github.com/MartinThoma/flake8-simplify/issues/63)
* `SIM120` ![](https://shields.io/badge/-legacyfix-inactive): Use 'class FooBar:' instead of 'class FooBar(object):' ([example](#SIM120))
* `SIM121`: Reserved for [SIM908](#sim908) once it's stable
* `SIM124`: Reserved for [SIM904](#sim904) once it's stable
Expand Down Expand Up @@ -340,22 +340,6 @@ key in a_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

Expand Down
3 changes: 1 addition & 2 deletions flake8_simplify/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
get_sim905,
get_sim906,
)
from flake8_simplify.rules.ast_classdef import get_sim119, get_sim120
from flake8_simplify.rules.ast_classdef import get_sim120
from flake8_simplify.rules.ast_compare import get_sim118, get_sim300
from flake8_simplify.rules.ast_expr import get_sim112
from flake8_simplify.rules.ast_for import (
Expand Down Expand Up @@ -142,7 +142,6 @@ def visit_Compare(self, node: ast.Compare) -> None:
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)

Expand Down
82 changes: 0 additions & 82 deletions flake8_simplify/rules/ast_classdef.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,88 +3,6 @@
from typing import List, Tuple


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())],
)
"""
RULE = "SIM119 Use a dataclass for 'class {classname}'"
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_dataclass_functions = True
has_any_functions = False
has_complex_statements = False
for body_el in node.body:
if isinstance(body_el, (ast.FunctionDef, ast.AsyncFunctionDef)):
has_any_functions = True
if body_el.name == "__init__":
# Ensure constructor only has pure assignments
# without any calculation.
for el in body_el.body:
if not isinstance(el, ast.Assign):
has_complex_statements = True
break
# It is an assignment, but we only allow
# `self.attribute = name`.
if any(
[
not isinstance(target, ast.Attribute)
for target in el.targets
]
) or not isinstance(el.value, ast.Name):
has_complex_statements = True
break
if body_el.name not in dataclass_functions:
has_only_dataclass_functions = False

if (
has_any_functions
and has_only_dataclass_functions
and not has_complex_statements
):
errors.append(
(node.lineno, node.col_offset, RULE.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.
Expand Down
98 changes: 0 additions & 98 deletions tests/test_100_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -493,104 +493,6 @@ def test_sim118_del_key():
assert results == set()


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


def test_sim119_ignored_dunder_methods():
"""
Dunder methods do not make a class not be a dataclass candidate.
Examples for dunder (double underscore) methods are:
* __str__
* __eq__
* __hash__
"""
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'"}


@pytest.mark.xfail(
reason="https://github.com/MartinThoma/flake8-simplify/issues/63"
)
def test_sim119_false_positive():
results = _results(
'''class OfType:
"""
>>> 3 == OfType(int, str, bool)
True
>>> 'txt' == OfType(int)
False
"""
def __init__(self, *types):
self.types = types
def __eq__(self, other):
return isinstance(other, self.types)'''
)
for el in results:
assert "SIM119" not in el


def test_sim119_async():
results = _results(
"""
class FooBar:
def __init__(self, a, b):
self.a = a
self.b = b
async def foo(self):
return "FooBar"
"""
)
assert results == set()


def test_sim119_constructor_processing():
results = _results(
"""
class FooBar:
def __init__(self, a):
self.a = a + 5
"""
)
assert results == set()


def test_sim119_pydantic():
results = _results(
"""
from pydantic import BaseModel
class FooBar(BaseModel):
foo : str
class Config:
extra = "allow"
"""
)
assert results == set()


def test_sim120():
results = _results("class FooBar(object): pass")
assert results == {
Expand Down

0 comments on commit 91f371f

Please sign in to comment.