flutter-review

What this skill does

# Flutter Code Review

**This skill is directly invocable. See [workflow.md](workflow.md) for the complete 5-step execution strategy.**

Expert knowledge for identifying critical bugs, anti-patterns, and code quality issues in Flutter/Dart code.

## Execution Instructions

When invoked directly (e.g., via /flutter-review command):

1. Follow the 5-step workflow in [workflow.md](workflow.md)
2. Apply the priority-based checks defined below
3. Generate a comprehensive review report
4. Offer to automatically fix issues found

## Priority-Based Analysis

### P0 - Critical (Must Fix Before Merge)

Issues that cause crashes, data loss, security vulnerabilities, or visual bugs.

#### Null Safety Violations

**Force Unwrap Without Checks** (variable!, expression!.property)

- Risk: Runtime crash with "Null check operator used on a null value"
- Fix: Extract nullable values to local variables, check for null, then use non-null variable
- **Preferred Pattern:** Extract property to local variable → check null → use promoted non-null variable
- **Avoid:** Checking widget.property == null then using widget.property with ! operator
- → Details: reference.md#avoid_non_null_assertion

**Unsafe Nullable Access**

- Risk: Null pointer exceptions when accessing properties without checks
- Fix: Add null checks before property access
- → Details: reference.md#missing_null_safety

**Unsafe Type Casts** (value as Type)

- Risk: Type cast errors at runtime
- Fix: Use pattern matching or check with is operator first
- → Details: reference.md#unsafe_type_casts

#### Flutter Lifecycle Issues

**setState Without Mounted Check**

- Risk: setState() called after dispose() crashes app
- Fix: Add if (!mounted) return; before setState in async callbacks
- → Details: reference.md#avoid_mounted_in_setstate

**State Modifications After Disposal**

- Risk: Memory leaks and unexpected behavior
- Fix: Check mounted or cancel operations in dispose
- → Details: reference.md#state_after_disposal

#### Memory Leaks

**Resources That Must Be Disposed:**
TextEditingController, AnimationController, ScrollController, TabController, PageController, FocusNode, StreamSubscription, Timer, any class with addListener() called

**Detection:** Check if controllers/subscriptions created in fields or initState have corresponding dispose() calls. Verify ALL resources disposed, not just presence of dispose method.

- → Details: reference.md#dispose_class_fields

#### Logic Errors

**Incorrect Conditional Logic**

- || vs && confusion, missing null checks in compounds, negation errors
- → Details: reference.md#logic_errors_conditionals

**Missing Error Handling**

- API calls, file operations, JSON parsing without try-catch
- → Details: reference.md#missing_error_handling

**Infinite Loops**

- Loops without termination or where condition never changes
- → Details: reference.md#infinite_loops

**Race Conditions**

- Multiple async operations modifying same state without guards
- → Details: reference.md#race_conditions

#### Bloc State Synchronization Issues

**Only check when flutter_bloc or bloc in dependencies.**

**BlocListener/BlocConsumer Missing Initial State**

- Risk: Widget state (TextEditingController, variables) not initialized from bloc's initial state
- Problem: Listener callbacks only fire on state changes, not initial state. If bloc emits state before widget creation, initial state is missed
- Fix: Initialize widget state in initState() from current bloc state, AND have listener for updates
- → Details: reference.md#bloc_missing_initial_state

#### Text Overflow in Flex Layouts

**Text Without Constraints in Row/Column**

- Risk: RenderFlex overflow errors, visual bugs, unusable UI
- Fix: Wrap Text in Flexible/Expanded widget AND/OR add overflow handling (TextOverflow.ellipsis)
- → Details: reference.md#text_overflow_flex

---

### P1 - Important (Should Fix)

Issues causing logic bugs and maintainability problems.

#### Collection Equality

**Using == on Collections** (list1 == list2, map1 == map2)

