deal with optional return types correctly

ChangeLog.md

@@ -5,6 +5,8 @@ Modelerfour version: 4.13.351
 
 **Bug Fixes**
 - Corrected generation of the item name of paging response when extracting data  #648
+- Corrected return type typing annotation for operations that return an optional body  #656
+- Fixed mypy issue by only setting the generated `deserialized` variable to None if the operation has an optional return typ  #656
 
 ### 2020-05-22 - 5.0.0-preview.8
 Modelerfour version: 4.13.351

autorest/codegen/models/lro_operation.py

@@ -79,6 +79,11 @@ def set_lro_response_type(self) -> None:
             response_type = response_types[0]
         self.lro_response = response_type
 
+    @property
+    def has_optional_return_type(self) -> bool:
+        """An LROOperation will never have an optional return type, we will always return LROPoller[return type]"""
+        return False
+
     def imports(self, code_model, async_mode: bool) -> FileImport:
         file_import = super().imports(code_model, async_mode)
         file_import.add_from_import("typing", "Union", ImportType.STDLIB, TypingSection.CONDITIONAL)

autorest/codegen/models/operation.py

@@ -165,6 +165,30 @@ def is_stream_response(self) -> bool:
         """Is the response expected to be streamable, like a download."""
         return any(response.is_stream_response for response in self.responses)
 
+    @property
+    def has_optional_return_type(self) -> bool:
+        """Has optional return type if there are multiple successful response types where some have
+        bodies and some are None
+        """
+
+        # successful status codes of responses that have bodies
+        status_codes_for_responses_with_bodies = [
+            code for code in self.success_status_code
+            if isinstance(code, int) and self.get_response_from_status(code).has_body
+        ]
+
+        successful_responses = [
+            response for response in self.responses
+            if any(code in self.success_status_code for code in response.status_codes)
+        ]
+
+        return (
+            self.has_response_body and
+            len(successful_responses) > 1 and
+            len(self.success_status_code) != len(status_codes_for_responses_with_bodies)
+        )
+
+
     @staticmethod
     def build_serialize_data_call(parameter: Parameter, function_name: str) -> str:
 

autorest/codegen/models/paging_operation.py

@@ -93,6 +93,11 @@ def next_link_name(self) -> Optional[str]:
             return None
         return self._find_python_name(self._next_link_name, "nextLinkName")
 
+    @property
+    def has_optional_return_type(self) -> bool:
+        """A paging will never have an optional return type, we will always return ItemPaged[return type]"""
+        return False
+
     def imports(self, code_model, async_mode: bool) -> FileImport:
         file_import = super(PagingOperation, self).imports(code_model, async_mode)
 

autorest/codegen/templates/lro_operation.py.jinja2

