/flutter-dart-code-review

name: flutter-dart-code-review
description: Library-agnostic Flutter/Dart code review checklist covering widget best practices, state management patterns (BLoC, Riverpod, Provider, GetX, MobX, Signals), Dart idioms, performance, accessibility, security, and clean architecture. Use when reviewing Flutter or Dart code, whatever state management library the project uses.
metadata:
  origin: ECC

Flutter/Dart Code Review Best Practices

Comprehensive, library-agnostic checklist for reviewing Flutter/Dart applications. These principles apply regardless of which state management solution, routing library, or DI framework is used.

---

1. General Project Health

• [ ] Project follows consistent folder structure (feature-first or layer-first)
• [ ] Proper separation of concerns: UI, business logic, data layers
• [ ] No business logic in widgets; widgets are purely presentational
• [ ] `pubspec.yaml` is clean — no unused dependencies, versions pinned appropriately
• [ ] `analysis_options.yaml` includes a strict lint set with strict analyzer settings enabled
• [ ] No `print()` statements in production code — use `dart:developer` `log()` or a logging package
• [ ] Generated files (`.g.dart`, `.freezed.dart`, `.gr.dart`) are up-to-date or in `.gitignore`
• [ ] Platform-specific code isolated behind abstractions

---

2. Dart Language Pitfalls

• [ ] **Implicit dynamic**: Missing type annotations leading to `dynamic` — enable `strict-casts`, `strict-inference`, `strict-raw-types`
• [ ] **Null safety misuse**: Excessive `!` (bang operator) instead of proper null checks or Dart 3 pattern matching (`if (value case var v?)`)
• [ ] **Type promotion failures**: Using `this.field` where local variable promotion would work
• [ ] **Catching too broadly**: `catch (e)` without `on` clause; always specify exception types
• [ ] **Catching `Error`**: `Error` subtypes indicate bugs and should not be caught
• [ ] **Unused `async`**: Functions marked `async` that never `await` — unnecessary overhead
• [ ] **`late` overuse**: `late` used where nullable or constructor initialization would be safer; defers errors to runtime
• [ ] **String concatenation in loops**: Use `StringBuffer` instead of `+` for iterative string building
• [ ] **Mutable state in `const` contexts**: Fields in `const` constructor classes should not be mutable
• [ ] **Ignoring `Future` return values**: Use `await` or explicitly call `unawaited()` to signal intent
• [ ] **`var` where `final` works**: Prefer `final` for locals and `const` for compile-time constants
• [ ] **Relative imports**: Use `package:` imports for consistency
• [ ] **Mutable collections exposed**: Public APIs should return unmodifiable views, not raw `List`/`Map`
• [ ] **Missing Dart 3 pattern matching**: Prefer switch expressions and `if-case` over verbose `is` checks and manual casting
• [ ] **Throwaway classes for multiple returns**: Use Dart 3 records `(String, int)` instead of single-use DTOs
• [ ] **`print()` in production code**: Use `dart:developer` `log()` or the project's logging package; `print()` has no log levels and cannot be filtered

---

3. Widget Best Practices

Widget decomposition:

• [ ] No single widget with a `build()` method exceeding ~80-100 lines
• [ ] Widgets split by encapsulation AND by how they change (rebuild boundaries)
• [ ] Private `_build*()` helper methods that return widgets are extracted to separate widget classes (enables element reuse, const propagation, and framework optimizations)
• [ ] Stateless widgets preferred over Stateful where no mutable local state is needed
• [ ] Extracted widgets are in separate files when reusable

Const usage:

• [ ] `const` constructors used wherever possible — prevents unnecessary rebuilds
• [ ] `const` literals for collections that don't change (`const []`, `const {}`)
• [ ] Constructor is declared `const` when all fields are final

Key usage:

• [ ] `ValueKey` used in lists/grids to preserve state across reorders
• [ ] `GlobalKey` used sparingly — only when accessing state across the tree is truly needed
• [ ] `UniqueKey` avoided in `build()` — it forces rebuild every frame
• [ ] `ObjectKey` used when identity is based on a data object rather than a single value

Theming & design system:

• [ ] Colors come from `Theme.of(context).colorScheme` — no hardcoded `Colors.red` or hex values
• [ ] Text styles come from `Theme.of(context).textTheme` — no inline `TextStyle` with raw font sizes
• [ ] Dark mode compatibility verified — no assumptions about light background
• [ ] Spacing and sizing use consistent design tokens or constants, not magic numbers

Build method complexity:

• [ ] No network calls, file I/O, or heavy computation in `build()`
• [ ] No `Future.then()` or `async` work in `build()`
• [ ] No subscription creation (`.listen()`) in `build()`
• [ ] `setState()` localized to smallest possible subtree

---

4. State Management (Library-Agnostic)

These principles apply to all Flutter state management solutions (BLoC, Riverpod, Provider, GetX, MobX, Signals, ValueNotifier, etc.).

Architecture:

• [ ] Business logic lives outside the widget layer — in a state management component (BLoC, Notifier, Controller, Store, ViewModel, etc.)
• [ ] State managers receive dependencies via injection, not by constructing them internally
• [ ] A service or repository layer abstracts data sources — widgets and state managers should not call APIs or databases directly
• [ ] State managers have a single responsibility — no "god" managers handling unrelated concerns
• [ ] Cross-component dependencies follow the solution's conventions:
• In **Riverpod**: providers depending on providers via `ref.watch` is expected — flag only circular or overly tangled chains
• In **BLoC**: blocs should not directly depend on other blocs — prefer shared repositories or presentation-layer coordination
• In other solutions: follow the documented conventions for inter-component communication

###