From a06076126ab5e446a3961c22e125ab761e90c920 Mon Sep 17 00:00:00 2001 From: Martin Thoma Date: Mon, 28 Mar 2022 16:19:18 +0200 Subject: [PATCH 1/2] SCR-119: Add --- .isort.cfg | 2 +- README.md | 36 +++++++++++ flake8_scream/__init__.py | 5 ++ flake8_scream/rules/ast_classdef.py | 79 +++++++++++++++++++++++ tests/test_100_rules.py | 99 +++++++++++++++++++++++++++++ 5 files changed, 220 insertions(+), 1 deletion(-) create mode 100644 flake8_scream/rules/ast_classdef.py create mode 100644 tests/test_100_rules.py diff --git a/.isort.cfg b/.isort.cfg index 26677df..19dd031 100644 --- a/.isort.cfg +++ b/.isort.cfg @@ -3,5 +3,5 @@ line_length=79 indent=' ' multi_line_output=3 length_sort=0 -known_third_party =flake8_simplify,setuptools +known_third_party =flake8_simplify,pytest,setuptools include_trailing_comma=True diff --git a/README.md b/README.md index f379805..fcc979d 100644 --- a/README.md +++ b/README.md @@ -6,3 +6,39 @@ # flake8-scream A [flake8](https://flake8.pycqa.org/en/latest/index.html) plugin that helps you scream your code. + + +## Rules + +* [`SCR119`](https://github.com/MartinThoma/flake8-simplify/issues/37) ![](https://shields.io/badge/-legacyfix-inactive): Use dataclasses for data containers ([example](#SCR119)) + + +## Disabling Rules + +You might have good reasons to +[ignore some flake8 rules](https://flake8.pycqa.org/en/3.1.1/user/ignoring-errors.html). +To do that, use the standard Flake8 configuration. For example, within the `setup.cfg` file: + +```python +[flake8] +ignore = SCR106, SCR113, SCR119, SCR9 +``` + +## Examples + +### SCR119 + +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) diff --git a/flake8_scream/__init__.py b/flake8_scream/__init__.py index e9608a1..8065f81 100644 --- a/flake8_scream/__init__.py +++ b/flake8_scream/__init__.py @@ -5,6 +5,7 @@ from flake8_simplify.utils import UnaryOp +from flake8_scream.rules.ast_classdef import get_scr119 from flake8_scream.rules.ast_unary_op import ( get_scr204, get_scr205, @@ -35,6 +36,10 @@ def visit_UnaryOp(self, node_v: ast.UnaryOp) -> None: self.errors += get_scr207(node) self.generic_visit(node) + def visit_ClassDef(self, node: ast.ClassDef) -> None: + self.errors += get_scr119(node) + self.generic_visit(node) + class Plugin: name = __name__ diff --git a/flake8_scream/rules/ast_classdef.py b/flake8_scream/rules/ast_classdef.py new file mode 100644 index 0000000..ccdd48b --- /dev/null +++ b/flake8_scream/rules/ast_classdef.py @@ -0,0 +1,79 @@ +import ast +from typing import List, Tuple + + +def get_scr119(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 = "SCR119 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 diff --git a/tests/test_100_rules.py b/tests/test_100_rules.py new file mode 100644 index 0000000..b6d75b3 --- /dev/null +++ b/tests/test_100_rules.py @@ -0,0 +1,99 @@ +import pytest + +from tests import _results + + +def test_scr119(): + results = _results( + """ +class FooBar: + def __init__(self, a, b): + self.a = a + self.b = b +""" + ) + assert results == {"2:0 SCR119 Use a dataclass for 'class FooBar'"} + + +def test_scr119_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 SCR119 Use a dataclass for 'class FooBar'"} + + +@pytest.mark.xfail(reason="https://github.com/MartinThoma/flake8-simplify/issues/63") +def test_scr119_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 "SCR119" not in el + + +def test_scr119_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_scr119_constructor_processing(): + results = _results( + """ +class FooBar: + def __init__(self, a): + self.a = a + 5 +""" + ) + assert results == set() + + +def test_scr119_pydantic(): + results = _results( + """ +from pydantic import BaseModel + +class FooBar(BaseModel): + foo : str + + class Config: + extra = "allow" +""" + ) + assert results == set() From 85d91192e49bab7a631d8e53c1edf5ec7771c91f Mon Sep 17 00:00:00 2001 From: Martin Thoma Date: Mon, 28 Mar 2022 16:23:51 +0200 Subject: [PATCH 2/2] Fix CI --- .github/workflows/lint.yml | 6 +++--- .github/workflows/unit-tests.yml | 4 ++-- flake8_scream/rules/ast_classdef.py | 9 +++++++-- flake8_scream/rules/ast_unary_op.py | 16 ++++++++++++---- pyproject.toml | 4 ++++ tests/test_100_rules.py | 4 +++- 6 files changed, 31 insertions(+), 12 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e4254c4..87376b4 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -5,9 +5,9 @@ name: Lint on: push: - branches: [ master ] + branches: [ main ] pull_request: - branches: [ master ] + branches: [ main ] jobs: build: @@ -26,7 +26,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install -r requirements/lint.txt + pip install -r requirements/ci.txt pip install . - name: Test with mypy run: | diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index b83467f..27ae0d0 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -5,9 +5,9 @@ name: Unit Tests on: push: - branches: [ master ] + branches: [ main ] pull_request: - branches: [ master ] + branches: [ main ] jobs: build: diff --git a/flake8_scream/rules/ast_classdef.py b/flake8_scream/rules/ast_classdef.py index ccdd48b..9d0993d 100644 --- a/flake8_scream/rules/ast_classdef.py +++ b/flake8_scream/rules/ast_classdef.py @@ -62,7 +62,10 @@ def get_scr119(node: ast.ClassDef) -> List[Tuple[int, int, str]]: # It is an assignment, but we only allow # `self.attribute = name`. if any( - [not isinstance(target, ast.Attribute) for target in el.targets] + [ + not isinstance(target, ast.Attribute) + for target in el.targets + ] ) or not isinstance(el.value, ast.Name): has_complex_statements = True break @@ -74,6 +77,8 @@ def get_scr119(node: ast.ClassDef) -> List[Tuple[int, int, str]]: and has_only_dataclass_functions and not has_complex_statements ): - errors.append((node.lineno, node.col_offset, RULE.format(classname=node.name))) + errors.append( + (node.lineno, node.col_offset, RULE.format(classname=node.name)) + ) return errors diff --git a/flake8_scream/rules/ast_unary_op.py b/flake8_scream/rules/ast_unary_op.py index 2719d3f..842fa4d 100644 --- a/flake8_scream/rules/ast_unary_op.py +++ b/flake8_scream/rules/ast_unary_op.py @@ -22,7 +22,9 @@ def get_scr204(node: UnaryOp) -> List[Tuple[int, int, str]]: comparison = node.operand left = to_source(comparison.left) right = to_source(comparison.comparators[0]) - errors.append((node.lineno, node.col_offset, SCR204.format(a=left, b=right))) + errors.append( + (node.lineno, node.col_offset, SCR204.format(a=left, b=right)) + ) return errors @@ -44,7 +46,9 @@ def get_scr205(node: UnaryOp) -> List[Tuple[int, int, str]]: comparison = node.operand left = to_source(comparison.left) right = to_source(comparison.comparators[0]) - errors.append((node.lineno, node.col_offset, SCR205.format(a=left, b=right))) + errors.append( + (node.lineno, node.col_offset, SCR205.format(a=left, b=right)) + ) return errors @@ -66,7 +70,9 @@ def get_scr206(node: UnaryOp) -> List[Tuple[int, int, str]]: comparison = node.operand left = to_source(comparison.left) right = to_source(comparison.comparators[0]) - errors.append((node.lineno, node.col_offset, SCR206.format(a=left, b=right))) + errors.append( + (node.lineno, node.col_offset, SCR206.format(a=left, b=right)) + ) return errors @@ -88,5 +94,7 @@ def get_scr207(node: UnaryOp) -> List[Tuple[int, int, str]]: comparison = node.operand left = to_source(comparison.left) right = to_source(comparison.comparators[0]) - errors.append((node.lineno, node.col_offset, SCR207.format(a=left, b=right))) + errors.append( + (node.lineno, node.col_offset, SCR207.format(a=left, b=right)) + ) return errors diff --git a/pyproject.toml b/pyproject.toml index dff6b32..e84de10 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,7 @@ +[tool.black] +line-length = 79 +target-version = ['py37'] + [tool.pytest.ini_options] markers = [ "slow: marks tests as slow (deselect with '-m \"not slow\"')", diff --git a/tests/test_100_rules.py b/tests/test_100_rules.py index b6d75b3..b9536b7 100644 --- a/tests/test_100_rules.py +++ b/tests/test_100_rules.py @@ -37,7 +37,9 @@ def __str__(self): assert results == {"2:0 SCR119 Use a dataclass for 'class FooBar'"} -@pytest.mark.xfail(reason="https://github.com/MartinThoma/flake8-simplify/issues/63") +@pytest.mark.xfail( + reason="https://github.com/MartinThoma/flake8-simplify/issues/63" +) def test_scr119_false_positive(): results = _results( '''class OfType: