Catch Stopwatch with static analysis (#140019)

I did not include the `'// flutter_ignore_for_file: stopwatch (see analyze.dart)'` directive since it's currently not used, and adding that shouldn't be too difficult.

dev/bots/analyze.dart

@@ -19,6 +19,7 @@ import 'package:path/path.dart' as path;
 import 'allowlist.dart';
 import 'custom_rules/analyze.dart';
 import 'custom_rules/no_double_clamp.dart';
+import 'custom_rules/no_stop_watches.dart';
 import 'run_command.dart';
 import 'utils.dart';
 
@@ -123,9 +124,6 @@ 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);
 
@@ -175,10 +173,19 @@ Future<void> run(List<String> arguments) async {
     // Only run the private lints when the code is free of type errors. The
     // lints are easier to write when they can assume, for example, there is no
     // inheritance cycles.
-    final List<AnalyzeRule> rules = <AnalyzeRule>[noDoubleClamp];
+    final List<AnalyzeRule> rules = <AnalyzeRule>[noDoubleClamp, noStopwatches];
     final String ruleNames = rules.map((AnalyzeRule rule) => '\n * $rule').join();
     printProgress('Analyzing code in the framework with the following rules:$ruleNames');
-    await analyzeFrameworkWithRules(flutterRoot, rules);
+    await analyzeWithRules(flutterRoot, rules,
+      includePaths: <String>['packages/flutter/lib'],
+      excludePaths: <String>['packages/flutter/lib/fix_data'],
+    );
+    final List<AnalyzeRule> testRules = <AnalyzeRule>[noStopwatches];
+    final String testRuleNames = testRules.map((AnalyzeRule rule) => '\n * $rule').join();
+    printProgress('Analyzing code in the test folder with the following rules:$testRuleNames');
+    await analyzeWithRules(flutterRoot, testRules,
+      includePaths: <String>['packages/flutter/test'],
+    );
   } else {
     printProgress('Skipped performing further analysis in the framework because "flutter analyze" finished with a non-zero exit code.');
   }
@@ -535,48 +542,6 @@ 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/custom_rules/analyze.dart

@@ -12,19 +12,31 @@ import 'package:path/path.dart' as path;
 
 import '../utils.dart';
 
-/// Analyzes the given `flutterRootDirectory` containing the flutter framework
+/// Analyzes the dart source files in the given `flutterRootDirectory` with the
-/// source files, with the given [AnalyzeRule]s.
+/// given [AnalyzeRule]s.
+///
+/// The `includePath` parameter takes a collection of paths relative to the given
+/// `flutterRootDirectory`. It specifies the files or directory this function
+/// should analyze. Defaults to null in which case this function analyzes the
+/// all dart source files in `flutterRootDirectory`.
+///
+/// The `excludePath` parameter takes a collection of paths relative to the given
+/// `flutterRootDirectory` that this function should skip analyzing.
 ///
 /// If a compilation unit can not be resolved, this function ignores the
 /// corresponding dart source file and logs an error using [foundError].