- Problem: Dart uses reference equality, always false for different instances
- Fix: Use package:collection - ListEquality().equals(), MapEquality().equals(), or DeepCollectionEquality().equals()
- Exception: const collections (same instance)
- → Details: reference.md#avoid_collection_equality_checks

#### Code Complexity

**Deep Nesting** (>4 levels)

- Problem: Reduces readability, increases cognitive load
- Fix: Extract methods, use early returns, guard clauses
- → Details: reference.md#deep_nesting

**Long Methods** (>100 lines)

- Problem: Violates Single Responsibility Principle
- Fix: Break into smaller focused methods
- → Details: reference.md#long_methods

**Long Build Methods** (>50 lines)

- Problem: Too much UI logic in build method, hard to maintain
- Fix: Extract to buildWidget helpers (buildHeader, buildBody) or create separate widget files
- → Details: reference.md#long_build_methods

#### Widget Organization

**Multiple Widget Definitions Per File**

- Problem: Reduces code organization, testability, and reusability
- Fix: Each widget class must be in its own file (e.g., UserCard in user_card.dart)
- **No exceptions**: Even private (`_Widget`) or small helper widgets should be extracted
- → Details: reference.md#multiple_widgets_per_file

#### Late Modifier Usage

**Avoid Late Keyword**

- Risk: Defers initialization to runtime, can cause LateInitializationError
- Fix: Initialize in constructor, use nullable types, or required parameters
- Exception: DI frameworks with guaranteed initialization
- → Details: reference.md#avoid_late_keyword

#### Bloc Anti-Patterns

**Only check when flutter_bloc or bloc in dependencies.**

**Public Fields in BLoC**

- Problem: Breaks encapsulation, allows external state modification
- Fix: Make fields private, expose through state
- → Details: reference.md#bloc_public_fields

**Public Methods in BLoC**

- Problem: BLoCs should only respond to events
- Fix: Create events and use add() instead
- → Details: reference.md#bloc_public_methods

**Mutable Events**

- Problem: Events should be immutable data
- Fix: Add @immutable, make fields final, use const constructors
- → Details: reference.md#bloc_mutable_events

**Non-Sealed States**

- Problem: Can't exhaustively pattern match
- Fix: Use sealed class for state base, final class for implementations
- → Details: reference.md#bloc_non_sealed_states

#### Provider Anti-Patterns

**Only check when provider in dependencies.**

**context.read() in Build**

- Problem: Won't rebuild when provider changes
- Fix: Use context.watch() for reactive data
- → Details: reference.md#provider_read_in_build

**Old Provider Syntax** (Provider.of<Type>(context, listen: true))

- Problem: Outdated, less readable
- Fix: Use context.watch() or context.read()
- → Details: reference.md#provider_old_syntax

**Missing Disposal in ChangeNotifier**

- Problem: Memory leaks from undisposed resources
- Fix: Override dispose() and clean up all resources
- → Details: reference.md#provider_missing_disposal

#### Logic Issues

**Incorrect Operators**

- Using = instead of == in conditions, comparing incompatible types
- → Details: reference.md#incorrect_operators

**Missing Edge Cases**

- No empty collection checks before .first/.last, no bounds checking
- → Details: reference.md#missing_edge_cases

**Off-By-One Errors**

- Loop conditions with <= instead of < for length
- → Details: reference.md#off_by_one_errors

---

### P2 - Code Quality (Nice to Have)

Improvements for maintainability.

#### Magic Numbers

**Hardcoded Numbers Without Context**

- Exceptions: 0, 1, -1, obvious widget dimensions (padding: 16)
- Fix: Define as named constants
- → Details: reference.md#magic_numbers

#### TODO/FIXME Comments

**Markers for Incomplete Work**

- Patterns: // TODO:, // FIXME:, // HACK:
- Action: Track and address before merge
- → Details: reference.md#todo_comments

#### Function Ordering

**Private Functions After Public Functions**