Document why some lints aren't enabled and fix some minor issues. (#91527)
diff --git a/analysis_options.yaml b/analysis_options.yaml
index f34fbe3..cd428eb 100644
--- a/analysis_options.yaml
+++ b/analysis_options.yaml
@@ -27,13 +27,12 @@
missing_required_param: warning
# treat missing returns as a warning (not a hint)
missing_return: warning
- # allow having TODO/HACK comments in the code
+ # allow having TODO comments in the code
todo: ignore
- hack: ignore
# allow self-reference to deprecated members (we do this because otherwise we have
# to annotate every member in every test, assert, etc, when we deprecate something)
deprecated_member_use_from_same_package: ignore
- # TODO(https://github.com/flutter/flutter/issues/74381):
+ # TODO(ianh): https://github.com/flutter/flutter/issues/74381
# Clean up existing unnecessary imports, and remove line to ignore.
unnecessary_import: ignore
# Turned off until null-safe rollout is complete.
@@ -92,12 +91,12 @@
- avoid_unnecessary_containers
- avoid_unused_constructor_parameters
- avoid_void_async
- # - avoid_web_libraries_in_flutter # not yet tested
+ # - avoid_web_libraries_in_flutter # we use web libraries in web-specific code, and our tests prevent us from using them elsewhere
- await_only_futures
- camel_case_extensions
- camel_case_types
- cancel_subscriptions
- # - cascade_invocations # not yet tested
+ # - cascade_invocations # doesn't match the typical style of this repo
- cast_nullable_to_non_nullable
# - close_sinks # not reliable enough
# - comment_references # blocked on https://github.com/dart-lang/linter/issues/1142
@@ -105,9 +104,9 @@
- control_flow_in_finally
# - curly_braces_in_flow_control_structures # not required by flutter style
- deprecated_consistency
- # - diagnostic_describe_all_properties # not yet tested
+ # - diagnostic_describe_all_properties # enabled only at the framework level (packages/flutter/lib)
- directives_ordering
- # - do_not_use_environment # we do this commonly
+ # - do_not_use_environment # there are appropriate times to use the environment, especially in our tests and build logic
- empty_catches
- empty_constructor_bodies
- empty_statements
@@ -125,7 +124,7 @@
- library_private_types_in_public_api
# - lines_longer_than_80_chars # not required by flutter style
- list_remove_unrelated_type
- # - literal_only_boolean_expressions # too many false positives: https://github.com/dart-lang/sdk/issues/34181
+ # - literal_only_boolean_expressions # too many false positives: https://github.com/dart-lang/linter/issues/453
- missing_whitespace_between_adjacent_strings
- no_adjacent_strings_in_list
# - no_default_cases # too many false positives
diff --git a/dev/devicelab/lib/framework/utils.dart b/dev/devicelab/lib/framework/utils.dart
index ea2856d..a38a740 100644
--- a/dev/devicelab/lib/framework/utils.dart
+++ b/dev/devicelab/lib/framework/utils.dart
@@ -21,22 +21,14 @@
/// The local engine to use for [flutter] and [evalFlutter], if any.
String? get localEngine {
- // Use two distinct `defaultValue`s to determine whether a 'localEngine'
- // declaration exists in the environment.
- const bool isDefined =
- String.fromEnvironment('localEngine', defaultValue: 'a') ==
- String.fromEnvironment('localEngine', defaultValue: 'b');
+ const bool isDefined = bool.hasEnvironment('localEngine');
return isDefined ? const String.fromEnvironment('localEngine') : null;
}
/// The local engine source path to use if a local engine is used for [flutter]
/// and [evalFlutter].
String? get localEngineSrcPath {
- // Use two distinct `defaultValue`s to determine whether a
- // 'localEngineSrcPath' declaration exists in the environment.
- const bool isDefined =
- String.fromEnvironment('localEngineSrcPath', defaultValue: 'a') ==
- String.fromEnvironment('localEngineSrcPath', defaultValue: 'b');
+ const bool isDefined = bool.hasEnvironment('localEngineSrcPath');
return isDefined ? const String.fromEnvironment('localEngineSrcPath') : null;
}
diff --git a/packages/flutter/lib/analysis_options.yaml b/packages/flutter/lib/analysis_options.yaml
new file mode 100644
index 0000000..dbb6737
--- /dev/null
+++ b/packages/flutter/lib/analysis_options.yaml
@@ -0,0 +1,5 @@
+include: ../analysis_options.yaml
+
+linter:
+ rules:
+ # - diagnostic_describe_all_properties # blocked on https://github.com/dart-lang/sdk/issues/47418
diff --git a/packages/flutter_driver/lib/src/driver/driver.dart b/packages/flutter_driver/lib/src/driver/driver.dart
index 3f746b9..3f87dd9 100644
--- a/packages/flutter_driver/lib/src/driver/driver.dart
+++ b/packages/flutter_driver/lib/src/driver/driver.dart
@@ -556,53 +556,55 @@
///
/// The image will be returned as a PNG.
///
- /// HACK: There will be a 2-second artificial delay before screenshotting,
- /// the delay here is to deal with a race between the driver script and
- /// the raster thread (formerly known as the GPU thread). The issue is
- /// that driver API synchronizes with the framework based on transient
- /// callbacks, which are out of sync with the raster thread.
- /// Here's the timeline of events in ASCII art:
+ /// **Warning:** This is not reliable.
///
- /// -------------------------------------------------------------------
- /// Without this delay:
- /// -------------------------------------------------------------------
- /// UI : <-- build -->
- /// Raster: <-- rasterize -->
- /// Gap : | random |
- /// Driver: <-- screenshot -->
+ /// There is a two-second artificial delay before screenshotting. The delay
+ /// here is to deal with a race between the driver script and the raster
+ /// thread (formerly known as the GPU thread). The issue is that the driver
+ /// API synchronizes with the framework based on transient callbacks, which
+ /// are out of sync with the raster thread.
///
- /// In the diagram above, the gap is the time between the last driver
- /// action taken, such as a `tap()`, and the subsequent call to
- /// `screenshot()`. The gap is random because it is determined by the
- /// unpredictable network communication between the driver process and
- /// the application. If this gap is too short, which it typically will
- /// be, the screenshot is taken before the raster thread is done
- /// rasterizing the frame, so the screenshot of the previous frame is
- /// taken, which is wrong.
+ /// Here's the timeline of events in ASCII art:
///
- /// -------------------------------------------------------------------
- /// With this delay, if we're lucky:
- /// -------------------------------------------------------------------
- /// UI : <-- build -->
- /// Raster: <-- rasterize -->
- /// Gap : | 2 seconds or more |
- /// Driver: <-- screenshot -->
+ /// ---------------------------------------------------------------
+ /// Without this delay:
+ /// ---------------------------------------------------------------
+ /// UI : <-- build -->
+ /// Raster: <-- rasterize -->
+ /// Gap : | random |
+ /// Driver: <-- screenshot -->
///
- /// The two-second gap should be long enough for the raster thread to
- /// finish rasterizing the frame, but not longer than necessary to keep
- /// driver tests as fast a possible.
+ /// In the diagram above, the gap is the time between the last driver action
+ /// taken, such as a `tap()`, and the subsequent call to `screenshot()`. The
+ /// gap is random because it is determined by the unpredictable communication
+ /// channel between the driver process and the application. If this gap is too
+ /// short, which it typically will be, the screenshot is taken before the
+ /// raster thread is done rasterizing the frame, so the screenshot of the
+ /// previous frame is taken, which is not what is intended.
///
- /// -------------------------------------------------------------------
- /// With this delay, if we're not lucky:
- /// -------------------------------------------------------------------
- /// UI : <-- build -->
- /// Raster: <-- rasterize randomly slow today -->
- /// Gap : | 2 seconds or more |
- /// Driver: <-- screenshot -->
+ /// ---------------------------------------------------------------
+ /// With this delay, if we're lucky:
+ /// ---------------------------------------------------------------
+ /// UI : <-- build -->
+ /// Raster: <-- rasterize -->
+ /// Gap : | 2 seconds or more |
+ /// Driver: <-- screenshot -->
///
- /// In practice, sometimes the device gets really busy for a while and
- /// even two seconds isn't enough, which means that this is still racy
- /// and a source of flakes.
+ /// The two-second gap should be long enough for the raster thread to finish
+ /// rasterizing the frame, but not longer than necessary to keep driver tests
+ /// as fast a possible.
+ ///
+ /// ---------------------------------------------------------------
+ /// With this delay, if we're not lucky:
+ /// ---------------------------------------------------------------
+ /// UI : <-- build -->
+ /// Raster: <-- rasterize randomly slow today -->
+ /// Gap : | 2 seconds or more |
+ /// Driver: <-- screenshot -->
+ ///
+ /// In practice, sometimes the device gets really busy for a while and even
+ /// two seconds isn't enough, which means that this is still racy and a source
+ /// of flakes.
Future<List<int>> screenshot() async {
throw UnimplementedError();
}
diff --git a/packages/flutter_tools/lib/src/commands/packages.dart b/packages/flutter_tools/lib/src/commands/packages.dart
index bb50782..6258817 100644
--- a/packages/flutter_tools/lib/src/commands/packages.dart
+++ b/packages/flutter_tools/lib/src/commands/packages.dart
@@ -305,10 +305,11 @@
List<String> rest = argResults.rest;
final bool isHelp = rest.contains('-h') || rest.contains('--help');
String target;
- if (rest.length == 1 && (rest[0].contains('/') || rest[0].contains(r'\'))) {
- // HACK: Supporting flutter specific behavior where you can pass a
- // folder to the command.
- target = findProjectRoot(globals.fs, rest[0]);
+ if (rest.length == 1 && (rest.single.contains('/') || rest.single.contains(r'\'))) {
+ // For historical reasons, if there is one argument to the command and it contains
+ // a multiple-component path (i.e. contains a slash) then we use that to determine
+ // to which project we're applying the command.
+ target = findProjectRoot(globals.fs, rest.single);
rest = <String>[];
} else {
target = findProjectRoot(globals.fs);