-Future<void> analyzeFrameworkWithRules(String flutterRootDirectory, List<AnalyzeRule> rules) async {
+Future<void> analyzeWithRules(String flutterRootDirectory, List<AnalyzeRule> rules, {
-  final String flutterLibPath = path.canonicalize('$flutterRootDirectory/packages/flutter/lib');
+  Iterable<String>? includePaths,
-  if (!Directory(flutterLibPath).existsSync()) {
+  Iterable<String>? excludePaths,
-    foundError(<String>['Analyzer error: the specified $flutterLibPath does not exist.']);
+}) async {
+  if (!Directory(flutterRootDirectory).existsSync()) {
+    foundError(<String>['Analyzer error: the specified $flutterRootDirectory does not exist.']);
   }
+  final Iterable<String> includes = includePaths?.map((String relativePath) => path.canonicalize('$flutterRootDirectory/$relativePath'))
+                                 ?? <String>[path.canonicalize(flutterRootDirectory)];
   final AnalysisContextCollection collection = AnalysisContextCollection(
-    includedPaths: <String>[flutterLibPath],
+    includedPaths: includes.toList(),
-    excludedPaths: <String>[path.canonicalize('$flutterLibPath/fix_data')],
+    excludedPaths: excludePaths?.map((String relativePath) => path.canonicalize('$flutterRootDirectory/$relativePath')).toList(),
   );
 
   final List<String> analyzerErrors = <String>[];
@@ -55,7 +67,7 @@ Future<void> analyzeFrameworkWithRules(String flutterRootDirectory, List<Analyze
 /// An interface that defines a set of best practices, and collects information
 /// about code that violates the best practices in a [ResolvedUnitResult].
 ///
-/// The [analyzeFrameworkWithRules] function scans and analyzes the specified
+/// The [analyzeWithRules] function scans and analyzes the specified
 /// source directory using the dart analyzer package, and applies custom rules
 /// defined in the form of this interface on each resulting [ResolvedUnitResult].
 /// The [reportViolations] method will be called at the end, once all

dev/bots/custom_rules/no_stop_watches.dart

@@ -0,0 +1,144 @@
+// 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.
+
+import 'package:analyzer/dart/analysis/results.dart';
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/dart/element/type.dart';
+import 'package:path/path.dart' as path;
+
+import '../utils.dart';
+import 'analyze.dart';
+
+// The comment pattern representing the "flutter_ignore" inline directive that
+// indicates the line should be exempt from the stopwatch check.
+final Pattern _ignoreStopwatch = RegExp(r'// flutter_ignore: .*stopwatch .*\(see analyze\.dart\)');
+
+/// 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 AnalyzeRule noStopwatches = _NoStopwatches();
+
+class _NoStopwatches implements AnalyzeRule {
+  final Map<ResolvedUnitResult, List<AstNode>> _errors = <ResolvedUnitResult, List<AstNode>>{};
+
+  @override
+  void applyTo(ResolvedUnitResult unit) {
+    final _StopwatchVisitor visitor = _StopwatchVisitor(unit);
+    unit.unit.visitChildren(visitor);
+    final List<AstNode> violationsInUnit = visitor.stopwatchAccessNodes;
+    if (violationsInUnit.isNotEmpty) {
+      _errors.putIfAbsent(unit, () => <AstNode>[]).addAll(violationsInUnit);
+    }
+  }
+
+  @override
+  void reportViolations(String workingDirectory) {
+    if (_errors.isEmpty) {
+      return;
+    }
+
+    String locationInFile(ResolvedUnitResult unit, AstNode node) {
+      return '${path.relative(path.relative(unit.path, from: workingDirectory))}:${unit.lineInfo.getLocation(node.offset).lineNumber}';
+    }
+
+    foundError(<String>[
+      for (final MapEntry<ResolvedUnitResult, List<AstNode>> entry in _errors.entries)
+        for (final AstNode node in entry.value)
+          '${locationInFile(entry.key, node)}: ${node.parent}',
+      '\n${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.'
+    ]);
+  }
+
+  @override
+  String toString() => 'No "Stopwatch"';
+}
+
+// This visitor finds invocation sites of Stopwatch (and subclasses) constructors
+// and references to "external" functions that return a Stopwatch (and subclasses),
+// including constructors, and put them in the stopwatchAccessNodes list.
+class _StopwatchVisitor extends RecursiveAstVisitor<void> {
+  _StopwatchVisitor(this.compilationUnit);
+
+  final ResolvedUnitResult compilationUnit;
+
+  final List<AstNode> stopwatchAccessNodes = <AstNode>[];
+
+  final Map<ClassElement, bool> _isStopwatchClassElementCache = <ClassElement, bool>{};
+
+  bool _checkIfImplementsStopwatchRecurively(ClassElement classElement) {
+    if (classElement.library.isDartCore) {
+      return classElement.name == 'Stopwatch';
+    }
+    return classElement.allSupertypes.any((InterfaceType interface) {
+      final InterfaceElement interfaceElement = interface.element;
+      return interfaceElement is ClassElement && _implementsStopwatch(interfaceElement);
+    });
+  }
+
+  // The cached version, call this method instead of _checkIfImplementsStopwatchRecurively.
+  bool _implementsStopwatch(ClassElement classElement) {
+    return classElement.library.isDartCore
+      ? classElement.name == 'Stopwatch'
+      :_isStopwatchClassElementCache.putIfAbsent(classElement, () => _checkIfImplementsStopwatchRecurively(classElement));
+  }
+
+  bool _isInternal(LibraryElement libraryElement) {
+    return path.isWithin(
+      compilationUnit.session.analysisContext.contextRoot.root.path,
+      libraryElement.source.fullName,
+    );
+  }
+
+  bool _hasTrailingFlutterIgnore(AstNode node) {
+    return compilationUnit.content
+      .substring(node.offset + node.length, compilationUnit.lineInfo.getOffsetOfLineAfter(node.offset + node.length))
+      .contains(_ignoreStopwatch);
+  }
+
+  // We don't care about directives or comments, skip them.
+  @override
+  void visitImportDirective(ImportDirective node) { }
+
+  @override
+  void visitExportDirective(ExportDirective node) { }
+
+  @override
+  void visitComment(Comment node) { }
+
+  @override
+  void visitConstructorName(ConstructorName node) {
+    final Element? element = node.staticElement;
+    if (element is! ConstructorElement) {
+      assert(false, '$element of $node is not a ConstructorElement.');
+      return;
+    }
+    final bool isAllowed = switch (element.returnType) {
+      InterfaceType(element: final ClassElement classElement) => !_implementsStopwatch(classElement),
+      InterfaceType(element: InterfaceElement()) => true,
+    };
+    if (isAllowed || _hasTrailingFlutterIgnore(node)) {
+      return;
+    }
+    stopwatchAccessNodes.add(node);
+  }
+
+  @override
+  void visitSimpleIdentifier(SimpleIdentifier node) {
+    final bool isAllowed = switch (node.staticElement) {
+      ExecutableElement(
+        returnType: DartType(element: final ClassElement classElement),
+        library: final LibraryElement libraryElement
+      ) => _isInternal(libraryElement) || !_implementsStopwatch(classElement),
+      Element() || null => true,
+    };
+    if (isAllowed || _hasTrailingFlutterIgnore(node)) {
+      return;
+    }
+    stopwatchAccessNodes.add(node);
+  }
+}

dev/bots/test/analyze-test-input/root/packages/flutter/lib/stopwatch.dart

@@ -0,0 +1,56 @@
+// 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.
+
+import '../../foo/stopwatch_external_lib.dart' as externallib;
+
+typedef ExternalStopwatchConstructor = externallib.MyStopwatch Function();
+
+class StopwatchAtHome extends Stopwatch {
+  StopwatchAtHome();
+  StopwatchAtHome.create(): this();
+
+  Stopwatch get stopwatch => this;
+}
+
+void testNoStopwatches(Stopwatch stopwatch) {
+  stopwatch.runtimeType;                       // OK for now, but we probably want to catch public APIs that take a Stopwatch?
+  final Stopwatch localVariable = Stopwatch(); // Bad: introducing Stopwatch from dart:core.
+  Stopwatch().runtimeType;                     // Bad: introducing Stopwatch from dart:core.
+
+  (localVariable..runtimeType)                 // OK: not directly introducing Stopwatch.
+   .runtimeType;
+
+  StopwatchAtHome().runtimeType;               // Bad: introducing a Stopwatch subclass.
+
+  Stopwatch anothorStopwatch = stopwatch;      // OK: not directly introducing Stopwatch.
+  StopwatchAtHome Function() constructor = StopwatchAtHome.new; // Bad: introducing a Stopwatch constructor.
+  assert(() {
+    anothorStopwatch = constructor()..runtimeType;
+    constructor = StopwatchAtHome.create;               // Bad: introducing a Stopwatch constructor.
+    anothorStopwatch = constructor()..runtimeType;
+    return true;
+  }());
+  anothorStopwatch.runtimeType;
+
+  externallib.MyStopwatch.create();                     // Bad: introducing an external Stopwatch constructor.
+  ExternalStopwatchConstructor? externalConstructor;
+
+  assert(() {
+    externalConstructor = externallib.MyStopwatch.new;  // Bad: introducing an external Stopwatch constructor.
+    return true;
+  }());
+  externalConstructor?.call();
+
+  externallib.stopwatch.runtimeType;                    // Bad: introducing an external Stopwatch.
+  externallib.createMyStopwatch().runtimeType;          // Bad: calling an external function that returns a Stopwatch.
+  externallib.createStopwatch().runtimeType;            // Bad: calling an external function that returns a Stopwatch.
+  externalConstructor = externallib.createMyStopwatch;  // Bad: introducing the tear-off form of an external function that returns a Stopwatch.
+
+  constructor.call().stopwatch;                         // OK: existing instance.
+}
+
+void testStopwatchIgnore(Stopwatch stopwatch) {
+  Stopwatch().runtimeType;  // flutter_ignore: stopwatch (see analyze.dart)
+  Stopwatch().runtimeType;  // flutter_ignore: some_other_ignores, stopwatch (see analyze.dart)
+}

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

@@ -1,20 +0,0 @@
-// 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_external_lib.dart

@@ -0,0 +1,43 @@
+// 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.
+
+// External Library that creates Stopwatches. This file will not be analyzed but
+// its symbols will be imported by tests.
+
+class MyStopwatch implements Stopwatch {
+  MyStopwatch();
+  MyStopwatch.create(): this();
+
+  @override
+  Duration get elapsed => throw UnimplementedError();
+
+  @override
+  int get elapsedMicroseconds => throw UnimplementedError();
+
+  @override
+  int get elapsedMilliseconds => throw UnimplementedError();
+
+  @override
+  int get elapsedTicks => throw UnimplementedError();
+
+  @override
+  int get frequency => throw UnimplementedError();
+
+  @override
+  bool get isRunning => throw UnimplementedError();
+
+  @override
+  void reset() { }
+
+  @override
+  void start() { }
+
+  @override
+  void stop() { }
+}
+
+final MyStopwatch stopwatch = MyStopwatch.create();
+
+MyStopwatch createMyStopwatch() => MyStopwatch();
+Stopwatch createStopwatch() => Stopwatch();

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

@@ -1,14 +0,0 @@
-// 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

@@ -1,10 +0,0 @@
-// 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

@@ -9,6 +9,7 @@ import 'package:path/path.dart' as path;
 import '../analyze.dart';
 import '../custom_rules/analyze.dart';
 import '../custom_rules/no_double_clamp.dart';
+import '../custom_rules/no_stop_watches.dart';
 import '../utils.dart';
 import 'common.dart';
 
@@ -94,24 +95,6 @@ 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'
@@ -230,9 +213,10 @@ void main() {
   });
 
   test('analyze.dart - clampDouble', () async {
-    final String result = await capture(() => analyzeFrameworkWithRules(
+    final String result = await capture(() => analyzeWithRules(
       testRootPath,
       <AnalyzeRule>[noDoubleClamp],
+      includePaths: <String>['packages/flutter/lib'],
     ), shouldHaveErrors: true);
     final String lines = <String>[
         '║ packages/flutter/lib/bar.dart:37: input.clamp(0.0, 2)',
@@ -252,4 +236,35 @@ void main() {
       '╚═══════════════════════════════════════════════════════════════════════════════\n'
     );
   });
+
+  test('analyze.dart - stopwatch', () async {
+    final String result = await capture(() => analyzeWithRules(
+      testRootPath,
+      <AnalyzeRule>[noStopwatches],
+      includePaths: <String>['packages/flutter/lib'],
+    ), shouldHaveErrors: true);
+    final String lines = <String>[
+      '║ packages/flutter/lib/stopwatch.dart:18: Stopwatch()',
+      '║ packages/flutter/lib/stopwatch.dart:19: Stopwatch()',
+      '║ packages/flutter/lib/stopwatch.dart:24: StopwatchAtHome()',
+      '║ packages/flutter/lib/stopwatch.dart:27: StopwatchAtHome.new',
+      '║ packages/flutter/lib/stopwatch.dart:30: StopwatchAtHome.create',
+      '║ packages/flutter/lib/stopwatch.dart:36: externallib.MyStopwatch.create()',
+      '║ packages/flutter/lib/stopwatch.dart:40: externallib.MyStopwatch.new',
+      '║ packages/flutter/lib/stopwatch.dart:45: externallib.stopwatch',
+      '║ packages/flutter/lib/stopwatch.dart:46: externallib.createMyStopwatch()',
+      '║ packages/flutter/lib/stopwatch.dart:47: externallib.createStopwatch()',
+      '║ packages/flutter/lib/stopwatch.dart:48: externallib.createMyStopwatch'
+    ]
+      .map((String line) => line.replaceAll('/', Platform.isWindows ? r'\' : '/'))
+      .join('\n');
+    expect(result,
+      '╔═╡ERROR╞═══════════════════════════════════════════════════════════════════════\n'
+      '$lines\n'
+      '║ \n'
+      '║ Stopwatches introduce flakes by falling out of sync with the FakeAsync used in testing.\n'
+      '║ A Stopwatch that stays in sync with FakeAsync is available through the Gesture or Test bindings, through samplingClock.\n'
+      '╚═══════════════════════════════════════════════════════════════════════════════\n'
+    );
+  });
 }

packages/flutter/test/gestures/gesture_binding_resample_event_test.dart

@@ -47,7 +47,8 @@ class TestSamplingClock implements SamplingClock {
   DateTime now() => clock.now();
 
   @override
-  Stopwatch stopwatch() => clock.stopwatch();
+  Stopwatch stopwatch() => clock.stopwatch(); // flutter_ignore: stopwatch (see analyze.dart)
+  // Ignore context: FakeAsync controls clock.stopwatch(), this is safe in tests.
 }
 
 typedef ResampleEventTest = void Function(FakeAsync async);