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
This commit is contained in:
		
				
					committed by
					
						 GitHub
						GitHub
					
				
			
			
				
	
			
			
			
						parent
						
							9c1bf25304
						
					
				
				
					commit
					69f4192341
				
			| @@ -1,16 +1,15 @@ | |||||||
| import dataclasses | import dataclasses | ||||||
| import enum | import enum | ||||||
| import inspect |  | ||||||
| import json | import json | ||||||
| import math | import math | ||||||
| import struct | import struct | ||||||
| import sys | import sys | ||||||
| import typing | import typing | ||||||
|  | import warnings | ||||||
| from abc import ABC | from abc import ABC | ||||||
| from base64 import b64decode, b64encode | from base64 import b64decode, b64encode | ||||||
| from copy import deepcopy | from copy import deepcopy | ||||||
| from datetime import datetime, timedelta, timezone | from datetime import datetime, timedelta, timezone | ||||||
| from dateutil.parser import isoparse |  | ||||||
| from typing import ( | from typing import ( | ||||||
|     Any, |     Any, | ||||||
|     Callable, |     Callable, | ||||||
| @@ -26,12 +25,13 @@ from typing import ( | |||||||
|     get_type_hints, |     get_type_hints, | ||||||
| ) | ) | ||||||
|  |  | ||||||
|  | from dateutil.parser import isoparse | ||||||
|  |  | ||||||
| from ._types import T | from ._types import T | ||||||
| from ._version import __version__ | from ._version import __version__ | ||||||
| from .casing import camel_case, safe_snake_case, snake_case | from .casing import camel_case, safe_snake_case, snake_case | ||||||
| from .grpc.grpclib_client import ServiceStub | from .grpc.grpclib_client import ServiceStub | ||||||
|  |  | ||||||
|  |  | ||||||
| # Proto 3 data types | # Proto 3 data types | ||||||
| TYPE_ENUM = "enum" | TYPE_ENUM = "enum" | ||||||
| TYPE_BOOL = "bool" | TYPE_BOOL = "bool" | ||||||
| @@ -867,7 +867,10 @@ class Message(ABC): | |||||||
|         return field_cls |         return field_cls | ||||||
|  |  | ||||||
|     def _get_field_default(self, field_name: str) -> Any: |     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 |     @classmethod | ||||||
|     def _get_field_default_gen(cls, field: dataclasses.Field) -> Any: |     def _get_field_default_gen(cls, field: dataclasses.Field) -> Any: | ||||||
| @@ -1288,6 +1291,22 @@ class Message(ABC): | |||||||
|         """ |         """ | ||||||
|         return self.from_dict(json.loads(value)) |         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: | def serialized_on_wire(message: Message) -> bool: | ||||||
|     """ |     """ | ||||||
|   | |||||||
| @@ -63,7 +63,7 @@ class {{ message.py_name }}(betterproto.Message): | |||||||
|         {% endif %} |         {% endif %} | ||||||
|         super().__post_init__() |         super().__post_init__() | ||||||
|         {% for field in message.deprecated_fields %} |         {% for field in message.deprecated_fields %} | ||||||
|         if self.{{ field }}: |         if self.is_set("{{ field }}"): | ||||||
|             warnings.warn("{{ message.py_name }}.{{ field }} is deprecated", DeprecationWarning) |             warnings.warn("{{ message.py_name }}.{{ field }} is deprecated", DeprecationWarning) | ||||||
|         {% endfor %} |         {% endfor %} | ||||||
|     {%  endif %} |     {%  endif %} | ||||||
|   | |||||||
| @@ -1,4 +1,6 @@ | |||||||
| { | { | ||||||
|   "v": 10, |   "message": { | ||||||
|  |     "value": "hello" | ||||||
|  |   }, | ||||||
|   "value": 10 |   "value": 10 | ||||||
| } | } | ||||||
|   | |||||||
| @@ -4,8 +4,11 @@ package deprecated; | |||||||
|  |  | ||||||
| // Some documentation about the Test message. | // Some documentation about the Test message. | ||||||
| message Test { | message Test { | ||||||
|     // Some documentation about the value. |     Message message = 1 [deprecated=true]; | ||||||
|     option deprecated = true; |  | ||||||
|     int32 v = 1 [deprecated=true]; |  | ||||||
|     int32 value = 2; |     int32 value = 2; | ||||||
| } | } | ||||||
|  |  | ||||||
|  | message Message { | ||||||
|  |     option deprecated = true; | ||||||
|  |     string value = 1; | ||||||
|  | } | ||||||
|   | |||||||
| @@ -1,4 +0,0 @@ | |||||||
| { |  | ||||||
|   "v": 10, |  | ||||||
|   "value": 10 |  | ||||||
| } |  | ||||||
| @@ -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; |  | ||||||
| } |  | ||||||
| @@ -1,26 +1,42 @@ | |||||||
|  | import warnings | ||||||
|  |  | ||||||
| import pytest | import pytest | ||||||
|  |  | ||||||
| from tests.output_betterproto.deprecated import Test as DeprecatedMessageTest | from tests.output_betterproto.deprecated import Message, Test | ||||||
| from tests.output_betterproto.deprecated_field import Test as DeprecatedFieldTest |  | ||||||
|  |  | ||||||
|  | @pytest.fixture | ||||||
|  | def message(): | ||||||
|  |     with warnings.catch_warnings(): | ||||||
|  |         warnings.filterwarnings("ignore", category=DeprecationWarning) | ||||||
|  |         return Message(value="hello") | ||||||
|  |  | ||||||
|  |  | ||||||
| def test_deprecated_message(): | def test_deprecated_message(): | ||||||
|     with pytest.deprecated_call(): |     with pytest.warns(DeprecationWarning) as record: | ||||||
|         DeprecatedMessageTest(value=10) |         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: |     with pytest.warns(None) as record: | ||||||
|         DeprecatedMessageTest(v=10, value=10) |         Test(value=10) | ||||||
|     assert len(record) == 2 |  | ||||||
|  |     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 |     assert not record | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user