Fix compiler-rs lint warnings (#2318)

* Fix clippy warnings and errors
* Do not ignore .d.ts files, update wasm module
* Update code owners and labeler config

.github/CODEOWNERS

@@ -4,6 +4,7 @@
 # Specification tooling owners
 /.github/                   @elastic/clients-team
 /compiler/                  @elastic/clients-team
+/compiler-rs/               @elastic/clients-team
 /typescript-generator/      @elastic/clients-team
 /specification/_json_spec/  @elastic/clients-team
 /specification/_spec_utils/ @elastic/clients-team

.github/labeler.yml

@@ -9,3 +9,4 @@ generator:typescript:
 
 compiler:
   - 'compiler/**/*'
+  - 'compiler-rs/**/*'

.gitignore

@@ -40,8 +40,6 @@ node_modules
 # Build output directory
 dist/
 flight-recorder-generator/output
-*.d.ts
-java-generator/*.js
 specification/**/*.js
 specification/lib
 

compiler-rs/.gitignore

@@ -1,2 +1,2 @@
 .idea
-target
+target/

compiler-rs/clients_schema/src/lib.rs

@@ -15,7 +15,6 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use std::cmp::Ordering;
 use std::fmt::{Debug, Display, Formatter};
 use anyhow::anyhow;
 use indexmap::IndexMap;
@@ -37,24 +36,26 @@ const fn is_false(v: &bool) -> bool {
     !(*v)
 }
 
+// String type where efficient cloning brings significant improvements
+// ArcStr is a compact reference counted string that makes cloning a very cheap operation.
+// This is particularly important for `TypeName` that is cloned a lot, e.g. for `IndexedModel` keys
+// See https://swatinem.de/blog/optimized-strings/ for a general discussion
+//
+// TODO: interning at deserialization time to reuse identical values (links from values to types)
+type FastString = arcstr::ArcStr;
+
 pub trait  Documented {
     fn doc_url(&self) -> Option<&str>;
     fn doc_id(&self) -> Option<&str>;
     fn description(&self) -> Option<&str>;
     fn since(&self) -> Option<&str>;
 }
 
-// ArcStr is a compact reference counted string that makes cloning a very cheap operation.
-// This is particularly important for `TypeName` that is cloned a lot, e.g. for `IndexedModel` keys
-// See https://swatinem.de/blog/optimized-strings/ for a general discussion
-//
-// TODO: interning at deserialization time to reuse identical values (links from values to types)
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Hash)]
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct TypeName {
-    pub namespace: arcstr::ArcStr,
-    pub name: arcstr::ArcStr,
-    // pub namespace: String,
-    // pub name: String,
+    // Order is important for Ord implementation
+    pub namespace: FastString,
+    pub name: FastString,
 }
 
 impl TypeName {
@@ -76,15 +77,6 @@ impl Display for TypeName {
     }
 }
 
-impl Ord for TypeName {
-    fn cmp(&self, other: &Self) -> Ordering {
-        match self.namespace.cmp(&other.namespace) {
-            Ordering::Equal => self.name.cmp(&other.name),
-            ordering @ _ => ordering,
-        }
-    }
-}
-
 //-----------------------------------------------------------------------------
 
 ///
