From d0924726f88a35bbd9ef2e17aef4ca0322834cea Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 17:51:23 +0000 Subject: [PATCH 1/6] refactor: replace blanket clippy::restriction with a curated workspace lint policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_018VzKriyNnMHmqQ6UxPhsv2 --- Cargo.toml | 39 +++++++++++++++++++ auth/oauth_flow/Cargo.toml | 3 ++ auth/oauth_flow/src/routes/callback.rs | 8 ++-- binary/apicli/Cargo.toml | 3 ++ binary/apicli/src/engine.rs | 11 +++--- binary/apicli/src/main.rs | 19 --------- binary/apicli/src/stub.rs | 2 - binary/apicli/src/template.rs | 7 ++-- binary/apid/Cargo.toml | 3 ++ binary/apid/src/main.rs | 23 +---------- binary/apid/src/workers/loader.rs | 5 +-- binary/apid/src/workers/mod.rs | 5 +-- binary/apid/src/workers/watcher.rs | 7 +--- common/data_structures/Cargo.toml | 3 ++ common/data_structures/src/lib.rs | 15 ------- common/data_structures/src/trie.rs | 5 ++- prototypes/workflow_engine/Cargo.toml | 3 ++ runners/api_caller/Cargo.toml | 3 ++ runners/api_caller/src/error.rs | 2 - runners/api_caller/src/lib.rs | 37 +++++------------- runners/filtered_runner/Cargo.toml | 3 ++ runners/filtered_runner/src/error.rs | 2 - runners/filtered_runner/src/lib.rs | 29 ++------------ runners/javascript_runner/Cargo.toml | 3 ++ runners/javascript_runner/src/error.rs | 2 - runners/javascript_runner/src/lib.rs | 27 ++----------- runners/python_runner/Cargo.toml | 3 ++ runners/python_runner/src/bindings.rs | 15 +++---- runners/python_runner/src/error.rs | 2 - runners/python_runner/src/lib.rs | 25 +----------- runners/user_input/Cargo.toml | 3 ++ runners/user_input/src/error.rs | 2 - runners/user_input/src/lib.rs | 27 +------------ runners/workflow_runner/Cargo.toml | 3 ++ runners/workflow_runner/src/lib.rs | 17 -------- storage/in_memory_storage/Cargo.toml | 3 ++ storage/in_memory_storage/src/error.rs | 2 - storage/in_memory_storage/src/lib.rs | 19 --------- storage/in_memory_storage/src/repo.rs | 10 ++--- storage/local_file_loader/Cargo.toml | 3 ++ storage/local_file_loader/src/lib.rs | 15 ------- usecases/execution_engine/Cargo.toml | 3 ++ usecases/execution_engine/src/error.rs | 2 - usecases/execution_engine/src/lib.rs | 33 +++++----------- usecases/service_loader/Cargo.toml | 3 ++ usecases/service_loader/src/error.rs | 2 - usecases/service_loader/src/lib.rs | 18 --------- .../service_loader/src/loaders/openapi/mod.rs | 2 - .../src/loaders/openapi/utils.rs | 1 - usecases/service_writer/Cargo.toml | 3 ++ usecases/service_writer/src/error.rs | 2 - usecases/service_writer/src/lib.rs | 21 +--------- 52 files changed, 151 insertions(+), 357 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0479472..c1b9bd8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,45 @@ members = [ "prototypes/workflow_engine", ] +[workspace.lints.clippy] +# Curated per issue #1, replacing the former per-crate blanket +# `#![warn(clippy::restriction, clippy::pedantic)]`. `restriction` is not +# meant to be enabled wholesale (clippy's own docs say so — it contains +# mutually contradictory lints); each restriction-tier lint below was +# reviewed individually and either enabled or explicitly rejected with a +# reason. `pedantic` stays fully enabled. +pedantic = { level = "warn", priority = -1 } + +# Restriction-tier lints deliberately kept on: +ref_patterns = "warn" +map_err_ignore = "warn" +allow_attributes_without_reason = "warn" + +# Not restriction-tier, but were being suppressed anyway (default-on via +# clippy::all / clippy::pedantic) — explicitly un-suppressing: +too_many_lines = "warn" +match_ref_pats = "warn" +needless_borrowed_reference = "warn" + +# Restriction-tier lints reviewed and deliberately left off: +implicit_return = "allow" # bans expression-as-return-value, Rust's core idiom +question_mark_used = "allow" # fires on every `?`; universally not recommended to enable +shadow_reuse = "allow" # this codebase's dominant "transform in place" style +shadow_unrelated = "allow" # same family as shadow_reuse +shadow_same = "allow" # same family +single_call_fn = "allow" # named single-call helpers are a deliberate readability tool here +absolute_paths = "allow" # error.rs files intentionally fully-qualify wrapped error types +mod_module_files = "allow" # codebase's actual, consistent convention (mod.rs for nested modules) +self_named_module_files = "allow" # mutually exclusive with the above; moot either way +min_ident_chars = "allow" # short loop/index identifiers are fine +separated_literal_suffix = "allow" # pure style preference, no correctness value +std_instead_of_core = "allow" # no crate in this workspace is #![no_std]; inapplicable +std_instead_of_alloc = "allow" # same +arbitrary_source_item_ordering = "allow" # would force alphabetical ordering over the +# codebase's logical grouping (constructors before accessors, related +# methods together) for no readability gain — reviewed and rejected +doc_paragraphs_missing_punctuation = "allow" # low-signal nitpick, not worth the churn + [workspace.dependencies] anyhow = "1.0" async-trait = "0.1" diff --git a/auth/oauth_flow/Cargo.toml b/auth/oauth_flow/Cargo.toml index fecb2a5..0de6e96 100644 --- a/auth/oauth_flow/Cargo.toml +++ b/auth/oauth_flow/Cargo.toml @@ -18,3 +18,6 @@ credential_entities = { path = "../../entities/credentials" } thiserror = { workspace = true } + +[lints] +workspace = true diff --git a/auth/oauth_flow/src/routes/callback.rs b/auth/oauth_flow/src/routes/callback.rs index f6c0a44..269ae14 100644 --- a/auth/oauth_flow/src/routes/callback.rs +++ b/auth/oauth_flow/src/routes/callback.rs @@ -91,12 +91,12 @@ pub async fn route( oauth_config.accessTokenPath.clone() }; - let expression = jmespath::compile(&access_token_path).map_err(|_| { - error::CallbackResponse::InternalError("Invalid access token path".to_string()) + let expression = jmespath::compile(&access_token_path).map_err(|err| { + error::CallbackResponse::InternalError(format!("Invalid access token path: {err}")) })?; - let access_token = expression.search(response_body).map_err(|_| { - error::CallbackResponse::InternalError("Unable to find access token".to_string()) + let access_token = expression.search(response_body).map_err(|err| { + error::CallbackResponse::InternalError(format!("Unable to find access token: {err}")) })?; { diff --git a/binary/apicli/Cargo.toml b/binary/apicli/Cargo.toml index 89b08cd..4b83e14 100644 --- a/binary/apicli/Cargo.toml +++ b/binary/apicli/Cargo.toml @@ -39,3 +39,6 @@ tonic-build = { workspace = true } dhat-heap = [] dhat-ad-hoc = [] + +[lints] +workspace = true diff --git a/binary/apicli/src/engine.rs b/binary/apicli/src/engine.rs index bb84715..25920e3 100644 --- a/binary/apicli/src/engine.rs +++ b/binary/apicli/src/engine.rs @@ -1,13 +1,12 @@ -#![allow(clippy::print_stdout)] -#![allow(clippy::too_many_lines)] -#![allow(clippy::needless_borrowed_reference)] +#![allow( + clippy::print_stdout, + reason = "this CLI's actual output mechanism for command results" +)] //! Handlers for every CLI subcommand: the gRPC client calls to `apid` //! ([`Cli`]), the local JSON-schema inference/merge helpers, and the //! `generate` command's template rendering. -extern crate alloc; -use alloc::sync::Arc; use serde::{Deserialize, Serialize}; use tera::{Context, Tera}; @@ -15,7 +14,7 @@ use std::{ collections::HashMap, env, fs, io, path::{Path, PathBuf}, - sync::Mutex, + sync::{Arc, Mutex}, }; use anyhow::{anyhow, Context as _}; diff --git a/binary/apicli/src/main.rs b/binary/apicli/src/main.rs index 679d4d3..47b5182 100644 --- a/binary/apicli/src/main.rs +++ b/binary/apicli/src/main.rs @@ -1,22 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - clippy::implicit_return, - clippy::shadow_reuse, - clippy::match_ref_pats, - clippy::shadow_unrelated, - clippy::shadow_same, - clippy::question_mark_used, - // clippy::too_many_lines - clippy::absolute_paths, - clippy::single_call_fn, - clippy::ref_patterns, - - clippy::min_ident_chars, -)] - //! The CLI binary: a thin gRPC client to `apid`, plus local scaffolding //! tools for generating new service definitions and an embedded //! interactive-login web server. diff --git a/binary/apicli/src/stub.rs b/binary/apicli/src/stub.rs index 07448ac..8573d2b 100644 --- a/binary/apicli/src/stub.rs +++ b/binary/apicli/src/stub.rs @@ -1,5 +1,3 @@ -#![allow(clippy::separated_literal_suffix)] - //! Generates a sample JSON input/output payload for an operation, used by //! the `InputStub`/`OutputStub` CLI commands. diff --git a/binary/apicli/src/template.rs b/binary/apicli/src/template.rs index bcdf823..0102628 100644 --- a/binary/apicli/src/template.rs +++ b/binary/apicli/src/template.rs @@ -1,5 +1,3 @@ -#![allow(clippy::needless_borrowed_reference)] - //! A hand-rolled lexer/parser for the `Generate` command's small input/ //! output-mapping DSL: //! @@ -482,7 +480,10 @@ impl FromStr for InputDescription { #[cfg(test)] mod test { - #![allow(clippy::panic_in_result_fn)] + #![allow( + clippy::panic_in_result_fn, + reason = "test code — unwrap/expect panics are expected on failure" + )] use super::*; diff --git a/binary/apid/Cargo.toml b/binary/apid/Cargo.toml index 82cef20..9e506e6 100644 --- a/binary/apid/Cargo.toml +++ b/binary/apid/Cargo.toml @@ -59,3 +59,6 @@ workflow = [] input = [] wrapper = [] + +[lints] +workspace = true diff --git a/binary/apid/src/main.rs b/binary/apid/src/main.rs index 2f0ad3d..d31e365 100644 --- a/binary/apid/src/main.rs +++ b/binary/apid/src/main.rs @@ -1,22 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - clippy::implicit_return, - clippy::shadow_reuse, - clippy::match_ref_pats, - clippy::shadow_unrelated, - clippy::shadow_same, - // clippy::too_many_lines - clippy::question_mark_used, - clippy::absolute_paths, - clippy::single_call_fn, - clippy::ref_patterns, - - clippy::min_ident_chars, -)] - //! The daemon binary: a `tonic` gRPC server that wires concrete adapters //! into an [`execution_engine::Engine`] and exposes it as the [`Engine`] //! service, the composition root of the whole workspace. @@ -26,8 +7,6 @@ mod constants; mod util; mod workers; -extern crate alloc; -use alloc::sync::Arc; use config::Configuration; use std::{ @@ -36,7 +15,7 @@ use std::{ fs::{self, File}, panic, path::PathBuf, - sync::{mpsc::Sender, Mutex, PoisonError, RwLock}, + sync::{mpsc::Sender, Arc, Mutex, PoisonError, RwLock}, }; use anyhow::{anyhow, Context}; diff --git a/binary/apid/src/workers/loader.rs b/binary/apid/src/workers/loader.rs index 9d7586f..5c276f6 100644 --- a/binary/apid/src/workers/loader.rs +++ b/binary/apid/src/workers/loader.rs @@ -1,15 +1,12 @@ //! The background thread that (re)loads changed services into the shared //! repositories, signalled by [`super::watcher`]. -extern crate alloc; -use alloc::sync::Arc; - use std::{ collections::HashMap, path::PathBuf, sync::{ mpsc::{Receiver, Sender}, - Mutex, PoisonError, + Arc, Mutex, PoisonError, }, thread::{self, JoinHandle}, }; diff --git a/binary/apid/src/workers/mod.rs b/binary/apid/src/workers/mod.rs index 9cda96a..b0e36ed 100644 --- a/binary/apid/src/workers/mod.rs +++ b/binary/apid/src/workers/mod.rs @@ -4,13 +4,10 @@ mod loader; mod watcher; -extern crate alloc; -use alloc::sync::Arc; - use std::{ collections::HashMap, path::PathBuf, - sync::{mpsc, Mutex}, + sync::{mpsc, Arc, Mutex}, thread::JoinHandle, }; diff --git a/binary/apid/src/workers/watcher.rs b/binary/apid/src/workers/watcher.rs index 0964a35..b3abe7a 100644 --- a/binary/apid/src/workers/watcher.rs +++ b/binary/apid/src/workers/watcher.rs @@ -1,18 +1,15 @@ //! The background thread that polls loaded services' directories for //! filesystem changes and reports them to [`super::loader`]. -extern crate alloc; -use alloc::sync::Arc; - -use core::time::Duration; use std::{ collections::{HashMap, HashSet}, path::PathBuf, sync::{ mpsc::{self, Receiver, Sender}, - Mutex, PoisonError, + Arc, Mutex, PoisonError, }, thread::{self, JoinHandle}, + time::Duration, }; use notify::Watcher; diff --git a/common/data_structures/Cargo.toml b/common/data_structures/Cargo.toml index 7a1784a..c0202ce 100644 --- a/common/data_structures/Cargo.toml +++ b/common/data_structures/Cargo.toml @@ -9,3 +9,6 @@ edition = "2021" [dev-dependencies] tempfile = "3" + +[lints] +workspace = true diff --git a/common/data_structures/src/lib.rs b/common/data_structures/src/lib.rs index b293349..9dff2bf 100644 --- a/common/data_structures/src/lib.rs +++ b/common/data_structures/src/lib.rs @@ -1,18 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - - clippy::implicit_return, - clippy::shadow_reuse, - clippy::shadow_unrelated, - clippy::match_ref_pats, - - // Would like to turn on (Configured to 50?) - clippy::too_many_lines -)] - //! Small, dependency-free data structures shared across the workspace. //! //! A byte-wise, wildcard-aware [`trie`](trie::Trie), and a background-thread diff --git a/common/data_structures/src/trie.rs b/common/data_structures/src/trie.rs index a06a8e1..6790181 100644 --- a/common/data_structures/src/trie.rs +++ b/common/data_structures/src/trie.rs @@ -1,4 +1,7 @@ -#![allow(clippy::arithmetic_side_effects)] +#![allow( + clippy::arithmetic_side_effects, + reason = "byte-index arithmetic for wildcard matching, bounds-checked by the slice ops around it" +)] //! A byte-wise trie for wildcard-aware string-key lookups — e.g. matching a //! concrete key like `"application/json"` against a registered pattern like diff --git a/prototypes/workflow_engine/Cargo.toml b/prototypes/workflow_engine/Cargo.toml index 4dbe1b2..11bc1e0 100644 --- a/prototypes/workflow_engine/Cargo.toml +++ b/prototypes/workflow_engine/Cargo.toml @@ -12,3 +12,6 @@ serde_json = { workspace = true } [dev-dependencies] tokio = { workspace = true, features = ["test-util", "macros"] } + +[lints] +workspace = true diff --git a/runners/api_caller/Cargo.toml b/runners/api_caller/Cargo.toml index 2a939f2..cc674e9 100644 --- a/runners/api_caller/Cargo.toml +++ b/runners/api_caller/Cargo.toml @@ -27,3 +27,6 @@ async-trait = { workspace = true } tempfile = "3" protobuf = { workspace = true } tokio = { workspace = true, features = ["macros", "rt"] } + +[lints] +workspace = true diff --git a/runners/api_caller/src/error.rs b/runners/api_caller/src/error.rs index 9c4f9f6..3202b71 100644 --- a/runners/api_caller/src/error.rs +++ b/runners/api_caller/src/error.rs @@ -1,5 +1,3 @@ -#![allow(clippy::std_instead_of_core)] - //! Errors produced while making an API call. use std::{io, num::TryFromIntError}; diff --git a/runners/api_caller/src/lib.rs b/runners/api_caller/src/lib.rs index ecc62fc..1b0f0e5 100644 --- a/runners/api_caller/src/lib.rs +++ b/runners/api_caller/src/lib.rs @@ -1,25 +1,9 @@ -#![warn(clippy::restriction, clippy::pedantic)] #![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - - clippy::implicit_return, - clippy::shadow_reuse, - clippy::shadow_unrelated, - clippy::match_ref_pats, - clippy::separated_literal_suffix, - clippy::as_conversions, clippy::cast_possible_truncation, - - // Would like to turn on (Configured to 50?) - clippy::too_many_lines, - clippy::needless_borrowed_reference, - clippy::separated_literal_suffix, - clippy::question_mark_used, - clippy::absolute_paths, - clippy::ref_patterns, + reason = "pagination limit/offset casts between i32/usize/u64 are unaudited; \ + tracked as a dedicated numeric-safety follow-up to issue #1, not \ + rushed into this lint-hygiene pass" )] //! A [`DataConnectionRunner`] adapter that resolves an operation's request @@ -29,8 +13,6 @@ mod constants; pub mod error; -extern crate alloc; - use std::collections::HashMap; use base64::Engine as _; @@ -492,10 +474,9 @@ impl APICallState { creds: Option<&Authentication>, ) -> error::Result<()> { let defined_auth = &manifest.auth; - let auth_type = defined_auth - .type_ - .enum_value() - .map_err(|_| error::APICaller::Unimplemented("Unrecognized auth type".into()))?; + let auth_type = defined_auth.type_.enum_value().map_err(|raw| { + error::APICaller::Unimplemented(format!("Unrecognized auth type: {raw}")) + })?; match auth_type { core_entities::service::swagger_service::service_auth::Type::HEADER => { let key = defined_auth @@ -1102,11 +1083,13 @@ impl AsyncDataConnectionRunner for AsyncAPICaller { #[cfg(test)] mod tests { - use alloc::sync::Arc; use std::{ io::{BufRead, BufReader, Write}, net::TcpListener, - sync::atomic::{AtomicUsize, Ordering}, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }, thread, }; diff --git a/runners/filtered_runner/Cargo.toml b/runners/filtered_runner/Cargo.toml index 167721d..96fc9b4 100644 --- a/runners/filtered_runner/Cargo.toml +++ b/runners/filtered_runner/Cargo.toml @@ -18,3 +18,6 @@ core_entities = { path = "../../entities/core" } credential_entities = { path = "../../entities/credentials" } common_data_structures = { path = "../../common/data_structures" } + +[lints] +workspace = true diff --git a/runners/filtered_runner/src/error.rs b/runners/filtered_runner/src/error.rs index 7e09ffe..5744a38 100644 --- a/runners/filtered_runner/src/error.rs +++ b/runners/filtered_runner/src/error.rs @@ -1,5 +1,3 @@ -#![allow(clippy::std_instead_of_core)] - //! Errors produced while resolving an [`APIWrapper`](crate::APIWrapper) //! call. diff --git a/runners/filtered_runner/src/lib.rs b/runners/filtered_runner/src/lib.rs index 84d87cf..9a2c974 100644 --- a/runners/filtered_runner/src/lib.rs +++ b/runners/filtered_runner/src/lib.rs @@ -1,37 +1,16 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - - clippy::implicit_return, - clippy::shadow_reuse, - clippy::shadow_unrelated, - clippy::match_ref_pats, - clippy::separated_literal_suffix, - - // Would like to turn on (Configured to 50?) - clippy::too_many_lines, - clippy::question_mark_used, - clippy::needless_borrowed_reference, - - clippy::absolute_paths, - clippy::single_call_fn, - clippy::ref_patterns, -)] - //! A [`FilteredRunner`] adapter that calls another already-registered //! operation through the [`execution_engine`] and narrows its result down to //! a specific set of selected output fields. pub mod error; -extern crate alloc; -use alloc::{rc::Rc, sync::Arc}; +use std::{ + rc::Rc, + sync::{Arc, RwLock}, +}; use common_data_structures::log_writer::LogWriter; use lazy_static::lazy_static; -use std::sync::RwLock; use execution_engine::services::FilteredRunner; use regex::Regex; diff --git a/runners/javascript_runner/Cargo.toml b/runners/javascript_runner/Cargo.toml index 9742bf1..f0303a4 100644 --- a/runners/javascript_runner/Cargo.toml +++ b/runners/javascript_runner/Cargo.toml @@ -21,3 +21,6 @@ regex = { workspace = true } mini-v8 = "0.4" + +[lints] +workspace = true diff --git a/runners/javascript_runner/src/error.rs b/runners/javascript_runner/src/error.rs index d96132f..50cdba0 100644 --- a/runners/javascript_runner/src/error.rs +++ b/runners/javascript_runner/src/error.rs @@ -1,5 +1,3 @@ -#![allow(clippy::std_instead_of_core)] - //! Errors produced while running a [`JsActionRunner`](crate::JsActionRunner) //! operation. diff --git a/runners/javascript_runner/src/lib.rs b/runners/javascript_runner/src/lib.rs index 4e87d94..6ec98d5 100644 --- a/runners/javascript_runner/src/lib.rs +++ b/runners/javascript_runner/src/lib.rs @@ -1,23 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - - clippy::implicit_return, - clippy::shadow_reuse, - clippy::shadow_unrelated, - clippy::match_ref_pats, - clippy::separated_literal_suffix, - - // Would like to turn on (Configured to 50?) - clippy::too_many_lines, - clippy::question_mark_used, - clippy::needless_borrowed_reference, - clippy::single_call_fn, - clippy::absolute_paths, -)] - //! A [`CodeRunner`] adapter that executes a JavaScript operation body //! inside a [`MiniV8`] interpreter reused per thread. @@ -26,11 +6,12 @@ mod constants; mod converters; pub mod error; -extern crate alloc; -use alloc::sync::Arc; use mini_v8::MiniV8; -use std::{cell::RefCell, sync::RwLock}; +use std::{ + cell::RefCell, + sync::{Arc, RwLock}, +}; use common_data_structures::log_writer::LogWriter; use execution_engine::services::CodeRunner; diff --git a/runners/python_runner/Cargo.toml b/runners/python_runner/Cargo.toml index 66d2204..62e89f6 100644 --- a/runners/python_runner/Cargo.toml +++ b/runners/python_runner/Cargo.toml @@ -21,3 +21,6 @@ regex = { workspace = true } pyo3 = "0.17" + +[lints] +workspace = true diff --git a/runners/python_runner/src/bindings.rs b/runners/python_runner/src/bindings.rs index df85511..84a5384 100644 --- a/runners/python_runner/src/bindings.rs +++ b/runners/python_runner/src/bindings.rs @@ -1,16 +1,10 @@ -#![allow(clippy::std_instead_of_core)] - //! `pyo3` classes installed into a running script's module namespace as the //! `api`/`workflow`/`action`/`task` bindings, giving the script a way to //! call back into the engine and to log activity. -extern crate alloc; -use alloc::sync::Arc; - -use std::sync::RwLock; +use std::sync::{Arc, RwLock}; use std::thread; - -use core::time::Duration; +use std::time::Duration; use common_data_structures::log_writer::LogWriter; @@ -327,7 +321,10 @@ impl WorkflowLogger { /// Logs `display` at [`constants::LOG_STATUS`], tagged with /// `groupId`, without affecting the script's recorded outcome. - #[allow(non_snake_case)] + #[allow( + non_snake_case, + reason = "groupId matches the script-facing binding's parameter name" + )] fn status(&mut self, py: Python<'_>, display: &PyAny, groupId: &str) -> PyResult> { self.print_display(display, &format!("{}={groupId}", constants::LOG_STATUS))?; Ok(py.None()) diff --git a/runners/python_runner/src/error.rs b/runners/python_runner/src/error.rs index 551f2eb..818a3a0 100644 --- a/runners/python_runner/src/error.rs +++ b/runners/python_runner/src/error.rs @@ -1,5 +1,3 @@ -#![allow(clippy::std_instead_of_core)] - //! Errors produced while running a //! [`PyActionRunner`](crate::PyActionRunner) operation. diff --git a/runners/python_runner/src/lib.rs b/runners/python_runner/src/lib.rs index 9296761..2c68bf5 100644 --- a/runners/python_runner/src/lib.rs +++ b/runners/python_runner/src/lib.rs @@ -1,23 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - - clippy::implicit_return, - clippy::shadow_reuse, - clippy::shadow_unrelated, - clippy::match_ref_pats, - clippy::separated_literal_suffix, - - // Would like to turn on (Configured to 50?) - clippy::too_many_lines, - clippy::question_mark_used, - clippy::single_call_fn, - clippy::absolute_paths, - clippy::min_ident_chars -)] - //! A [`CodeRunner`] adapter that executes a Python operation body in an //! embedded `CPython` interpreter, exposing `api`/`workflow`/`action`/`task` //! bindings the script can use to interact with the engine. @@ -27,10 +7,7 @@ mod constants; mod converters; pub mod error; -extern crate alloc; -use alloc::sync::Arc; - -use std::sync::RwLock; +use std::sync::{Arc, RwLock}; use common_data_structures::log_writer::LogWriter; use execution_engine::services::CodeRunner; diff --git a/runners/user_input/Cargo.toml b/runners/user_input/Cargo.toml index f89eca7..2490abc 100644 --- a/runners/user_input/Cargo.toml +++ b/runners/user_input/Cargo.toml @@ -11,3 +11,6 @@ serde_json = { workspace = true } execution_engine = { path = "../../usecases/execution_engine" } thiserror = { workspace = true } + +[lints] +workspace = true diff --git a/runners/user_input/src/error.rs b/runners/user_input/src/error.rs index cafc993..90bcc8f 100644 --- a/runners/user_input/src/error.rs +++ b/runners/user_input/src/error.rs @@ -1,5 +1,3 @@ -#![allow(clippy::std_instead_of_core)] - //! Errors produced while waiting on a [`UserInput`](crate::UserInput) //! prompt. diff --git a/runners/user_input/src/lib.rs b/runners/user_input/src/lib.rs index 81ce5f1..a69c8ba 100644 --- a/runners/user_input/src/lib.rs +++ b/runners/user_input/src/lib.rs @@ -1,38 +1,15 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - - clippy::implicit_return, - clippy::shadow_reuse, - clippy::match_ref_pats, - - // Would like to turn on (Configured to 50?) - clippy::too_many_lines, - clippy::question_mark_used, - clippy::needless_borrowed_reference, - clippy::absolute_paths, - clippy::ref_patterns, - clippy::single_call_fn, - clippy::min_ident_chars, -)] - //! An [`InputPrompter`] adapter that pauses a running workflow and waits for //! an external caller to supply the answer. pub mod error; -extern crate alloc; -use alloc::sync::Arc; -use core::time::Duration; - use std::{ collections::HashMap, sync::{ mpsc::{self, Sender}, - Mutex, + Arc, Mutex, }, + time::Duration, }; use execution_engine::services::InputPrompter; diff --git a/runners/workflow_runner/Cargo.toml b/runners/workflow_runner/Cargo.toml index 24484c7..13052e2 100644 --- a/runners/workflow_runner/Cargo.toml +++ b/runners/workflow_runner/Cargo.toml @@ -22,3 +22,6 @@ common_data_structures = { path = "../../common/data_structures" } tempfile = "3" credential_entities = { path = "../../entities/credentials" } protobuf = { workspace = true } + +[lints] +workspace = true diff --git a/runners/workflow_runner/src/lib.rs b/runners/workflow_runner/src/lib.rs index 08e02a9..a6b9635 100644 --- a/runners/workflow_runner/src/lib.rs +++ b/runners/workflow_runner/src/lib.rs @@ -1,20 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - clippy::implicit_return, - clippy::shadow_reuse, - clippy::shadow_unrelated, - clippy::match_ref_pats, - clippy::separated_literal_suffix, - clippy::question_mark_used, - clippy::single_call_fn, - clippy::absolute_paths, - clippy::ref_patterns, - clippy::min_ident_chars -)] - //! Adapts `prototypes/workflow_engine::WorkflowEngine` to //! `execution_engine`'s async `WorkflowRunner` output port - the concrete //! wiring that connects the standalone prototype crate to the daemon's diff --git a/storage/in_memory_storage/Cargo.toml b/storage/in_memory_storage/Cargo.toml index 29beb2c..1ce3c9c 100644 --- a/storage/in_memory_storage/Cargo.toml +++ b/storage/in_memory_storage/Cargo.toml @@ -19,3 +19,6 @@ credential_entities = { path = "../../entities/credentials" } regex = { workspace = true } thiserror = { workspace = true } + +[lints] +workspace = true diff --git a/storage/in_memory_storage/src/error.rs b/storage/in_memory_storage/src/error.rs index 296374f..c9dae05 100644 --- a/storage/in_memory_storage/src/error.rs +++ b/storage/in_memory_storage/src/error.rs @@ -1,5 +1,3 @@ -#![allow(clippy::std_instead_of_core)] - //! Errors produced by an [`OperationRepos`](crate::OperationRepos) //! repository. diff --git a/storage/in_memory_storage/src/lib.rs b/storage/in_memory_storage/src/lib.rs index 9696c5d..537cbfe 100644 --- a/storage/in_memory_storage/src/lib.rs +++ b/storage/in_memory_storage/src/lib.rs @@ -1,22 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - - clippy::implicit_return, - clippy::shadow_reuse, - clippy::match_ref_pats, - - // Would like to turn on (Configured to 50?) - clippy::too_many_lines, - clippy::question_mark_used, - clippy::needless_borrowed_reference, - clippy::absolute_paths, - clippy::ref_patterns, - clippy::single_call_fn -)] - //! An in-memory storage adapter that backs both [`service_loader`]'s //! [`LoaderOutput`] (persisting loaded services/credentials) and //! [`execution_engine`]'s [`EngineLookup`] (resolving them again at diff --git a/storage/in_memory_storage/src/repo.rs b/storage/in_memory_storage/src/repo.rs index 9256eb1..9b1c2a4 100644 --- a/storage/in_memory_storage/src/repo.rs +++ b/storage/in_memory_storage/src/repo.rs @@ -1,9 +1,8 @@ //! The [`Repository`] storage port and its in-memory implementation. -use super::error; +use std::collections::BTreeMap; -extern crate alloc; -use alloc::collections::BTreeMap; +use super::error; /// A keyed store of values of type `V`. pub trait Repository { @@ -52,10 +51,7 @@ impl Default for InMemoryRepository { impl Repository for InMemoryRepository { #[inline] fn list(&self) -> Vec { - self.storage - .keys() - .map(alloc::borrow::ToOwned::to_owned) - .collect() + self.storage.keys().cloned().collect() } #[inline] diff --git a/storage/local_file_loader/Cargo.toml b/storage/local_file_loader/Cargo.toml index f02ae82..11b57d6 100644 --- a/storage/local_file_loader/Cargo.toml +++ b/storage/local_file_loader/Cargo.toml @@ -10,3 +10,6 @@ service_loader = { path = "../../usecases/service_loader" } service_writer = { path = "../../usecases/service_writer" } thiserror = { workspace = true } + +[lints] +workspace = true diff --git a/storage/local_file_loader/src/lib.rs b/storage/local_file_loader/src/lib.rs index 2e0ee1f..b165cdd 100644 --- a/storage/local_file_loader/src/lib.rs +++ b/storage/local_file_loader/src/lib.rs @@ -1,18 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - - clippy::implicit_return, - clippy::shadow_reuse, - clippy::match_ref_pats, - - // Would like to turn on (Configured to 50?) - clippy::too_many_lines, - clippy::absolute_paths -)] - //! A local-filesystem adapter implementing [`service_loader`]'s [`Fetcher`] //! and [`service_writer`]'s [`Storage`] output ports. diff --git a/usecases/execution_engine/Cargo.toml b/usecases/execution_engine/Cargo.toml index 4f92947..cf94387 100644 --- a/usecases/execution_engine/Cargo.toml +++ b/usecases/execution_engine/Cargo.toml @@ -22,3 +22,6 @@ anyhow = { workspace = true } tempfile = "3" tokio = { workspace = true, features = ["macros", "rt"] } + +[lints] +workspace = true diff --git a/usecases/execution_engine/src/error.rs b/usecases/execution_engine/src/error.rs index 9f2221d..a40b906 100644 --- a/usecases/execution_engine/src/error.rs +++ b/usecases/execution_engine/src/error.rs @@ -1,5 +1,3 @@ -#![allow(clippy::std_instead_of_core, clippy::absolute_paths)] - //! Errors produced while resolving and running an operation identifier. use std::io; diff --git a/usecases/execution_engine/src/lib.rs b/usecases/execution_engine/src/lib.rs index 3203d2d..965850f 100644 --- a/usecases/execution_engine/src/lib.rs +++ b/usecases/execution_engine/src/lib.rs @@ -1,20 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - - clippy::implicit_return, - clippy::shadow_reuse, - clippy::match_ref_pats, - - // Would like to turn on (Configured to 50?) - clippy::too_many_lines, - clippy::needless_borrowed_reference, - clippy::question_mark_used, - clippy::ref_patterns -)] - //! The core orchestrator: [`Engine`] resolves a `service.operation` //! identifier against a loaded manifest and dispatches it to the //! registered [`services`] output port for that manifest's type. @@ -25,16 +8,13 @@ pub mod services; /// Shared constants for the engine. mod constants; -extern crate alloc; -use alloc::sync::Arc; - use common_data_structures::log_writer::LogWriter; use serde_json::Value; use services::{ AsyncDataConnectionRunner, CodeRunner, DataConnectionRunner, DataConnectorBundle, EngineInputContext, EngineLookup, FilteredRunner, InputPrompter, ScriptRunner, WorkflowRunner, }; -use std::collections::HashMap; +use std::{collections::HashMap, sync::Arc}; use chrono::offset::Local; use core_entities::service::{code_resource::Language, service_manifest_latest}; @@ -489,7 +469,10 @@ impl Engine { /// Returns an error if the identifier can't be parsed, the service /// isn't found, the manifest isn't a `Swagger` manifest, or no /// [`AsyncDataConnectionRunner`] is registered. - #[allow(clippy::type_complexity)] + #[allow( + clippy::type_complexity, + reason = "return type mirrors the resolved connector's own nested Result/Option shape" + )] pub fn resolve_data_connector( &self, identifier: &str, @@ -544,7 +527,11 @@ impl Engine { /// # Errors /// Returns an error if no `CodeRunner` is registered for `lang_key`, or /// if the registered runner's own call fails. - #[allow(clippy::too_many_arguments)] + #[allow( + clippy::too_many_arguments, + reason = "each argument is a distinct dispatch input; see the doc comment above for why \ + this isn't yet unified with the sibling dispatch fn (#16)" + )] fn dispatch_code_runner( &self, identifier: &str, diff --git a/usecases/service_loader/Cargo.toml b/usecases/service_loader/Cargo.toml index d009783..827f078 100644 --- a/usecases/service_loader/Cargo.toml +++ b/usecases/service_loader/Cargo.toml @@ -21,3 +21,6 @@ core_entities = { path = "../../entities/core" } thiserror = { workspace = true } anyhow = { workspace = true } + +[lints] +workspace = true diff --git a/usecases/service_loader/src/error.rs b/usecases/service_loader/src/error.rs index 5291943..5c3281d 100644 --- a/usecases/service_loader/src/error.rs +++ b/usecases/service_loader/src/error.rs @@ -1,5 +1,3 @@ -#![allow(clippy::std_instead_of_core)] - //! Errors produced while loading a service, its credentials, or its //! override configuration. diff --git a/usecases/service_loader/src/lib.rs b/usecases/service_loader/src/lib.rs index 30e129a..30320ac 100644 --- a/usecases/service_loader/src/lib.rs +++ b/usecases/service_loader/src/lib.rs @@ -1,19 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - clippy::implicit_return, - clippy::shadow_reuse, - clippy::shadow_unrelated, - clippy::too_many_lines, - clippy::question_mark_used, - clippy::needless_borrowed_reference, - clippy::absolute_paths, - clippy::ref_patterns, - clippy::single_call_fn -)] - //! Loads a service manifest (currently OpenAPI-based), its credentials, and //! its override configuration from a [`Fetcher`] source into a //! [`LoaderOutput`] sink. @@ -213,8 +197,6 @@ impl Default for ServiceLoader { #[cfg(test)] mod test { - #![allow(clippy::restriction, clippy::pedantic)] - use std::cell::RefCell; use std::collections::HashMap; diff --git a/usecases/service_loader/src/loaders/openapi/mod.rs b/usecases/service_loader/src/loaders/openapi/mod.rs index 9c6a165..3d30de9 100644 --- a/usecases/service_loader/src/loaders/openapi/mod.rs +++ b/usecases/service_loader/src/loaders/openapi/mod.rs @@ -538,8 +538,6 @@ fn handle_schema( #[cfg(test)] mod test { - #![allow(clippy::restriction, clippy::pedantic)] - use core::cell::RefCell; use super::*; diff --git a/usecases/service_loader/src/loaders/openapi/utils.rs b/usecases/service_loader/src/loaders/openapi/utils.rs index 1cdf481..462503e 100644 --- a/usecases/service_loader/src/loaders/openapi/utils.rs +++ b/usecases/service_loader/src/loaders/openapi/utils.rs @@ -170,7 +170,6 @@ impl FromStr for Reference { #[cfg(test)] mod test { - #![allow(clippy::restriction, clippy::pedantic)] use super::*; #[test] diff --git a/usecases/service_writer/Cargo.toml b/usecases/service_writer/Cargo.toml index 78afd28..729173a 100644 --- a/usecases/service_writer/Cargo.toml +++ b/usecases/service_writer/Cargo.toml @@ -19,3 +19,6 @@ core_entities = { path = "../../entities/core" } thiserror = { workspace = true } anyhow = { workspace = true } + +[lints] +workspace = true diff --git a/usecases/service_writer/src/error.rs b/usecases/service_writer/src/error.rs index 19af49f..584d5af 100644 --- a/usecases/service_writer/src/error.rs +++ b/usecases/service_writer/src/error.rs @@ -1,5 +1,3 @@ -#![allow(clippy::std_instead_of_core)] - //! Errors produced while writing a service manifest or its credentials. use std::io; diff --git a/usecases/service_writer/src/lib.rs b/usecases/service_writer/src/lib.rs index 8295472..ddf49ee 100644 --- a/usecases/service_writer/src/lib.rs +++ b/usecases/service_writer/src/lib.rs @@ -1,20 +1,3 @@ -#![warn(clippy::restriction, clippy::pedantic)] -#![allow( - clippy::blanket_clippy_restriction_lints, - clippy::mod_module_files, - clippy::self_named_module_files, - clippy::implicit_return, - clippy::shadow_reuse, - clippy::match_ref_pats, - // clippy::shadow_unrelated, - // clippy::too_many_lines - clippy::question_mark_used, - clippy::needless_borrowed_reference, - clippy::absolute_paths, - clippy::ref_patterns, - clippy::single_call_fn -)] - //! Serializes an internal [`VersionedServiceTree`]/[`Authentication`] pair //! back out to OpenAPI-shaped JSON/YAML and credential JSON, the inverse of //! `service_loader`'s `OpenAPI` loader. @@ -330,8 +313,8 @@ fn handle_parameter( ) -> error::Result<()> { // TODO: extract into a referece based on a flag - let in_type = source.in_.enum_value().map_err(|_| { - error::ServiceWriter::Unimplemented("Unrecognized parameter location".into()) + let in_type = source.in_.enum_value().map_err(|raw| { + error::ServiceWriter::Unimplemented(format!("Unrecognized parameter location: {raw}")) })?; sink.insert( "in".into(), From f5107336bdded071a9e2f533302c505555ff4f10 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 18:08:16 +0000 Subject: [PATCH 2/6] refactor(usecases): rewrite pre-2018 ref-patterns to match ergonomics 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 Claude-Session: https://claude.ai/code/session_018VzKriyNnMHmqQ6UxPhsv2 --- usecases/execution_engine/src/lib.rs | 20 +++++++++---------- usecases/service_loader/src/lib.rs | 4 ++-- .../service_loader/src/loaders/openapi/mod.rs | 8 ++++---- usecases/service_writer/src/lib.rs | 20 +++++++++---------- 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/usecases/execution_engine/src/lib.rs b/usecases/execution_engine/src/lib.rs index 965850f..a5ec677 100644 --- a/usecases/execution_engine/src/lib.rs +++ b/usecases/execution_engine/src/lib.rs @@ -187,7 +187,7 @@ impl Engine { // ScriptedAction -> ScriptRunner if identifier == "$input" { - if let &Some(ref input_handler) = &self.input_handler { + if let Some(input_handler) = &self.input_handler { return input_handler.run(params, context); } @@ -209,8 +209,8 @@ impl Engine { let manifest = service.manifest.v2(); let result = match &manifest.value { - &Some(service_manifest_latest::Value::Swagger(ref swagger)) => { - if let &Some(ref connector) = &self.connector { + Some(service_manifest_latest::Value::Swagger(swagger)) => { + if let Some(connector) = &self.connector { let api = &service.commonApi; let creds = credentials.as_ref(); @@ -233,7 +233,7 @@ impl Engine { )) } } - &Some(service_manifest_latest::Value::Action(ref action)) => { + Some(service_manifest_latest::Value::Action(action)) => { let operation = action .operations .iter() @@ -275,8 +275,8 @@ impl Engine { ))) } } - &Some(service_manifest_latest::Value::ApiWrapped(ref api_wrapped)) => { - if let &Some(ref filtered_runner) = &self.filtered_runner { + Some(service_manifest_latest::Value::ApiWrapped(api_wrapped)) => { + if let Some(filtered_runner) = &self.filtered_runner { self.log(identifier, "API_WRAPPED", "STARTED")?; let result = filtered_runner.run( service_name, @@ -294,7 +294,7 @@ impl Engine { )) } } - &Some(service_manifest_latest::Value::SimpleCode(ref simple_code)) => { + Some(service_manifest_latest::Value::SimpleCode(simple_code)) => { match simple_code.code.language.enum_value() { Ok(Language::PYTHON) => self.dispatch_code_runner( identifier, @@ -371,7 +371,7 @@ impl Engine { let manifest = service.manifest.v2(); match &manifest.value { - &Some(service_manifest_latest::Value::Workflow(ref workflow)) => { + Some(service_manifest_latest::Value::Workflow(workflow)) => { let workflow_runner = self.workflow_runner.clone().ok_or_else(|| { error::ExecutionEngine::NotFound("Workflow runner not registered".into()) })?; @@ -451,7 +451,7 @@ impl Engine { matches!( &manifest.value, - &Some(service_manifest_latest::Value::Workflow(_)) + Some(service_manifest_latest::Value::Workflow(_)) ) } @@ -498,7 +498,7 @@ impl Engine { let manifest = service.manifest.v2(); match &manifest.value { - &Some(service_manifest_latest::Value::Swagger(ref swagger)) => { + Some(service_manifest_latest::Value::Swagger(swagger)) => { let async_connector = self.async_connector.clone().ok_or_else(|| { error::ExecutionEngine::NotFound("Async data connector not registered".into()) })?; diff --git a/usecases/service_loader/src/lib.rs b/usecases/service_loader/src/lib.rs index 30320ac..da42842 100644 --- a/usecases/service_loader/src/lib.rs +++ b/usecases/service_loader/src/lib.rs @@ -97,8 +97,8 @@ pub fn merge( .ok_or_else(|| error::ServiceLoader::NotFound("Auth Configuration".into()))?; let oauth_config = oauth_config.mut_oauthConfig(); - if let &Some(core_entities::service::swagger_overrides::AuthOverrides::OauthConfig( - ref oauth_config_override, + if let Some(core_entities::service::swagger_overrides::AuthOverrides::OauthConfig( + oauth_config_override, )) = &overrides.authOverrides { apply_if_exists!(name, oauth_config_override => oauth_config); diff --git a/usecases/service_loader/src/loaders/openapi/mod.rs b/usecases/service_loader/src/loaders/openapi/mod.rs index 3d30de9..e761646 100644 --- a/usecases/service_loader/src/loaders/openapi/mod.rs +++ b/usecases/service_loader/src/loaders/openapi/mod.rs @@ -100,7 +100,7 @@ fn collect_operations( schemas: &mut HashMap, ) -> error::Result> { let reference = handle_reference(item, root, fetcher, cache, &mut HashSet::new())?; - let item = reference.as_ref().map_or(item, |&(_, ref item)| item); + let item = reference.as_ref().map_or(item, |(_, item)| item); let parameters: Vec = default_field(item, "parameters")?; let mut common_params = vec![]; @@ -286,7 +286,7 @@ fn handle_parameter( schemas: &mut HashMap, ) -> error::Result<()> { let reference = handle_reference(source, root, fetcher, cache, &mut HashSet::new())?; - let source = reference.as_ref().map_or(source, |&(_, ref item)| item); + let source = reference.as_ref().map_or(source, |(_, item)| item); let in_ = required_field::(source, "in")?; let in_ = match in_.as_str() { @@ -325,7 +325,7 @@ fn handle_request_body( schemas: &mut HashMap, ) -> error::Result<()> { let reference = handle_reference(source, root, fetcher, cache, &mut HashSet::new())?; - let source = reference.as_ref().map_or(source, |&(_, ref item)| item); + let source = reference.as_ref().map_or(source, |(_, item)| item); if let Some(description) = optional_field(source, "description")? { sink.description = description; @@ -353,7 +353,7 @@ fn handle_response( schemas: &mut HashMap, ) -> error::Result<()> { let reference = handle_reference(source, root, fetcher, cache, &mut HashSet::new())?; - let source = reference.as_ref().map_or(source, |&(_, ref item)| item); + let source = reference.as_ref().map_or(source, |(_, item)| item); let content: HashMap = default_field(source, "content")?; for (key, value) in &content { diff --git a/usecases/service_writer/src/lib.rs b/usecases/service_writer/src/lib.rs index ddf49ee..cdef76a 100644 --- a/usecases/service_writer/src/lib.rs +++ b/usecases/service_writer/src/lib.rs @@ -223,13 +223,13 @@ fn handle_operation( sink.insert("parameters".into(), parameters.into()); } - if let &Some(ref source_body) = &source.requestBody.0 { + if let Some(source_body) = &source.requestBody.0 { let mut request_body = serde_json::Map::new(); handle_request_body(&mut request_body, source_body)?; sink.insert("requestBody".into(), request_body.into()); } - if let &Some(ref common_responses) = &source.apiResponses.0 { + if let Some(common_responses) = &source.apiResponses.0 { let mut responses = serde_json::Map::new(); for (status, common_response) in &common_responses.apiResponses { @@ -294,7 +294,7 @@ fn handle_media( sink: &mut serde_json::Map, source: &service::MediaType, ) -> error::Result<()> { - if let &Some(ref common_schema) = &source.schema.0 { + if let Some(common_schema) = &source.schema.0 { let mut schema = serde_json::Map::new(); handle_schema(&mut schema, common_schema)?; sink.insert("schema".into(), schema.into()); @@ -327,7 +327,7 @@ fn handle_parameter( sink.insert("description".into(), source.description.clone().into()); } - if let &Some(ref common_schema) = &source.schema.0 { + if let Some(common_schema) = &source.schema.0 { let mut schema = serde_json::Map::new(); handle_schema(&mut schema, common_schema)?; sink.insert("schema".into(), schema.into()); @@ -346,10 +346,10 @@ fn handle_schema( // TODO: extract into a referece based on a flag match &source.value { - &Some(service::schema::Value::Ref(ref reference)) => { + Some(service::schema::Value::Ref(reference)) => { sink.insert("$ref".into(), reference.clone().into()); } - &Some(service::schema::Value::SchemaObject(ref schema)) => { + Some(service::schema::Value::SchemaObject(schema)) => { match schema.type_.enum_value() { Ok(service::schema_object::SchemaType::STRING) => { sink.insert("type".into(), "string".into()); @@ -387,7 +387,7 @@ fn handle_schema( Ok(service::schema_object::SchemaType::ARRAY) => { sink.insert("type".into(), "array".into()); - if let &Some(ref common_items) = &schema.items.0 { + if let Some(common_items) = &schema.items.0 { let mut items = serde_json::Map::new(); handle_schema(&mut items, common_items)?; sink.insert("items".into(), items.into()); @@ -398,13 +398,13 @@ fn handle_schema( _ => {} } } - &Some(service::schema::Value::AllOf(ref values)) => { + Some(service::schema::Value::AllOf(values)) => { handle_composed_schema(sink, "allOf", values)?; } - &Some(service::schema::Value::AnyOf(ref values)) => { + Some(service::schema::Value::AnyOf(values)) => { handle_composed_schema(sink, "anyOf", values)?; } - &Some(service::schema::Value::OneOf(ref values)) => { + Some(service::schema::Value::OneOf(values)) => { handle_composed_schema(sink, "oneOf", values)?; } _ => {} From 63b759380847a11c4130fab1ed248a463af7392e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 18:16:29 +0000 Subject: [PATCH 3/6] refactor(runners): rewrite pre-2018 ref-patterns to match ergonomics 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 Claude-Session: https://claude.ai/code/session_018VzKriyNnMHmqQ6UxPhsv2 --- runners/api_caller/src/lib.rs | 76 +++++++++++++-------------- runners/filtered_runner/src/lib.rs | 4 +- runners/python_runner/src/bindings.rs | 2 +- 3 files changed, 41 insertions(+), 41 deletions(-) diff --git a/runners/api_caller/src/lib.rs b/runners/api_caller/src/lib.rs index 1b0f0e5..aa7cf6e 100644 --- a/runners/api_caller/src/lib.rs +++ b/runners/api_caller/src/lib.rs @@ -29,11 +29,11 @@ use http::{HeaderMap, HeaderName, HeaderValue}; /// unambiguous scalar representation. fn simplify_value(value: &serde_json::Value) -> error::Result { match value { - &serde_json::Value::String(ref val) => Ok(val.to_string()), - &serde_json::Value::Bool(val) => Ok(val.to_string()), - &serde_json::Value::Number(ref val) => Ok(val.to_string()), - &serde_json::Value::Null => Ok("null".to_owned()), - &serde_json::Value::Array(_) | &serde_json::Value::Object(_) => { + serde_json::Value::String(val) => Ok(val.to_string()), + serde_json::Value::Bool(val) => Ok(val.to_string()), + serde_json::Value::Number(val) => Ok(val.to_string()), + serde_json::Value::Null => Ok("null".to_owned()), + serde_json::Value::Array(_) | serde_json::Value::Object(_) => { Err(error::APICaller::SimpleValueAssertion) } } @@ -60,9 +60,9 @@ fn find_results<'item>( result: &'item serde_json::Value, pagination_config: &Option, ) -> error::Result<&'item serde_json::Value> { - let result = if let &Some(ref pagination) = pagination_config { + let result = if let Some(pagination) = pagination_config { match pagination { - &core_entities::service::pagination::Value::PageOffset(ref page_offset) => { + core_entities::service::pagination::Value::PageOffset(page_offset) => { let path = page_offset.resultsPath.jmesPath(); let path = path .strip_prefix(constants::RESPONSE_BODY_PREFIX) @@ -81,7 +81,7 @@ fn find_results<'item>( path.resolve(result)? } } - &core_entities::service::pagination::Value::MultiCursor(ref cursor) => { + core_entities::service::pagination::Value::MultiCursor(cursor) => { let path = cursor.resultsPath.jmesPath(); let path = path .strip_prefix(constants::RESPONSE_BODY_PREFIX) @@ -100,7 +100,7 @@ fn find_results<'item>( path.resolve(result)? } } - &core_entities::service::pagination::Value::Offset(ref offset) => { + core_entities::service::pagination::Value::Offset(offset) => { let path = offset.resultsPath.jmesPath(); let path = path .strip_prefix(constants::RESPONSE_BODY_PREFIX) @@ -119,7 +119,7 @@ fn find_results<'item>( path.resolve(result)? } } - &core_entities::service::pagination::Value::Unpaginated(ref unpaginated) => { + core_entities::service::pagination::Value::Unpaginated(unpaginated) => { let path = unpaginated.resultsPath.jmesPath(); let path = path .strip_prefix(constants::RESPONSE_BODY_PREFIX) @@ -138,7 +138,7 @@ fn find_results<'item>( path.resolve(result)? } } - &core_entities::service::pagination::Value::NextUrl(_) | &_ => result, + core_entities::service::pagination::Value::NextUrl(_) | _ => result, } } else { result @@ -216,7 +216,7 @@ impl APICallState { builder = builder.headers(headers); - if let &Some(ref body) = &self.body { + if let Some(body) = &self.body { log.write_all(format!("\n{}\n", serde_json::to_string_pretty(body)?).as_bytes())?; builder = builder.json(body); } else { @@ -295,7 +295,7 @@ impl APICallState { builder = builder.headers(headers); - if let &Some(ref body) = &self.body { + if let Some(body) = &self.body { log.write_all(format!("\n{}\n", serde_json::to_string_pretty(body)?).as_bytes())?; builder = builder.json(body); } else { @@ -591,9 +591,9 @@ impl APICallState { current_page: i32, parameters: &[Parameter], ) -> error::Result { - let requested = if let &Some(ref pagination) = pagination_config { + let requested = if let Some(pagination) = pagination_config { match pagination { - &core_entities::service::pagination::Value::PageOffset(ref page_offset) => { + core_entities::service::pagination::Value::PageOffset(page_offset) => { let current_page = page_offset .startPage .value @@ -614,7 +614,7 @@ impl APICallState { max_limit } - &core_entities::service::pagination::Value::MultiCursor(ref cursor) => { + core_entities::service::pagination::Value::MultiCursor(cursor) => { let max_limit = cursor.maxLimit.value; self.apply_runtime_expression( &cursor.limitParam, @@ -649,7 +649,7 @@ impl APICallState { max_limit } - &core_entities::service::pagination::Value::Offset(ref offset) => { + core_entities::service::pagination::Value::Offset(offset) => { let max_limit = offset.maxLimit.value; self.apply_runtime_expression( @@ -665,7 +665,7 @@ impl APICallState { max_limit } - &pagination::Value::NextUrl(_) | &pagination::Value::Unpaginated(_) | &_ => 0_i32, + pagination::Value::NextUrl(_) | pagination::Value::Unpaginated(_) | _ => 0_i32, } } else { 0_i32 @@ -783,15 +783,15 @@ impl APICaller { let total_limit: i32 = total_limit .and_then(|value| match value { - &serde_json::Value::Number(ref n) if n.is_f64() => n.as_f64().map(|n| n as i32), - &serde_json::Value::Number(ref n) if n.is_i64() => n.as_i64().map(|n| n as i32), - &serde_json::Value::Number(ref n) if n.is_u64() => n.as_u64().map(|n| n as i32), - &serde_json::Value::Null - | &serde_json::Value::Bool(_) - | &serde_json::Value::Number(_) - | &serde_json::Value::String(_) - | &serde_json::Value::Array(_) - | &serde_json::Value::Object(_) => None, + serde_json::Value::Number(n) if n.is_f64() => n.as_f64().map(|n| n as i32), + serde_json::Value::Number(n) if n.is_i64() => n.as_i64().map(|n| n as i32), + serde_json::Value::Number(n) if n.is_u64() => n.as_u64().map(|n| n as i32), + serde_json::Value::Null + | serde_json::Value::Bool(_) + | serde_json::Value::Number(_) + | serde_json::Value::String(_) + | serde_json::Value::Array(_) + | serde_json::Value::Object(_) => None, }) .unwrap_or(constants::DEFAULT_LIMIT); @@ -833,7 +833,7 @@ impl APICaller { let actual_result = find_results(&result, &operation.pagination.value)?; // Determine how many items we got in a request - let current_size = if let &serde_json::Value::Array(ref arr) = actual_result { + let current_size = if let serde_json::Value::Array(arr) = actual_result { i32::try_from(arr.len())? } else { 1_i32 @@ -954,15 +954,15 @@ impl AsyncAPICaller { let total_limit: i32 = total_limit .and_then(|value| match value { - &serde_json::Value::Number(ref n) if n.is_f64() => n.as_f64().map(|n| n as i32), - &serde_json::Value::Number(ref n) if n.is_i64() => n.as_i64().map(|n| n as i32), - &serde_json::Value::Number(ref n) if n.is_u64() => n.as_u64().map(|n| n as i32), - &serde_json::Value::Null - | &serde_json::Value::Bool(_) - | &serde_json::Value::Number(_) - | &serde_json::Value::String(_) - | &serde_json::Value::Array(_) - | &serde_json::Value::Object(_) => None, + serde_json::Value::Number(n) if n.is_f64() => n.as_f64().map(|n| n as i32), + serde_json::Value::Number(n) if n.is_i64() => n.as_i64().map(|n| n as i32), + serde_json::Value::Number(n) if n.is_u64() => n.as_u64().map(|n| n as i32), + serde_json::Value::Null + | serde_json::Value::Bool(_) + | serde_json::Value::Number(_) + | serde_json::Value::String(_) + | serde_json::Value::Array(_) + | serde_json::Value::Object(_) => None, }) .unwrap_or(constants::DEFAULT_LIMIT); @@ -1006,7 +1006,7 @@ impl AsyncAPICaller { let actual_result = find_results(&result, &operation.pagination.value)?; // Determine how many items we got in a request - let current_size = if let &serde_json::Value::Array(ref arr) = actual_result { + let current_size = if let serde_json::Value::Array(arr) = actual_result { i32::try_from(arr.len())? } else { 1_i32 diff --git a/runners/filtered_runner/src/lib.rs b/runners/filtered_runner/src/lib.rs index 9a2c974..1d1f95e 100644 --- a/runners/filtered_runner/src/lib.rs +++ b/runners/filtered_runner/src/lib.rs @@ -61,7 +61,7 @@ impl APIWrapper { let mut input = serde_json::Value::Object(serde_json::Map::new()); for input_param in &manifest.inputs { - if let &Some(ref param) = &input_param.param.0 { + if let Some(param) = &input_param.param.0 { let param = ¶m.name; if let Some(param) = params.get(param) { let path: Vec<_> = input_param.apiParamName.split('.').collect(); @@ -141,7 +141,7 @@ fn traverse_map( value: serde_json::Value, ) -> error::Result<()> { if let Some(next) = parts.first() { - if let &mut serde_json::Value::Object(ref mut current) = current { + if let serde_json::Value::Object(current) = current { let key = (*next).to_owned(); let child = current .entry(key) diff --git a/runners/python_runner/src/bindings.rs b/runners/python_runner/src/bindings.rs index 84a5384..1ea7ee0 100644 --- a/runners/python_runner/src/bindings.rs +++ b/runners/python_runner/src/bindings.rs @@ -149,7 +149,7 @@ impl Task { let mut params = converters::from_py(self.params.as_ref(py))?; - if let &mut Value::Object(ref mut map) = &mut params { + if let Value::Object(map) = &mut params { map.insert("input_results".into(), result); } else { // TODO: Verify this functionality From 46ee4e012cfeaa0030fcc0030547033cdba008cd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 18:24:39 +0000 Subject: [PATCH 4/6] 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| { From 6234fbe106d232b6c3d7f3e0e81e040cbed01e23 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 22:17:44 +0000 Subject: [PATCH 5/6] refactor(execution_engine,service_loader): split the two too_many_lines 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 Claude-Session: https://claude.ai/code/session_018VzKriyNnMHmqQ6UxPhsv2 --- usecases/execution_engine/src/lib.rs | 327 +++++++++++------- .../service_loader/src/loaders/openapi/mod.rs | 66 ++-- 2 files changed, 238 insertions(+), 155 deletions(-) diff --git a/usecases/execution_engine/src/lib.rs b/usecases/execution_engine/src/lib.rs index a5ec677..2cc5a4b 100644 --- a/usecases/execution_engine/src/lib.rs +++ b/usecases/execution_engine/src/lib.rs @@ -182,10 +182,6 @@ impl Engine { options: Value, context: &EngineInputContext, ) -> error::Result { - // SimpleCode -> CodeRunner - // ApiWrapper -> FilteredRunner - // ScriptedAction -> ScriptRunner - if identifier == "$input" { if let Some(input_handler) = &self.input_handler { return input_handler.run(params, context); @@ -209,127 +205,220 @@ impl Engine { let manifest = service.manifest.v2(); let result = match &manifest.value { - Some(service_manifest_latest::Value::Swagger(swagger)) => { - if let Some(connector) = &self.connector { - let api = &service.commonApi; - let creds = credentials.as_ref(); - - let bundle = DataConnectorBundle { - manifest: swagger, - api, - creds, - }; - connector.run( - service_name, - operation_name, - &bundle, - params, - options, - context, - ) - } else { - Err(error::ExecutionEngine::NotFound( - "Data connector runner".into(), - )) - } - } - Some(service_manifest_latest::Value::Action(action)) => { - let operation = action - .operations - .iter() - .find(|item| item.id == *operation_name); - if let Some(operation) = operation { - let operation = operation.function(); - - let path = format!("{}/{}", action.source, operation.js()); - - let source = service - .resources - .iter() - .find(|item| item.relativePath == path) - .ok_or(error::ExecutionEngine::NotFound(format!( - "Source file for {service_name}.{operation_name}" - )))?; - - if let Some(code_runner) = self.code_runners.get(&operation.lang) { - self.log(identifier, "ACTION", "STARTED")?; - let result = code_runner.run( - service_name, - operation_name, - &source.content, - params, - context, - )?; - self.log(identifier, "ACTION", "COMPLETED")?; - - Ok(result) - } else { - Err(error::ExecutionEngine::NotFound(format!( - "Code Runner for language {} not found", - operation.lang - ))) - } - } else { - Err(error::ExecutionEngine::NotFound(format!( - "Action operation {operation_name}" - ))) - } - } - Some(service_manifest_latest::Value::ApiWrapped(api_wrapped)) => { - if let Some(filtered_runner) = &self.filtered_runner { - self.log(identifier, "API_WRAPPED", "STARTED")?; - let result = filtered_runner.run( - service_name, - operation_name, - api_wrapped, - params, - context, - )?; - self.log(identifier, "API_WRAPPED", "COMPLETED")?; - - Ok(result) - } else { - Err(error::ExecutionEngine::NotFound( - "API Wrapper runner not found".into(), - )) - } - } - Some(service_manifest_latest::Value::SimpleCode(simple_code)) => { - match simple_code.code.language.enum_value() { - Ok(Language::PYTHON) => self.dispatch_code_runner( - identifier, - service_name, - operation_name, - "python", - simple_code.code.codeString(), - params, - context, - ), - Ok(Language::JAVASCRIPT) => self.dispatch_code_runner( - identifier, - service_name, - operation_name, - "js", - simple_code.code.codeString(), - params, - context, - ), - // LUA is deliberately not dispatched to here - see #73: - // `Workflow`-kind manifests (via `WorkflowRunner`) are - // the replacement for Lua `SimpleCode` operations, not - // a second parallel Lua execution path through this - // arm. The `LUA` enum variant itself stays defined - // (harmless, and a smaller footprint than removing a - // wire enum value), it's just unreachable here now. - _ => Err(error::ExecutionEngine::NotFound("Unknown language".into())), - } - } + Some(service_manifest_latest::Value::Swagger(swagger)) => self.dispatch_swagger( + service_name, + operation_name, + service, + swagger, + credentials, + params, + options, + context, + ), + Some(service_manifest_latest::Value::Action(action)) => self.dispatch_action( + identifier, + service_name, + operation_name, + service, + action, + params, + context, + ), + Some(service_manifest_latest::Value::ApiWrapped(api_wrapped)) => self + .dispatch_api_wrapped( + identifier, + service_name, + operation_name, + api_wrapped, + params, + context, + ), + Some(service_manifest_latest::Value::SimpleCode(simple_code)) => self + .dispatch_simple_code( + identifier, + service_name, + operation_name, + simple_code, + params, + context, + ), _ => Err(error::ExecutionEngine::Unimplemented("API Runner".into())), }?; Ok(wrap_result(result, context.raw_response)) } + /// Dispatches a `Swagger`-kind manifest to the registered + /// [`DataConnectionRunner`] - the `Swagger` arm of [`Engine::run`]'s + /// dispatch. + #[allow( + clippy::too_many_arguments, + reason = "each argument is a distinct dispatch input passed through from Engine::run; \ + see dispatch_code_runner's doc comment above for the same reasoning (#16)" + )] + fn dispatch_swagger( + &self, + service_name: &str, + operation_name: &str, + service: &core_entities::service::versioned_service_tree::V1, + swagger: &core_entities::service::SwaggerService, + credentials: Option, + params: Value, + options: Value, + context: &EngineInputContext, + ) -> error::Result { + if let Some(connector) = &self.connector { + let api = &service.commonApi; + let creds = credentials.as_ref(); + + let bundle = DataConnectorBundle { + manifest: swagger, + api, + creds, + }; + connector.run( + service_name, + operation_name, + &bundle, + params, + options, + context, + ) + } else { + Err(error::ExecutionEngine::NotFound( + "Data connector runner".into(), + )) + } + } + + /// Dispatches an `Action`-kind manifest to the registered [`CodeRunner`] + /// for the resolved operation's language - the `Action` arm of + /// [`Engine::run`]'s dispatch. + #[allow( + clippy::too_many_arguments, + reason = "each argument is a distinct dispatch input passed through from Engine::run; \ + see dispatch_code_runner's doc comment above for the same reasoning (#16)" + )] + fn dispatch_action( + &self, + identifier: &str, + service_name: &str, + operation_name: &str, + service: &core_entities::service::versioned_service_tree::V1, + action: &core_entities::service::ActionService, + params: Value, + context: &EngineInputContext, + ) -> error::Result { + let operation = action + .operations + .iter() + .find(|item| item.id == *operation_name); + if let Some(operation) = operation { + let operation = operation.function(); + + let path = format!("{}/{}", action.source, operation.js()); + + let source = service + .resources + .iter() + .find(|item| item.relativePath == path) + .ok_or(error::ExecutionEngine::NotFound(format!( + "Source file for {service_name}.{operation_name}" + )))?; + + if let Some(code_runner) = self.code_runners.get(&operation.lang) { + self.log(identifier, "ACTION", "STARTED")?; + let result = code_runner.run( + service_name, + operation_name, + &source.content, + params, + context, + )?; + self.log(identifier, "ACTION", "COMPLETED")?; + + Ok(result) + } else { + Err(error::ExecutionEngine::NotFound(format!( + "Code Runner for language {} not found", + operation.lang + ))) + } + } else { + Err(error::ExecutionEngine::NotFound(format!( + "Action operation {operation_name}" + ))) + } + } + + /// Dispatches an `ApiWrapped`-kind manifest to the registered + /// [`FilteredRunner`] - the `ApiWrapped` arm of [`Engine::run`]'s + /// dispatch. + fn dispatch_api_wrapped( + &self, + identifier: &str, + service_name: &str, + operation_name: &str, + api_wrapped: &core_entities::service::APIWrappedService, + params: Value, + context: &EngineInputContext, + ) -> error::Result { + if let Some(filtered_runner) = &self.filtered_runner { + self.log(identifier, "API_WRAPPED", "STARTED")?; + let result = + filtered_runner.run(service_name, operation_name, api_wrapped, params, context)?; + self.log(identifier, "API_WRAPPED", "COMPLETED")?; + + Ok(result) + } else { + Err(error::ExecutionEngine::NotFound( + "API Wrapper runner not found".into(), + )) + } + } + + /// Dispatches a `SimpleCode`-kind manifest to the [`CodeRunner`] + /// registered for its `language` - the `SimpleCode` arm of + /// [`Engine::run`]'s dispatch. + fn dispatch_simple_code( + &self, + identifier: &str, + service_name: &str, + operation_name: &str, + simple_code: &core_entities::service::SimpleCodeService, + params: Value, + context: &EngineInputContext, + ) -> error::Result { + match simple_code.code.language.enum_value() { + Ok(Language::PYTHON) => self.dispatch_code_runner( + identifier, + service_name, + operation_name, + "python", + simple_code.code.codeString(), + params, + context, + ), + Ok(Language::JAVASCRIPT) => self.dispatch_code_runner( + identifier, + service_name, + operation_name, + "js", + simple_code.code.codeString(), + params, + context, + ), + // LUA is deliberately not dispatched to here - see #73: + // `Workflow`-kind manifests (via `WorkflowRunner`) are + // the replacement for Lua `SimpleCode` operations, not + // a second parallel Lua execution path through this + // arm. The `LUA` enum variant itself stays defined + // (harmless, and a smaller footprint than removing a + // wire enum value), it's just unreachable here now. + _ => Err(error::ExecutionEngine::NotFound("Unknown language".into())), + } + } + /// Resolves `identifier` against a `Workflow`-kind manifest, returning /// the fully **owned** pieces (`service_name`, `operation_name`, the /// manifest's cloned `WorkflowService`, and the registered diff --git a/usecases/service_loader/src/loaders/openapi/mod.rs b/usecases/service_loader/src/loaders/openapi/mod.rs index e761646..88b6baa 100644 --- a/usecases/service_loader/src/loaders/openapi/mod.rs +++ b/usecases/service_loader/src/loaders/openapi/mod.rs @@ -478,54 +478,21 @@ fn handle_schema( _ => {} } } else { - let result = optional_field::>(source, "oneOf")?; - if let Some(result) = result { - let schema: error::Result> = result - .iter() - .map(|value| { - let mut common_schema = service::Schema::new(); - handle_schema(value, &mut common_schema, root, fetcher, cache, schemas)?; - Ok(common_schema) - }) - .collect(); - let schema = schema?; - + if let Some(schema) = resolve_schema_list(source, "oneOf", root, fetcher, cache, schemas)? { sink.set_oneOf(service::ComposedSchema { schema, ..Default::default() }); } - let result = optional_field::>(source, "anyOf")?; - if let Some(result) = result { - let schema: error::Result> = result - .iter() - .map(|value| { - let mut common_schema = service::Schema::new(); - handle_schema(value, &mut common_schema, root, fetcher, cache, schemas)?; - Ok(common_schema) - }) - .collect(); - let schema = schema?; - + if let Some(schema) = resolve_schema_list(source, "anyOf", root, fetcher, cache, schemas)? { sink.set_anyOf(service::ComposedSchema { schema, ..Default::default() }); } - let result = optional_field::>(source, "allOf")?; - if let Some(result) = result { - let schema: error::Result> = result - .iter() - .map(|value| { - let mut common_schema = service::Schema::new(); - handle_schema(value, &mut common_schema, root, fetcher, cache, schemas)?; - Ok(common_schema) - }) - .collect(); - let schema = schema?; - + if let Some(schema) = resolve_schema_list(source, "allOf", root, fetcher, cache, schemas)? { sink.set_allOf(service::ComposedSchema { schema, ..Default::default() @@ -536,6 +503,33 @@ fn handle_schema( Ok(()) } +/// Resolves `source`'s `field` (`"oneOf"`/`"anyOf"`/`"allOf"`) as a list of +/// schemas, recursively converting each branch via [`handle_schema`]. +/// Returns `None` if `field` is absent - the shared body of +/// [`handle_schema`]'s three composition-field arms. +fn resolve_schema_list( + source: &serde_json::Value, + field: &str, + root: &serde_json::Value, + fetcher: &dyn Fetcher, + cache: &mut HashMap, + schemas: &mut HashMap, +) -> error::Result>> { + let Some(values) = optional_field::>(source, field)? else { + return Ok(None); + }; + + values + .iter() + .map(|value| { + let mut common_schema = service::Schema::new(); + handle_schema(value, &mut common_schema, root, fetcher, cache, schemas)?; + Ok(common_schema) + }) + .collect::>>() + .map(Some) +} + #[cfg(test)] mod test { use core::cell::RefCell; From 7d616619038926aea288a09ef177d6c9612a3c06 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 23:02:07 +0000 Subject: [PATCH 6/6] refactor(api_caller): audit and fix as-casts in pagination limit parsing Closes #87 (Phase 4 of #1). Every `as`-cast site in api_caller turned out to be the same duplicated total_limit resolver (both APICaller::run_internal and AsyncAPICaller::run_internal): f64/i64/u64 -> i32 for options["limit"]. - i64/u64 -> i32 now use i32::try_from(...).ok(), falling back to DEFAULT_LIMIT on overflow instead of silently wrapping (e.g. a u64 limit of 2^32 previously wrapped to 0, which this code treats as "no cap enforced" - a real correctness bug, not just a lint nag). - f64 -> i32 keeps the `as` cast with a narrow, reasoned allow: float-to-int `as` casts saturate rather than wrap (defined behavior since Rust 1.45), so this one was already safe. - Extracted the duplicated resolver into resolve_total_limit(), which also fixed a too_many_lines regression the fix introduced and removed the duplication between the two run_internal methods. - Removed the crate-level #![allow(clippy::as_conversions, clippy::cast_possible_truncation)]; api_caller now lints like every other crate in the workspace. - Added regression tests confirming the old wraparound behavior for both the i64 and u64 arms (verified failing against the pre-fix casts). --- runners/api_caller/src/lib.rs | 121 +++++++++++++++++++++++----------- 1 file changed, 83 insertions(+), 38 deletions(-) diff --git a/runners/api_caller/src/lib.rs b/runners/api_caller/src/lib.rs index aa7cf6e..09881cf 100644 --- a/runners/api_caller/src/lib.rs +++ b/runners/api_caller/src/lib.rs @@ -1,11 +1,3 @@ -#![allow( - clippy::as_conversions, - clippy::cast_possible_truncation, - reason = "pagination limit/offset casts between i32/usize/u64 are unaudited; \ - tracked as a dedicated numeric-safety follow-up to issue #1, not \ - rushed into this lint-hygiene pass" -)] - //! A [`DataConnectionRunner`] adapter that resolves an operation's request //! (method, endpoint, params, auth) and executes it over HTTP, handling //! pagination across multiple requests when configured. @@ -50,6 +42,38 @@ where .collect() } +/// Resolves the `options.limit` pagination cap from JSON to an `i32`, +/// falling back to [`constants::DEFAULT_LIMIT`] when absent, non-numeric, or +/// out of `i32`'s range (rather than silently wrapping to an unrelated +/// value). +fn resolve_total_limit(options: &serde_json::Value) -> i32 { + options + .get("limit") + .and_then(|value| match value { + #[allow( + clippy::cast_possible_truncation, + reason = "float-to-int `as` casts saturate rather than wrap (defined \ + behavior since Rust 1.45): an out-of-range or NaN limit clamps \ + to i32::MAX/i32::MIN/0, and a fractional limit truncates toward \ + zero — both are the intended behavior for a pagination limit" + )] + serde_json::Value::Number(n) if n.is_f64() => n.as_f64().map(|n| n as i32), + serde_json::Value::Number(n) if n.is_i64() => { + n.as_i64().and_then(|n| i32::try_from(n).ok()) + } + serde_json::Value::Number(n) if n.is_u64() => { + n.as_u64().and_then(|n| i32::try_from(n).ok()) + } + serde_json::Value::Null + | serde_json::Value::Bool(_) + | serde_json::Value::Number(_) + | serde_json::Value::String(_) + | serde_json::Value::Array(_) + | serde_json::Value::Object(_) => None, + }) + .unwrap_or(constants::DEFAULT_LIMIT) +} + /// Extracts the paginated results from a raw response, by resolving the /// configured pagination strategy's `resultsPath` (stripped of its /// `$response.body#` runtime-expression prefix) as a JSON pointer into @@ -779,21 +803,7 @@ impl APICaller { .get(operation_name) .ok_or_else(|| error::APICaller::OperationNotFound(operation_name.into()))?; - let total_limit = options.get("limit"); - - let total_limit: i32 = total_limit - .and_then(|value| match value { - serde_json::Value::Number(n) if n.is_f64() => n.as_f64().map(|n| n as i32), - serde_json::Value::Number(n) if n.is_i64() => n.as_i64().map(|n| n as i32), - serde_json::Value::Number(n) if n.is_u64() => n.as_u64().map(|n| n as i32), - serde_json::Value::Null - | serde_json::Value::Bool(_) - | serde_json::Value::Number(_) - | serde_json::Value::String(_) - | serde_json::Value::Array(_) - | serde_json::Value::Object(_) => None, - }) - .unwrap_or(constants::DEFAULT_LIMIT); + let total_limit: i32 = resolve_total_limit(options); let mut total: i32 = 0; let mut current_page: i32 = 0; @@ -950,21 +960,7 @@ impl AsyncAPICaller { .get(operation_name) .ok_or_else(|| error::APICaller::OperationNotFound(operation_name.into()))?; - let total_limit = options.get("limit"); - - let total_limit: i32 = total_limit - .and_then(|value| match value { - serde_json::Value::Number(n) if n.is_f64() => n.as_f64().map(|n| n as i32), - serde_json::Value::Number(n) if n.is_i64() => n.as_i64().map(|n| n as i32), - serde_json::Value::Number(n) if n.is_u64() => n.as_u64().map(|n| n as i32), - serde_json::Value::Null - | serde_json::Value::Bool(_) - | serde_json::Value::Number(_) - | serde_json::Value::String(_) - | serde_json::Value::Array(_) - | serde_json::Value::Object(_) => None, - }) - .unwrap_or(constants::DEFAULT_LIMIT); + let total_limit: i32 = resolve_total_limit(options); let mut total: i32 = 0; let mut current_page: i32 = 0; @@ -1292,4 +1288,53 @@ mod tests { "expected an unrecognized auth type to error instead of silently skipping auth, got {result:?}" ); } + + #[test] + fn resolve_total_limit_passes_through_in_range_numbers() { + assert_eq!(resolve_total_limit(&serde_json::json!({ "limit": 42 })), 42); + assert_eq!( + resolve_total_limit(&serde_json::json!({ "limit": 42.9 })), + 42 + ); + assert_eq!( + resolve_total_limit(&serde_json::json!({ "limit": 1_000_000_000_u64 })), + 1_000_000_000 + ); + } + + #[test] + fn resolve_total_limit_falls_back_to_default_when_absent_or_non_numeric() { + assert_eq!( + resolve_total_limit(&serde_json::json!({})), + constants::DEFAULT_LIMIT + ); + assert_eq!( + resolve_total_limit(&serde_json::json!({ "limit": "not a number" })), + constants::DEFAULT_LIMIT + ); + } + + #[test] + fn resolve_total_limit_falls_back_to_default_instead_of_wrapping_an_out_of_range_i64() { + // i64::from(i32::MAX) + 1 wraps to i32::MIN under `as i32`, which + // would corrupt the pagination limit into a large negative number + // instead of safely falling back to the default. + let oversized = i64::from(i32::MAX) + 1; + assert_eq!( + resolve_total_limit(&serde_json::json!({ "limit": oversized })), + constants::DEFAULT_LIMIT + ); + } + + #[test] + fn resolve_total_limit_falls_back_to_default_instead_of_wrapping_an_out_of_range_u64() { + // 2^32 + 1 wraps to 1 under `as i32`, which would be silently + // misread as a valid (tiny) limit instead of falling back to the + // configured default. + let oversized = u64::from(u32::MAX) + 2; + assert_eq!( + resolve_total_limit(&serde_json::json!({ "limit": oversized })), + constants::DEFAULT_LIMIT + ); + } }