From 69f41923411e9d84fc8c8ecc70fa22bcb51f9b1d Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Sat, 12 Mar 2022 00:36:14 +0100 Subject: [PATCH] Fix incorrect deprecation warnings on defaults (#348) This change ensures that deprecation warnings are only raised when either a deprecated field is explicitly set or a deprecated message is initialised. Resolves: #347 --- src/betterproto/__init__.py | 27 +++++++++-- src/betterproto/templates/template.py.j2 | 2 +- tests/inputs/deprecated/deprecated.json | 4 +- tests/inputs/deprecated/deprecated.proto | 9 ++-- .../deprecated_field/deprecated_field.json | 4 -- .../deprecated_field/deprecated_field.proto | 10 ---- tests/test_deprecated.py | 48 ++++++++++++------- 7 files changed, 65 insertions(+), 39 deletions(-) delete mode 100644 tests/inputs/deprecated_field/deprecated_field.json delete mode 100644 tests/inputs/deprecated_field/deprecated_field.proto diff --git a/src/betterproto/__init__.py b/src/betterproto/__init__.py index 8207c40..0a46af6 100644 --- a/src/betterproto/__init__.py +++ b/src/betterproto/__init__.py @@ -1,16 +1,15 @@ import dataclasses import enum -import inspect import json import math import struct import sys import typing +import warnings from abc import ABC from base64 import b64decode, b64encode from copy import deepcopy from datetime import datetime, timedelta, timezone -from dateutil.parser import isoparse from typing import ( Any, Callable, @@ -26,12 +25,13 @@ from typing import ( get_type_hints, ) +from dateutil.parser import isoparse + from ._types import T from ._version import __version__ from .casing import camel_case, safe_snake_case, snake_case from .grpc.grpclib_client import ServiceStub - # Proto 3 data types TYPE_ENUM = "enum" TYPE_BOOL = "bool" @@ -867,7 +867,10 @@ class Message(ABC): return field_cls def _get_field_default(self, field_name: str) -> Any: - return self._betterproto.default_gen[field_name]() + with warnings.catch_warnings(): + # ignore warnings when initialising deprecated field defaults + warnings.filterwarnings("ignore", category=DeprecationWarning) + return self._betterproto.default_gen[field_name]() @classmethod def _get_field_default_gen(cls, field: dataclasses.Field) -> Any: @@ -1288,6 +1291,22 @@ class Message(ABC): """ return self.from_dict(json.loads(value)) + def is_set(self, name: str) -> bool: + """ + Check if field with the given name has been set. + + Parameters + ----------- + name: :class:`str` + The name of the field to check for. + + Returns + -------- + :class:`bool` + `True` if field has been set, otherwise `False`. + """ + return self.__raw_get(name) is not PLACEHOLDER + def serialized_on_wire(message: Message) -> bool: """ diff --git a/src/betterproto/templates/template.py.j2 b/src/betterproto/templates/template.py.j2 index f346672..718cda9 100644 --- a/src/betterproto/templates/template.py.j2 +++ b/src/betterproto/templates/template.py.j2 @@ -63,7 +63,7 @@ class {{ message.py_name }}(betterproto.Message): {% endif %} super().__post_init__() {% for field in message.deprecated_fields %} - if self.{{ field }}: + if self.is_set("{{ field }}"): warnings.warn("{{ message.py_name }}.{{ field }} is deprecated", DeprecationWarning) {% endfor %} {% endif %} diff --git a/tests/inputs/deprecated/deprecated.json b/tests/inputs/deprecated/deprecated.json index 9da52bb..43b2b65 100644 --- a/tests/inputs/deprecated/deprecated.json +++ b/tests/inputs/deprecated/deprecated.json @@ -1,4 +1,6 @@ { - "v": 10, + "message": { + "value": "hello" + }, "value": 10 } diff --git a/tests/inputs/deprecated/deprecated.proto b/tests/inputs/deprecated/deprecated.proto index d7102d0..81d69c0 100644 --- a/tests/inputs/deprecated/deprecated.proto +++ b/tests/inputs/deprecated/deprecated.proto @@ -4,8 +4,11 @@ package deprecated; // Some documentation about the Test message. message Test { - // Some documentation about the value. - option deprecated = true; - int32 v = 1 [deprecated=true]; + Message message = 1 [deprecated=true]; int32 value = 2; } + +message Message { + option deprecated = true; + string value = 1; +} diff --git a/tests/inputs/deprecated_field/deprecated_field.json b/tests/inputs/deprecated_field/deprecated_field.json deleted file mode 100644 index 9da52bb..0000000 --- a/tests/inputs/deprecated_field/deprecated_field.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "v": 10, - "value": 10 -} diff --git a/tests/inputs/deprecated_field/deprecated_field.proto b/tests/inputs/deprecated_field/deprecated_field.proto deleted file mode 100644 index d303c8d..0000000 --- a/tests/inputs/deprecated_field/deprecated_field.proto +++ /dev/null @@ -1,10 +0,0 @@ -syntax = "proto3"; - -package deprecated_field; - -// Some documentation about the Test message. -message Test { - // Some documentation about the value. - int32 v = 1 [deprecated=true]; - int32 value = 2; -} diff --git a/tests/test_deprecated.py b/tests/test_deprecated.py index cbdea33..713af7d 100644 --- a/tests/test_deprecated.py +++ b/tests/test_deprecated.py @@ -1,26 +1,42 @@ +import warnings + import pytest -from tests.output_betterproto.deprecated import Test as DeprecatedMessageTest -from tests.output_betterproto.deprecated_field import Test as DeprecatedFieldTest +from tests.output_betterproto.deprecated import Message, Test + + +@pytest.fixture +def message(): + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + return Message(value="hello") def test_deprecated_message(): - with pytest.deprecated_call(): - DeprecatedMessageTest(value=10) + with pytest.warns(DeprecationWarning) as record: + Message(value="hello") + + assert len(record) == 1 + assert str(record[0].message) == f"{Message.__name__} is deprecated" -def test_deprecated_message_with_deprecated_field(): +def test_message_with_deprecated_field(message): + with pytest.warns(DeprecationWarning) as record: + Test(message=message, value=10) + + assert len(record) == 1 + assert str(record[0].message) == f"{Test.__name__}.message is deprecated" + + +def test_message_with_deprecated_field_not_set(message): with pytest.warns(None) as record: - DeprecatedMessageTest(v=10, value=10) - assert len(record) == 2 + Test(value=10) + + assert not record + + +def test_message_with_deprecated_field_not_set_default(message): + with pytest.warns(None) as record: + _ = Test(value=10).message - -def test_deprecated_field_warning(): - with pytest.deprecated_call(): - DeprecatedFieldTest(v=10, value=10) - - -def test_deprecated_field_no_warning(): - with pytest.warns(None) as record: - DeprecatedFieldTest(value=10) assert not record