@@ -23,8 +23,7 @@ response_headers = {
 {% set return_type_wrapper = "" if async_mode else "LROPoller" %}
 {{ op_tools.method_signature(operation, operation_name, async_mode=async_mode, coroutine=async_mode, return_type_wrapper=return_type_wrapper) }}
 {%- if not async_mode %}
-{# overriding sync_return_type_annotation because we know if it's sync, it's return type is just LROPoller #}
-    # type: (...) -> LROPoller
+    {{ op_tools.sync_return_type_annotation(operation, return_type_wrapper) }}
 {% endif %}
     """{{ operation.summary if operation.summary else operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
     {% if operation.summary and operation.description %}

autorest/codegen/templates/operation.py.jinja2

@@ -62,7 +62,9 @@
 {% if operation.any_response_has_headers %}
     response_headers = {}
 {% endif %}
-{% if operation.has_response_body and operation.responses|count > 1 %}
+{# now we only initialize deserialized to None if we know there is both > 1 response with body and > 1 response of None #}
+{# otherwise, we know that deserialized will be set to a value then returned #}
+{% if operation.has_optional_return_type %}
     deserialized = None
 {% endif %}
 {% if operation.has_response_body or operation.any_response_has_headers %}

autorest/codegen/templates/operation_tools.jinja2

@@ -2,7 +2,7 @@
 {% macro return_docstring(operation) %}
 {% if operation.responses | selectattr('has_body') | first %}
 :return: {{ operation.responses|selectattr('has_body')|map(attribute='schema')| map(attribute='docstring_text')|unique|join(' or ') }}, or the result of cls(response)
-:rtype: {{ operation.responses|selectattr('has_body')|map(attribute='schema')| map(attribute='docstring_type')|unique|join(' or ') }}{{ " or None" if operation.responses|selectattr('has_body', 'false') | first }}
+:rtype: {{ operation.responses|selectattr('has_body')|map(attribute='schema')| map(attribute='docstring_type')|unique|join(' or ') }}{{ " or None" if operation.has_optional_return_type }}
 {%- else %}
 :return: None, or the result of cls(response)
 :rtype: None
@@ -37,11 +37,11 @@
 :raises: ~azure.core.exceptions.HttpResponseError
 """{% endmacro %}
 {% macro return_type_annotation(operation, return_type_wrapper) %}
-{{ ((return_type_wrapper + "[") if return_type_wrapper else "") ~ ("Union[" if operation.responses | selectattr('has_body') | map(attribute='schema') | map(attribute='operation_type_annotation') | unique | list  | length > 1 else "") ~
+{{ ((return_type_wrapper + "[") if return_type_wrapper else "") ~ ("Optional[" if operation.has_optional_return_type else "" ) ~ ("Union[" if operation.responses | selectattr('has_body') | map(attribute='schema') | map(attribute='operation_type_annotation') | unique | list  | length > 1 else "") ~
 (operation.responses | selectattr('has_body') | map(attribute='schema') | map(attribute='operation_type_annotation') | unique | join(', ')) ~
-("]" if operation.responses | selectattr('has_body') | map(attribute='schema') | map(attribute='operation_type_annotation')| unique | list | length > 1 else "") ~ ( "]" if return_type_wrapper else "")
+("]" if operation.responses | selectattr('has_body') | map(attribute='schema') | map(attribute='operation_type_annotation')| unique | list | length > 1 else "") ~ ("]" if operation.has_optional_return_type else "") ~ ( "]" if return_type_wrapper else "")
 if operation.responses | selectattr('has_body') | first
-else ((return_type_wrapper + "[") if return_type_wrapper else "") ~ "None" ~ ( "]" if return_type_wrapper else "") }}{% endmacro %}
+else ((return_type_wrapper + "[") if return_type_wrapper else "") ~ ("Optional[" if operation.has_optional_return_type else "" ) ~ "None" ~ ("]" if operation.has_optional_return_type else "") ~ ( "]" if return_type_wrapper else "") }}{% endmacro %}
 {# get async mypy typing #}
 {% macro async_return_type_annotation(operation, return_type_wrapper) %}
 {{ " -> " + return_type_annotation(operation, return_type_wrapper) }}{% endmacro %}

test/azure/Expected/AcceptanceTests/Lro/lro/aio/operations_async/_lr_os_custom_header_operations_async.py

@@ -184,7 +184,6 @@ async def _put201_creating_succeeded200_initial(
             map_error(status_code=response.status_code, response=response, error_map=error_map)
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 

test/azure/Expected/AcceptanceTests/Lro/lro/aio/operations_async/_lro_retrys_operations_async.py

@@ -80,7 +80,6 @@ async def _put201_creating_succeeded200_initial(
             map_error(status_code=response.status_code, response=response, error_map=error_map)
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -273,7 +272,6 @@ async def _delete_provisioning202_accepted200_succeeded_initial(
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
         response_headers = {}
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 

test/azure/Expected/AcceptanceTests/Lro/lro/aio/operations_async/_lros_operations_async.py

@@ -47,8 +47,8 @@ async def _put200_succeeded_initial(
         self,
         product: Optional["models.Product"] = None,
         **kwargs
-    ) -> "models.Product":
-        cls = kwargs.pop('cls', None)  # type: ClsType["models.Product"]
+    ) -> Optional["models.Product"]:
+        cls = kwargs.pop('cls', None)  # type: ClsType[Optional["models.Product"]]
         error_map = {404: ResourceNotFoundError, 409: ResourceExistsError}
         error_map.update(kwargs.pop('error_map', {}))
         content_type = kwargs.pop("content_type", "application/json")
@@ -234,8 +234,8 @@ def get_long_running_output(pipeline_response):
     async def _post202_list_initial(
         self,
         **kwargs
-    ) -> List["models.Product"]:
-        cls = kwargs.pop('cls', None)  # type: ClsType[List["models.Product"]]
+    ) -> Optional[List["models.Product"]]:
+        cls = kwargs.pop('cls', None)  # type: ClsType[Optional[List["models.Product"]]]
         error_map = {404: ResourceNotFoundError, 409: ResourceExistsError}
         error_map.update(kwargs.pop('error_map', {}))
 
@@ -541,7 +541,6 @@ async def _put201_creating_succeeded200_initial(
             map_error(status_code=response.status_code, response=response, error_map=error_map)
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -734,7 +733,6 @@ async def _put201_creating_failed200_initial(
             map_error(status_code=response.status_code, response=response, error_map=error_map)
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -1895,7 +1893,6 @@ async def _delete_provisioning202_accepted200_succeeded_initial(
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
         response_headers = {}
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -1987,7 +1984,6 @@ async def _delete_provisioning202_deleting_failed200_initial(
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
         response_headers = {}
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -2079,7 +2075,6 @@ async def _delete_provisioning202_deletingcanceled200_initial(
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
         response_headers = {}
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -2217,8 +2212,8 @@ def get_long_running_output(pipeline_response):
     async def _delete202_retry200_initial(
         self,
         **kwargs
-    ) -> "models.Product":
-        cls = kwargs.pop('cls', None)  # type: ClsType["models.Product"]
+    ) -> Optional["models.Product"]:
+        cls = kwargs.pop('cls', None)  # type: ClsType[Optional["models.Product"]]
         error_map = {404: ResourceNotFoundError, 409: ResourceExistsError}
         error_map.update(kwargs.pop('error_map', {}))
 
@@ -2303,8 +2298,8 @@ def get_long_running_output(pipeline_response):
     async def _delete202_no_retry204_initial(
         self,
         **kwargs
-    ) -> "models.Product":
-        cls = kwargs.pop('cls', None)  # type: ClsType["models.Product"]
+    ) -> Optional["models.Product"]:
+        cls = kwargs.pop('cls', None)  # type: ClsType[Optional["models.Product"]]
         error_map = {404: ResourceNotFoundError, 409: ResourceExistsError}
         error_map.update(kwargs.pop('error_map', {}))
 
@@ -2873,7 +2868,6 @@ async def _post200_with_payload_initial(
             map_error(status_code=response.status_code, response=response, error_map=error_map)
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Sku', pipeline_response)
 
@@ -3364,8 +3358,8 @@ async def _post_async_retry_succeeded_initial(
         self,
         product: Optional["models.Product"] = None,
         **kwargs
-    ) -> "models.Product":
-        cls = kwargs.pop('cls', None)  # type: ClsType["models.Product"]
+    ) -> Optional["models.Product"]:
+        cls = kwargs.pop('cls', None)  # type: ClsType[Optional["models.Product"]]
         error_map = {404: ResourceNotFoundError, 409: ResourceExistsError}
         error_map.update(kwargs.pop('error_map', {}))
         content_type = kwargs.pop("content_type", "application/json")
@@ -3466,8 +3460,8 @@ async def _post_async_no_retry_succeeded_initial(
         self,
         product: Optional["models.Product"] = None,
         **kwargs
-    ) -> "models.Product":
-        cls = kwargs.pop('cls', None)  # type: ClsType["models.Product"]
+    ) -> Optional["models.Product"]:
+        cls = kwargs.pop('cls', None)  # type: ClsType[Optional["models.Product"]]
         error_map = {404: ResourceNotFoundError, 409: ResourceExistsError}
         error_map.update(kwargs.pop('error_map', {}))
         content_type = kwargs.pop("content_type", "application/json")

test/azure/Expected/AcceptanceTests/Lro/lro/aio/operations_async/_lrosads_operations_async.py

@@ -80,7 +80,6 @@ async def _put_non_retry400_initial(
             map_error(status_code=response.status_code, response=response, error_map=error_map)
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -177,7 +176,6 @@ async def _put_non_retry201_creating400_initial(
             map_error(status_code=response.status_code, response=response, error_map=error_map)
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -275,7 +273,6 @@ async def _put_non_retry201_creating400_invalid_json_initial(
             map_error(status_code=response.status_code, response=response, error_map=error_map)
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -971,7 +968,6 @@ async def _put_error201_no_provisioning_state_payload_initial(
             map_error(status_code=response.status_code, response=response, error_map=error_map)
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -1571,8 +1567,8 @@ async def _put200_invalid_json_initial(
         self,
         product: Optional["models.Product"] = None,
         **kwargs
-    ) -> "models.Product":
-        cls = kwargs.pop('cls', None)  # type: ClsType["models.Product"]
+    ) -> Optional["models.Product"]:
+        cls = kwargs.pop('cls', None)  # type: ClsType[Optional["models.Product"]]
         error_map = {404: ResourceNotFoundError, 409: ResourceExistsError}
         error_map.update(kwargs.pop('error_map', {}))
         content_type = kwargs.pop("content_type", "application/json")

test/azure/Expected/AcceptanceTests/Lro/lro/operations/_lr_os_custom_header_operations.py

@@ -103,7 +103,7 @@ def begin_put_async_retry_succeeded(
         product=None,  # type: Optional["models.Product"]
         **kwargs  # type: Any
     ):
-        # type: (...) -> LROPoller
+        # type: (...) -> LROPoller["models.Product"]
         """x-ms-client-request-id = 9C4D50EE-2D56-4CD3-8152-34347DC9F2B0 is required message header for
     all requests. Long running put request, service returns a 200 to the initial request, with an
     entity that contains ProvisioningState=’Creating’. Poll the endpoint indicated in the Azure-
@@ -191,7 +191,6 @@ def _put201_creating_succeeded200_initial(
             map_error(status_code=response.status_code, response=response, error_map=error_map)
             raise HttpResponseError(response=response, error_format=ARMErrorFormat)
 
-        deserialized = None
         if response.status_code == 200:
             deserialized = self._deserialize('Product', pipeline_response)
 
@@ -210,7 +209,7 @@ def begin_put201_creating_succeeded200(
         product=None,  # type: Optional["models.Product"]
         **kwargs  # type: Any
     ):
-        # type: (...) -> LROPoller
+        # type: (...) -> LROPoller["models.Product"]
         """x-ms-client-request-id = 9C4D50EE-2D56-4CD3-8152-34347DC9F2B0 is required message header for
     all requests. Long running put request, service returns a 201 to the initial request, with an
     entity that contains ProvisioningState=’Creating’.  Polls return this value until the last poll
@@ -307,7 +306,7 @@ def begin_post202_retry200(
         product=None,  # type: Optional["models.Product"]
         **kwargs  # type: Any
     ):
-        # type: (...) -> LROPoller
+        # type: (...) -> LROPoller[None]
         """x-ms-client-request-id = 9C4D50EE-2D56-4CD3-8152-34347DC9F2B0 is required message header for
     all requests. Long running post request, service returns a 202 to the initial request, with
     'Location' and 'Retry-After' headers, Polls return a 200 with a response body after success.
@@ -401,7 +400,7 @@ def begin_post_async_retry_succeeded(
         product=None,  # type: Optional["models.Product"]
         **kwargs  # type: Any
     ):
-        # type: (...) -> LROPoller
+        # type: (...) -> LROPoller[None]
         """x-ms-client-request-id = 9C4D50EE-2D56-4CD3-8152-34347DC9F2B0 is required message header for
     all requests. Long running post request, service returns a 202 to the initial request, with an
     entity that contains ProvisioningState=’Creating’. Poll the endpoint indicated in the Azure-