From b7261586e5fb275ab5b672d87fcb2930405eb74f Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Tue, 21 Aug 2018 14:02:11 -0700 Subject: [PATCH] Audit `TODO` syntax (#20837) Fixes the pattern for some TODOs to match our style guide. (Also, a couple of minor code order fixes.) --- analysis_options.yaml | 2 +- dev/devicelab/lib/framework/adb.dart | 2 +- dev/integration_tests/ui/lib/keyboard_resize.dart | 2 +- packages/flutter/lib/src/cupertino/action_sheet.dart | 2 +- packages/flutter/lib/src/material/app_bar.dart | 2 +- packages/flutter/lib/src/material/slider.dart | 2 +- .../flutter/lib/src/widgets/nested_scroll_view.dart | 2 +- packages/flutter/lib/src/widgets/widget_inspector.dart | 5 +++-- packages/flutter/test/foundation/stack_trace_test.dart | 4 ++-- packages/flutter/test/material/buttons_test.dart | 5 ++--- .../flutter/test/material/outline_button_test.dart | 5 ++--- packages/flutter/test/rendering/paragraph_test.dart | 6 ++---- packages/flutter_driver/lib/src/driver/timeline.dart | 2 +- .../lib/src/material_localizations.dart | 8 ++++---- packages/flutter_tools/bin/xcode_backend.sh | 2 +- .../flutter_tools/lib/src/android/android_device.dart | 2 +- packages/flutter_tools/lib/src/base/build.dart | 4 ++-- packages/flutter_tools/lib/src/build_info.dart | 2 +- packages/flutter_tools/lib/src/commands/daemon.dart | 6 +++--- packages/flutter_tools/lib/src/compile.dart | 2 +- packages/flutter_tools/lib/src/intellij/intellij.dart | 2 +- packages/flutter_tools/lib/src/vscode/vscode.dart | 10 +++++----- 22 files changed, 38 insertions(+), 41 deletions(-) diff --git a/analysis_options.yaml b/analysis_options.yaml index 84050d3ccd..b7974391a2 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -26,7 +26,7 @@ analyzer: errors: # treat missing required parameters as a warning (not a hint) missing_required_param: warning - # TODO: https://github.com/flutter/flutter/issues/20114 + # TODO(devoncarew): https://github.com/flutter/flutter/issues/20114 # treat missing returns as a warning (not a hint) missing_return: ignore # allow having TODOs in the code diff --git a/dev/devicelab/lib/framework/adb.dart b/dev/devicelab/lib/framework/adb.dart index 6849e03dd6..1e47885c16 100644 --- a/dev/devicelab/lib/framework/adb.dart +++ b/dev/devicelab/lib/framework/adb.dart @@ -399,7 +399,7 @@ class IosDeviceDiscovery implements DeviceDiscovery { Future> checkDevices() async { final Map results = {}; for (String deviceId in await discoverDevices()) { - // TODO: do a more meaningful connectivity check than just recording the ID + // TODO(ianh): do a more meaningful connectivity check than just recording the ID results['ios-device-$deviceId'] = new HealthCheckResult.success(); } return results; diff --git a/dev/integration_tests/ui/lib/keyboard_resize.dart b/dev/integration_tests/ui/lib/keyboard_resize.dart index b74ab5818e..cfa023fb76 100644 --- a/dev/integration_tests/ui/lib/keyboard_resize.dart +++ b/dev/integration_tests/ui/lib/keyboard_resize.dart @@ -9,7 +9,7 @@ import 'keys.dart' as keys; void main() { enableFlutterDriverExtension(handler: (String message) async { - // TODO(cbernaschina) remove when test flakiness is resolved + // TODO(cbernaschina): remove when test flakiness is resolved return 'keyboard_resize'; }); runApp(new MyApp()); diff --git a/packages/flutter/lib/src/cupertino/action_sheet.dart b/packages/flutter/lib/src/cupertino/action_sheet.dart index 99f42635c9..662361f674 100644 --- a/packages/flutter/lib/src/cupertino/action_sheet.dart +++ b/packages/flutter/lib/src/cupertino/action_sheet.dart @@ -911,7 +911,7 @@ class _PressableActionButtonState extends State<_PressableActionButton> { Widget build(BuildContext context) { return new _ActionButtonParentDataWidget( isPressed: _isPressed, - // TODO:(mattcarroll): Button press dynamics need overhaul for iOS: https://github.com/flutter/flutter/issues/19786 + // TODO(mattcarroll): Button press dynamics need overhaul for iOS: https://github.com/flutter/flutter/issues/19786 child: new GestureDetector( excludeFromSemantics: true, behavior: HitTestBehavior.opaque, diff --git a/packages/flutter/lib/src/material/app_bar.dart b/packages/flutter/lib/src/material/app_bar.dart index 025aa306ce..87e6a953a6 100644 --- a/packages/flutter/lib/src/material/app_bar.dart +++ b/packages/flutter/lib/src/material/app_bar.dart @@ -51,7 +51,7 @@ class _ToolbarContainerLayout extends SingleChildLayoutDelegate { bool shouldRelayout(_ToolbarContainerLayout oldDelegate) => false; } -// TODO(eseidel) Toolbar needs to change size based on orientation: +// TODO(eseidel): Toolbar needs to change size based on orientation: // http://material.google.com/layout/structure.html#structure-app-bar // Mobile Landscape: 48dp // Mobile Portrait: 56dp diff --git a/packages/flutter/lib/src/material/slider.dart b/packages/flutter/lib/src/material/slider.dart index 5e8fd97844..4ace398338 100644 --- a/packages/flutter/lib/src/material/slider.dart +++ b/packages/flutter/lib/src/material/slider.dart @@ -962,7 +962,7 @@ class _RenderSlider extends RenderBox { void _paintOverlay(Canvas canvas, Offset center) { if (!_overlayAnimation.isDismissed) { - // TODO(gspencer) : We don't really follow the spec here for overlays. + // TODO(gspencer): We don't really follow the spec here for overlays. // The spec says to use 16% opacity for drawing over light material, // and 32% for colored material, but we don't really have a way to // know what the underlying color is, so there's no easy way to diff --git a/packages/flutter/lib/src/widgets/nested_scroll_view.dart b/packages/flutter/lib/src/widgets/nested_scroll_view.dart index 0113eba2c2..3d93c16cbe 100644 --- a/packages/flutter/lib/src/widgets/nested_scroll_view.dart +++ b/packages/flutter/lib/src/widgets/nested_scroll_view.dart @@ -889,7 +889,7 @@ class _NestedScrollController extends ScrollController { } Iterable<_NestedScrollPosition> get nestedPositions sync* { - // TODO(vegorov) use instance method version of castFrom when it is available. + // TODO(vegorov): use instance method version of castFrom when it is available. yield* Iterable.castFrom(positions); } } diff --git a/packages/flutter/lib/src/widgets/widget_inspector.dart b/packages/flutter/lib/src/widgets/widget_inspector.dart index ff90cf7e4f..52c29a59f1 100644 --- a/packages/flutter/lib/src/widgets/widget_inspector.dart +++ b/packages/flutter/lib/src/widgets/widget_inspector.dart @@ -812,8 +812,9 @@ class WidgetInspectorService { /// the value is returned over the Observatory protocol and when the /// separate observatory protocol command has to be used to retrieve its full /// contents. - /// TODO(jacobr): Replace this with a better solution once - /// https://github.com/dart-lang/sdk/issues/32919 is fixed. + // + // TODO(jacobr): Replace this with a better solution once + // https://github.com/dart-lang/sdk/issues/32919 is fixed. String _safeJsonEncode(Object object) { final String jsonString = json.encode(object); _serializeRing[_serializeRingIndex] = jsonString; diff --git a/packages/flutter/test/foundation/stack_trace_test.dart b/packages/flutter/test/foundation/stack_trace_test.dart index daf726ee51..ef1af40f8d 100644 --- a/packages/flutter/test/foundation/stack_trace_test.dart +++ b/packages/flutter/test/foundation/stack_trace_test.dart @@ -6,8 +6,8 @@ import 'package:flutter/foundation.dart'; import '../flutter_test_alternative.dart'; void main() { - // TODO(8128): These tests and the filtering mechanism should be revisited to account for causal async stack traces. - + // TODO(ianh): These tests and the filtering mechanism should be revisited to + // account for causal async stack traces. https://github.com/flutter/flutter/issues/8128 test('FlutterError.defaultStackFilter', () { final List filtered = FlutterError.defaultStackFilter(StackTrace.current.toString().trimRight().split('\n')).toList(); expect(filtered.length, greaterThanOrEqualTo(4)); diff --git a/packages/flutter/test/material/buttons_test.dart b/packages/flutter/test/material/buttons_test.dart index 58f0ce733d..627977690f 100644 --- a/packages/flutter/test/material/buttons_test.dart +++ b/packages/flutter/test/material/buttons_test.dart @@ -136,11 +136,10 @@ void main() { expect(tester.getSize(find.byType(FlatButton)), equals(const Size(88.0, 48.0))); // Scaled text rendering is different on Linux and Mac by one pixel. - // TODO(#12357): Update this test when text rendering is fixed. + // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357 expect(tester.getSize(find.byType(Text)).width, isIn([54.0, 55.0])); expect(tester.getSize(find.byType(Text)).height, isIn([18.0, 19.0])); - // Set text scale large enough to expand text and button. await tester.pumpWidget( new Directionality( @@ -160,7 +159,7 @@ void main() { ); // Scaled text rendering is different on Linux and Mac by one pixel. - // TODO(#12357): Update this test when text rendering is fixed. + // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357 expect(tester.getSize(find.byType(FlatButton)).width, isIn([158.0, 159.0])); expect(tester.getSize(find.byType(FlatButton)).height, equals(48.0)); expect(tester.getSize(find.byType(Text)).width, isIn([126.0, 127.0])); diff --git a/packages/flutter/test/material/outline_button_test.dart b/packages/flutter/test/material/outline_button_test.dart index 81ddb1e4c0..38385181b8 100644 --- a/packages/flutter/test/material/outline_button_test.dart +++ b/packages/flutter/test/material/outline_button_test.dart @@ -241,11 +241,10 @@ void main() { expect(tester.getSize(find.byType(FlatButton)), equals(const Size(88.0, 48.0))); // Scaled text rendering is different on Linux and Mac by one pixel. - // TODO(#12357): Update this test when text rendering is fixed. + // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357 expect(tester.getSize(find.byType(Text)).width, isIn([54.0, 55.0])); expect(tester.getSize(find.byType(Text)).height, isIn([18.0, 19.0])); - // Set text scale large enough to expand text and button. await tester.pumpWidget( new Directionality( @@ -265,7 +264,7 @@ void main() { ); // Scaled text rendering is different on Linux and Mac by one pixel. - // TODO(#12357): Update this test when text rendering is fixed. + // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357 expect(tester.getSize(find.byType(FlatButton)).width, isIn([158.0, 159.0])); expect(tester.getSize(find.byType(FlatButton)).height, equals(48.0)); expect(tester.getSize(find.byType(Text)).width, isIn([126.0, 127.0])); diff --git a/packages/flutter/test/rendering/paragraph_test.dart b/packages/flutter/test/rendering/paragraph_test.dart index aed367d794..e67419c5ed 100644 --- a/packages/flutter/test/rendering/paragraph_test.dart +++ b/packages/flutter/test/rendering/paragraph_test.dart @@ -255,8 +255,7 @@ void main() { paragraph.layout(const BoxConstraints()); // anyOf is needed here because Linux and Mac have different text // rendering widths in tests. - // TODO(#12357): Figure out why this is, and fix it (if needed) once Blink - // text rendering is replaced. + // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357 expect(paragraph.size.width, anyOf(79.0, 78.0)); expect(paragraph.size.height, 26.0); @@ -272,8 +271,7 @@ void main() { // anyOf is needed here and below because Linux and Mac have different text // rendering widths in tests. - // TODO(#12357): Figure out why this is, and fix it (if needed) once Blink - // text rendering is replaced. + // TODO(gspencergoog): Figure out why this is, and fix it. https://github.com/flutter/flutter/issues/12357 expect(boxes[0].toRect().width, anyOf(14.0, 13.0)); expect(boxes[0].toRect().height, closeTo(13.0, 0.0001)); expect(boxes[1].toRect().width, anyOf(27.0, 26.0)); diff --git a/packages/flutter_driver/lib/src/driver/timeline.dart b/packages/flutter_driver/lib/src/driver/timeline.dart index fdc85a84a5..11a17df90b 100644 --- a/packages/flutter_driver/lib/src/driver/timeline.dart +++ b/packages/flutter_driver/lib/src/driver/timeline.dart @@ -127,7 +127,7 @@ List _parseEvents(Map json) { if (jsonEvents == null) return null; - // TODO(vegorov) use instance method version of castFrom when it is available. + // TODO(vegorov): use instance method version of castFrom when it is available. return Iterable.castFrom>(jsonEvents) .map((Map eventJson) => new TimelineEvent(eventJson)) .toList(); diff --git a/packages/flutter_localizations/lib/src/material_localizations.dart b/packages/flutter_localizations/lib/src/material_localizations.dart index 85f4d9b87f..0223476c04 100644 --- a/packages/flutter_localizations/lib/src/material_localizations.dart +++ b/packages/flutter_localizations/lib/src/material_localizations.dart @@ -395,10 +395,10 @@ abstract class GlobalMaterialLocalizations implements MaterialLocalizations { /// The script category used by [localTextGeometry]. Must be one of the strings /// declared in [MaterialTextGeometry]. - /// - /// TODO(ianh): make this return a TextTheme from MaterialTextGeometry. - /// TODO(ianh): drop the constructor on MaterialTextGeometry. - /// TODO(ianh): drop the strings on MaterialTextGeometry. + // + // TODO(ianh): make this return a TextTheme from MaterialTextGeometry. + // TODO(ianh): drop the constructor on MaterialTextGeometry. + // TODO(ianh): drop the strings on MaterialTextGeometry. @protected String get scriptCategory; diff --git a/packages/flutter_tools/bin/xcode_backend.sh b/packages/flutter_tools/bin/xcode_backend.sh index 2041c4d41f..c7ce59a2ee 100755 --- a/packages/flutter_tools/bin/xcode_backend.sh +++ b/packages/flutter_tools/bin/xcode_backend.sh @@ -280,7 +280,7 @@ ThinAppFrameworks() { # Main entry point. -# TODO(cbracken) improve error handling, then enable set -e +# TODO(cbracken): improve error handling, then enable set -e if [[ $# == 0 ]]; then # Backwards-compatibility: if no args are provided, build. diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index 3cdd7555c2..fec83dfdcb 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -451,7 +451,7 @@ class AndroidDevice extends Device { // device has printed "Observatory is listening on...". printTrace('Waiting for observatory port to be available...'); - // TODO(danrubel) Waiting for observatory services can be made common across all devices. + // TODO(danrubel): Waiting for observatory services can be made common across all devices. try { Uri observatoryUri; diff --git a/packages/flutter_tools/lib/src/base/build.dart b/packages/flutter_tools/lib/src/base/build.dart index a5fa0c5129..95aee4ffcd 100644 --- a/packages/flutter_tools/lib/src/base/build.dart +++ b/packages/flutter_tools/lib/src/base/build.dart @@ -230,7 +230,7 @@ class AOTSnapshotter { if (platform == TargetPlatform.android_arm || iosArch == IOSArch.armv7) { // Use softfp for Android armv7 devices. // Note that this is the default for armv7 iOS builds, but harmless to set. - // TODO(cbracken) eliminate this when we fix https://github.com/flutter/flutter/issues/17489 + // TODO(cbracken): eliminate this when we fix https://github.com/flutter/flutter/issues/17489 genSnapshotArgs.add('--no-sim-use-hardfp'); // Not supported by the Pixel in 32-bit mode. @@ -469,7 +469,7 @@ class CoreJITSnapshotter { if (platform == TargetPlatform.android_arm) { // Use softfp for Android armv7 devices. - // TODO(cbracken) eliminate this when we fix https://github.com/flutter/flutter/issues/17489 + // TODO(cbracken): eliminate this when we fix https://github.com/flutter/flutter/issues/17489 genSnapshotArgs.add('--no-sim-use-hardfp'); // Not supported by the Pixel in 32-bit mode. diff --git a/packages/flutter_tools/lib/src/build_info.dart b/packages/flutter_tools/lib/src/build_info.dart index 50238a1547..895d1b1c7f 100644 --- a/packages/flutter_tools/lib/src/build_info.dart +++ b/packages/flutter_tools/lib/src/build_info.dart @@ -272,7 +272,7 @@ String getBuildDirectory() { /// Returns the Android build output directory. String getAndroidBuildDirectory() { - // TODO(cbracken) move to android subdir. + // TODO(cbracken): move to android subdir. return getBuildDirectory(); } diff --git a/packages/flutter_tools/lib/src/commands/daemon.dart b/packages/flutter_tools/lib/src/commands/daemon.dart index eaf51f70c0..73e5852748 100644 --- a/packages/flutter_tools/lib/src/commands/daemon.dart +++ b/packages/flutter_tools/lib/src/commands/daemon.dart @@ -831,9 +831,9 @@ class EmulatorDomain extends Domain { /// This class can either: /// 1) Send stdout messages and progress events to the client IDE /// 1) Log messages to stdout and send progress events to the client IDE -/// -/// TODO(devoncarew): To simplify this code a bit, we could choose to specialize -/// this class into two, one for each of the above use cases. +// +// TODO(devoncarew): To simplify this code a bit, we could choose to specialize +// this class into two, one for each of the above use cases. class _AppRunLogger extends Logger { _AppRunLogger(this.domain, this.app, { this.parent }); diff --git a/packages/flutter_tools/lib/src/compile.dart b/packages/flutter_tools/lib/src/compile.dart index 2faa021da6..ea2f1dbb2e 100644 --- a/packages/flutter_tools/lib/src/compile.dart +++ b/packages/flutter_tools/lib/src/compile.dart @@ -90,7 +90,7 @@ class KernelCompiler { Artifact.frontendServerSnapshotForEngineDartSdk ); - // TODO(cbracken) eliminate pathFilter. + // TODO(cbracken): eliminate pathFilter. // Currently the compiler emits buildbot paths for the core libs in the // depfile. None of these are available on the local host. Fingerprinter fingerprinter; diff --git a/packages/flutter_tools/lib/src/intellij/intellij.dart b/packages/flutter_tools/lib/src/intellij/intellij.dart index bf34145c5b..ced7d723d2 100644 --- a/packages/flutter_tools/lib/src/intellij/intellij.dart +++ b/packages/flutter_tools/lib/src/intellij/intellij.dart @@ -53,7 +53,7 @@ class IntelliJPlugins { final String jarPath = packageName.endsWith('.jar') ? fs.path.join(pluginsPath, packageName) : fs.path.join(pluginsPath, packageName, 'lib', '$packageName.jar'); - // TODO(danrubel) look for a better way to extract a single 2K file from the zip + // TODO(danrubel): look for a better way to extract a single 2K file from the zip // rather than reading the entire file into memory. try { final Archive archive = diff --git a/packages/flutter_tools/lib/src/vscode/vscode.dart b/packages/flutter_tools/lib/src/vscode/vscode.dart index de6caae7c9..9a81d84172 100644 --- a/packages/flutter_tools/lib/src/vscode/vscode.dart +++ b/packages/flutter_tools/lib/src/vscode/vscode.dart @@ -13,8 +13,6 @@ import '../base/version.dart'; const bool _includeInsiders = false; class VsCode { - static const String extensionIdentifier = 'Dart-Code.flutter'; - VsCode._(this.directory, this.extensionDirectory, { Version version, this.edition }) : this.version = version ?? Version.unknown { @@ -52,6 +50,8 @@ class VsCode { final Version version; final String edition; + static const String extensionIdentifier = 'Dart-Code.flutter'; + bool _isValid = false; Version _extensionVersion; final List _validationMessages = []; @@ -118,7 +118,7 @@ class VsCode { // Windows: // $programfiles(x86)\Microsoft VS Code // $programfiles(x86)\Microsoft VS Code Insiders - // TODO: Confirm these are correct for 64bit + // TODO(dantup): Confirm these are correct for 64bit // $programfiles\Microsoft VS Code // $programfiles\Microsoft VS Code Insiders // Windows Extensions: @@ -187,10 +187,10 @@ class VsCode { } class _VsCodeInstallLocation { + const _VsCodeInstallLocation(this.installPath, this.extensionsFolder, { this.edition, bool isInsiders }) + : this.isInsiders = isInsiders ?? false; final String installPath; final String extensionsFolder; final String edition; final bool isInsiders; - const _VsCodeInstallLocation(this.installPath, this.extensionsFolder, { this.edition, bool isInsiders }) - : this.isInsiders = isInsiders ?? false; }