From 99cd11d972661dcc257f7ecfbcd78dec4ab572e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 18:24:39 +0000 Subject: [PATCH] refactor(binary): rewrite pre-2018 ref-patterns to match ergonomics Closes #85 (Phase 2 of #1). Converts every &Variant(ref x)-style site in apicli and apid to plain match-ergonomics form -- purely syntactic, binds the identical reference type as before, with one exception handled explicitly: apicli/template.rs's Integer(key) arm bound key: &i64 via ergonomics where the original &InputTokens::Integer(key) explicit-deref pattern bound an owned (Copy) i64 -- fixed by dereferencing at the use site instead of changing the match arm. apid/main.rs's provide_input handler matches Some((_, tx)) against a &mut-sourced get_mut() call; ergonomics binds tx: &mut Sender where the original ref tx bound &Sender, but Sender::send only needs &self so this is behaviorally inert. apicli's path.rs and stub.rs account for most of this PR's sites (30 of 33) and weren't flagged by clippy::ref_patterns/match_ref_pats/ needless_borrowed_reference at all -- those lints don't reliably fire on every &Some(Variant(_))-shaped match arm mixed with `ref`-bound arms in the same match. Found instead via the original manual site inventory from #1's scoping research; worth noting since a future clippy-only search of this codebase would miss them. apicli/engine.rs's merge() function keeps its outer `match &left`/ `match &right` (left/right are owned Schema values reused later in the same arms via `one_of.push(left)`/`vec![left, right]`, so the scrutinee itself can't drop its `&` without moving out from under later use) -- only the arm patterns lost their redundant `&`/`ref`. apicli/template.rs also fixes the impl ToString for PathKey match's ref-pattern shape only, not the ToString/Display issue itself (#13). Confirmed via a full `cargo clippy --workspace --all-features`: zero ref_patterns/match_ref_pats/needless_borrowed_reference warnings remain anywhere in the workspace, closing out issue #85. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018VzKriyNnMHmqQ6UxPhsv2 --- binary/apicli/src/engine.rs | 52 +++++++++++++++++------------------ binary/apicli/src/path.rs | 30 ++++++++++---------- binary/apicli/src/stub.rs | 24 ++++++++-------- binary/apicli/src/template.rs | 35 +++++++++++------------ binary/apid/src/main.rs | 2 +- 5 files changed, 72 insertions(+), 71 deletions(-) diff --git a/binary/apicli/src/engine.rs b/binary/apicli/src/engine.rs index 25920e3..588f896 100644 --- a/binary/apicli/src/engine.rs +++ b/binary/apicli/src/engine.rs @@ -557,11 +557,11 @@ enum SchemaObject { /// nothing to merge). fn schemaify(value: &serde_json::Value) -> Schema { match value { - &serde_json::Value::Null => Schema::Single(SchemaObject::Null), - &serde_json::Value::Bool(_) => Schema::Single(SchemaObject::Boolean), - &serde_json::Value::Number(_) => Schema::Single(SchemaObject::Number), - &serde_json::Value::String(_) => Schema::Single(SchemaObject::String), - &serde_json::Value::Object(ref obj) => { + serde_json::Value::Null => Schema::Single(SchemaObject::Null), + serde_json::Value::Bool(_) => Schema::Single(SchemaObject::Boolean), + serde_json::Value::Number(_) => Schema::Single(SchemaObject::Number), + serde_json::Value::String(_) => Schema::Single(SchemaObject::String), + serde_json::Value::Object(obj) => { let mut properties = HashMap::new(); for (key, value) in obj { @@ -570,7 +570,7 @@ fn schemaify(value: &serde_json::Value) -> Schema { Schema::Single(SchemaObject::Object { properties }) } - &serde_json::Value::Array(ref arr) => { + serde_json::Value::Array(arr) => { let result = arr.iter().map(schemaify).reduce(merge); if let Some(result) = result { @@ -596,9 +596,9 @@ fn merge(left: Schema, right: Schema) -> Schema { left } else { match &left { - &Schema::Single(SchemaObject::Object { ref properties }) => match &right { - &Schema::Single(SchemaObject::Object { - properties: ref right_properties, + Schema::Single(SchemaObject::Object { properties }) => match &right { + Schema::Single(SchemaObject::Object { + properties: right_properties, }) => { let mut existing = HashMap::new(); @@ -620,7 +620,7 @@ fn merge(left: Schema, right: Schema) -> Schema { properties: existing, }) } - &Schema::Composite(SchemaComposite { ref one_of }) => { + Schema::Composite(SchemaComposite { one_of }) => { let mut one_of = one_of.clone(); if !one_of.contains(&left) { @@ -629,17 +629,17 @@ fn merge(left: Schema, right: Schema) -> Schema { Schema::Composite(SchemaComposite { one_of }) } - &Schema::Single(_) => Schema::Composite(SchemaComposite { + Schema::Single(_) => Schema::Composite(SchemaComposite { one_of: vec![left, right], }), }, - &Schema::Single(SchemaObject::Array { ref items }) => match &right { - &Schema::Single(SchemaObject::Array { - items: ref right_items, - }) => Schema::Single(SchemaObject::Array { - items: Box::new(merge((**items).clone(), (**right_items).clone())), - }), - &Schema::Composite(SchemaComposite { ref one_of }) => { + Schema::Single(SchemaObject::Array { items }) => match &right { + Schema::Single(SchemaObject::Array { items: right_items }) => { + Schema::Single(SchemaObject::Array { + items: Box::new(merge((**items).clone(), (**right_items).clone())), + }) + } + Schema::Composite(SchemaComposite { one_of }) => { let mut one_of = one_of.clone(); if !one_of.contains(&left) { @@ -648,12 +648,12 @@ fn merge(left: Schema, right: Schema) -> Schema { Schema::Composite(SchemaComposite { one_of }) } - &Schema::Single(_) => Schema::Composite(SchemaComposite { + Schema::Single(_) => Schema::Composite(SchemaComposite { one_of: vec![left, right], }), }, - &Schema::Composite(SchemaComposite { ref one_of }) => match &right { - &Schema::Single(_) => { + Schema::Composite(SchemaComposite { one_of }) => match &right { + Schema::Single(_) => { let mut one_of = one_of.clone(); if !one_of.contains(&right) { one_of.push(right); @@ -661,8 +661,8 @@ fn merge(left: Schema, right: Schema) -> Schema { Schema::Composite(SchemaComposite { one_of }) } - &Schema::Composite(SchemaComposite { - one_of: ref right_one_of, + Schema::Composite(SchemaComposite { + one_of: right_one_of, }) => { let mut one_of = one_of.clone(); for right_value in right_one_of { @@ -674,11 +674,11 @@ fn merge(left: Schema, right: Schema) -> Schema { Schema::Composite(SchemaComposite { one_of }) } }, - &Schema::Single(_) => match &right { - &Schema::Single(_) => Schema::Composite(SchemaComposite { + Schema::Single(_) => match &right { + Schema::Single(_) => Schema::Composite(SchemaComposite { one_of: vec![left, right], }), - &Schema::Composite(SchemaComposite { ref one_of }) => { + Schema::Composite(SchemaComposite { one_of }) => { let mut one_of = one_of.clone(); if !one_of.contains(&left) { one_of.push(left.clone()); diff --git a/binary/apicli/src/path.rs b/binary/apicli/src/path.rs index d06b9d6..5c12fd1 100644 --- a/binary/apicli/src/path.rs +++ b/binary/apicli/src/path.rs @@ -24,7 +24,7 @@ pub fn get_input_paths( let mut input_paths = Vec::new(); match manifest { - &Some(core_entities::service::service_manifest_latest::Value::Swagger(_)) => { + Some(core_entities::service::service_manifest_latest::Value::Swagger(_)) => { let operation = service .commonApi .operations @@ -74,7 +74,7 @@ pub fn get_input_paths( } } } - &Some(core_entities::service::service_manifest_latest::Value::Action(ref manifest)) => { + Some(core_entities::service::service_manifest_latest::Value::Action(manifest)) => { let operation = manifest .operations .iter() @@ -94,13 +94,13 @@ pub fn get_input_paths( ); } } - &Some(core_entities::service::service_manifest_latest::Value::ApiWrapped(_)) => { + Some(core_entities::service::service_manifest_latest::Value::ApiWrapped(_)) => { bail!("Unimplemented manifest type: ApiWrapped") } - &Some(core_entities::service::service_manifest_latest::Value::SimpleCode(_)) => { + Some(core_entities::service::service_manifest_latest::Value::SimpleCode(_)) => { bail!("Unimplemented manifest type: SimpleCode") } - &Some(core_entities::service::service_manifest_latest::Value::ScriptedAction(_)) => { + Some(core_entities::service::service_manifest_latest::Value::ScriptedAction(_)) => { bail!("Unimplemented manifest type: ScriptedAction") } _ => bail!("Unknown manifest type"), @@ -125,7 +125,7 @@ pub fn get_output_paths( let mut output_paths = Vec::new(); match manifest { - &Some(core_entities::service::service_manifest_latest::Value::Swagger(_)) => { + Some(core_entities::service::service_manifest_latest::Value::Swagger(_)) => { let operation = service .commonApi .operations @@ -164,7 +164,7 @@ pub fn get_output_paths( } } } - &Some(core_entities::service::service_manifest_latest::Value::Action(ref manifest)) => { + Some(core_entities::service::service_manifest_latest::Value::Action(manifest)) => { let operation = manifest .operations .iter() @@ -181,13 +181,13 @@ pub fn get_output_paths( ); } } - &Some(core_entities::service::service_manifest_latest::Value::ApiWrapped(_)) => { + Some(core_entities::service::service_manifest_latest::Value::ApiWrapped(_)) => { bail!("Unimplemented manifest type: ApiWrapped") } - &Some(core_entities::service::service_manifest_latest::Value::SimpleCode(_)) => { + Some(core_entities::service::service_manifest_latest::Value::SimpleCode(_)) => { bail!("Unimplemented manifest type: SimpleCode") } - &Some(core_entities::service::service_manifest_latest::Value::ScriptedAction(_)) => { + Some(core_entities::service::service_manifest_latest::Value::ScriptedAction(_)) => { bail!("Unimplemented manifest type: ScriptedAction") } _ => bail!("Unknown manifest type"), @@ -312,7 +312,7 @@ pub fn populate_schema_list( prefix: &mut Vec, ) { match schema { - &Some(core_entities::service::schema::Value::Ref(ref reference)) => { + Some(core_entities::service::schema::Value::Ref(reference)) => { let schema = types.get(reference).cloned().and_then(|s| s.value); if seen.contains_key(reference) { @@ -335,22 +335,22 @@ pub fn populate_schema_list( populate_schema_list(list, &schema, types, seen, path, is_required, prefix); seen.remove(reference); } - &Some(core_entities::service::schema::Value::SchemaObject(ref schema)) => { + Some(core_entities::service::schema::Value::SchemaObject(schema)) => { populate_schema_object_list(list, schema, types, seen, path, is_required, prefix); } - &Some(core_entities::service::schema::Value::AllOf(ref all_of)) => { + Some(core_entities::service::schema::Value::AllOf(all_of)) => { for schema in &all_of.schema { populate_schema_list(list, &schema.value, types, seen, path, is_required, prefix); } } - &Some(core_entities::service::schema::Value::OneOf(ref one_of)) => { + Some(core_entities::service::schema::Value::OneOf(one_of)) => { for (idx, schema) in one_of.schema.iter().enumerate() { prefix.push(format!("one:{idx}")); populate_schema_list(list, &schema.value, types, seen, path, is_required, prefix); prefix.pop(); } } - &Some(core_entities::service::schema::Value::AnyOf(ref any_of)) => { + Some(core_entities::service::schema::Value::AnyOf(any_of)) => { for (idx, schema) in any_of.schema.iter().enumerate() { prefix.push(format!("any:{idx}")); populate_schema_list(list, &schema.value, types, seen, path, is_required, prefix); diff --git a/binary/apicli/src/stub.rs b/binary/apicli/src/stub.rs index 8573d2b..9a77249 100644 --- a/binary/apicli/src/stub.rs +++ b/binary/apicli/src/stub.rs @@ -23,7 +23,7 @@ pub fn get_input( let mut input_example = serde_json::Map::new(); match manifest { - &Some(core_entities::service::service_manifest_latest::Value::Swagger(_)) => { + Some(core_entities::service::service_manifest_latest::Value::Swagger(_)) => { let operation = service .commonApi .operations @@ -69,7 +69,7 @@ pub fn get_input( } } } - &Some(core_entities::service::service_manifest_latest::Value::Action(ref manifest)) => { + Some(core_entities::service::service_manifest_latest::Value::Action(manifest)) => { let operation = manifest .operations .iter() @@ -85,16 +85,16 @@ pub fn get_input( input_example.insert(param.name.clone(), default_value); } } - &Some(core_entities::service::service_manifest_latest::Value::ApiWrapped(ref manifest)) => { + Some(core_entities::service::service_manifest_latest::Value::ApiWrapped(manifest)) => { for param in &manifest.inputs { let default_value = parameter_to_value(param.param.type_); input_example.insert(param.param.name.clone(), default_value); } } - &Some(core_entities::service::service_manifest_latest::Value::SimpleCode(_)) => { + Some(core_entities::service::service_manifest_latest::Value::SimpleCode(_)) => { bail!("Unimplemented manifest type: SimpleCode") } - &Some(core_entities::service::service_manifest_latest::Value::ScriptedAction(_)) => { + Some(core_entities::service::service_manifest_latest::Value::ScriptedAction(_)) => { bail!("Unimplemented manifest type: ScriptedAction") } _ => bail!("Unknown manifest type"), @@ -118,7 +118,7 @@ pub fn get_output( let manifest = &service.manifest.v2().value; match manifest { - &Some(core_entities::service::service_manifest_latest::Value::Swagger(_)) => { + Some(core_entities::service::service_manifest_latest::Value::Swagger(_)) => { let operation = service .commonApi .operations @@ -160,7 +160,7 @@ pub fn get_output( Ok(serde_json::Value::Object(serde_json::Map::new())) } } - &Some(core_entities::service::service_manifest_latest::Value::Action(ref manifest)) => { + Some(core_entities::service::service_manifest_latest::Value::Action(manifest)) => { let operation = manifest .operations .iter() @@ -176,7 +176,7 @@ pub fn get_output( Ok(serde_json::Value::Object(output_examples)) } - &Some(core_entities::service::service_manifest_latest::Value::ApiWrapped(ref manifest)) => { + Some(core_entities::service::service_manifest_latest::Value::ApiWrapped(manifest)) => { let mut output_examples = serde_json::Map::new(); for param in &manifest.outputSelectors { // TODO: use JMES path to determine type @@ -186,10 +186,10 @@ pub fn get_output( Ok(serde_json::Value::Object(output_examples)) } - &Some(core_entities::service::service_manifest_latest::Value::SimpleCode(_)) => { + Some(core_entities::service::service_manifest_latest::Value::SimpleCode(_)) => { bail!("Unimplemented manifest type: SimpleCode") } - &Some(core_entities::service::service_manifest_latest::Value::ScriptedAction(_)) => { + Some(core_entities::service::service_manifest_latest::Value::ScriptedAction(_)) => { bail!("Unimplemented manifest type: ScriptedAction") } _ => bail!("Unknown manifest type"), @@ -237,7 +237,7 @@ pub fn schema_to_value( required: bool, ) -> serde_json::Value { match schema { - &Some(core_entities::service::schema::Value::Ref(ref reference)) => { + Some(core_entities::service::schema::Value::Ref(reference)) => { let schema = types.get(reference).cloned().and_then(|s| s.value); if seen.contains_key(reference) { @@ -254,7 +254,7 @@ pub fn schema_to_value( seen.remove(reference); schema } - &Some(core_entities::service::schema::Value::SchemaObject(ref schema)) => { + Some(core_entities::service::schema::Value::SchemaObject(schema)) => { schema_object_to_value(schema, types, seen, path, required) } _ => serde_json::Value::Object(serde_json::Map::new()), diff --git a/binary/apicli/src/template.rs b/binary/apicli/src/template.rs index 0102628..1b44ada 100644 --- a/binary/apicli/src/template.rs +++ b/binary/apicli/src/template.rs @@ -210,8 +210,8 @@ fn parse(input: &[InputTokens]) -> anyhow::Result { let name = parse_name(&mut walker)?; let direction = match walker.peek() { - Some(&InputTokens::InputArrow) => Direction::Input, - Some(&InputTokens::OutputArrow) => Direction::Output, + Some(InputTokens::InputArrow) => Direction::Input, + Some(InputTokens::OutputArrow) => Direction::Output, _ => return Err(anyhow::anyhow!("Invalid arrow token")), }; walker.advance(); @@ -231,7 +231,7 @@ fn parse(input: &[InputTokens]) -> anyhow::Result { /// Consumes a leading identifier as the mapping's name. fn parse_name(walker: &mut Walker) -> anyhow::Result { - if let Some(&InputTokens::Identifier(ref name)) = walker.peek() { + if let Some(InputTokens::Identifier(name)) = walker.peek() { let name = name.clone(); walker.advance(); Ok(name) @@ -249,11 +249,11 @@ fn parse_path(walker: &mut Walker) -> anyhow::Result<(Vec, let mut path = Vec::new(); let mut raw_path = String::new(); - if let Some(&InputTokens::LeftBracket) = walker.peek() { + if let Some(InputTokens::LeftBracket) = walker.peek() { walker.advance(); let key = parse_integer_key(walker)?; - if let Some(&InputTokens::RightBracket) = walker.peek() { + if let Some(InputTokens::RightBracket) = walker.peek() { walker.advance(); raw_path.push('['); @@ -275,7 +275,7 @@ fn parse_path(walker: &mut Walker) -> anyhow::Result<(Vec, loop { match walker.peek() { - Some(&InputTokens::Dot) => { + Some(InputTokens::Dot) => { walker.advance(); let key = parse_string_key(walker)?; @@ -284,11 +284,11 @@ fn parse_path(walker: &mut Walker) -> anyhow::Result<(Vec, path.push(key); } - Some(&InputTokens::LeftBracket) => { + Some(InputTokens::LeftBracket) => { walker.advance(); let key = parse_integer_key(walker)?; - if let Some(&InputTokens::RightBracket) = walker.peek() { + if let Some(InputTokens::RightBracket) = walker.peek() { walker.advance(); raw_path.push('['); @@ -311,7 +311,7 @@ fn parse_path(walker: &mut Walker) -> anyhow::Result<(Vec, /// Consumes an identifier as a dotted path segment (`.foo`). fn parse_string_key(walker: &mut Walker) -> anyhow::Result { - if let Some(&InputTokens::Identifier(ref key)) = walker.peek() { + if let Some(InputTokens::Identifier(key)) = walker.peek() { let key = key.clone(); walker.advance(); Ok(PathKey::Identifier(key)) @@ -327,11 +327,12 @@ fn parse_string_key(walker: &mut Walker) -> anyhow::Result /// (`[0]` or `["key"]`). fn parse_integer_key(walker: &mut Walker) -> anyhow::Result { match walker.peek() { - Some(&InputTokens::Integer(key)) => { + Some(InputTokens::Integer(key)) => { + let key = *key; walker.advance(); Ok(PathKey::Integer(key)) } - Some(&InputTokens::String(ref key)) => { + Some(InputTokens::String(key)) => { let key = key.clone(); walker.advance(); Ok(PathKey::String(key)) @@ -345,7 +346,7 @@ fn parse_integer_key(walker: &mut Walker) -> anyhow::Result` annotation and resolves it to an [`InputType`]. fn parse_input_type(walker: &mut Walker) -> anyhow::Result { - if let Some(&InputTokens::Lt) = walker.peek() { + if let Some(InputTokens::Lt) = walker.peek() { walker.advance(); } else { return Err(anyhow::anyhow!( @@ -353,7 +354,7 @@ fn parse_input_type(walker: &mut Walker) -> anyhow::Result InputType::String, "integer" => InputType::Integer, @@ -379,7 +380,7 @@ fn parse_input_type(walker: &mut Walker) -> anyhow::Result String { match self { - &PathKey::Identifier(ref key) => key.to_string(), - &PathKey::Integer(ref key) => key.to_string(), - &PathKey::String(ref key) => format!("\"{key}\""), + PathKey::Identifier(key) => key.to_string(), + PathKey::Integer(key) => key.to_string(), + PathKey::String(key) => format!("\"{key}\""), } } } diff --git a/binary/apid/src/main.rs b/binary/apid/src/main.rs index d31e365..8e93eb9 100644 --- a/binary/apid/src/main.rs +++ b/binary/apid/src/main.rs @@ -359,7 +359,7 @@ impl Engine for ApiDaemon { let req = req.into_inner(); let mut signals = self.signals.lock().unwrap_or_else(PoisonError::into_inner); - if let Some(&mut (_, ref tx)) = signals.get_mut(&req.execution_id) { + if let Some((_, tx)) = signals.get_mut(&req.execution_id) { let value = serde_json::from_str::(&req.input); if let Ok(value) = value { tx.send(value).map_err(|e| {