From 32aa3128eea9e37ba4fa56bcbe593b8c29b82468 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Tue, 15 Aug 2023 11:02:30 -0700 Subject: [PATCH] Analyze code snippets in flutter_driver docs (#132337) --- dev/bots/analyze_snippet_code.dart | 49 ++++-- .../custom_imports_broken.dart | 21 +++ dev/bots/test/analyze_snippet_code_test.dart | 5 +- .../flutter_driver/lib/src/driver/driver.dart | 9 +- .../lib/src/driver/gc_summarizer.dart | 2 +- .../lib/src/driver/profiling_summarizer.dart | 2 +- .../src/driver/raster_cache_summarizer.dart | 2 +- .../driver/scene_display_lag_summarizer.dart | 2 +- .../lib/src/extension/extension.dart | 154 +++++++++--------- 9 files changed, 144 insertions(+), 102 deletions(-) create mode 100644 dev/bots/test/analyze-snippet-code-test-input/custom_imports_broken.dart diff --git a/dev/bots/analyze_snippet_code.dart b/dev/bots/analyze_snippet_code.dart index e6de1e4cf59..4b635c90586 100644 --- a/dev/bots/analyze_snippet_code.dart +++ b/dev/bots/analyze_snippet_code.dart @@ -55,10 +55,12 @@ // // At the top of a file you can say `// Examples can assume:` and then list some // commented-out declarations that will be included in the analysis for snippets -// in that file. +// in that file. This section may also contain explicit import statements. // -// Snippets generally import all the main Flutter packages (including material -// and flutter_test), as well as most core Dart packages with the usual prefixes. +// For files without an `// Examples can assume:` section or if that section +// contains no explicit imports, the snippets will implicitly import all the +// main Flutter packages (including material and flutter_test), as well as most +// core Dart packages with the usual prefixes. import 'dart:async'; import 'dart:convert'; @@ -72,6 +74,7 @@ import 'package:watcher/watcher.dart'; final String _flutterRoot = path.dirname(path.dirname(path.dirname(path.fromUri(Platform.script)))); final String _packageFlutter = path.join(_flutterRoot, 'packages', 'flutter', 'lib'); final String _packageFlutterTest = path.join(_flutterRoot, 'packages', 'flutter_test', 'lib'); +final String _packageFlutterDriver = path.join(_flutterRoot, 'packages', 'flutter_driver', 'lib'); final String _packageIntegrationTest = path.join(_flutterRoot, 'packages', 'integration_test', 'lib'); final String _defaultDartUiLocation = path.join(_flutterRoot, 'bin', 'cache', 'pkg', 'sky_engine', 'lib', 'ui'); final String _flutter = path.join(_flutterRoot, 'bin', Platform.isWindows ? 'flutter.bat' : 'flutter'); @@ -153,7 +156,8 @@ Future main(List arguments) async { Directory(_packageFlutter), Directory(_packageFlutterTest), Directory(_packageIntegrationTest), - // TODO(goderbauer): Add all other packages. + Directory(_packageFlutterDriver), + // TODO(goderbauer): Add all other packages for which we publish docs. ]; } @@ -574,6 +578,7 @@ class _SnippetChecker { final List fileLines = file.readAsLinesSync(); final List<_Line> ignorePreambleLinesOnly = <_Line>[]; final List<_Line> preambleLines = <_Line>[]; + final List<_Line> customImports = <_Line>[]; bool inExamplesCanAssumePreamble = false; // Whether or not we're in the file-wide preamble section ("Examples can assume"). bool inToolSection = false; // Whether or not we're in a code snippet bool inDartSection = false; // Whether or not we're in a '```dart' segment. @@ -592,7 +597,11 @@ class _SnippetChecker { throw _SnippetCheckerException('Unexpected content in snippet code preamble.', file: relativeFilePath, line: lineNumber); } else { final _Line newLine = _Line(line: lineNumber, indent: 3, code: line.substring(3)); - preambleLines.add(newLine); + if (newLine.code.startsWith('import ')) { + customImports.add(newLine); + } else { + preambleLines.add(newLine); + } if (line.startsWith('// // ignore_for_file: ')) { ignorePreambleLinesOnly.add(newLine); } @@ -612,7 +621,7 @@ class _SnippetChecker { } if (trimmedLine.startsWith(_codeBlockEndRegex)) { inDartSection = false; - final _SnippetFile snippet = _processBlock(startLine, block, preambleLines, ignorePreambleLinesOnly, relativeFilePath, lastExample); + final _SnippetFile snippet = _processBlock(startLine, block, preambleLines, ignorePreambleLinesOnly, relativeFilePath, lastExample, customImports); final String path = _writeSnippetFile(snippet).path; assert(!snippetMap.containsKey(path)); snippetMap[path] = snippet; @@ -655,6 +664,7 @@ class _SnippetChecker { line.contains('```kotlin') || line.contains('```swift') || line.contains('```glsl') || + line.contains('```json') || line.contains('```csv')) { inOtherBlock = true; } else if (line.startsWith(_uncheckedCodeBlockStartRegex)) { @@ -694,7 +704,7 @@ class _SnippetChecker { /// a primitive heuristic to make snippet blocks into valid Dart code. /// /// `block` argument will get mutated, but is copied before this function returns. - _SnippetFile _processBlock(_Line startingLine, List block, List<_Line> assumptions, List<_Line> ignoreAssumptionsOnly, String filename, _SnippetFile? lastExample) { + _SnippetFile _processBlock(_Line startingLine, List block, List<_Line> assumptions, List<_Line> ignoreAssumptionsOnly, String filename, _SnippetFile? lastExample, List<_Line> customImports) { if (block.isEmpty) { throw _SnippetCheckerException('${startingLine.asLocation(filename, 0)}: Empty ```dart block in snippet code.'); } @@ -755,7 +765,7 @@ class _SnippetChecker { return _SnippetFile.fromStrings( startingLine, block.toList(), - importPreviousExample ? <_Line>[] : headersWithoutImports, + headersWithoutImports, <_Line>[ ...ignoreAssumptionsOnly, if (hasEllipsis) @@ -764,13 +774,24 @@ class _SnippetChecker { 'self-contained program', filename, ); - } else if (hasStatefulWidgetComment) { + } + + final List<_Line> headers = switch ((importPreviousExample, customImports.length)) { + (true, _) => <_Line>[], + (false, 0) => headersWithImports, + (false, _) => <_Line>[ + ...headersWithoutImports, + const _Line.generated(code: '// ignore_for_file: unused_import'), + ...customImports, + ] + }; + if (hasStatefulWidgetComment) { return _SnippetFile.fromStrings( startingLine, prefix: 'class _State extends State {', block.toList(), postfix: '}', - importPreviousExample ? <_Line>[] : headersWithImports, + headers, preamble, 'stateful widget', filename, @@ -782,7 +803,7 @@ class _SnippetChecker { return _SnippetFile.fromStrings( startingLine, block.toList(), - importPreviousExample ? <_Line>[] : headersWithImports, + headers, preamble, 'top-level declaration', filename, @@ -795,7 +816,7 @@ class _SnippetChecker { prefix: 'Future function() async {', block.toList(), postfix: '}', - importPreviousExample ? <_Line>[] : headersWithImports, + headers, preamble, 'statement', filename, @@ -807,7 +828,7 @@ class _SnippetChecker { prefix: 'class Class {', block.toList(), postfix: '}', - importPreviousExample ? <_Line>[] : headersWithImports, + headers, <_Line>[ ...preamble, const _Line.generated(code: '// ignore_for_file: avoid_classes_with_only_static_members'), @@ -849,7 +870,7 @@ class _SnippetChecker { prefix: 'dynamic expression = ', block.toList(), postfix: ';', - importPreviousExample ? <_Line>[] : headersWithImports, + headers, preamble, 'expression', filename, diff --git a/dev/bots/test/analyze-snippet-code-test-input/custom_imports_broken.dart b/dev/bots/test/analyze-snippet-code-test-input/custom_imports_broken.dart new file mode 100644 index 00000000000..cdf466efe26 --- /dev/null +++ b/dev/bots/test/analyze-snippet-code-test-input/custom_imports_broken.dart @@ -0,0 +1,21 @@ +// 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 file is used by ../analyze_snippet_code_test.dart, which depends on the +// precise contents (including especially the comments) of this file. + +// Examples can assume: +// import 'package:flutter/rendering.dart'; + +/// no error: rendering library was imported. +/// ```dart +/// print(RenderObject); +/// ``` +String? bar; + +/// error: widgets library was not imported (not even implicitly). +/// ```dart +/// print(Widget); +/// ``` +String? foo; diff --git a/dev/bots/test/analyze_snippet_code_test.dart b/dev/bots/test/analyze_snippet_code_test.dart index 712052faf6e..90b6366cb93 100644 --- a/dev/bots/test/analyze_snippet_code_test.dart +++ b/dev/bots/test/analyze_snippet_code_test.dart @@ -11,6 +11,7 @@ import 'dart:io'; import 'common.dart'; const List expectedMainErrors = [ + 'dev/bots/test/analyze-snippet-code-test-input/custom_imports_broken.dart:19:11: (statement) (undefined_identifier)', 'dev/bots/test/analyze-snippet-code-test-input/known_broken_documentation.dart:30:5: (expression) (unnecessary_new)', 'dev/bots/test/analyze-snippet-code-test-input/known_broken_documentation.dart:103:5: (statement) (always_specify_types)', 'dev/bots/test/analyze-snippet-code-test-input/known_broken_documentation.dart:111:5: (top-level declaration) (prefer_const_declarations)', @@ -68,7 +69,7 @@ void main() { final List stderrNoDescriptions = stderrLines.map(removeLintDescriptions).toList(); expect(stderrNoDescriptions, [ ...expectedMainErrors, - 'Found 15 snippet code errors.', + 'Found 16 snippet code errors.', 'See the documentation at the top of dev/bots/analyze_snippet_code.dart for details.', '', // because we end with a newline, split gives us an extra blank line ]); @@ -92,7 +93,7 @@ void main() { expect(stderrNoDescriptions, [ ...expectedUiErrors, ...expectedMainErrors, - 'Found 19 snippet code errors.', + 'Found 20 snippet code errors.', 'See the documentation at the top of dev/bots/analyze_snippet_code.dart for details.', '', // because we end with a newline, split gives us an extra blank line ]); diff --git a/packages/flutter_driver/lib/src/driver/driver.dart b/packages/flutter_driver/lib/src/driver/driver.dart index b29becca68e..bedb7f4ffae 100644 --- a/packages/flutter_driver/lib/src/driver/driver.dart +++ b/packages/flutter_driver/lib/src/driver/driver.dart @@ -83,6 +83,11 @@ const CommonFinders find = CommonFinders._(); /// See also [FlutterDriver.waitFor]. typedef EvaluatorFunction = dynamic Function(); +// Examples can assume: +// import 'package:flutter_driver/flutter_driver.dart'; +// import 'package:test/test.dart'; +// late FlutterDriver driver; + /// Drives a Flutter Application running in another process. abstract class FlutterDriver { /// Default constructor. @@ -478,7 +483,7 @@ abstract class FlutterDriver { /// /// ```dart /// test('enters text in a text field', () async { - /// var textField = find.byValueKey('enter-text-field'); + /// final SerializableFinder textField = find.byValueKey('enter-text-field'); /// await driver.tap(textField); // acquire focus /// await driver.enterText('Hello!'); // enter text /// await driver.waitFor(find.text('Hello!')); // verify text appears on UI @@ -520,7 +525,7 @@ abstract class FlutterDriver { /// /// ```dart /// test('submit text in a text field', () async { - /// var textField = find.byValueKey('enter-text-field'); + /// final SerializableFinder textField = find.byValueKey('enter-text-field'); /// await driver.tap(textField); // acquire focus /// await driver.enterText('Hello!'); // enter text /// await driver.waitFor(find.text('Hello!')); // verify text appears on UI diff --git a/packages/flutter_driver/lib/src/driver/gc_summarizer.dart b/packages/flutter_driver/lib/src/driver/gc_summarizer.dart index 2779e782fc7..36abab586c9 100644 --- a/packages/flutter_driver/lib/src/driver/gc_summarizer.dart +++ b/packages/flutter_driver/lib/src/driver/gc_summarizer.dart @@ -17,7 +17,7 @@ const Set kGCRootEvents = { /// Summarizes [TimelineEvents]s corresponding to [kGCRootEvents] category. /// /// A sample event (some fields have been omitted for brevity): -/// ``` +/// ```json /// { /// "name": "StartConcurrentMarking", /// "cat": "GC", diff --git a/packages/flutter_driver/lib/src/driver/profiling_summarizer.dart b/packages/flutter_driver/lib/src/driver/profiling_summarizer.dart index b3e708a6b88..e70e8667975 100644 --- a/packages/flutter_driver/lib/src/driver/profiling_summarizer.dart +++ b/packages/flutter_driver/lib/src/driver/profiling_summarizer.dart @@ -36,7 +36,7 @@ enum ProfileType { /// Summarizes [TimelineEvents]s corresponding to [kProfilingEvents] category. /// /// A sample event (some fields have been omitted for brevity): -/// ``` +/// ```json /// { /// "category": "embedder", /// "name": "CpuUsage", diff --git a/packages/flutter_driver/lib/src/driver/raster_cache_summarizer.dart b/packages/flutter_driver/lib/src/driver/raster_cache_summarizer.dart index 2b750116a8e..3ae673d270a 100644 --- a/packages/flutter_driver/lib/src/driver/raster_cache_summarizer.dart +++ b/packages/flutter_driver/lib/src/driver/raster_cache_summarizer.dart @@ -16,7 +16,7 @@ const String _kPictureMemory = 'PictureMBytes'; /// Summarizes [TimelineEvents]s corresponding to [kRasterCacheEvent] events. /// /// A sample event (some fields have been omitted for brevity): -/// ``` +/// ```json /// { /// "name": "RasterCache", /// "ts": 75598996256, diff --git a/packages/flutter_driver/lib/src/driver/scene_display_lag_summarizer.dart b/packages/flutter_driver/lib/src/driver/scene_display_lag_summarizer.dart index d6eaec031b2..6dd2023e845 100644 --- a/packages/flutter_driver/lib/src/driver/scene_display_lag_summarizer.dart +++ b/packages/flutter_driver/lib/src/driver/scene_display_lag_summarizer.dart @@ -13,7 +13,7 @@ const String _kVsyncTransitionsMissed = 'vsync_transitions_missed'; /// Summarizes [TimelineEvents]s corresponding to [kSceneDisplayLagEvent] events. /// /// A sample event (some fields have been omitted for brevity): -/// ``` +/// ```json /// { /// "name": "SceneDisplayLag", /// "ts": 408920509340, diff --git a/packages/flutter_driver/lib/src/extension/extension.dart b/packages/flutter_driver/lib/src/extension/extension.dart index 2bb6705dd25..6e29825d325 100644 --- a/packages/flutter_driver/lib/src/extension/extension.dart +++ b/packages/flutter_driver/lib/src/extension/extension.dart @@ -58,6 +58,19 @@ class _DriverBinding extends BindingBase with SchedulerBinding, ServicesBinding, } } +// Examples can assume: +// import 'package:flutter_driver/flutter_driver.dart'; +// import 'package:flutter/widgets.dart'; +// import 'package:flutter_driver/driver_extension.dart'; +// import 'package:flutter_test/flutter_test.dart' hide find; +// import 'package:flutter_test/flutter_test.dart' as flutter_test; +// typedef MyHomeWidget = Placeholder; +// abstract class SomeWidget extends StatelessWidget { const SomeWidget({super.key, required this.title}); final String title; } +// late FlutterDriver driver; +// abstract class StubNestedCommand { int get times; SerializableFinder get finder; } +// class StubCommandResult extends Result { const StubCommandResult(this.arg); final String arg; @override Map toJson() => {}; } +// abstract class StubProberCommand { int get times; SerializableFinder get finder; } + /// Enables Flutter Driver VM service extension. /// /// This extension is required for tests that use `package:flutter_driver` to @@ -87,26 +100,40 @@ class _DriverBinding extends BindingBase with SchedulerBinding, ServicesBinding, /// The `finders` and `commands` parameters are optional and used to add custom /// finders or commands, as in the following example. /// -/// ```dart main +/// ```dart /// void main() { /// enableFlutterDriverExtension( /// finders: [ SomeFinderExtension() ], /// commands: [ SomeCommandExtension() ], /// ); /// -/// app.main(); +/// runApp(const MyHomeWidget()); /// } -/// ``` /// -/// ```dart -/// driver.sendCommand(SomeCommand(ByValueKey('Button'), 7)); -/// ``` +/// class SomeFinderExtension extends FinderExtension { +/// @override +/// String get finderType => 'SomeFinder'; /// -/// `SomeFinder` and `SomeFinderExtension` must be placed in different files -/// to avoid `dart:ui` import issue. Imports relative to `dart:ui` can't be -/// accessed from host runner, where flutter runtime is not accessible. +/// @override +/// SerializableFinder deserialize(Map params, DeserializeFinderFactory finderFactory) { +/// return SomeFinder(params['title']!); +/// } /// -/// ```dart +/// @override +/// Finder createFinder(SerializableFinder finder, CreateFinderFactory finderFactory) { +/// final SomeFinder someFinder = finder as SomeFinder; +/// +/// return flutter_test.find.byElementPredicate((Element element) { +/// final Widget widget = element.widget; +/// if (widget is SomeWidget) { +/// return widget.title == someFinder.title; +/// } +/// return false; +/// }); +/// } +/// } +/// +/// // Use this class in a test anywhere where a SerializableFinder is expected. /// class SomeFinder extends SerializableFinder { /// const SomeFinder(this.title); /// @@ -120,43 +147,51 @@ class _DriverBinding extends BindingBase with SchedulerBinding, ServicesBinding, /// 'title': title, /// }); /// } -/// ``` /// -/// ```dart -/// class SomeFinderExtension extends FinderExtension { +/// class SomeCommandExtension extends CommandExtension { +/// @override +/// String get commandKind => 'SomeCommand'; /// -/// String get finderType => 'SomeFinder'; +/// @override +/// Future call(Command command, WidgetController prober, CreateFinderFactory finderFactory, CommandHandlerFactory handlerFactory) async { +/// final SomeCommand someCommand = command as SomeCommand; /// -/// SerializableFinder deserialize(Map params, DeserializeFinderFactory finderFactory) { -/// return SomeFinder(json['title']); -/// } +/// // Deserialize [Finder]: +/// final Finder finder = finderFactory.createFinder(someCommand.finder); /// -/// Finder createFinder(SerializableFinder finder, CreateFinderFactory finderFactory) { -/// Some someFinder = finder as SomeFinder; +/// // Wait for [Element]: +/// handlerFactory.waitForElement(finder); /// -/// return find.byElementPredicate((Element element) { -/// final Widget widget = element.widget; -/// if (element.widget is SomeWidget) { -/// return element.widget.title == someFinder.title; -/// } -/// return false; -/// }); -/// } +/// // Alternatively, wait for [Element] absence: +/// handlerFactory.waitForAbsentElement(finder); +/// +/// // Submit known [Command]s: +/// for (int i = 0; i < someCommand.times; i++) { +/// await handlerFactory.handleCommand(Tap(someCommand.finder), prober, finderFactory); +/// } +/// +/// // Alternatively, use [WidgetController]: +/// for (int i = 0; i < someCommand.times; i++) { +/// await prober.tap(finder); +/// } +/// +/// return const SomeCommandResult('foo bar'); +/// } +/// +/// @override +/// Command deserialize(Map params, DeserializeFinderFactory finderFactory, DeserializeCommandFactory commandFactory) { +/// return SomeCommand.deserialize(params, finderFactory); +/// } /// } -/// ``` /// -/// `SomeCommand`, `SomeResult` and `SomeCommandExtension` must be placed in -/// different files to avoid `dart:ui` import issue. Imports relative to `dart:ui` -/// can't be accessed from host runner, where flutter runtime is not accessible. -/// -/// ```dart +/// // Pass an instance of this class to `FlutterDriver.sendCommand` to invoke +/// // the custom command during a test. /// class SomeCommand extends CommandWithTarget { -/// SomeCommand(SerializableFinder finder, this.times, {Duration? timeout}) -/// : super(finder, timeout: timeout); +/// SomeCommand(super.finder, this.times, {super.timeout}); /// -/// SomeCommand.deserialize(Map json, DeserializeFinderFactory finderFactory) +/// SomeCommand.deserialize(super.json, super.finderFactory) /// : times = int.parse(json['times']!), -/// super.deserialize(json, finderFactory); +/// super.deserialize(); /// /// @override /// Map serialize() { @@ -168,9 +203,7 @@ class _DriverBinding extends BindingBase with SchedulerBinding, ServicesBinding, /// /// final int times; /// } -/// ``` /// -/// ```dart /// class SomeCommandResult extends Result { /// const SomeCommandResult(this.resultParam); /// @@ -184,45 +217,6 @@ class _DriverBinding extends BindingBase with SchedulerBinding, ServicesBinding, /// } /// } /// ``` -/// -/// ```dart -/// class SomeCommandExtension extends CommandExtension { -/// @override -/// String get commandKind => 'SomeCommand'; -/// -/// @override -/// Future call(Command command, WidgetController prober, CreateFinderFactory finderFactory, CommandHandlerFactory handlerFactory) async { -/// final SomeCommand someCommand = command as SomeCommand; -/// -/// // Deserialize [Finder]: -/// final Finder finder = finderFactory.createFinder(stubCommand.finder); -/// -/// // Wait for [Element]: -/// handlerFactory.waitForElement(finder); -/// -/// // Alternatively, wait for [Element] absence: -/// handlerFactory.waitForAbsentElement(finder); -/// -/// // Submit known [Command]s: -/// for (int index = 0; i < someCommand.times; index++) { -/// await handlerFactory.handleCommand(Tap(someCommand.finder), prober, finderFactory); -/// } -/// -/// // Alternatively, use [WidgetController]: -/// for (int index = 0; i < stubCommand.times; index++) { -/// await prober.tap(finder); -/// } -/// -/// return const SomeCommandResult('foo bar'); -/// } -/// -/// @override -/// Command deserialize(Map params, DeserializeFinderFactory finderFactory, DeserializeCommandFactory commandFactory) { -/// return SomeCommand.deserialize(params, finderFactory); -/// } -/// } -/// ``` -/// void enableFlutterDriverExtension({ DataHandler? handler, bool silenceErrors = false, bool enableTextEntryEmulation = true, List? finders, List? commands}) { _DriverBinding(handler, silenceErrors, enableTextEntryEmulation, finders ?? [], commands ?? []); assert(WidgetsBinding.instance is _DriverBinding); @@ -287,7 +281,7 @@ abstract class CommandExtension { /// @override /// Future call(Command command, WidgetController prober, CreateFinderFactory finderFactory, CommandHandlerFactory handlerFactory) async { /// final StubNestedCommand stubCommand = command as StubNestedCommand; - /// for (int index = 0; i < stubCommand.times; index++) { + /// for (int i = 0; i < stubCommand.times; i++) { /// await handlerFactory.handleCommand(Tap(stubCommand.finder), prober, finderFactory); /// } /// return const StubCommandResult('stub response'); @@ -300,7 +294,7 @@ abstract class CommandExtension { /// @override /// Future call(Command command, WidgetController prober, CreateFinderFactory finderFactory, CommandHandlerFactory handlerFactory) async { /// final StubProberCommand stubCommand = command as StubProberCommand; - /// for (int index = 0; i < stubCommand.times; index++) { + /// for (int i = 0; i < stubCommand.times; i++) { /// await prober.tap(finderFactory.createFinder(stubCommand.finder)); /// } /// return const StubCommandResult('stub response');