diff options
40 files changed, 179 insertions, 123 deletions
diff --git a/crates/shirabe/src/autoload/autoload_generator.rs b/crates/shirabe/src/autoload/autoload_generator.rs index bb8fcd0b..70740a59 100644 --- a/crates/shirabe/src/autoload/autoload_generator.rs +++ b/crates/shirabe/src/autoload/autoload_generator.rs @@ -123,7 +123,7 @@ impl AutoloadGenerator { // we assume no-dev mode if no vendor dir is present or it is too old to contain dev information self.dev_mode = Some(false); - let mut installed_json = JsonFile::new( + let installed_json = JsonFile::new( format!( "{}/composer/installed.json", config.get("vendor-dir").as_string().unwrap_or("") @@ -608,7 +608,7 @@ impl AutoloadGenerator { platform_check_content = self.get_platform_check( &package_map, config.get("platform-check"), - dev_package_names, + &dev_package_names, ); if platform_check_content.is_none() { check_platform = false; diff --git a/crates/shirabe/src/command/bump_command.rs b/crates/shirabe/src/command/bump_command.rs index 78b1f627..a4518b0b 100644 --- a/crates/shirabe/src/command/bump_command.rs +++ b/crates/shirabe/src/command/bump_command.rs @@ -72,7 +72,7 @@ impl BumpCommand { return Ok(Self::ERROR_GENERIC); } - let mut composer_json = JsonFile::new(composer_json_path.clone(), None, None)?; + let composer_json = JsonFile::new(composer_json_path.clone(), None, None)?; let contents = match file_get_contents(composer_json.get_path()) { Some(c) => c, None => { diff --git a/crates/shirabe/src/command/config_command.rs b/crates/shirabe/src/command/config_command.rs index 7e8b82e9..129b6614 100644 --- a/crates/shirabe/src/command/config_command.rs +++ b/crates/shirabe/src/command/config_command.rs @@ -1569,7 +1569,7 @@ impl ConfigCommand { let mut config = Factory::create_config(None, None)?; // load configuration - let mut config_file = JsonFile::new( + let config_file = JsonFile::new( this.get_composer_config_file(input_handle.clone(), &config)?, None, None, @@ -1581,7 +1581,7 @@ impl ConfigCommand { } // load auth-configuration - let mut auth_config_file = JsonFile::new( + let auth_config_file = JsonFile::new( this.get_auth_config_file(input_handle.clone(), &config)?, None, None, diff --git a/crates/shirabe/src/command/package_discovery_trait.rs b/crates/shirabe/src/command/package_discovery_trait.rs index 229d84e9..cfd2d302 100644 --- a/crates/shirabe/src/command/package_discovery_trait.rs +++ b/crates/shirabe/src/command/package_discovery_trait.rs @@ -589,10 +589,12 @@ pub trait PackageDiscoveryTrait: BaseCommand { all_repos_package .get_repository() .map(|r| r.get_repo_name()) + .transpose()? .unwrap_or_default(), package .get_repository() .map(|r| r.get_repo_name()) + .transpose()? .unwrap_or_default(), ), code: 0, diff --git a/crates/shirabe/src/command/remove_command.rs b/crates/shirabe/src/command/remove_command.rs index e611542f..77deaf14 100644 --- a/crates/shirabe/src/command/remove_command.rs +++ b/crates/shirabe/src/command/remove_command.rs @@ -285,7 +285,7 @@ impl Command for RemoveCommand { let file = Factory::get_composer_file()?; - let mut json_file = JsonFile::new(file.clone(), None, None)?; + let json_file = JsonFile::new(file.clone(), None, None)?; let composer_data = json_file.read()?; let composer_backup = std::fs::read_to_string(json_file.get_path())?; diff --git a/crates/shirabe/src/command/repository_command.rs b/crates/shirabe/src/command/repository_command.rs index d23e114c..ab2cf2d4 100644 --- a/crates/shirabe/src/command/repository_command.rs +++ b/crates/shirabe/src/command/repository_command.rs @@ -165,7 +165,7 @@ impl RepositoryCommand { >, > = std::rc::Rc::new(std::cell::RefCell::new(input.clone())); let config = crate::factory::Factory::create_config(None, None)?; - let mut config_file = JsonFile::new( + let config_file = JsonFile::new( this.get_composer_config_file(input_handle, &config)?, None, None, diff --git a/crates/shirabe/src/dependency_resolver/problem.rs b/crates/shirabe/src/dependency_resolver/problem.rs index a944ff0c..40f21d19 100644 --- a/crates/shirabe/src/dependency_resolver/problem.rs +++ b/crates/shirabe/src/dependency_resolver/problem.rs @@ -907,7 +907,7 @@ impl Problem { RepositorySet::ALLOW_SHADOWED_REPOSITORIES, )?; if !all_repos_packages.is_empty() { - return Ok(Self::compute_check_for_lower_prio_repo( + return Self::compute_check_for_lower_prio_repo( pool, is_verbose, package_name, @@ -915,7 +915,7 @@ impl Problem { &all_repos_packages, "minimum-stability", constraint, - )); + ); } return Ok(( @@ -950,7 +950,7 @@ impl Problem { RepositorySet::ALLOW_SHADOWED_REPOSITORIES, )?; if !all_repos_packages.is_empty() { - return Ok(Self::compute_check_for_lower_prio_repo( + return Self::compute_check_for_lower_prio_repo( pool, is_verbose, package_name, @@ -958,7 +958,7 @@ impl Problem { &all_repos_packages, "constraint", constraint, - )); + ); } let mut suffix = String::new(); @@ -1267,7 +1267,7 @@ impl Problem { all_repos_packages: &Vec<BasePackageHandle>, reason: &str, constraint: Option<&AnyConstraint>, - ) -> (String, String) { + ) -> anyhow::Result<(String, String)> { let mut next_repo_packages: Vec<BasePackageHandle> = Vec::new(); let mut next_repo: Option<crate::repository::RepositoryInterfaceHandle> = None; for package in all_repos_packages { @@ -1289,7 +1289,7 @@ impl Problem { if !higher_repo_packages.is_empty() { let top_package = higher_repo_packages.first().unwrap(); if top_package.as_root().is_some() { - return ( + return Ok(( format!( "- Root composer.json requires {}{}, it is ", package_name, @@ -1304,11 +1304,11 @@ impl Problem { constraint, false ), - next_repo.get_repo_name(), + next_repo.get_repo_name()?, top_package.get_pretty_name(), top_package.get_pretty_version() ), - ); + )); } } @@ -1339,7 +1339,7 @@ impl Problem { } } - return ( + return Ok(( format!( "- Root composer.json requires {}{}, ", package_name, @@ -1359,10 +1359,10 @@ impl Problem { if singular { "is" } else { "are" }, suggestion ), - ); + )); } - ( + Ok(( format!( "- Root composer.json requires {}{}, it is ", package_name, @@ -1377,7 +1377,7 @@ impl Problem { constraint, false ), - next_repo.get_repo_name(), + next_repo.get_repo_name()?, Self::get_package_list( higher_repo_packages, is_verbose, @@ -1389,10 +1389,11 @@ impl Problem { .first() .and_then(|p| p.get_repository()) .map(|r| r.get_repo_name()) + .transpose()? .unwrap_or_default(), reason ), - ) + )) } /// Turns a constraint into text usable in a sentence describing a request diff --git a/crates/shirabe/src/factory.rs b/crates/shirabe/src/factory.rs index b5f96cc3..29dc3e05 100644 --- a/crates/shirabe/src/factory.rs +++ b/crates/shirabe/src/factory.rs @@ -261,7 +261,7 @@ impl Factory { // load global config let global_config_path = format!("{}/config.json", config.get_str("home")?); - let mut file = JsonFile::new(global_config_path, None, io.clone())?; + let file = JsonFile::new(global_config_path, None, io.clone())?; if file.exists() { if let Some(io_ref) = &io { io_ref.write_error3( @@ -316,7 +316,7 @@ impl Factory { // load global auth file let auth_file_path = format!("{}/auth.json", config.get_str("home")?); - let mut auth_file = JsonFile::new(auth_file_path, None, io.clone())?; + let auth_file = JsonFile::new(auth_file_path, None, io.clone())?; if auth_file.exists() { if let Some(io_ref) = &io { io_ref.write_error3( @@ -460,7 +460,7 @@ impl Factory { if let Some(LocalConfigInput::Path(path)) = &local_config { composer_file = Some(path.clone()); - let mut file = JsonFile::new(path.clone(), None, Some(io.clone()))?; + let file = JsonFile::new(path.clone(), None, Some(io.clone()))?; if !file.exists() { let message = if path == "./composer.json" || path == "composer.json" { @@ -535,7 +535,7 @@ impl Factory { false, ))); - let mut local_auth_file = JsonFile::new( + let local_auth_file = JsonFile::new( format!( "{}/auth.json", dirname(&realpath(composer_file_path).unwrap_or_default()) diff --git a/crates/shirabe/src/installer.rs b/crates/shirabe/src/installer.rs index 93bee0bf..d46d693d 100644 --- a/crates/shirabe/src/installer.rs +++ b/crates/shirabe/src/installer.rs @@ -873,7 +873,7 @@ impl Installer { { let operation_pkg = operation.get_target_package(); if let Some(repo) = operation_pkg.get_repository() { - source_repo = format!(" from {}", repo.get_repo_name()); + source_repo = format!(" from {}", repo.get_repo_name()?); } } self.io diff --git a/crates/shirabe/src/json/json_file.rs b/crates/shirabe/src/json/json_file.rs index 632519a7..e37ba10c 100644 --- a/crates/shirabe/src/json/json_file.rs +++ b/crates/shirabe/src/json/json_file.rs @@ -72,7 +72,9 @@ pub struct JsonFile { /// @var ?IOInterface io: Option<std::rc::Rc<std::cell::RefCell<dyn IOInterface>>>, /// @var string - indent: String, + // RefCell so that read() can stay `&self`: PHP's late-bound repository initialize() + // chains (getRepoName -> count -> initialize -> file read) run from shared contexts. + indent: std::cell::RefCell<String>, } impl JsonFile { @@ -116,7 +118,7 @@ impl JsonFile { path, http_downloader, io, - indent: Self::INDENT_DEFAULT.to_string(), + indent: std::cell::RefCell::new(Self::INDENT_DEFAULT.to_string()), }) } @@ -134,7 +136,7 @@ impl JsonFile { /// @throws ParsingException /// @throws \RuntimeException /// @return mixed - pub fn read(&mut self) -> anyhow::Result<PhpMixed> { + pub fn read(&self) -> anyhow::Result<PhpMixed> { let json: Option<String> = match (|| -> anyhow::Result<Option<String>> { if let Some(http_downloader) = &self.http_downloader { Ok(http_downloader @@ -198,7 +200,7 @@ impl JsonFile { } }; - self.indent = Self::detect_indenting(Some(&json)); + *self.indent.borrow_mut() = Self::detect_indenting(Some(&json)); Self::parse_json(Some(&json), Some(&self.path)) } @@ -213,7 +215,7 @@ impl JsonFile { options: JsonEncodeOptions, ) -> anyhow::Result<()> { let options = JsonEncodeOptions { - indent: self.indent.clone(), + indent: self.indent.borrow().clone(), ..options }; diff --git a/crates/shirabe/src/package/archiver/archive_manager.rs b/crates/shirabe/src/package/archiver/archive_manager.rs index 92fe9d6c..afc89f32 100644 --- a/crates/shirabe/src/package/archiver/archive_manager.rs +++ b/crates/shirabe/src/package/archiver/archive_manager.rs @@ -182,7 +182,7 @@ impl ArchiveManager { let composer_json_path = format!("{}/composer.json", source_path); if file_exists(&composer_json_path) { - let mut json_file = JsonFile::new(composer_json_path, None, None)?; + let json_file = JsonFile::new(composer_json_path, None, None)?; let json_data = json_file.read()?; if let Some(archive) = json_data.get("archive") { if let Some(name) = archive.get("name").and_then(|v| v.as_string()) diff --git a/crates/shirabe/src/package/loader/json_loader.rs b/crates/shirabe/src/package/loader/json_loader.rs index 205028f7..c30fbe07 100644 --- a/crates/shirabe/src/package/loader/json_loader.rs +++ b/crates/shirabe/src/package/loader/json_loader.rs @@ -23,7 +23,7 @@ impl JsonLoader { pub fn load(&self, json: JsonLoaderInput) -> anyhow::Result<PackageInterfaceHandle> { let config = match json { - JsonLoaderInput::File(mut json_file) => json_file.read()?, + JsonLoaderInput::File(json_file) => json_file.read()?, JsonLoaderInput::String(ref s) if Path::new(s).exists() => { let contents = std::fs::read_to_string(s)?; JsonFile::parse_json(Some(&contents), Some(s))? diff --git a/crates/shirabe/src/repository/array_repository.rs b/crates/shirabe/src/repository/array_repository.rs index 30598b62..c05434fc 100644 --- a/crates/shirabe/src/repository/array_repository.rs +++ b/crates/shirabe/src/repository/array_repository.rs @@ -231,13 +231,13 @@ impl RepositoryInterface for ArrayRepository { Ok(self.base_count()) } - fn get_repo_name(&self) -> String { + fn get_repo_name(&self) -> anyhow::Result<String> { let count = self.base_count(); - format!( + Ok(format!( "array repo (defining {} package{})", count, if count > 1 { "s" } else { "" }, - ) + )) } fn load_packages( diff --git a/crates/shirabe/src/repository/artifact_repository.rs b/crates/shirabe/src/repository/artifact_repository.rs index f18f7f04..99f340d1 100644 --- a/crates/shirabe/src/repository/artifact_repository.rs +++ b/crates/shirabe/src/repository/artifact_repository.rs @@ -314,8 +314,8 @@ impl RepositoryInterface for ArtifactRepository { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { - ArtifactRepository::get_repo_name(self) + fn get_repo_name(&self) -> anyhow::Result<String> { + Ok(ArtifactRepository::get_repo_name(self)) } fn as_any(&self) -> &dyn std::any::Any { diff --git a/crates/shirabe/src/repository/composer_repository.rs b/crates/shirabe/src/repository/composer_repository.rs index 18de557b..c83ffcd9 100644 --- a/crates/shirabe/src/repository/composer_repository.rs +++ b/crates/shirabe/src/repository/composer_repository.rs @@ -1651,15 +1651,20 @@ impl ComposerRepository { Url::sanitize(self.get_packages_json_url()) ); for package in self.create_packages(repo_data, Some(source))? { - self.add_package(package); + self.add_package(package)?; } Ok(()) } /// Adds a new package to the repository - pub fn add_package(&mut self, package: BasePackageHandle) { + pub fn add_package(&mut self, package: BasePackageHandle) -> anyhow::Result<()> { self.configure_package_transport_options(package.clone()); - self.inner.add_package(package); + // PHP: ArrayRepository::addPackage() calls the late-bound $this->initialize(), which + // resolves to ComposerRepository::initialize (loading the root file). + if !self.inner.is_initialized() { + self.initialize()?; + } + self.inner.add_package(package) } /// Forwards the outermost handle's weak to the inner `ArrayRepository` so that packages added @@ -3425,11 +3430,25 @@ impl RepositoryInterface for ComposerRepository { self.inner.count() } + // PHP's ArrayRepository::hasPackage() builds its packageMap from the late-bound + // $this->getPackages(), which resolves to ComposerRepository::getPackages() — throwing a + // LogicException on lazy/provider repos and loading everything on available-packages repos. fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { - if !self.inner.is_initialized() { - self.initialize()?; + if self.inner.package_map.borrow().is_none() { + let mut map: IndexMap<String, BasePackageHandle> = IndexMap::new(); + for repo_package in ComposerRepository::get_packages(self)? { + map.insert(repo_package.get_unique_name(), repo_package); + } + *self.inner.package_map.borrow_mut() = Some(map); } - self.inner.has_package(package) + + Ok(self + .inner + .package_map + .borrow() + .as_ref() + .unwrap() + .contains_key(&package.get_unique_name())) } /// @inheritDoc @@ -3671,8 +3690,8 @@ impl RepositoryInterface for ComposerRepository { .collect()) } - fn get_repo_name(&self) -> String { - ComposerRepository::get_repo_name(self) + fn get_repo_name(&self) -> anyhow::Result<String> { + Ok(ComposerRepository::get_repo_name(self)) } fn as_advisory_provider(&self) -> Option<&dyn crate::repository::AdvisoryProviderInterface> { diff --git a/crates/shirabe/src/repository/composite_repository.rs b/crates/shirabe/src/repository/composite_repository.rs index a9c55a0f..6973ccda 100644 --- a/crates/shirabe/src/repository/composite_repository.rs +++ b/crates/shirabe/src/repository/composite_repository.rs @@ -69,13 +69,13 @@ impl RepositoryInterface for CompositeRepository { Ok(total) } - fn get_repo_name(&self) -> String { + fn get_repo_name(&self) -> anyhow::Result<String> { let names: Vec<String> = self .repositories .iter() .map(|r| r.get_repo_name()) - .collect(); - format!("composite repo ({})", names.join(", ")) + .collect::<anyhow::Result<_>>()?; + Ok(format!("composite repo ({})", names.join(", "))) } fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { diff --git a/crates/shirabe/src/repository/filesystem_repository.rs b/crates/shirabe/src/repository/filesystem_repository.rs index 88317be2..151cf2d3 100644 --- a/crates/shirabe/src/repository/filesystem_repository.rs +++ b/crates/shirabe/src/repository/filesystem_repository.rs @@ -38,7 +38,9 @@ pub struct FilesystemRepository { /// @var Filesystem filesystem: std::rc::Rc<std::cell::RefCell<Filesystem>>, /// @var bool|null - dev_mode: Option<bool>, + // Cell so that initialize() can stay `&self` (late-bound initialization out of shared + // contexts such as getRepoName). + dev_mode: std::cell::Cell<Option<bool>>, } impl FilesystemRepository { @@ -67,24 +69,27 @@ impl FilesystemRepository { dump_versions, root_package, filesystem, - dev_mode: None, + dev_mode: std::cell::Cell::new(None), }) } /// @return bool|null true if dev requirements were installed, false if --no-dev was used, null if yet unknown pub fn get_dev_mode(&self) -> Option<bool> { - self.dev_mode + self.dev_mode.get() } pub fn set_self_handle(&self, weak: crate::repository::RepositoryInterfaceWeakHandle) { self.inner.set_self_handle(weak); } - pub fn get_repo_name(&self) -> String { + pub fn get_repo_name(&self) -> anyhow::Result<String> { + // PHP: ArrayRepository::getRepoName() counts through the late-bound $this->initialize(), + // which resolves to FilesystemRepository::initialize (reading the file). + self.ensure_initialized()?; self.inner.get_repo_name() } - fn ensure_initialized(&mut self) -> anyhow::Result<()> { + fn ensure_initialized(&self) -> anyhow::Result<()> { if !self.inner.is_initialized() { self.initialize()?; } @@ -92,7 +97,7 @@ impl FilesystemRepository { } /// Initializes repository (reads file, or remote address). - pub(crate) fn initialize(&mut self) -> anyhow::Result<()> { + pub(crate) fn initialize(&self) -> anyhow::Result<()> { self.inner.initialize(); if !self.file.exists() { @@ -124,7 +129,7 @@ impl FilesystemRepository { self.inner.set_dev_package_names(dev_names); } if let Some(dev) = m.get("dev") { - self.dev_mode = dev.as_bool(); + self.dev_mode.set(dev.as_bool()); } } @@ -203,7 +208,7 @@ impl FilesystemRepository { self.inner.set_dev_package_names(dev_package_names); } - pub fn get_dev_package_names(&self) -> &Vec<String> { + pub fn get_dev_package_names(&self) -> Vec<String> { self.inner.get_dev_package_names() } @@ -281,6 +286,7 @@ impl FilesystemRepository { &PhpMixed::List( self.inner .dev_package_names + .borrow() .iter() .map(|s| PhpMixed::String(s.clone())) .collect(), @@ -436,6 +442,7 @@ impl FilesystemRepository { let dev_packages = array_flip(&PhpMixed::List( self.inner .dev_package_names + .borrow() .iter() .map(|s| PhpMixed::String(s.clone())) .collect(), @@ -793,8 +800,8 @@ impl RepositoryInterface for FilesystemRepository { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { - self.inner.get_repo_name() + fn get_repo_name(&self) -> anyhow::Result<String> { + FilesystemRepository::get_repo_name(self) } fn as_any(&self) -> &dyn std::any::Any { diff --git a/crates/shirabe/src/repository/filter_repository.rs b/crates/shirabe/src/repository/filter_repository.rs index 32a32ba1..d7be0a9f 100644 --- a/crates/shirabe/src/repository/filter_repository.rs +++ b/crates/shirabe/src/repository/filter_repository.rs @@ -52,7 +52,7 @@ impl FilterRepository { return Err(InvalidArgumentException { message: format!( r#""only" key for repository {} should be an array"#, - repo.get_repo_name() + repo.get_repo_name()? ), code: 0, } @@ -82,7 +82,7 @@ impl FilterRepository { return Err(InvalidArgumentException { message: format!( r#""exclude" key for repository {} should be an array"#, - repo.get_repo_name() + repo.get_repo_name()? ), code: 0, } @@ -94,7 +94,7 @@ impl FilterRepository { return Err(InvalidArgumentException { message: format!( r#"Only one of "only" and "exclude" can be specified for repository {}"#, - repo.get_repo_name() + repo.get_repo_name()? ), code: 0, } @@ -109,7 +109,7 @@ impl FilterRepository { return Err(InvalidArgumentException { message: format!( r#""canonical" key for repository {} should be a boolean"#, - repo.get_repo_name() + repo.get_repo_name()? ), code: 0, } @@ -260,7 +260,7 @@ impl RepositoryInterface for FilterRepository { Ok(result) } - fn get_repo_name(&self) -> String { + fn get_repo_name(&self) -> anyhow::Result<String> { self.repo.get_repo_name() } diff --git a/crates/shirabe/src/repository/handle.rs b/crates/shirabe/src/repository/handle.rs index 497e2b91..2a7239b9 100644 --- a/crates/shirabe/src/repository/handle.rs +++ b/crates/shirabe/src/repository/handle.rs @@ -87,7 +87,7 @@ impl RepositoryInterfaceHandle { self.0.borrow_mut().count() } - pub fn get_repo_name(&self) -> String { + pub fn get_repo_name(&self) -> anyhow::Result<String> { self.0.borrow().get_repo_name() } @@ -181,7 +181,7 @@ impl RepositoryInterfaceHandle { self.0 .borrow() .as_installed_repository_interface() - .map(|r| r.get_dev_package_names().clone()) + .map(|r| r.get_dev_package_names()) .unwrap_or_default() } diff --git a/crates/shirabe/src/repository/installed_array_repository.rs b/crates/shirabe/src/repository/installed_array_repository.rs index b458ff61..c0c0fbb7 100644 --- a/crates/shirabe/src/repository/installed_array_repository.rs +++ b/crates/shirabe/src/repository/installed_array_repository.rs @@ -28,8 +28,8 @@ impl InstalledArrayRepository { }) } - pub fn get_repo_name(&self) -> String { - format!("installed {}", self.inner.get_repo_name()) + pub fn get_repo_name(&self) -> anyhow::Result<String> { + Ok(format!("installed {}", self.inner.get_repo_name()?)) } } @@ -73,7 +73,7 @@ impl WritableRepositoryInterface for InstalledArrayRepository { self.inner.set_dev_package_names(dev_package_names); } - fn get_dev_package_names(&self) -> &Vec<String> { + fn get_dev_package_names(&self) -> Vec<String> { self.inner.get_dev_package_names() } } @@ -131,8 +131,8 @@ impl RepositoryInterface for InstalledArrayRepository { ) -> anyhow::Result<IndexMap<String, ProviderInfo>> { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { - format!("installed {}", self.inner.get_repo_name()) + fn get_repo_name(&self) -> anyhow::Result<String> { + Ok(format!("installed {}", self.inner.get_repo_name()?)) } fn as_advisory_provider(&self) -> Option<&dyn AdvisoryProviderInterface> { None diff --git a/crates/shirabe/src/repository/installed_filesystem_repository.rs b/crates/shirabe/src/repository/installed_filesystem_repository.rs index 85fb514b..0bb962fd 100644 --- a/crates/shirabe/src/repository/installed_filesystem_repository.rs +++ b/crates/shirabe/src/repository/installed_filesystem_repository.rs @@ -55,8 +55,8 @@ impl InstalledFilesystemRepository { }) } - pub fn get_repo_name(&self) -> String { - format!("installed {}", self.inner.get_repo_name()) + pub fn get_repo_name(&self) -> anyhow::Result<String> { + Ok(format!("installed {}", self.inner.get_repo_name()?)) } } @@ -107,7 +107,7 @@ impl WritableRepositoryInterface for InstalledFilesystemRepository { self.inner.set_dev_package_names(dev_package_names); } - fn get_dev_package_names(&self) -> &Vec<String> { + fn get_dev_package_names(&self) -> Vec<String> { self.inner.get_dev_package_names() } } @@ -165,8 +165,8 @@ impl RepositoryInterface for InstalledFilesystemRepository { ) -> anyhow::Result<IndexMap<String, ProviderInfo>> { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { - format!("installed {}", self.inner.get_repo_name()) + fn get_repo_name(&self) -> anyhow::Result<String> { + Ok(format!("installed {}", self.inner.get_repo_name()?)) } fn as_advisory_provider(&self) -> Option<&dyn AdvisoryProviderInterface> { None diff --git a/crates/shirabe/src/repository/installed_repository.rs b/crates/shirabe/src/repository/installed_repository.rs index 08b1f36b..36ad510c 100644 --- a/crates/shirabe/src/repository/installed_repository.rs +++ b/crates/shirabe/src/repository/installed_repository.rs @@ -367,8 +367,12 @@ impl InstalledRepository { || repository.is::<RootPackageRepository>() || repository.is::<PlatformRepository>(), "An InstalledRepository can not contain a repository of type {} ({})", - repository.get_repo_name(), - repository.get_repo_name() + repository + .get_repo_name() + .expect("getRepoName failed while reporting an invalid repository"), + repository + .get_repo_name() + .expect("getRepoName failed while reporting an invalid repository") ); self.inner.add_repository(repository); @@ -380,14 +384,14 @@ impl RepositoryInterface for InstalledRepository { self.inner.count() } - fn get_repo_name(&self) -> String { + fn get_repo_name(&self) -> anyhow::Result<String> { let names: Vec<String> = self .inner .get_repositories() .iter() .map(|repo| repo.get_repo_name()) - .collect(); - format!("installed repo ({})", names.join(", ")) + .collect::<anyhow::Result<_>>()?; + Ok(format!("installed repo ({})", names.join(", "))) } fn has_package(&mut self, package: PackageInterfaceHandle) -> anyhow::Result<bool> { diff --git a/crates/shirabe/src/repository/lock_array_repository.rs b/crates/shirabe/src/repository/lock_array_repository.rs index 8b909fbc..9ae311da 100644 --- a/crates/shirabe/src/repository/lock_array_repository.rs +++ b/crates/shirabe/src/repository/lock_array_repository.rs @@ -90,8 +90,8 @@ impl RepositoryInterface for LockArrayRepository { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { - "lock repo".to_string() + fn get_repo_name(&self) -> anyhow::Result<String> { + Ok("lock repo".to_string()) } fn as_any(&self) -> &dyn std::any::Any { diff --git a/crates/shirabe/src/repository/package_repository.rs b/crates/shirabe/src/repository/package_repository.rs index 06b28451..62c8c5b3 100644 --- a/crates/shirabe/src/repository/package_repository.rs +++ b/crates/shirabe/src/repository/package_repository.rs @@ -83,13 +83,16 @@ impl PackageRepository { Ok(Ok(())) } - pub fn get_repo_name(&self) -> String { + pub fn get_repo_name(&self) -> anyhow::Result<String> { use crate::repository::RepositoryInterface; - Preg::replace( + // PHP: parent::getRepoName() counts through the late-bound $this->initialize(), + // which resolves to PackageRepository::initialize (loading the config packages). + self.ensure_initialized()?; + Ok(Preg::replace( php_regex!(r"{^array }"), "package ", - &self.inner.get_repo_name(), - ) + &self.inner.get_repo_name()?, + )) } // In PHP the inherited ArrayRepository methods lazily call the overridden initialize() to load @@ -175,7 +178,7 @@ impl RepositoryInterface for PackageRepository { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { + fn get_repo_name(&self) -> anyhow::Result<String> { PackageRepository::get_repo_name(self) } @@ -235,7 +238,7 @@ impl AdvisoryProviderInterface for PackageRepository { message: format!( "Advisory for {} could not be loaded as a full advisory from {}\n{}", package_name, - self.get_repo_name(), + self.get_repo_name()?, var_export(data, true) ), code: 0, diff --git a/crates/shirabe/src/repository/path_repository.rs b/crates/shirabe/src/repository/path_repository.rs index 879b769c..ec2da043 100644 --- a/crates/shirabe/src/repository/path_repository.rs +++ b/crates/shirabe/src/repository/path_repository.rs @@ -464,8 +464,8 @@ impl RepositoryInterface for PathRepository { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { - PathRepository::get_repo_name(self) + fn get_repo_name(&self) -> anyhow::Result<String> { + Ok(PathRepository::get_repo_name(self)) } fn as_any(&self) -> &dyn std::any::Any { diff --git a/crates/shirabe/src/repository/platform_repository.rs b/crates/shirabe/src/repository/platform_repository.rs index 30164237..f0d8b46e 100644 --- a/crates/shirabe/src/repository/platform_repository.rs +++ b/crates/shirabe/src/repository/platform_repository.rs @@ -100,14 +100,20 @@ impl PlatformRepository { }, ); } - Ok(Self { - inner: ArrayRepository::new(packages)?, + // PHP: parent::__construct($packages) runs after the overrides are set, and the parent + // ctor's $this->addPackage() calls late-bind to PlatformRepository::addPackage. + let mut this = Self { + inner: ArrayRepository::new(vec![])?, version_parser: None, overrides: overrides_map, disabled_packages: IndexMap::new(), runtime, hhvm_detector, - }) + }; + for package in packages { + this.add_package(package)?; + } + Ok(this) } pub fn get_repo_name(&self) -> String { @@ -1541,6 +1547,9 @@ impl PlatformRepository { return Ok(()); } + // PHP: $this->findPackage() reaches ArrayRepository::findPackage, whose + // getPackages() call late-binds to PlatformRepository::initialize. + self.ensure_initialized()?; let overrider = self.inner.find_package( &name, crate::repository::FindPackageConstraint::String("*".to_string()), @@ -1583,7 +1592,9 @@ impl PlatformRepository { return Ok(()); } - self.inner.add_package(package); + // PHP: parent::addPackage() calls the late-bound $this->initialize() before pushing. + self.ensure_initialized()?; + self.inner.add_package(package)?; Ok(()) } @@ -1611,6 +1622,8 @@ impl PlatformRepository { extra.insert("config.platform".to_string(), PhpMixed::Bool(true)); package.inner.set_extra(extra); let package = CompletePackageHandle::from_complete_package(package); + // PHP: parent::addPackage() calls the late-bound $this->initialize() before pushing. + self.ensure_initialized()?; self.inner.add_package(package.clone().into())?; if package.get_name() == "php" { @@ -1934,8 +1947,8 @@ impl crate::repository::RepositoryInterface for PlatformRepository { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { - PlatformRepository::get_repo_name(self) + fn get_repo_name(&self) -> anyhow::Result<String> { + Ok(PlatformRepository::get_repo_name(self)) } fn as_any(&self) -> &dyn std::any::Any { diff --git a/crates/shirabe/src/repository/repository_factory.rs b/crates/shirabe/src/repository/repository_factory.rs index 972a5161..856f58ad 100644 --- a/crates/shirabe/src/repository/repository_factory.rs +++ b/crates/shirabe/src/repository/repository_factory.rs @@ -40,7 +40,7 @@ impl RepositoryFactory { .unwrap_or(""); if extension == "json" { - let mut json = JsonFile::new( + let json = JsonFile::new( repository.to_string(), Some(std::rc::Rc::new(std::cell::RefCell::new( Factory::create_http_downloader(io.clone(), config, IndexMap::new())?, diff --git a/crates/shirabe/src/repository/repository_interface.rs b/crates/shirabe/src/repository/repository_interface.rs index ab25242c..cece0d6f 100644 --- a/crates/shirabe/src/repository/repository_interface.rs +++ b/crates/shirabe/src/repository/repository_interface.rs @@ -94,7 +94,10 @@ pub trait RepositoryInterface: std::fmt::Debug { package_name: String, ) -> anyhow::Result<IndexMap<String, ProviderInfo>>; - fn get_repo_name(&self) -> String; + // PHP's getRepoName() can throw: ArrayRepository::getRepoName() counts through the + // late-bound $this->initialize(), which is fallible in subclasses that read files + // (FilesystemRepository, PackageRepository). + fn get_repo_name(&self) -> anyhow::Result<String>; fn as_advisory_provider(&self) -> Option<&dyn AdvisoryProviderInterface> { None diff --git a/crates/shirabe/src/repository/root_package_repository.rs b/crates/shirabe/src/repository/root_package_repository.rs index 7630f6c5..d077d245 100644 --- a/crates/shirabe/src/repository/root_package_repository.rs +++ b/crates/shirabe/src/repository/root_package_repository.rs @@ -85,8 +85,8 @@ impl RepositoryInterface for RootPackageRepository { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { - RootPackageRepository::get_repo_name(self) + fn get_repo_name(&self) -> anyhow::Result<String> { + Ok(RootPackageRepository::get_repo_name(self)) } fn as_any(&self) -> &dyn std::any::Any { diff --git a/crates/shirabe/src/repository/vcs_repository.rs b/crates/shirabe/src/repository/vcs_repository.rs index 9b1b0d84..dd7b4b06 100644 --- a/crates/shirabe/src/repository/vcs_repository.rs +++ b/crates/shirabe/src/repository/vcs_repository.rs @@ -1120,8 +1120,8 @@ impl RepositoryInterface for VcsRepository { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { - VcsRepository::get_repo_name(self) + fn get_repo_name(&self) -> anyhow::Result<String> { + Ok(VcsRepository::get_repo_name(self)) } fn as_any(&self) -> &dyn std::any::Any { diff --git a/crates/shirabe/src/repository/writable_array_repository.rs b/crates/shirabe/src/repository/writable_array_repository.rs index f5db2bd9..bd685652 100644 --- a/crates/shirabe/src/repository/writable_array_repository.rs +++ b/crates/shirabe/src/repository/writable_array_repository.rs @@ -13,7 +13,9 @@ use shirabe_semver::constraint::AnyConstraint; #[derive(Debug)] pub struct WritableArrayRepository { inner: ArrayRepository, - pub(crate) dev_package_names: Vec<String>, + // RefCell so that FilesystemRepository::initialize can stay `&self` (late-bound + // initialization out of shared contexts such as getRepoName). + pub(crate) dev_package_names: std::cell::RefCell<Vec<String>>, dev_mode: Option<bool>, } @@ -21,7 +23,7 @@ impl WritableArrayRepository { pub fn new(packages: Vec<crate::package::PackageInterfaceHandle>) -> anyhow::Result<Self> { Ok(Self { inner: ArrayRepository::new(packages)?, - dev_package_names: Vec::new(), + dev_package_names: std::cell::RefCell::new(Vec::new()), dev_mode: None, }) } @@ -36,12 +38,12 @@ impl WritableArrayRepository { self.inner.base_count() } - pub fn set_dev_package_names(&mut self, dev_package_names: Vec<String>) { - self.dev_package_names = dev_package_names; + pub fn set_dev_package_names(&self, dev_package_names: Vec<String>) { + *self.dev_package_names.borrow_mut() = dev_package_names; } - pub fn get_dev_package_names(&self) -> &Vec<String> { - &self.dev_package_names + pub fn get_dev_package_names(&self) -> Vec<String> { + self.dev_package_names.borrow().clone() } pub fn write( @@ -66,7 +68,7 @@ impl WritableArrayRepository { } pub fn add_package( - &mut self, + &self, package: crate::package::PackageInterfaceHandle, ) -> anyhow::Result<()> { self.inner.add_package(package) @@ -84,7 +86,7 @@ impl WritableArrayRepository { Ok(()) } - pub fn initialize(&mut self) -> anyhow::Result<()> { + pub fn initialize(&self) -> anyhow::Result<()> { self.inner.initialize(); Ok(()) } @@ -123,8 +125,8 @@ impl WritableArrayRepository { self.inner.get_packages() } - pub fn get_repo_name(&self) -> String { - self.inner.get_repo_name() + pub fn get_repo_name(&self) -> anyhow::Result<String> { + RepositoryInterface::get_repo_name(&self.inner) } } @@ -188,7 +190,7 @@ impl RepositoryInterface for WritableArrayRepository { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { + fn get_repo_name(&self) -> anyhow::Result<String> { self.inner.get_repo_name() } diff --git a/crates/shirabe/src/repository/writable_repository_interface.rs b/crates/shirabe/src/repository/writable_repository_interface.rs index 75b21e9a..d7b35066 100644 --- a/crates/shirabe/src/repository/writable_repository_interface.rs +++ b/crates/shirabe/src/repository/writable_repository_interface.rs @@ -21,5 +21,5 @@ pub trait WritableRepositoryInterface: RepositoryInterface { fn set_dev_package_names(&mut self, dev_package_names: Vec<String>); - fn get_dev_package_names(&self) -> &Vec<String>; + fn get_dev_package_names(&self) -> Vec<String>; } diff --git a/crates/shirabe/src/util/config_validator.rs b/crates/shirabe/src/util/config_validator.rs index 626c4db1..35f14192 100644 --- a/crates/shirabe/src/util/config_validator.rs +++ b/crates/shirabe/src/util/config_validator.rs @@ -39,7 +39,7 @@ impl ConfigValidator { let mut lax_valid = false; let mut manifest: Option<IndexMap<String, PhpMixed>> = None; - let mut json = JsonFile::new(file.to_string(), None, Some(self.io.clone())) + let json = JsonFile::new(file.to_string(), None, Some(self.io.clone())) .expect("config file path is always local"); let schema_result: anyhow::Result<()> = (|| -> anyhow::Result<()> { manifest = Some(match json.read()? { diff --git a/crates/shirabe/tests/advisory/auditor_test.rs b/crates/shirabe/tests/advisory/auditor_test.rs index 65e848dd..e3b72536 100644 --- a/crates/shirabe/tests/advisory/auditor_test.rs +++ b/crates/shirabe/tests/advisory/auditor_test.rs @@ -190,8 +190,8 @@ impl RepositoryInterface for MockAdvisoryRepository { ) -> anyhow::Result<IndexMap<String, ProviderInfo>> { unimplemented!("not used by Auditor") } - fn get_repo_name(&self) -> String { - "mock advisory repo".to_string() + fn get_repo_name(&self) -> anyhow::Result<String> { + Ok("mock advisory repo".to_string()) } fn as_advisory_provider_mut(&mut self) -> Option<&mut dyn AdvisoryProviderInterface> { Some(self) diff --git a/crates/shirabe/tests/command/bump_command_test.rs b/crates/shirabe/tests/command/bump_command_test.rs index d511f2a1..14ac17a6 100644 --- a/crates/shirabe/tests/command/bump_command_test.rs +++ b/crates/shirabe/tests/command/bump_command_test.rs @@ -41,7 +41,7 @@ fn run_bump_case( let status_code = app_tester.run(input, RunOptions::default()).unwrap(); assert_eq!(exit_code, status_code); - let mut json = JsonFile::new("./composer.json".to_string(), None, None).unwrap(); + let json = JsonFile::new("./composer.json".to_string(), None, None).unwrap(); let read = json.read().unwrap(); let actual: serde_json::Value = serde_json::from_str(&JsonFile::encode(&read).unwrap()).unwrap(); diff --git a/crates/shirabe/tests/command/init_command_test.rs b/crates/shirabe/tests/command/init_command_test.rs index 57777e35..914680fb 100644 --- a/crates/shirabe/tests/command/init_command_test.rs +++ b/crates/shirabe/tests/command/init_command_test.rs @@ -23,7 +23,7 @@ fn default_authors() -> serde_json::Value { /// `serde_json::Value` so the comparison ignores object key order (matching PHPUnit's `assertEquals` /// on arrays) while staying order-sensitive for lists. fn read_composer_json(dir: &std::path::Path) -> serde_json::Value { - let mut file = JsonFile::new( + let file = JsonFile::new( dir.join("composer.json").to_string_lossy().to_string(), None, None, diff --git a/crates/shirabe/tests/command/remove_command_test.rs b/crates/shirabe/tests/command/remove_command_test.rs index 7b195746..9139bf26 100644 --- a/crates/shirabe/tests/command/remove_command_test.rs +++ b/crates/shirabe/tests/command/remove_command_test.rs @@ -26,7 +26,7 @@ fn input(pairs: Vec<(&str, PhpMixed)>) -> Vec<(PhpMixed, PhpMixed)> { /// Read a JSON file in the CWD and decode it to a `serde_json::Value`. fn read_json_file(path: &str) -> serde_json::Value { - let mut json = JsonFile::new(path.to_string(), None, None).unwrap(); + let json = JsonFile::new(path.to_string(), None, None).unwrap(); let read = json.read().unwrap(); serde_json::from_str(&JsonFile::encode(&read).unwrap()).unwrap() } diff --git a/crates/shirabe/tests/command/repository_command_test.rs b/crates/shirabe/tests/command/repository_command_test.rs index 48ca301a..36b74cc8 100644 --- a/crates/shirabe/tests/command/repository_command_test.rs +++ b/crates/shirabe/tests/command/repository_command_test.rs @@ -7,7 +7,7 @@ use shirabe_php_shim::PhpMixed; /// Read the composer.json in the CWD and decode it. fn read_composer_json() -> serde_json::Value { - let mut json = JsonFile::new("./composer.json".to_string(), None, None).unwrap(); + let json = JsonFile::new("./composer.json".to_string(), None, None).unwrap(); let read = json.read().unwrap(); serde_json::from_str(&JsonFile::encode(&read).unwrap()).unwrap() } diff --git a/crates/shirabe/tests/command/require_command_test.rs b/crates/shirabe/tests/command/require_command_test.rs index 4237c4f8..2a9a0411 100644 --- a/crates/shirabe/tests/command/require_command_test.rs +++ b/crates/shirabe/tests/command/require_command_test.rs @@ -382,7 +382,7 @@ fn test_inconsistent_require_keys() { app_tester.get_display() ); - let mut composer_content = + let composer_content = JsonFile::new(format!("{}/composer.json", dir.display()), None, None).unwrap(); let content = composer_content.read().unwrap(); let content: serde_json::Value = diff --git a/crates/shirabe/tests/json/json_file_test.rs b/crates/shirabe/tests/json/json_file_test.rs index 7aac35a9..785b4721 100644 --- a/crates/shirabe/tests/json/json_file_test.rs +++ b/crates/shirabe/tests/json/json_file_test.rs @@ -209,7 +209,7 @@ fn test_preserve_indentation_after_read() { let dst = fixture_path("tabs2.json"); std::fs::copy(&src, &dst).unwrap(); - let mut json_file = JsonFile::new(dst.to_str().unwrap().to_string(), None, None).unwrap(); + let json_file = JsonFile::new(dst.to_str().unwrap().to_string(), None, None).unwrap(); let _data = json_file.read().unwrap(); let mut hash: IndexMap<String, PhpMixed> = IndexMap::new(); hash.insert("foo".to_string(), PhpMixed::String("baz".to_string())); |