12 ways agents write bad Rust

Agents do not write bad Rust randomly. They write bad Rust in a small, recognizable set of patterns. Once you can name the patterns, code review becomes pattern matching, not deep reading.

Each section below names the failure, shows the smell, explains the cost, and gives a prompt that surfaces the fix.

1. Clone spam

The agent hit a borrow checker error and reached for .clone() until the compiler stopped complaining. The code compiles. It also allocates and copies on every call.

△ Bad

fn build_query(parts: &Vec<String>, table: String) -> String {
  let parts = parts.clone();           // clone 1
  let mut out = table.clone();          // clone 2
  for p in parts {
      out = out.clone() + "." + &p.clone();  // clones 3, 4 per iter
  }
  out
}

◇ Good

fn build_query(parts: &[String], table: &str) -> String {
  let mut out = String::with_capacity(64);
  out.push_str(table);
  for p in parts {
      out.push('.');
      out.push_str(p);
  }
  out
}

Agents reach for shared mutable state because the prompt mentioned "threads" or "async." Most async code does not need a mutex. Most state does not need to be shared. Arc<Mutex<T>> is a real lock and a real allocation. Use it only when the data is actually shared and actually mutated from multiple tasks.

△ Bad

pub struct Counter {
  value: Arc<Mutex<u64>>,
}

impl Counter {
  pub fn incr(&self) {
      let mut v = self.value.lock().unwrap();
      *v += 1;
  }
}

◇ Good

use std::sync::atomic::{AtomicU64, Ordering};

pub struct Counter {
  value: AtomicU64,
}

impl Counter {
  pub fn incr(&self) {
      self.value.fetch_add(1, Ordering::Relaxed);
  }
}

3. `unwrap()` in libraries

unwrap() and expect("...") panic on the unhappy path. Inside main() or a test, that is sometimes fine. Inside a library, it crashes your caller. Every unwrap in non-test library code is a contract violation waiting to happen.

△ Bad

pub fn parse_config(s: &str) -> Config {
  let json: Value = serde_json::from_str(s).unwrap();
  let port = json["port"].as_u64().unwrap() as u16;
  Config { port }
}

◇ Good

pub fn parse_config(s: &str) -> Result<Config, ConfigError> {
  let json: Value = serde_json::from_str(s)
      .map_err(ConfigError::Json)?;
  let port = json["port"]
      .as_u64()
      .ok_or(ConfigError::MissingPort)? as u16;
  Ok(Config { port })
}

4. Swallowed errors via `Box<dyn Error>` or `anyhow!`

Box<dyn Error> and anyhow::Error are great in application top levels. They are bad as the return type of library functions, because callers lose the ability to match on specific failures. Agents reach for them to "make it compile faster." Push back.

△ Bad

pub fn read_catalog(path: &str)
  -> Result<Catalog, Box<dyn std::error::Error>>
{
  let bytes = std::fs::read(path)?;
  let cat: Catalog = serde_json::from_slice(&bytes)?;
  Ok(cat)
}

◇ Good

