fix incorrect deprecation warnings on defaults

abn · abn · commit b1fd73d21138 · 2022-03-02T21:46:27.000+01:00
This change ensures that deprecation warnings are only raised when either a deprecated field is explicitly set or a deprecated message is initialised. Resolves: danielgtaylor#347
diff --git a/src/betterproto/__init__.py b/src/betterproto/__init__.py
@@ -867,7 +867,12 @@ def _cls_for(cls, field: dataclasses.Field, index: int = 0) -> Type:
         return field_cls
 
     def _get_field_default(self, field_name: str) -> Any:
-        return self._betterproto.default_gen[field_name]()
+        import warnings
+
+        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 +1293,22 @@ def from_json(self: T, value: Union[str, bytes]) -> T:
         """
         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
@@ -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
@@ -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
@@ -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
diff --git a/tests/inputs/deprecated_field/deprecated_field.proto b/tests/inputs/deprecated_field/deprecated_field.proto
diff --git 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():
-    with pytest.warns(None) as record:
-        DeprecatedMessageTest(v=10, value=10)
-    assert len(record) == 2
 
+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_deprecated_field_warning():
-    with pytest.deprecated_call():
-        DeprecatedFieldTest(v=10, value=10)
 
+def test_message_with_deprecated_field_not_set(message):
+    with pytest.warns(None) as record:
+        Test(value=10)
+
+    assert not record
 
-def test_deprecated_field_no_warning():
+
+def test_message_with_deprecated_field_not_set_default(message):
     with pytest.warns(None) as record:
-        DeprecatedFieldTest(value=10)
+        _ = Test(value=10).message
+
     assert not record

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,6 @@`
`1`	`1`	`{`
`2`		`- "v": 10,`
	`2`	`+ "message": {`
	`3`	`+ "value": "hello"`
	`4`	`+ },`
`3`	`5`	`"value": 10`
`4`	`6`	`}`