tomevault-io Flutter Code Review
This skill is directly invocable. See 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):
- Follow the 5-step workflow in workflow.md
- Apply the priority-based checks defined below
- Generate a comprehensive review report
- 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(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
- Problem: Private functions placed before public ones reduces readability and makes it harder to understand the public API
- Fix: Move all private methods (starting with _) after public methods
- Widget-specific order: initState → other lifecycle methods → dispose → build → public methods → private methods
- → Details: reference.md#function_ordering
Long Classes (>500 lines)
Classes Exceeding 500 Lines
- Problem: Violates Single Responsibility, may contain dead code
- Fix: Split into multiple classes, extract to mixins/utilities
- Action: Review for unused methods, commented code, duplicate logic
- → Details: reference.md#long_classes
Moderate Nesting (3-4 levels)
Nesting Depth 3-4
- Consider simplification for readability
- → Details: reference.md#moderate_nesting
Dependency-Specific Knowledge
Bloc Pattern Detection
Trigger: flutter_bloc or bloc in pubspec.yaml
Key Principles:
- State is immutable and sealed
- Events are immutable
- No public methods (only event handlers)
- No public fields (only private state)
- UI never contains business logic
→ See reference.md#bloc_pattern for state/event patterns
Provider Pattern Detection
Trigger: provider in pubspec.yaml
Key Principles:
- Use context.watch() for reactive rebuilds
- Use context.read() only for one-time reads (not in build)
- Use context.select() for performance optimization
- Always dispose resources in ChangeNotifier
→ See reference.md#provider_pattern for usage patterns
Context Awareness
When NOT to Flag
Test Files:
- More lenient with force unwraps in test setup
- Focus on logic errors, not style
Generated Code:
- Files matching _.g.dart, _.freezed.dart, *.gr.dart
- Skip entirely - will be regenerated
Intentional Patterns:
- Type check before cast (e.g.,
if (x is String) { x.length; }) - Controlled environments after validation (e.g., validated user input)
Note: Even with null check immediately before force unwrap, prefer extracting to local variable for type promotion instead of using the ! operator.
When TO Flag Even If...
Pattern is common:
- Common doesn't mean correct
- Flag reference equality on collections even if widespread
"It works":
- Working code can still have bugs
- Flag potential issues even if not currently crashing
Dispose exists but incomplete:
- If new controllers added but not in dispose
- Check ALL resources, not just presence of dispose method
Widget is private or small:
- Flag ALL widget definitions in same file (except StatefulWidget's State class)
- Private (
_Widget) does not exempt from one-widget-per-file rule - "Small" size does not exempt from one-widget-per-file rule
- Every widget class deserves its own file for testability and organization
Review Output Format
Report Structure
Summary:
- Count by priority (P0: X, P1: Y, P2: Z)
- Clear status (Pass / Must Fix / Recommended)
- Overall recommendation (🔴 BLOCK / 🟡 REVIEW NEEDED / 🟢 APPROVED)
Issue Format:
- File path:line_number
- Issue type and priority
- Problem explanation
- Current code snippet (with context)
- Fixed code snippet
- Benefit of fix
Organization:
- P0 first (most critical)
- Within priority, group by type
- Use tables for many similar issues (>3)
- Detailed format for complex issues
Recommendation Guidelines
🔴 BLOCK - Any P0 issues present (must fix before merge) 🟡 REVIEW NEEDED - No P0, but P1 issues present (should fix) 🟢 APPROVED - No P0 or P1, only P2 optional improvements
Analysis Efficiency
Focus on Changed Code:
- Analyze additions and modifications (+ in diff)
- Use surrounding context from diff
- Don't review entire codebase
Selective Deep Dives: Read full files only when:
- Diff context insufficient
- Need to verify disposal (new controllers/streams added)
- Need class structure or imports
File Type Guides Analysis:
- Widgets: Lifecycle, mounted, dispose
- BLoCs: Encapsulation, events, states
- Providers: Disposal, context usage
- Models: Immutability, equality
- Services: Error handling, async patterns
Key Principles
- Safety First - P0 issues can crash apps, prioritize these
- Be Specific - Always provide file:line, code snippets, fixes
- Avoid False Positives - When uncertain, don't flag or mark P2
- Context Matters - Understand intent before flagging
- Actionable Feedback - Every issue needs clear, copy-paste ready fix
- Respect Patterns - Understand project architecture before suggesting changes
Additional Resources
For detailed explanations of each check, see reference.md.
For code examples showing good vs bad patterns, see examples.md.
Converted and distributed by TomeVault — claim your Tome and manage your conversions.