From b3a8375ba142b46b923ba1453a25a7926a57afd8 Mon Sep 17 00:00:00 2001 From: Taku Kodma <79110363+risu729@users.noreply.github.com> Date: Mon, 18 May 2026 20:47:34 +1000 Subject: [PATCH 1/8] refactor(core): parse plugin tool options locally --- src/plugins/core/dotnet.rs | 65 +++++++++++++++++++-- src/plugins/core/java.rs | 85 +++++++++++++++++++++------- src/plugins/core/python.rs | 51 ++++++++++++++++- src/plugins/core/rust.rs | 112 ++++++++++++++++++++++++++++++------- 4 files changed, 264 insertions(+), 49 deletions(-) diff --git a/src/plugins/core/dotnet.rs b/src/plugins/core/dotnet.rs index fa900b6292..c558b44adf 100644 --- a/src/plugins/core/dotnet.rs +++ b/src/plugins/core/dotnet.rs @@ -9,13 +9,14 @@ use versions::Versioning; use crate::backend::Backend; use crate::backend::VersionInfo; +use crate::backend::options::BackendOptions; use crate::cli::args::BackendArg; use crate::cmd::CmdLineRunner; use crate::config::{Config, Settings}; use crate::http::{HTTP, HTTP_FETCH}; use crate::install_context::InstallContext; use crate::parallel; -use crate::toolset::{ToolVersion, Toolset}; +use crate::toolset::{ToolVersion, ToolVersionOptions, Toolset}; use crate::ui::progress_report::SingleReport; use crate::{dirs, env, file, plugins}; @@ -24,6 +25,27 @@ pub struct DotnetPlugin { ba: Arc, } +#[derive(Debug, Clone, Copy)] +struct DotnetOptions<'a> { + values: BackendOptions<'a>, +} + +impl<'a> DotnetOptions<'a> { + fn new(raw: &'a ToolVersionOptions) -> Self { + Self { + values: BackendOptions::new(raw), + } + } + + fn runtime(&self) -> Option<&'a str> { + self.values.str("runtime") + } + + fn runtime_framework_name(&self) -> Option<&'static str> { + self.runtime().and_then(runtime_framework_name) + } +} + impl DotnetPlugin { pub fn new() -> Self { Self { @@ -36,7 +58,9 @@ impl DotnetPlugin { } async fn test_dotnet(&self, ctx: &InstallContext, tv: &ToolVersion) -> Result<()> { - if tv.request.options().get("runtime").is_some() { + let raw_opts = tv.request.options(); + let opts = DotnetOptions::new(&raw_opts); + if opts.runtime().is_some() { // Skip version check for runtime-only installs — `dotnet --version` exits non-zero without an SDK return Ok(()); } @@ -126,7 +150,9 @@ impl Backend for DotnetPlugin { file::create_dir_all(&install_dir)?; // Read and validate runtime options - let runtime = tv.request.options().get("runtime").map(|s| s.to_string()); + let raw_opts = tv.request.options(); + let opts = DotnetOptions::new(&raw_opts); + let runtime = opts.runtime().map(str::to_string); if let Some(ref rt) = runtime && runtime_framework_name(rt).is_none() { @@ -176,10 +202,11 @@ impl Backend for DotnetPlugin { if Self::is_isolated() { // Isolated: mise handles removal of install_path by default } else { - let runtime = tv.request.options().get("runtime").map(|s| s.to_string()); - if let Some(rt) = runtime { + let raw_opts = tv.request.options(); + let opts = DotnetOptions::new(&raw_opts); + if let Some(rt) = opts.runtime() { // Runtime: remove the shared runtime directory for this version - let Some(framework) = runtime_framework_name(&rt) else { + let Some(framework) = runtime_framework_name(rt) else { return Ok(()); }; let runtime_dir = dotnet_root() @@ -386,3 +413,29 @@ impl PartialOrd for SortedVersion { Some(self.cmp(other)) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn opts_with_runtime(runtime: &str) -> ToolVersionOptions { + let mut opts = ToolVersionOptions::default(); + opts.opts.insert( + "runtime".to_string(), + toml::Value::String(runtime.to_string()), + ); + opts + } + + #[test] + fn dotnet_options_reads_runtime() { + let opts = opts_with_runtime("aspnetcore"); + let parsed = DotnetOptions::new(&opts); + + assert_eq!(parsed.runtime(), Some("aspnetcore")); + assert_eq!( + parsed.runtime_framework_name(), + Some("Microsoft.AspNetCore.App") + ); + } +} diff --git a/src/plugins/core/java.rs b/src/plugins/core/java.rs index 3d289e0b66..8652637b32 100644 --- a/src/plugins/core/java.rs +++ b/src/plugins/core/java.rs @@ -4,6 +4,7 @@ use std::fs::{self}; use std::path::{Path, PathBuf}; use std::sync::Arc; +use crate::backend::options::BackendOptions; use crate::backend::{ Backend, VersionInfo, normalize_idiomatic_contents, platform_target::PlatformTarget, }; @@ -17,7 +18,7 @@ use crate::http::{HTTP, HTTP_FETCH}; use crate::install_context::InstallContext; use crate::lockfile::PlatformInfo; use crate::platform::Platform; -use crate::toolset::{ToolRequest, ToolVersion, Toolset}; +use crate::toolset::{ToolRequest, ToolVersion, ToolVersionOptions, Toolset}; use crate::ui::progress_report::SingleReport; use crate::{file, plugins}; use async_trait::async_trait; @@ -47,6 +48,32 @@ pub struct JavaPlugin { tokio::sync::Mutex>>, } +#[derive(Debug, Clone, Copy)] +struct JavaOptions<'a> { + values: BackendOptions<'a>, +} + +impl<'a> JavaOptions<'a> { + fn new(raw: &'a ToolVersionOptions) -> Self { + Self { + values: BackendOptions::new(raw), + } + } + + fn release_type(&self) -> &'a str { + self.values.str("release_type").unwrap_or("ga") + } + + fn lockfile_options(&self) -> BTreeMap { + let mut opts = BTreeMap::new(); + let release_type = self.release_type(); + if release_type != "ga" { + opts.insert("release_type".to_string(), release_type.to_string()); + } + opts + } +} + impl JavaPlugin { pub fn new() -> Self { let settings = Settings::get(); @@ -297,11 +324,8 @@ impl JavaPlugin { } fn tv_release_type(&self, tv: &ToolVersion) -> String { - tv.request - .options() - .get("release_type") - .map(|s| s.to_string()) - .unwrap_or(String::from("ga")) + let raw_opts = tv.request.options(); + JavaOptions::new(&raw_opts).release_type().to_string() } fn tv_to_java_version(&self, tv: &ToolVersion) -> String { @@ -361,11 +385,8 @@ impl Backend for JavaPlugin { } async fn _list_remote_versions(&self, config: &Arc) -> Result> { - let opts = config.get_tool_opts_with_overrides(&self.ba).await?; - let release_type = opts - .get("release_type") - .map(|s| s.to_string()) - .unwrap_or_else(|| "ga".to_string()); + let raw_opts = config.get_tool_opts_with_overrides(&self.ba).await?; + let release_type = JavaOptions::new(&raw_opts).release_type().to_string(); let versions = self .fetch_java_metadata(&release_type) @@ -458,14 +479,8 @@ impl Backend for JavaPlugin { request: &ToolRequest, _target: &PlatformTarget, ) -> BTreeMap { - let mut opts = BTreeMap::new(); - if let Some(release_type) = request.options().get("release_type") { - let release_type = release_type.to_string(); - if release_type != "ga" { - opts.insert("release_type".to_string(), release_type); - } - } - opts + let raw_opts = request.options(); + JavaOptions::new(&raw_opts).lockfile_options() } async fn resolve_lock_info( @@ -759,6 +774,38 @@ impl JavaMetadata { } } +#[cfg(test)] +mod tests { + use super::*; + + fn opts_with_release_type(release_type: &str) -> ToolVersionOptions { + let mut opts = ToolVersionOptions::default(); + opts.opts.insert( + "release_type".to_string(), + toml::Value::String(release_type.to_string()), + ); + opts + } + + #[test] + fn java_options_reads_release_type() { + let default_opts = ToolVersionOptions::default(); + assert_eq!(JavaOptions::new(&default_opts).release_type(), "ga"); + assert!( + JavaOptions::new(&default_opts) + .lockfile_options() + .is_empty() + ); + + let opts = opts_with_release_type("ea"); + assert_eq!(JavaOptions::new(&opts).release_type(), "ea"); + assert_eq!( + JavaOptions::new(&opts).lockfile_options(), + BTreeMap::from([("release_type".to_string(), "ea".to_string())]) + ); + } +} + // only care about these features static JAVA_FEATURES: Lazy> = Lazy::new(|| { HashSet::from(["crac", "javafx", "jcef", "leyden", "lite", "musl"].map(|s| s.to_string())) diff --git a/src/plugins/core/python.rs b/src/plugins/core/python.rs index cd22f63d5e..0aa0b924b4 100644 --- a/src/plugins/core/python.rs +++ b/src/plugins/core/python.rs @@ -1,3 +1,4 @@ +use crate::backend::options::BackendOptions; use crate::backend::platform_target::PlatformTarget; use crate::backend::static_helpers::fetch_checksum_from_shasums; use crate::backend::{Backend, VersionCacheManager, VersionInfo}; @@ -12,7 +13,7 @@ use crate::http::{HTTP, HTTP_FETCH}; use crate::install_context::InstallContext; use crate::lockfile::{PlatformInfo, ProvenanceType}; use crate::platform::Platform; -use crate::toolset::{ToolRequest, ToolVersion, Toolset}; +use crate::toolset::{ToolRequest, ToolVersion, ToolVersionOptions, Toolset}; use crate::ui::progress_report::SingleReport; use crate::{Result, lock_file::LockFile}; use crate::{dirs, file, plugins, sysconfig}; @@ -37,6 +38,27 @@ pub struct PythonPlugin { ba: Arc, } +#[derive(Debug, Clone, Copy)] +struct PythonOptions<'a> { + values: BackendOptions<'a>, +} + +impl<'a> PythonOptions<'a> { + fn new(raw: &'a ToolVersionOptions) -> Self { + Self { + values: BackendOptions::new(raw), + } + } + + fn patch_sysconfig(&self) -> bool { + self.values.str("patch_sysconfig") != Some("false") + } + + fn virtualenv(&self) -> Option<&'a str> { + self.values.str("virtualenv") + } +} + pub fn python_path(tv: &ToolVersion) -> PathBuf { if cfg!(windows) { tv.install_path().join("python.exe") @@ -354,7 +376,9 @@ impl PythonPlugin { .map(|s| re_digits.replace(s, "").to_string()); if cfg!(unix) { if let (Some(major), Some(minor), Some(suffix)) = (major, minor, suffix) { - if tv.request.options().get("patch_sysconfig") != Some("false") { + let raw_opts = tv.request.options(); + let opts = PythonOptions::new(&raw_opts); + if opts.patch_sysconfig() { sysconfig::update_sysconfig(&install, major, minor, &suffix)?; } } else { @@ -447,7 +471,9 @@ impl PythonPlugin { config: &Arc, tv: &ToolVersion, ) -> eyre::Result> { - if let Some(virtualenv) = tv.request.options().get("virtualenv") { + let raw_opts = tv.request.options(); + let opts = PythonOptions::new(&raw_opts); + if let Some(virtualenv) = opts.virtualenv() { if !Settings::get().experimental { warn!( "please enable experimental mode with `mise settings experimental=true` \ @@ -1043,6 +1069,25 @@ fn filter_freethreaded(v: &str, flavor: &Option) -> bool { mod tests { use super::*; + fn opts_with(key: &str, value: &str) -> ToolVersionOptions { + let mut opts = ToolVersionOptions::default(); + opts.opts + .insert(key.to_string(), toml::Value::String(value.to_string())); + opts + } + + #[test] + fn python_options_reads_patch_sysconfig() { + assert!(PythonOptions::new(&ToolVersionOptions::default()).patch_sysconfig()); + assert!(!PythonOptions::new(&opts_with("patch_sysconfig", "false")).patch_sysconfig()); + } + + #[test] + fn python_options_reads_virtualenv() { + let opts = opts_with("virtualenv", ".venv"); + assert_eq!(PythonOptions::new(&opts).virtualenv(), Some(".venv")); + } + #[test] fn test_resolve_python_arch_windows_x64() { assert_eq!(resolve_python_arch("windows", "x64"), "x86_64"); diff --git a/src/plugins/core/rust.rs b/src/plugins/core/rust.rs index 98c56c531d..817d12f6f4 100644 --- a/src/plugins/core/rust.rs +++ b/src/plugins/core/rust.rs @@ -3,6 +3,7 @@ use std::{collections::BTreeMap, sync::Arc}; use crate::backend::Backend; use crate::backend::VersionInfo; +use crate::backend::options::BackendOptions; use crate::build_time::TARGET; use crate::cli::args::BackendArg; use crate::cmd::CmdLineRunner; @@ -11,7 +12,7 @@ use crate::http::HTTP; use crate::install_context::InstallContext; use crate::toolset::ToolSource::IdiomaticVersionFile; use crate::toolset::outdated_info::OutdatedInfo; -use crate::toolset::{ResolveOptions, ToolVersion, Toolset}; +use crate::toolset::{ResolveOptions, ToolVersion, ToolVersionOptions, Toolset}; use crate::ui::progress_report::SingleReport; use crate::{dirs, env, file, github, plugins}; use async_trait::async_trait; @@ -23,6 +24,46 @@ pub struct RustPlugin { ba: Arc, } +#[derive(Debug, Clone, Copy)] +struct RustOptions<'a> { + values: BackendOptions<'a>, +} + +impl<'a> RustOptions<'a> { + fn new(raw: &'a ToolVersionOptions) -> Self { + Self { + values: BackendOptions::new(raw), + } + } + + fn profile(&self) -> Option<&'a str> { + self.values.str("profile") + } + + fn comma_list(&self, name: &str) -> Option> { + self.values + .str(name) + .map(|c| c.split(',').map(|s| s.to_string()).collect()) + } + + fn install_args( + &self, + rt: Option<&RustToolchain>, + ) -> (Option, Option>, Option>) { + let profile = rt + .and_then(|rt| rt.profile.clone()) + .or_else(|| self.profile().map(str::to_string)); + let components = rt + .and_then(|rt| rt.components.clone()) + .or_else(|| self.comma_list("components")); + let targets = rt + .and_then(|rt| rt.targets.clone()) + .or_else(|| self.comma_list("targets")); + + (profile, components, targets) + } +} + impl RustPlugin { pub fn new() -> Self { Self { @@ -271,26 +312,8 @@ fn get_args(tv: &ToolVersion) -> (Option, Option>, Option Result { @@ -411,3 +434,50 @@ fn cargo_bin() -> PathBuf { fn cargo_bindir() -> PathBuf { cargo_home().join("bin") } + +#[cfg(test)] +mod tests { + use super::*; + + fn opts_with(key: &str, value: &str) -> ToolVersionOptions { + let mut opts = ToolVersionOptions::default(); + opts.opts + .insert(key.to_string(), toml::Value::String(value.to_string())); + opts + } + + #[test] + fn rust_options_reads_install_args() { + let mut opts = opts_with("profile", "minimal"); + opts.opts.insert( + "components".to_string(), + toml::Value::String("clippy,rustfmt".to_string()), + ); + opts.opts.insert( + "targets".to_string(), + toml::Value::String("wasm32-wasip1".to_string()), + ); + + let (profile, components, targets) = RustOptions::new(&opts).install_args(None); + + assert_eq!(profile, Some("minimal".to_string())); + assert_eq!( + components, + Some(vec!["clippy".to_string(), "rustfmt".to_string()]) + ); + assert_eq!(targets, Some(vec!["wasm32-wasip1".to_string()])); + } + + #[test] + fn rust_idiomatic_options_override_tool_options() { + let opts = opts_with("profile", "minimal"); + let rt = RustToolchain { + profile: Some("default".to_string()), + ..Default::default() + }; + + let (profile, _, _) = RustOptions::new(&opts).install_args(Some(&rt)); + + assert_eq!(profile, Some("default".to_string())); + } +} From 922b4abb4694c08a8d3d4b562e4b1d2f346d37e4 Mon Sep 17 00:00:00 2001 From: Taku Kodma <79110363+risu729@users.noreply.github.com> Date: Mon, 18 May 2026 20:54:52 +1000 Subject: [PATCH 2/8] refactor(core): address plugin option review --- src/plugins/core/python.rs | 2 ++ src/plugins/core/rust.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/plugins/core/python.rs b/src/plugins/core/python.rs index 0aa0b924b4..f5bd477b3a 100644 --- a/src/plugins/core/python.rs +++ b/src/plugins/core/python.rs @@ -51,6 +51,8 @@ impl<'a> PythonOptions<'a> { } fn patch_sysconfig(&self) -> bool { + // Preserve the existing option semantics: only the string "false" + // disables sysconfig patching. self.values.str("patch_sysconfig") != Some("false") } diff --git a/src/plugins/core/rust.rs b/src/plugins/core/rust.rs index 817d12f6f4..57fd6ad9e8 100644 --- a/src/plugins/core/rust.rs +++ b/src/plugins/core/rust.rs @@ -43,7 +43,7 @@ impl<'a> RustOptions<'a> { fn comma_list(&self, name: &str) -> Option> { self.values .str(name) - .map(|c| c.split(',').map(|s| s.to_string()).collect()) + .map(|c| c.split(',').map(|s| s.trim().to_string()).collect()) } fn install_args( @@ -451,7 +451,7 @@ mod tests { let mut opts = opts_with("profile", "minimal"); opts.opts.insert( "components".to_string(), - toml::Value::String("clippy,rustfmt".to_string()), + toml::Value::String("clippy, rustfmt".to_string()), ); opts.opts.insert( "targets".to_string(), From f5e73b612a694dd7a0142329f0a928d5fb389087 Mon Sep 17 00:00:00 2001 From: Taku Kodma <79110363+risu729@users.noreply.github.com> Date: Mon, 18 May 2026 21:13:25 +1000 Subject: [PATCH 3/8] refactor(core): use dotnet option parser in runtime checks --- src/plugins/core/dotnet.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/core/dotnet.rs b/src/plugins/core/dotnet.rs index c558b44adf..2f9a1d7611 100644 --- a/src/plugins/core/dotnet.rs +++ b/src/plugins/core/dotnet.rs @@ -154,7 +154,7 @@ impl Backend for DotnetPlugin { let opts = DotnetOptions::new(&raw_opts); let runtime = opts.runtime().map(str::to_string); if let Some(ref rt) = runtime - && runtime_framework_name(rt).is_none() + && opts.runtime_framework_name().is_none() { return Err(eyre::eyre!( "Invalid runtime option '{}'. Valid options: dotnet, aspnetcore, windowsdesktop", @@ -204,9 +204,9 @@ impl Backend for DotnetPlugin { } else { let raw_opts = tv.request.options(); let opts = DotnetOptions::new(&raw_opts); - if let Some(rt) = opts.runtime() { + if opts.runtime().is_some() { // Runtime: remove the shared runtime directory for this version - let Some(framework) = runtime_framework_name(rt) else { + let Some(framework) = opts.runtime_framework_name() else { return Ok(()); }; let runtime_dir = dotnet_root() From 32b088269c6530f4061d5b069b2632f5f83d3685 Mon Sep 17 00:00:00 2001 From: Taku Kodma <79110363+risu729@users.noreply.github.com> Date: Mon, 18 May 2026 21:39:13 +1000 Subject: [PATCH 4/8] refactor(core): place java option tests after items --- src/plugins/core/java.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plugins/core/java.rs b/src/plugins/core/java.rs index 8652637b32..0e2f220209 100644 --- a/src/plugins/core/java.rs +++ b/src/plugins/core/java.rs @@ -774,6 +774,11 @@ impl JavaMetadata { } } +// only care about these features +static JAVA_FEATURES: Lazy> = Lazy::new(|| { + HashSet::from(["crac", "javafx", "jcef", "leyden", "lite", "musl"].map(|s| s.to_string())) +}); + #[cfg(test)] mod tests { use super::*; @@ -805,8 +810,3 @@ mod tests { ); } } - -// only care about these features -static JAVA_FEATURES: Lazy> = Lazy::new(|| { - HashSet::from(["crac", "javafx", "jcef", "leyden", "lite", "musl"].map(|s| s.to_string())) -}); From 3fc24b2c6cb1dc4a400074cd5fa2f93b4d7df020 Mon Sep 17 00:00:00 2001 From: Taku Kodma <79110363+risu729@users.noreply.github.com> Date: Tue, 19 May 2026 05:03:05 +1000 Subject: [PATCH 5/8] refactor(core): parse boolean tool options consistently --- src/backend/mod.rs | 20 ++++--- src/backend/options.rs | 107 +++++++++++++++++++++++++++++++++++-- src/plugins/core/python.rs | 22 ++++++-- 3 files changed, 135 insertions(+), 14 deletions(-) diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 60cac5c9c2..a1300b2db0 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -765,10 +765,22 @@ mod tests { opts.opts.insert( "prerelease".to_string(), - toml::Value::String("false".into()), + toml::Value::String("FALSE".into()), ); assert!(!backend.include_prereleases(&opts)); + opts.opts + .insert("prerelease".to_string(), toml::Value::String("1".into())); + assert!(backend.include_prereleases(&opts)); + + opts.opts + .insert("prerelease".to_string(), toml::Value::String("0".into())); + assert!(!backend.include_prereleases(&opts)); + + opts.opts + .insert("prerelease".to_string(), toml::Value::String("00".into())); + assert!(!backend.include_prereleases(&opts)); + // Defense-in-depth: also accept a native TOML boolean, in case a future // config path stores the value without string normalization. opts.opts @@ -2881,11 +2893,7 @@ pub(crate) fn mark_prerelease(mut version: VersionInfo) -> VersionInfo { } fn tool_option_bool(value: &toml::Value) -> bool { - match value { - toml::Value::Boolean(b) => *b, - toml::Value::String(s) => s.parse::().unwrap_or(false), - _ => false, - } + crate::backend::options::bool_value_or_default("prerelease", value, false) } /// Fuzzy-match `versions` against `query` with PEP 440 prerelease detection diff --git a/src/backend/options.rs b/src/backend/options.rs index 0265298b00..a5733b61aa 100644 --- a/src/backend/options.rs +++ b/src/backend/options.rs @@ -47,11 +47,18 @@ impl<'a> BackendOptions<'a> { pub(crate) fn platform_bool_for_target(&self, key: &str, target: &PlatformTarget) -> bool { self.platform_string_for_target(key, target) - .is_some_and(|v| is_truthy(&v)) + .is_some_and(|v| bool_str_or_default(key, &v, false)) } pub(crate) fn bool(&self, key: &str) -> bool { - self.raw.get_string(key).is_some_and(|v| is_truthy(&v)) + self.bool_with_default(key, false) + } + + pub(crate) fn bool_with_default(&self, key: &str, default: bool) -> bool { + self.raw + .opts + .get(key) + .map_or(default, |value| bool_value_or_default(key, value, default)) } pub(crate) fn available_platforms_with_key(&self, key: &str) -> Vec { @@ -60,7 +67,42 @@ impl<'a> BackendOptions<'a> { } pub(crate) fn is_truthy(value: &str) -> bool { - matches!(value.trim(), "true" | "1") + matches!(value.trim().to_ascii_lowercase().as_str(), "true" | "1") +} + +pub(crate) fn is_falsey(value: &str) -> bool { + matches!(value.trim().to_ascii_lowercase().as_str(), "false" | "0") +} + +pub(crate) fn bool_value_or_default(key: &str, value: &toml::Value, default: bool) -> bool { + match value { + toml::Value::Boolean(value) => *value, + toml::Value::String(value) => bool_str_or_default(key, value, default), + toml::Value::Integer(0) => false, + toml::Value::Integer(1) => true, + _ => invalid_bool_value(key, value, default), + } +} + +fn bool_str_or_default(key: &str, value: &str, default: bool) -> bool { + parse_bool_str(value).unwrap_or_else(|| invalid_bool_value(key, value, default)) +} + +fn parse_bool_str(value: &str) -> Option { + if is_truthy(value) { + Some(true) + } else if is_falsey(value) { + Some(false) + } else { + None + } +} + +fn invalid_bool_value(key: &str, value: impl std::fmt::Display, default: bool) -> bool { + warn!( + "invalid boolean value for tool option `{key}`: {value}; expected true, false, 1, or 0; using {default}" + ); + default } #[cfg(test)] @@ -68,6 +110,65 @@ mod tests { use super::*; use crate::platform::Platform; + fn opts_with_value(key: &str, value: toml::Value) -> ToolVersionOptions { + let mut opts = ToolVersionOptions::default(); + opts.opts.insert(key.to_string(), value); + opts + } + + #[test] + fn test_bool_parses_consistent_formats() { + assert!( + BackendOptions::new(&opts_with_value("flag", toml::Value::Boolean(true))).bool("flag") + ); + assert!( + !BackendOptions::new(&opts_with_value("flag", toml::Value::Boolean(false))) + .bool("flag") + ); + assert!( + BackendOptions::new(&opts_with_value("flag", toml::Value::String("TRUE".into()))) + .bool("flag") + ); + assert!( + !BackendOptions::new(&opts_with_value( + "flag", + toml::Value::String("FALSE".into()) + )) + .bool("flag") + ); + assert!( + BackendOptions::new(&opts_with_value("flag", toml::Value::String("1".into()))) + .bool("flag") + ); + assert!( + !BackendOptions::new(&opts_with_value("flag", toml::Value::String("0".into()))) + .bool("flag") + ); + assert!( + BackendOptions::new(&opts_with_value("flag", toml::Value::Integer(1))).bool("flag") + ); + assert!( + !BackendOptions::new(&opts_with_value("flag", toml::Value::Integer(0))).bool("flag") + ); + } + + #[test] + fn test_bool_invalid_values_fall_back_to_default() { + assert!(!BackendOptions::new(&ToolVersionOptions::default()).bool("missing")); + assert!( + !BackendOptions::new(&opts_with_value("flag", toml::Value::String("00".into()))) + .bool("flag") + ); + assert!( + BackendOptions::new(&opts_with_value("flag", toml::Value::String("00".into()))) + .bool_with_default("flag", true) + ); + assert!( + BackendOptions::new(&opts_with_value("flag", toml::Value::Integer(2))) + .bool_with_default("flag", true) + ); + } + #[test] fn test_platform_bool_for_target_uses_requested_target() { let mut opts = ToolVersionOptions::default(); diff --git a/src/plugins/core/python.rs b/src/plugins/core/python.rs index f5bd477b3a..57bfe1a984 100644 --- a/src/plugins/core/python.rs +++ b/src/plugins/core/python.rs @@ -51,9 +51,7 @@ impl<'a> PythonOptions<'a> { } fn patch_sysconfig(&self) -> bool { - // Preserve the existing option semantics: only the string "false" - // disables sysconfig patching. - self.values.str("patch_sysconfig") != Some("false") + self.values.bool_with_default("patch_sysconfig", true) } fn virtualenv(&self) -> Option<&'a str> { @@ -1072,9 +1070,12 @@ mod tests { use super::*; fn opts_with(key: &str, value: &str) -> ToolVersionOptions { + opts_with_value(key, toml::Value::String(value.to_string())) + } + + fn opts_with_value(key: &str, value: toml::Value) -> ToolVersionOptions { let mut opts = ToolVersionOptions::default(); - opts.opts - .insert(key.to_string(), toml::Value::String(value.to_string())); + opts.opts.insert(key.to_string(), value); opts } @@ -1082,6 +1083,17 @@ mod tests { fn python_options_reads_patch_sysconfig() { assert!(PythonOptions::new(&ToolVersionOptions::default()).patch_sysconfig()); assert!(!PythonOptions::new(&opts_with("patch_sysconfig", "false")).patch_sysconfig()); + assert!(!PythonOptions::new(&opts_with("patch_sysconfig", "FALSE")).patch_sysconfig()); + assert!(!PythonOptions::new(&opts_with("patch_sysconfig", "0")).patch_sysconfig()); + assert!( + !PythonOptions::new(&opts_with_value( + "patch_sysconfig", + toml::Value::Boolean(false) + )) + .patch_sysconfig() + ); + assert!(PythonOptions::new(&opts_with("patch_sysconfig", "1")).patch_sysconfig()); + assert!(PythonOptions::new(&opts_with("patch_sysconfig", "00")).patch_sysconfig()); } #[test] From c6fb99ac714bc10e024d115b4bc5d962036fe491 Mon Sep 17 00:00:00 2001 From: Taku Kodma <79110363+risu729@users.noreply.github.com> Date: Tue, 19 May 2026 05:14:00 +1000 Subject: [PATCH 6/8] refactor(core): expose optional boolean parser --- src/backend/options.rs | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/backend/options.rs b/src/backend/options.rs index a5733b61aa..561cedb6db 100644 --- a/src/backend/options.rs +++ b/src/backend/options.rs @@ -75,17 +75,28 @@ pub(crate) fn is_falsey(value: &str) -> bool { } pub(crate) fn bool_value_or_default(key: &str, value: &toml::Value, default: bool) -> bool { - match value { - toml::Value::Boolean(value) => *value, - toml::Value::String(value) => bool_str_or_default(key, value, default), - toml::Value::Integer(0) => false, - toml::Value::Integer(1) => true, - _ => invalid_bool_value(key, value, default), - } + bool_value(key, value).unwrap_or(default) +} + +pub(crate) fn bool_value(key: &str, value: &toml::Value) -> Option { + let parsed = match value { + toml::Value::Boolean(value) => Some(*value), + toml::Value::String(value) => parse_bool_str(value), + toml::Value::Integer(0) => Some(false), + toml::Value::Integer(1) => Some(true), + _ => None, + }; + if parsed.is_none() { + warn_invalid_bool_value(key, value); + } + parsed } fn bool_str_or_default(key: &str, value: &str, default: bool) -> bool { - parse_bool_str(value).unwrap_or_else(|| invalid_bool_value(key, value, default)) + parse_bool_str(value).unwrap_or_else(|| { + warn_invalid_bool_value(key, value); + default + }) } fn parse_bool_str(value: &str) -> Option { @@ -98,11 +109,10 @@ fn parse_bool_str(value: &str) -> Option { } } -fn invalid_bool_value(key: &str, value: impl std::fmt::Display, default: bool) -> bool { +fn warn_invalid_bool_value(key: &str, value: impl std::fmt::Display) { warn!( - "invalid boolean value for tool option `{key}`: {value}; expected true, false, 1, or 0; using {default}" + "invalid boolean value for tool option `{key}`: {value}; expected true, false, 1, or 0; using default" ); - default } #[cfg(test)] @@ -167,6 +177,7 @@ mod tests { BackendOptions::new(&opts_with_value("flag", toml::Value::Integer(2))) .bool_with_default("flag", true) ); + assert_eq!(bool_value("flag", &toml::Value::String("00".into())), None); } #[test] From 16230d8e08fc9f6ed123922e4b92fbaad01de977 Mon Sep 17 00:00:00 2001 From: Taku Kodma <79110363+risu729@users.noreply.github.com> Date: Mon, 18 May 2026 20:40:32 +1000 Subject: [PATCH 7/8] refactor(spm): parse tool options locally --- src/backend/spm.rs | 276 ++++++++++++++++++++++++++------------------- 1 file changed, 158 insertions(+), 118 deletions(-) diff --git a/src/backend/spm.rs b/src/backend/spm.rs index 8670f0c3ac..bae2d9ba24 100644 --- a/src/backend/spm.rs +++ b/src/backend/spm.rs @@ -1,6 +1,7 @@ use crate::backend::Backend; use crate::backend::VersionInfo; use crate::backend::backend_type::BackendType; +use crate::backend::options::BackendOptions; use crate::cli::args::BackendArg; use crate::cmd::CmdLineRunner; use crate::config::{Config, Settings}; @@ -32,6 +33,64 @@ pub struct SPMBackend { ba: Arc, } +#[derive(Debug, Clone, Copy)] +struct SpmOptions<'a> { + values: BackendOptions<'a>, +} + +impl<'a> SpmOptions<'a> { + fn new(raw: &'a ToolVersionOptions) -> Self { + Self { + values: BackendOptions::new(raw), + } + } + + fn provider(&self) -> &'a str { + self.values + .str("provider") + .unwrap_or(GitProviderKind::GitHub.as_ref()) + } + + fn api_url(&self) -> Option<&'a str> { + self.values.str("api_url") + } + + fn artifactbundle_asset(&self) -> Option<&'a str> { + self.values.str("artifactbundle_asset") + } + + fn artifactbundle_mode(&self) -> eyre::Result { + match self.values.raw().get_string("artifactbundle").as_deref() { + None => Ok(ArtifactBundleMode::Auto), + Some("true") => Ok(ArtifactBundleMode::Required), + Some("false") => Ok(ArtifactBundleMode::SourceOnly), + Some(value) => bail!("artifactbundle must be true or false, got {value}"), + } + } + + fn requires_artifactbundle(&self, mode: ArtifactBundleMode) -> bool { + mode.requires_artifactbundle() || self.artifactbundle_asset().is_some() + } + + fn filter_bins(&self) -> Option> { + let value = self.values.raw().opts.get("filter_bins")?; + let bins: Vec = match value { + toml::Value::Array(arr) => arr + .iter() + .filter_map(|v| v.as_str().map(|s| s.trim().to_string())) + .filter(|s| !s.is_empty()) + .collect(), + toml::Value::String(s) => s + .split(',') + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect(), + _ => return None, + }; + if bins.is_empty() { None } else { Some(bins) } + } +} + #[async_trait] impl Backend for SPMBackend { fn get_type(&self) -> BackendType { @@ -55,7 +114,8 @@ impl Backend for SPMBackend { } async fn _list_remote_versions(&self, config: &Arc) -> eyre::Result> { - let opts = config.get_tool_opts_with_overrides(&self.ba).await?; + let raw_opts = config.get_tool_opts_with_overrides(&self.ba).await?; + let opts = SpmOptions::new(&raw_opts); let provider = GitProvider::from_ba_with_opts(&self.ba, &opts); let repo = SwiftPackageRepo::new(&self.tool_name(), &provider)?; let versions = match provider.kind { @@ -105,7 +165,8 @@ impl Backend for SPMBackend { ) .await; let mut tv = tv; - let opts = tv.request.options(); + let raw_opts = tv.request.options(); + let opts = SpmOptions::new(&raw_opts); let provider = GitProvider::from_ba_with_opts(&self.ba, &opts); let repo = SwiftPackageRepo::new(&self.tool_name(), &provider)?; let revision = if tv.version == "latest" { @@ -116,14 +177,14 @@ impl Backend for SPMBackend { tv.version.clone() }; - let artifactbundle_mode = resolve_artifactbundle_mode(&opts)?; + let artifactbundle_mode = opts.artifactbundle_mode()?; if artifactbundle_mode == ArtifactBundleMode::SourceOnly && Settings::get().spm.artifactbundle_only { bail!("artifactbundle = false conflicts with spm.artifactbundle_only"); } if artifactbundle_mode != ArtifactBundleMode::SourceOnly { - let artifactbundle_required = requires_artifactbundle(artifactbundle_mode, &opts); + let artifactbundle_required = opts.requires_artifactbundle(artifactbundle_mode); match self .try_install_artifactbundle(ctx, &mut tv, &provider, &repo, &revision, &opts) .await @@ -226,7 +287,8 @@ impl SPMBackend { tv: &ToolVersion, executables: Vec, ) -> eyre::Result> { - let opts = tv.request.options(); + let raw_opts = tv.request.options(); + let opts = SpmOptions::new(&raw_opts); filter_executables(&opts, executables) } @@ -323,7 +385,7 @@ impl SPMBackend { provider: &GitProvider, repo: &SwiftPackageRepo, revision: &str, - opts: &ToolVersionOptions, + opts: &SpmOptions<'_>, ) -> eyre::Result { let Some(asset) = resolve_artifactbundle_asset(provider, repo, revision, opts).await? else { @@ -397,37 +459,15 @@ fn with_install_env(mut command: Expression, tv: &ToolVersion) -> Expression { command } -/// Parses the `filter_bins` tool option if set. -/// -/// Accepts either a comma-separated string (`filter_bins = "foo,bar"`) or a -/// TOML array (`filter_bins = ["foo", "bar"]`). Empty entries are ignored. -fn parse_filter_bins(opts: &crate::toolset::ToolVersionOptions) -> Option> { - let value = opts.opts.get("filter_bins")?; - let bins: Vec = match value { - toml::Value::Array(arr) => arr - .iter() - .filter_map(|v| v.as_str().map(|s| s.trim().to_string())) - .filter(|s| !s.is_empty()) - .collect(), - toml::Value::String(s) => s - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect(), - _ => return None, - }; - if bins.is_empty() { None } else { Some(bins) } -} - /// Restricts `executables` to those listed in `filter_bins`, preserving the /// original declaration order from `Package.swift` rather than the order in /// `filter_bins`. Returns an error if any name in `filter_bins` does not match /// an available executable product. fn filter_executables( - opts: &crate::toolset::ToolVersionOptions, + opts: &SpmOptions<'_>, executables: Vec, ) -> eyre::Result> { - let Some(filter) = parse_filter_bins(opts) else { + let Some(filter) = opts.filter_bins() else { return Ok(executables); }; let missing: Vec<&str> = filter @@ -474,14 +514,13 @@ pub enum GitProviderKind { impl GitProvider { #[cfg(test)] fn from_ba(ba: &BackendArg) -> Self { - let opts = ba.opts(); + let raw_opts = ba.opts(); + let opts = SpmOptions::new(&raw_opts); Self::from_ba_with_opts(ba, &opts) } - fn from_ba_with_opts(ba: &BackendArg, opts: &ToolVersionOptions) -> Self { - let provider = opts - .get("provider") - .unwrap_or(GitProviderKind::GitHub.as_ref()); + fn from_ba_with_opts(ba: &BackendArg, opts: &SpmOptions<'_>) -> Self { + let provider = opts.provider(); let kind = if ba.tool_name.contains("gitlab.com") { GitProviderKind::GitLab } else { @@ -491,7 +530,7 @@ impl GitProvider { } }; - let api_url = match opts.get("api_url") { + let api_url = match opts.api_url() { Some(api_url) => api_url.trim_end_matches('/').to_string(), None => { Self::derive_api_url_from_tool_name(&ba.tool_name, &kind).unwrap_or_else(|| { @@ -596,19 +635,6 @@ impl ArtifactBundleMode { } } -fn resolve_artifactbundle_mode(opts: &ToolVersionOptions) -> eyre::Result { - match opts.get_string("artifactbundle").as_deref() { - None => Ok(ArtifactBundleMode::Auto), - Some("true") => Ok(ArtifactBundleMode::Required), - Some("false") => Ok(ArtifactBundleMode::SourceOnly), - Some(value) => bail!("artifactbundle must be true or false, got {value}"), - } -} - -fn requires_artifactbundle(mode: ArtifactBundleMode, opts: &ToolVersionOptions) -> bool { - mode.requires_artifactbundle() || opts.get("artifactbundle_asset").is_some() -} - #[derive(Clone, Debug, Eq, PartialEq)] struct ArtifactBundleReleaseAsset { name: String, @@ -621,7 +647,7 @@ async fn resolve_artifactbundle_asset( provider: &GitProvider, repo: &SwiftPackageRepo, revision: &str, - opts: &ToolVersionOptions, + opts: &SpmOptions<'_>, ) -> eyre::Result> { let assets = match provider.kind { GitProviderKind::GitLab => { @@ -668,9 +694,9 @@ async fn resolve_artifactbundle_asset( fn select_artifactbundle_asset( assets: Vec, - opts: &ToolVersionOptions, + opts: &SpmOptions<'_>, ) -> eyre::Result> { - let artifactbundle_asset = opts.get("artifactbundle_asset"); + let artifactbundle_asset = opts.artifactbundle_asset(); if let Some(name) = artifactbundle_asset { if !is_artifactbundle_zip(name) { bail!("artifactbundle_asset must end with .artifactbundle.zip, got {name}"); @@ -820,7 +846,7 @@ fn is_artifactbundle_dir(path: &Path) -> bool { } fn filter_artifactbundle_binaries( - opts: &ToolVersionOptions, + opts: &SpmOptions<'_>, binaries: Vec, ) -> eyre::Result> { let names = binaries.iter().map(|b| b.name.clone()).collect::>(); @@ -1052,111 +1078,123 @@ mod tests { } } + fn filter_bins(raw: &ToolVersionOptions) -> Option> { + SpmOptions::new(raw).filter_bins() + } + + fn filter_executables_with_raw( + raw: &ToolVersionOptions, + executables: Vec, + ) -> eyre::Result> { + let opts = SpmOptions::new(raw); + filter_executables(&opts, executables) + } + #[test] fn test_resolve_artifactbundle_mode() { + let default_opts = ToolVersionOptions::default(); assert_eq!( - resolve_artifactbundle_mode(&ToolVersionOptions::default()).unwrap(), + SpmOptions::new(&default_opts) + .artifactbundle_mode() + .unwrap(), ArtifactBundleMode::Auto ); + let required_opts = opts_with("artifactbundle", toml::Value::Boolean(true)); assert_eq!( - resolve_artifactbundle_mode(&opts_with("artifactbundle", toml::Value::Boolean(true))) + SpmOptions::new(&required_opts) + .artifactbundle_mode() .unwrap(), ArtifactBundleMode::Required ); + let source_only_opts = opts_with("artifactbundle", toml::Value::Boolean(false)); assert_eq!( - resolve_artifactbundle_mode(&opts_with("artifactbundle", toml::Value::Boolean(false))) + SpmOptions::new(&source_only_opts) + .artifactbundle_mode() .unwrap(), ArtifactBundleMode::SourceOnly ); + let invalid_opts = opts_with( + "artifactbundle", + toml::Value::String("sometimes".to_string()), + ); assert!( - resolve_artifactbundle_mode(&opts_with( - "artifactbundle", - toml::Value::String("sometimes".to_string()) - )) - .is_err() + SpmOptions::new(&invalid_opts) + .artifactbundle_mode() + .is_err() ); } #[test] fn test_requires_artifactbundle() { - assert!(!requires_artifactbundle( - ArtifactBundleMode::Auto, - &ToolVersionOptions::default() - )); - assert!(requires_artifactbundle( - ArtifactBundleMode::Required, - &ToolVersionOptions::default() - )); - assert!(requires_artifactbundle( - ArtifactBundleMode::Auto, - &opts_with( - "artifactbundle_asset", - toml::Value::String("tool.artifactbundle.zip".to_string()), - ) - )); + let default_opts = ToolVersionOptions::default(); + let opts = SpmOptions::new(&default_opts); + assert!(!opts.requires_artifactbundle(ArtifactBundleMode::Auto)); + assert!(opts.requires_artifactbundle(ArtifactBundleMode::Required)); + + let asset_opts = opts_with( + "artifactbundle_asset", + toml::Value::String("tool.artifactbundle.zip".to_string()), + ); + let opts = SpmOptions::new(&asset_opts); + assert!(opts.requires_artifactbundle(ArtifactBundleMode::Auto)); } #[test] fn test_select_artifactbundle_asset() { - let selected = select_artifactbundle_asset( - vec![release_asset("tool.artifactbundle.zip")], - &ToolVersionOptions::default(), - ) - .unwrap() - .unwrap(); + let default_opts = ToolVersionOptions::default(); + let opts = SpmOptions::new(&default_opts); + let selected = + select_artifactbundle_asset(vec![release_asset("tool.artifactbundle.zip")], &opts) + .unwrap() + .unwrap(); assert_eq!(selected.name, "tool.artifactbundle.zip"); + let selected_opts = opts_with( + "artifactbundle_asset", + toml::Value::String("tool.artifactbundle.zip".to_string()), + ); + let opts = SpmOptions::new(&selected_opts); let selected = select_artifactbundle_asset( vec![ release_asset("tool.artifactbundle.zip"), release_asset("tool.tar.gz"), ], - &opts_with( - "artifactbundle_asset", - toml::Value::String("tool.artifactbundle.zip".to_string()), - ), + &opts, ) .unwrap() .unwrap(); assert_eq!(selected.name, "tool.artifactbundle.zip"); - assert!( - select_artifactbundle_asset( - vec![release_asset("tool.tar.gz")], - &opts_with( - "artifactbundle_asset", - toml::Value::String("tool.tar.gz".to_string()), - ), - ) - .is_err() + let non_bundle_opts = opts_with( + "artifactbundle_asset", + toml::Value::String("tool.tar.gz".to_string()), + ); + let opts = SpmOptions::new(&non_bundle_opts); + assert!(select_artifactbundle_asset(vec![release_asset("tool.tar.gz")], &opts).is_err()); + let missing_opts = opts_with( + "artifactbundle_asset", + toml::Value::String("missing.artifactbundle.zip".to_string()), ); + let opts = SpmOptions::new(&missing_opts); assert!( - select_artifactbundle_asset( - vec![release_asset("tool.artifactbundle.zip")], - &opts_with( - "artifactbundle_asset", - toml::Value::String("missing.artifactbundle.zip".to_string()), - ), - ) - .is_err() + select_artifactbundle_asset(vec![release_asset("tool.artifactbundle.zip")], &opts,) + .is_err() ); + let opts = SpmOptions::new(&default_opts); assert!( select_artifactbundle_asset( vec![ release_asset("a.artifactbundle.zip"), release_asset("b.artifactbundle.zip"), ], - &ToolVersionOptions::default(), + &opts, ) .is_err() ); assert!( - select_artifactbundle_asset( - vec![release_asset("tool.tar.gz")], - &ToolVersionOptions::default(), - ) - .unwrap() - .is_none() + select_artifactbundle_asset(vec![release_asset("tool.tar.gz")], &opts,) + .unwrap() + .is_none() ); } @@ -1339,6 +1377,7 @@ mod tests { }, ]; let opts = opts_with_filter_bins(toml::Value::String("b".to_string())); + let opts = SpmOptions::new(&opts); let filtered = filter_artifactbundle_binaries(&opts, binaries).unwrap(); assert_eq!(filtered.len(), 1); assert_eq!(filtered[0].name, "b"); @@ -1346,24 +1385,24 @@ mod tests { #[test] fn test_parse_filter_bins() { - assert_eq!(parse_filter_bins(&ToolVersionOptions::default()), None); + assert_eq!(filter_bins(&ToolVersionOptions::default()), None); assert_eq!( - parse_filter_bins(&opts_with_filter_bins(toml::Value::String( + filter_bins(&opts_with_filter_bins(toml::Value::String( "swiftly".to_string() ))), Some(vec!["swiftly".to_string()]) ); assert_eq!( - parse_filter_bins(&opts_with_filter_bins(toml::Value::String( + filter_bins(&opts_with_filter_bins(toml::Value::String( " foo , bar , ".to_string() ))), Some(vec!["foo".to_string(), "bar".to_string()]) ); assert_eq!( - parse_filter_bins(&opts_with_filter_bins(toml::Value::Array(vec![ + filter_bins(&opts_with_filter_bins(toml::Value::Array(vec![ toml::Value::String("foo".to_string()), toml::Value::String(" bar".to_string()), toml::Value::String("".to_string()), @@ -1372,7 +1411,7 @@ mod tests { ); assert_eq!( - parse_filter_bins(&opts_with_filter_bins(toml::Value::String( + filter_bins(&opts_with_filter_bins(toml::Value::String( " , ".to_string() ))), None, @@ -1384,7 +1423,8 @@ mod tests { fn test_filter_executables_passthrough_when_unset() { let executables = vec!["a".to_string(), "b".to_string()]; let result = - filter_executables(&ToolVersionOptions::default(), executables.clone()).unwrap(); + filter_executables_with_raw(&ToolVersionOptions::default(), executables.clone()) + .unwrap(); assert_eq!(result, executables); } @@ -1399,7 +1439,7 @@ mod tests { toml::Value::String("helper".to_string()), toml::Value::String("swiftly".to_string()), ])); - let result = filter_executables(&opts, executables).unwrap(); + let result = filter_executables_with_raw(&opts, executables).unwrap(); assert_eq!(result, vec!["swiftly".to_string(), "helper".to_string()]); } @@ -1407,7 +1447,7 @@ mod tests { fn test_filter_executables_errors_on_missing_name() { let executables = vec!["swiftly".to_string(), "test-swiftly".to_string()]; let opts = opts_with_filter_bins(toml::Value::String("does-not-exist".to_string())); - let err = filter_executables(&opts, executables).unwrap_err(); + let err = filter_executables_with_raw(&opts, executables).unwrap_err(); let msg = err.to_string(); assert!(msg.contains("does-not-exist"), "got: {msg}"); assert!(msg.contains("swiftly"), "got: {msg}"); From ed6ed553debc68921b284f1e189ecabb0559ec6d Mon Sep 17 00:00:00 2001 From: Taku Kodma <79110363+risu729@users.noreply.github.com> Date: Tue, 19 May 2026 05:16:50 +1000 Subject: [PATCH 8/8] refactor(spm): use shared bool option parser --- src/backend/spm.rs | 63 ++++++++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/src/backend/spm.rs b/src/backend/spm.rs index bae2d9ba24..884f27fc93 100644 --- a/src/backend/spm.rs +++ b/src/backend/spm.rs @@ -1,7 +1,7 @@ use crate::backend::Backend; use crate::backend::VersionInfo; use crate::backend::backend_type::BackendType; -use crate::backend::options::BackendOptions; +use crate::backend::options::{BackendOptions, bool_value}; use crate::cli::args::BackendArg; use crate::cmd::CmdLineRunner; use crate::config::{Config, Settings}; @@ -59,12 +59,14 @@ impl<'a> SpmOptions<'a> { self.values.str("artifactbundle_asset") } - fn artifactbundle_mode(&self) -> eyre::Result { - match self.values.raw().get_string("artifactbundle").as_deref() { - None => Ok(ArtifactBundleMode::Auto), - Some("true") => Ok(ArtifactBundleMode::Required), - Some("false") => Ok(ArtifactBundleMode::SourceOnly), - Some(value) => bail!("artifactbundle must be true or false, got {value}"), + fn artifactbundle_mode(&self) -> ArtifactBundleMode { + let Some(value) = self.values.raw().opts.get("artifactbundle") else { + return ArtifactBundleMode::Auto; + }; + match bool_value("artifactbundle", value) { + Some(true) => ArtifactBundleMode::Required, + Some(false) => ArtifactBundleMode::SourceOnly, + None => ArtifactBundleMode::Auto, } } @@ -177,7 +179,7 @@ impl Backend for SPMBackend { tv.version.clone() }; - let artifactbundle_mode = opts.artifactbundle_mode()?; + let artifactbundle_mode = opts.artifactbundle_mode(); if artifactbundle_mode == ArtifactBundleMode::SourceOnly && Settings::get().spm.artifactbundle_only { @@ -1094,33 +1096,52 @@ mod tests { fn test_resolve_artifactbundle_mode() { let default_opts = ToolVersionOptions::default(); assert_eq!( - SpmOptions::new(&default_opts) - .artifactbundle_mode() - .unwrap(), + SpmOptions::new(&default_opts).artifactbundle_mode(), ArtifactBundleMode::Auto ); let required_opts = opts_with("artifactbundle", toml::Value::Boolean(true)); assert_eq!( - SpmOptions::new(&required_opts) - .artifactbundle_mode() - .unwrap(), + SpmOptions::new(&required_opts).artifactbundle_mode(), ArtifactBundleMode::Required ); let source_only_opts = opts_with("artifactbundle", toml::Value::Boolean(false)); assert_eq!( - SpmOptions::new(&source_only_opts) - .artifactbundle_mode() - .unwrap(), + SpmOptions::new(&source_only_opts).artifactbundle_mode(), + ArtifactBundleMode::SourceOnly + ); + let required_opts = opts_with("artifactbundle", toml::Value::String("TRUE".to_string())); + assert_eq!( + SpmOptions::new(&required_opts).artifactbundle_mode(), + ArtifactBundleMode::Required + ); + let required_opts = opts_with("artifactbundle", toml::Value::String("1".to_string())); + assert_eq!( + SpmOptions::new(&required_opts).artifactbundle_mode(), + ArtifactBundleMode::Required + ); + let source_only_opts = + opts_with("artifactbundle", toml::Value::String("FALSE".to_string())); + assert_eq!( + SpmOptions::new(&source_only_opts).artifactbundle_mode(), ArtifactBundleMode::SourceOnly ); + let source_only_opts = opts_with("artifactbundle", toml::Value::String("0".to_string())); + assert_eq!( + SpmOptions::new(&source_only_opts).artifactbundle_mode(), + ArtifactBundleMode::SourceOnly + ); + let invalid_opts = opts_with("artifactbundle", toml::Value::String("00".to_string())); + assert_eq!( + SpmOptions::new(&invalid_opts).artifactbundle_mode(), + ArtifactBundleMode::Auto + ); let invalid_opts = opts_with( "artifactbundle", toml::Value::String("sometimes".to_string()), ); - assert!( - SpmOptions::new(&invalid_opts) - .artifactbundle_mode() - .is_err() + assert_eq!( + SpmOptions::new(&invalid_opts).artifactbundle_mode(), + ArtifactBundleMode::Auto ); }