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.

Sign up for GitHub

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

SIM116: Removed due to false-positives #123

Merged
merged 1 commit into from
Mar 28, 2022
Merged

SIM116: Removed due to false-positives #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 1 addition & 17 deletions README.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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