Repository: Azure/autorest.python

Issue:
default value followed by non default value in operation signature
if you change cost management service swagger like this
<img width="770" alt="Screen Shot 2020-04-03 at 10 40 18 AM" src="https://user-images.githubusercontent.com/43154838/78372652-8760c900-7597-11ea-957f-8abf14e4582c.png">
code gen generates:
<img width="985" alt="Screen Shot 2020-04-03 at 10 40 52 AM" src="https://user-images.githubusercontent.com/43154838/78372709-9ba4c600-7597-11ea-9639-6828f881c775.png">


Discussion:

Patch:
diff --git a/ChangeLog.md b/ChangeLog.md
index cb8b2555c..0c66ad340 100644
--- a/ChangeLog.md
+++ b/ChangeLog.md
@@ -1,5 +1,11 @@
 # Change Log
 
+### Current
+
+**Bug Fixes**
+
+- Fixes parameter ordering so parameters with default values are all ordered at the end  #545
+
 ### 2020-04-01 - 5.0.0-dev.20200401.1
 Modelerfour version: 4.12.276
 
diff --git a/autorest/codegen/models/parameter.py b/autorest/codegen/models/parameter.py
index 3a841780e..08011c56b 100644
--- a/autorest/codegen/models/parameter.py
+++ b/autorest/codegen/models/parameter.py
@@ -75,7 +75,7 @@ class Parameter(BaseModel):  # pylint: disable=too-many-instance-attributes
         self.flattened = flattened
         self.grouped_by = grouped_by
         self.original_parameter = original_parameter
-        self.client_default_value = client_default_value
+        self._client_default_value = client_default_value
         self.is_kwarg: bool = False
         self.has_multiple_media_types: bool = False
         self.multiple_media_types_type_annot: Optional[str] = None
@@ -122,8 +122,8 @@ class Parameter(BaseModel):  # pylint: disable=too-many-instance-attributes
         if not self.required:
             type_annot = f"Optional[{type_annot}]"
 
-        if self.client_default_value is not None:
-            return self.client_default_value, self.schema.get_declaration(self.client_default_value), type_annot
+        if self._client_default_value is not None:
+            return self._client_default_value, self.schema.get_declaration(self._client_default_value), type_annot
 
         if self.multiple_media_types_type_annot:
             # means this parameter has multiple media types. We force default value to be None.
@@ -144,6 +144,12 @@ class Parameter(BaseModel):  # pylint: disable=too-many-instance-attributes
 
         return default_value, default_value_declaration, type_annot
 
+    @property
+    def default_value(self) -> Optional[Any]:
+        # exposing default_value because client_default_value doesn't get updated with
+        # default values we bubble up from the schema
+        return self._default_value()[0]
+
     @property
     def docstring_type(self) -> str:
         return self.multiple_media_types_docstring_type or self.schema.docstring_type
diff --git a/autorest/codegen/models/parameter_list.py b/autorest/codegen/models/parameter_list.py
index afa9c7de8..d88db7334 100644
--- a/autorest/codegen/models/parameter_list.py
+++ b/autorest/codegen/models/parameter_list.py
@@ -108,7 +108,7 @@ class ParameterList(MutableSequence):
         )
         for parameter in parameters_of_this_implementation:
             if parameter.in_method_signature:
-                if not parameter.client_default_value and parameter.required:
+                if not parameter.default_value and parameter.required:
                     signature_parameters_no_default_value.append(parameter)
                 else:
                     signature_parameters_default_value.append(parameter)


Test patch:
diff --git a/test/unittests/test_parameter_ordering.py b/test/unittests/test_parameter_ordering.py
index ce4e06df7..06a8739d3 100644
--- a/test/unittests/test_parameter_ordering.py
+++ b/test/unittests/test_parameter_ordering.py
@@ -1,13 +1,15 @@
 import pytest
 from autorest.codegen.models import Parameter, AnySchema
+from autorest.codegen.models.primitive_schemas import StringSchema
 from autorest.codegen.models.parameter_list import ParameterList
 from autorest.codegen.models.parameter import ParameterLocation
 
-def get_parameter(name, required, default_value=None):
-    schema = AnySchema(
-        namespace="parameterordering",
-        yaml_data={}
-    )
+def get_parameter(name, required, default_value=None, schema=None):
+    if not schema:
+        schema = AnySchema(
+            namespace="parameterordering",
+            yaml_data={}
+        )
     return Parameter(
         schema=schema,
         yaml_data={},
@@ -22,6 +24,25 @@ def get_parameter(name, required, default_value=None):
         client_default_value=default_value
     )
 
+def test_sort_parameters_with_default_value_from_schema():
+    schema = StringSchema(
+        namespace="parameterordering",
+        yaml_data={"defaultValue": "this_is_the_default", "type": str}
+    )
+    parameter_with_default_schema_value_required = get_parameter(
+        name="required_param_with_schema_default",
+        required=True,
+        schema=schema
+    )
+    required_parameter = get_parameter(
+        name="required_parameter",
+        required=True
+    )
+
+    parameter_list = [parameter_with_default_schema_value_required, required_parameter]
+
+    assert [required_parameter, parameter_with_default_schema_value_required] == ParameterList(parameter_list).method
+
 def test_sort_required_parameters():
     required_default_value_parameter = get_parameter(
         name="required_default_value_parameter",