Disallow expect via lints (#865)
Adds `expect()` as a denied lint. Same deal applies with `unwrap()` where we now need to put `#[expect(...` on ones that we legit want. Took care to enable `expect()` in test contexts. # Tests ``` cargo fmt cargo clippy --all-features --all-targets --no-deps -- -D warnings cargo test ```
This commit is contained in:
@@ -26,6 +26,7 @@ edition = "2024"
|
|||||||
rust = { }
|
rust = { }
|
||||||
|
|
||||||
[workspace.lints.clippy]
|
[workspace.lints.clippy]
|
||||||
|
expect_used = "deny"
|
||||||
unwrap_used = "deny"
|
unwrap_used = "deny"
|
||||||
|
|
||||||
[profile.release]
|
[profile.release]
|
||||||
|
|||||||
@@ -212,7 +212,9 @@ fn extract_heredoc_body_from_apply_patch_command(src: &str) -> anyhow::Result<St
|
|||||||
|
|
||||||
let lang = BASH.into();
|
let lang = BASH.into();
|
||||||
let mut parser = Parser::new();
|
let mut parser = Parser::new();
|
||||||
parser.set_language(&lang).expect("load bash grammar");
|
parser
|
||||||
|
.set_language(&lang)
|
||||||
|
.context("failed to load bash grammar")?;
|
||||||
let tree = parser
|
let tree = parser
|
||||||
.parse(src, None)
|
.parse(src, None)
|
||||||
.ok_or_else(|| anyhow::anyhow!("failed to parse patch into AST"))?;
|
.ok_or_else(|| anyhow::anyhow!("failed to parse patch into AST"))?;
|
||||||
@@ -225,7 +227,7 @@ fn extract_heredoc_body_from_apply_patch_command(src: &str) -> anyhow::Result<St
|
|||||||
if node.kind() == "heredoc_body" {
|
if node.kind() == "heredoc_body" {
|
||||||
let text = node
|
let text = node
|
||||||
.utf8_text(bytes)
|
.utf8_text(bytes)
|
||||||
.with_context(|| "failed to interpret heredoc body as UTF-8")?;
|
.context("failed to interpret heredoc body as UTF-8")?;
|
||||||
return Ok(text.trim_end_matches('\n').to_owned());
|
return Ok(text.trim_end_matches('\n').to_owned());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -81,8 +81,13 @@ pub async fn run_main(_opts: ProtoCli) -> anyhow::Result<()> {
|
|||||||
};
|
};
|
||||||
match event {
|
match event {
|
||||||
Ok(event) => {
|
Ok(event) => {
|
||||||
let event_str =
|
let event_str = match serde_json::to_string(&event) {
|
||||||
serde_json::to_string(&event).expect("JSON serialization failed");
|
Ok(s) => s,
|
||||||
|
Err(e) => {
|
||||||
|
error!("Failed to serialize event: {e}");
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
};
|
||||||
println!("{event_str}");
|
println!("{event_str}");
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ use crate::client_common::ResponseEvent;
|
|||||||
use crate::client_common::ResponseStream;
|
use crate::client_common::ResponseStream;
|
||||||
use crate::client_common::Summary;
|
use crate::client_common::Summary;
|
||||||
use crate::error::CodexErr;
|
use crate::error::CodexErr;
|
||||||
|
use crate::error::EnvVarError;
|
||||||
use crate::error::Result;
|
use crate::error::Result;
|
||||||
use crate::flags::CODEX_RS_SSE_FIXTURE;
|
use crate::flags::CODEX_RS_SSE_FIXTURE;
|
||||||
use crate::flags::OPENAI_REQUEST_MAX_RETRIES;
|
use crate::flags::OPENAI_REQUEST_MAX_RETRIES;
|
||||||
@@ -151,10 +152,10 @@ impl ModelClient {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Assemble tool list: built-in tools + any extra tools from the prompt.
|
// Assemble tool list: built-in tools + any extra tools from the prompt.
|
||||||
let mut tools_json: Vec<serde_json::Value> = DEFAULT_TOOLS
|
let mut tools_json = Vec::with_capacity(DEFAULT_TOOLS.len() + prompt.extra_tools.len());
|
||||||
.iter()
|
for t in DEFAULT_TOOLS.iter() {
|
||||||
.map(|t| serde_json::to_value(t).expect("serialize builtin tool"))
|
tools_json.push(serde_json::to_value(t)?);
|
||||||
.collect();
|
}
|
||||||
tools_json.extend(
|
tools_json.extend(
|
||||||
prompt
|
prompt
|
||||||
.extra_tools
|
.extra_tools
|
||||||
@@ -191,10 +192,12 @@ impl ModelClient {
|
|||||||
loop {
|
loop {
|
||||||
attempt += 1;
|
attempt += 1;
|
||||||
|
|
||||||
let api_key = self
|
let api_key = self.provider.api_key()?.ok_or_else(|| {
|
||||||
.provider
|
CodexErr::EnvVar(EnvVarError {
|
||||||
.api_key()?
|
var: self.provider.env_key.clone().unwrap_or_default(),
|
||||||
.expect("Repsones API requires an API key");
|
instructions: None,
|
||||||
|
})
|
||||||
|
})?;
|
||||||
let res = self
|
let res = self
|
||||||
.client
|
.client
|
||||||
.post(&url)
|
.post(&url)
|
||||||
|
|||||||
@@ -1654,6 +1654,7 @@ fn format_exec_output(output: &str, exit_code: i32, duration: std::time::Duratio
|
|||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
|
#[expect(clippy::expect_used)]
|
||||||
serde_json::to_string(&payload).expect("serialize ExecOutput")
|
serde_json::to_string(&payload).expect("serialize ExecOutput")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -246,27 +246,30 @@ impl Config {
|
|||||||
})?
|
})?
|
||||||
.clone();
|
.clone();
|
||||||
|
|
||||||
|
let resolved_cwd = {
|
||||||
|
use std::env;
|
||||||
|
|
||||||
|
match cwd {
|
||||||
|
None => {
|
||||||
|
tracing::info!("cwd not set, using current dir");
|
||||||
|
env::current_dir()?
|
||||||
|
}
|
||||||
|
Some(p) if p.is_absolute() => p,
|
||||||
|
Some(p) => {
|
||||||
|
// Resolve relative path against the current working directory.
|
||||||
|
tracing::info!("cwd is relative, resolving against current dir");
|
||||||
|
let mut current = env::current_dir()?;
|
||||||
|
current.push(p);
|
||||||
|
current
|
||||||
|
}
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
let config = Self {
|
let config = Self {
|
||||||
model: model.or(cfg.model).unwrap_or_else(default_model),
|
model: model.or(cfg.model).unwrap_or_else(default_model),
|
||||||
model_provider_id,
|
model_provider_id,
|
||||||
model_provider,
|
model_provider,
|
||||||
cwd: cwd.map_or_else(
|
cwd: resolved_cwd,
|
||||||
|| {
|
|
||||||
tracing::info!("cwd not set, using current dir");
|
|
||||||
std::env::current_dir().expect("cannot determine current dir")
|
|
||||||
},
|
|
||||||
|p| {
|
|
||||||
if p.is_absolute() {
|
|
||||||
p
|
|
||||||
} else {
|
|
||||||
// Resolve relative paths against the current working directory.
|
|
||||||
tracing::info!("cwd is relative, resolving against current dir");
|
|
||||||
let mut cwd = std::env::current_dir().expect("cannot determine cwd");
|
|
||||||
cwd.push(p);
|
|
||||||
cwd
|
|
||||||
}
|
|
||||||
},
|
|
||||||
),
|
|
||||||
approval_policy: approval_policy
|
approval_policy: approval_policy
|
||||||
.or(cfg.approval_policy)
|
.or(cfg.approval_policy)
|
||||||
.unwrap_or_else(AskForApproval::default),
|
.unwrap_or_else(AskForApproval::default),
|
||||||
@@ -292,6 +295,7 @@ impl Config {
|
|||||||
/// Meant to be used exclusively for tests: `load_with_overrides()` should
|
/// Meant to be used exclusively for tests: `load_with_overrides()` should
|
||||||
/// be used in all other cases.
|
/// be used in all other cases.
|
||||||
pub fn load_default_config_for_test() -> Self {
|
pub fn load_default_config_for_test() -> Self {
|
||||||
|
#[expect(clippy::expect_used)]
|
||||||
Self::load_from_base_config_with_overrides(
|
Self::load_from_base_config_with_overrides(
|
||||||
ConfigToml::default(),
|
ConfigToml::default(),
|
||||||
ConfigOverrides::default(),
|
ConfigOverrides::default(),
|
||||||
@@ -371,6 +375,7 @@ pub fn parse_sandbox_permission_with_base_path(
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
|
#![allow(clippy::expect_used, clippy::unwrap_used)]
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
/// Verify that the `sandbox_permissions` field on `ConfigToml` correctly
|
/// Verify that the `sandbox_permissions` field on `ConfigToml` correctly
|
||||||
|
|||||||
@@ -344,13 +344,30 @@ pub(crate) async fn consume_truncated_output(
|
|||||||
ctrl_c: Arc<Notify>,
|
ctrl_c: Arc<Notify>,
|
||||||
timeout_ms: Option<u64>,
|
timeout_ms: Option<u64>,
|
||||||
) -> Result<RawExecToolCallOutput> {
|
) -> Result<RawExecToolCallOutput> {
|
||||||
|
// Both stdout and stderr were configured with `Stdio::piped()`
|
||||||
|
// above, therefore `take()` should normally return `Some`. If it doesn't
|
||||||
|
// we treat it as an exceptional I/O error
|
||||||
|
|
||||||
|
let stdout_reader = child.stdout.take().ok_or_else(|| {
|
||||||
|
CodexErr::Io(io::Error::new(
|
||||||
|
io::ErrorKind::Other,
|
||||||
|
"stdout pipe was unexpectedly not available",
|
||||||
|
))
|
||||||
|
})?;
|
||||||
|
let stderr_reader = child.stderr.take().ok_or_else(|| {
|
||||||
|
CodexErr::Io(io::Error::new(
|
||||||
|
io::ErrorKind::Other,
|
||||||
|
"stderr pipe was unexpectedly not available",
|
||||||
|
))
|
||||||
|
})?;
|
||||||
|
|
||||||
let stdout_handle = tokio::spawn(read_capped(
|
let stdout_handle = tokio::spawn(read_capped(
|
||||||
BufReader::new(child.stdout.take().expect("stdout is not piped")),
|
BufReader::new(stdout_reader),
|
||||||
MAX_STREAM_OUTPUT,
|
MAX_STREAM_OUTPUT,
|
||||||
MAX_STREAM_OUTPUT_LINES,
|
MAX_STREAM_OUTPUT_LINES,
|
||||||
));
|
));
|
||||||
let stderr_handle = tokio::spawn(read_capped(
|
let stderr_handle = tokio::spawn(read_capped(
|
||||||
BufReader::new(child.stderr.take().expect("stderr is not piped")),
|
BufReader::new(stderr_reader),
|
||||||
MAX_STREAM_OUTPUT,
|
MAX_STREAM_OUTPUT,
|
||||||
MAX_STREAM_OUTPUT_LINES,
|
MAX_STREAM_OUTPUT_LINES,
|
||||||
));
|
));
|
||||||
|
|||||||
@@ -27,8 +27,7 @@ pub fn exec_linux(
|
|||||||
let tool_call_output = std::thread::spawn(move || {
|
let tool_call_output = std::thread::spawn(move || {
|
||||||
let rt = tokio::runtime::Builder::new_current_thread()
|
let rt = tokio::runtime::Builder::new_current_thread()
|
||||||
.enable_all()
|
.enable_all()
|
||||||
.build()
|
.build()?;
|
||||||
.expect("Failed to create runtime");
|
|
||||||
|
|
||||||
rt.block_on(async {
|
rt.block_on(async {
|
||||||
let ExecParams {
|
let ExecParams {
|
||||||
|
|||||||
@@ -75,6 +75,7 @@ fn is_safe_to_call_with_exec(command: &[String]) -> bool {
|
|||||||
fn try_parse_bash(bash_lc_arg: &str) -> Option<Tree> {
|
fn try_parse_bash(bash_lc_arg: &str) -> Option<Tree> {
|
||||||
let lang = BASH.into();
|
let lang = BASH.into();
|
||||||
let mut parser = Parser::new();
|
let mut parser = Parser::new();
|
||||||
|
#[expect(clippy::expect_used)]
|
||||||
parser.set_language(&lang).expect("load bash grammar");
|
parser.set_language(&lang).expect("load bash grammar");
|
||||||
|
|
||||||
let old_tree: Option<&Tree> = None;
|
let old_tree: Option<&Tree> = None;
|
||||||
|
|||||||
@@ -140,7 +140,7 @@ fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(),
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
#![allow(clippy::unwrap_used)]
|
#![expect(clippy::unwrap_used, clippy::expect_used)]
|
||||||
|
|
||||||
use super::*;
|
use super::*;
|
||||||
use crate::exec::ExecParams;
|
use crate::exec::ExecParams;
|
||||||
|
|||||||
@@ -134,7 +134,7 @@ async fn load_first_candidate(
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
#![allow(clippy::unwrap_used)]
|
#![allow(clippy::expect_used, clippy::unwrap_used)]
|
||||||
|
|
||||||
use super::*;
|
use super::*;
|
||||||
use crate::config::Config;
|
use crate::config::Config;
|
||||||
|
|||||||
@@ -1,3 +1,5 @@
|
|||||||
|
#![expect(clippy::unwrap_used, clippy::expect_used)]
|
||||||
|
|
||||||
//! Live integration tests that exercise the full [`Agent`] stack **against the real
|
//! Live integration tests that exercise the full [`Agent`] stack **against the real
|
||||||
//! OpenAI `/v1/responses` API**. These tests complement the lightweight mock‑based
|
//! OpenAI `/v1/responses` API**. These tests complement the lightweight mock‑based
|
||||||
//! unit tests by verifying that the agent can drive an end‑to‑end conversation,
|
//! unit tests by verifying that the agent can drive an end‑to‑end conversation,
|
||||||
@@ -65,7 +67,6 @@ async fn spawn_codex() -> Result<Codex, CodexErr> {
|
|||||||
#[ignore]
|
#[ignore]
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn live_streaming_and_prev_id_reset() {
|
async fn live_streaming_and_prev_id_reset() {
|
||||||
#![allow(clippy::unwrap_used)]
|
|
||||||
if !api_key_available() {
|
if !api_key_available() {
|
||||||
eprintln!("skipping live_streaming_and_prev_id_reset – OPENAI_API_KEY not set");
|
eprintln!("skipping live_streaming_and_prev_id_reset – OPENAI_API_KEY not set");
|
||||||
return;
|
return;
|
||||||
@@ -140,7 +141,6 @@ async fn live_streaming_and_prev_id_reset() {
|
|||||||
#[ignore]
|
#[ignore]
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn live_shell_function_call() {
|
async fn live_shell_function_call() {
|
||||||
#![allow(clippy::unwrap_used)]
|
|
||||||
if !api_key_available() {
|
if !api_key_available() {
|
||||||
eprintln!("skipping live_shell_function_call – OPENAI_API_KEY not set");
|
eprintln!("skipping live_shell_function_call – OPENAI_API_KEY not set");
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -1,3 +1,5 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
|
|
||||||
//! Optional smoke tests that hit the real OpenAI /v1/responses endpoint. They are `#[ignore]` by
|
//! Optional smoke tests that hit the real OpenAI /v1/responses endpoint. They are `#[ignore]` by
|
||||||
//! default so CI stays deterministic and free. Developers can run them locally with
|
//! default so CI stays deterministic and free. Developers can run them locally with
|
||||||
//! `cargo test --test live_cli -- --ignored` provided they set a valid `OPENAI_API_KEY`.
|
//! `cargo test --test live_cli -- --ignored` provided they set a valid `OPENAI_API_KEY`.
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
use codex_execpolicy::NegativeExamplePassedCheck;
|
use codex_execpolicy::NegativeExamplePassedCheck;
|
||||||
use codex_execpolicy::get_default_policy;
|
use codex_execpolicy::get_default_policy;
|
||||||
|
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
extern crate codex_execpolicy;
|
extern crate codex_execpolicy;
|
||||||
|
|
||||||
use codex_execpolicy::ArgMatcher;
|
use codex_execpolicy::ArgMatcher;
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
use codex_execpolicy::PositiveExampleFailedCheck;
|
use codex_execpolicy::PositiveExampleFailedCheck;
|
||||||
use codex_execpolicy::get_default_policy;
|
use codex_execpolicy::get_default_policy;
|
||||||
|
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
use codex_execpolicy::ArgMatcher;
|
use codex_execpolicy::ArgMatcher;
|
||||||
use codex_execpolicy::ArgType;
|
use codex_execpolicy::ArgType;
|
||||||
use codex_execpolicy::Error;
|
use codex_execpolicy::Error;
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
use codex_execpolicy::ArgType;
|
use codex_execpolicy::ArgType;
|
||||||
use codex_execpolicy::Error;
|
use codex_execpolicy::Error;
|
||||||
use codex_execpolicy::ExecCall;
|
use codex_execpolicy::ExecCall;
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
extern crate codex_execpolicy;
|
extern crate codex_execpolicy;
|
||||||
|
|
||||||
use codex_execpolicy::ArgType;
|
use codex_execpolicy::ArgType;
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
extern crate codex_execpolicy;
|
extern crate codex_execpolicy;
|
||||||
|
|
||||||
use std::vec;
|
use std::vec;
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
extern crate codex_execpolicy;
|
extern crate codex_execpolicy;
|
||||||
|
|
||||||
use codex_execpolicy::ArgType;
|
use codex_execpolicy::ArgType;
|
||||||
|
|||||||
@@ -117,6 +117,8 @@ pub(crate) fn create_tool_for_codex_tool_call_param() -> Tool {
|
|||||||
})
|
})
|
||||||
.into_generator()
|
.into_generator()
|
||||||
.into_root_schema_for::<CodexToolCallParam>();
|
.into_root_schema_for::<CodexToolCallParam>();
|
||||||
|
|
||||||
|
#[expect(clippy::expect_used)]
|
||||||
let schema_value =
|
let schema_value =
|
||||||
serde_json::to_value(&schema).expect("Codex tool schema should serialise to JSON");
|
serde_json::to_value(&schema).expect("Codex tool schema should serialise to JSON");
|
||||||
|
|
||||||
@@ -186,6 +188,7 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn verify_codex_tool_json_schema() {
|
fn verify_codex_tool_json_schema() {
|
||||||
let tool = create_tool_for_codex_tool_call_param();
|
let tool = create_tool_for_codex_tool_call_param();
|
||||||
|
#[expect(clippy::expect_used)]
|
||||||
let tool_json = serde_json::to_value(&tool).expect("tool serializes");
|
let tool_json = serde_json::to_value(&tool).expect("tool serializes");
|
||||||
let expected_tool_json = serde_json::json!({
|
let expected_tool_json = serde_json::json!({
|
||||||
"name": "codex",
|
"name": "codex",
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ use tokio::sync::mpsc::Sender;
|
|||||||
|
|
||||||
/// Convert a Codex [`Event`] to an MCP notification.
|
/// Convert a Codex [`Event`] to an MCP notification.
|
||||||
fn codex_event_to_notification(event: &Event) -> JSONRPCMessage {
|
fn codex_event_to_notification(event: &Event) -> JSONRPCMessage {
|
||||||
|
#[expect(clippy::expect_used)]
|
||||||
JSONRPCMessage::Notification(mcp_types::JSONRPCNotification {
|
JSONRPCMessage::Notification(mcp_types::JSONRPCNotification {
|
||||||
jsonrpc: JSONRPC_VERSION.into(),
|
jsonrpc: JSONRPC_VERSION.into(),
|
||||||
method: "codex/event".into(),
|
method: "codex/event".into(),
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
use mcp_types::ClientCapabilities;
|
use mcp_types::ClientCapabilities;
|
||||||
use mcp_types::ClientRequest;
|
use mcp_types::ClientRequest;
|
||||||
use mcp_types::Implementation;
|
use mcp_types::Implementation;
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
#![expect(clippy::expect_used)]
|
||||||
use mcp_types::JSONRPCMessage;
|
use mcp_types::JSONRPCMessage;
|
||||||
use mcp_types::ProgressNotificationParams;
|
use mcp_types::ProgressNotificationParams;
|
||||||
use mcp_types::ProgressToken;
|
use mcp_types::ProgressToken;
|
||||||
|
|||||||
Reference in New Issue
Block a user