From bb0c3172f84abb5d55986df48ed76a60e8efc0e4 Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Wed, 26 Jul 2023 18:21:05 -0500 Subject: [PATCH] Minor adjustments on 2D APIs (#131358) These tweaks came from https://github.com/flutter/packages/pull/4536 - The TwoDimensionalChildBuilderDelegate asserts that maxXIndex and maxYIndex are null or >= 0 - The TwoDimensionalChildDelegate setter in RenderTwoDimensionalViewport has a covariant to allow type safety for subclasses of RenderTwoDimensionalViewport implementing with other subclasses of TwoDimensionalChildDelegate I'd like to cherry pick this so https://github.com/flutter/packages/pull/4536 will not have to wait for it to reach stable. --- .../lib/src/widgets/scroll_delegate.dart | 8 +- .../src/widgets/two_dimensional_viewport.dart | 2 +- .../two_dimensional_viewport_test.dart | 97 +++++++++++++++++++ 3 files changed, 105 insertions(+), 2 deletions(-) diff --git a/packages/flutter/lib/src/widgets/scroll_delegate.dart b/packages/flutter/lib/src/widgets/scroll_delegate.dart index 28f9e28e13a..765bf31d8a1 100644 --- a/packages/flutter/lib/src/widgets/scroll_delegate.dart +++ b/packages/flutter/lib/src/widgets/scroll_delegate.dart @@ -933,7 +933,9 @@ class TwoDimensionalChildBuilderDelegate extends TwoDimensionalChildDelegate { required this.builder, int? maxXIndex, int? maxYIndex, - }) : _maxYIndex = maxYIndex, + }) : assert(maxYIndex == null || maxYIndex >= 0), + assert(maxXIndex == null || maxXIndex >= 0), + _maxYIndex = maxYIndex, _maxXIndex = maxXIndex; /// Called to build children on demand. @@ -974,6 +976,8 @@ class TwoDimensionalChildBuilderDelegate extends TwoDimensionalChildDelegate { /// [TwoDimensionalViewport] subclass to learn how this value is applied in /// the specific use case. /// + /// If not null, the value must be non-negative. + /// /// If the value changes, the delegate will call [notifyListeners]. This /// informs the [RenderTwoDimensionalViewport] that any cached information /// from the delegate is invalid. @@ -993,6 +997,7 @@ class TwoDimensionalChildBuilderDelegate extends TwoDimensionalChildDelegate { if (value == maxXIndex) { return; } + assert(value == null || value >= 0); _maxXIndex = value; notifyListeners(); } @@ -1015,6 +1020,7 @@ class TwoDimensionalChildBuilderDelegate extends TwoDimensionalChildDelegate { if (maxYIndex == value) { return; } + assert(value == null || value >= 0); _maxYIndex = value; notifyListeners(); } diff --git a/packages/flutter/lib/src/widgets/two_dimensional_viewport.dart b/packages/flutter/lib/src/widgets/two_dimensional_viewport.dart index 69976eb1e48..82befbce084 100644 --- a/packages/flutter/lib/src/widgets/two_dimensional_viewport.dart +++ b/packages/flutter/lib/src/widgets/two_dimensional_viewport.dart @@ -612,7 +612,7 @@ abstract class RenderTwoDimensionalViewport extends RenderBox implements RenderA /// Supplies children for layout in the viewport. TwoDimensionalChildDelegate get delegate => _delegate; TwoDimensionalChildDelegate _delegate; - set delegate(TwoDimensionalChildDelegate value) { + set delegate(covariant TwoDimensionalChildDelegate value) { if (_delegate == value) { return; } diff --git a/packages/flutter/test/widgets/two_dimensional_viewport_test.dart b/packages/flutter/test/widgets/two_dimensional_viewport_test.dart index b44fac359e3..a445d3412c9 100644 --- a/packages/flutter/test/widgets/two_dimensional_viewport_test.dart +++ b/packages/flutter/test/widgets/two_dimensional_viewport_test.dart @@ -111,6 +111,78 @@ void main() { ); }, variant: TargetPlatformVariant.all()); + test('maxXIndex and maxYIndex assertions', () { + final TwoDimensionalChildBuilderDelegate delegate = TwoDimensionalChildBuilderDelegate( + maxXIndex: 0, + maxYIndex: 0, + builder: (BuildContext context, ChildVicinity vicinity) { + return const SizedBox.shrink(); + } + ); + // Update + expect( + () { + delegate.maxXIndex = -1; + }, + throwsA( + isA().having( + (AssertionError error) => error.toString(), + 'description', + contains('value == null || value >= 0'), + ), + ), + ); + expect( + () { + delegate.maxYIndex = -1; + }, + throwsA( + isA().having( + (AssertionError error) => error.toString(), + 'description', + contains('value == null || value >= 0'), + ), + ), + ); + // Constructor + expect( + () { + TwoDimensionalChildBuilderDelegate( + maxXIndex: -1, + maxYIndex: 0, + builder: (BuildContext context, ChildVicinity vicinity) { + return const SizedBox.shrink(); + } + ); + }, + throwsA( + isA().having( + (AssertionError error) => error.toString(), + 'description', + contains('maxXIndex == null || maxXIndex >= 0'), + ), + ), + ); + expect( + () { + TwoDimensionalChildBuilderDelegate( + maxXIndex: 0, + maxYIndex: -1, + builder: (BuildContext context, ChildVicinity vicinity) { + return const SizedBox.shrink(); + } + ); + }, + throwsA( + isA().having( + (AssertionError error) => error.toString(), + 'description', + contains('maxYIndex == null || maxYIndex >= 0'), + ), + ), + ); + }); + testWidgets('throws an error when builder throws', (WidgetTester tester) async { final List exceptions = []; final FlutterExceptionHandler? oldHandler = FlutterError.onError; @@ -1822,3 +1894,28 @@ Future restoreScrollAndVerify(WidgetTester tester) async { 100.0, ); } + +// Validates covariant through analysis. +mixin _SomeDelegateMixin on TwoDimensionalChildDelegate {} + +class _SomeRenderTwoDimensionalViewport extends RenderTwoDimensionalViewport { // ignore: unused_element + _SomeRenderTwoDimensionalViewport({ + required super.horizontalOffset, + required super.horizontalAxisDirection, + required super.verticalOffset, + required super.verticalAxisDirection, + required _SomeDelegateMixin super.delegate, + required super.mainAxis, + required super.childManager, + }); + + @override + _SomeDelegateMixin get delegate => super.delegate as _SomeDelegateMixin; + @override + set delegate(_SomeDelegateMixin value) { // Analysis would fail without covariant + super.delegate = value; + } + + @override + void layoutChildSequence() {} +}