From 20d665bb1247500d514f05b160d01f5f8e223a38 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Thu, 4 Jun 2026 22:37:18 +0900 Subject: feat(loader): wire RootPackageLoader::load end-to-end Resolve the remaining todo!()/placeholder sites in RootPackageLoader::load so the root package loads with real data: - Unbox config at use sites and change load()'s parameter from IndexMap> to IndexMap, matching ArrayLoader::load / VersionGuesser::guess_version and dropping the redundant box/unbox round-trip at the factory call site. - Expose replace_version through CompletePackage/RootPackage inherent delegation and a RootPackageHandle method (PHP Package::replaceVersion, inherited), and apply it for the auto-versioned default. - Collect require/require-dev links via getter dispatch and build the pretty-string map feeding extractAliases/StabilityFlags/References. Also reconcile the package repositories type with Config: change CompletePackageInterface::{get,set}_repositories from Vec> to IndexMap (PHP's array), wire setRepositories(config.getRepositories()), and dump repositories verbatim as a keyed array (matching PHP ArrayDumper) instead of forcing a list. Co-Authored-By: Claude Opus 4.8 --- crates/shirabe/src/factory.rs | 5 +- .../shirabe/src/package/complete_alias_package.rs | 4 +- crates/shirabe/src/package/complete_package.rs | 12 ++- .../src/package/complete_package_interface.rs | 4 +- crates/shirabe/src/package/dumper/array_dumper.rs | 8 +- crates/shirabe/src/package/handle.rs | 11 ++- .../src/package/loader/root_package_loader.rs | 100 ++++++++------------- crates/shirabe/src/package/root_alias_package.rs | 4 +- crates/shirabe/src/package/root_package.rs | 8 +- 9 files changed, 69 insertions(+), 87 deletions(-) (limited to 'crates/shirabe') diff --git a/crates/shirabe/src/factory.rs b/crates/shirabe/src/factory.rs index ce45306..24ac2e0 100644 --- a/crates/shirabe/src/factory.rs +++ b/crates/shirabe/src/factory.rs @@ -673,10 +673,7 @@ impl Factory { io.clone(), ); let package = loader.load( - local_config_data - .iter() - .map(|(k, v)| (k.clone(), Box::new(v.clone()))) - .collect(), + local_config_data.clone(), "Composer\\Package\\RootPackage", Some(&cwd), )?; diff --git a/crates/shirabe/src/package/complete_alias_package.rs b/crates/shirabe/src/package/complete_alias_package.rs index 7720258..9e1f1f6 100644 --- a/crates/shirabe/src/package/complete_alias_package.rs +++ b/crates/shirabe/src/package/complete_alias_package.rs @@ -60,11 +60,11 @@ impl CompletePackageInterface for CompleteAliasPackage { self.alias_of.set_scripts(scripts); } - fn get_repositories(&self) -> Vec> { + fn get_repositories(&self) -> IndexMap { self.alias_of.get_repositories() } - fn set_repositories(&mut self, repositories: Vec>) { + fn set_repositories(&mut self, repositories: IndexMap) { self.alias_of.set_repositories(repositories); } diff --git a/crates/shirabe/src/package/complete_package.rs b/crates/shirabe/src/package/complete_package.rs index f8293a5..033db9b 100644 --- a/crates/shirabe/src/package/complete_package.rs +++ b/crates/shirabe/src/package/complete_package.rs @@ -11,7 +11,7 @@ use shirabe_php_shim::PhpMixed; #[derive(Debug, Clone)] pub struct CompletePackage { pub(crate) inner: Package, - pub(crate) repositories: Vec>, + pub(crate) repositories: IndexMap, pub(crate) license: Vec, pub(crate) keywords: Vec, pub(crate) authors: Vec>, @@ -29,7 +29,7 @@ impl CompletePackage { pub fn new(name: String, version: String, pretty_version: String) -> Self { Self { inner: crate::package::Package::new(name, version, pretty_version), - repositories: Vec::new(), + repositories: IndexMap::new(), license: Vec::new(), keywords: Vec::new(), authors: Vec::new(), @@ -43,6 +43,10 @@ impl CompletePackage { archive_excludes: Vec::new(), } } + + pub fn replace_version(&mut self, version: String, pretty_version: String) { + self.inner.replace_version(version, pretty_version); + } } impl CompletePackageInterface for CompletePackage { @@ -54,11 +58,11 @@ impl CompletePackageInterface for CompletePackage { self.scripts.clone() } - fn set_repositories(&mut self, repositories: Vec>) { + fn set_repositories(&mut self, repositories: IndexMap) { self.repositories = repositories; } - fn get_repositories(&self) -> Vec> { + fn get_repositories(&self) -> IndexMap { self.repositories.clone() } diff --git a/crates/shirabe/src/package/complete_package_interface.rs b/crates/shirabe/src/package/complete_package_interface.rs index e638021..549aeb4 100644 --- a/crates/shirabe/src/package/complete_package_interface.rs +++ b/crates/shirabe/src/package/complete_package_interface.rs @@ -10,9 +10,9 @@ pub trait CompletePackageInterface: PackageInterface { fn set_scripts(&mut self, scripts: IndexMap>); - fn get_repositories(&self) -> Vec>; + fn get_repositories(&self) -> IndexMap; - fn set_repositories(&mut self, repositories: Vec>); + fn set_repositories(&mut self, repositories: IndexMap); fn get_license(&self) -> Vec; diff --git a/crates/shirabe/src/package/dumper/array_dumper.rs b/crates/shirabe/src/package/dumper/array_dumper.rs index 3523d58..6a693bd 100644 --- a/crates/shirabe/src/package/dumper/array_dumper.rs +++ b/crates/shirabe/src/package/dumper/array_dumper.rs @@ -365,14 +365,10 @@ impl ArrayDumper { if !repositories.is_empty() { data.insert( "repositories".to_string(), - PhpMixed::List( + PhpMixed::Array( repositories .into_iter() - .map(|r| { - Box::new(PhpMixed::Array( - r.into_iter().map(|(k, v)| (k, Box::new(v))).collect(), - )) - }) + .map(|(k, v)| (k, Box::new(v))) .collect(), ), ); diff --git a/crates/shirabe/src/package/handle.rs b/crates/shirabe/src/package/handle.rs index ab1bd0f..2e723b3 100644 --- a/crates/shirabe/src/package/handle.rs +++ b/crates/shirabe/src/package/handle.rs @@ -726,7 +726,7 @@ macro_rules! impl_complete_package_interface_handle { pub fn get_repositories( &self, - ) -> Vec> { + ) -> indexmap::IndexMap { self.0 .borrow() .as_complete_package_interface() @@ -736,7 +736,7 @@ macro_rules! impl_complete_package_interface_handle { pub fn set_repositories( &self, - repositories: Vec>, + repositories: indexmap::IndexMap, ) { self.0 .borrow_mut() @@ -1387,6 +1387,13 @@ impl RootPackageHandle { pub fn new(name: String, version: String, pretty_version: String) -> Self { Self::from_root_package(RootPackage::new(name, version, pretty_version)) } + + pub fn replace_version(&self, version: String, pretty_version: String) { + match &mut *self.0.borrow_mut() { + AnyPackage::RootPackage(p) => p.replace_version(version, pretty_version), + _ => unreachable!("RootPackageHandle invariant"), + } + } } impl AliasPackageHandle { diff --git a/crates/shirabe/src/package/loader/root_package_loader.rs b/crates/shirabe/src/package/loader/root_package_loader.rs index b31525f..04ae470 100644 --- a/crates/shirabe/src/package/loader/root_package_loader.rs +++ b/crates/shirabe/src/package/loader/root_package_loader.rs @@ -3,7 +3,7 @@ use indexmap::IndexMap; use shirabe_external_packages::composer::pcre::{CaptureKey, Preg}; use shirabe_php_shim::{ - LogicException, RuntimeException, UnexpectedValueException, strtolower, ucfirst, + LogicException, PhpMixed, RuntimeException, UnexpectedValueException, strtolower, ucfirst, }; use crate::config::Config; @@ -17,7 +17,7 @@ use crate::package::loader::LoaderInterface; use crate::package::loader::ValidatingArrayLoader; use crate::package::version::VersionGuesser; use crate::package::version::VersionParser; -use crate::package::{BasePackage, STABILITIES, SUPPORTED_LINK_TYPES}; +use crate::package::{BasePackage, RootPackage, STABILITIES, SUPPORTED_LINK_TYPES}; use crate::repository::RepositoryFactory; use crate::repository::RepositoryManager; use crate::util::Platform; @@ -62,7 +62,7 @@ impl RootPackageLoader { pub fn load( &mut self, - config: IndexMap>, + config: IndexMap, class: &str, cwd: Option<&str>, ) -> anyhow::Result { @@ -76,10 +76,7 @@ impl RootPackageLoader { let mut config = config; if !config.contains_key("name") { - config.insert( - "name".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String("__root__".to_string())), - ); + config.insert("name".to_string(), PhpMixed::String("__root__".to_string())); } else if let Some(err) = ValidatingArrayLoader::has_package_naming_error( config["name"].as_string().unwrap_or(""), false, @@ -96,32 +93,20 @@ impl RootPackageLoader { if Platform::get_env("COMPOSER_ROOT_VERSION").is_some() { let version = self.version_guesser.get_root_version_from_env()?; - config.insert( - "version".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String(version)), - ); + config.insert("version".to_string(), PhpMixed::String(version)); } else { let cwd_str = cwd .map(|s| s.to_string()) .unwrap_or_else(|| Platform::get_cwd(true).unwrap_or_default()); - // TODO(phase-b): config here is IndexMap> but guess_version - // expects IndexMap; pass an empty map as placeholder. - let unboxed_config: IndexMap = IndexMap::new(); - let version_data = self - .version_guesser - .guess_version(&unboxed_config, &cwd_str)?; + let version_data = self.version_guesser.guess_version(&config, &cwd_str)?; if let Some(data) = version_data { config.insert( "version".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String( - data.pretty_version.clone().unwrap_or_default(), - )), + PhpMixed::String(data.pretty_version.clone().unwrap_or_default()), ); config.insert( "version_normalized".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String( - data.version.clone().unwrap_or_default(), - )), + PhpMixed::String(data.version.clone().unwrap_or_default()), ); commit = data.commit; } @@ -138,10 +123,7 @@ impl RootPackageLoader { ), &[]); } } - config.insert( - "version".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String("1.0.0".to_string())), - ); + config.insert("version".to_string(), PhpMixed::String("1.0.0".to_string())); auto_versioned = true; } @@ -149,46 +131,31 @@ impl RootPackageLoader { let mut source = IndexMap::new(); source.insert( "type".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String(String::new())), - ); - source.insert( - "url".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String(String::new())), + Box::new(PhpMixed::String(String::new())), ); + source.insert("url".to_string(), Box::new(PhpMixed::String(String::new()))); source.insert( "reference".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String(commit_hash.clone())), - ); - config.insert( - "source".to_string(), - Box::new(shirabe_php_shim::PhpMixed::Array(source)), + Box::new(PhpMixed::String(commit_hash.clone())), ); + config.insert("source".to_string(), PhpMixed::Array(source)); let mut dist = IndexMap::new(); dist.insert( "type".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String(String::new())), - ); - dist.insert( - "url".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String(String::new())), + Box::new(PhpMixed::String(String::new())), ); + dist.insert("url".to_string(), Box::new(PhpMixed::String(String::new()))); dist.insert( "reference".to_string(), - Box::new(shirabe_php_shim::PhpMixed::String(commit_hash)), - ); - config.insert( - "dist".to_string(), - Box::new(shirabe_php_shim::PhpMixed::Array(dist)), + Box::new(PhpMixed::String(commit_hash)), ); + config.insert("dist".to_string(), PhpMixed::Array(dist)); } } - // TODO(phase-b): config is IndexMap> but LoaderInterface::load - // expects IndexMap; pass empty placeholder. - let unboxed_config: IndexMap = IndexMap::new(); - let mut package = self.inner.load( - unboxed_config, + let package = self.inner.load( + config.clone(), Some("Composer\\Package\\RootPackage".to_string()), )?; @@ -201,8 +168,10 @@ impl RootPackageLoader { }; if auto_versioned { - // TODO(phase-b): replace_version is an inherent method on Package, not exposed via trait. - todo!("replace_version is not accessible through RootPackage's embedded Package"); + real_package.replace_version( + real_package.get_version(), + RootPackage::DEFAULT_PRETTY_VERSION.to_string(), + ); } if let Some(min_stability) = config.get("minimum-stability").and_then(|v| v.as_string()) { @@ -217,11 +186,19 @@ impl RootPackageLoader { for link_type in ["require", "require-dev"] { if config.contains_key(link_type) { - let link_info = &SUPPORTED_LINK_TYPES[link_type]; - let _method = format!("get_{}", link_info.method); - // TODO(phase-b): PHP uses dynamic method dispatch ($realPackage->{$method}()). - // We need a Rust-side equivalent (e.g. a match on link_type) to collect Links. - let links: IndexMap = IndexMap::new(); + // PHP dynamic dispatch: $realPackage->{'get'.ucfirst($linkInfo['method'])}() + let parsed_links = match link_type { + "require" => real_package.get_requires(), + "require-dev" => real_package.get_dev_requires(), + _ => unreachable!(), + }; + let mut links: IndexMap = IndexMap::new(); + for link in parsed_links.values() { + links.insert( + link.get_target().to_string(), + link.get_constraint().get_pretty_string(), + ); + } aliases = self.extract_aliases(&links, aliases); stability_flags = Self::extract_stability_flags( &links, @@ -285,10 +262,7 @@ impl RootPackageLoader { for (_, repo) in repos { self.manager.borrow_mut().add_repository(repo); } - // TODO(phase-b): Config::get_repositories returns IndexMap, but - // set_repositories expects Vec>; pass empty placeholder. - real_package.set_repositories(Vec::new()); - let _ = self.config.borrow().get_repositories(); + real_package.set_repositories(self.config.borrow().get_repositories()); Ok(package) } diff --git a/crates/shirabe/src/package/root_alias_package.rs b/crates/shirabe/src/package/root_alias_package.rs index de4733a..d16cfc9 100644 --- a/crates/shirabe/src/package/root_alias_package.rs +++ b/crates/shirabe/src/package/root_alias_package.rs @@ -160,11 +160,11 @@ impl CompletePackageInterface for RootAliasPackage { self.inner.set_scripts(scripts); } - fn get_repositories(&self) -> Vec> { + fn get_repositories(&self) -> IndexMap { self.inner.get_repositories() } - fn set_repositories(&mut self, repositories: Vec>) { + fn set_repositories(&mut self, repositories: IndexMap) { self.inner.set_repositories(repositories); } diff --git a/crates/shirabe/src/package/root_package.rs b/crates/shirabe/src/package/root_package.rs index 0fd50f1..fc04d25 100644 --- a/crates/shirabe/src/package/root_package.rs +++ b/crates/shirabe/src/package/root_package.rs @@ -39,6 +39,10 @@ impl RootPackage { aliases: Vec::new(), } } + + pub fn replace_version(&mut self, version: String, pretty_version: String) { + self.inner.replace_version(version, pretty_version); + } } impl RootPackageInterface for RootPackage { @@ -136,11 +140,11 @@ impl CompletePackageInterface for RootPackage { CompletePackageInterface::set_scripts(&mut self.inner, scripts) } - fn get_repositories(&self) -> Vec> { + fn get_repositories(&self) -> IndexMap { CompletePackageInterface::get_repositories(&self.inner) } - fn set_repositories(&mut self, repositories: Vec>) { + fn set_repositories(&mut self, repositories: IndexMap) { CompletePackageInterface::set_repositories(&mut self.inner, repositories) } -- cgit v1.3.1