Merge pull request #381 from philippeitis/code-quality

Code quality

src/generator/lib/util.py

@@ -1242,13 +1242,14 @@ def size_to_bytes(size):
 
 
 def string_impl(p):
+    """Returns a function which will convert instances of p to a string"""
     return {
-        "google-duration": "::client::serde::duration::to_string",
-        "byte": "::client::serde::urlsafe_base64::to_string",
-        "google-datetime": "::client::serde::datetime_to_string",
-        "date-time": "::client::serde::datetime_to_string",
-        "google-fieldmask": "(|x: &client::FieldMask| x.to_string())"
-    }.get(p.get("format"), "(|x: &dyn std::fmt::Display| x.to_string())")
+        "google-duration": lambda x: f"::client::serde::duration::to_string(&{x})",
+        "byte": lambda x: f"::client::serde::urlsafe_base64::to_string(&{x})",
+        "google-datetime": lambda x: f"::client::serde::datetime_to_string(&{x})",
+        "date-time": lambda x: f"::client::serde::datetime_to_string(&{x})",
+        "google-fieldmask": lambda x: f"{x}.to_string()"
+    }.get(p.get("format"), lambda x: f"{x}.to_string()")
 
 
 if __name__ == '__main__':

src/generator/templates/api/api.rs.mako

@@ -18,7 +18,7 @@
 use std::collections::HashMap;
 use std::cell::RefCell;
 use std::default::Default;
-use std::collections::BTreeMap;
+use std::collections::BTreeSet;
 use std::error::Error as StdError;
 use serde_json as json;
 use std::io;

src/generator/templates/api/lib/mbuild.mako

@@ -126,7 +126,7 @@ pub struct ${ThisType}
     ${api.properties.params}: HashMap<String, String>,
     % if method_default_scope(m):
 ## We need the scopes sorted, to not unnecessarily query new tokens
-    ${api.properties.scopes}: BTreeMap<String, ()>
+    ${api.properties.scopes}: BTreeSet<String>
     % endif
 }
 
@@ -188,9 +188,8 @@ ${self._setter_fn(resource, method, m, p, part_prop, ThisType, c)}\
     pub fn ${ADD_SCOPE_FN}<T, St>(mut self, scope: T) -> ${ThisType}
                                                         where T: Into<Option<St>>,
                                                               St: AsRef<str> {
-        match scope.into() {
-          Some(scope) => self.${api.properties.scopes}.insert(scope.as_ref().to_string(), ()),
-          None => None,
+        if let Some(scope) = scope.into() {
+          self.${api.properties.scopes}.insert(scope.as_ref().to_string());
         };
         self
     }
@@ -557,15 +556,15 @@ match result {
         % if p.get('repeated', False):
         if ${pname}.len() > 0 {
             for f in ${pname}.iter() {
-                params.push(("${p.name}", ${to_string_impl}(f)));
+                params.push(("${p.name}", ${to_string_impl("f")}));
             }
         }
         % elif not is_required_property(p):
         if let Some(value) = ${pname}.as_ref() {
-            params.push(("${p.name}", ${to_string_impl}(value)));
+            params.push(("${p.name}", ${to_string_impl("value")}));
         }
         % else:
-        params.push(("${p.name}", ${to_string_impl}(&${pname})));
+        params.push(("${p.name}", ${to_string_impl(pname)}));
         % endif
         % endfor
         ## Additional params - may not overlap with optional params
@@ -583,17 +582,15 @@ match result {
         % if supports_download:
         let (json_field_missing, enable_resource_parsing) = {
             let mut enable = true;
-            let mut field_present = true;
+            let mut field_missing = true;
             for &(name, ref value) in params.iter() {
                 if name == "alt" {
-                    field_present = false;
-                    if <String as AsRef<str>>::as_ref(&value) != "json" {
-                        enable = false;
-                    }
+                    field_missing = false;
+                    enable = value == "json";
                     break;
                 }
             }
-            (field_present, enable)
+            (field_missing, enable)
         };
         if json_field_missing {
             params.push(("alt", "json".to_string()));
@@ -637,8 +634,8 @@ else {
         }
         % endif
         % else:
-        if self.${api.properties.scopes}.len() == 0 {
-            self.${api.properties.scopes}.insert(${scope_url_to_variant(name, default_scope, fully_qualified=True)}.as_ref().to_string(), ());
+        if self.${api.properties.scopes}.is_empty() {
+            self.${api.properties.scopes}.insert(${scope_url_to_variant(name, default_scope, fully_qualified=True)}.as_ref().to_string());
         }
         % endif
 
@@ -671,15 +668,8 @@ else {
         }
         ## Remove all used parameters
         {
-            let mut indices_for_removal: Vec<usize> = Vec::with_capacity(${len(replacements)});
-            for param_name in [${', '.join(reversed(['"%s"' % r[1] for r in replacements]))}].iter() {
-                if let Some(index) = params.iter().position(|t| &t.0 == param_name) {
-                    indices_for_removal.push(index);
-                }
-            }
-            for &index in indices_for_removal.iter() {
-                params.remove(index);
-            }
+            let to_remove = [${', '.join(reversed(['"%s"' % r[1] for r in replacements]))}];
+            params.retain(|(n, _)| !to_remove.contains(n));
         }
         % endif
 
@@ -707,7 +697,7 @@ else {
 
         loop {
             % if default_scope:
-            let token = match ${auth_call}.get_token(&self.${api.properties.scopes}.keys().map(String::as_str).collect::<Vec<_>>()[..]).await {
+            let token = match ${auth_call}.get_token(&self.${api.properties.scopes}.iter().map(String::as_str).collect::<Vec<_>>()[..]).await {
                 // TODO: remove Ok / Err branches
                 Ok(Some(token)) => token.clone(),
                 Ok(None) => {