Handle a missing ListView separator as an error (#24312)

* Handle a missing ListView separator as an error

* Handle missing item, and errors in itemBuilder and separatorBuilder

* CR fixes and move error handling into sliver.dart to handle all ListView constructors

* Only show an error for null separatorBuilder value in debug mode

packages/flutter/lib/src/widgets/scroll_view.dart

@@ -889,9 +889,19 @@ class ListView extends BoxScrollView {
        childrenDelegate = SliverChildBuilderDelegate(
          (BuildContext context, int index) {
            final int itemIndex = index ~/ 2;
-           return index.isEven
-             ? itemBuilder(context, itemIndex)
-             : separatorBuilder(context, itemIndex);
+           Widget widget;
+           if (index.isEven) {
+             widget = itemBuilder(context, itemIndex);
+           } else {
+             widget = separatorBuilder(context, itemIndex);
+             assert(() {
+               if (widget == null) {
+                 throw FlutterError('separatorBuilder cannot return null.');
+               }
+               return true;
+             }());
+           }
+           return widget;
          },
          childCount: _computeSemanticChildCount(itemCount),
          addAutomaticKeepAlives: addAutomaticKeepAlives,

packages/flutter/lib/src/widgets/sliver.dart

@@ -392,7 +392,12 @@ class SliverChildBuilderDelegate extends SliverChildDelegate {
     assert(builder != null);
     if (index < 0 || (childCount != null && index >= childCount))
       return null;
-    Widget child = builder(context, index);
+    Widget child;
+    try {
+      child = builder(context, index);
+    } catch (exception, stackTrace) {
+      child = _createErrorWidget(exception, stackTrace);
+    }
     if (child == null)
       return null;
     if (addRepaintBoundaries)
@@ -1267,3 +1272,16 @@ class KeepAlive extends ParentDataWidget<SliverMultiBoxAdaptorWidget> {
     properties.add(DiagnosticsProperty<bool>('keepAlive', keepAlive));
   }
 }
+
+// Return an ErrorWidget for the given Exception
+ErrorWidget _createErrorWidget(dynamic exception, StackTrace stackTrace) {
+  final FlutterErrorDetails details = FlutterErrorDetails(
+    exception: exception,
+    stack: stackTrace,
+    library: 'widgets library',
+    context: 'building',
+    informationCollector: null,
+  );
+  FlutterError.reportError(details);
+  return ErrorWidget.builder(details);
+}

packages/flutter/test/widgets/scroll_view_test.dart

@@ -4,6 +4,7 @@
 
 import 'package:flutter_test/flutter_test.dart';
 import 'package:flutter/widgets.dart';
+import 'package:flutter/material.dart';
 
 import 'states.dart';
 
@@ -410,4 +411,134 @@ void main() {
     await tester.dragFrom(const Offset(100.0, 100.0), const Offset(0.0, 100.0));
     expect(scrolled, isFalse);
   });
+
+  testWidgets('separatorBuilder must return something', (WidgetTester tester) async {
+    const List<String> listOfValues = <String>['ALPHA', 'BETA', 'GAMMA', 'DELTA'];
+
+    Widget buildFrame(Widget firstSeparator) {
+      return MaterialApp(
+        home: Material(
+          child: ListView.separated(
+            itemBuilder: (BuildContext context, int index) {
+              return Text(listOfValues[index]);
+            },
+            separatorBuilder: (BuildContext context, int index) {
+              if (index == 0) {
+                return firstSeparator;
+              } else  {
+                return const Divider();
+              }
+            },
+            itemCount: listOfValues.length,
+          ),
+        ),
+      );
+    }
+
+    // A separatorBuilder that always returns a Divider is fine
+    await tester.pumpWidget(buildFrame(const Divider()));
+    expect(tester.takeException(), isNull);
+
+    // A separatorBuilder that returns null throws a FlutterError
+    await tester.pumpWidget(buildFrame(null));
+    expect(tester.takeException(), isInstanceOf<FlutterError>());
+    expect(find.byType(ErrorWidget), findsOneWidget);
+  });
+
+  testWidgets('itemBuilder can return null', (WidgetTester tester) async {
+    const List<String> listOfValues = <String>['ALPHA', 'BETA', 'GAMMA', 'DELTA'];
+    const Key key = Key('list');
+    const int RENDER_NULL_AT = 2; // only render the first 2 values
+
+    Widget buildFrame() {
+      return MaterialApp(
+        home: Material(
+          child: ListView.builder(
+            key: key,
+            itemBuilder: (BuildContext context, int index) {
+              if (index == RENDER_NULL_AT) {
+                return null;
+              }
+              return Text(listOfValues[index]);
+            },
+            itemCount: listOfValues.length,
+          ),
+        ),
+      );
+    }
+
+    // The length of a list is itemCount or the index of the first itemBuilder
+    // that returns null, whichever is smaller
+    await tester.pumpWidget(buildFrame());
+    expect(tester.takeException(), isNull);
+    expect(find.byType(ErrorWidget), findsNothing);
+    expect(find.byType(Text), findsNWidgets(RENDER_NULL_AT));
+  });
+
+  testWidgets('when itemBuilder throws, creates Error Widget', (WidgetTester tester) async {
+    const List<String> listOfValues = <String>['ALPHA', 'BETA', 'GAMMA', 'DELTA'];
+
+    Widget buildFrame(bool throwOnFirstItem) {
+      return MaterialApp(
+        home: Material(
+          child: ListView.builder(
+            itemBuilder: (BuildContext context, int index) {
+              if (index == 0 && throwOnFirstItem) {
+                throw Exception('itemBuilder fail');
+              }
+              return Text(listOfValues[index]);
+            },
+            itemCount: listOfValues.length,
+          ),
+        ),
+      );
+    }
+
+    // When itemBuilder doesn't throw, no ErrorWidget
+    await tester.pumpWidget(buildFrame(false));
+    expect(tester.takeException(), isNull);
+    final Finder finder = find.byType(ErrorWidget);
+    expect(find.byType(ErrorWidget), findsNothing);
+
+    // When it does throw, one error widget is rendered in the item's place
+    await tester.pumpWidget(buildFrame(true));
+    expect(tester.takeException(), isInstanceOf<Exception>());
+    expect(finder, findsOneWidget);
+  });
+
+  testWidgets('when separatorBuilder throws, creates ErrorWidget', (WidgetTester tester) async {
+    const List<String> listOfValues = <String>['ALPHA', 'BETA', 'GAMMA', 'DELTA'];
+    const Key key = Key('list');
+
+    Widget buildFrame(bool throwOnFirstSeparator) {
+      return MaterialApp(
+        home: Material(
+          child: ListView.separated(
+            key: key,
+            itemBuilder: (BuildContext context, int index) {
+              return Text(listOfValues[index]);
+            },
+            separatorBuilder: (BuildContext context, int index) {
+              if (index == 0 && throwOnFirstSeparator) {
+                throw Exception('separatorBuilder fail');
+              }
+              return const Divider();
+            },
+            itemCount: listOfValues.length,
+          ),
+        ),
+      );
+    }
+
+    // When separatorBuilder doesn't throw, no ErrorWidget
+    await tester.pumpWidget(buildFrame(false));
+    expect(tester.takeException(), isNull);
+    final Finder finder = find.byType(ErrorWidget);
+    expect(find.byType(ErrorWidget), findsNothing);
+
+    // When it does throw, one error widget is rendered in the separator's place
+    await tester.pumpWidget(buildFrame(true));
+    expect(tester.takeException(), isInstanceOf<Exception>());
+    expect(finder, findsOneWidget);
+  });
 }