Skip to content

Commit 3825cf2

Browse files
committed
style: Replace disallowed methods with idiomatic alternatives
Replace map_or/map_or_else with map().unwrap_or()/map().unwrap_or_else() per clippy disallowed_methods configuration. Replace for_each with for loops for side-effectful iteration. Files changed: - claude.rs: map_or -> map().unwrap_or() - filter.rs: map_or_else -> map().unwrap_or_else() - results.rs: for_each -> for loop - manager.rs: map_or -> if let Some - output_formatter.rs: map_or -> map().unwrap_or() - progress.rs: for_each -> for loops - composer.rs: for_each -> for loop, map_or -> if let Some - git_operations_tests.rs: for_each with panic -> collect + assert - error_handling_test.rs: map_or -> map().unwrap_or()
1 parent 2bb9c88 commit 3825cf2

File tree

9 files changed

+65
-70
lines changed

9 files changed

+65
-70
lines changed

src/cook/execution/claude.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ impl<R: CommandRunner + 'static> ClaudeExecutor for ClaudeExecutorImpl<R> {
7878
env_vars: HashMap<String, String>,
7979
) -> Result<ExecutionResult> {
8080
// Handle test mode
81-
let test_mode = self.test_config.as_ref().map_or(false, |c| c.test_mode);
81+
let test_mode = self.test_config.as_ref().map(|c| c.test_mode).unwrap_or(false);
8282
if test_mode {
8383
return self.handle_test_mode_execution(command).await;
8484
}
@@ -110,7 +110,7 @@ impl<R: CommandRunner + 'static> ClaudeExecutor for ClaudeExecutorImpl<R> {
110110

111111
async fn check_claude_cli(&self) -> Result<bool> {
112112
// Always return true in test mode
113-
let test_mode = self.test_config.as_ref().map_or(false, |c| c.test_mode);
113+
let test_mode = self.test_config.as_ref().map(|c| c.test_mode).unwrap_or(false);
114114
if test_mode {
115115
return Ok(true);
116116
}

src/cook/execution/data_pipeline/filter.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -610,13 +610,12 @@ impl FilterExpression {
610610
/// Pure function: Compare string against regex pattern
611611
pub(crate) fn compare_regex(actual: Option<&Value>, expected: &Value) -> bool {
612612
match (actual, expected) {
613-
(Some(Value::String(a)), Value::String(pattern)) => Regex::new(pattern).map_or_else(
614-
|e| {
613+
(Some(Value::String(a)), Value::String(pattern)) => Regex::new(pattern)
614+
.map(|re| re.is_match(a))
615+
.unwrap_or_else(|e| {
615616
warn!("Invalid regex pattern '{}': {}", pattern, e);
616617
false
617-
},
618-
|re| re.is_match(a),
619-
),
618+
}),
620619
_ => false,
621620
}
622621
}
@@ -728,13 +727,12 @@ impl FilterExpression {
728727

729728
/// Pure function: Check if string matches regex pattern
730729
pub(crate) fn regex_matches(text: &str, pattern: &str) -> bool {
731-
Regex::new(pattern).map_or_else(
732-
|e| {
730+
Regex::new(pattern)
731+
.map(|re| re.is_match(text))
732+
.unwrap_or_else(|e| {
733733
warn!("Invalid regex pattern '{}': {}", pattern, e);
734734
false
735-
},
736-
|re| re.is_match(text),
737-
)
735+
})
738736
}
739737
}
740738

src/cook/execution/mapreduce/agent/results.rs

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -174,28 +174,24 @@ impl AgentResultAggregator for DefaultResultAggregator {
174174
context.set("map.results", results_value);
175175
}
176176

177-
// Add individual successful results using iterator
178-
results
179-
.successful
180-
.iter()
181-
.enumerate()
182-
.for_each(|(i, result)| {
183-
context.set(
184-
format!("map.successful.{}.item_id", i),
185-
json!(result.item_id),
186-
);
187-
if let Some(output) = &result.output {
188-
context.set(format!("map.successful.{}.output", i), json!(output));
189-
}
190-
});
177+
// Add individual successful results
178+
for (i, result) in results.successful.iter().enumerate() {
179+
context.set(
180+
format!("map.successful.{}.item_id", i),
181+
json!(result.item_id),
182+
);
183+
if let Some(output) = &result.output {
184+
context.set(format!("map.successful.{}.output", i), json!(output));
185+
}
186+
}
191187

192-
// Add individual failed results using iterator
193-
results.failed.iter().enumerate().for_each(|(i, result)| {
188+
// Add individual failed results
189+
for (i, result) in results.failed.iter().enumerate() {
194190
context.set(format!("map.failed.{}.item_id", i), json!(result.item_id));
195191
if let Some(error) = &result.error {
196192
context.set(format!("map.failed.{}.error", i), json!(error));
197193
}
198-
});
194+
}
199195

200196
context
201197
}

src/cook/execution/mapreduce/checkpoint/manager.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -157,18 +157,19 @@ impl CheckpointManager {
157157
);
158158
}
159159

160-
// Validate agent state consistency using functional find
161-
checkpoint
160+
// Validate agent state consistency
161+
if let Some(agent_id) = checkpoint
162162
.agent_state
163163
.agent_assignments
164164
.keys()
165165
.find(|agent_id| !checkpoint.agent_state.active_agents.contains_key(*agent_id))
166-
.map_or(Ok(()), |agent_id| {
167-
Err(anyhow!(
168-
"Agent {} has assignments but is not active",
169-
agent_id
170-
))
171-
})
166+
{
167+
return Err(anyhow!(
168+
"Agent {} has assignments but is not active",
169+
agent_id
170+
));
171+
}
172+
Ok(())
172173
}
173174

