From 27764ef4447a02e5c59bbcc7b4547838aae82d89 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Wed, 3 Jun 2026 21:05:15 +0900 Subject: refactor(downloader,repository,json): drop stale try/catch TODOs; fix svn getFileContent catch type Remove 17 TODO(phase-b) markers across exception-modeling sites where the existing single-Result closure+match already faithfully models the PHP try/catch (catch paths only rethrow, retry, log, or silently ignore and never yield a recoverable value, so a double Result is unnecessary). Fix SvnDriver::get_file_content to catch only RuntimeException (matching PHP's catch RuntimeException) and extract its .message instead of calling e.to_string() on the anyhow::Error, which would panic while the shim exception Display is unimplemented; other errors now propagate. Co-Authored-By: Claude Opus 4.8 --- crates/shirabe/src/command/package_discovery_trait.rs | 1 - crates/shirabe/src/downloader/download_manager.rs | 2 -- crates/shirabe/src/downloader/vcs_downloader.rs | 3 --- crates/shirabe/src/json/json_file.rs | 2 -- crates/shirabe/src/package/version/version_guesser.rs | 2 -- crates/shirabe/src/repository/filesystem_repository.rs | 1 - crates/shirabe/src/repository/repository_set.rs | 1 - crates/shirabe/src/repository/vcs/svn_driver.rs | 8 ++++---- crates/shirabe/src/util/forgejo.rs | 3 --- crates/shirabe/src/util/platform.rs | 1 - 10 files changed, 4 insertions(+), 20 deletions(-) (limited to 'crates/shirabe') diff --git a/crates/shirabe/src/command/package_discovery_trait.rs b/crates/shirabe/src/command/package_discovery_trait.rs index 939982a..7119b7f 100644 --- a/crates/shirabe/src/command/package_discovery_trait.rs +++ b/crates/shirabe/src/command/package_discovery_trait.rs @@ -803,7 +803,6 @@ pub trait PackageDiscoveryTrait { /// @return array fn find_similar(&mut self, package: &str) -> Result> { - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let results: Vec = match (|| -> Result> { if self.get_repos_mut().is_none() { return Err(LogicException { diff --git a/crates/shirabe/src/downloader/download_manager.rs b/crates/shirabe/src/downloader/download_manager.rs index 8131bcf..b230ae3 100644 --- a/crates/shirabe/src/downloader/download_manager.rs +++ b/crates/shirabe/src/downloader/download_manager.rs @@ -238,7 +238,6 @@ impl DownloadManager { } }; - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let result = match downloader .download3(package.clone(), &target_dir, prev_package.clone()) .await @@ -363,7 +362,6 @@ impl DownloadManager { let initial_type = self.get_downloader_type(initial_downloader.unwrap()); let target_type = self.get_downloader_type(downloader.unwrap()); if initial_type == target_type { - // TODO(phase-b): use anyhow::Result> to model PHP try/catch match downloader .unwrap() .update(initial.clone(), target.clone(), &target_dir) diff --git a/crates/shirabe/src/downloader/vcs_downloader.rs b/crates/shirabe/src/downloader/vcs_downloader.rs index c3a1611..fadfa98 100644 --- a/crates/shirabe/src/downloader/vcs_downloader.rs +++ b/crates/shirabe/src/downloader/vcs_downloader.rs @@ -142,7 +142,6 @@ pub trait VcsDownloader: let mut urls = self.prepare_urls(package.get_source_urls()); while let Some(url) = array_shift(&mut urls) { - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let attempt: Result> = self .do_download(package.clone(), path, &url, prev_package.clone()) .await; @@ -260,7 +259,6 @@ pub trait VcsDownloader: let mut urls = self.prepare_urls(package.get_source_urls()); while let Some(url) = array_shift(&mut urls) { - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let attempt: Result> = self.do_install(package.clone(), path, &url).await; match attempt { @@ -336,7 +334,6 @@ pub trait VcsDownloader: let mut exception: Option = None; while let Some(url) = array_shift(&mut urls) { - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let attempt: Result> = self .do_update(initial.clone(), target.clone(), path, &url) .await; diff --git a/crates/shirabe/src/json/json_file.rs b/crates/shirabe/src/json/json_file.rs index 364c2b9..33496df 100644 --- a/crates/shirabe/src/json/json_file.rs +++ b/crates/shirabe/src/json/json_file.rs @@ -102,7 +102,6 @@ impl JsonFile { /// @throws \RuntimeException /// @return mixed pub fn read(&mut self) -> Result { - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let json: Option = match (|| -> Result> { if let Some(http_downloader) = &self.http_downloader { Ok(http_downloader @@ -216,7 +215,6 @@ impl JsonFile { let mut retries = 3; while retries > 0 { retries -= 1; - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let attempt: Result<()> = (|| -> Result<()> { self.file_put_contents_if_modified( &self.path, diff --git a/crates/shirabe/src/package/version/version_guesser.rs b/crates/shirabe/src/package/version/version_guesser.rs index c7ffd65..20d9974 100644 --- a/crates/shirabe/src/package/version/version_guesser.rs +++ b/crates/shirabe/src/package/version/version_guesser.rs @@ -362,7 +362,6 @@ impl VersionGuesser { &mut output, Some(path.to_string()), ) { - // TODO(phase-b): use anyhow::Result> to model PHP try/catch match self.version_parser.normalize(&trim(&output, None), None) { Ok(version) => return Ok(Some((version, trim(&output, None)))), Err(_e) => {} @@ -639,7 +638,6 @@ impl VersionGuesser { &mut output, Some(path.to_string()), ) { - // TODO(phase-b): use anyhow::Result> to model PHP try/catch match self.version_parser.normalize(&trim(&output, None), None) { Ok(v) => { version = Some(v); diff --git a/crates/shirabe/src/repository/filesystem_repository.rs b/crates/shirabe/src/repository/filesystem_repository.rs index a721b4f..db9a8c6 100644 --- a/crates/shirabe/src/repository/filesystem_repository.rs +++ b/crates/shirabe/src/repository/filesystem_repository.rs @@ -94,7 +94,6 @@ impl FilesystemRepository { return Ok(()); } - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let packages: PhpMixed = match (|| -> Result { let data = self.file.read()?; let packages_value = if let PhpMixed::Array(ref m) = data { diff --git a/crates/shirabe/src/repository/repository_set.rs b/crates/shirabe/src/repository/repository_set.rs index 1323fbd..077c3f6 100644 --- a/crates/shirabe/src/repository/repository_set.rs +++ b/crates/shirabe/src/repository/repository_set.rs @@ -374,7 +374,6 @@ impl RepositorySet { ) -> Result>> { let mut repo_advisories: Vec>> = vec![]; for repository in &self.repositories { - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let attempt: Result<()> = (|| -> Result<()> { let repo_ref = repository.borrow(); let Some(advisory_repo) = repo_ref.as_advisory_provider() else { diff --git a/crates/shirabe/src/repository/vcs/svn_driver.rs b/crates/shirabe/src/repository/vcs/svn_driver.rs index 2812c8d..23ea703 100644 --- a/crates/shirabe/src/repository/vcs/svn_driver.rs +++ b/crates/shirabe/src/repository/vcs/svn_driver.rs @@ -171,7 +171,6 @@ impl SvnDriver { } } - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let base_result = self.get_file_content("composer.json", identifier) .and_then(|file_content| { @@ -259,14 +258,16 @@ impl SvnDriver { (identifier.clone(), String::new()) }; - // TODO(phase-b): use anyhow::Result> to model PHP try/catch let output: String = match self.execute( vec!["svn".to_string(), "cat".to_string()], &format!("{}{}{}", self.base_url, path, rev), ) { Ok(o) => o, Err(e) => { - return Err(TransportException::new(e.to_string(), 0).into()); + if let Some(e) = e.downcast_ref::() { + return Err(TransportException::new(e.message.clone(), 0).into()); + } + return Err(e); } }; if trim(&output, None) == "" { @@ -557,7 +558,6 @@ impl SvnDriver { .set_cache_credentials(self.cache_credentials); } - // TODO(phase-b): use anyhow::Result> to model PHP try/catch match self .util .as_mut() diff --git a/crates/shirabe/src/util/forgejo.rs b/crates/shirabe/src/util/forgejo.rs index 4db5815..232c8f3 100644 --- a/crates/shirabe/src/util/forgejo.rs +++ b/crates/shirabe/src/util/forgejo.rs @@ -122,8 +122,6 @@ impl Forgejo { ) { Ok(_) => {} Err(e) => { - // TODO(phase-b): anyhow::Error has no get_code(); HTTP status codes come from - // TransportException::get_status_code(). let code = e .downcast_ref::() .and_then(|te| te.get_status_code()) @@ -140,7 +138,6 @@ impl Forgejo { return Ok(Ok(false)); } - // TODO(phase-b): downcast anyhow::Error to TransportException for the inner Err return Err(e); } } diff --git a/crates/shirabe/src/util/platform.rs b/crates/shirabe/src/util/platform.rs index ecc362c..bb5ce1b 100644 --- a/crates/shirabe/src/util/platform.rs +++ b/crates/shirabe/src/util/platform.rs @@ -399,7 +399,6 @@ impl Platform { if php_os_family() == "Linux" { let mut process = ProcessExecutor::new(None); - // TODO(phase-b): inner Result for catch(\Exception); use anyhow::Result> let mut output = String::new(); let result: Result<()> = (|| { if process.execute_args(&["lsmod".to_string()], &mut output, ()) == 0 -- cgit v1.3.1