GPUI Code Quality

This skill covers best practices and code quality guidelines for GPUI development.

Error Handling

Never Use unwrap()

Rule: Avoid unwrap() and panic-inducing methods

// ❌ WRONG
let value = option.unwrap();
let result = operation().unwrap();

// ✅ CORRECT - propagate with ?
let value = option.ok_or_else(|| anyhow::anyhow!("Missing value"))?;
let result = operation()?;

// ✅ ALSO CORRECT - explicit handling
match option {
    Some(value) => {
        // Use value
    }
    None => {
        // Handle error case
    }
}

Never Silently Discard Errors

Rule: Don't use let _ = on fallible operations

// ❌ WRONG - silently discards error
let _ = client.request(url).await?;

// ✅ CORRECT - propagate error
client.request(url).await?;

// ✅ ALSO CORRECT - log but ignore
operation().log_err();

// ✅ ALSO CORRECT - explicit handling
if let Err(e) = operation() {
    eprintln!("Operation failed: {}", e);
}

Propagate Errors to UI

fn save_file(&mut self, cx: &mut Context<Self>) {
    cx.spawn(async move |this, cx| {
        match write_file(path, data).await {
            Ok(()) => {
                this.update(&mut *cx, |view, cx| {
                    view.status = "Saved successfully".into();
                    cx.notify();
                })?;
            }
            Err(e) => {
                this.update(&mut *cx, |view, cx| {
                    view.error = Some(format!("Save failed: {}", e));
                    cx.notify();
                })?;
            }
        }
        
        Ok(())
    }).detach_and_log_err(cx);
}

Variable Naming

Use Full Words

Rule: Avoid abbreviations, use descriptive names

// ❌ WRONG
let q = VecDeque::new();
let cnt = 0;
let btn = Button::new();

// ✅ CORRECT
let queue = VecDeque::new();
let count = 0;
let button = Button::new();

Meaningful Names

// ❌ WRONG
let x = calculate();
let temp = process(data);
let thing = Entity::new();

// ✅ CORRECT
let result = calculate();
let processed_data = process(data);
let user_profile = Entity::new();

Async Context Variable Shadowing

Use variable shadowing to scope clones in async contexts:

// ✅ CORRECT - clear scoping
fn start_work(&mut self, cx: &mut Context<Self>) {
    let data = self.data.clone();
    let config = self.config.clone();
    
    cx.spawn(async move |this, cx| {
        let result = process(data, config).await;
        
        this.update(&mut *cx, |view, cx| {
            view.result = result;
            cx.notify();
        })?;
        
        Ok(())
    }).detach();
}

File Organization

Avoid mod.rs Files

Rule: Use src/module.rs instead of src/module/mod.rs

// ❌ WRONG
src/
  components/
    mod.rs      # Don't do this
    button.rs

// ✅ CORRECT
src/
  components.rs  # Module file
  components/
    button.rs

Library Root Paths

For crates, specify library root in Cargo.toml:

[lib]
path = "src/my_lib.rs"  # Instead of default lib.rs

State Management

Call cx.notify() After Changes

// ❌ WRONG
fn update_data(&mut self, data: String, cx: &mut Context<Self>) {
    self.data = data;
    // Missing cx.notify() - UI won't update!
}

// ✅ CORRECT
fn update_data(&mut self, data: String, cx: &mut Context<Self>) {
    self.data = data;
    cx.notify(); // Trigger re-render
}

Use WeakEntity for Back-References

// ❌ WRONG - memory leak
struct Child {
    parent: Entity<Parent>, // Strong reference creates cycle!
}

// ✅ CORRECT
struct Child {
    parent: WeakEntity<Parent>, // Weak reference prevents leak
}

Rendering

Keep Render Pure

Rule: Don't mutate state in render() method

// ❌ WRONG
impl Render for MyView {
    fn render(&mut self, _window: &mut Window, _cx: &mut Context<Self>) -> impl IntoElement {
        self.render_count += 1; // Don't mutate in render!
        div().child(format!("Renders: {}", self.render_count))
    }
}

// ✅ CORRECT - mutate in event handlers
impl Render for MyView {
    fn render(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
        div()
            .child(format!("Count: {}", self.count))
            .child(
                div()
                    .child("Increment")
                    .on_click(cx.listener(|this, _event, _window, cx| {
                        this.count += 1;
                        cx.notify();
                    }))
            )
    }
}