#[derive(Debug, thiserror::Error)]
pub enum CatalogError {
  #[error("read failed at {path}: {source}")]
  Io { path: String, source: std::io::Error },
  #[error("parse failed: {0}")]
  Parse(#[from] serde_json::Error),
}

pub fn read_catalog(path: &str) -> Result<Catalog, CatalogError> {
  let bytes = std::fs::read(path).map_err(|e| CatalogError::Io {
      path: path.to_owned(),
      source: e,
  })?;
  Ok(serde_json::from_slice(&bytes)?)
}

5. Unjustified `unsafe`

unsafe turns off some compiler checks. Sometimes it is the right call (FFI, real performance work, raw pointers with verified invariants). Most of the time the agent reached for it to silence a complaint. Every unsafe block needs a comment justifying the invariants you are upholding by hand.

6. Hand-rolled futures and async wrong-think

Agents sometimes invent their own Future impls when async fn would do, or fire-and-forget with tokio::spawn and lose error visibility, or hold a lock across .await. The async failure modes look syntactically reasonable.

△ Bad

async fn process(state: Arc<Mutex<State>>) -> Result<()> {
  let mut s = state.lock().unwrap();  // sync mutex, blocks runtime
  s.touch();
  fetch_external().await?;             // lock held across await!
  s.commit();
  Ok(())
}

◇ Good

async fn process(state: Arc<Mutex<State>>) -> Result<()> {
  {
      let mut s = state.lock().unwrap();
      s.touch();
  }                                     // lock released
  fetch_external().await?;
  {
      let mut s = state.lock().unwrap();
      s.commit();
  }
  Ok(())
}

7. Manual `match` instead of `?`

The agent wrote a five-arm match on every Result instead of using ?. The code works. It is also five times the lines and ten times the noise.

△ Bad

fn load(p: &Path) -> Result<Doc, MyError> {
  let bytes = match std::fs::read(p) {
      Ok(b) => b,
      Err(e) => return Err(MyError::Io(e)),
  };
  let s = match std::str::from_utf8(&bytes) {
      Ok(s) => s,
      Err(e) => return Err(MyError::Utf8(e)),
  };
  Ok(Doc::new(s))
}

◇ Good

fn load(p: &Path) -> Result<Doc, MyError> {
  let bytes = std::fs::read(p)?;
  let s = std::str::from_utf8(&bytes)?;
  Ok(Doc::new(s))
}

The ? operator needs From<E1> for E2 impls (or #[from] via thiserror). If those are missing, the agent should add them, not work around them with match.

8. `dyn Trait` when generics work

Box<dyn Trait> is dynamic dispatch with a heap allocation. Generic T: Trait is static dispatch, often inlined, no heap. Both are valid. Agents default to dyn because it is easier to satisfy the type system. Push for generics in hot paths.

△ Bad

fn run(plans: Vec<Box<dyn LogicalPlan>>) {
  for p in plans {
      p.execute();
  }
}

◇ Good

fn run<P: LogicalPlan>(plans: Vec<P>) {
  for p in plans {
      p.execute();
  }
}

Use dyn deliberately when you need heterogeneous collections or stable ABI. Otherwise generics.

9. Panic in `From`/`TryFrom` impls

From says "this conversion cannot fail." If your From impl panics on bad input, you have lied to every caller. Use TryFrom for fallible conversions.

△ Bad

impl From<String> for Port {
  fn from(s: String) -> Self {
      Port(s.parse().unwrap())  // panics on invalid input
  }
}

◇ Good

impl TryFrom<String> for Port {
  type Error = ParsePortError;
  fn try_from(s: String) -> Result<Self, Self::Error> {
      let n = s.parse().map_err(|_| ParsePortError)?;
      Ok(Port(n))
  }
}

10. Missing `Send`/`Sync` bounds in async

If an async function will be tokio::spawn-ed, its future must be Send. Agents sometimes write async code that compiles standalone but breaks the moment you try to spawn it because some non-Send value (an Rc, a raw pointer, a RefCell) snuck in.

11. `'static` lifetimes as a workaround

Lifetime errors confuse agents. The lazy fix is to slap 'static on everything, often by cloning into owned types or using Box::leak. The code compiles. It also forfeits Rust's whole point: borrowing without copying.

12. Untested error paths

The happy path has tests. The error path does not. Agents test what they wrote, not what could go wrong. Every Result-returning function should have at least one test for the Err side.

△ Bad

#[test]
fn parses_valid_config() {
  let c = parse_config(r#"{"port":8080}"#).unwrap();
  assert_eq!(c.port, 8080);
}
// no test for missing port, invalid json, wrong type

◇ Good

#[test]
fn parses_valid_config() { /* ... */ }

#[test]
fn rejects_invalid_json() {
  let err = parse_config("{not json").unwrap_err();
  assert!(matches!(err, ConfigError::Json(_)));
}

#[test]
fn rejects_missing_port() {
  let err = parse_config("{}").unwrap_err();
  assert!(matches!(err, ConfigError::MissingPort));
}

The orchestrator's grep checklist

Run before any agent-written Rust PR is merged:

# Risk surface, in order of severity.
rg "unsafe"      --type rust    # justify each one
rg "\.unwrap\(\)" --type rust   # libraries: zero in non-test code
rg "\.expect\(" --type rust    # same
rg "Box<dyn Error" --type rust # library APIs: zero
rg "'static"     --type rust   # late additions are suspicious
rg "Arc<Mutex"   --type rust   # justify each one
rg "\.clone\(\)" --type rust | wc -l   # count, compare to baseline

The numbers should be small and stable. New entries are review checkpoints.