174175
/// Validate checkpoint integrity

src/cook/execution/mapreduce/dry_run/output_formatter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ impl OutputFormatter {
216216
// Show available variables
217217
let total_vars = preview.setup_variables.len()
218218
+ preview.reduce_variables.len()
219-
+ preview.item_variables.first().map_or(0, |v| v.len());
219+
+ preview.item_variables.first().map(|v| v.len()).unwrap_or(0);
220220

221221
writeln!(output, " Available variables: {}", total_vars).unwrap();
222222
}

src/cook/execution/mapreduce/resources/git_operations_tests.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -539,11 +539,13 @@ mod tests {
539539
),
540540
];
541541

542-
// Apply all validations using functional approach
543-
validations
542+
// Apply all validations
543+
let failures: Vec<_> = validations
544544
.iter()
545545
.filter(|(condition, _)| !condition)
546-
.for_each(|(_, msg)| panic!("{}", msg));
546+
.map(|(_, msg)| *msg)
547+
.collect();
548+
assert!(failures.is_empty(), "Validation failures: {:?}", failures);
547549

548550
// Modified files check - we created 2 files across 2 commits
549551
// The exact count depends on deduplication logic

src/cook/execution/progress.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -744,10 +744,10 @@ impl ProgressWebServer {
744744
let connections = self.connections.read().await;
745745
let message = serde_json::to_string(&update).unwrap_or_default();
746746

747-
// Broadcast to all connections using functional iteration
748-
connections.values().for_each(|sender| {
747+
// Broadcast to all connections
748+
for sender in connections.values() {
749749
let _ = sender.send(message.clone());
750-
});
750+
}
751751
}
752752
}
753753

@@ -893,13 +893,13 @@ impl ProgressWebServer {
893893
output.push_str("# HELP mapreduce_agent_states Count of agents by state\n");
894894
output.push_str("# TYPE mapreduce_agent_states gauge\n");
895895

896-
// Build state metrics using functional iteration
897-
state_counts.iter().for_each(|(state, count)| {
896+
// Build state metrics
897+
for (state, count) in &state_counts {
898898
output.push_str(&format!(
899899
"mapreduce_agent_states{{job_id=\"{}\",state=\"{}\"}} {}\n",
900900
server.tracker.job_id, state, count
901901
));
902-
});
902+
}
903903

904904
// Job duration
905905
let duration = server.tracker.start_time.elapsed().as_secs();

src/cook/workflow/composition/composer.rs

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -243,15 +243,15 @@ impl WorkflowComposer {
243243

244244
// Apply parameter defaults if defined
245245
if let Some(params) = &mut workflow.parameters {
246-
params
246+
for param in params
247247
.optional
248248
.iter_mut()
249249
.filter(|param| param.default.is_none())
250-
.for_each(|param| {
251-
if let Some(default_value) = defaults.get(&param.name) {
252-
param.default = Some(default_value.clone());
253-
}
254-
});
250+
{
251+
if let Some(default_value) = defaults.get(&param.name) {
252+
param.default = Some(default_value.clone());
253+
}
254+
}
255255
}
256256

257257
Ok(())
@@ -628,17 +628,15 @@ impl WorkflowComposer {
628628
&self,
629629
sub_workflows: &HashMap<String, super::SubWorkflow>,
630630
) -> Result<()> {
631-
// Check for missing sources using functional approach
632-
sub_workflows
633-
.iter()
634-
.find(|(_, sub)| !sub.source.exists())
635-
.map_or(Ok(()), |(name, sub)| {
636-
anyhow::bail!(
637-
"Sub-workflow '{}' source does not exist: {:?}",
638-
name,
639-
sub.source
640-
)
641-
})
631+
// Check for missing sources
632+
if let Some((name, sub)) = sub_workflows.iter().find(|(_, sub)| !sub.source.exists()) {
633+
anyhow::bail!(
634+
"Sub-workflow '{}' source does not exist: {:?}",
635+
name,
636+
sub.source
637+
)
638+
}
639+
Ok(())
642640
}
643641
}
644642

tests/error_handling_test.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ fn test_option_handling_without_unwrap() -> Result<()> {
2929
assert_eq!(some_value.unwrap_or(0), 42);
3030
assert_eq!(none_value.unwrap_or(0), 0);
3131

32-
// Using map_or for transformations
33-
assert_eq!(some_value.map_or(0, |v| v * 2), 84);
34-
assert_eq!(none_value.map_or(0, |v| v * 2), 0);
32+
// Using map + unwrap_or for transformations
33+
assert_eq!(some_value.map(|v| v * 2).unwrap_or(0), 84);
34+
assert_eq!(none_value.map(|v| v * 2).unwrap_or(0), 0);
3535

3636
Ok(())
3737
}
@@ -56,9 +56,9 @@ fn test_result_handling_without_unwrap() -> Result<()> {
5656
assert_eq!(ok_result.as_ref().unwrap_or(&0), &42);
5757
assert_eq!(err_result.as_ref().unwrap_or(&0), &0);
5858

59-
// Using map_or for transformations
60-
assert_eq!(ok_result.as_ref().map_or(0, |v| v * 2), 84);
61-
assert_eq!(err_result.map_or(0, |v| v * 2), 0);
59+
// Using map + unwrap_or for transformations
60+
assert_eq!(ok_result.as_ref().map(|v| v * 2).unwrap_or(0), 84);
61+
assert_eq!(err_result.map(|v| v * 2).unwrap_or(0), 0);
6262

6363
Ok(())
6464
}

0 commit comments

Comments
 (0)