feat(codegen): various improvements to robustness

torn-api-codegen/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "torn-api-codegen"
 authors = ["Pyrit [2111649]"]
-version = "0.6.2"
+version = "0.7.0"
 edition = "2021"
 description = "Contains the v2 torn API model descriptions and codegen for the bindings"
 license-file = { workspace = true }

torn-api-codegen/src/model/enum.rs

@@ -77,6 +77,11 @@ impl EnumVariantTupleValue {
                 format: Some("int32"),
                 ..
             } => Some(Self::Primitive(PrimitiveType::I32)),
+            OpenApiType {
+                r#type: Some("number"),
+                format: Some("float") | None,
+                ..
+            } => Some(Self::Primitive(PrimitiveType::Float)),
             _ => None,
         }
     }
@@ -146,6 +151,7 @@ impl EnumVariantTupleValue {
 
     pub fn is_comparable(&self, resolved: &ResolvedSchema) -> bool {
         match self {
+            Self::Primitive(PrimitiveType::Float) => false,
             Self::Primitive(_) => true,
             Self::Enum { inner, .. } => inner.is_comparable(resolved),
             Self::Ref { ty_name } | Self::ArrayOfRefs { ty_name } => resolved

torn-api-codegen/src/model/mod.rs

@@ -1,8 +1,10 @@
+use std::{cell::RefCell, rc::Rc};
+
 use indexmap::IndexMap;
 use newtype::Newtype;
 use object::Object;
 use parameter::Parameter;
-use path::Path;
+use path::{Path, PrettySegments};
 use proc_macro2::TokenStream;
 use r#enum::Enum;
 use scope::Scope;
@@ -35,11 +37,73 @@ impl Model {
     }
 }
 
-#[derive(Debug, Default)]
+#[derive(Default)]
 pub struct ResolvedSchema {
     pub models: IndexMap<String, Model>,
     pub paths: IndexMap<String, Path>,
     pub parameters: Vec<Parameter>,
+
+    pub warnings: WarningReporter,
+}
+
+#[derive(Clone)]
+pub struct Warning {
+    pub message: String,
+    pub path: Vec<String>,
+}
+
+impl std::fmt::Display for Warning {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "in {}: {}", self.path.join("."), self.message)
+    }
+}
+
+#[derive(Default)]
+struct WarningReporterState {
+    warnings: Vec<Warning>,
+    path: Vec<String>,
+}
+
+#[derive(Clone, Default)]
+pub struct WarningReporter {
+    state: Rc<RefCell<WarningReporterState>>,
+}
+
+impl WarningReporter {
+    pub fn new() -> Self {
+        Self::default()
+    }
+
+    fn push(&self, message: impl ToString) {
+        let mut state = self.state.borrow_mut();
+        let path = state.path.iter().map(ToString::to_string).collect();
+        state.warnings.push(Warning {
+            message: message.to_string(),
+            path,
+        });
+    }
+
+    fn child(&self, name: impl ToString) -> WarningReporter {
+        self.state.borrow_mut().path.push(name.to_string());
+
+        Self {
+            state: self.state.clone(),
+        }
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.state.borrow().warnings.is_empty()
+    }
+
+    pub fn get_warnings(&self) -> Vec<Warning> {
+        self.state.borrow().warnings.clone()
+    }
+}
+
+impl Drop for WarningReporter {
+    fn drop(&mut self) {
+        self.state.borrow_mut().path.pop();
+    }
 }
 
 impl ResolvedSchema {
@@ -49,14 +113,20 @@ impl ResolvedSchema {
         for (name, r#type) in &schema.components.schemas {
             result.models.insert(
                 name.to_string(),
-                resolve(r#type, name, &schema.components.schemas),
+                resolve(r#type, name, &schema.components.schemas, &result.warnings),
             );
         }
 
         for (path, body) in &schema.paths {
             result.paths.insert(
                 path.to_string(),
-                Path::from_schema(path, body, &schema.components.parameters).unwrap(),
+                Path::from_schema(
+                    path,
+                    body,
+                    &schema.components.parameters,
+                    result.warnings.child(path),
+                )
+                .unwrap(),
             );
         }
 
@@ -83,7 +153,9 @@ impl ResolvedSchema {
         let mut output = TokenStream::default();
 
         for path in self.paths.values() {
-            output.extend(path.codegen_request(self));
+            output.extend(
+                path.codegen_request(self, self.warnings.child(PrettySegments(&path.segments))),
+            );
         }
 
         output
@@ -112,7 +184,12 @@ impl ResolvedSchema {
     }
 }
 
-pub fn resolve(r#type: &OpenApiType, name: &str, schemas: &IndexMap<&str, OpenApiType>) -> Model {
+pub fn resolve(
+    r#type: &OpenApiType,
+    name: &str,
+    schemas: &IndexMap<&str, OpenApiType>,
+    warnings: &WarningReporter,
+) -> Model {
     match r#type {
         OpenApiType {
             r#enum: Some(_), ..
@@ -120,8 +197,12 @@ pub fn resolve(r#type: &OpenApiType, name: &str, schemas: &IndexMap<&str, OpenAp
         OpenApiType {
             r#type: Some("object"),
             ..
-        } => Object::from_schema_object(name, r#type, schemas)
+        } => Model::Object(Object::from_schema_object(
-            .map_or(Model::Unresolved, Model::Object),
+            name,
+            r#type,
+            schemas,
+            warnings.child(name),
+        )),
         OpenApiType {
             r#type: Some(_), ..
         } => Newtype::from_schema(name, r#type).map_or(Model::Unresolved, Model::Newtype),
@@ -132,7 +213,12 @@ pub fn resolve(r#type: &OpenApiType, name: &str, schemas: &IndexMap<&str, OpenAp
         OpenApiType {
             all_of: Some(types),
             ..
-        } => Object::from_all_of(name, types, schemas).map_or(Model::Unresolved, Model::Object),
+        } => Model::Object(Object::from_all_of(
+            name,
+            types,
+            schemas,
+            warnings.child(name),
+        )),
         _ => Model::Unresolved,
     }
 }
@@ -159,7 +245,14 @@ mod test {
 
         let user_id_schema = schema.components.schemas.get("UserId").unwrap();
 
-        let user_id = resolve(user_id_schema, "UserId", &schema.components.schemas);
+        let reporter = WarningReporter::new();
+        let user_id = resolve(
+            user_id_schema,
+            "UserId",
+            &schema.components.schemas,
+            &reporter,
+        );
+        assert!(reporter.is_empty());
 
         assert_eq!(
             user_id,
@@ -174,7 +267,13 @@ mod test {
 
         let attack_code_schema = schema.components.schemas.get("AttackCode").unwrap();
 
-        let attack_code = resolve(attack_code_schema, "AttackCode", &schema.components.schemas);
+        let attack_code = resolve(
+            attack_code_schema,
+            "AttackCode",
+            &schema.components.schemas,
+            &reporter,
+        );
+        assert!(reporter.is_empty());
 
         assert_eq!(
             attack_code,
@@ -196,7 +295,10 @@ mod test {
         let total = schema.components.schemas.len();
 
         for (name, desc) in &schema.components.schemas {
-            if resolve(desc, name, &schema.components.schemas) == Model::Unresolved {
+            let reporter = WarningReporter::new();
+            if resolve(desc, name, &schema.components.schemas, &reporter) == Model::Unresolved
+                || !reporter.is_empty()
+            {
                 unresolved.push(name);
             }
         }

torn-api-codegen/src/model/object.rs

@@ -1,12 +1,12 @@
 use heck::{ToSnakeCase, ToUpperCamelCase};
-use indexmap::IndexMap;
+use indexmap::{map::Entry, IndexMap};
 use proc_macro2::TokenStream;
 use quote::{format_ident, quote, ToTokens};
 use syn::Ident;
 
 use crate::openapi::r#type::OpenApiType;
 
-use super::{r#enum::Enum, ResolvedSchema};
+use super::{r#enum::Enum, ResolvedSchema, WarningReporter};
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum PrimitiveType {
@@ -85,6 +85,7 @@ impl PropertyType {
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Property {
+    pub field_name: String,
     pub name: String,
     pub description: Option<String>,
     pub required: bool,
@@ -99,24 +100,31 @@ impl Property {
         required: bool,
         schema: &OpenApiType,
         schemas: &IndexMap<&str, OpenApiType>,
+        warnings: WarningReporter,
     ) -> Option<Self> {
         let name = name.to_owned();
+        let field_name = name.to_snake_case();
         let description = schema.description.as_deref().map(ToOwned::to_owned);
 
         match schema {
             OpenApiType {
                 r#enum: Some(_), ..
-            } => Some(Self {
+            } => {
-                r#type: PropertyType::Enum(Enum::from_schema(
+                let Some(r#enum) = Enum::from_schema(&name.clone().to_upper_camel_case(), schema)
-                    &name.clone().to_upper_camel_case(),
+                else {
-                    schema,
+                    warnings.push("Failed to create enum");
-                )?),
+                    return None;
+                };
+                Some(Self {
+                    r#type: PropertyType::Enum(r#enum),
                     name,
+                    field_name,
                     description,
                     required,
                     deprecated: schema.deprecated,
                     nullable: false,
-            }),
+                })
+            }
             OpenApiType {
                 one_of: Some(types),
                 ..
@@ -125,7 +133,7 @@ impl Property {
                     r#type: Some("null"),
                     ..
                 }] => {
-                    let mut inner = Self::from_schema(&name, required, left, schemas)?;
+                    let mut inner = Self::from_schema(&name, required, left, schemas, warnings)?;
                     inner.nullable = true;
                     Some(inner)
                 }
@@ -137,14 +145,19 @@ impl Property {
                         one_of: Some(left.to_owned()),
                         ..schema.clone()
                     };
-                    let mut inner = Self::from_schema(&name, required, &rest, schemas)?;
+                    let mut inner = Self::from_schema(&name, required, &rest, schemas, warnings)?;
                     inner.nullable = true;
                     Some(inner)
                 }
                 cases => {
-                    let r#enum = Enum::from_one_of(&name.to_upper_camel_case(), cases)?;
+                    let Some(r#enum) = Enum::from_one_of(&name.to_upper_camel_case(), cases) else {
+                        warnings.push("Failed to create oneOf enum");
+                        return None;
+                    };
+
                     Some(Self {
                         name,
+                        field_name,
                         description,
                         required,
                         nullable: false,
@@ -157,9 +170,12 @@ impl Property {
                 all_of: Some(types),
                 ..
             } => {
-                let composite = Object::from_all_of(&name.to_upper_camel_case(), types, schemas)?;
+                let obj_name = name.to_upper_camel_case();
+                let composite =
+                    Object::from_all_of(&obj_name, types, schemas, warnings.child(&obj_name));
                 Some(Self {
                     name,
+                    field_name,
                     description,
                     required,
                     nullable: false,
@@ -170,23 +186,29 @@ impl Property {
             OpenApiType {
                 r#type: Some("object"),
                 ..
-            } => Some(Self {
+            } => {
+                let obj_name = name.to_upper_camel_case();
+                Some(Self {
                     r#type: PropertyType::Nested(Box::new(Object::from_schema_object(
-                    &name.clone().to_upper_camel_case(),
+                        &obj_name,
                         schema,
                         schemas,
-                )?)),
+                        warnings.child(&obj_name),
+                    ))),
                     name,
+                    field_name,
                     description,
                     required,
                     deprecated: schema.deprecated,
                     nullable: false,
-            }),
+                })
+            }
             OpenApiType {
                 ref_path: Some(path),
                 ..
             } => Some(Self {
                 name,
+                field_name,
                 description,
                 r#type: PropertyType::Ref((*path).to_owned()),
                 required,
@@ -198,10 +220,11 @@ impl Property {
                 items: Some(items),
                 ..
             } => {
-                let inner = Self::from_schema(&name, required, items, schemas)?;
+                let inner = Self::from_schema(&name, required, items, schemas, warnings)?;
 
                 Some(Self {
                     name,
+                    field_name,
                     description,
                     required,
                     nullable: false,
@@ -226,6 +249,7 @@ impl Property {
 
                 Some(Self {
                     name,
+                    field_name,
                     description,
                     required,
                     nullable: false,
@@ -233,7 +257,10 @@ impl Property {
                     r#type: PropertyType::Primitive(prim),
                 })
             }
-            _ => None,
+            _ => {
+                warnings.push("Could not resolve property type");
+                None
+            }
         }
     }
 
@@ -247,11 +274,11 @@ impl Property {
         let name = &self.name;
         let (name, serde_attr) = match name.as_str() {
             "type" => (format_ident!("r#type"), None),
-            name if name != name.to_snake_case() => (
+            name if name != self.field_name => (
-                format_ident!("{}", name.to_snake_case()),
+                format_ident!("{}", self.field_name),
                 Some(quote! { #[serde(rename = #name)]}),
             ),
-            _ => (format_ident!("{name}"), None),
+            _ => (format_ident!("{}", self.field_name), None),
         };
 
         let ty_inner = self.r#type.codegen(namespace, resolved)?;
@@ -283,7 +310,7 @@ impl Property {
 pub struct Object {
     pub name: String,
     pub description: Option<String>,
-    pub properties: Vec<Property>,
+    pub properties: IndexMap<String, Property>,
 }
 
 impl Object {
@@ -291,7 +318,8 @@ impl Object {
         name: &str,
         schema: &OpenApiType,
         schemas: &IndexMap<&str, OpenApiType>,
-    ) -> Option<Self> {
+        warnings: WarningReporter,
+    ) -> Self {
         let mut result = Object {
             name: name.to_owned(),
             description: schema.description.as_deref().map(ToOwned::to_owned),
@@ -299,39 +327,54 @@ impl Object {
         };
 
         let Some(props) = &schema.properties else {
-            return None;
+            warnings.push("Missing properties");
+            return result;
         };
 
         let required = schema.required.clone().unwrap_or_default();
 
         for (prop_name, prop) in props {
-            // HACK: This will cause a duplicate key otherwise
+            let Some(prop) = Property::from_schema(
-            if ["itemDetails", "sci-fi", "non-attackers", "co-leader_id"].contains(prop_name) {
-                continue;
-            }
-
-            // TODO: implement custom enum for this (depends on overrides being added)
-            // Maybe this is an issue with the schema instead?
-            if *prop_name == "value" && name == "TornHof" {
-                continue;
-            }
-
-            result.properties.push(Property::from_schema(
                 prop_name,
                 required.contains(prop_name),
                 prop,
                 schemas,
-            )?);
+                warnings.child(prop_name),
+            ) else {
+                continue;
+            };
+
+            let field_name = prop.field_name.clone();
+
+            let entry = result.properties.entry(field_name.clone());
+            if let Entry::Occupied(mut entry) = entry {
+                let other_name = entry.get().name.clone();
+                warnings.push(format!(
+                    "Property name collision: {other_name} and {field_name}"
+                ));
+                // deprioritise kebab and camelcase
+                if other_name.contains('-')
+                    || other_name
+                        .chars()
+                        .filter(|c| c.is_alphabetic())
+                        .all(|c| c.is_ascii_lowercase())
+                {
+                    entry.insert(prop);
+                }
+            } else {
+                entry.insert_entry(prop);
+            }
         }
 
-        Some(result)
+        result
     }
 
     pub fn from_all_of(
         name: &str,
         types: &[OpenApiType],
         schemas: &IndexMap<&str, OpenApiType>,
-    ) -> Option<Self> {
+        warnings: WarningReporter,
+    ) -> Self {
         let mut result = Self {
             name: name.to_owned(),
             ..Default::default()
@@ -339,22 +382,29 @@ impl Object {
 
         for r#type in types {
             let r#type = if let OpenApiType {
-                ref_path: Some(path),
+                ref_path: Some(ref_path),
                 ..
             } = r#type
             {
-                let name = path.strip_prefix("#/components/schemas/")?;
+                let Some(name) = ref_path.strip_prefix("#/components/schemas/") else {
-                schemas.get(name)?
+                    warnings.push(format!("Malformed ref {ref_path}"));
+                    continue;
+                };
+                let Some(schema) = schemas.get(name) else {
+                    warnings.push(format!("Missing schema for ref {name}"));
+                    continue;
+                };
+                schema
             } else {
                 r#type
             };
-            let obj = Self::from_schema_object(name, r#type, schemas)?;
+            let obj = Self::from_schema_object(name, r#type, schemas, warnings.child("variant"));
 
             result.description = result.description.or(obj.description);
             result.properties.extend(obj.properties);
         }
 
-        Some(result)
+        result
     }
 
     pub fn codegen(&self, resolved: &ResolvedSchema) -> Option<TokenStream> {
@@ -371,7 +421,7 @@ impl Object {
         };
 
         let mut props = Vec::with_capacity(self.properties.len());
-        for prop in &self.properties {
+        for (_, prop) in &self.properties {
             props.push(prop.codegen(&mut namespace, resolved)?);
         }
 
@@ -441,7 +491,14 @@ mod test {
         for (name, desc) in &schema.components.schemas {
             if desc.r#type == Some("object") {
                 objects += 1;
-                if Object::from_schema_object(name, desc, &schema.components.schemas).is_none() {
+                let reporter = WarningReporter::new();
+                Object::from_schema_object(
+                    name,
+                    desc,
+                    &schema.components.schemas,
+                    reporter.clone(),
+                );
+                if !reporter.is_empty() {
                     unresolved.push(name);
                 }
             }

torn-api-codegen/src/model/path.rs

@@ -14,7 +14,7 @@ use crate::openapi::{
 use super::{
     parameter::{Parameter, ParameterLocation, ParameterType},
     union::Union,
-    ResolvedSchema,
+    ResolvedSchema, WarningReporter,
 };
 
 #[derive(Debug, Clone)]
@@ -23,6 +23,21 @@ pub enum PathSegment {
     Parameter { name: String },
 }
 
+pub struct PrettySegments<'a>(pub &'a [PathSegment]);
+
+impl std::fmt::Display for PrettySegments<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        for segment in self.0 {
+            match segment {
+                PathSegment::Constant(c) => write!(f, "/{c}")?,
+                PathSegment::Parameter { name } => write!(f, "/{{{name}}}")?,
+            }
+        }
+
+        Ok(())
+    }
+}
+
 #[derive(Debug, Clone)]
 pub enum PathParameter {
     Inline(Parameter),
@@ -51,6 +66,7 @@ impl Path {
         path: &str,
         schema: &OpenApiPath,
         parameters: &IndexMap<&str, OpenApiParameter>,
+        warnings: WarningReporter,
     ) -> Option<Self> {
         let mut segments = Vec::new();
         for segment in path.strip_prefix('/')?.split('/') {
@@ -111,9 +127,13 @@ impl Path {
                     .strip_prefix("#/components/schemas/")?
                     .to_owned(),
             },
-            OpenApiResponseBody::Union { any_of: _ } => PathResponse::ArbitraryUnion(
+            OpenApiResponseBody::Union { any_of: _ } => {
-                Union::from_schema("Response", &schema.get.response_content)?,
+                PathResponse::ArbitraryUnion(Union::from_schema(
-            ),
+                    "Response",
+                    &schema.get.response_content,
+                    warnings.child("response"),
+                )?)
+            }
         };
 
         Some(Self {
@@ -126,7 +146,11 @@ impl Path {
         })
     }
 
-    pub fn codegen_request(&self, resolved: &ResolvedSchema) -> Option<TokenStream> {
+    pub fn codegen_request(
+        &self,
+        resolved: &ResolvedSchema,
+        warnings: WarningReporter,
+    ) -> Option<TokenStream> {
         let name = if self.segments.len() == 1 {
             let Some(PathSegment::Constant(first)) = self.segments.first() else {
                 return None;
@@ -207,6 +231,13 @@ impl Path {
             let query_val = &param.value;
 
             if param.location == ParameterLocation::Path {
+                if self.segments.iter().any(|s| {
+                    if let PathSegment::Parameter { name } = s {
+                        name == &param.value
+                    } else {
+                        false
+                    }
+                }) {
                     discriminant.push(ty.clone());
                     discriminant_val.push(quote! { self.#name });
                     let path_name = format_ident!("{}", param.value);
@@ -218,6 +249,12 @@ impl Path {
                     fmt_val.push(quote! {
                         #path_name=self.#name
                     });
+                } else {
+                    warnings.push(format!(
+                        "Provided path parameter is not present in the url: {}",
+                        param.value
+                    ));
+                }
             } else {
                 let ty = if param.required {
                     convert_field.push(quote! {
@@ -329,7 +366,15 @@ impl Path {
                 PathParameter::Component(param) => (param, false),
             };
 
-            if param.location == ParameterLocation::Path {
+            if param.location == ParameterLocation::Path
+                && self.segments.iter().any(|s| {
+                    if let PathSegment::Parameter { name } = s {
+                        name == &param.value
+                    } else {
+                        false
+                    }
+                })
+            {
                 let ty = match &param.r#type {
                     ParameterType::I32 { .. } | ParameterType::Enum { .. } => {
                         let ty_name = format_ident!("{}", param.name);
@@ -444,8 +489,15 @@ impl Path {
                 PathParameter::Inline(param) => (param, true),
                 PathParameter::Component(param) => (param, false),
             };
-
+            if param.location == ParameterLocation::Path
-            if param.location == ParameterLocation::Path {
+                && self.segments.iter().any(|s| {
+                    if let PathSegment::Parameter { name } = s {
+                        name == &param.value
+                    } else {
+                        false
+                    }
+                })
+            {
                 let ty = match &param.r#type {
                     ParameterType::I32 { .. } | ParameterType::Enum { .. } => {
                         let ty_name = format_ident!("{}", param.name);
@@ -604,7 +656,14 @@ mod test {
 
         for (name, desc) in &schema.paths {
             paths += 1;
-            if Path::from_schema(name, desc, &schema.components.parameters).is_none() {
+            if Path::from_schema(
+                name,
+                desc,
+                &schema.components.parameters,
+                WarningReporter::new(),
+            )
+            .is_none()
+            {
                 unresolved.push(name);
             }
         }
@@ -627,18 +686,23 @@ mod test {
     fn codegen_paths() {
         let schema = get_schema();
         let resolved = ResolvedSchema::from_open_api(&schema);
+        let reporter = WarningReporter::new();
 
         let mut paths = 0;
         let mut unresolved = vec![];
 
         for (name, desc) in &schema.paths {
             paths += 1;
-            let Some(path) = Path::from_schema(name, desc, &schema.components.parameters) else {
+            let Some(path) =
+                Path::from_schema(name, desc, &schema.components.parameters, reporter.clone())
+            else {
                 unresolved.push(name);
                 continue;
             };
 
-            if path.codegen_scope_call().is_none() || path.codegen_request(&resolved).is_none() {
+            if path.codegen_scope_call().is_none()
+                || path.codegen_request(&resolved, reporter.clone()).is_none()
+            {
                 unresolved.push(name);
             }
         }

torn-api-codegen/src/model/union.rs

@@ -4,6 +4,8 @@ use quote::{format_ident, quote};
 
 use crate::openapi::path::OpenApiResponseBody;
 
+use super::WarningReporter;
+
 #[derive(Debug, Clone)]
 pub struct Union {
     pub name: String,
@@ -11,10 +13,23 @@ pub struct Union {
 }
 
 impl Union {
-    pub fn from_schema(name: &str, schema: &OpenApiResponseBody) -> Option<Self> {
+    pub fn from_schema(
+        name: &str,
+        schema: &OpenApiResponseBody,
+        warnings: WarningReporter,
+    ) -> Option<Self> {
         let members = match schema {
             OpenApiResponseBody::Union { any_of } => {
-                any_of.iter().map(|l| l.ref_path.to_owned()).collect()
+                let mut members = Vec::with_capacity(any_of.len());
+                for l in any_of {
+                    let path = l.ref_path.to_owned();
+                    if members.contains(&path) {
+                        warnings.push(format!("Duplicate member: {path}"));
+                    } else {
+                        members.push(path);
+                    }
+                }
+                members
             }
             _ => return None,
         };

torn-api/build.rs

@@ -28,4 +28,8 @@ fn main() {
     let scopes_file = syn::parse2(resolved.codegen_scopes()).unwrap();
     let scopes_pretty = prettyplease::unparse(&scopes_file);
     fs::write(&scopes_dest, scopes_pretty).unwrap();
+
+    for warning in resolved.warnings.get_warnings() {
+        println!("cargo:warning={}", warning);
+    }
 }