Generate correct docstrings for paging and LRO operations

ChangeLog.md

@@ -1,5 +1,12 @@
 # Change Log
 
+### Unreleased
+Modelerfour version: 4.13.351
+
+**Bug Fixes**
+- Generating correct formatting for LRO and paging operation docstrings  #652
+- Generating correct content and formatting for LRO and paging operations in multiapi mixin  #652
+
 ### 2020-06-03 - 5.1.0-preview.1
 Modelerfour version: 4.13.351
 

autorest/codegen/templates/lro_operation.py.jinja2

@@ -4,7 +4,7 @@
 {% set async_prefix = "Async" if async_mode else "" %}
 {% set poller = "AsyncLROPoller" if async_mode else "LROPoller" %}
 {% set operation_name = "begin_"+operation.python_name %}
-{% macro return_docstring() %}
+{% macro return_docstring(async_mode) %}
 :return: An instance of {{ "Async" if async_mode }}LROPoller that returns either {{ operation.responses[0].schema.docstring_text if operation.responses[0].has_body else "None"}} or the result of cls(response)
 :rtype: ~azure.core.polling.{{ "Async" if async_mode }}LROPoller[{{ operation.responses[0].schema.docstring_type if operation.responses[0].has_body else "None" }}]{% endmacro %}
 {% macro param_documentation_string(parameter) %}:param {{ parameter.serialized_name }}: {{ parameter.description }}{% endmacro %}
@@ -15,6 +15,33 @@ response_headers = {
     {% endfor %}
 }
 {% endmacro %}
