Skip to content

Commit 89103cb

Browse files
fix: resolve $ref parameters, merge path-item params, avoid url variable collision, and use r#type raw identifiers
Issue #2: Fix three bugs when ingesting specs with path-item-level parameters: - Resolve $ref parameter references via components/parameters lookup - Merge path-item-level parameters into operations (operation params take precedence) - Rename internal `url` variable to `request_url` to avoid collision with params named `url` Issue #3: Use raw identifiers (r#type) instead of suffixed names (type_) for Rust keyword field names. This is more idiomatic and avoids polluting the API surface with trailing underscores. Serde rename attributes are still emitted where the Rust field name differs from the JSON key after stripping the r# prefix. Closes #2, closes #3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7daf104 commit 89103cb

21 files changed

Lines changed: 135 additions & 166 deletions

src/analysis.rs

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,7 @@ pub struct SchemaAnalyzer {
530530
resolved_cache: BTreeMap<String, AnalyzedSchema>,
531531
openapi_spec: Value,
532532
current_schema_name: Option<String>,
533+
component_parameters: BTreeMap<String, crate::openapi::Parameter>,
533534
}
534535

535536
impl SchemaAnalyzer {
@@ -538,11 +539,19 @@ impl SchemaAnalyzer {
538539
serde_json::from_value(openapi_spec.clone()).map_err(GeneratorError::ParseError)?;
539540
let schemas = Self::extract_schemas(&spec)?;
540541

542+
let component_parameters = spec
543+
.components
544+
.as_ref()
545+
.and_then(|c| c.parameters.as_ref())
546+
.cloned()
547+
.unwrap_or_default();
548+
541549
Ok(Self {
542550
schemas,
543551
resolved_cache: BTreeMap::new(),
544552
openapi_spec,
545553
current_schema_name: None,
554+
component_parameters,
546555
})
547556
}
548557

@@ -3388,6 +3397,7 @@ impl SchemaAnalyzer {
33883397
method,
33893398
path,
33903399
operation,
3400+
path_item.parameters.as_ref(),
33913401
analysis,
33923402
)?;
33933403
analysis.operations.insert(operation_id, op_info);
@@ -3443,6 +3453,7 @@ impl SchemaAnalyzer {
34433453
method: &str,
34443454
path: &str,
34453455
operation: &crate::openapi::Operation,
3456+
path_item_parameters: Option<&Vec<crate::openapi::Parameter>>,
34463457
_analysis: &mut SchemaAnalysis,
34473458
) -> Result<OperationInfo> {
34483459
let mut op_info = OperationInfo {
@@ -3510,15 +3521,33 @@ impl SchemaAnalyzer {
35103521
}
35113522
}
35123523

3513-
// Extract parameters
3524+
// Extract parameters (operation-level first, then merge path-item-level)
35143525
if let Some(parameters) = &operation.parameters {
35153526
for param in parameters {
3516-
if let Some(param_info) = self.analyze_parameter(param)? {
3527+
let resolved = self.resolve_parameter(param);
3528+
if let Some(param_info) = self.analyze_parameter(&resolved)? {
35173529
op_info.parameters.push(param_info);
35183530
}
35193531
}
35203532
}
35213533

3534+
// Merge path-item-level parameters (operation params take precedence per OpenAPI spec)
3535+
if let Some(path_params) = path_item_parameters {
3536+
let existing_keys: std::collections::HashSet<(String, String)> = op_info
3537+
.parameters
3538+
.iter()
3539+
.map(|p| (p.name.clone(), p.location.clone()))
3540+
.collect();
3541+
for param in path_params {
3542+
let resolved = self.resolve_parameter(param);
3543+
if let Some(param_info) = self.analyze_parameter(&resolved)? {
3544+
if !existing_keys.contains(&(param_info.name.clone(), param_info.location.clone())) {
3545+
op_info.parameters.push(param_info);
3546+
}
3547+
}
3548+
}
3549+
}
3550+
35223551
Ok(op_info)
35233552
}
35243553

@@ -3566,6 +3595,22 @@ impl SchemaAnalyzer {
35663595
Ok(synthetic_name)
35673596
}
35683597

3598+
/// Resolve a parameter reference ($ref) to the actual parameter definition.
3599+
/// Returns the resolved parameter, or the original if it's not a reference.
3600+
fn resolve_parameter<'a>(
3601+
&'a self,
3602+
param: &'a crate::openapi::Parameter,
3603+
) -> std::borrow::Cow<'a, crate::openapi::Parameter> {
3604+
if let Some(ref_str) = param.extra.get("$ref").and_then(|v| v.as_str()) {
3605+
if let Some(param_name) = ref_str.strip_prefix("#/components/parameters/") {
3606+
if let Some(resolved) = self.component_parameters.get(param_name) {
3607+
return std::borrow::Cow::Borrowed(resolved);
3608+
}
3609+
}
3610+
}
3611+
std::borrow::Cow::Borrowed(param)
3612+
}
3613+
35693614
/// Analyze a parameter
35703615
fn analyze_parameter(
35713616
&self,

src/client_generator.rs

Lines changed: 14 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ impl CodeGenerator {
406406
#url_construction
407407

408408
let mut req = self.http_client
409-
.#http_method(url)
409+
.#http_method(request_url)
410410
#request_body;
411411

412412
#query_params
@@ -444,7 +444,7 @@ impl CodeGenerator {
444444
for param in query_params {
445445
// Use snake_case for Rust variable name with keyword escaping
446446
let param_name_snake = self.sanitize_param_name(&param.name);
447-
let param_name = syn::Ident::new(&param_name_snake, proc_macro2::Span::call_site());
447+
let param_name = Self::to_field_ident(&param_name_snake);
448448

449449
// Use the original parameter name from OpenAPI spec as the query string key
450450
let param_key = &param.name;
@@ -540,7 +540,7 @@ impl CodeGenerator {
540540
for param in &op.parameters {
541541
if param.location == "path" {
542542
let param_name_snake = self.sanitize_param_name(&param.name);
543-
let param_name = syn::Ident::new(&param_name_snake, proc_macro2::Span::call_site());
543+
let param_name = Self::to_field_ident(&param_name_snake);
544544
let param_type = self.get_param_rust_type(param);
545545
params.push(quote! { #param_name: #param_type });
546546
}
@@ -550,7 +550,7 @@ impl CodeGenerator {
550550
for param in &op.parameters {
551551
if param.location == "query" {
552552
let param_name_snake = self.sanitize_param_name(&param.name);
553-
let param_name = syn::Ident::new(&param_name_snake, proc_macro2::Span::call_site());
553+
let param_name = Self::to_field_ident(&param_name_snake);
554554
let param_type = self.get_param_rust_type(param);
555555

556556
// Query parameters should be Option unless explicitly required
@@ -714,7 +714,7 @@ impl CodeGenerator {
714714
self.generate_url_with_params(path, op)
715715
} else {
716716
quote! {
717-
let url = format!("{}{}", self.base_url, #path);
717+
let request_url = format!("{}{}", self.base_url, #path);
718718
}
719719
}
720720
}
@@ -740,8 +740,7 @@ impl CodeGenerator {
740740

741741
// Use snake_case for the Rust variable name with keyword escaping
742742
let param_name_snake = self.sanitize_param_name(&param.name);
743-
let param_ident =
744-
syn::Ident::new(&param_name_snake, proc_macro2::Span::call_site());
743+
let param_ident = Self::to_field_ident(&param_name_snake);
745744

746745
// Use .as_ref() for string types to handle impl AsRef<str>
747746
if param.rust_type == "String" {
@@ -755,66 +754,24 @@ impl CodeGenerator {
755754
if format_args.is_empty() {
756755
// No path parameters found, use simple format
757756
quote! {
758-
let url = format!("{}{}", self.base_url, #path);
757+
let request_url = format!("{}{}", self.base_url, #path);
759758
}
760759
} else {
761760
// Build format call with path parameters
762761
quote! {
763-
let url = format!("{}{}", self.base_url, format!(#format_string, #(#format_args),*));
762+
let request_url = format!("{}{}", self.base_url, format!(#format_string, #(#format_args),*));
764763
}
765764
}
766765
}
767766

768-
/// Sanitize a parameter name by escaping Rust reserved keywords
767+
/// Sanitize a parameter name by escaping Rust reserved keywords with raw identifiers
769768
fn sanitize_param_name(&self, name: &str) -> String {
770769
let snake_case = name.to_snake_case();
771-
match snake_case.as_str() {
772-
"type" => "type_".to_string(),
773-
"match" => "match_".to_string(),
774-
"fn" => "fn_".to_string(),
775-
"impl" => "impl_".to_string(),
776-
"trait" => "trait_".to_string(),
777-
"struct" => "struct_".to_string(),
778-
"enum" => "enum_".to_string(),
779-
"mod" => "mod_".to_string(),
780-
"use" => "use_".to_string(),
781-
"pub" => "pub_".to_string(),
782-
"const" => "const_".to_string(),
783-
"static" => "static_".to_string(),
784-
"let" => "let_".to_string(),
785-
"mut" => "mut_".to_string(),
786-
"ref" => "ref_".to_string(),
787-
"move" => "move_".to_string(),
788-
"return" => "return_".to_string(),
789-
"if" => "if_".to_string(),
790-
"else" => "else_".to_string(),
791-
"while" => "while_".to_string(),
792-
"for" => "for_".to_string(),
793-
"loop" => "loop_".to_string(),
794-
"break" => "break_".to_string(),
795-
"continue" => "continue_".to_string(),
796-
"self" => "self_".to_string(),
797-
"super" => "super_".to_string(),
798-
"crate" => "crate_".to_string(),
799-
"async" => "async_".to_string(),
800-
"await" => "await_".to_string(),
801-
"override" => "override_".to_string(),
802-
"box" => "box_".to_string(),
803-
"dyn" => "dyn_".to_string(),
804-
"where" => "where_".to_string(),
805-
"in" => "in_".to_string(),
806-
"abstract" => "abstract_".to_string(),
807-
"become" => "become_".to_string(),
808-
"do" => "do_".to_string(),
809-
"final" => "final_".to_string(),
810-
"macro" => "macro_".to_string(),
811-
"priv" => "priv_".to_string(),
812-
"try" => "try_".to_string(),
813-
"typeof" => "typeof_".to_string(),
814-
"unsized" => "unsized_".to_string(),
815-
"virtual" => "virtual_".to_string(),
816-
"yield" => "yield_".to_string(),
817-
_ => snake_case,
770+
if Self::is_rust_keyword(&snake_case) {
771+
format!("r#{snake_case}")
772+
} else {
773+
snake_case
818774
}
819775
}
776+
820777
}

src/generator.rs

Lines changed: 33 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,7 @@ impl CodeGenerator {
851851
}
852852
})
853853
.map(|(field_name, prop)| {
854-
let field_ident = format_ident!("{}", self.to_rust_field_name(field_name));
854+
let field_ident = Self::to_field_ident(&self.to_rust_field_name(field_name));
855855
let is_required = required.contains(field_name);
856856
let field_type =
857857
self.generate_field_type(&schema.name, field_name, prop, is_required, analysis);
@@ -1249,9 +1249,11 @@ impl CodeGenerator {
12491249
) -> TokenStream {
12501250
let mut attrs = Vec::new();
12511251

1252-
// Generate rename attribute if field name is not valid Rust identifier
1252+
// Generate rename attribute if field name differs from Rust identifier
1253+
// Strip r# prefix for comparison since serde handles raw idents transparently
12531254
let rust_field_name = self.to_rust_field_name(field_name);
1254-
if rust_field_name != field_name {
1255+
let comparison_name = rust_field_name.strip_prefix("r#").unwrap_or(&rust_field_name);
1256+
if comparison_name != field_name {
12551257
attrs.push(quote! { rename = #field_name });
12561258
}
12571259

@@ -1610,57 +1612,34 @@ impl CodeGenerator {
16101612
result = format!("field_{result}");
16111613
}
16121614

1613-
// Handle reserved keywords
1614-
match result.as_str() {
1615-
"type" => "type_".to_string(),
1616-
"match" => "match_".to_string(),
1617-
"fn" => "fn_".to_string(),
1618-
"struct" => "struct_".to_string(),
1619-
"enum" => "enum_".to_string(),
1620-
"impl" => "impl_".to_string(),
1621-
"trait" => "trait_".to_string(),
1622-
"mod" => "mod_".to_string(),
1623-
"use" => "use_".to_string(),
1624-
"pub" => "pub_".to_string(),
1625-
"const" => "const_".to_string(),
1626-
"static" => "static_".to_string(),
1627-
"let" => "let_".to_string(),
1628-
"mut" => "mut_".to_string(),
1629-
"ref" => "ref_".to_string(),
1630-
"move" => "move_".to_string(),
1631-
"return" => "return_".to_string(),
1632-
"if" => "if_".to_string(),
1633-
"else" => "else_".to_string(),
1634-
"while" => "while_".to_string(),
1635-
"for" => "for_".to_string(),
1636-
"loop" => "loop_".to_string(),
1637-
"break" => "break_".to_string(),
1638-
"continue" => "continue_".to_string(),
1639-
// Handle some common edge cases
1640-
"self" => "self_".to_string(),
1641-
"super" => "super_".to_string(),
1642-
"crate" => "crate_".to_string(),
1643-
"async" => "async_".to_string(),
1644-
"await" => "await_".to_string(),
1645-
// Reserved keywords for edition 2018+
1646-
"override" => "override_".to_string(),
1647-
"box" => "box_".to_string(),
1648-
"dyn" => "dyn_".to_string(),
1649-
"where" => "where_".to_string(),
1650-
"in" => "in_".to_string(),
1651-
// Reserved for future use
1652-
"abstract" => "abstract_".to_string(),
1653-
"become" => "become_".to_string(),
1654-
"do" => "do_".to_string(),
1655-
"final" => "final_".to_string(),
1656-
"macro" => "macro_".to_string(),
1657-
"priv" => "priv_".to_string(),
1658-
"try" => "try_".to_string(),
1659-
"typeof" => "typeof_".to_string(),
1660-
"unsized" => "unsized_".to_string(),
1661-
"virtual" => "virtual_".to_string(),
1662-
"yield" => "yield_".to_string(),
1663-
_ => result,
1615+
// Handle reserved keywords using raw identifiers (r#keyword)
1616+
if Self::is_rust_keyword(&result) {
1617+
format!("r#{result}")
1618+
} else {
1619+
result
1620+
}
1621+
}
1622+
1623+
/// Check if a string is a Rust keyword that needs raw identifier treatment
1624+
pub fn is_rust_keyword(s: &str) -> bool {
1625+
matches!(
1626+
s,
1627+
"type" | "match" | "fn" | "struct" | "enum" | "impl" | "trait" | "mod"
1628+
| "use" | "pub" | "const" | "static" | "let" | "mut" | "ref" | "move"
1629+
| "return" | "if" | "else" | "while" | "for" | "loop" | "break"
1630+
| "continue" | "self" | "super" | "crate" | "async" | "await"
1631+
| "override" | "box" | "dyn" | "where" | "in"
1632+
| "abstract" | "become" | "do" | "final" | "macro" | "priv" | "try"
1633+
| "typeof" | "unsized" | "virtual" | "yield"
1634+
)
1635+
}
1636+
1637+
/// Create a proc_macro2::Ident from a field name, handling r# raw identifiers
1638+
pub fn to_field_ident(name: &str) -> proc_macro2::Ident {
1639+
if let Some(raw) = name.strip_prefix("r#") {
1640+
proc_macro2::Ident::new_raw(raw, proc_macro2::Span::call_site())
1641+
} else {
1642+
proc_macro2::Ident::new(name, proc_macro2::Span::call_site())
16641643
}
16651644
}
16661645

src/openapi.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub struct Info {
2525
#[derive(Debug, Clone, Deserialize, Serialize)]
2626
pub struct Components {
2727
pub schemas: Option<BTreeMap<String, Schema>>,
28+
pub parameters: Option<BTreeMap<String, Parameter>>,
2829
#[serde(flatten)]
2930
pub extra: BTreeMap<String, Value>,
3031
}

src/snapshots/openapi_to_rust__test_helpers__beta_tools_array_union_test.snap

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ pub struct BetaTool {
3636
pub description: Option<String>,
3737
pub input_schema: BetaInputSchema,
3838
pub name: String,
39-
#[serde(rename = "type", skip_serializing_if = "Option::is_none")]
40-
pub type_: Option<serde_json::Value>,
39+
#[serde(skip_serializing_if = "Option::is_none")]
40+
pub r#type: Option<serde_json::Value>,
4141
}
4242
#[derive(Debug, Clone, Deserialize, Serialize)]
4343
pub struct BetaInputSchema {
4444
#[serde(skip_serializing_if = "Option::is_none")]
4545
pub properties: Option<serde_json::Value>,
46-
#[serde(rename = "type", skip_serializing_if = "Option::is_none")]
47-
pub type_: Option<String>,
46+
#[serde(skip_serializing_if = "Option::is_none")]
47+
pub r#type: Option<String>,
4848
}
4949
#[derive(Debug, Clone, Deserialize, Serialize)]
5050
pub struct BetaComputerUseTool {
@@ -53,6 +53,6 @@ pub struct BetaComputerUseTool {
5353
#[serde(skip_serializing_if = "Option::is_none")]
5454
pub display_width_px: Option<i64>,
5555
pub name: String,
56-
#[serde(rename = "type", skip_serializing_if = "Option::is_none")]
57-
pub type_: Option<serde_json::Value>,
56+
#[serde(skip_serializing_if = "Option::is_none")]
57+
pub r#type: Option<serde_json::Value>,
5858
}

src/snapshots/openapi_to_rust__test_helpers__content_union_structured.snap

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,5 @@ pub type ContentBlockArray = Vec<ContentBlock>;
2828
#[derive(Debug, Clone, Deserialize, Serialize)]
2929
pub struct ContentBlock {
3030
pub text: String,
31-
#[serde(rename = "type")]
32-
pub type_: String,
31+
pub r#type: String,
3332
}

src/snapshots/openapi_to_rust__test_helpers__create_response_input_typing.snap

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ pub enum CreateResponseInput {
2727
#[derive(Debug, Clone, Deserialize, Serialize)]
2828
pub struct InputItem {
2929
pub content: String,
30-
#[serde(rename = "type")]
31-
pub type_: InputItemType,
30+
pub r#type: InputItemType,
3231
}
3332
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, Default)]
3433
pub enum InputItemType {

0 commit comments

Comments
 (0)