@@ -318,7 +310,7 @@ impl Flavor {
     pub fn visibility(&self, availabilities: &Option<Availabilities>) -> Option<Visibility> {
         if let Some(ref availabilities) = availabilities {
             // Some availabilities defined
-            if let Some(ref availability) = availabilities.get(self) {
+            if let Some(availability) = availabilities.get(self) {
                 // This one exists. Public by default
                 availability.visibility.clone().or(Some(Visibility::Public))
             } else {
@@ -468,6 +460,7 @@ pub struct Inherits {
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case", tag="kind")]
+#[allow(clippy::large_enum_variant)]
 pub enum TypeDefinition {
     Interface(Interface),
     Request(Request),
@@ -1143,22 +1136,22 @@ impl<'a> serde::de::Visitor<'a> for IndexMapVisitor {
 // Conversions between Model and IndexedModel
 //-------------------------------------------------------------------------------------------------
 
-impl Into<IndexedModel> for Model {
-    fn into(self) -> IndexedModel {
+impl From<Model> for IndexedModel {
+    fn from(value: Model) -> Self {
         IndexedModel {
-            info: self.info,
-            endpoints: self.endpoints,
-            types: self.types.into_iter().map(|t| (t.name().clone(), t)).collect(),
+            info: value.info,
+            endpoints: value.endpoints,
+            types: value.types.into_iter().map(|t| (t.name().clone(), t)).collect(),
         }
     }
 }
 
-impl Into<Model> for IndexedModel {
-    fn into(self) -> Model {
+impl From<IndexedModel> for Model {
+    fn from(value: IndexedModel) -> Model {
         Model {
-            info: self.info,
-            endpoints: self.endpoints,
-            types: self.types.into_iter().map(|(_, t)| t).collect(),
+            info: value.info,
+            endpoints: value.endpoints,
+            types: value.types.into_iter().map(|(_, t)| t).collect(),
         }
     }
 }
@@ -1193,7 +1186,7 @@ mod tests {
         let search_type = result.get_type(&search_req).unwrap();
 
         match search_type {
-            TypeDefinition::Request(r) => assert_eq!(true, !r.path.is_empty()),
+            TypeDefinition::Request(r) => assert!(!r.path.is_empty()),
             _ => panic!("Expecting a Request")
         };
     }

compiler-rs/clients_schema/src/transform/availability.rs

@@ -20,13 +20,14 @@ use crate::{Availabilities, Body, IndexedModel, Inherits, Property, TypeDefiniti
 use crate::transform::Worksheet;
 
 pub struct Availability {
+    #[allow(clippy::type_complexity)]
     avail_filter: Box<dyn Fn(&Option<Availabilities>) -> bool>,
     // Note: we could have avoided the use of interior mutability by adding
     // a `&mut Worksheet` parameter to all methods.
     worksheet: RefCell<Worksheet>,
 }
 
-impl <'a> Availability {
+impl Availability {
     pub fn filter (mut model: IndexedModel, avail_filter: fn(&Option<Availabilities>) -> bool) -> anyhow::Result<IndexedModel> {
         let filter = Availability {
             avail_filter: Box::new(avail_filter),
@@ -38,8 +39,9 @@ impl <'a> Availability {
 
         // Initialize worksheet with request and response of all retained endpoints
         for endpoint in &model.endpoints {
-            endpoint.request.as_ref().map(|name| filter.filter_type(name));
-            endpoint.response.as_ref().map(|name| filter.filter_type(name));
+            for name in [&endpoint.request, &endpoint.response].into_iter().flatten() {
+                filter.filter_type(name);
+            }
         }
 
         while let Some(name) = {
@@ -57,7 +59,7 @@ impl <'a> Availability {
         let ws = filter.worksheet.borrow();
         model.types.retain(|k, _| ws.was_visited(k));
 
-        return Ok(model);
+        Ok(model)
     }
 
     fn is_available(&self, availabilities: &Option<Availabilities>) -> bool {
@@ -71,7 +73,7 @@ impl <'a> Availability {
     fn filter_typedef(&self, typedef: &mut TypeDefinition) {
         match typedef {
             TypeDefinition::Interface(ref mut itf) => {
-                itf.inherits.as_ref().map(|i| self.filter_inherits(i));
+                itf.inherits.iter().for_each(|i| self.filter_inherits(i));
                 itf.behaviors.iter().for_each(|i| self.filter_behaviors(i));
                 self.filter_properties(&mut itf.properties);
             },
@@ -86,7 +88,7 @@ impl <'a> Availability {
             },
 
             TypeDefinition::Request(ref mut request) => {
-                request.inherits.as_ref().map(|i| self.filter_inherits(i));
+                request.inherits.iter().for_each(|i| self.filter_inherits(i));
                 request.behaviors.iter().for_each(|i| self.filter_behaviors(i));
                 self.filter_properties(&mut request.path);
                 self.filter_properties(&mut request.query);
@@ -113,7 +115,7 @@ impl <'a> Availability {
     fn filter_properties(&self, props: &mut Vec<Property>) {
         props.retain(|p| self.is_available(&p.availability));
         for prop in props {
-            self.filter_value_of(&mut prop.typ);
+            self.filter_value_of(&prop.typ);
         }
     }
 

compiler-rs/clients_schema/src/transform/expand_generics.rs

@@ -46,8 +46,9 @@ pub fn expand_generics(
     let mut ctx = Ctx::default();
 
     for endpoint in &model.endpoints {
-        endpoint.request.as_ref().map(|t| expand_root_type(&t, &model, &mut ctx));
-        endpoint.response.as_ref().map(|t| expand_root_type(&t, &model, &mut ctx));
+        for name in [&endpoint.request, &endpoint.response].into_iter().flatten() {
+            expand_root_type(name, &model, &mut ctx)?;
+        }
     }
 
     // Add new types that were created to the model
@@ -210,7 +211,7 @@ pub fn expand_generics(
         // We keep the generic parameters of behaviors, but expand their value
         for behavior in behaviors {
             for arg in &mut behavior.generics {
-                *arg = expand_valueof(arg, &mappings, model, ctx)?;
+                *arg = expand_valueof(arg, mappings, model, ctx)?;
             }
         }
         Ok(())
@@ -309,7 +310,7 @@ pub fn expand_generics(
     /// Builds the mapping from generic parameter name to actual value
     ///
     fn param_mapping(generics: &GenericParams, args: GenericArgs) -> GenericMapping {
-        generics.iter().map(|name| name.clone()).zip(args).collect()
+        generics.iter().cloned().zip(args).collect()
     }
 
     ///
@@ -394,10 +395,10 @@ mod tests {
         if canonical_json != json_no_generics {
             std::fs::create_dir("test-output")?;
             let mut out = std::fs::File::create("test-output/schema-no-generics-canonical.json")?;
-            out.write(canonical_json.as_bytes())?;
+            out.write_all(canonical_json.as_bytes())?;
 
             let mut out = std::fs::File::create("test-output/schema-no-generics-new.json")?;
-            out.write(json_no_generics.as_bytes())?;
+            out.write_all(json_no_generics.as_bytes())?;
 
             panic!("Output differs from the canonical version. Both were written to 'test-output'");
         }

compiler-rs/clients_schema/src/transform/mod.rs

@@ -47,20 +47,24 @@ impl Worksheet {
         }
     }
 
+    /// Has this type name been visited?
+    pub fn was_visited(&self, name: &TypeName) -> bool {
+        self.visited.contains(name)
+    }
+}
+
+impl Iterator for Worksheet {
+    type Item = TypeName;
+
     /// Retrieves a type name from the work list, if some are left. This assumes the caller will
     /// process the corresponding type, and thus adds it to the list of visited type names.
-    pub fn next(&mut self) -> Option<TypeName> {
+    fn next(&mut self) -> Option<Self::Item> {
         let result = self.pending.pop();
         if let Some(ref name) = result {
             self.visited.insert(name.clone());
         }
         result
     }
-
-    /// Has this type name been visited?
-    pub fn was_visited(&self, name: &TypeName) -> bool {
-        self.visited.contains(name)
-    }
 }
 
 /// Transform a model to only keep the endpoints and types that match a predicate on the `availability`

compiler-rs/clients_schema_to_openapi/src/lib.rs

@@ -39,7 +39,7 @@ pub fn convert_schema_file(
     // Parsing from a string is faster than using a buffered reader when there is a need for look-ahead
     // See https://github.com/serde-rs/json/issues/160
     let json = &std::fs::read_to_string(path)?;
-    let json_deser = &mut serde_json::Deserializer::from_str(&json);
+    let json_deser = &mut serde_json::Deserializer::from_str(json);
 
     let mut unused = HashSet::new();
     let mut model: IndexedModel = serde_ignored::deserialize(json_deser, |path| {
@@ -95,7 +95,7 @@ pub fn convert_schema(
         extensions: Default::default(),
     };
 
-    let mut tac = TypesAndComponents::new(&model, openapi.components.as_mut().unwrap());
+    let mut tac = TypesAndComponents::new(model, openapi.components.as_mut().unwrap());
 
     // Endpoints
     for endpoint in &model.endpoints {

compiler-rs/clients_schema_to_openapi/src/paths.rs

@@ -48,7 +48,7 @@ pub fn add_endpoint(endpoint: &clients_schema::Endpoint, tac: &mut TypesAndCompo
     //};
 
     // Will we produce multiple paths? If true, we will register components for reuse across paths
-    let is_multipath = endpoint.urls.len() > 1 || endpoint.urls.iter().find(|u| u.methods.len() > 1).is_some();
+    let is_multipath = endpoint.urls.len() > 1 || endpoint.urls.iter().any(|u| u.methods.len() > 1);
 
     let request = tac.model.get_request(endpoint.request.as_ref().unwrap())?;
 

compiler-rs/clients_schema_to_openapi/src/schemas.rs

@@ -292,7 +292,7 @@ impl <'a> TypesAndComponents<'a> {
             // A container is represented by an object will all optional properties and exactly one that
             // needs to be set.
             let mut schema = ObjectType {
-                properties: self.convert_properties(variant_props.iter().map(|p| *p))?,
+                properties: self.convert_properties(variant_props.iter().copied())?,
                 required: vec![],
                 additional_properties: None,
                 min_properties: Some(1),
@@ -302,8 +302,8 @@ impl <'a> TypesAndComponents<'a> {
             if !container_props.is_empty() {
                 // Create a schema for the container property, and group it in an "allOf" with variants
                 let container_props_schema = ObjectType {
-                    properties: self.convert_properties(container_props.iter().map(|p| *p))?,
-                    required: self.required_properties(container_props.iter().map(|p| *p)),
+                    properties: self.convert_properties(container_props.iter().copied())?,
+                    required: self.required_properties(container_props.iter().copied()),
                     additional_properties: None,
                     min_properties: None,
                     max_properties: None,

compiler-rs/compiler-wasm-lib/pkg/compiler_wasm_lib.d.ts

@@ -0,0 +1,8 @@
+/* tslint:disable */
+/* eslint-disable */
+/**
+* @param {string} json
+* @param {string} flavor
+* @returns {string}
+*/
+export function convert_schema_to_openapi(json: string, flavor: string): string;

compiler-rs/compiler-wasm-lib/pkg/compiler_wasm_lib_bg.wasm.d.ts

@@ -0,0 +1,8 @@
+/* tslint:disable */
+/* eslint-disable */
+export const memory: WebAssembly.Memory;
+export function convert_schema_to_openapi(a: number, b: number, c: number, d: number, e: number): void;
+export function __wbindgen_add_to_stack_pointer(a: number): number;
+export function __wbindgen_malloc(a: number, b: number): number;
+export function __wbindgen_realloc(a: number, b: number, c: number, d: number): number;
+export function __wbindgen_free(a: number, b: number, c: number): void;

compiler-rs/openapi_to_clients_schema/src/lib.rs

@@ -53,14 +53,14 @@ fn generate_types(
     if let Some(ref components) = open_api.components {
         let mut types = types::Types::default();
         for (id, schema) in &components.schemas {
-            let result = types::generate_type(open_api, &id, &schema.into(), &mut types);
+            let result = types::generate_type(open_api, id, &schema.into(), &mut types);
 
             if let Err(err) = result {
                 warn!("Problem with type '{id}'\n    {err}\n    Definition: {:?}", &schema);
             }
         }
         let _ = types.check_tracker(); // TODO: actually fail
-        model.types = types.types();
+        model.types = types.types().into_iter().map(|t| (t.name().clone(), t)).collect();
     }
 
     Ok(())