Skip to content

Clone less often #1357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
notpeter wants to merge 3 commits into from
Closed

Clone less often #1357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
08c24ba
Only clone when we need to
notpeter Apr 30, 2026
f2c8146
Better option handling
notpeter Apr 30, 2026
f07539b
Update test output
notpeter Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 60 additions & 7 deletions progenitor-impl/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,11 @@ impl Generator {
}

fn cli_method(&mut self, method: &crate::method::OperationMethod) -> CliOperation {
let args = self.cli_method_args(method);
let CliArg {
parser: parser_args,
consumer: consumer_args,
} = self.cli_method_args(method);
} = args;

let about = method.summary.as_ref().map(|summary| {
quote! {
Expand Down Expand Up @@ -427,13 +428,13 @@ impl Generator {
let arg_type = self.type_space.get_type(arg_type_id).unwrap();
let arg_type_name = arg_type.ident();

let value_expr = arg_value_tokens(&arg_type, false);

let consumer = quote! {
if let Some(value) =
matches.get_one::<#arg_type_name>(#arg_name)
{
// clone here in case the arg type doesn't impl
// From<&T>
request = request.#arg_fn(value.clone());
request = request.#arg_fn(#value_expr);
}
};

Expand Down Expand Up @@ -566,6 +567,7 @@ impl Generator {
} else {
None
};
let is_optional = maybe_inner_type.is_some();

let prop_type = if let Some(inner_type) = maybe_inner_type {
inner_type
Expand All @@ -591,16 +593,16 @@ impl Generator {

let prop_fn = format_ident!("{}", sanitize(name, Case::Snake));
let prop_type_ident = prop_type.ident();
let value_expr = arg_value_tokens(&prop_type, is_optional);

let consumer = quote! {
if let Some(value) =
matches.get_one::<#prop_type_ident>(
#prop_name,
)
{
// clone here in case the arg type
// doesn't impl TryFrom<&T>
request = request.body_map(|body| {
body.#prop_fn(value.clone())
body.#prop_fn(#value_expr)
})
}
};
Expand All @@ -622,6 +624,57 @@ impl Generator {
}
}

fn is_copy_type(ty: &Type<'_>) -> bool {
match ty.details() {
// Builtins and native format types that typify emits and we know are Copy.
typify::TypeDetails::Builtin(name) => matches!(
name,
"bool"
| "i8"
| "i16"
| "i32"
| "i64"
| "u8"
| "u16"
| "u32"
| "u64"
| "::std::num::NonZeroU8"
| "::std::num::NonZeroU16"
| "::std::num::NonZeroU32"
| "::std::num::NonZeroU64"
| "f32"
| "f64"
| "::uuid::Uuid"
| "::chrono::naive::NaiveDate"
| "::chrono::DateTime<::chrono::offset::Utc>"
| "::std::net::IpAddr"
| "::std::net::Ipv4Addr"
| "::std::net::Ipv6Addr"
),
// () is Copy.
typify::TypeDetails::Unit => true,
// Simple enums derive Copy; enums with data do not.
typify::TypeDetails::Enum(e) => e
.variants_info()
.all(|v| matches!(v.details, TypeEnumVariant::Simple)),
_ => false,
}
}

/// Returns the token expression for passing `value: &T` to a builder setter
/// accepting either `V: TryInto<T>` or `V: TryInto<Option<T>>`.
fn arg_value_tokens(ty: &Type<'_>, needs_owned_value: bool) -> TokenStream {
if is_copy_type(ty) {
quote! { *value }
} else if !needs_owned_value && matches!(ty.details(), typify::TypeDetails::String) {
// &str: TryInto<String> via String: From<&str>, so no clone needed.
quote! { value.as_str() }
} else {
// Clone because the builder takes T not &T, and the type may not impl TryFrom<&T>.
quote! { value.clone() }
}
}

enum Volitionality {
Optional,
Required,
Expand Down
46 changes: 23 additions & 23 deletions progenitor-impl/tests/output/src/buildomat_cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_task_get(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.task_get();
if let Some(value) = matches.get_one::<::std::string::String>("task") {
request = request.task(value.clone());
request = request.task(value.as_str());
}

self.config.execute_task_get(matches, &mut request)?;
Expand Down Expand Up @@ -454,15 +454,15 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_task_submit(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.task_submit();
if let Some(value) = matches.get_one::<bool>("default") {
request = request.body_map(|body| body.default(value.clone()))
request = request.body_map(|body| body.default(*value))
}

if let Some(value) = matches.get_one::<::std::string::String>("name") {
request = request.body_map(|body| body.name(value.clone()))
request = request.body_map(|body| body.name(value.as_str()))
}

if let Some(value) = matches.get_one::<::std::string::String>("script") {
request = request.body_map(|body| body.script(value.clone()))
request = request.body_map(|body| body.script(value.as_str()))
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down Expand Up @@ -493,11 +493,11 @@ impl<T: CliConfig> Cli<T> {
) -> anyhow::Result<()> {
let mut request = self.client.task_events_get();
if let Some(value) = matches.get_one::<u32>("minseq") {
request = request.minseq(value.clone());
request = request.minseq(*value);
}

if let Some(value) = matches.get_one::<::std::string::String>("task") {
request = request.task(value.clone());
request = request.task(value.as_str());
}

self.config.execute_task_events_get(matches, &mut request)?;
Expand All @@ -520,7 +520,7 @@ impl<T: CliConfig> Cli<T> {
) -> anyhow::Result<()> {
let mut request = self.client.task_outputs_get();
if let Some(value) = matches.get_one::<::std::string::String>("task") {
request = request.task(value.clone());
request = request.task(value.as_str());
}

self.config
Expand All @@ -544,11 +544,11 @@ impl<T: CliConfig> Cli<T> {
) -> anyhow::Result<()> {
let mut request = self.client.task_output_download();
if let Some(value) = matches.get_one::<::std::string::String>("output") {
request = request.output(value.clone());
request = request.output(value.as_str());
}

if let Some(value) = matches.get_one::<::std::string::String>("task") {
request = request.task(value.clone());
request = request.task(value.as_str());
}

self.config
Expand All @@ -568,7 +568,7 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_user_create(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.user_create();
if let Some(value) = matches.get_one::<::std::string::String>("name") {
request = request.body_map(|body| body.name(value.clone()))
request = request.body_map(|body| body.name(value.as_str()))
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down Expand Up @@ -634,11 +634,11 @@ impl<T: CliConfig> Cli<T> {
) -> anyhow::Result<()> {
let mut request = self.client.worker_bootstrap();
if let Some(value) = matches.get_one::<::std::string::String>("bootstrap") {
request = request.body_map(|body| body.bootstrap(value.clone()))
request = request.body_map(|body| body.bootstrap(value.as_str()))
}

if let Some(value) = matches.get_one::<::std::string::String>("token") {
request = request.body_map(|body| body.token(value.clone()))
request = request.body_map(|body| body.token(value.as_str()))
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down Expand Up @@ -686,19 +686,19 @@ impl<T: CliConfig> Cli<T> {
) -> anyhow::Result<()> {
let mut request = self.client.worker_task_append();
if let Some(value) = matches.get_one::<::std::string::String>("payload") {
request = request.body_map(|body| body.payload(value.clone()))
request = request.body_map(|body| body.payload(value.as_str()))
}

if let Some(value) = matches.get_one::<::std::string::String>("stream") {
request = request.body_map(|body| body.stream(value.clone()))
request = request.body_map(|body| body.stream(value.as_str()))
}

if let Some(value) = matches.get_one::<::std::string::String>("task") {
request = request.task(value.clone());
request = request.task(value.as_str());
}

if let Some(value) = matches.get_one::<::chrono::DateTime<::chrono::offset::Utc>>("time") {
request = request.body_map(|body| body.time(value.clone()))
request = request.body_map(|body| body.time(*value))
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down Expand Up @@ -730,7 +730,7 @@ impl<T: CliConfig> Cli<T> {
) -> anyhow::Result<()> {
let mut request = self.client.worker_task_upload_chunk();
if let Some(value) = matches.get_one::<::std::string::String>("task") {
request = request.task(value.clone());
request = request.task(value.as_str());
}

self.config
Expand All @@ -754,11 +754,11 @@ impl<T: CliConfig> Cli<T> {
) -> anyhow::Result<()> {
let mut request = self.client.worker_task_complete();
if let Some(value) = matches.get_one::<bool>("failed") {
request = request.body_map(|body| body.failed(value.clone()))
request = request.body_map(|body| body.failed(*value))
}

if let Some(value) = matches.get_one::<::std::string::String>("task") {
request = request.task(value.clone());
request = request.task(value.as_str());
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down Expand Up @@ -790,15 +790,15 @@ impl<T: CliConfig> Cli<T> {
) -> anyhow::Result<()> {
let mut request = self.client.worker_task_add_output();
if let Some(value) = matches.get_one::<::std::string::String>("path") {
request = request.body_map(|body| body.path(value.clone()))
request = request.body_map(|body| body.path(value.as_str()))
}

if let Some(value) = matches.get_one::<i64>("size") {
request = request.body_map(|body| body.size(value.clone()))
request = request.body_map(|body| body.size(*value))
}

if let Some(value) = matches.get_one::<::std::string::String>("task") {
request = request.task(value.clone());
request = request.task(value.as_str());
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down Expand Up @@ -886,7 +886,7 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_header_arg(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.header_arg();
if let Some(value) = matches.get_one::<types::HeaderArgAcceptLanguage>("accept-language") {
request = request.accept_language(value.clone());
request = request.accept_language(*value);
}

self.config.execute_header_arg(matches, &mut request)?;
Expand Down
2 changes: 1 addition & 1 deletion progenitor-impl/tests/output/src/cli_gen_cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_uno(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.uno();
if let Some(value) = matches.get_one::<::std::string::String>("gateway") {
request = request.gateway(value.clone());
request = request.gateway(value.as_str());
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down
26 changes: 13 additions & 13 deletions progenitor-impl/tests/output/src/keeper_cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,15 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_enrol(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.enrol();
if let Some(value) = matches.get_one::<::std::string::String>("authorization") {
request = request.authorization(value.clone());
request = request.authorization(value.as_str());
}

if let Some(value) = matches.get_one::<::std::string::String>("host") {
request = request.body_map(|body| body.host(value.clone()))
request = request.body_map(|body| body.host(value.as_str()))
}

if let Some(value) = matches.get_one::<::std::string::String>("key") {
request = request.body_map(|body| body.key(value.clone()))
request = request.body_map(|body| body.key(value.as_str()))
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down Expand Up @@ -241,7 +241,7 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_global_jobs(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.global_jobs();
if let Some(value) = matches.get_one::<::std::string::String>("authorization") {
request = request.authorization(value.clone());
request = request.authorization(value.as_str());
}

self.config.execute_global_jobs(matches, &mut request)?;
Expand All @@ -261,7 +261,7 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_ping(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.ping();
if let Some(value) = matches.get_one::<::std::string::String>("authorization") {
request = request.authorization(value.clone());
request = request.authorization(value.as_str());
}

self.config.execute_ping(matches, &mut request)?;
Expand All @@ -281,21 +281,21 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_report_finish(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.report_finish();
if let Some(value) = matches.get_one::<::std::string::String>("authorization") {
request = request.authorization(value.clone());
request = request.authorization(value.as_str());
}

if let Some(value) = matches.get_one::<i32>("duration-millis") {
request = request.body_map(|body| body.duration_millis(value.clone()))
request = request.body_map(|body| body.duration_millis(*value))
}

if let Some(value) =
matches.get_one::<::chrono::DateTime<::chrono::offset::Utc>>("end-time")
{
request = request.body_map(|body| body.end_time(value.clone()))
request = request.body_map(|body| body.end_time(*value))
}

if let Some(value) = matches.get_one::<i32>("exit-status") {
request = request.body_map(|body| body.exit_status(value.clone()))
request = request.body_map(|body| body.exit_status(*value))
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down Expand Up @@ -323,7 +323,7 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_report_output(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.report_output();
if let Some(value) = matches.get_one::<::std::string::String>("authorization") {
request = request.authorization(value.clone());
request = request.authorization(value.as_str());
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down Expand Up @@ -351,17 +351,17 @@ impl<T: CliConfig> Cli<T> {
pub async fn execute_report_start(&self, matches: &::clap::ArgMatches) -> anyhow::Result<()> {
let mut request = self.client.report_start();
if let Some(value) = matches.get_one::<::std::string::String>("authorization") {
request = request.authorization(value.clone());
request = request.authorization(value.as_str());
}

if let Some(value) = matches.get_one::<::std::string::String>("script") {
request = request.body_map(|body| body.script(value.clone()))
request = request.body_map(|body| body.script(value.as_str()))
}

if let Some(value) =
matches.get_one::<::chrono::DateTime<::chrono::offset::Utc>>("start-time")
{
request = request.body_map(|body| body.start_time(value.clone()))
request = request.body_map(|body| body.start_time(*value))
}

if let Some(value) = matches.get_one::<std::path::PathBuf>("json-body") {
Expand Down
Loading
Loading