diff --git a/tests/fixtures/noqa/noqa.py b/tests/fixtures/noqa/noqa.py index a92acb6a5..fd556e699 100644 --- a/tests/fixtures/noqa/noqa.py +++ b/tests/fixtures/noqa/noqa.py @@ -582,6 +582,7 @@ def public(self): return self class LambdaAssignment(object): + cls_attr = lambda: ... # noqa: WPS467 def __init__(self): self._attr = lambda: ... # noqa: WPS467 diff --git a/tests/test_checker/test_noqa.py b/tests/test_checker/test_noqa.py index 8483b8e99..4ecc50fd4 100644 --- a/tests/test_checker/test_noqa.py +++ b/tests/test_checker/test_noqa.py @@ -246,7 +246,7 @@ 'WPS464': 0, # logically unacceptable. 'WPS465': 1, 'WPS466': 0, # defined in version specific table. - 'WPS467': 1, + 'WPS467': 2, 'WPS500': 1, 'WPS501': 1, diff --git a/tests/test_visitors/test_ast/test_classes/test_instance_assignment.py b/tests/test_visitors/test_ast/test_classes/test_attributes_assignment.py similarity index 67% rename from tests/test_visitors/test_ast/test_classes/test_instance_assignment.py rename to tests/test_visitors/test_ast/test_classes/test_attributes_assignment.py index dac8d9942..e78c2c6da 100644 --- a/tests/test_visitors/test_ast/test_classes/test_instance_assignment.py +++ b/tests/test_visitors/test_ast/test_classes/test_attributes_assignment.py @@ -18,6 +18,13 @@ class Example(object): {0} """ +classmethod_lambda_assignment = """ +class Example(object): + @classmethod + def some(cls): + {0} +""" + @pytest.mark.parametrize('assignment', [ 'self.attr = lambda: ...', @@ -63,3 +70,27 @@ def test_class_lambda_assignment( visitor.run() assert_errors(visitor, [InstanceLambdaAssignmentViolation]) + +@pytest.mark.parametrize('assignment', [ + 'cls.attr = lambda: ...', + 'cls.attr1 = cls.attr2 = lambda: ...', + 'cls.attr1, cls.attr2 = lambda: ..., "value"', + 'cls.attr: Callable[[], None] = lambda: ...', +]) +def test_classmethod_lambda_assignment( + assert_errors, + assert_error_text, + parse_ast_tree, + default_options, + assignment, + mode, +): + """Testing lambda assignment to class.""" + tree = parse_ast_tree(mode( + classmethod_lambda_assignment.format(assignment), + )) + + visitor = AttributesAssignmentVisitor(default_options, tree=tree) + visitor.run() + + assert_errors(visitor, [InstanceLambdaAssignmentViolation]) diff --git a/wemake_python_styleguide/logic/naming/name_nodes.py b/wemake_python_styleguide/logic/naming/name_nodes.py index e1795dcf7..604ccf9e0 100644 --- a/wemake_python_styleguide/logic/naming/name_nodes.py +++ b/wemake_python_styleguide/logic/naming/name_nodes.py @@ -4,6 +4,7 @@ from wemake_python_styleguide.compat.functions import get_assign_targets from wemake_python_styleguide.compat.types import AnyAssignWithWalrus +from wemake_python_styleguide.types import AnyAssign def is_same_variable(left: ast.AST, right: ast.AST) -> bool: @@ -59,6 +60,37 @@ def flat_variable_names(nodes: Iterable[AnyAssignWithWalrus]) -> Iterable[str]: )) +def flat_assignment_values(assigns: Iterable[AnyAssign]) -> Iterable[ast.AST]: + """ + Returns flat values from assignment. + + Use this function when you need to get list of values + from assign nodes. + """ + return itertools.chain.from_iterable(( + flat_tuples(assign.value) + for assign in assigns + if isinstance(assign.value, ast.AST) + )) + + +def flat_tuples(node: ast.AST) -> List[ast.AST]: + """ + Returns flat values from tuples. + + Use this function when you need to get list of values + from tuple nodes. + """ + flatten_nodes: List[ast.AST] = [] + + if isinstance(node, ast.Tuple): + for subnode in node.elts: + flatten_nodes.extend(flat_tuples(subnode)) + else: + flatten_nodes.append(node) + return flatten_nodes + + def get_variables_from_node(node: ast.AST) -> List[str]: """ Gets the assigned names from the list of nodes. diff --git a/wemake_python_styleguide/logic/tree/assignments.py b/wemake_python_styleguide/logic/tree/assignments.py deleted file mode 100644 index 92c661149..000000000 --- a/wemake_python_styleguide/logic/tree/assignments.py +++ /dev/null @@ -1,87 +0,0 @@ -import ast -import itertools -from typing import Iterable, List, Optional, Tuple - -from wemake_python_styleguide import constants -from wemake_python_styleguide.compat.aliases import AssignNodes -from wemake_python_styleguide.compat.functions import get_assign_targets -from wemake_python_styleguide.logic import nodes -from wemake_python_styleguide.types import AnyAssign - -#: Type alias for the assignments we return from class inspection. -_AllAssignments = Tuple[List[AnyAssign], List[AnyAssign]] - - -def flat_assignment_values(assigns: Iterable[AnyAssign]) -> Iterable[ast.AST]: - """ - Returns flat values from assignment. - - Use this function when you need to get list of values - from assign nodes. - """ - return itertools.chain.from_iterable(( - _flat_nodes(assign.value) - for assign in assigns - if isinstance(assign.value, ast.AST) - )) - - -def _flat_nodes(node: ast.AST) -> List[ast.AST]: - flatten_nodes: List[ast.AST] = [] - - if isinstance(node, ast.Tuple): - for subnode in node.elts: - flatten_nodes.extend(_flat_nodes(subnode)) - else: - flatten_nodes.append(node) - return flatten_nodes - - -def get_assignments(node: ast.ClassDef) -> _AllAssignments: - """ - Helper to get all assignments from class nod definitions. - - Args: - node: class node definition. - - Returns: - A tuple of lists for both class and instance level variables. - """ - class_assignments = [] - instance_assignments = [] - - for subnode in ast.walk(node): - instance_assign = _get_instance_assignment(subnode) - if instance_assign is not None: - instance_assignments.append(instance_assign) - continue - - class_assign = _get_class_assignment(node, subnode) - if class_assign is not None: - class_assignments.append(class_assign) - - return class_assignments, instance_assignments - - -def _get_instance_assignment(subnode: ast.AST) -> Optional[AnyAssign]: - return subnode if ( - isinstance(subnode, AssignNodes) and - any( - isinstance(target, ast.Attribute) and - isinstance(target.value, ast.Name) and - target.value.id in constants.SPECIAL_ARGUMENT_NAMES_WHITELIST - for targets in get_assign_targets(subnode) - for target in _flat_nodes(targets) - ) - ) else None - - -def _get_class_assignment( - node: ast.ClassDef, - subnode: ast.AST, -) -> Optional[AnyAssign]: - return subnode if ( - isinstance(subnode, AssignNodes) and - nodes.get_context(subnode) is node and - getattr(subnode, 'value', None) - ) else None diff --git a/wemake_python_styleguide/logic/tree/classes.py b/wemake_python_styleguide/logic/tree/classes.py index 3c90dec67..62b0fa9fc 100644 --- a/wemake_python_styleguide/logic/tree/classes.py +++ b/wemake_python_styleguide/logic/tree/classes.py @@ -3,14 +3,17 @@ from typing_extensions import Final +from wemake_python_styleguide import constants from wemake_python_styleguide.compat.aliases import AssignNodes, FunctionNodes +from wemake_python_styleguide.compat.functions import get_assign_targets from wemake_python_styleguide.constants import ALLOWED_BUILTIN_CLASSES from wemake_python_styleguide.logic import nodes from wemake_python_styleguide.logic.naming.builtins import is_builtin_name +from wemake_python_styleguide.logic.naming.name_nodes import flat_tuples from wemake_python_styleguide.types import AnyAssign, AnyFunctionDef -#: Type alias for the attributes we return from class inspection. -_AllAttributes = Tuple[List[AnyAssign], List[ast.Attribute]] +#: Type alias for the assignments we return from class inspection. +_AllAssignments = Tuple[List[AnyAssign], List[AnyAssign]] #: Prefixes that usually define getters and setters. _GetterSetterPrefixes: Final = ('get_', 'set_') @@ -49,11 +52,11 @@ def is_forbidden_super_class(class_name: Optional[str]) -> bool: return is_builtin_name(class_name) -def get_attributes( +def get_assignments( node: ast.ClassDef, *, include_annotated: bool, -) -> _AllAttributes: +) -> _AllAssignments: """ Helper to get all attributes from class nod definitions. @@ -70,35 +73,40 @@ def get_attributes( A tuple of lists for both class and instance level variables. """ - class_attributes = [] - instance_attributes = [] + class_assignments = [] + instance_assignments = [] for subnode in ast.walk(node): - instance_attr = _get_instance_attribute(subnode) - if instance_attr is not None: - instance_attributes.append(instance_attr) + instance_assign = _get_instance_assignment(subnode) + if instance_assign is not None: + instance_assignments.append(instance_assign) continue if include_annotated: - class_attr = _get_annotated_class_attribute(node, subnode) + class_assign = _get_annotated_class_attribute(node, subnode) else: - class_attr = _get_class_attribute(node, subnode) - if class_attr is not None: - class_attributes.append(class_attr) + class_assign = _get_class_assignment(node, subnode) + if class_assign is not None: + class_assignments.append(class_assign) - return class_attributes, instance_attributes + return class_assignments, instance_assignments -def _get_instance_attribute(node: ast.AST) -> Optional[ast.Attribute]: - return node if ( - isinstance(node, ast.Attribute) and - isinstance(node.ctx, ast.Store) and - isinstance(node.value, ast.Name) and - node.value.id == 'self' +def _get_instance_assignment(subnode: ast.AST) -> Optional[AnyAssign]: + return subnode if ( + isinstance(subnode, AssignNodes) and + any( + isinstance(target, ast.Attribute) and + isinstance(target.ctx, ast.Store) and + isinstance(target.value, ast.Name) and + target.value.id in constants.SPECIAL_ARGUMENT_NAMES_WHITELIST + for targets in get_assign_targets(subnode) + for target in flat_tuples(targets) + ) ) else None -def _get_class_attribute( +def _get_class_assignment( node: ast.ClassDef, subnode: ast.AST, ) -> Optional[AnyAssign]: diff --git a/wemake_python_styleguide/violations/best_practices.py b/wemake_python_styleguide/violations/best_practices.py index 4c0fbffae..666b6ac96 100644 --- a/wemake_python_styleguide/violations/best_practices.py +++ b/wemake_python_styleguide/violations/best_practices.py @@ -2594,9 +2594,13 @@ class InstanceLambdaAssignmentViolation(ASTViolation): class Example(object): def foo(self): ... + @classmethod + def cls_foo(cls): + ... # Wrong: class Example(object): + cls_foo = lambda: ... def __init__(self): self.foo = lambda: ... diff --git a/wemake_python_styleguide/visitors/ast/classes.py b/wemake_python_styleguide/visitors/ast/classes.py index bf0f1cb89..4c838b0f7 100644 --- a/wemake_python_styleguide/visitors/ast/classes.py +++ b/wemake_python_styleguide/visitors/ast/classes.py @@ -12,7 +12,6 @@ from wemake_python_styleguide.logic.arguments import function_args, super_args from wemake_python_styleguide.logic.naming import access, name_nodes from wemake_python_styleguide.logic.tree import ( - assignments, attributes, classes, functions, @@ -120,18 +119,22 @@ def _is_correct_base_class(self, base_class: ast.AST) -> bool: return False def _check_getters_setters_methods(self, node: ast.ClassDef) -> None: - class_attributes, instance_attributes = classes.get_attributes( + class_assignments, instance_assignments = classes.get_assignments( node, include_annotated=True, ) - flat_class_attributes = name_nodes.flat_variable_names(class_attributes) + flat_class_attributes = name_nodes.flat_variable_names(class_assignments) attributes_stripped = { class_attribute.lstrip(constants.UNUSED_PLACEHOLDER) for class_attribute in flat_class_attributes }.union({ - instance.attr.lstrip(constants.UNUSED_PLACEHOLDER) - for instance in instance_attributes + instance_attribute.attr.lstrip(constants.UNUSED_PLACEHOLDER) + for assign in instance_assignments + for instance_attribute in walk.get_subnodes_by_type( + assign, + ast.Attribute, + ) }) for method in classes.find_getters_and_setters(node): @@ -373,22 +376,23 @@ def visit_ClassDef(self, node: ast.ClassDef) -> None: self.generic_visit(node) def _check_attributes_shadowing(self, node: ast.ClassDef) -> None: - class_attributes, instance_attributes = classes.get_attributes( + class_assignments, instance_assignments = classes.get_assignments( node, include_annotated=False, ) class_attribute_names = set( - name_nodes.flat_variable_names(class_attributes), + name_nodes.flat_variable_names(class_assignments), ) - for instance_attr in instance_attributes: - if instance_attr.attr in class_attribute_names: - self.add_violation( - oop.ShadowedClassAttributeViolation( - instance_attr, - text=instance_attr.attr, - ), - ) + for instance_assignment in instance_assignments: + for instance_attr in walk.get_subnodes_by_type(instance_assignment, ast.Attribute): + if instance_attr.attr in class_attribute_names: + self.add_violation( + oop.ShadowedClassAttributeViolation( + instance_attr, + text=instance_attr.attr, + ), + ) @final @@ -452,10 +456,11 @@ def visit_ClassDef(self, node: ast.ClassDef) -> None: self.generic_visit(node) def _check_lambda_assignment(self, node: ast.ClassDef) -> None: - class_assignments, instance_assignments = assignments.get_assignments( + class_assignments, instance_assignments = classes.get_assignments( node, + include_annotated=True, ) - flatten_values = assignments.flat_assignment_values( + flatten_values = name_nodes.flat_assignment_values( itertools.chain( class_assignments, instance_assignments, diff --git a/wemake_python_styleguide/visitors/ast/complexity/classes.py b/wemake_python_styleguide/visitors/ast/complexity/classes.py index b356a24ea..d775fa987 100644 --- a/wemake_python_styleguide/visitors/ast/complexity/classes.py +++ b/wemake_python_styleguide/visitors/ast/complexity/classes.py @@ -7,6 +7,7 @@ from wemake_python_styleguide.logic.naming import access from wemake_python_styleguide.logic.nodes import get_parent from wemake_python_styleguide.logic.tree import classes +from wemake_python_styleguide.logic.walk import get_subnodes_by_type from wemake_python_styleguide.types import AnyFunctionDef from wemake_python_styleguide.violations.complexity import ( TooManyBaseClassesViolation, @@ -58,13 +59,14 @@ def _check_base_classes(self, node: ast.ClassDef) -> None: ) def _check_public_attributes(self, node: ast.ClassDef) -> None: - _, instance_attributes = classes.get_attributes( + _, instance_assignments = classes.get_assignments( node, include_annotated=False, ) attrs_count = len({ attr.attr - for attr in instance_attributes + for assign in instance_assignments + for attr in get_subnodes_by_type(assign, ast.Attribute) if access.is_public(attr.attr) }) diff --git a/wemake_python_styleguide/visitors/ast/naming/validation.py b/wemake_python_styleguide/visitors/ast/naming/validation.py index ef41724e2..cd822d426 100644 --- a/wemake_python_styleguide/visitors/ast/naming/validation.py +++ b/wemake_python_styleguide/visitors/ast/naming/validation.py @@ -199,11 +199,11 @@ def check_function_signature(self, node: AnyFunctionDefAndLambda) -> None: @final class _ClassBasedNameValidator(_RegularNameValidator): def check_attribute_names(self, node: ast.ClassDef) -> None: - class_attributes, _ = classes.get_attributes( + class_assignments, _ = classes.get_assignments( node, include_annotated=True, ) - for assign in class_attributes: + for assign in class_assignments: for target in get_assign_targets(assign): for attr_name in name_nodes.get_variables_from_node(target): self._ensure_case(assign, attr_name)