Remove read_optional in favor of is_null/read_null

This removes read_optional in favor of is_null and read_null on
shape deserializers. This lets us reduce the number of lambdas and
gives us more flexibility. This allows, for example, for skipping
nulls in sparse collections.

Author: JordonPhillips

codegen/smithy-python-codegen/src/main/java/software/amazon/smithy/python/codegen/generators/ListGenerator.java

@@ -81,15 +81,13 @@ private void generateDeserializer() {
                     ${?includeSchema}
                     member_schema = schema.members["member"]
                     ${/includeSchema}
-                    deserializer.read_list(
-                        schema,
-                        ${?sparse}
-                        lambda d: result.append(d.read_optional(member_schema, lambda s: ${3C|}))
-                        ${/sparse}
-                        ${^sparse}
-                        lambda d: result.append(${3C|})
-                        ${/sparse}
-                    )
+                    def _read_value(d: ShapeDeserializer):
+                        if d.is_null():
+                            d.read_null()
+                            ${?sparse}result.append(None)${/sparse}
+                        else:
+                            result.append(${3C|})
+                    deserializer.read_list(schema, _read_value)
                     return result
                 """, deserializerSymbol.getName(), listSymbol,
                 writer.consumer(w -> memberTarget.accept(

codegen/smithy-python-codegen/src/main/java/software/amazon/smithy/python/codegen/generators/MapGenerator.java

@@ -82,15 +82,13 @@ private void generateDeserializer() {
                 def $1L(deserializer: ShapeDeserializer, schema: Schema) -> $2T:
                     result: $2T = {}
                     value_schema = schema.members["value"]
-                    deserializer.read_map(
-                        schema,
-                        ${?sparse}
-                        lambda k, d: result.__setitem__(k, d.read_optional(value_schema, lambda s: ${3C|}))
-                        ${/sparse}
-                        ${^sparse}
-                        lambda k, d: result.__setitem__(k, ${3C|})
-                        ${/sparse}
-                    )
+                    def _read_value(k: str, d: ShapeDeserializer):
+                        if d.is_null():
+                            d.read_null()
+                            ${?sparse}result[k] = None${/sparse}
+                        else:
+                            result[k] = ${3C|}
+                    deserializer.read_map(schema, _read_value)
                     return result
                 """, deserializerSymbol.getName(), listSymbol,
                 writer.consumer(w -> valueTarget.accept(

python-packages/smithy-core/smithy_core/deserializers.py

@@ -53,24 +53,15 @@ def read_map(
         """
         ...
 
-    def read_null(self, schema: "Schema") -> None:
-        """Read a null value from the underlying data.
+    def is_null(self) -> bool:
+        """Returns whether the next value in the underlying data represents null.
 
         :param schema: The shape's schema.
         """
         ...
 
-    def read_optional[
-        T
-    ](self, schema: "Schema", optional: Callable[["Schema"], T]) -> T | None:
-        """Read an optional value from the underlying data.
-
-        This is intended to be used with sparse lists or maps.
-
-        :param schema: The shape's schema.
-        :param optional: A callable that takes a schema and reads a non-nullable value
-            from the underlying data.
-        """
+    def read_null(self) -> None:
+        """Read a null value from the underlying data."""
         ...
 
     def read_boolean(self, schema: "Schema") -> bool:

python-packages/smithy-core/smithy_core/documents.py

@@ -578,20 +578,16 @@ def read_map(
             consumer(k, _DocumentDeserializer(v))
 
     @override
-    def read_null(self, schema: "Schema") -> None:
+    def is_null(self) -> bool:
+        return self._value.is_none()
+
+    @override
+    def read_null(self) -> None:
         if (value := self._value.as_value()) is not None:
             raise ExpectationNotMetException(
                 f"Expected document value to be None, but was: {value}"
             )
 
-    @override
-    def read_optional[
-        T
-    ](self, schema: "Schema", optional: Callable[["Schema"], T]) -> T | None:
-        if self._value.is_none():
-            return None
-        return optional(schema)
-
     @override
     def read_boolean(self, schema: "Schema") -> bool:
         return self._value.as_boolean()

python-packages/smithy-core/tests/unit/test_documents.py

@@ -707,20 +707,32 @@ def _consumer(schema: Schema, de: ShapeDeserializer) -> None:
                     kwargs["struct_member"] = DocumentSerdeShape.deserialize(de)
                 case 11:
                     sparse_list_value: list[str | None] = []
+
+                    def _read_optional_list(d: ShapeDeserializer):
+                        if d.is_null():
+                            d.read_null()
+                            sparse_list_value.append(None)
+                        else:
+                            sparse_list_value.append(d.read_string(STRING))
+
                     de.read_list(
                         SCHEMA.members["sparseListMember"],
-                        lambda d: sparse_list_value.append(
-                            d.read_optional(STRING, d.read_string)
-                        ),
+                        _read_optional_list,
                     )
                     kwargs["list_member"] = sparse_list_value
                 case 12:
                     sparse_map_value: dict[str, str | None] = {}
+
+                    def _read_optional_map(k: str, d: ShapeDeserializer):
+                        if d.is_null():
+                            d.read_null()
+                            sparse_map_value[k] = None
+                        else:
+                            sparse_map_value[k] = d.read_string(STRING)
+
                     de.read_map(
                         SCHEMA.members["sparseMapMember"],
-                        lambda k, d: sparse_map_value.__setitem__(
-                            k, d.read_optional(STRING, d.read_string)
-                        ),
+                        _read_optional_map,
                     )
                     kwargs["map_member"] = sparse_map_value
                 case _:
@@ -891,18 +903,32 @@ def test_document_deserializer(given: Document, expected: Any):
         case list():
             actual = []
             deserializer = _DocumentDeserializer(given)
+
+            def _read_optional_list(d: ShapeDeserializer):
+                if d.is_null():
+                    d.read_null()
+                    actual.append(None)
+                else:
+                    actual.append(d.read_string(STRING))
+
             deserializer.read_list(
                 SCHEMA.members["sparseListMember"],
-                lambda d: actual.append(d.read_optional(STRING, d.read_string)),
+                _read_optional_list,
             )
         case dict():
             actual = {}
             deserializer = _DocumentDeserializer(given)
+
+            def _read_optional_map(k: str, d: ShapeDeserializer):
+                if d.is_null():
+                    d.read_null()
+                    actual[k] = None
+                else:
+                    actual[k] = d.read_string(STRING)
+
             deserializer.read_map(
                 SCHEMA.members["sparseMapMember"],
-                lambda k, d: actual.__setitem__(
-                    k, d.read_optional(STRING, d.read_string)
-                ),
+                _read_optional_map,
             )
         case DocumentSerdeShape():
             actual = given.as_shape(DocumentSerdeShape)

python-packages/smithy-json/smithy_json/_private/deserializers.py

@@ -106,20 +106,15 @@ def __init__(
         # populated on an as-needed basis.
         self._json_names: dict[ShapeID, dict[str, str]] = {}
 
-    def read_null(self, schema: Schema) -> None:
+    def is_null(self) -> bool:
+        return self._stream.peek().type == "null"
+
+    def read_null(self) -> None:
         event = next(self._stream)
         if event.value is not None:
             raise JSONTokenError("null", event)
         return None
 
-    def read_optional[
-        T
-    ](self, schema: "Schema", optional: Callable[["Schema"], T]) -> T | None:
-        if self._stream.peek().value is None:
-            next(self._stream)
-            return None
-        return optional(schema)
-
     def read_boolean(self, schema: Schema) -> bool:
         event = next(self._stream)
         if not isinstance(event.value, bool):

python-packages/smithy-json/tests/unit/__init__.py

@@ -270,20 +270,32 @@ def _consumer(schema: Schema, de: ShapeDeserializer) -> None:
                     kwargs["struct_member"] = SerdeShape.deserialize(de)
                 case 15:
                     sparse_list_value: list[str | None] = []
+
+                    def _read_optional_list(d: ShapeDeserializer):
+                        if d.is_null():
+                            d.read_null()
+                            sparse_list_value.append(None)
+                        else:
+                            sparse_list_value.append(d.read_string(STRING))
+
                     de.read_list(
                         SCHEMA.members["sparseListMember"],
-                        lambda d: sparse_list_value.append(
-                            d.read_optional(STRING, d.read_string)
-                        ),
+                        _read_optional_list,
                     )
                     kwargs["sparse_list_member"] = sparse_list_value
                 case 16:
                     sparse_map_value: dict[str, str | None] = {}
+
+                    def _read_optional_map(k: str, d: ShapeDeserializer):
+                        if d.is_null():
+                            d.read_null()
+                            sparse_map_value[k] = None
+                        else:
+                            sparse_map_value[k] = d.read_string(STRING)
+
                     de.read_map(
                         SCHEMA.members["sparseMapMember"],
-                        lambda k, d: sparse_map_value.__setitem__(
-                            k, d.read_optional(STRING, d.read_string)
-                        ),
+                        _read_optional_map,
                     )
                     kwargs["sparse_map_member"] = sparse_map_value
                 case _:

python-packages/smithy-json/tests/unit/test_deserializers.py

@@ -3,6 +3,7 @@
 from typing import Any
 
 import pytest
+from smithy_core.deserializers import ShapeDeserializer
 from smithy_core.documents import Document
 from smithy_core.prelude import (
     BIG_DECIMAL,
@@ -30,7 +31,7 @@ def test_json_deserializer(expected: Any, given: bytes) -> None:
     deserializer = codec.create_deserializer(given)
     match expected:
         case None:
-            actual = deserializer.read_null(STRING)
+            actual = deserializer.read_null()
         case bool():
             actual = deserializer.read_boolean(BOOLEAN)
         case int():
@@ -49,18 +50,32 @@ def test_json_deserializer(expected: Any, given: bytes) -> None:
             actual = deserializer.read_document(expected._schema)  # type: ignore
         case list():
             actual_list: list[str | None] = []
+
+            def _read_optional_list(d: ShapeDeserializer):
+                if d.is_null():
+                    d.read_null()
+                    actual_list.append(None)
+                else:
+                    actual_list.append(d.read_string(STRING))
+
             deserializer.read_list(
                 SPARSE_STRING_LIST_SCHEMA,
-                lambda d: actual_list.append(d.read_optional(STRING, d.read_string)),
+                _read_optional_list,
             )
             actual = actual_list
         case dict():
             actual_map: dict[str, str | None] = {}
+
+            def _read_optional_map(k: str, d: ShapeDeserializer):
+                if d.is_null():
+                    d.read_null()
+                    actual_map[k] = None
+                else:
+                    actual_map[k] = d.read_string(STRING)
+
             deserializer.read_map(
                 SPARSE_STRING_MAP_SCHEMA,
-                lambda k, d: actual_map.__setitem__(
-                    k, d.read_optional(STRING, d.read_string)
-                ),
+                _read_optional_map,
             )
             actual = actual_map
         case SerdeShape():