From 1320e9051f6743d87ba628dcc67d65ccc9a008fd Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Wed, 20 May 2020 23:33:26 -0700 Subject: [PATCH] null-annotate SemanticsUpdateBuilder.updateNode The underlying _updateNode call requires that all parameters be set non-null. There's a single call site in the framework in lib/src/semantics/semantics.dart in SemanticsNode.updateWith(). At that call site, all parameters are either asserted non-null in the constructor of SemanticsData or defaulted to null, with the sole exception of textDirection. The ergonomics of this method are currently pretty ugly and we should consider migrating most of the defaulting and assertions that we apply at the call site up to the definition in dart:ui. That work is filed as https://github.com/flutter/flutter/issues/57720. --- lib/ui/semantics.dart | 158 +++++++++++++++------------ lib/web_ui/lib/src/ui/semantics.dart | 80 +++++++++----- 2 files changed, 143 insertions(+), 95 deletions(-) diff --git a/lib/ui/semantics.dart b/lib/ui/semantics.dart index c0b32450fbd58..cddb3f2c79b63 100644 --- a/lib/ui/semantics.dart +++ b/lib/ui/semantics.dart @@ -645,19 +645,21 @@ class SemanticsUpdateBuilder extends NativeFieldWrapperClass2 { /// reading direction of all these strings is given by `textDirection`. /// /// The fields `textSelectionBase` and `textSelectionExtent` describe the - /// currently selected text within `value`. + /// currently selected text within `value`. A value of -1 indicates no + /// current text selection base or extent. /// - /// The field `maxValueLength` is used to indicate that an editable text field - /// has a limit on the number of characters entered. If it is -1 there is - /// no limit on the number of characters entered. The field + /// The field `maxValueLength` is used to indicate that an editable text + /// field has a limit on the number of characters entered. If it is -1 there + /// is no limit on the number of characters entered. The field /// `currentValueLength` indicates how much of that limit has already been - /// used up. When `maxValueLength` is set, `currentValueLength` must also be - /// set. + /// used up. When `maxValueLength` is >= 0, `currentValueLength` must also be + /// >= 0, otherwise it should be specified to be -1. /// /// The field `platformViewId` references the platform view, whose semantics /// nodes will be added as children to this node. If a platform view is - /// specified, `childrenInHitTestOrder` and `childrenInTraversalOrder` must be - /// empty. + /// specified, `childrenInHitTestOrder` and `childrenInTraversalOrder` must + /// be empty. A value of -1 indicates that this node is not associated with a + /// platform view. /// /// For scrollable nodes `scrollPosition` describes the current scroll /// position in logical pixel. `scrollExtentMax` and `scrollExtentMin` @@ -681,39 +683,61 @@ class SemanticsUpdateBuilder extends NativeFieldWrapperClass2 { /// z-direction starting at `elevation`. Basically, in the z-direction the /// node starts at `elevation` above the parent and ends at `elevation` + /// `thickness` above the parent. + // TODO(cbracken): https://github.com/flutter/flutter/issues/57720 void updateNode({ - int id, - int flags, - int actions, - int maxValueLength, - int currentValueLength, - int textSelectionBase, - int textSelectionExtent, - int platformViewId, - int scrollChildren, - int scrollIndex, - double scrollPosition, - double scrollExtentMax, - double scrollExtentMin, - double elevation, - double thickness, - Rect rect, - String label, - String hint, - String value, - String increasedValue, - String decreasedValue, - TextDirection textDirection, - Float64List transform, - Int32List childrenInTraversalOrder, - Int32List childrenInHitTestOrder, - Int32List additionalActions, + /*required*/ int/*!*/ id, + /*required*/ int/*!*/ flags, + /*required*/ int/*!*/ actions, + /*required*/ int/*!*/ maxValueLength, + /*required*/ int/*!*/ currentValueLength, + /*required*/ int/*!*/ textSelectionBase, + /*required*/ int/*!*/ textSelectionExtent, + /*required*/ int/*!*/ platformViewId, + /*required*/ int/*!*/ scrollChildren, + /*required*/ int/*!*/ scrollIndex, + /*required*/ double/*!*/ scrollPosition, + /*required*/ double/*!*/ scrollExtentMax, + /*required*/ double/*!*/ scrollExtentMin, + /*required*/ double/*!*/ elevation, + /*required*/ double/*!*/ thickness, + /*required*/ Rect/*!*/ rect, + /*required*/ String/*!*/ label, + /*required*/ String/*!*/ hint, + /*required*/ String/*!*/ value, + /*required*/ String/*!*/ increasedValue, + /*required*/ String/*!*/ decreasedValue, + TextDirection/*?*/ textDirection, + /*required*/ Float64List/*!*/ transform, + /*required*/ Int32List/*!*/ childrenInTraversalOrder, + /*required*/ Int32List/*!*/ childrenInHitTestOrder, + /*required*/ Int32List/*!*/ additionalActions, }) { + assert(id != null); + assert(flags != null); + assert(actions != null); + assert(maxValueLength != null); + assert(currentValueLength != null); + assert(textSelectionBase != null); + assert(textSelectionExtent != null); + assert(platformViewId != null); + assert(scrollChildren != null); + assert(scrollIndex != null); + assert(scrollPosition != null); + assert(scrollExtentMax != null); + assert(scrollExtentMin != null); + assert(elevation != null); + assert(thickness != null); + assert(rect != null); + assert(label != null); + assert(hint != null); + assert(value != null); + assert(increasedValue != null); + assert(decreasedValue != null); + assert(transform != null); + assert(childrenInTraversalOrder != null); + assert(childrenInHitTestOrder != null); + assert(additionalActions != null); assert(_matrix4IsValid(transform)); - assert( - scrollChildren == 0 || scrollChildren == null || (scrollChildren > 0 && childrenInHitTestOrder != null), - 'If a node has scrollChildren, it must have childrenInHitTestOrder', - ); _updateNode( id, flags, @@ -747,35 +771,35 @@ class SemanticsUpdateBuilder extends NativeFieldWrapperClass2 { ); } void _updateNode( - int id, - int flags, - int actions, - int maxValueLength, - int currentValueLength, - int textSelectionBase, - int textSelectionExtent, - int platformViewId, - int scrollChildren, - int scrollIndex, - double scrollPosition, - double scrollExtentMax, - double scrollExtentMin, - double left, - double top, - double right, - double bottom, - double elevation, - double thickness, - String label, - String hint, - String value, - String increasedValue, - String decreasedValue, - int textDirection, - Float64List transform, - Int32List childrenInTraversalOrder, - Int32List childrenInHitTestOrder, - Int32List additionalActions, + int/*!*/ id, + int/*!*/ flags, + int/*!*/ actions, + int/*!*/ maxValueLength, + int/*!*/ currentValueLength, + int/*!*/ textSelectionBase, + int/*!*/ textSelectionExtent, + int/*!*/ platformViewId, + int/*!*/ scrollChildren, + int/*!*/ scrollIndex, + double/*!*/ scrollPosition, + double/*!*/ scrollExtentMax, + double/*!*/ scrollExtentMin, + double/*!*/ left, + double/*!*/ top, + double/*!*/ right, + double/*!*/ bottom, + double/*!*/ elevation, + double/*!*/ thickness, + String/*!*/ label, + String/*!*/ hint, + String/*!*/ value, + String/*!*/ increasedValue, + String/*!*/ decreasedValue, + int/*!*/ textDirection, + Float64List/*!*/ transform, + Int32List/*!*/ childrenInTraversalOrder, + Int32List/*!*/ childrenInHitTestOrder, + Int32List/*!*/ additionalActions, ) native 'SemanticsUpdateBuilder_updateNode'; /// Update the custom semantics action associated with the given `id`. diff --git a/lib/web_ui/lib/src/ui/semantics.dart b/lib/web_ui/lib/src/ui/semantics.dart index a2a02b4387f71..d06bfbdc46b46 100644 --- a/lib/web_ui/lib/src/ui/semantics.dart +++ b/lib/web_ui/lib/src/ui/semantics.dart @@ -665,35 +665,59 @@ class SemanticsUpdateBuilder { /// The `transform` is a matrix that maps this node's coordinate system into /// its parent's coordinate system. void updateNode({ - int id, - int flags, - int actions, - int maxValueLength, - int currentValueLength, - int textSelectionBase, - int textSelectionExtent, - int platformViewId, - int scrollChildren, - int scrollIndex, - double scrollPosition, - double scrollExtentMax, - double scrollExtentMin, - double elevation, - double thickness, - Rect rect, - String label, - String hint, - String value, - String increasedValue, - String decreasedValue, - TextDirection textDirection, - Float64List transform, - Int32List childrenInTraversalOrder, - Int32List childrenInHitTestOrder, - Int32List additionalActions, + /*required*/ int/*!*/ id, + /*required*/ int/*!*/ flags, + /*required*/ int/*!*/ actions, + /*required*/ int/*!*/ maxValueLength, + /*required*/ int/*!*/ currentValueLength, + /*required*/ int/*!*/ textSelectionBase, + /*required*/ int/*!*/ textSelectionExtent, + /*required*/ int/*!*/ platformViewId, + /*required*/ int/*!*/ scrollChildren, + /*required*/ int/*!*/ scrollIndex, + /*required*/ double/*!*/ scrollPosition, + /*required*/ double/*!*/ scrollExtentMax, + /*required*/ double/*!*/ scrollExtentMin, + /*required*/ double/*!*/ elevation, + /*required*/ double/*!*/ thickness, + /*required*/ Rect/*!*/ rect, + /*required*/ String/*!*/ label, + /*required*/ String/*!*/ hint, + /*required*/ String/*!*/ value, + /*required*/ String/*!*/ increasedValue, + /*required*/ String/*!*/ decreasedValue, + TextDirection/*?*/ textDirection, + /*required*/ Float64List/*!*/ transform, + /*required*/ Int32List/*!*/ childrenInTraversalOrder, + /*required*/ Int32List/*!*/ childrenInHitTestOrder, + /*required*/ Int32List/*!*/ additionalActions, }) { - if (transform.length != 16) - throw ArgumentError('transform argument must have 16 entries.'); + assert(id != null); + assert(flags != null); + assert(actions != null); + assert(maxValueLength != null); + assert(currentValueLength != null); + assert(textSelectionBase != null); + assert(textSelectionExtent != null); + assert(platformViewId != null); + assert(scrollChildren != null); + assert(scrollIndex != null); + assert(scrollPosition != null); + assert(scrollExtentMax != null); + assert(scrollExtentMin != null); + assert(elevation != null); + assert(thickness != null); + assert(rect != null); + assert(label != null); + assert(hint != null); + assert(value != null); + assert(increasedValue != null); + assert(decreasedValue != null); + assert(transform != null); + assert(childrenInTraversalOrder != null); + assert(childrenInHitTestOrder != null); + assert(additionalActions != null); + assert(transform.length == 16, 'transform argument must have 16 entries.'); _nodeUpdates.add(engine.SemanticsNodeUpdate( id: id, flags: flags,