From c9b5d3bac8f7c1f779dd57653f718dd0fac4db4b Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Sat, 12 Jun 2021 15:05:36 -0700 Subject: chore: improve type-hinting for managers The 'managers' are dynamically created. This unfortunately means that we don't have any type-hints for them and so editors which understand type-hints won't know that they are valid attributes. * Add the type-hints for the managers we define. * Add a unit test that makes sure that the type-hints and the '_managers' attribute are kept in sync with each other. * Add unit test that makes sure specified managers in '_managers' have a name ending in 'Managers' to keep with current convention. * Make RESTObject._managers always present with a default value of None. * Fix a type-issue revealed now that mypy knows what the type is --- tests/unit/objects/test_type_hints.py | 74 +++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 tests/unit/objects/test_type_hints.py (limited to 'tests/unit') diff --git a/tests/unit/objects/test_type_hints.py b/tests/unit/objects/test_type_hints.py new file mode 100644 index 0000000..6742698 --- /dev/null +++ b/tests/unit/objects/test_type_hints.py @@ -0,0 +1,74 @@ +import inspect +from typing import Dict + +import gitlab +import gitlab.v4.objects + + +def test_managers_annotated(): + """Ensure _managers have been type annotated""" + + failed_messages = [] + for module_name, module_value in inspect.getmembers(gitlab.v4.objects): + if not inspect.ismodule(module_value): + # We only care about the modules + continue + # Iterate through all the classes in our module + for class_name, class_value in sorted(inspect.getmembers(module_value)): + if not inspect.isclass(class_value): + continue + + # Ignore imported classes from gitlab.base + if class_value.__module__ == "gitlab.base": + continue + + # A '_managers' attribute is only on a RESTObject + if not issubclass(class_value, gitlab.base.RESTObject): + continue + + if class_value._managers is None: + continue + + # Collect all of our annotations into a Dict[str, str] + annotations: Dict[str, str] = {} + for attr, annotation in sorted(class_value.__annotations__.items()): + if isinstance(annotation, type): + type_name = annotation.__name__ + else: + type_name = annotation + annotations[attr] = type_name + + for attr, manager_class_name in sorted(class_value._managers): + # All of our managers need to end with "Manager" for example + # "ProjectManager" + if not manager_class_name.endswith("Manager"): + failed_messages.append( + ( + f"ERROR: Class: {class_name!r} for '_managers' attribute " + f"{attr!r} The specified manager class " + f"{manager_class_name!r} does not have a name ending in " + f"'Manager'. Manager class names are required to end in " + f"'Manager'" + ) + ) + continue + if attr not in annotations: + failed_messages.append( + ( + f"ERROR: Class: {class_name!r}: Type annotation missing " + f"for '_managers' attribute {attr!r}" + ) + ) + continue + if manager_class_name != annotations[attr]: + failed_messages.append( + ( + f"ERROR: Class: {class_name!r}: Type annotation mismatch " + f"for '_managers' attribute {attr!r}. Type annotation is " + f"{annotations[attr]!r} while '_managers' is " + f"{manager_class_name!r}" + ) + ) + + failed_msg = "\n".join(failed_messages) + assert not failed_messages, failed_msg -- cgit v1.2.1 From d8de4dc373dc608be6cf6ba14a2acc7efd3fa7a7 Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Sun, 13 Jun 2021 14:40:46 -0700 Subject: chore: convert to using type-annotations for managers Convert our manager usage to be done via type annotations. Now to define a manager to be used in a RESTObject subclass can simply do: class ExampleClass(CRUDMixin, RESTObject): my_manager: MyManager Any type-annotation that annotates it to be of type *Manager (with the exception of RESTManager) will cause the manager to be created on the object. --- tests/unit/objects/test_type_hints.py | 74 ----------------------------------- tests/unit/test_base.py | 2 +- 2 files changed, 1 insertion(+), 75 deletions(-) delete mode 100644 tests/unit/objects/test_type_hints.py (limited to 'tests/unit') diff --git a/tests/unit/objects/test_type_hints.py b/tests/unit/objects/test_type_hints.py deleted file mode 100644 index 6742698..0000000 --- a/tests/unit/objects/test_type_hints.py +++ /dev/null @@ -1,74 +0,0 @@ -import inspect -from typing import Dict - -import gitlab -import gitlab.v4.objects - - -def test_managers_annotated(): - """Ensure _managers have been type annotated""" - - failed_messages = [] - for module_name, module_value in inspect.getmembers(gitlab.v4.objects): - if not inspect.ismodule(module_value): - # We only care about the modules - continue - # Iterate through all the classes in our module - for class_name, class_value in sorted(inspect.getmembers(module_value)): - if not inspect.isclass(class_value): - continue - - # Ignore imported classes from gitlab.base - if class_value.__module__ == "gitlab.base": - continue - - # A '_managers' attribute is only on a RESTObject - if not issubclass(class_value, gitlab.base.RESTObject): - continue - - if class_value._managers is None: - continue - - # Collect all of our annotations into a Dict[str, str] - annotations: Dict[str, str] = {} - for attr, annotation in sorted(class_value.__annotations__.items()): - if isinstance(annotation, type): - type_name = annotation.__name__ - else: - type_name = annotation - annotations[attr] = type_name - - for attr, manager_class_name in sorted(class_value._managers): - # All of our managers need to end with "Manager" for example - # "ProjectManager" - if not manager_class_name.endswith("Manager"): - failed_messages.append( - ( - f"ERROR: Class: {class_name!r} for '_managers' attribute " - f"{attr!r} The specified manager class " - f"{manager_class_name!r} does not have a name ending in " - f"'Manager'. Manager class names are required to end in " - f"'Manager'" - ) - ) - continue - if attr not in annotations: - failed_messages.append( - ( - f"ERROR: Class: {class_name!r}: Type annotation missing " - f"for '_managers' attribute {attr!r}" - ) - ) - continue - if manager_class_name != annotations[attr]: - failed_messages.append( - ( - f"ERROR: Class: {class_name!r}: Type annotation mismatch " - f"for '_managers' attribute {attr!r}. Type annotation is " - f"{annotations[attr]!r} while '_managers' is " - f"{manager_class_name!r}" - ) - ) - - failed_msg = "\n".join(failed_messages) - assert not failed_messages, failed_msg diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index 8872dbd..cccdfad 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -147,7 +147,7 @@ class TestRESTObject: def test_create_managers(self, fake_gitlab, fake_manager): class ObjectWithManager(FakeObject): - _managers = (("fakes", "FakeManager"),) + fakes: "FakeManager" obj = ObjectWithManager(fake_manager, {"foo": "bar"}) obj.id = 42 -- cgit v1.2.1