Use SharedString for Text

// ❌ LESS EFFICIENT
struct MyView {
    title: String,
}

// ✅ MORE EFFICIENT
struct MyView {
    title: SharedString,
}

Async Patterns

Detach or Store Tasks

// ❌ WRONG - task dropped immediately
cx.spawn(async move |this, cx| {
    // Work...
    Ok(())
});

// ✅ CORRECT - detach
cx.spawn(async move |this, cx| {
    // Work...
    Ok(())
}).detach();

// ✅ ALSO CORRECT - store for cancellation
self.current_task = Some(cx.spawn(async move |this, cx| {
    // Work...
    Ok(())
}));

Use background_spawn for CPU Work

// ❌ WRONG - blocks UI thread
cx.spawn(async move |this, cx| {
    let result = expensive_computation(); // Blocks UI!
    Ok(())
});

// ✅ CORRECT - run on background thread
cx.spawn(async move |this, cx| {
    let result = cx.background_spawn(async {
        expensive_computation()
    }).await;
    
    this.update(&mut *cx, |view, cx| {
        view.result = result;
        cx.notify();
    })?;
    
    Ok(())
});

Comments

Only Explain "Why"

Rule: Don't write comments that summarize code

// ❌ WRONG - obvious from code
// Increment the counter
self.counter += 1;

// Set the title to "Hello"
self.title = "Hello".into();

// ✅ CORRECT - explains non-obvious reasoning
// We must update the cache before notifying observers
// to ensure they see the most recent data
self.update_cache();
cx.notify();

// ✅ ALSO CORRECT - explains tricky logic
// Use saturating_sub to avoid underflow when count is 0
self.count = self.count.saturating_sub(1);

Testing

Use GPUI Timers in Tests

// ❌ WRONG
#[gpui::test]
async fn test_delay(cx: &mut TestAppContext) {
    smol::Timer::after(Duration::from_secs(1)).await;
}

// ✅ CORRECT
#[gpui::test]
async fn test_delay(cx: &mut TestAppContext) {
    cx.background_executor.timer(Duration::from_secs(1)).await;
    cx.background_executor.run_until_parked();
}

Build Guidelines

Use Project Scripts

# ❌ WRONG
cargo clippy

# ✅ CORRECT (for Zed-based projects)
./script/clippy

# Always use -q flag for cargo
cargo build -q
cargo test -q

Code Organization

Prefer Existing Files

Rule: Add functionality to existing files unless it's a new logical component

// Instead of creating many small files:
// src/button_click.rs
// src/button_hover.rs
// src/button_focus.rs

// Keep related functionality together:
// src/button.rs (with all button behavior)

Module Organization

// Good module structure
src/
  ui/
    components.rs   // Component definitions
    theme.rs        // Theme and colors
    styles.rs       // Shared styles
  data/
    models.rs       // Data models
    state.rs        // Application state

Summary of Best Practices

| Rule | Rationale | |------|-----------| | Never use unwrap() | Prevents panics, forces explicit error handling | | Never let _ = on fallible ops | Errors should be handled or logged | | Use full variable names | Improves readability and maintainability | | Avoid mod.rs files | Cleaner project structure | | Call cx.notify() after mutations | Ensures UI updates | | Use WeakEntity for back-refs | Prevents memory leaks | | Keep render() pure | Avoids race conditions | | Use SharedString for text | Reduces allocations | | Detach or store tasks | Prevents cancelled work | | Use GPUI timers in tests | Prevents test failures | | Only comment "why", not "what" | Reduces noise, focuses on reasoning | | Propagate errors to UI | Provides user feedback | | Use background_spawn() for CPU work | Keeps UI responsive |

Checklist for Code Review

[ ] No unwrap() calls
[ ] No silently discarded errors (let _ = on fallible ops)
[ ] cx.notify() called after state changes
[ ] Tasks are detached or stored
[ ] Weak references used for back-pointers
[ ] SharedString used for text
[ ] No mutations in render()
[ ] Descriptive variable names
[ ] Comments explain "why", not "what"
[ ] GPUI timers used in tests
[ ] Errors propagated to UI layer