From 28366002d9a86ec1403764453df31580362c9097 Mon Sep 17 00:00:00 2001 From: Alexandre Ardhuin Date: Wed, 25 Oct 2017 08:25:44 +0200 Subject: [PATCH] enable lint prefer_foreach (#12674) * enable lint prefer_foreach * fix tests --- analysis_options.yaml | 2 +- analysis_options_repo.yaml | 2 +- .../flutter/lib/src/gestures/multidrag.dart | 3 +- .../flutter/lib/src/gestures/multitap.dart | 4 +- .../flutter/lib/src/painting/text_style.dart | 13 +++--- .../flutter/lib/src/rendering/object.dart | 6 +-- .../flutter/lib/src/rendering/semantics.dart | 5 +-- .../rendering/sliver_multi_box_adaptor.dart | 9 ++-- packages/flutter/lib/src/rendering/table.dart | 11 ++--- .../flutter/lib/src/widgets/framework.dart | 3 +- .../flutter/lib/src/widgets/navigator.dart | 6 +-- .../test/foundation/diagnostics_test.dart | 3 +- .../test/scheduler/scheduler_test.dart | 3 +- .../test/widgets/linked_scroll_view_test.dart | 9 ++-- packages/flutter_tools/lib/src/asset.dart | 4 +- packages/flutter_tools/lib/src/base/net.dart | 3 +- .../flutter_tools/lib/src/base/utils.dart | 6 +-- packages/flutter_tools/lib/src/compile.dart | 3 +- packages/flutter_tools/lib/src/devfs.dart | 3 +- .../flutter_tools/test/base/net_test.dart | 2 +- packages/flutter_tools/test/compile_test.dart | 44 ++++++++++++++----- 21 files changed, 66 insertions(+), 78 deletions(-) diff --git a/analysis_options.yaml b/analysis_options.yaml index 575b032fca..35b9e47300 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -119,7 +119,7 @@ linter: # - prefer_expression_function_bodies # conflicts with https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#consider-using--for-short-functions-and-methods - prefer_final_fields - prefer_final_locals - # - prefer_foreach # not yet tested + - prefer_foreach # - prefer_function_declarations_over_variables # not yet tested - prefer_initializing_formals # - prefer_interpolation_to_compose_strings # not yet tested diff --git a/analysis_options_repo.yaml b/analysis_options_repo.yaml index 0a50bfcbf2..eacdb1a510 100644 --- a/analysis_options_repo.yaml +++ b/analysis_options_repo.yaml @@ -113,7 +113,7 @@ linter: # - prefer_expression_function_bodies # conflicts with https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#consider-using--for-short-functions-and-methods - prefer_final_fields - prefer_final_locals - # - prefer_foreach # not yet tested + - prefer_foreach # - prefer_function_declarations_over_variables # not yet tested - prefer_initializing_formals # - prefer_interpolation_to_compose_strings # not yet tested diff --git a/packages/flutter/lib/src/gestures/multidrag.dart b/packages/flutter/lib/src/gestures/multidrag.dart index d5d54cdcff..7d9f60ec4a 100644 --- a/packages/flutter/lib/src/gestures/multidrag.dart +++ b/packages/flutter/lib/src/gestures/multidrag.dart @@ -293,8 +293,7 @@ abstract class MultiDragGestureRecognizer exten @override void dispose() { - for (int pointer in _pointers.keys.toList()) - _removeState(pointer); + _pointers.keys.toList().forEach(_removeState); assert(_pointers.isEmpty); _pointers = null; super.dispose(); diff --git a/packages/flutter/lib/src/gestures/multitap.dart b/packages/flutter/lib/src/gestures/multitap.dart index b13888cf3e..a40129e73f 100644 --- a/packages/flutter/lib/src/gestures/multitap.dart +++ b/packages/flutter/lib/src/gestures/multitap.dart @@ -199,9 +199,7 @@ class DoubleTapGestureRecognizer extends GestureRecognizer { } void _clearTrackers() { - final List<_TapTracker> localTrackers = new List<_TapTracker>.from(_trackers.values); - for (_TapTracker tracker in localTrackers) - _reject(tracker); + _trackers.values.toList().forEach(_reject); assert(_trackers.isEmpty); } diff --git a/packages/flutter/lib/src/painting/text_style.dart b/packages/flutter/lib/src/painting/text_style.dart index 98b10c6a9b..8f66e10d60 100644 --- a/packages/flutter/lib/src/painting/text_style.dart +++ b/packages/flutter/lib/src/painting/text_style.dart @@ -173,10 +173,10 @@ const String _kDefaultDebugLabel = 'unknown'; /// If the package internally uses the font it defines, it should still specify /// the `package` argument when creating the text style as in the example above. /// -/// A package can also provide font files without declaring a font in its -/// `pubspec.yaml`. These files should then be in the `lib/` folder of the -/// package. The font files will not automatically be bundled in the app, instead -/// the app can use these selectively when declaring a font. Suppose a package +/// A package can also provide font files without declaring a font in its +/// `pubspec.yaml`. These files should then be in the `lib/` folder of the +/// package. The font files will not automatically be bundled in the app, instead +/// the app can use these selectively when declaring a font. Suppose a package /// named `my_package` has: /// /// ``` @@ -197,7 +197,7 @@ const String _kDefaultDebugLabel = 'unknown'; /// /// The `lib/` is implied, so it should not be included in the asset path. /// -/// In this case, since the app locally defines the font, the TextStyle is +/// In this case, since the app locally defines the font, the TextStyle is /// created without the `package` argument: /// ///```dart @@ -709,8 +709,7 @@ class TextStyle extends Diagnosticable { final bool styleSpecified = styles.any((DiagnosticsNode n) => !n.isFiltered(DiagnosticLevel.info)); properties.add(new DiagnosticsProperty('${prefix}inherit', inherit, level: (!styleSpecified && inherit) ? DiagnosticLevel.fine : DiagnosticLevel.info)); - for (DiagnosticsNode style in styles) - properties.add(style); + styles.forEach(properties.add); if (!styleSpecified) properties.add(new FlagProperty('inherit', value: inherit, ifTrue: '$prefix', ifFalse: '$prefix')); diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 38583e8963..a6e0c8a074 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -2736,9 +2736,7 @@ abstract class ContainerRenderObjectMixin children) { - if (children != null) - for (ChildType child in children) - add(child); + children?.forEach(add); } void _removeFromChildList(ChildType child) { @@ -2923,7 +2921,7 @@ class FlutterErrorDetailsForRendering extends FlutterErrorDetails { /// Describes the semantics information a [RenderObject] wants to add to its /// parent. -/// +/// /// It has two notable subclasses: /// * [_InterestingSemanticsFragment] describing actual semantic information to /// be added to the parent. diff --git a/packages/flutter/lib/src/rendering/semantics.dart b/packages/flutter/lib/src/rendering/semantics.dart index 357827ae39..347b93306a 100644 --- a/packages/flutter/lib/src/rendering/semantics.dart +++ b/packages/flutter/lib/src/rendering/semantics.dart @@ -438,10 +438,7 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin { @override void redepthChildren() { - if (_children != null) { - for (SemanticsNode child in _children) - redepthChild(child); - } + _children?.forEach(redepthChild); } @override diff --git a/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart b/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart index 292f373d40..b2c9f3bde8 100644 --- a/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart +++ b/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart @@ -219,8 +219,7 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver @override void removeAll() { super.removeAll(); - for (RenderBox child in _keepAliveBucket.values) - dropChild(child); + _keepAliveBucket.values.forEach(dropChild); _keepAliveBucket.clear(); } @@ -274,15 +273,13 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver @override void redepthChildren() { super.redepthChildren(); - for (RenderBox child in _keepAliveBucket.values) - redepthChild(child); + _keepAliveBucket.values.forEach(redepthChild); } @override void visitChildren(RenderObjectVisitor visitor) { super.visitChildren(visitor); - for (RenderBox child in _keepAliveBucket.values) - visitor(child); + _keepAliveBucket.values.forEach(visitor); } @override diff --git a/packages/flutter/lib/src/rendering/table.dart b/packages/flutter/lib/src/rendering/table.dart index 2b3b506417..58855f7f28 100644 --- a/packages/flutter/lib/src/rendering/table.dart +++ b/packages/flutter/lib/src/rendering/table.dart @@ -384,10 +384,7 @@ class RenderTable extends RenderBox { _configuration = configuration; _defaultVerticalAlignment = defaultVerticalAlignment; _textBaseline = textBaseline; - if (children != null) { - for (List row in children) - addRow(row); - } + children?.forEach(addRow); } // Children are stored in row-major order. @@ -628,8 +625,7 @@ class RenderTable extends RenderBox { y += 1; } // drop all the lost children - for (RenderBox oldChild in lostChildren) - dropChild(oldChild); + lostChildren.forEach(dropChild); // update our internal values _columns = columns; _rows = cells.length ~/ columns; @@ -652,8 +648,7 @@ class RenderTable extends RenderBox { _children.clear(); _columns = cells.isNotEmpty ? cells.first.length : 0; _rows = 0; - for (List row in cells) - addRow(row); + cells.forEach(addRow); assert(_children.length == rows * columns); } diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index b227b07c5f..6a41ceb20b 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -1736,8 +1736,7 @@ class _InactiveElements { final List elements = _elements.toList()..sort(Element._sort); _elements.clear(); try { - for (Element element in elements.reversed) - _unmount(element); + elements.reversed.forEach(_unmount); } finally { assert(_elements.isEmpty); _locked = false; diff --git a/packages/flutter/lib/src/widgets/navigator.dart b/packages/flutter/lib/src/widgets/navigator.dart index e782aa8322..87eab3ee57 100644 --- a/packages/flutter/lib/src/widgets/navigator.dart +++ b/packages/flutter/lib/src/widgets/navigator.dart @@ -791,8 +791,7 @@ class NavigatorState extends State with TickerProviderStateMixin { }()); push(_routeNamed(Navigator.defaultRouteName)); } else { - for (Route route in plannedInitialRoutes) - push(route); + plannedInitialRoutes.forEach(push); } } else { Route route; @@ -1328,8 +1327,7 @@ class NavigatorState extends State with TickerProviderStateMixin { absorber?.absorbing = true; }); } - for (int pointer in _activePointers.toList()) - WidgetsBinding.instance.cancelPointer(pointer); + _activePointers.toList().forEach(WidgetsBinding.instance.cancelPointer); } @override diff --git a/packages/flutter/test/foundation/diagnostics_test.dart b/packages/flutter/test/foundation/diagnostics_test.dart index c939c92c58..74a558715d 100644 --- a/packages/flutter/test/foundation/diagnostics_test.dart +++ b/packages/flutter/test/foundation/diagnostics_test.dart @@ -36,8 +36,7 @@ class TestTree extends Object with DiagnosticableTreeMixin { if (style != null) properties.defaultDiagnosticsTreeStyle = style; - for (DiagnosticsNode property in this.properties) - properties.add(property); + this.properties.forEach(properties.add); } } diff --git a/packages/flutter/test/scheduler/scheduler_test.dart b/packages/flutter/test/scheduler/scheduler_test.dart index edf4234f18..bd39db36a1 100644 --- a/packages/flutter/test/scheduler/scheduler_test.dart +++ b/packages/flutter/test/scheduler/scheduler_test.dart @@ -28,8 +28,7 @@ void main() { scheduler.scheduleTask(() { executedTasks.add(x); }, Priority.idle + x); } - for (int x in input) - scheduleAddingTask(x); + input.forEach(scheduleAddingTask); strategy.allowedPriority = 100; for (int i = 0; i < 3; i += 1) diff --git a/packages/flutter/test/widgets/linked_scroll_view_test.dart b/packages/flutter/test/widgets/linked_scroll_view_test.dart index 8bb106fe9f..b967635c96 100644 --- a/packages/flutter/test/widgets/linked_scroll_view_test.dart +++ b/packages/flutter/test/widgets/linked_scroll_view_test.dart @@ -26,13 +26,11 @@ class LinkedScrollController extends ScrollController { void setParent(ScrollController newParent) { if (_parent != null) { - for (ScrollPosition position in positions) - _parent.detach(position); + positions.forEach(_parent.detach); } _parent = newParent; if (_parent != null) { - for (ScrollPosition position in positions) - _parent.attach(position); + positions.forEach(_parent.attach); } } @@ -54,8 +52,7 @@ class LinkedScrollController extends ScrollController { @override void dispose() { if (_parent != null) { - for (ScrollPosition position in positions) - _parent.detach(position); + positions.forEach(_parent.detach); } super.dispose(); } diff --git a/packages/flutter_tools/lib/src/asset.dart b/packages/flutter_tools/lib/src/asset.dart index 92bc656196..9dc1d6daee 100644 --- a/packages/flutter_tools/lib/src/asset.dart +++ b/packages/flutter_tools/lib/src/asset.dart @@ -194,9 +194,7 @@ class AssetBundle { void dump() { printTrace('Dumping AssetBundle:'); - for (String archivePath in entries.keys.toList()..sort()) { - printTrace(archivePath); - } + (entries.keys.toList()..sort()).forEach(printTrace); } } diff --git a/packages/flutter_tools/lib/src/base/net.dart b/packages/flutter_tools/lib/src/base/net.dart index 1d9adfb8c8..52ef656911 100644 --- a/packages/flutter_tools/lib/src/base/net.dart +++ b/packages/flutter_tools/lib/src/base/net.dart @@ -55,8 +55,7 @@ Future> _attempt(Uri url) async { printTrace('Received response from server, collecting bytes...'); try { final BytesBuilder responseBody = new BytesBuilder(copy: false); - await for (List chunk in response) - responseBody.add(chunk); + await response.forEach(responseBody.add); return responseBody.takeBytes(); } on IOException catch (error) { printTrace('Download error: $error'); diff --git a/packages/flutter_tools/lib/src/base/utils.dart b/packages/flutter_tools/lib/src/base/utils.dart index af7b8b8c79..2a4fee1049 100644 --- a/packages/flutter_tools/lib/src/base/utils.dart +++ b/packages/flutter_tools/lib/src/base/utils.dart @@ -142,10 +142,8 @@ class ItemListNotifier { _items = updatedSet; - for (T item in addedItems) - _addedController.add(item); - for (T item in removedItems) - _removedController.add(item); + addedItems.forEach(_addedController.add); + removedItems.forEach(_removedController.add); } /// Close the streams. diff --git a/packages/flutter_tools/lib/src/compile.dart b/packages/flutter_tools/lib/src/compile.dart index 4299b8bfec..7d4912f422 100644 --- a/packages/flutter_tools/lib/src/compile.dart +++ b/packages/flutter_tools/lib/src/compile.dart @@ -131,8 +131,7 @@ class ResidentCompiler { final String inputKey = new Uuid().generateV4(); _server.stdin.writeln('recompile $inputKey'); - for (String invalidatedFile in invalidatedFiles) - _server.stdin.writeln(invalidatedFile); + invalidatedFiles.forEach(_server.stdin.writeln); _server.stdin.writeln(inputKey); return stdoutHandler.outputFilename.future; diff --git a/packages/flutter_tools/lib/src/devfs.dart b/packages/flutter_tools/lib/src/devfs.dart index e1b1e6e109..f94de198d2 100644 --- a/packages/flutter_tools/lib/src/devfs.dart +++ b/packages/flutter_tools/lib/src/devfs.dart @@ -478,8 +478,7 @@ class DevFS { } // No need to send source files because all compilation is done on the // host and result of compilation is single kernel file. - for (Uri fileUri in filesUris) - dirtyEntries.remove(fileUri); + filesUris.forEach(dirtyEntries.remove); printTrace('Compiling dart to kernel with ${invalidatedFiles.length} updated files'); final String compiledBinary = fullRestart ? await compile( diff --git a/packages/flutter_tools/test/base/net_test.dart b/packages/flutter_tools/test/base/net_test.dart index 384075055f..922c72c1d6 100644 --- a/packages/flutter_tools/test/base/net_test.dart +++ b/packages/flutter_tools/test/base/net_test.dart @@ -91,7 +91,7 @@ class MockHttpClientRequest implements io.HttpClientRequest { } } -class MockHttpClientResponse implements io.HttpClientResponse { +class MockHttpClientResponse extends Stream> implements io.HttpClientResponse { MockHttpClientResponse(this.statusCode); @override diff --git a/packages/flutter_tools/test/compile_test.dart b/packages/flutter_tools/test/compile_test.dart index d0550b3767..500bc20b27 100644 --- a/packages/flutter_tools/test/compile_test.dart +++ b/packages/flutter_tools/test/compile_test.dart @@ -45,7 +45,7 @@ void main() { final String output = await compile(sdkRoot: '/path/to/sdkroot', mainPath: '/path/to/main.dart' ); - verifyNever(mockFrontendServerStdIn.writeln(any)); + expect(mockFrontendServerStdIn.getAndClear(), isEmpty); expect(logger.traceText, equals('compile debug message: line1\ncompile debug message: line2\n')); expect(output, equals('/path/to/main.dart.dill')); }, overrides: { @@ -64,7 +64,7 @@ void main() { final String output = await compile(sdkRoot: '/path/to/sdkroot', mainPath: '/path/to/main.dart' ); - verifyNever(mockFrontendServerStdIn.writeln(any)); + expect(mockFrontendServerStdIn.getAndClear(), isEmpty); expect(logger.traceText, equals('compile debug message: line1\ncompile debug message: line2\n')); expect(output, equals(null)); }, overrides: { @@ -110,7 +110,7 @@ void main() { final String output = await generator.recompile( '/path/to/main.dart', null /* invalidatedFiles */ ); - verify(mockFrontendServerStdIn.writeln('compile /path/to/main.dart')); + expect(mockFrontendServerStdIn.getAndClear(), 'compile /path/to/main.dart\n'); verifyNoMoreInteractions(mockFrontendServerStdIn); expect(logger.traceText, equals('compile debug message: line1\ncompile debug message: line2\n')); expect(output, equals('/path/to/main.dart.dill')); @@ -125,12 +125,13 @@ void main() { when(mockFrontendServer.stdout).thenReturn(streamController.stream); streamController.add(UTF8.encode('result abc\nline0\nline1\nabc /path/to/main.dart.dill\n')); await generator.recompile('/path/to/main.dart', null /* invalidatedFiles */); - verify(mockFrontendServerStdIn.writeln('compile /path/to/main.dart')); + expect(mockFrontendServerStdIn.getAndClear(), 'compile /path/to/main.dart\n'); await _recompile(streamController, generator, mockFrontendServerStdIn, 'result abc\nline1\nline2\nabc /path/to/main.dart.dill\n'); verifyNoMoreInteractions(mockFrontendServerStdIn); + expect(mockFrontendServerStdIn.getAndClear(), isEmpty); expect(logger.traceText, equals( 'compile debug message: line0\ncompile debug message: line1\n' 'compile debug message: line1\ncompile debug message: line2\n' @@ -148,7 +149,7 @@ void main() { 'result abc\nline0\nline1\nabc /path/to/main.dart.dill\n' )); await generator.recompile('/path/to/main.dart', null /* invalidatedFiles */); - verify(mockFrontendServerStdIn.writeln('compile /path/to/main.dart')); + expect(mockFrontendServerStdIn.getAndClear(), 'compile /path/to/main.dart\n'); await _recompile(streamController, generator, mockFrontendServerStdIn, 'result abc\nline1\nline2\nabc /path/to/main.dart.dill\n'); @@ -156,6 +157,7 @@ void main() { 'result abc\nline2\nline3\nabc /path/to/main.dart.dill\n'); verifyNoMoreInteractions(mockFrontendServerStdIn); + expect(mockFrontendServerStdIn.getAndClear(), isEmpty); expect(logger.traceText, equals( 'compile debug message: line0\ncompile debug message: line1\n' 'compile debug message: line1\ncompile debug message: line2\n' @@ -177,15 +179,33 @@ Future _recompile(StreamController> streamController, }); final String output = await generator.recompile(null /* mainPath */, ['/path/to/main.dart']); expect(output, equals('/path/to/main.dart.dill')); - final String recompileCommand = verify( - mockFrontendServerStdIn.writeln(captureThat(startsWith('recompile '))) - ).captured[0]; - final String token1 = recompileCommand.split(' ')[1]; - verify(mockFrontendServerStdIn.writeln('/path/to/main.dart')); - verify(mockFrontendServerStdIn.writeln(token1)); + final String commands = mockFrontendServerStdIn.getAndClear(); + final RegExp re = new RegExp(r'^recompile (.*)\n/path/to/main.dart\n(.*)\n$'); + expect(commands, matches(re)); + final Match match = re.firstMatch(commands); + expect(match[1] == match[2], isTrue); + mockFrontendServerStdIn._stdInWrites.clear(); } class MockProcessManager extends Mock implements ProcessManager {} class MockProcess extends Mock implements Process {} class MockStream extends Mock implements Stream> {} -class MockStdIn extends Mock implements IOSink {} +class MockStdIn extends Mock implements IOSink { + final StringBuffer _stdInWrites = new StringBuffer(); + + String getAndClear() { + final String result = _stdInWrites.toString(); + _stdInWrites.clear(); + return result; + } + + @override + void write([Object o = '']) { + _stdInWrites.write(o); + } + + @override + void writeln([Object o = '']) { + _stdInWrites.writeln(o); + } +}