refactor(execution_engine,service_loader): split the two too_many_lines offenders - #93
Merged
Merged
Conversation
…e lint policy Phase 1 of issue #1. Nearly every crate opened with `#![warn(clippy::restriction, clippy::pedantic)]` plus a long, drifted `#![allow(...)]` list — clippy's own docs say `restriction` isn't meant to be enabled wholesale, and three of the "commonly allowed" lints turned out to be default-on via `clippy::all` (match_ref_pats, needless_borrowed_reference, blanket_clippy_restriction_lints), meaning they were suppressing mainstream clippy output, not opting out of a restriction-tier lint. - Centralize the policy in root Cargo.toml's new [workspace.lints.clippy] table: pedantic stays fully on, each restriction-tier lint was reviewed individually and either enabled (ref_patterns, map_err_ignore, allow_attributes_without_reason) or explicitly allowed with a documented reason (implicit_return, question_mark_used, shadow_reuse/unrelated/same, single_call_fn, absolute_paths, mod_module_files, min_ident_chars, separated_literal_suffix, std_instead_of_core/alloc, arbitrary_source_item_ordering, doc_paragraphs_missing_punctuation). too_many_lines/match_ref_pats/needless_borrowed_reference are explicitly re-enabled since they were being suppressed despite not being restriction-tier. - Every hand-written crate (14 that had the old block, plus auth/oauth_flow and prototypes/workflow_engine which never opted into any lint policy) now just does `[lints] workspace = true`. - Removed service_loader's three `#[cfg(test)] mod test { #![allow(clippy::restriction, clippy::pedantic)] }` blocks, now meaningless since the crate root no longer blanket-enables either group. - Fixed the small, mechanical anti-patterns the old suppressions were hiding: `extern crate alloc; use alloc::sync::Arc` (etc.) in 13 ordinary std crates (zero crates in this workspace are #![no_std]) replaced with plain std paths; 4 `.map_err(|_| ...)` sites that discarded the original error now capture it in the resulting message. The ~86-site ref-pattern/match_ref_pats/needless_borrowed_reference rewrite this now surfaces as warnings, the too_many_lines fallout, and the api_caller numeric-safety audit (as_conversions/cast_possible_truncation, kept allowed here with a documented reason) are tracked as follow-ups in #85, #86, #87 rather than folded into this policy change. Fixes #1 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VzKriyNnMHmqQ6UxPhsv2
Part of #85 (Phase 2 of #1). Converts every &Some(ref x)/match &value { &Variant(ref x) => ... } site in service_loader, execution_engine, and service_writer to plain match-ergonomics form (Some(x), Variant(x)) -- purely syntactic, binds the identical reference type as before. service_writer's clippy::ref_patterns/match_ref_pats/needless_borrowed_reference warnings are now fully zero; service_loader/execution_engine only retain pre-existing, out-of-scope warnings (too_many_lines, implicit_clone, etc.) tracked separately in #86. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VzKriyNnMHmqQ6UxPhsv2
Part of #85 (Phase 2 of #1). Converts every &Some(ref x)/match &value { &Variant(ref x) => ... }/&mut Value::Object(ref mut x) site in api_caller, filtered_runner, and python_runner to plain match-ergonomics form -- purely syntactic, binds the identical (mutable or immutable) reference type as before. api_caller had two duplicated pagination-matching functions (find_results and the page-size calculator) each repeating the same 4-arm match, plus duplicated Number-coercion matches in two request-limit resolvers -- fixed all instances in each. filtered_runner and python_runner each had one &mut/ref mut site not caught by these clippy lints (which don't cover &mut patterns) but flagged by the same #85 inventory as the same anti-pattern. All three crates now have zero clippy::ref_patterns/match_ref_pats/ needless_borrowed_reference warnings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VzKriyNnMHmqQ6UxPhsv2
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VzKriyNnMHmqQ6UxPhsv2
…es offenders Closes #86 (Phase 3 of #1). Phase 1 re-enabled clippy::too_many_lines workspace-wide; this was the entire remaining fallout -- exactly 2 functions over the 100-line threshold in the whole workspace. - execution_engine::Engine::run (123 lines): extracted each match arm (Swagger/Action/ApiWrapped/SimpleCode) into its own private dispatch_* helper method, mirroring the existing dispatch_code_runner/ resolve_data_connector/resolve_workflow methods already in this impl block. run() itself is now just the $input check, identifier parsing, the service/manifest lookup, a one-line-per-arm match delegating to the new helpers, and the final wrap_result call. dispatch_swagger and dispatch_action pick up clippy::too_many_arguments as a result -- allowed with the same #16 reasoning already used on dispatch_code_runner in this file, since each argument is a distinct pass-through dispatch input, not a case for a config struct. - service_loader's handle_schema (122 lines): the oneOf/anyOf/allOf composition handling repeated the identical fetch-map-collect shape three times. Extracted into a single resolve_schema_list helper (parameterized by field name), the read-side mirror of the exact dedup already done on the write side in #6 (service_writer's handle_composed_schema). Correction to Phase 1's PR descriptions: binary/apicli/src/engine.rs (issue #12's target) does NOT trip this lint -- no individual function there exceeds 100 lines, even though the file itself is oversized overall. too_many_lines is a per-function metric, so #86 and #12 don't actually overlap; #12 remains its own separate file-level refactor. Both changes are pure extract-method refactors with identical logic; existing tests (16 in execution_engine, 28 in service_loader) all pass unchanged. Confirmed via a full workspace clippy run: zero too_many_lines warnings remain anywhere in the repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VzKriyNnMHmqQ6UxPhsv2
8 tasks
jhamill34
force-pushed
the
claude/issue-1-phase2-binaries
branch
from
August 26, 2026 23:50
46ee4e0 to
99cd11d
Compare
…-too-many-lines # Conflicts: # usecases/execution_engine/src/lib.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #86 (Phase 3 of #1). Stacked on #91 (
binary/*), which is stacked on #90 (runners/*), which is stacked on #89 (usecases/*), which is stacked on #88 (Phase 1's lint policy).Phase 1 re-enabled
clippy::too_many_linesworkspace-wide (pedantic-tier, was being suppressed anyway). A freshcargo clippy --workspace --all-featureson top of the full stack found this was the entire remaining fallout — exactly 2 functions over the default 100-line threshold in the whole workspace:execution_engine::Engine::run(123 lines): extracted each match arm (Swagger/Action/ApiWrapped/SimpleCode) into its own privatedispatch_*helper method, mirroring the existingdispatch_code_runner/resolve_data_connector/resolve_workflowmethods already in thisimplblock.run()is now just the$inputcheck, identifier parsing, the service/manifest lookup, a one-line-per-armmatchdelegating to the new helpers, and the finalwrap_resultcall. Two of the new helpers (dispatch_swagger,dispatch_action) pick upclippy::too_many_arguments— allowed with the same#16reasoning already used ondispatch_code_runnerin this file, since each argument is a distinct pass-through dispatch input.service_loader'shandle_schema(122 lines): theoneOf/anyOf/allOfcomposition handling repeated the identical fetch-map-collect shape three times. Extracted into oneresolve_schema_listhelper parameterized by field name — the read-side mirror of the exact dedup already done on the write side in 8-arm per-HTTP-verb duplication in OpenAPI loader and service_writer #6 (service_writer'shandle_composed_schema).Correction to Phase 1's PR descriptions:
binary/apicli/src/engine.rs(issue #12's target) does not trip this lint — no individual function there exceeds 100 lines, even though the file itself is oversized overall (too_many_linesis a per-function metric). #86 and #12 don't actually overlap; #12 remains its own separate file-level refactor, untouched here.Both changes are pure extract-method refactors with identical logic — no behavior change.
Test plan
cargo build -p execution_engine -p service_loader— clean.cargo clippy -p execution_engine -p service_loader --all-features— zerotoo_many_lines/too_many_argumentswarnings in either crate.cargo test -p execution_engine -p service_loader --all-features— 44 tests (16 + 28), 0 failures.cargo clippy --workspace --all-features— zerotoo_many_lineswarnings anywhere in the repo, closing out Phase 3 of #1: measure and fix clippy::too_many_lines fallout #86.cargo build --workspace --all-features/cargo test --workspace --all-features— clean, zero failures.cargo fmt --all -- --check— clean.Generated by Claude Code