From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 89B5D1FF0AF for ; Thu, 08 Oct 2026 15:50:28 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 2FDA52133C; Thu, 08 Oct 2026 15:50:28 +0200 (CEST) From: =?UTF-8?q?Michael=20K=C3=B6ppl?= To: pbs-devel@lists.proxmox.com, pdm-devel@lists.proxmox.com Subject: [PATCH proxmox 1/1] api-macro: correctly recognize fully qualified Option paths Date: Thu, 8 Oct 2026 15:50:20 +0200 Message-ID: <20261008135020.808248-1-m.koeppl@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791467422311 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.300 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: XM5WDNZDGASNBZJAEKNZIYKYZASPCJYO X-Message-ID-Hash: XM5WDNZDGASNBZJAEKNZIYKYZASPCJYO X-MailFrom: m.koeppl@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: is_option_type only matches Option and std::Option, so std::option::Option and core::option::Option were not considered valid paths for Option. For optional parameters this caused a spurious "optional types need a default or be an Option" error and struct fields failed to compile (e.g. with "description not allows on external type"). Also drop the std::Option case. That path only resolved is std is shadowed by a local module or import and renamed imports are not handled here anyway, as noted in is_option_type's comment. Signed-off-by: Michael Köppl --- I stumbled upon this while reviewing Lukas' application context injection series. I'm not quite sure if it makes sense to handle it this way or even fix it at all, but std::option::Option should be legitimate to use, as well as core::option::Option, so I wanted to propose it anyway. It is a bit of a band-aid solution since we'll never be able to cover every case here, but since the is_option_type function even mentions that renames are explicitly not handled, std::Option should not work (since AFAICT that would require a rename, unless I'm missing something), and the correct paths (without renaming) should work. proxmox-api-macro/src/util.rs | 6 ++++- proxmox-api-macro/tests/options.rs | 35 ++++++++++++++++++++++++++++++ proxmox-api-macro/tests/types.rs | 30 +++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/proxmox-api-macro/src/util.rs b/proxmox-api-macro/src/util.rs index 260a168d..65ff1732 100644 --- a/proxmox-api-macro/src/util.rs +++ b/proxmox-api-macro/src/util.rs @@ -585,7 +585,11 @@ pub fn is_option_type(ty: &syn::Type) -> Option<&syn::Type> { let segs = &p.path.segments; let is_option = match segs.len() { 1 => segs.last().unwrap().ident == "Option", - 2 => segs.first().unwrap().ident == "std" && segs.last().unwrap().ident == "Option", + 3 => { + (segs.first().unwrap().ident == "std" || segs.first().unwrap().ident == "core") + && segs.get(1).unwrap().ident == "option" + && segs.last().unwrap().ident == "Option" + } _ => false, }; if !is_option { diff --git a/proxmox-api-macro/tests/options.rs b/proxmox-api-macro/tests/options.rs index 6d61cc10..d85950e6 100644 --- a/proxmox-api-macro/tests/options.rs +++ b/proxmox-api-macro/tests/options.rs @@ -39,6 +39,28 @@ pub fn test_default_macro(value: Option) -> Result { Ok(value.unwrap_or(api_get_default!("value"))) } +#[api( + input: { + properties: { + a: { + description: "An optional value using the full std path", + optional: true, + }, + b: { + description: "An optional value using the full core path", + optional: true, + }, + } + } +)] +/// Return the sum of both values, treating missing values as 0. +pub fn test_qualified_option( + a: std::option::Option, + b: ::core::option::Option, +) -> Result { + Ok(a.unwrap_or(0) + b.unwrap_or(0)) +} + struct RpcEnv; impl proxmox_router::RpcEnvironment for RpcEnv { fn result_attrib_mut(&mut self) -> &mut Value { @@ -86,4 +108,17 @@ fn test_invocations() { api_function_test_default_macro(json!({}), &API_METHOD_TEST_DEFAULT_MACRO, &mut env) .expect("func with option should work"); assert_eq!(value, 5); + + let value = + api_function_test_qualified_option(json!({}), &API_METHOD_TEST_QUALIFIED_OPTION, &mut env) + .expect("func with qualified option should work"); + assert_eq!(value, 0); + + let value = api_function_test_qualified_option( + json!({"a": 2, "b": 3}), + &API_METHOD_TEST_QUALIFIED_OPTION, + &mut env, + ) + .expect("func with qualified option should work"); + assert_eq!(value, 5); } diff --git a/proxmox-api-macro/tests/types.rs b/proxmox-api-macro/tests/types.rs index b6609cfb..c7b47907 100644 --- a/proxmox-api-macro/tests/types.rs +++ b/proxmox-api-macro/tests/types.rs @@ -46,6 +46,12 @@ pub struct TestStruct { /// An optional auto-derived value for testing: another: Option, + /// Optional value using a fully qualified std path. + another_qualified: std::option::Option, + + /// Optional value using a global core path. + another_qualified_global: ::core::option::Option, + /// Disabled field. #[cfg(false)] disabled_field: String, @@ -62,6 +68,20 @@ fn test_struct() { &::proxmox_schema::StringSchema::new("An optional auto-derived value for testing:") .schema(), ), + ( + "another_qualified", + true, + &::proxmox_schema::StringSchema::new( + "Optional value using a fully qualified std path.", + ) + .schema(), + ), + ( + "another_qualified_global", + true, + &::proxmox_schema::StringSchema::new("Optional value using a global core path.") + .schema(), + ), ( "test_string", false, @@ -81,6 +101,16 @@ fn test_struct() { description: "An optional auto-derived value for testing:", optional: true, }, + another_qualified: { + type: String, + description: "Optional value using a fully qualified std path.", + optional: true, + }, + another_qualified_global: { + type: String, + description: "Optional value using a global core path.", + optional: true, + }, test_string: { type: String, description: "A test string.", -- 2.47.3