From 15601fe55cdafe3d854f0125dde1b47b1be2a526 Mon Sep 17 00:00:00 2001 From: Alexandre Ardhuin Date: Wed, 8 Nov 2017 22:59:49 +0100 Subject: [PATCH] Enable lint prefer asserts in initializer lists (#12903) * enable lint prefer_asserts_in_initializer_lists * enable --assert-initializer --- analysis_options.yaml | 2 +- analysis_options_repo.yaml | 2 +- bin/flutter.bat | 1 + dev/bots/test.dart | 3 ++- packages/flutter/lib/src/cupertino/route.dart | 1 + .../lib/src/foundation/diagnostics.dart | 26 +++++++++---------- packages/flutter/lib/src/material/page.dart | 1 + .../flutter/lib/src/painting/borders.dart | 9 +++---- .../lib/src/widgets/localizations.dart | 18 ++++++------- .../flutter/lib/src/widgets/page_storage.dart | 5 ++-- packages/flutter/lib/src/widgets/pages.dart | 5 ++-- packages/flutter_driver/lib/src/health.dart | 5 ++-- .../lib/src/application_package.dart | 12 ++++----- .../flutter_tools/lib/src/base/build.dart | 5 ++-- .../flutter_tools/lib/src/base/flags.dart | 5 ++-- .../flutter_tools/lib/src/base/logger.dart | 4 +-- .../lib/src/commands/build_apk.dart | 4 +-- .../flutter_tools/lib/src/commands/drive.dart | 2 +- packages/flutter_tools/lib/src/compile.dart | 4 +-- packages/flutter_tools/lib/src/dart/sdk.dart | 3 +++ .../lib/src/flutter_manifest.dart | 14 +++++----- .../lib/src/protocol_discovery.dart | 6 ++--- .../lib/src/test/flutter_platform.dart | 4 +-- .../test/commands/create_test.dart | 17 +++++++----- .../test/commands/test_test.dart | 12 +++++---- 25 files changed, 82 insertions(+), 88 deletions(-) diff --git a/analysis_options.yaml b/analysis_options.yaml index 3c2c466e20..74c22f49de 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -109,7 +109,7 @@ linter: - package_prefixed_library_names # - parameter_assignments # we do this commonly - prefer_adjacent_string_concatenation - # - prefer_asserts_in_initializer_lists # not yet tested + - prefer_asserts_in_initializer_lists - prefer_collection_literals - prefer_conditional_assignment - prefer_const_constructors diff --git a/analysis_options_repo.yaml b/analysis_options_repo.yaml index 29dbefb14b..571e3c224b 100644 --- a/analysis_options_repo.yaml +++ b/analysis_options_repo.yaml @@ -103,7 +103,7 @@ linter: - package_prefixed_library_names # - parameter_assignments # we do this commonly - prefer_adjacent_string_concatenation - # - prefer_asserts_in_initializer_lists # not yet tested + - prefer_asserts_in_initializer_lists - prefer_collection_literals - prefer_conditional_assignment - prefer_const_constructors diff --git a/bin/flutter.bat b/bin/flutter.bat index f875b4a58e..5318ccf834 100644 --- a/bin/flutter.bat +++ b/bin/flutter.bat @@ -39,6 +39,7 @@ IF NOT EXIST "%flutter_root%\.git" ( REM Ensure that bin/cache exists. IF NOT EXIST "%cache_dir%" MKDIR "%cache_dir%" +SET FLUTTER_TOOL_ARGS=--assert-initializer %FLUTTER_TOOL_ARGS% REM To debug the tool, you can uncomment the following lines to enable checked mode and set an observatory port: REM SET FLUTTER_TOOL_ARGS="--checked %FLUTTER_TOOL_ARGS%" REM SET FLUTTER_TOOL_ARGS="%FLUTTER_TOOL_ARGS% --observe=65432" diff --git a/dev/bots/test.dart b/dev/bots/test.dart index e2b8ec8c92..295fb76008 100644 --- a/dev/bots/test.dart +++ b/dev/bots/test.dart @@ -206,7 +206,8 @@ Future _pubRunTest( final List args = ['run', 'test', '-j1', '-rexpanded']; if (testPath != null) args.add(testPath); - return _runCommand(pub, args, workingDirectory: workingDirectory); + return _runCommand(pub, args, workingDirectory: workingDirectory, + environment: {'DART_VM_OPTIONS': '--assert-initializer'}); } class EvalResult { diff --git a/packages/flutter/lib/src/cupertino/route.dart b/packages/flutter/lib/src/cupertino/route.dart index a2a9b3a067..de06d9b7c7 100644 --- a/packages/flutter/lib/src/cupertino/route.dart +++ b/packages/flutter/lib/src/cupertino/route.dart @@ -89,6 +89,7 @@ class CupertinoPageRoute extends PageRoute { assert(maintainState != null), assert(fullscreenDialog != null), super(settings: settings, fullscreenDialog: fullscreenDialog) { + // ignore: prefer_asserts_in_initializer_lists , https://github.com/dart-lang/sdk/issues/31223 assert(opaque); // PageRoute makes it return true. } diff --git a/packages/flutter/lib/src/foundation/diagnostics.dart b/packages/flutter/lib/src/foundation/diagnostics.dart index f223ea8ad2..a3e4dbecca 100644 --- a/packages/flutter/lib/src/foundation/diagnostics.dart +++ b/packages/flutter/lib/src/foundation/diagnostics.dart @@ -642,12 +642,11 @@ abstract class DiagnosticsNode { this.showName: true, this.showSeparator: true, }) : assert(showName != null), - assert(showSeparator != null) { - // A name ending with ':' indicates that the user forgot that the ':' will - // be automatically added for them when generating descriptions of the - // property. - assert(name == null || !name.endsWith(':'), 'Names of diagnostic nodes must not end with colons.'); - } + assert(showSeparator != null), + // A name ending with ':' indicates that the user forgot that the ':' will + // be automatically added for them when generating descriptions of the + // property. + assert(name == null || !name.endsWith(':'), 'Names of diagnostic nodes must not end with colons.'); /// Diagnostics containing just a string `message` and not a concrete name or /// value. @@ -1318,15 +1317,14 @@ class FlagProperty extends DiagnosticsProperty { DiagnosticLevel level: DiagnosticLevel.info, }) : assert(showName != null), assert(level != null), + assert(ifTrue != null || ifFalse != null), super( - name, - value, - showName: showName, - defaultValue: defaultValue, - level: level, - ) { - assert(ifTrue != null || ifFalse != null); - } + name, + value, + showName: showName, + defaultValue: defaultValue, + level: level, + ); /// Description to use if the property [value] is true. /// diff --git a/packages/flutter/lib/src/material/page.dart b/packages/flutter/lib/src/material/page.dart index a400175c4d..c5fddcbbeb 100644 --- a/packages/flutter/lib/src/material/page.dart +++ b/packages/flutter/lib/src/material/page.dart @@ -71,6 +71,7 @@ class MaterialPageRoute extends PageRoute { bool fullscreenDialog: false, }) : assert(builder != null), super(settings: settings, fullscreenDialog: fullscreenDialog) { + // ignore: prefer_asserts_in_initializer_lists , https://github.com/dart-lang/sdk/issues/31223 assert(opaque); } diff --git a/packages/flutter/lib/src/painting/borders.dart b/packages/flutter/lib/src/painting/borders.dart index 03f910ea4e..d1741b1b06 100644 --- a/packages/flutter/lib/src/painting/borders.dart +++ b/packages/flutter/lib/src/painting/borders.dart @@ -411,11 +411,10 @@ abstract class ShapeBorder { /// /// The borders are listed from the outside to the inside. class _CompoundBorder extends ShapeBorder { - _CompoundBorder(this.borders) { - assert(borders != null); - assert(borders.length >= 2); - assert(!borders.any((ShapeBorder border) => border is _CompoundBorder)); - } + _CompoundBorder(this.borders) + : assert(borders != null), + assert(borders.length >= 2), + assert(!borders.any((ShapeBorder border) => border is _CompoundBorder)); final List borders; diff --git a/packages/flutter/lib/src/widgets/localizations.dart b/packages/flutter/lib/src/widgets/localizations.dart index 1335fd63a2..13e4733be2 100644 --- a/packages/flutter/lib/src/widgets/localizations.dart +++ b/packages/flutter/lib/src/widgets/localizations.dart @@ -220,16 +220,15 @@ class DefaultWidgetsLocalizations implements WidgetsLocalizations { } class _LocalizationsScope extends InheritedWidget { - _LocalizationsScope ({ + const _LocalizationsScope ({ Key key, @required this.locale, @required this.localizationsState, @required this.typeToResources, Widget child, - }) : super(key: key, child: child) { - assert(localizationsState != null); - assert(typeToResources != null); - } + }) : assert(localizationsState != null), + assert(typeToResources != null), + super(key: key, child: child); final Locale locale; final _LocalizationsState localizationsState; @@ -337,11 +336,10 @@ class Localizations extends StatefulWidget { @required this.locale, @required this.delegates, this.child, - }) : super(key: key) { - assert(locale != null); - assert(delegates != null); - assert(delegates.any((LocalizationsDelegate delegate) => delegate is LocalizationsDelegate)); - } + }) : assert(locale != null), + assert(delegates != null), + assert(delegates.any((LocalizationsDelegate delegate) => delegate is LocalizationsDelegate)), + super(key: key); /// Overrides the inherited [Locale] or [LocalizationsDelegate]s for `child`. /// diff --git a/packages/flutter/lib/src/widgets/page_storage.dart b/packages/flutter/lib/src/widgets/page_storage.dart index a9acc70e1b..e391b7b685 100644 --- a/packages/flutter/lib/src/widgets/page_storage.dart +++ b/packages/flutter/lib/src/widgets/page_storage.dart @@ -39,9 +39,8 @@ class PageStorageKey extends ValueKey { } class _StorageEntryIdentifier { - _StorageEntryIdentifier(this.keys) { - assert(keys != null); - } + _StorageEntryIdentifier(this.keys) + : assert(keys != null); final List> keys; diff --git a/packages/flutter/lib/src/widgets/pages.dart b/packages/flutter/lib/src/widgets/pages.dart index 026d6d0351..b20a0ec398 100644 --- a/packages/flutter/lib/src/widgets/pages.dart +++ b/packages/flutter/lib/src/widgets/pages.dart @@ -83,9 +83,8 @@ class PageRouteBuilder extends PageRoute { assert(transitionsBuilder != null), assert(barrierDismissible != null), assert(maintainState != null), - super(settings: settings) { - assert(opaque != null); - } + assert(opaque != null), + super(settings: settings); /// Used build the route's primary contents. /// diff --git a/packages/flutter_driver/lib/src/health.dart b/packages/flutter_driver/lib/src/health.dart index d6abfcaf6b..df343e5d20 100644 --- a/packages/flutter_driver/lib/src/health.dart +++ b/packages/flutter_driver/lib/src/health.dart @@ -33,9 +33,8 @@ final EnumIndex _healthStatusIndex = /// [FlutterDriver.checkHealth] test. class Health extends Result { /// Creates a [Health] object with the given [status]. - Health(this.status) { - assert(status != null); - } + Health(this.status) + : assert(status != null); /// The status represented by this object. /// diff --git a/packages/flutter_tools/lib/src/application_package.dart b/packages/flutter_tools/lib/src/application_package.dart index 7f0e6875a5..ad41013ed1 100644 --- a/packages/flutter_tools/lib/src/application_package.dart +++ b/packages/flutter_tools/lib/src/application_package.dart @@ -21,9 +21,8 @@ abstract class ApplicationPackage { /// Package ID from the Android Manifest or equivalent. final String id; - ApplicationPackage({ @required this.id }) { - assert(id != null); - } + ApplicationPackage({ @required this.id }) + : assert(id != null); String get name; @@ -46,10 +45,9 @@ class AndroidApk extends ApplicationPackage { String id, @required this.apkPath, @required this.launchActivity - }) : super(id: id) { - assert(apkPath != null); - assert(launchActivity != null); - } + }) : assert(apkPath != null), + assert(launchActivity != null), + super(id: id); /// Creates a new AndroidApk from an existing APK. factory AndroidApk.fromApk(String applicationBinary) { diff --git a/packages/flutter_tools/lib/src/base/build.dart b/packages/flutter_tools/lib/src/base/build.dart index dcb4ad14cd..0f92167642 100644 --- a/packages/flutter_tools/lib/src/base/build.dart +++ b/packages/flutter_tools/lib/src/base/build.dart @@ -21,9 +21,8 @@ GenSnapshot get genSnapshot => context.putIfAbsent(GenSnapshot, () => const GenS /// A snapshot build configuration. class SnapshotType { - SnapshotType(this.platform, this.mode) { - assert(mode != null); - } + SnapshotType(this.platform, this.mode) + : assert(mode != null); final TargetPlatform platform; final BuildMode mode; diff --git a/packages/flutter_tools/lib/src/base/flags.dart b/packages/flutter_tools/lib/src/base/flags.dart index 8007266893..a6b750faeb 100644 --- a/packages/flutter_tools/lib/src/base/flags.dart +++ b/packages/flutter_tools/lib/src/base/flags.dart @@ -17,9 +17,8 @@ Flags get flags => context?.getVariable(Flags) ?? const _EmptyFlags(); /// the Flutter tool (immediately after the arguments have been parsed in /// [FlutterCommandRunner]) and is available via the [flags] global property. class Flags { - Flags(this._globalResults) { - assert(_globalResults != null); - } + Flags(this._globalResults) + : assert(_globalResults != null); final ArgResults _globalResults; diff --git a/packages/flutter_tools/lib/src/base/logger.dart b/packages/flutter_tools/lib/src/base/logger.dart index 8770efa254..053012be80 100644 --- a/packages/flutter_tools/lib/src/base/logger.dart +++ b/packages/flutter_tools/lib/src/base/logger.dart @@ -177,8 +177,8 @@ class BufferLogger extends Logger { } class VerboseLogger extends Logger { - VerboseLogger(this.parent) { - assert(terminal != null); + VerboseLogger(this.parent) + : assert(terminal != null) { stopwatch.start(); } diff --git a/packages/flutter_tools/lib/src/commands/build_apk.dart b/packages/flutter_tools/lib/src/commands/build_apk.dart index 63c757db36..2041908c43 100644 --- a/packages/flutter_tools/lib/src/commands/build_apk.dart +++ b/packages/flutter_tools/lib/src/commands/build_apk.dart @@ -21,9 +21,7 @@ class ApkKeystoreInfo { this.password, this.keyAlias, @required this.keyPassword, - }) { - assert(keystore != null); - } + }) : assert(keystore != null); final String keystore; final String password; diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index d0d67addda..fe62bf82dc 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/packages/flutter_tools/lib/src/commands/drive.dart @@ -297,7 +297,7 @@ Future _runTests(List testArgs, String observatoryUri) async { ..add('-rexpanded'); final String dartVmPath = fs.path.join(dartSdkPath, 'bin', 'dart'); final int result = await runCommandAndStreamOutput( - [dartVmPath]..addAll(args), + [dartVmPath]..addAll(dartVmFlags)..addAll(args), environment: { 'VM_SERVICE_URL': observatoryUri } ); if (result != 0) diff --git a/packages/flutter_tools/lib/src/compile.dart b/packages/flutter_tools/lib/src/compile.dart index 7d4912f422..d2b6607b51 100644 --- a/packages/flutter_tools/lib/src/compile.dart +++ b/packages/flutter_tools/lib/src/compile.dart @@ -104,8 +104,8 @@ Future compile( /// The wrapper is intended to stay resident in memory as user changes, reloads, /// restarts the Flutter app. class ResidentCompiler { - ResidentCompiler(this._sdkRoot) { - assert(_sdkRoot != null); + ResidentCompiler(this._sdkRoot) + : assert(_sdkRoot != null) { // This is a URI, not a file path, so the forward slash is correct even on Windows. if (!_sdkRoot.endsWith('/')) _sdkRoot = '$_sdkRoot/'; diff --git a/packages/flutter_tools/lib/src/dart/sdk.dart b/packages/flutter_tools/lib/src/dart/sdk.dart index 64f128d963..c8d6fe97a6 100644 --- a/packages/flutter_tools/lib/src/dart/sdk.dart +++ b/packages/flutter_tools/lib/src/dart/sdk.dart @@ -11,6 +11,9 @@ String get dartSdkPath { return fs.path.join(Cache.flutterRoot, 'bin', 'cache', 'dart-sdk'); } +/// The required Dart language flags +const List dartVmFlags = const ['--assert-initializer']; + /// Return the platform specific name for the given Dart SDK binary. So, `pub` /// ==> `pub.bat`. The default SDK location can be overridden with a specified /// [sdkLocation]. diff --git a/packages/flutter_tools/lib/src/flutter_manifest.dart b/packages/flutter_tools/lib/src/flutter_manifest.dart index b1bff4b3d8..c909ee675b 100644 --- a/packages/flutter_tools/lib/src/flutter_manifest.dart +++ b/packages/flutter_tools/lib/src/flutter_manifest.dart @@ -102,11 +102,10 @@ class FlutterManifest { } class Font { - Font(this.familyName, this.fontAssets) { - assert(familyName != null); - assert(fontAssets != null); - assert(fontAssets.isNotEmpty); - } + Font(this.familyName, this.fontAssets) + : assert(familyName != null), + assert(fontAssets != null), + assert(fontAssets.isNotEmpty); final String familyName; final List fontAssets; @@ -123,9 +122,8 @@ class Font { } class FontAsset { - FontAsset(this.asset, {this.weight, this.style}) { - assert(asset != null); - } + FontAsset(this.asset, {this.weight, this.style}) + : assert(asset != null); final String asset; final int weight; diff --git a/packages/flutter_tools/lib/src/protocol_discovery.dart b/packages/flutter_tools/lib/src/protocol_discovery.dart index 8a8b5cb4a8..1bc35d355d 100644 --- a/packages/flutter_tools/lib/src/protocol_discovery.dart +++ b/packages/flutter_tools/lib/src/protocol_discovery.dart @@ -18,9 +18,9 @@ class ProtocolDiscovery { this.portForwarder, this.hostPort, this.defaultHostPort, - }) : _prefix = '$serviceName listening on ' { - assert(logReader != null); - assert(portForwarder == null || defaultHostPort != null); + }) : assert(logReader != null), + assert(portForwarder == null || defaultHostPort != null), + _prefix = '$serviceName listening on ' { _deviceLogSubscription = logReader.logLines.listen(_handleLine); } diff --git a/packages/flutter_tools/lib/src/test/flutter_platform.dart b/packages/flutter_tools/lib/src/test/flutter_platform.dart index 21630ea68a..86f5bf1375 100644 --- a/packages/flutter_tools/lib/src/test/flutter_platform.dart +++ b/packages/flutter_tools/lib/src/test/flutter_platform.dart @@ -88,9 +88,7 @@ class _FlutterPlatform extends PlatformPlugin { this.startPaused, this.explicitObservatoryPort, this.host, - }) { - assert(shellPath != null); - } + }) : assert(shellPath != null); final String shellPath; final TestWatcher watcher; diff --git a/packages/flutter_tools/test/commands/create_test.dart b/packages/flutter_tools/test/commands/create_test.dart index bf93aa3f75..9f060c0fd7 100644 --- a/packages/flutter_tools/test/commands/create_test.dart +++ b/packages/flutter_tools/test/commands/create_test.dart @@ -209,12 +209,12 @@ void main() { // TODO(pq): enable when sky_shell is available if (!io.Platform.isWindows) { // Verify that the sample widget test runs cleanly. - final List args = [ - fs.path.absolute(fs.path.join('bin', 'flutter_tools.dart')), - 'test', - '--no-color', - fs.path.join(projectDir.path, 'test', 'widget_test.dart'), - ]; + final List args = [] + ..addAll(dartVmFlags) + ..add(fs.path.absolute(fs.path.join('bin', 'flutter_tools.dart'))) + ..add('test') + ..add('--no-color') + ..add(fs.path.join(projectDir.path, 'test', 'widget_test.dart')); final ProcessResult result = await Process.run( fs.path.join(dartSdkPath, 'bin', 'dart'), @@ -338,7 +338,10 @@ Future _analyzeProject(String workingDir, {String target}) async { 'flutter_tools.dart', )); - final List args = [flutterToolsPath, 'analyze']; + final List args = [] + ..addAll(dartVmFlags) + ..add(flutterToolsPath) + ..add('analyze'); if (target != null) args.add(target); diff --git a/packages/flutter_tools/test/commands/test_test.dart b/packages/flutter_tools/test/commands/test_test.dart index 52e3d15b89..9f24323dfd 100644 --- a/packages/flutter_tools/test/commands/test_test.dart +++ b/packages/flutter_tools/test/commands/test_test.dart @@ -143,11 +143,13 @@ Future _runFlutterTest( if (!testFile.existsSync()) fail('missing test file: $testFile'); - final List args = [ - fs.path.absolute(fs.path.join('bin', 'flutter_tools.dart')), - 'test', - '--no-color' - ]..addAll(extraArgs)..add(testFilePath); + final List args = [] + ..addAll(dartVmFlags) + ..add(fs.path.absolute(fs.path.join('bin', 'flutter_tools.dart'))) + ..add('test') + ..add('--no-color') + ..addAll(extraArgs) + ..add(testFilePath); while (_testExclusionLock != null) await _testExclusionLock;