Remove nullOk parameter from Focus.of, FocusTraversalOrder.of, and Fo…

…cusTraversalGroup.of
flutter · gspencergoog · Oct 29, 2020 · Oct 23, 2020 · Oct 28, 2020 · e224bcbf27212dd8d5d9f94f4b2a52c2c490e2fe
commit e224bcbf27212dd8d5d9f94f4b2a52c2c490e2fe
diff --git a/packages/flutter/lib/src/material/dropdown.dart b/packages/flutter/lib/src/material/dropdown.dart
@@ -1513,7 +1513,7 @@ class DropdownButtonFormField<T> extends FormField<T> {
                return InputDecorator(
                  decoration: effectiveDecoration.copyWith(errorText: field.errorText),
                  isEmpty: state.value == null,
-                 isFocused: Focus.of(context)!.hasFocus,
+                 isFocused: Focus.of(context).hasFocus,
                  child: DropdownButtonHideUnderline(
                    child: DropdownButton<T>(
                      items: items,

diff --git a/packages/flutter/lib/src/widgets/focus_manager.dart b/packages/flutter/lib/src/widgets/focus_manager.dart
@@ -143,7 +143,7 @@ class FocusAttachment {
     assert(_node != null);
     if (isAttached) {
       assert(_node.context != null);
-      parent ??= Focus.of(_node.context!, nullOk: true, scopeOk: true);
+      parent ??= Focus.maybeOf(_node.context!, scopeOk: true);
       parent ??= _node.context!.owner!.focusManager.rootScope;
       assert(parent != null);
       parent._reparent(_node);
@@ -1005,7 +1005,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
       _manager?.primaryFocus?._setAsFocusedChildForScope();
     }
     if (oldScope != null && child.context != null && child.enclosingScope != oldScope) {
-      FocusTraversalGroup.of(child.context!, nullOk: true)?.changedScope(node: child, oldScope: oldScope);
+      FocusTraversalGroup.maybeOf(child.context!)?.changedScope(node: child, oldScope: oldScope);
     }
     if (child._requestFocusWhenReparented) {
       child._doRequestFocus(findFirstFocus: true);
@@ -1145,19 +1145,19 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
   /// [FocusTraversalPolicy.next] method.
   ///
   /// Returns true if it successfully found a node and requested focus.
-  bool nextFocus() => FocusTraversalGroup.of(context!)!.next(this);
+  bool nextFocus() => FocusTraversalGroup.of(context!).next(this);
 
   /// Request to move the focus to the previous focus node, by calling the
   /// [FocusTraversalPolicy.previous] method.
   ///
   /// Returns true if it successfully found a node and requested focus.
-  bool previousFocus() => FocusTraversalGroup.of(context!)!.previous(this);
+  bool previousFocus() => FocusTraversalGroup.of(context!).previous(this);
 
   /// Request to move the focus to the nearest focus node in the given
   /// direction, by calling the [FocusTraversalPolicy.inDirection] method.
   ///
   /// Returns true if it successfully found a node and requested focus.
-  bool focusInDirection(TraversalDirection direction) => FocusTraversalGroup.of(context!)!.inDirection(this, direction);
+  bool focusInDirection(TraversalDirection direction) => FocusTraversalGroup.of(context!).inDirection(this, direction);
 
   @override
   void debugFillProperties(DiagnosticPropertiesBuilder properties) {

diff --git a/packages/flutter/lib/src/widgets/focus_scope.dart b/packages/flutter/lib/src/widgets/focus_scope.dart
@@ -433,21 +433,24 @@ class Focus extends StatefulWidget {
   ///
   /// If no [Focus] node is found before reaching the nearest [FocusScope]
   /// widget, or there is no [Focus] widget in scope, then this method will
-  /// throw an exception. To return null instead of throwing, pass true for
-  /// [nullOk].
+  /// throw an exception.
   ///
-  /// The [context] and [nullOk] arguments must not be null.
+  /// The `context` and `scopeOk` arguments must not be null.
   ///
   /// Calling this function creates a dependency that will rebuild the given
   /// context when the focus changes.
-  static FocusNode? of(BuildContext context, { bool nullOk = false, bool scopeOk = false }) {
+  ///
+  /// See also:
+  ///
+  ///  * [maybeOf], which is similar to this function, but will return null
+  ///    instead of throwing if it doesn't find a [Focus] node.
+  static FocusNode of(BuildContext context, { bool scopeOk = false }) {
     assert(context != null);
-    assert(nullOk != null);
     assert(scopeOk != null);
     final _FocusMarker? marker = context.dependOnInheritedWidgetOfExactType<_FocusMarker>();
     final FocusNode? node = marker?.notifier;
-    if (node == null) {
-      if (!nullOk) {
+    assert(() {
+      if (node == null) {
         throw FlutterError(
           'Focus.of() was called with a context that does not contain a Focus widget.\n'
           'No Focus widget ancestor could be found starting from the context that was passed to '
@@ -457,10 +460,10 @@ class Focus extends StatefulWidget {
           '  $context'
         );
       }
-      return null;
-    }
-    if (!scopeOk && node is FocusScopeNode) {
-      if (!nullOk) {
+      return true;
+    }());
+    assert(() {
+      if (!scopeOk && node is FocusScopeNode) {
         throw FlutterError(
           'Focus.of() was called with a context that does not contain a Focus between the given '
           'context and the nearest FocusScope widget.\n'
@@ -472,6 +475,36 @@ class Focus extends StatefulWidget {
           '  $context'
         );
       }
+      return true;
+    }());
+    return node!;
+  }
+
+  /// Returns the [focusNode] of the [Focus] that most tightly encloses the
+  /// given [BuildContext].
+  ///
+  /// If no [Focus] node is found before reaching the nearest [FocusScope]
+  /// widget, or there is no [Focus] widget in scope, then this method will
+  /// return null.
+  ///
+  /// The `context` and `scopeOk` arguments must not be null.
+  ///
+  /// Calling this function creates a dependency that will rebuild the given
+  /// context when the focus changes.
+  ///
+  /// See also:
+  ///
+  ///  * [of], which is similar to this function, but will throw an exception if
+  ///    it doesn't find a [Focus] node instead of returning null.
+  static FocusNode? maybeOf(BuildContext context, { bool scopeOk = false }) {
+    assert(context != null);
+    assert(scopeOk != null);
+    final _FocusMarker? marker = context.dependOnInheritedWidgetOfExactType<_FocusMarker>();
+    final FocusNode? node = marker?.notifier;
+    if (node == null) {
+      return null;
+    }
+    if (!scopeOk && node is FocusScopeNode) {
       return null;
     }
     return node;
@@ -489,7 +522,7 @@ class Focus extends StatefulWidget {
   ///
   /// Calling this function creates a dependency that will rebuild the given
   /// context when the focus changes.
-  static bool isAt(BuildContext context) => Focus.of(context, nullOk: true)?.hasFocus ?? false;
+  static bool isAt(BuildContext context) => Focus.maybeOf(context)?.hasFocus ?? false;
 
   @override
   void debugFillProperties(DiagnosticPropertiesBuilder properties) {

diff --git a/packages/flutter/lib/src/widgets/focus_traversal.dart b/packages/flutter/lib/src/widgets/focus_traversal.dart
@@ -1363,7 +1363,7 @@ class OrderedTraversalPolicy extends FocusTraversalPolicy with DirectionalFocusT
     final List<FocusNode> unordered = <FocusNode>[];
     final List<_OrderedFocusInfo> ordered = <_OrderedFocusInfo>[];
     for (final FocusNode node in sortedDescendants) {
-      final FocusOrder? order = FocusTraversalOrder.of(node.context!, nullOk: true);
+      final FocusOrder? order = FocusTraversalOrder.maybeOf(node.context!);
       if (order != null) {
         ordered.add(_OrderedFocusInfo(node: node, order: order));
       } else {
@@ -1393,30 +1393,49 @@ class OrderedTraversalPolicy extends FocusTraversalPolicy with DirectionalFocusT
 /// [FocusTraversalOrder.of] for a particular context.
 class FocusTraversalOrder extends InheritedWidget {
   /// A const constructor so that subclasses can be const.
-  const FocusTraversalOrder({Key? key, this.order, required Widget child}) : super(key: key, child: child);
+  const FocusTraversalOrder({Key? key, required this.order, required Widget child}) : super(key: key, child: child);
 
   /// The order for the widget descendants of this [FocusTraversalOrder].
-  final FocusOrder? order;
+  final FocusOrder order;
 
   /// Finds the [FocusOrder] in the nearest ancestor [FocusTraversalOrder] widget.
   ///
   /// It does not create a rebuild dependency because changing the traversal
   /// order doesn't change the widget tree, so nothing needs to be rebuilt as a
   /// result of an order change.
-  static FocusOrder? of(BuildContext context, {bool nullOk = false}) {
+  ///
+  /// If no [FocusTraversalOrder] ancestor exists, or the order is null, this
+  /// will assert in debug mode, and throw an exception in release mode.
+  static FocusOrder of(BuildContext context) {
     assert(context != null);
-    assert(nullOk != null);
     final FocusTraversalOrder? marker = context.getElementForInheritedWidgetOfExactType<FocusTraversalOrder>()?.widget as FocusTraversalOrder?;
-    final FocusOrder? order = marker?.order;
-    if (order == null && !nullOk) {
-      throw FlutterError('FocusTraversalOrder.of() was called with a context that '
-          'does not contain a TraversalOrder widget. No TraversalOrder widget '
+    assert((){
+      if (marker == null) {
+        throw FlutterError(
+          'FocusTraversalOrder.of() was called with a context that '
+          'does not contain a FocusTraversalOrder widget. No TraversalOrder widget '
           'ancestor could be found starting from the context that was passed to '
           'FocusTraversalOrder.of().\n'
           'The context used was:\n'
-          '  $context');
-    }
-    return order;
+          '  $context',
+        );
+      }
+      return true;
+    }());
+    return marker!.order;
+  }
+
+  /// Finds the [FocusOrder] in the nearest ancestor [FocusTraversalOrder] widget.
+  ///
+  /// It does not create a rebuild dependency because changing the traversal
+  /// order doesn't change the widget tree, so nothing needs to be rebuilt as a
+  /// result of an order change.
+  ///
+  /// If no [FocusTraversalOrder] ancestor exists, or the order is null, returns null.
+  static FocusOrder? maybeOf(BuildContext context) {
+    assert(context != null);
+    final FocusTraversalOrder? marker = context.getElementForInheritedWidgetOfExactType<FocusTraversalOrder>()?.widget as FocusTraversalOrder?;
+    return marker?.order;
   }
 
   // Since the order of traversal doesn't affect display of anything, we don't
@@ -1670,17 +1689,16 @@ class FocusTraversalGroup extends StatefulWidget {
   /// order doesn't change the widget tree, so nothing needs to be rebuilt as a
   /// result of an order change.
   ///
-  /// Will assert if no [FocusTraversalGroup] ancestor is found, and `nullOk` is false.
+  /// Will assert if no [FocusTraversalGroup] ancestor is found.
   ///
-  /// If `nullOk` is true, then it will return null if it doesn't find a
-  /// [FocusTraversalGroup] ancestor.
-  static FocusTraversalPolicy? of(BuildContext context, {bool nullOk = false}) {
+  /// See also:
+  ///
+  ///  * [maybeOf] for a similar function that will return null if no
+  ///    [FocusTraversalGroup] ancestor is found.
+  static FocusTraversalPolicy of(BuildContext context) {
     assert(context != null);
     final _FocusTraversalGroupMarker? inherited = context.dependOnInheritedWidgetOfExactType<_FocusTraversalGroupMarker>();
     assert(() {
-      if (nullOk) {
-        return true;
-      }
       if (inherited == null) {
         throw FlutterError(
           'Unable to find a FocusTraversalGroup widget in the context.\n'
@@ -1696,6 +1714,25 @@ class FocusTraversalGroup extends StatefulWidget {
       }
       return true;
     }());
+    return inherited!.policy;
+  }
+
+  /// Returns the focus policy set by the [FocusTraversalGroup] that most
+  /// tightly encloses the given [BuildContext].
+  ///
+  /// It does not create a rebuild dependency because changing the traversal
+  /// order doesn't change the widget tree, so nothing needs to be rebuilt as a
+  /// result of an order change.
+  ///
+  /// Will return null if it doesn't find a [FocusTraversalGroup] ancestor.
+  ///
+  /// See also:
+  ///
+  ///  * [of] for a similar function that will throw if no [FocusTraversalGroup]
+  ///    ancestor is found.
+  static FocusTraversalPolicy? maybeOf(BuildContext context) {
+    assert(context != null);
+    final _FocusTraversalGroupMarker? inherited = context.dependOnInheritedWidgetOfExactType<_FocusTraversalGroupMarker>();
     return inherited?.policy;
   }
 

diff --git a/packages/flutter/test/material/checkbox_list_tile_test.dart b/packages/flutter/test/material/checkbox_list_tile_test.dart
@@ -98,7 +98,7 @@ void main() {
     );
 
     await tester.pump();
-    expect(Focus.of(childKey.currentContext!, nullOk: true)!.hasPrimaryFocus, isTrue);
+    expect(Focus.maybeOf(childKey.currentContext!)!.hasPrimaryFocus, isTrue);
 
     await tester.pumpWidget(
       wrap(
@@ -112,7 +112,7 @@ void main() {
     );
 
     await tester.pump();
-    expect(Focus.of(childKey.currentContext!, nullOk: true)!.hasPrimaryFocus, isFalse);
+    expect(Focus.maybeOf(childKey.currentContext!)!.hasPrimaryFocus, isFalse);
   });
 
   testWidgets('CheckboxListTile contentPadding test', (WidgetTester tester) async {

diff --git a/packages/flutter/test/material/dropdown_test.dart b/packages/flutter/test/material/dropdown_test.dart
@@ -2328,7 +2328,7 @@ void main() {
     await tester.pump();
     await tester.pump(const Duration(seconds: 1)); // finish the menu open animation
     expect(value, equals('one'));
-    expect(Focus.of(tester.element(find.byKey(const ValueKey<String>('one')).last))!.hasPrimaryFocus, isTrue);
+    expect(Focus.of(tester.element(find.byKey(const ValueKey<String>('one')).last)).hasPrimaryFocus, isTrue);
 
     await tester.sendKeyEvent(LogicalKeyboardKey.tab); // Focus 'two'
     await tester.pump();
@@ -2345,7 +2345,7 @@ void main() {
     await tester.pump(const Duration(seconds: 1)); // finish the menu open animation
     expect(value, equals('two'));
     final Element element = tester.element(find.byKey(const ValueKey<String>('two')).last);
-    final FocusNode node = Focus.of(element)!;
+    final FocusNode node = Focus.of(element);
     expect(node.hasFocus, isTrue);
   }, skip: isBrowser); // https://github.com/flutter/flutter/issues/55320
 
@@ -2392,7 +2392,7 @@ void main() {
     await tester.pump();
     await tester.pump(const Duration(seconds: 1)); // finish the menu open animation
     expect(value, equals(1));
-    expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(1)).last))!.hasPrimaryFocus, isTrue);
+    expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(1)).last)).hasPrimaryFocus, isTrue);
 
     for (int i = 0; i < 41; ++i) {
       await tester.sendKeyEvent(LogicalKeyboardKey.tab); // Move to the next one.
@@ -2408,7 +2408,7 @@ void main() {
     await tester.pump(const Duration(seconds: 1)); // finish the menu open animation
     expect(value, equals(42));
     final Element element = tester.element(find.byKey(const ValueKey<int>(42)).last);
-    final FocusNode node = Focus.of(element)!;
+    final FocusNode node = Focus.of(element);
     expect(node.hasFocus, isTrue);
   }, skip: isBrowser); // https://github.com/flutter/flutter/issues/55320
 
@@ -2454,14 +2454,14 @@ void main() {
     await tester.sendKeyEvent(LogicalKeyboardKey.enter);
     await tester.pumpAndSettle();
     expect(value, equals(1));
-    expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(1)).last))!.hasPrimaryFocus, isTrue);
+    expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(1)).last)).hasPrimaryFocus, isTrue);
 
     // Move to an item very far down the menu.
     for (int i = 0; i < 90; ++i) {
       await tester.sendKeyEvent(LogicalKeyboardKey.tab); // Move to the next one.
       await tester.pumpAndSettle(); // Wait for it to animate the menu.
     }
-    expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(91)).last))!.hasPrimaryFocus, isTrue);
+    expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(91)).last)).hasPrimaryFocus, isTrue);
 
     // Scroll back to the top using touch, and make sure we end up there.
     final Finder menu = find.byWidgetPredicate((Widget widget) {
@@ -2483,7 +2483,7 @@ void main() {
 
     // Scrolling to the top again has removed the one the focus was on from the
     // tree, causing it to lose focus.
-    expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(91)).last))!.hasPrimaryFocus, isFalse);
+    expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(91)).last)).hasPrimaryFocus, isFalse);
   }, skip: isBrowser); // https://github.com/flutter/flutter/issues/55320
 
   testWidgets('DropdownButton onTap callback is called when defined', (WidgetTester tester) async {

diff --git a/packages/flutter/test/material/list_tile_test.dart b/packages/flutter/test/material/list_tile_test.dart
@@ -1173,10 +1173,10 @@ void main() {
     );
     await tester.pump(); // Let the focus take effect.
 
-    final FocusNode? tileNode = Focus.of(childKey.currentContext!);
-    tileNode!.requestFocus();
+    final FocusNode tileNode = Focus.of(childKey.currentContext!);
+    tileNode.requestFocus();
     await tester.pump(); // Let the focus take effect.
-    expect(Focus.of(childKey.currentContext!, nullOk: true)!.hasPrimaryFocus, isTrue);
+    expect(Focus.of(childKey.currentContext!).hasPrimaryFocus, isTrue);
 
     expect(tileNode.hasPrimaryFocus, isTrue);
     await tester.pumpWidget(
@@ -1197,7 +1197,7 @@ void main() {
     );
 
     expect(tester.binding.focusManager.primaryFocus, isNot(equals(tileNode)));
-    expect(Focus.of(childKey.currentContext!, nullOk: true)!.hasPrimaryFocus, isFalse);
+    expect(Focus.of(childKey.currentContext!).hasPrimaryFocus, isFalse);
   });
 
   testWidgets('ListTile can autofocus unless disabled.', (WidgetTester tester) async {
@@ -1222,7 +1222,7 @@ void main() {
     );
 
     await tester.pump();
-    expect(Focus.of(childKey.currentContext!, nullOk: true)!.hasPrimaryFocus, isTrue);
+    expect(Focus.of(childKey.currentContext!).hasPrimaryFocus, isTrue);
 
     await tester.pumpWidget(
       MaterialApp(
@@ -1243,7 +1243,7 @@ void main() {
     );
 
     await tester.pump();
-    expect(Focus.of(childKey.currentContext!, nullOk: true)!.hasPrimaryFocus, isFalse);
+    expect(Focus.of(childKey.currentContext!).hasPrimaryFocus, isFalse);
   });
 
   testWidgets('ListTile is focusable and has correct focus color', (WidgetTester tester) async {