Analyze against using Stopwatches in the framework (#138507)

Author: Piinks

dev/bots/analyze.dart

@@ -124,6 +124,9 @@ Future<void> run(List<String> arguments) async {
   printProgress('Goldens...');
   await verifyGoldenTags(flutterPackages);
 
+  printProgress('Prevent flakes from Stopwatches...');
+  await verifyNoStopwatches(flutterPackages);
+
   printProgress('Skip test comments...');
   await verifySkipTestComments(flutterRoot);
 
@@ -584,6 +587,48 @@ Future<void> verifyGoldenTags(String workingDirectory, { int minimumMatches = 20
   }
 }
 
+/// Use of Stopwatches can introduce test flakes as the logical time of a
+/// stopwatch can fall out of sync with the mocked time of FakeAsync in testing.
+/// The Clock object provides a safe stopwatch instead, which is paired with
+/// FakeAsync as part of the test binding.
+final RegExp _findStopwatchPattern = RegExp(r'Stopwatch\(\)');
+const String _ignoreStopwatch = '// flutter_ignore: stopwatch (see analyze.dart)';
+const String _ignoreStopwatchForFile = '// flutter_ignore_for_file: stopwatch (see analyze.dart)';
+
+Future<void> verifyNoStopwatches(String workingDirectory, { int minimumMatches = 2000 }) async {
+  final List<String> errors = <String>[];
+  await for (final File file in _allFiles(workingDirectory, 'dart', minimumMatches: minimumMatches)) {
+    if (file.path.contains('flutter_tool')) {
+      // Skip flutter_tool package.
+      continue;
+    }
+    int lineNumber = 1;
+    final List<String> lines = file.readAsLinesSync();
+    for (final String line in lines) {
+      // If the file is being ignored, skip parsing the rest of the lines.
+      if (line.contains(_ignoreStopwatchForFile)) {
+        break;
+      }
+
+      if (line.contains(_findStopwatchPattern)
+          && !line.contains(_leadingComment)
+          && !line.contains(_ignoreStopwatch)) {
+        // Stopwatch found
+        errors.add('\t${file.path}:$lineNumber');
+      }
+      lineNumber++;
+    }
+  }
+  if (errors.isNotEmpty) {
+    foundError(<String>[
+      'Stopwatch use was found in the following files:',
+      ...errors,
+      '${bold}Stopwatches introduce flakes by falling out of sync with the FakeAsync used in testing.$reset',
+      'A Stopwatch that stays in sync with FakeAsync is available through the Gesture or Test bindings, through samplingClock.'
+    ]);
+  }
+}
+
 final RegExp _findDeprecationPattern = RegExp(r'@[Dd]eprecated');
 final RegExp _deprecationStartPattern = RegExp(r'^(?<indent> *)@Deprecated\($'); // flutter_ignore: deprecation_syntax (see analyze.dart)
 final RegExp _deprecationMessagePattern = RegExp(r"^ *'(?<message>.+) '$");

dev/bots/test/analyze-test-input/root/packages/foo/stopwatch_doc.dart

@@ -0,0 +1,20 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/// Sample Code
+///
+/// No analysis failures should be found.
+///
+/// {@tool snippet}
+/// Sample invocations of [Stopwatch].
+///
+/// ```dart
+/// Stopwatch();
+/// ```
+/// {@end-tool}
+String? foo;
+// Other comments
+// Stopwatch();
+
+String literal = 'Stopwatch()'; // flutter_ignore: stopwatch (see analyze.dart)

dev/bots/test/analyze-test-input/root/packages/foo/stopwatch_fail.dart

@@ -0,0 +1,14 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// This should fail analysis.
+
+void main() {
+  Stopwatch();
+
+  // Identify more than one in a file.
+ Stopwatch myStopwatch;
+  myStopwatch = Stopwatch();
+  myStopwatch.reset();
+}

dev/bots/test/analyze-test-input/root/packages/foo/stopwatch_ignore.dart

@@ -0,0 +1,10 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// This would fail analysis, but it is ignored
+// flutter_ignore_for_file: stopwatch (see analyze.dart)
+
+void main() {
+  Stopwatch();
+}

dev/bots/test/analyze_test.dart

@@ -92,6 +92,24 @@ void main() {
     expect(result[result.length - 1], ''); // trailing newline
   });
 
+  test('analyze.dart - verifyNoStopwatches', () async {
+    final List<String> result = (await capture(() => verifyNoStopwatches(testRootPath, minimumMatches: 6), shouldHaveErrors: true)).split('\n');
+    final List<String> lines = <String>[
+      '║ \ttest/analyze-test-input/root/packages/foo/stopwatch_fail.dart:8',
+      '║ \ttest/analyze-test-input/root/packages/foo/stopwatch_fail.dart:12',
+    ]
+        .map((String line) => line.replaceAll('/', Platform.isWindows ? r'\' : '/'))
+        .toList();
+    expect(result.length, 6 + lines.length, reason: 'output had unexpected number of lines:\n${result.join('\n')}');
+    expect(result[0], '╔═╡ERROR╞═══════════════════════════════════════════════════════════════════════');
+    expect(result[1], '║ Stopwatch use was found in the following files:');
+    expect(result.getRange(2, result.length - 4).toSet(), lines.toSet());
+    expect(result[result.length - 4], '║ Stopwatches introduce flakes by falling out of sync with the FakeAsync used in testing.');
+    expect(result[result.length - 3], '║ A Stopwatch that stays in sync with FakeAsync is available through the Gesture or Test bindings, through samplingClock.');
+    expect(result[result.length - 2], '╚═══════════════════════════════════════════════════════════════════════════════');
+    expect(result[result.length - 1], ''); // trailing newline
+  });
+
   test('analyze.dart - verifyNoMissingLicense', () async {
     final String result = await capture(() => verifyNoMissingLicense(testRootPath, checkMinimums: false), shouldHaveErrors: true);
     final String file = 'test/analyze-test-input/root/packages/foo/foo.dart'

packages/flutter/lib/src/foundation/debug.dart

@@ -75,7 +75,8 @@ Future<T> debugInstrumentAction<T>(String description, Future<T> Function() acti
     return true;
   }());
   if (instrument) {
-    final Stopwatch stopwatch = Stopwatch()..start();
+    final Stopwatch stopwatch = Stopwatch()..start(); // flutter_ignore: stopwatch (see analyze.dart)
+    // Ignore context: The framework does not use this function internally so it will not cause flakes.
     try {
       return await action();
     } finally {

packages/flutter/lib/src/foundation/print.dart

@@ -66,7 +66,8 @@ int _debugPrintedCharacters = 0;
 const int _kDebugPrintCapacity = 12 * 1024;
 const Duration _kDebugPrintPauseTime = Duration(seconds: 1);
 final Queue<String> _debugPrintBuffer = Queue<String>();
-final Stopwatch _debugPrintStopwatch = Stopwatch();
+final Stopwatch _debugPrintStopwatch = Stopwatch(); // flutter_ignore: stopwatch (see analyze.dart)
+// Ignore context: This is not used in tests, only for throttled logging.
 Completer<void>? _debugPrintCompleter;
 bool _debugPrintScheduled = false;
 void _debugPrintTask() {

packages/flutter/lib/src/gestures/binding.dart

@@ -36,7 +36,13 @@ class SamplingClock {
   DateTime now() => DateTime.now();
 
   /// Returns a new stopwatch that uses the current time as reported by `this`.
-  Stopwatch stopwatch() => Stopwatch();
+  ///
+  /// See also:
+  ///
+  ///   * [GestureBinding.debugSamplingClock], which is used in tests and
+  ///     debug builds to observe [FakeAsync].
+  Stopwatch stopwatch() => Stopwatch(); // flutter_ignore: stopwatch (see analyze.dart)
+  // Ignore context: This is replaced by debugSampling clock in the test binding.
 }
 
 // Class that handles resampling of touch events for multiple pointer
@@ -59,7 +65,8 @@ class _Resampler {
   Duration _frameTime = Duration.zero;
 
   // Time since `_frameTime` was updated.
-  Stopwatch _frameTimeAge = Stopwatch();
+  Stopwatch _frameTimeAge = Stopwatch(); // flutter_ignore: stopwatch (see analyze.dart)
+  // Ignore context: This is tested safely outside of FakeAsync.
 
   // Last sample time and time stamp of last event.
   //

packages/flutter/test/foundation/timeline_test.dart

@@ -72,7 +72,8 @@ void main() {
     // a bit inconsistent with Stopwatch.
     final int start = FlutterTimeline.now - 1;
     FlutterTimeline.timeSync('TEST', () {
-      final Stopwatch watch = Stopwatch()..start();
+      final Stopwatch watch = Stopwatch()..start(); // flutter_ignore: stopwatch (see analyze.dart)
+      // Ignore context: Used safely for benchmarking.
       while (watch.elapsedMilliseconds < 5) {}
       watch.stop();
     });

packages/flutter_test/lib/src/test_compat.dart

@@ -297,7 +297,8 @@ class _Reporter {
   final bool _printPath;
 
   /// A stopwatch that tracks the duration of the full run.
-  final Stopwatch _stopwatch = Stopwatch();
+  final Stopwatch _stopwatch = Stopwatch(); // flutter_ignore: stopwatch (see analyze.dart)
+  // Ignore context: Used for logging of actual test runs, outside of FakeAsync.
 
   /// The size of `_engine.passed` last time a progress notification was
   /// printed.