From e7d83e9d21033c6c2e0ac15c6deec8ea99944f11 Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Wed, 8 Mar 2023 08:44:08 -0800 Subject: [PATCH] Python APIView Fixes (#5653) * Closes #3759. * Display class decorators. Fixes #4860 * Fix #5197. --- .../api-stub-generator/CHANGELOG.md | 3 ++ .../apistub/nodes/_annotation_parser.py | 7 +++- .../apistub/nodes/_class_node.py | 27 +++++++++++--- .../apistub/nodes/_data_class_node.py | 1 + .../tests/class_parsing_test.py | 37 +++++++++++++++++-- .../apistubgentest/models/__init__.py | 4 ++ .../apistubgentest/models/_models.py | 27 +++++++++++++- 7 files changed, 96 insertions(+), 10 deletions(-) diff --git a/packages/python-packages/api-stub-generator/CHANGELOG.md b/packages/python-packages/api-stub-generator/CHANGELOG.md index 29e28c43de3..9b7e06ebd0b 100644 --- a/packages/python-packages/api-stub-generator/CHANGELOG.md +++ b/packages/python-packages/api-stub-generator/CHANGELOG.md @@ -2,6 +2,9 @@ ## Version 0.3.7 (Unreleased) Fix incorrect type annotation. +Update to follow best practices for accessing '__annotations__'. +Fixed issue where class decorators were not displayed. +Fixed issue where ivars appeared as cvars. ## Version 0.3.6 (2022-10-27) Suppressed unwanted base class methods in DPG libraries. diff --git a/packages/python-packages/api-stub-generator/apistub/nodes/_annotation_parser.py b/packages/python-packages/api-stub-generator/apistub/nodes/_annotation_parser.py index 8692430f879..378699cd058 100644 --- a/packages/python-packages/api-stub-generator/apistub/nodes/_annotation_parser.py +++ b/packages/python-packages/api-stub-generator/apistub/nodes/_annotation_parser.py @@ -15,7 +15,12 @@ def __init__(self, obj, namespace: str, func_node): self.posargs = {} self.varargs = None self.return_type = None - annotations = getattr(obj, "__annotations__", None) + # TODO: Replace with "get_annotations" once min Python is 3.10+ + # See: https://docs.python.org/3.10/howto/annotations.html#accessing-the-annotations-dict-of-an-object-in-python-3-9-and-older + if isinstance(obj, type): + annotations = obj.__dict__.get("__annotations__", None) + else: + annotations = getattr(obj, "__annotations__", None) if annotations: self.return_type = annotations.pop('return', inspect.Parameter.empty) self.args = { diff --git a/packages/python-packages/api-stub-generator/apistub/nodes/_class_node.py b/packages/python-packages/api-stub-generator/apistub/nodes/_class_node.py index 68167e6bdf0..40cbd434f2b 100644 --- a/packages/python-packages/api-stub-generator/apistub/nodes/_class_node.py +++ b/packages/python-packages/api-stub-generator/apistub/nodes/_class_node.py @@ -110,8 +110,7 @@ def _should_include_function(self, func_obj): return function_module and function_module.startswith(self.pkg_root_namespace) and not function_module.endswith("_model_base") return False - def _handle_class_variable(self, child_obj, name, *, type_string=None, value=None): - # Add any public class level variables + def _handle_variable(self, child_obj, name, *, type_string=None, value=None): allowed_types = (str, int, dict, list, float, bool) if not isinstance(child_obj, allowed_types): return @@ -124,6 +123,9 @@ def _handle_class_variable(self, child_obj, name, *, type_string=None, value=Non if type_string: var_match[0].type = type_string else: + is_ivar = True + if type_string: + is_ivar = not type_string.startswith("ClassVar") self.child_nodes.append( VariableNode( namespace=self.namespace, @@ -131,10 +133,18 @@ def _handle_class_variable(self, child_obj, name, *, type_string=None, value=Non name=name, type_name=type_string, value=value, - is_ivar=False + is_ivar=is_ivar ) ) + def _parse_decorators_from_class(self, class_obj): + try: + class_node = astroid.parse(inspect.getsource(class_obj)).body[0] + class_decorators = class_node.decorators.nodes + self.decorators = [f"@{x.as_string(preserve_quotes=True)}" for x in class_decorators] + except: + self.decorators = [] + def _parse_functions_from_class(self, class_obj) -> List[astroid.FunctionDef]: try: class_node = astroid.parse(inspect.getsource(class_obj)).body[0] @@ -179,6 +189,8 @@ def _inspect(self): is_typeddict = hasattr(self.obj, "__required_keys__") or hasattr(self.obj, "__optional_keys__") + self._parse_decorators_from_class(self.obj) + # find members in node # enums with duplicate values are screened out by "getmembers" so # we must rely on __members__ instead. @@ -212,7 +224,7 @@ def _inspect(self): ) else: type_string = get_qualified_name(item_type, self.namespace) - self._handle_class_variable(child_obj, item_name, type_string=type_string) + self._handle_variable(child_obj, item_name, type_string=type_string) # now that we've looked at the specific dunder properties we are # willing to include, anything with a leading underscore should be ignored. @@ -243,7 +255,7 @@ def _inspect(self): # Add instance properties self.child_nodes.append(PropertyNode(self.namespace, self, name, child_obj)) else: - self._handle_class_variable(child_obj, name, value=str(child_obj)) + self._handle_variable(child_obj, name, value=str(child_obj)) def _parse_ivars(self): # This method will add instance variables by parsing docstring @@ -297,6 +309,11 @@ def generate_tokens(self, apiview): """ logging.info(f"Processing class {self.namespace_id}") # Generate class name line + for decorator in self.decorators: + apiview.add_whitespace() + apiview.add_keyword(decorator) + apiview.add_newline() + apiview.add_whitespace() apiview.add_line_marker(self.namespace_id) apiview.add_keyword("class", False, True) diff --git a/packages/python-packages/api-stub-generator/apistub/nodes/_data_class_node.py b/packages/python-packages/api-stub-generator/apistub/nodes/_data_class_node.py index e470d7ba2bd..a207baefa68 100644 --- a/packages/python-packages/api-stub-generator/apistub/nodes/_data_class_node.py +++ b/packages/python-packages/api-stub-generator/apistub/nodes/_data_class_node.py @@ -14,6 +14,7 @@ class DataClassNode(ClassNode): def __init__(self, *, name, namespace, parent_node, obj, pkg_root_namespace): super().__init__(name=name, namespace=namespace, parent_node=parent_node, obj=obj, pkg_root_namespace=pkg_root_namespace) + self.decorators = [x for x in self.decorators if not x.startswith("@dataclass")] # explicitly set synthesized __init__ return type to None to fix test flakiness for child in self.child_nodes: if child.display_name == "__init__": diff --git a/packages/python-packages/api-stub-generator/tests/class_parsing_test.py b/packages/python-packages/api-stub-generator/tests/class_parsing_test.py index 2eae6d61591..07f24b3ceca 100644 --- a/packages/python-packages/api-stub-generator/tests/class_parsing_test.py +++ b/packages/python-packages/api-stub-generator/tests/class_parsing_test.py @@ -9,6 +9,8 @@ from apistubgentest.models import ( AliasNewType, AliasUnion, + ClassWithDecorators, + ClassWithIvarsAndCvars, FakeTypedDict, FakeObject, GenericStack, @@ -39,6 +41,35 @@ class TestClassParsing: pkg_namespace = "apistubgentest.models" + def test_class_with_ivars_and_cvars(self): + obj = ClassWithIvarsAndCvars + class_node = ClassNode(name=obj.__name__, namespace=obj.__name__, parent_node=None, obj=obj, pkg_root_namespace=self.pkg_namespace) + actuals = _render_lines(_tokenize(class_node)) + expected = [ + "class ClassWithIvarsAndCvars:", + 'ivar captain: str = "Picard"', + "ivar damage: int", + "cvar stats: ClassVar[Dict[str, int]] = {}" + ] + _check_all(actuals, expected, obj) + + def test_class_with_decorators(self): + obj = ClassWithDecorators + class_node = ClassNode(name=obj.__name__, namespace=obj.__name__, parent_node=None, obj=obj, pkg_root_namespace=self.pkg_namespace) + actuals = _render_lines(_tokenize(class_node)) + expected = [ + "@add_id", + "class ClassWithDecorators:", + "", + "def __init__(", + "self, ", + "id, ", + "*args, ", + "**kwargs", + ")", + ] + _check_all(actuals, expected, obj) + def test_typed_dict_class(self): obj = FakeTypedDict class_node = ClassNode(name=obj.__name__, namespace=obj.__name__, parent_node=None, obj=obj, pkg_root_namespace=self.pkg_namespace) @@ -57,7 +88,7 @@ def test_object(self): actuals = _render_lines(_tokenize(class_node)) expected = [ "class FakeObject:", - 'cvar PUBLIC_CONST: str = "SOMETHING"', + 'ivar PUBLIC_CONST: str = "SOMETHING"', 'ivar age: int', 'ivar name: str', 'ivar union: Union[bool, PetEnumPy3MetaclassAlt]' @@ -72,8 +103,8 @@ def test_public_private(self): actuals = _render_lines(_tokenize(class_node)) expected = [ "class PublicPrivateClass:", - "cvar public_dict: dict = {'a': 'b'}", - 'cvar public_var: str = "SOMEVAL"', + "ivar public_dict: dict = {'a': 'b'}", + 'ivar public_var: str = "SOMEVAL"', ] _check_all(actuals, expected, obj) assert actuals[4].lstrip() == "def __init__(self)" diff --git a/packages/python-packages/apistubgentest/apistubgentest/models/__init__.py b/packages/python-packages/apistubgentest/apistubgentest/models/__init__.py index 0099052fc71..6fcf0f263b3 100644 --- a/packages/python-packages/apistubgentest/apistubgentest/models/__init__.py +++ b/packages/python-packages/apistubgentest/apistubgentest/models/__init__.py @@ -10,6 +10,8 @@ from ._models import ( AliasNewType, AliasUnion, + ClassWithDecorators, + ClassWithIvarsAndCvars, DocstringClass, FakeError, FakeObject, @@ -39,6 +41,8 @@ __all__ = ( "AliasNewType", "AliasUnion", + "ClassWithDecorators", + "ClassWithIvarsAndCvars", "DataClassSimple", "DataClassWithFields", "DataClassDynamic", diff --git a/packages/python-packages/apistubgentest/apistubgentest/models/_models.py b/packages/python-packages/apistubgentest/apistubgentest/models/_models.py index 214aee5bbe2..414a844c1d3 100644 --- a/packages/python-packages/apistubgentest/apistubgentest/models/_models.py +++ b/packages/python-packages/apistubgentest/apistubgentest/models/_models.py @@ -11,7 +11,7 @@ from collections.abc import Sequence from enum import Enum, EnumMeta import functools -from typing import Any, overload, Dict, TypedDict, Union, Optional, Generic, TypeVar, NewType, TypeAlias +from typing import Any, overload, Dict, TypedDict, Union, Optional, Generic, TypeVar, NewType, ClassVar from ._mixin import MixinWithOverloads @@ -31,6 +31,31 @@ def wrapper(*args, **kwargs): return wrapper return decorator +def get_id(self): + return self.__id + +def add_id(cls): + cls_init = cls.__init__ + + def __init__(self, id, *args, **kwargs): + self.__id = id + self.get_id = get_id + cls_init(self, *args, **kwargs) + + cls.__init__ = __init__ + return cls + + +@add_id +class ClassWithDecorators: + pass + + +class ClassWithIvarsAndCvars: + captain: str = "Picard" # instance var w/ default + damage: int # instance var w/out default + stats: ClassVar[Dict[str, int]] = {} # class var + class PublicCaseInsensitiveEnumMeta(EnumMeta): def __getitem__(self, name: str):