From a3171eefd8f8e520329bdd8f613e83ba6f0a7c13 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sun, 21 Jun 2026 18:49:24 +0900 Subject: refactor(php-shim): remove ArrayObject, inline IndexMap into PhpMixed::Object ArrayObject was only a thin wrapper around IndexMap used as an empty-{} vs empty-[] marker and as the assoc=false JSON object representation; no reference semantics were involved. Inline its payload directly into PhpMixed::Object and drop the type along with the now-dead StdClass. Side effects of the unification: - ArrayObject::new was todo!(); the config --global / object-typed get paths that built PhpMixed::Object(ArrayObject::new(None)) no longer panic. - base_config_command wrote 'config' as PhpMixed::Array(empty), emitting [] instead of {}; now matches PHP's new \ArrayObject ({}). - The dead is::()/is::() instanceof checks in JsonManipulator are replaced with the faithful as_object() mapping. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/shirabe-php-shim/src/array.rs | 2 +- crates/shirabe-php-shim/src/json.rs | 7 +-- crates/shirabe-php-shim/src/lib.rs | 69 ++++++----------------- crates/shirabe/src/command/base_config_command.rs | 2 +- crates/shirabe/src/command/config_command.rs | 6 +- crates/shirabe/src/json/json_manipulator.rs | 47 ++++++--------- 6 files changed, 41 insertions(+), 92 deletions(-) (limited to 'crates') diff --git a/crates/shirabe-php-shim/src/array.rs b/crates/shirabe-php-shim/src/array.rs index 4b3e160..8c1988d 100644 --- a/crates/shirabe-php-shim/src/array.rs +++ b/crates/shirabe-php-shim/src/array.rs @@ -568,7 +568,7 @@ pub fn count(value: &PhpMixed) -> usize { match value { PhpMixed::List(items) => items.len(), PhpMixed::Array(entries) => entries.len(), - PhpMixed::Object(object) => object.count(), + PhpMixed::Object(object) => object.len(), // PHP 8 throws a `TypeError` for non-countable arguments. PhpMixed::Null | PhpMixed::Bool(_) diff --git a/crates/shirabe-php-shim/src/json.rs b/crates/shirabe-php-shim/src/json.rs index 23c4c41..3d3ee5b 100644 --- a/crates/shirabe-php-shim/src/json.rs +++ b/crates/shirabe-php-shim/src/json.rs @@ -1,4 +1,3 @@ -use crate::ArrayObject; use crate::PhpMixed; use indexmap::IndexMap; @@ -58,7 +57,7 @@ pub fn json_encode_ex( // PHP's two-argument `json_decode`: without JSON_THROW_ON_ERROR it never throws, // returning null on malformed input. With `assoc` false, JSON objects decode to -// stdClass-equivalent ArrayObject values; with `assoc` true, to associative arrays. +// stdClass-equivalent `PhpMixed::Object` values; with `assoc` true, to associative arrays. pub fn json_decode(s: &str, assoc: bool) -> anyhow::Result { match serde_json::from_str::(s) { Ok(value) => Ok(json_value_to_php_mixed(value, assoc)), @@ -91,9 +90,7 @@ fn json_value_to_php_mixed(value: serde_json::Value, assoc: bool) -> PhpMixed { if assoc { PhpMixed::Array(data) } else { - PhpMixed::Object(ArrayObject { - data: data.into_iter().collect(), - }) + PhpMixed::Object(data) } } } diff --git a/crates/shirabe-php-shim/src/lib.rs b/crates/shirabe-php-shim/src/lib.rs index b42ed75..0d7f8ef 100644 --- a/crates/shirabe-php-shim/src/lib.rs +++ b/crates/shirabe-php-shim/src/lib.rs @@ -62,7 +62,8 @@ pub enum PhpMixed { String(String), List(Vec), Array(IndexMap), - Object(ArrayObject), + // TODO: consolidate Object to Array. + Object(IndexMap), } impl serde::Serialize for PhpMixed { @@ -91,7 +92,13 @@ impl serde::Serialize for PhpMixed { } map.end() } - PhpMixed::Object(object) => object.serialize(serializer), + PhpMixed::Object(entries) => { + let mut map = serializer.serialize_map(Some(entries.len()))?; + for (k, v) in entries { + map.serialize_entry(k, v)?; + } + map.end() + } } } } @@ -112,37 +119,17 @@ impl PartialEq for PhpMixed { .zip(b.iter()) .all(|((ka, va), (kb, vb))| ka == kb && va == vb) } - (PhpMixed::Object(a), PhpMixed::Object(b)) => a == b, + (PhpMixed::Object(a), PhpMixed::Object(b)) => { + a.len() == b.len() + && a.iter() + .zip(b.iter()) + .all(|((ka, va), (kb, vb))| ka == kb && va == vb) + } _ => false, } } } -impl PartialEq for ArrayObject { - fn eq(&self, other: &Self) -> bool { - self.data.len() == other.data.len() - && self - .data - .iter() - .zip(other.data.iter()) - .all(|((ka, va), (kb, vb))| ka == kb && va == vb) - } -} - -impl serde::Serialize for ArrayObject { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - use serde::ser::SerializeMap; - let mut map = serializer.serialize_map(Some(self.data.len()))?; - for (k, v) in &self.data { - map.serialize_entry(k, v)?; - } - map.end() - } -} - impl PhpMixed { pub fn as_bool(&self) -> Option { match self { @@ -200,7 +187,7 @@ impl PhpMixed { } } - pub fn as_object(&self) -> Option<&ArrayObject> { + pub fn as_object(&self) -> Option<&IndexMap> { match self { PhpMixed::Object(o) => Some(o), _ => None, @@ -351,30 +338,6 @@ impl std::fmt::Display for PhpMixed { } } -#[derive(Debug, Clone)] -pub struct ArrayObject { - data: IndexMap, -} - -impl ArrayObject { - pub fn new(_array: Option) -> Self { - todo!() - } - - pub fn to_array(&self) -> IndexMap { - self.data.clone() - } - - pub fn count(&self) -> usize { - self.data.len() - } -} - -#[derive(Debug)] -pub struct StdClass { - pub data: IndexMap, -} - #[derive(Debug, Clone)] pub enum PhpResource { Stdin, diff --git a/crates/shirabe/src/command/base_config_command.rs b/crates/shirabe/src/command/base_config_command.rs index 01952f6..4099ea5 100644 --- a/crates/shirabe/src/command/base_config_command.rs +++ b/crates/shirabe/src/command/base_config_command.rs @@ -89,7 +89,7 @@ pub trait BaseConfigCommand: BaseCommand { .borrow() .write(PhpMixed::Array({ let mut m = IndexMap::new(); - m.insert("config".to_string(), PhpMixed::Array(IndexMap::new())); + m.insert("config".to_string(), PhpMixed::Object(IndexMap::new())); m }))?; let _ = Silencer::call(|| { diff --git a/crates/shirabe/src/command/config_command.rs b/crates/shirabe/src/command/config_command.rs index 1517e57..2ee6fd2 100644 --- a/crates/shirabe/src/command/config_command.rs +++ b/crates/shirabe/src/command/config_command.rs @@ -10,7 +10,7 @@ use shirabe_external_packages::symfony::console::command::command::Command; use shirabe_external_packages::symfony::console::input::InputInterface; use shirabe_external_packages::symfony::console::output::OutputInterface; use shirabe_php_shim::{ - ArrayObject, InvalidArgumentException, JsonObject, PhpMixed, RuntimeException, array_filter, + InvalidArgumentException, JsonObject, PhpMixed, RuntimeException, array_filter, array_filter_use_key, array_is_list, array_map, array_merge, array_unique, count, escapeshellcmd, exec, explode, file_exists, file_get_contents, implode, in_array, is_array, is_bool, is_dir, is_numeric, is_object, is_string, json_encode, key, sort, sprintf, @@ -188,7 +188,7 @@ impl Command for ConfigCommand { "bearer", "forgejo-token", ] { - empty_objs.insert(k.to_string(), PhpMixed::Object(ArrayObject::new(None))); + empty_objs.insert(k.to_string(), PhpMixed::Object(IndexMap::new())); } self.auth_config_file .as_ref() @@ -487,7 +487,7 @@ impl Command for ConfigCommand { }) .unwrap_or_default(); if type_strings.iter().any(|s| s == "object") { - value = PhpMixed::Object(ArrayObject::new(None)); + value = PhpMixed::Object(IndexMap::new()); } } } diff --git a/crates/shirabe/src/json/json_manipulator.rs b/crates/shirabe/src/json/json_manipulator.rs index 904ea7a..30ceedb 100644 --- a/crates/shirabe/src/json/json_manipulator.rs +++ b/crates/shirabe/src/json/json_manipulator.rs @@ -4,10 +4,10 @@ use indexmap::IndexMap; use shirabe_external_packages::composer::pcre::{CaptureKey, Preg}; use shirabe_php_shim::{ - ArrayObject, InvalidArgumentException, LogicException, PhpMixed, StdClass, addcslashes, - array_key_exists, array_keys, array_reverse, count, empty, explode, implode, in_array, - is_array, is_int, is_numeric, json_decode, preg_quote, rtrim, str_contains, str_repeat, - str_replace, strlen, strnatcmp, strpos, substr, trim, uksort, + InvalidArgumentException, LogicException, PhpMixed, addcslashes, array_key_exists, array_keys, + array_reverse, count, empty, explode, implode, in_array, is_array, is_int, is_numeric, + json_decode, preg_quote, rtrim, str_contains, str_repeat, str_replace, strlen, strnatcmp, + strpos, substr, trim, uksort, }; use crate::json::JsonFile; @@ -254,17 +254,17 @@ impl JsonManipulator { let repositories_value: Option = decoded .as_object() - .and_then(|o| o.to_array().get("repositories").cloned()); + .and_then(|o| o.get("repositories").cloned()); let is_std_class = repositories_value .as_ref() - .map(|v| (v as &dyn std::any::Any).is::()) + .map(|v| v.as_object().is_some()) .unwrap_or(false); if is_std_class { // delete from bottom to top, to ensure keys stay the same let repos_arr: IndexMap = repositories_value .as_ref() - .and_then(|v| v.as_array().cloned()) + .and_then(|v| v.as_object().cloned()) .unwrap_or_default(); let entries_to_revert: Vec = array_reverse(&array_keys(&repos_arr), false); @@ -278,7 +278,7 @@ impl JsonManipulator { // re-add in order for (repository_name, repository) in &repos_arr { - let is_obj = (repository as &dyn std::any::Any).is::(); + let is_obj = repository.as_object().is_some(); if !is_obj { let mut m: IndexMap = IndexMap::new(); m.insert(repository_name.clone(), repository.clone()); @@ -494,15 +494,15 @@ impl JsonManipulator { let decoded = json_decode(&self.contents, false)?; let repositories_value: Option = decoded .as_object() - .and_then(|o| o.to_array().get("repositories").cloned()); + .and_then(|o| o.get("repositories").cloned()); let is_assoc = repositories_value .as_ref() - .map(|v| (v as &dyn std::any::Any).is::()) + .map(|v| v.as_object().is_some()) .unwrap_or(false); let repos: IndexMap = repositories_value .as_ref() - .and_then(|v| v.as_array().cloned()) + .and_then(|v| v.as_object().cloned()) .unwrap_or_default(); for (repository_index, repository) in &repos { @@ -516,7 +516,7 @@ impl JsonManipulator { let repo_name_owned: Option = repository .as_object() - .and_then(|o| o.to_array().get("name").cloned()) + .and_then(|o| o.get("name").cloned()) .and_then(|v| v.as_string().map(|s| s.to_string())); if Some(name) == repo_name_owned.as_deref() { if is_assoc { @@ -996,7 +996,7 @@ impl JsonManipulator { .map(|a| a.is_empty()) .unwrap_or(false); if now_empty { - arr.insert(name_owned.clone(), PhpMixed::Object(ArrayObject::new(None))); + arr.insert(name_owned.clone(), PhpMixed::Object(IndexMap::new())); } } let val = cur_val @@ -1042,10 +1042,7 @@ impl JsonManipulator { .map(|a| a.is_empty()) .unwrap_or(false); if now_empty { - arr.insert( - name_capture.clone(), - PhpMixed::Object(ArrayObject::new(None)), - ); + arr.insert(name_capture.clone(), PhpMixed::Object(IndexMap::new())); } } children_clean = formatter.format(&cur_val, 0, true).unwrap_or_default(); @@ -1565,13 +1562,9 @@ impl JsonManipulator { pub fn format(&self, data: &PhpMixed, depth: i64, was_object: bool) -> anyhow::Result { let mut data = data.clone(); let mut was_object = was_object; - if (&data as &dyn std::any::Any).is::() - || (&data as &dyn std::any::Any).is::() - { + if let Some(obj) = data.as_object() { // PHP: (array) $data — coerce to array - if let Some(obj) = data.as_object() { - data = PhpMixed::Array(obj.to_array().into_iter().collect()); - } + data = PhpMixed::Array(obj.clone()); was_object = true; } @@ -1639,12 +1632,8 @@ impl ManipulatorFormatter { fn format(&self, data: &PhpMixed, depth: i64, was_object: bool) -> anyhow::Result { let mut data = data.clone(); let mut was_object = was_object; - if (&data as &dyn std::any::Any).is::() - || (&data as &dyn std::any::Any).is::() - { - if let Some(obj) = data.as_object() { - data = PhpMixed::Array(obj.to_array().into_iter().collect()); - } + if let Some(obj) = data.as_object() { + data = PhpMixed::Array(obj.clone()); was_object = true; } -- cgit v1.3.1