Swift Code Review

Review Swift code against industry best practices drawn from Google's Swift Style Guide, Apple's API Design Guidelines, Apple's performance and memory documentation, and real-world patterns from Apple's open-source projects.

Determine Review Scope

Figure out what to review based on the user's request:

When reviewing diffs, always read surrounding context (the full file or function) to understand intent — don't review isolated hunks.

Review Checklist

Work through these categories in order. For each finding, cite the file, line, and the specific guideline being violated. Skip categories that don't apply to the scope.

1. Naming & API Design

Read references/api-design.md for the complete rules.

Key checks:

• Types use UpperCamelCase, everything else uses lowerCamelCase
• Methods read as grammatical English at the call site: x.insert(y, at: z)
• Names describe roles, not types: greeting not string
• Mutating/nonmutating pairs follow the -ed/-ing convention
• Boolean properties read as assertions: isEmpty, isValid
• Factory methods start with make
• Protocol names use nouns for "what it is", -able/-ible for capabilities
• Argument labels form grammatical phrases with the method name
• No abbreviations unless universally understood (min, max, URL)

2. Code Style & Formatting

Read references/code-style.md for the complete rules.

Key checks:

• 100-character line limit
• K&R brace style (opening brace on same line)
• No semicolons
• One statement per line
• // comments only, never /* */
• One variable per let/var declaration
• Trailing commas in multiline array/dictionary literals
• Imports grouped: standard modules, then individual declarations, then @testable
• // MARK: comments to organize type members
• Shorthand syntax for arrays [Element], dictionaries [Key: Value], optionals T?

3. Swift Patterns & Idioms

Read references/swift-patterns.md for the complete rules.

Key checks:

• guard for early exits instead of nested if blocks
• for-where instead of for + single if body
• Optional over sentinel values (-1, empty string)
• Errors thrown, not merged with return types
• No force-unwrap (!) without a comment explaining the invariant
• No implicitly unwrapped optionals except @IBOutlet and test fixtures
• Explicit access control where it differs from internal
• Nested types for scoping (error types, flag enums inside their parent)
• Prefer let over var when value doesn't change
• Prefer value types (struct, enum) over class when no identity needed
• Protocol extensions for default implementations, not base classes
• Computed properties omit get when read-only

4. Build Performance

Read references/performance.md for the complete rules.

Key checks:

• No complex expressions that slow type-checker (break into smaller statements)
• Explicit types on complex closures and collection literals
• Minimize use of AnyObject, Any — prefer concrete or generic types
• No unnecessary @objc or dynamic unless needed for Objective-C interop
• Module boundaries are clean — no circular dependencies
• Prefer final on classes not designed for inheritance (enables compiler optimizations)
• Large expressions broken into smaller sub-expressions with explicit types
• No excessive protocol conformance in extensions across files (merge where logical)

5. Memory Management

Read references/memory-management.md for the complete rules.

Key checks:

• No retain cycles in closures — use [weak self] or [unowned self] appropriately
• weak preferred over unowned unless lifetime is guaranteed
• Delegates declared as weak var
• Large resources freed in deinit or on memory warnings
• Discardable resource caches use NSCache with count/cost limits; deterministic maps or non-discardable data may use bounded dictionaries
• Autoreleasepool used in tight loops creating many temporary objects
• No strong reference chains between parent and child objects
• Image and data caches have size limits
• Observation tokens stored and invalidated properly

6. Testing

Read references/testing.md for the complete rules.

Key checks:

• Test methods named descriptively: test_<condition>_<expectedResult>
• One assertion concept per test (test may have setup + multiple related asserts)
• No test interdependencies — each test stands alone
• setUp() / tearDown() used for shared fixtures, not duplicated in every test
• Async code tested with expectations or Swift concurrency
• UI tests separated from unit tests
• Edge cases covered: nil, empty, boundary values, error paths
• No network calls in unit tests — use protocols and mocks
• Force-unwrap permitted in tests (fails the test if nil, which is desired)

7. Documentation

Key checks:

• Public API has /// doc comments with summary, parameters, returns, throws
• Doc comments use verb phrases for methods, noun phrases for properties
• No /** */ block comment syntax
• Parameter, Returns, Throws tags in that order
• Complex algorithms have inline comments explaining why, not what

8. Concurrency (if applicable)

Key checks:

• @Sendable closures don't capture mutable state
• Actors used for mutable shared state instead of locks
• MainActor for UI updates
• No data races — nonisolated used intentionally
• Structured concurrency (async let, TaskGroup) preferred over unstructured Task {}
• Cancellation handled properly

Output Format

Structure the review as:

## Summary
<1-2 sentence overall assessment>

## Findings

### Critical
<issues that will cause bugs, crashes, or data loss>

### Important
<style violations, performance issues, missing tests>

### Suggestions
<minor improvements, alternative approaches>

## What's Done Well
<2-3 things the code does right — always include this>

For each finding, include:

• File and line: Sources/Auth/TokenManager.swift:42
• Category: which checklist item it falls under
• Issue: what's wrong
• Fix: concrete code suggestion or direction

Keep findings actionable. Don't flag things that are clearly intentional project conventions unless they cause real problems.