+{% macro operation_docstring(async_mode) %}
+"""{{ operation.summary if operation.summary else operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
+{% if operation.summary and operation.description %}
+
+{{ operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
+{% endif %}
+
+{% if operation.deprecated -%}
+.. warning::
+    This method is deprecated
+
+{% endif -%}
+{% for parameter in operation.parameters.method %}
+{%- for doc_string in param_documentation_string(parameter).replace('\n', '\n ').split('\n') %}
+{{ doc_string | wordwrap(width=95, break_long_words=False, wrapstring='\n ')}}
+{% endfor %}
+:type {{ parameter.serialized_name }}: {{ parameter.schema.docstring_type }}
+{% endfor %}
+:keyword callable cls: A custom type or function that will be passed the direct response
+:keyword str continuation_token: A continuation token to restart a poller from a saved state.
+:keyword polling: True for ARMPolling, False for no polling, or a
+ polling object for personal polling strategy
+:paramtype polling: bool or ~azure.core.polling.{{ "Async" if async_mode else "" }}PollingMethod
+:keyword int polling_interval: Default waiting time between two polls for LRO operations if no Retry-After header is present.
+{{ return_docstring(async_mode) }}
+:raises ~azure.core.exceptions.HttpResponseError:
+"""{% endmacro %}
 {% set lro_options = (", lro_options={'final-state-via': '"+ operation.lro_options['final-state-via'] + "'}") if operation.lro_options else "" %}
 {# actual template starts here #}
 {% if code_model.options['tracing'] %}
@@ -25,32 +52,7 @@ response_headers = {
 {%- if not async_mode %}
     {{ 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 %}
-
-    {{ operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
-    {% endif %}
-
-{% if operation.deprecated -%}
-    .. warning::
-       This method is deprecated
-
-{% endif -%}
-{% for parameter in operation.parameters.method %}
-    {%- for doc_string in param_documentation_string(parameter).replace('\n', '\n ').split('\n') %}
-    {{ doc_string | wordwrap(width=95, break_long_words=False, wrapstring='\n ')}}
-    {% endfor %}
-    :type {{ parameter.serialized_name }}: {{ parameter.schema.docstring_type }}
-{% endfor %}
-    :keyword callable cls: A custom type or function that will be passed the direct response
-    :keyword str continuation_token: A continuation token to restart a poller from a saved state.
-    :keyword polling: True for ARMPolling, False for no polling, or a
-     polling object for personal polling strategy
-    :paramtype polling: bool or ~azure.core.polling.{{ "Async" if async_mode else "" }}PollingMethod
-    :keyword int polling_interval: Default waiting time between two polls for LRO operations if no Retry-After header is present.
-    {{ return_docstring()|indent }}
-    :raises ~azure.core.exceptions.HttpResponseError:
-    """
+    {{ operation_docstring(async_mode) | indent }}
     polling = kwargs.pop('polling', {{ "True" if code_model.options['azure_arm'] else "False" }})  # type: Union[bool, {{ "Async" if async_mode else "" }}PollingMethod]
     cls = kwargs.pop('cls', None)  # type: ClsType[{{ op_tools.return_type_annotation(operation) }}]
     lro_delay = kwargs.pop(

autorest/codegen/templates/metadata.json.jinja2

@@ -54,15 +54,30 @@
         {{ operation_name | tojson }} : {
             "sync": {
                 {% set sync_return_type_wrapper = "LROPoller" if is_lro(operation) else ("ItemPaged" if is_paging(operation) else "") %}
-                "signature": {{ op_tools.method_signature(operation, operation_name, False, False, sync_return_type_wrapper) | tojson }}
+                "signature": {{ op_tools.method_signature(operation, operation_name, False, False, sync_return_type_wrapper) | tojson }},
+                {% if is_lro(operation) %}
+                    {% from "lro_operation.py.jinja2" import operation_docstring with context %}
+                {% elif is_paging(operation) %}
+                    {% from "paging_operation.py.jinja2" import operation_docstring with context %}
+                {% else %}
+                    {% from "operation.py.jinja2" import operation_docstring with context %}
+                {% endif %}
+                "doc": {{ operation_docstring(async_mode=False) | tojson }}
             },
             "async": {
                 {% set coroutine = False if is_paging(operation) else True %}
                 {% set async_return_type_wrapper = "AsyncLROPoller" if is_lro(operation) else ("AsyncItemPaged" if is_paging(operation) else "") %}
                 "signature": {{ op_tools.method_signature(operation, operation_name, True, coroutine, async_return_type_wrapper) | tojson }},
-                "coroutine": {{ coroutine | tojson }}
+                "coroutine": {{ coroutine | tojson }},
+                {% if is_lro(operation) %}
+                    {% from "lro_operation.py.jinja2" import operation_docstring with context %}
+                {% elif is_paging(operation) %}
+                    {% from "paging_operation.py.jinja2" import operation_docstring with context %}
+                {% else %}
+                    {% from "operation.py.jinja2" import operation_docstring with context %}
+                {% endif %}
+                "doc": {{ operation_docstring(async_mode=True) | tojson }}
             },
-            "doc": {{ op_tools.operation_docstring(operation) | tojson }},
             "call": {{ operation.parameters.method | map(attribute="serialized_name") | join(', ') | tojson }}
         }{{ "," if not loop.last else "" }}
         {% endfor %}

autorest/codegen/templates/operation.py.jinja2

@@ -2,6 +2,52 @@
 {% import 'operation_tools.jinja2' as op_tools %}
 {% set trace_decorator = "@distributed_trace_async" if async_mode else "@distributed_trace" %}
 {% set stream_request_parameter = "stream=" ~ ("True" if operation.is_stream_response else "False") %}
+{% macro return_docstring(async_mode) %}
+{% 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 }}
+{%- else %}
+:return: None, or the result of cls(response)
+:rtype: None
+{%- endif -%}
+{% endmacro %}
+{% macro param_documentation_string(parameter) %}:param {{ parameter.serialized_name }}: {{ parameter.description }}{% endmacro %}
+{% macro return_docstring(async_mode) %}
+{% 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.has_optional_return_type }}
+{%- else %}
+:return: None, or the result of cls(response)
+:rtype: None
+{%- endif -%}
+{% endmacro %}
+{% macro operation_docstring(async_mode) %}
+"""{{ operation.summary if operation.summary else operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
+{% if operation.summary and operation.description %}
+
+{{ operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
+{% endif %}
+
+{% if operation.deprecated -%}
+.. warning::
+    This method is deprecated
+
+{% endif -%}
+{% for parameter in operation.parameters.method %}
+{%- for doc_string in param_documentation_string(parameter).replace('\n', '\n ').split('\n') %}
+{{ doc_string | wordwrap(width=95, break_long_words=False, wrapstring='\n ')}}
+{% endfor %}
+:type {{ parameter.serialized_name }}: {{ parameter.docstring_type }}
+{% endfor %}
+{% if (operation.requests | length) > 1 %}
+{% set content_type_constant = operation.parameters.constant|selectattr("implementation", "equalto", "Method")|selectattr("original_parameter", "equalto", None)|selectattr("in_method_code") | selectattr("serialized_name", "equalto", "content_type") | first  %}
+:keyword str content_type: Media type of the body sent to the API. Default value is {{ content_type_constant.schema.constant_value }}.
+ Allowed values are: "{{ operation.requests | map(attribute="media_types") | sum(start = []) | unique | list | join ('", "')  }}".
+{% endif %}
+:keyword callable cls: A custom type or function that will be passed the direct response
+{{ return_docstring(async_mode) }}
+:raises: ~azure.core.exceptions.HttpResponseError
+"""{% endmacro %}
 {# actual template starts here #}
 {%- if code_model.options['tracing'] and operation.want_tracing -%}
 {{ trace_decorator }}
@@ -11,7 +57,7 @@
     {{ op_tools.sync_return_type_annotation(operation, "") }}
 {% endif %}
 {% if operation.want_description_docstring %}
-{{ op_tools.operation_docstring(operation)|indent }}
+    {{ operation_docstring(async_mode)|indent }}
 {% endif %}
     cls = kwargs.pop('cls', None)  # type: {{ op_tools.return_type_annotation(operation, "ClsType") }}
 {% if operation.deprecated %}

autorest/codegen/templates/operation_tools.jinja2

@@ -1,41 +1,3 @@
-{% macro param_documentation_string(parameter) %}:param {{ parameter.serialized_name }}: {{ parameter.description }}{% endmacro %}
-{% 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.has_optional_return_type }}
-{%- else %}
-:return: None, or the result of cls(response)
-:rtype: None
-{%- endif -%}
-{% endmacro %}
-{# operation docstring. Used only for metadata and operation templates #}
-{% macro operation_docstring(operation) %}
-    """{{ operation.summary if operation.summary else operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
-{% if operation.summary and operation.description %}
-
-{{ operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
-{% endif %}
-
-{% if operation.deprecated -%}
-.. warning::
-    This method is deprecated
-
-{% endif -%}
-{% for parameter in operation.parameters.method %}
-{%- for doc_string in param_documentation_string(parameter).replace('\n', '\n ').split('\n') %}
-{{ doc_string | wordwrap(width=95, break_long_words=False, wrapstring='\n ')}}
-{% endfor %}
-:type {{ parameter.serialized_name }}: {{ parameter.docstring_type }}
-{% endfor %}
-{% if (operation.requests | length) > 1 %}
-{% set content_type_constant = operation.parameters.constant|selectattr("implementation", "equalto", "Method")|selectattr("original_parameter", "equalto", None)|selectattr("in_method_code") | selectattr("serialized_name", "equalto", "content_type") | first  %}
-:keyword str content_type: Media type of the body sent to the API. Default value is {{ content_type_constant.schema.constant_value }}.
- Allowed values are: "{{ operation.requests | map(attribute="media_types") | sum(start = []) | unique | list | join ('", "')  }}".
-{% endif %}
-:keyword callable cls: A custom type or function that will be passed the direct response
-{{ return_docstring(operation) }}
-:raises: ~azure.core.exceptions.HttpResponseError
-"""{% endmacro %}
 {% macro return_type_annotation(operation, return_type_wrapper) %}
 {{ ((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(', ')) ~

autorest/codegen/templates/paging_operation.py.jinja2

@@ -4,13 +4,36 @@
 {% set request_as_xml = ", is_xml=True" if send_xml else "" %}
 {% set stream_request_parameter = "stream=" ~ ("True" if operation.is_stream_response else "False") %}
 {% set item_paged = "AsyncItemPaged" if async_mode else "ItemPaged" %}
-{% macro return_docstring() %}
+{% macro return_docstring(async_mode) %}
 {% if operation.responses | selectattr('has_body') | first %}
 :return: An iterator like instance of either {{ operation.responses|selectattr('has_body')|map(attribute='schema')|map(attribute='docstring_text')|unique|join(' or ') }} or the result of cls(response)
-:rtype: ~azure.core.{{ "async_" if async_mode else "" }}paging.{{ item_paged }}[{% for response in operation.responses %}{{response.schema.docstring_type if response.has_body else "None"}}{% if not loop.last -%} or {% endif %}{% endfor %}]
-{% else %}
+{# can't use item_paged variable, otherwise the call to get docstring from the metadata template will always return ItemPaged #}
+:rtype: ~azure.core.{{ "async_" if async_mode else "" }}paging.{{ "Async" if async_mode }}ItemPaged[{% for response in operation.responses %}{{response.schema.docstring_type if response.has_body else "None"}}{% if not loop.last -%} or {% endif %}{% endfor %}]
+{%- else -%}
 :return: None
-:rtype: None{% endif %}{% endmacro %}
+:rtype: None{%- endif -%}{%- endmacro -%}
+{% macro operation_docstring(async_mode) %}
+"""{{ operation.summary if operation.summary else operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
+{% if operation.summary and operation.description %}
+
+{{ operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
+{% endif %}
+
+{% if operation.deprecated -%}
+.. warning::
+    This method is deprecated
+
+{% endif -%}
+{% for parameter in operation.parameters.method %}
+{%- for doc_string in param_documentation_string(parameter).replace('\n', '\n ').split('\n') %}
+{{ doc_string | wordwrap(width=95, break_long_words=False, wrapstring='\n ')}}
+{% endfor %}
+:type {{ parameter.serialized_name }}: {{ parameter.schema.docstring_type }}
+{% endfor %}
+:keyword callable cls: A custom type or function that will be passed the direct response
+{{ return_docstring(async_mode) }}
+:raises: ~azure.core.exceptions.HttpResponseError
+"""{% endmacro %}
 {% macro param_documentation_string(parameter) %}:param {{ parameter.serialized_name }}: {{ parameter.description }}{% endmacro %}
 {% set next_link_str = "deserialized." + operation.next_link_name + " or None" if operation.next_link_name else "None" %}
 {# actual template starts here #}
@@ -23,26 +46,7 @@
     {{ op_tools.sync_return_type_annotation(operation, return_type_wrapper) }}
 {% endif %}
 {% if operation.want_description_docstring %}
-    """{{ operation.summary if operation.summary else operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
-    {% if operation.summary and operation.description %}
-
-    {{ operation.description | wordwrap(width=95, break_long_words=False, wrapstring='\n') }}
-    {% endif %}
-
-{% if operation.deprecated -%}
-    .. warning::
-       This method is deprecated
-
-{% endif -%}
-{% for parameter in operation.parameters.method %}
-    {%- for doc_string in param_documentation_string(parameter).replace('\n', '\n ').split('\n') %}
-    {{ doc_string | wordwrap(width=95, break_long_words=False, wrapstring='\n ')}}
-    {% endfor %}
-    :type {{ parameter.serialized_name }}: {{ parameter.schema.docstring_type }}
-{% endfor %}
-    :keyword callable cls: A custom type or function that will be passed the direct response
-    {{ return_docstring()|indent }}    :raises: ~azure.core.exceptions.HttpResponseError
-    """
+    {{ operation_docstring(async_mode) | indent }}
 {% endif %}
 {% if operation.deprecated %}
     warnings.warn('Method {{operation.name}} is deprecated', DeprecationWarning)

autorest/multiapi/__init__.py

@@ -182,13 +182,13 @@ def _build_operation_mixin_meta(self, paths_to_versions: List[Path]) -> Dict[str
                 mixin_operations.setdefault(func_name, {}).setdefault('async', {})
                 mixin_operations[func_name]['sync'].update({
                     "signature": func['sync']['signature'],
-                    "doc": func['doc'],
+                    "doc": func['sync']['doc'],
                     "call": func['call']
                 })
                 mixin_operations[func_name]['async'].update({
                     "signature": func['async']['signature'],
                     "coroutine": func['async']['coroutine'],
-                    "doc": func['doc'],
+                    "doc": func['async']['doc'],
                     "call": func['call']
                 })
                 mixin_operations[func_name]['sync'].setdefault(

autorest/multiapi/templates/multiapi_operations_mixin.py.jinja2

@@ -19,7 +19,7 @@ class {{ client_name }}OperationsMixin(object):
 {% for operation_name, metadata_sync_and_async in mixin_operations|dictsort %}
 
     {% set metadata = metadata_sync_and_async['async'] if async_mode else metadata_sync_and_async['sync'] %}
-    {{ metadata['signature'] | indent }}    {{ metadata['doc'] | indent(8) }}
+    {{ metadata['signature'] | indent }}        {{ metadata['doc'] | indent(8) }}
         api_version = self._get_api_version('{{ operation_name }}')
         {% for api in metadata['available_apis']|sort %}
         {% set if_statement = "if" if loop.first else "elif" %}

test/azure/Expected/AcceptanceTests/CustomUrlPaging/custombaseurlpaging/aio/operations_async/_paging_operations_async.py

@@ -50,7 +50,7 @@ def get_pages_partial_url(
         **kwargs
     ) -> AsyncIterable["models.ProductResult"]:
         """A paging operation that combines custom url, paging and partial URL and expect to concat after
-    host.
+        host.
 
         :param account_name: Account Name.
         :type account_name: str

test/azure/Expected/AcceptanceTests/CustomUrlPaging/custombaseurlpaging/operations/_paging_operations.py

@@ -54,7 +54,7 @@ def get_pages_partial_url(
     ):
         # type: (...) -> Iterable["models.ProductResult"]
         """A paging operation that combines custom url, paging and partial URL and expect to concat after
-    host.
+        host.
 
         :param account_name: Account Name.
         :type account_name: str