From 3da003c45ebed519016e14fe0cf406ecd74727e6 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Tue, 14 Jan 2025 12:51:42 -0800 Subject: [PATCH] Last Engine<>Framework lint sync (#161560) This is the last time we have to do this because in https://github.com/flutter/flutter/pull/161554 I am refactoring the engine's analysis_options.yaml to just import the one from the root of the repository. When that lands, lints only have to be enabled in one place to apply across framework and engine. Before we can do that we have to do one last sync to make sure the engine code base is ready. This PR implements that last sync and fixing all lints that came up. --- engine/src/flutter/analysis_options.yaml | 23 +++++++++++++++---- .../flutter/lib/web_ui/analysis_options.yaml | 1 + .../web_ui/lib/src/engine/initialization.dart | 2 +- .../app_lifecycle_state.dart | 2 +- .../web_ui/test/engine/composition_test.dart | 2 +- .../event_position_helper_test.dart | 2 +- .../engine/semantics/text_field_test.dart | 22 +++++++++--------- .../lib/web_ui/test/engine/window_test.dart | 2 +- 8 files changed, 36 insertions(+), 20 deletions(-) diff --git a/engine/src/flutter/analysis_options.yaml b/engine/src/flutter/analysis_options.yaml index f06dc36175..0b5312ab5d 100644 --- a/engine/src/flutter/analysis_options.yaml +++ b/engine/src/flutter/analysis_options.yaml @@ -1,7 +1,7 @@ # Specify analysis options. # # This file is a copy of analysis_options.yaml from flutter repo -# as of 2023-12-18, but with some modifications marked with +# as of 2025-01-13, but with some modifications marked with # "DIFFERENT FROM FLUTTER/FLUTTER" below. analyzer: @@ -36,10 +36,11 @@ linter: # - always_specify_types # DIFFERENT FROM FLUTTER/FLUTTER; see https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#dart # - always_use_package_imports # we do this commonly - annotate_overrides + - annotate_redeclares # - avoid_annotating_with_dynamic # conflicts with always_specify_types - avoid_bool_literals_in_conditional_expressions # - avoid_catches_without_on_clauses # blocked on https://github.com/dart-lang/linter/issues/3023 - # - avoid_catching_errors # blocked on https://github.com/dart-lang/linter/issues/3023 + # - avoid_catching_errors # blocked on https://github.com/dart-lang/linter/issues/4998 # - avoid_classes_with_only_static_members # we do this commonly for `abstract final class`es - avoid_double_and_int_checks - avoid_dynamic_calls @@ -49,6 +50,7 @@ linter: - avoid_field_initializers_in_const_classes # - avoid_final_parameters # incompatible with prefer_final_parameters - avoid_function_literals_in_foreach_calls + # - avoid_futureor_void # not yet tested # - avoid_implementing_value_types # see https://github.com/dart-lang/linter/issues/4558 - avoid_init_to_null - avoid_js_rounded_ints @@ -96,6 +98,7 @@ linter: - directives_ordering # - discarded_futures # too many false positives, similar to unawaited_futures # - do_not_use_environment # there are appropriate times to use the environment, especially in our tests and build logic + # - document_ignores # not yet tested - empty_catches - empty_constructor_bodies - empty_statements @@ -108,6 +111,7 @@ linter: - implicit_call_tearoffs - implicit_reopen - invalid_case_patterns + - invalid_runtime_check_with_js_interop_types # - join_return_with_assignment # not required by flutter style - leading_newlines_in_multiline_strings - library_annotations @@ -134,6 +138,8 @@ linter: - null_check_on_nullable_type_parameter - null_closures # - omit_local_variable_types # opposite of always_specify_types + # - omit_obvious_local_variable_types # not yet tested + # - omit_obvious_property_types # not yet tested # - one_member_abstracts # too many false positives - only_throw_errors # this does get disabled in a few places where we have legacy code that uses strings et al - overridden_fields @@ -152,7 +158,7 @@ linter: # - prefer_constructors_over_static_methods # far too many false positives - prefer_contains # - prefer_double_quotes # opposite of prefer_single_quotes - # - prefer_expression_function_bodies # conflicts with https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#consider-using--for-short-functions-and-methods + # - prefer_expression_function_bodies # conflicts with ./docs/contributing/Style-guide-for-Flutter-repo.md#consider-using--for-short-functions-and-methods - prefer_final_fields - prefer_final_in_for_each - prefer_final_locals @@ -165,7 +171,7 @@ linter: - prefer_if_null_operators - prefer_initializing_formals - prefer_inlined_adds - # - prefer_int_literals # conflicts with https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#use-double-literals-for-double-constants + # - prefer_int_literals # conflicts with ./docs/contributing/Style-guide-for-Flutter-repo.md#use-double-literals-for-double-constants - prefer_interpolation_to_compose_strings - prefer_is_empty - prefer_is_not_empty @@ -191,6 +197,9 @@ linter: - sort_constructors_first # - sort_pub_dependencies # prevents separating pinned transitive dependencies - sort_unnamed_constructors_first + # - specify_nonobvious_local_variable_types # not yet tested + # - specify_nonobvious_property_types # not yet tested + - strict_top_level_inference - test_types_in_equals - throw_in_finally - tighten_type_of_initializing_formals @@ -198,6 +207,8 @@ linter: - type_init_formals - type_literal_in_constant_pattern # - unawaited_futures # too many false positives, especially with the way AnimationController works + # - unintended_html_in_doc_comment # blocked on https://github.com/dart-lang/linter/issues/5065 + # - unnecessary_async # not yet tested - unnecessary_await_in_return - unnecessary_brace_in_string_interps - unnecessary_breaks @@ -208,6 +219,7 @@ linter: # - unnecessary_lambdas # has false positives: https://github.com/dart-lang/linter/issues/498 - unnecessary_late - unnecessary_library_directive + # - unnecessary_library_name # blocked on https://github.com/dart-lang/dartdoc/issues/3882 - unnecessary_new - unnecessary_null_aware_assignments - unnecessary_null_aware_operator_on_extension_on_nullable @@ -222,8 +234,10 @@ linter: - unnecessary_string_interpolations - unnecessary_this - unnecessary_to_list_in_spreads + - unnecessary_underscores - unreachable_from_main - unrelated_type_equality_checks + # - unsafe_variance # not yet tested - use_build_context_synchronously - use_colored_box # - use_decorated_box # leads to bugs: DecoratedBox and Container are not equivalent (Container inserts extra padding) @@ -243,5 +257,6 @@ linter: - use_super_parameters - use_test_throws_matchers # - use_to_and_as_if_applicable # has false positives, so we prefer to catch this by code-review + - use_truncating_division - valid_regexps - void_checks diff --git a/engine/src/flutter/lib/web_ui/analysis_options.yaml b/engine/src/flutter/lib/web_ui/analysis_options.yaml index 0938119b50..3a172dcb3c 100644 --- a/engine/src/flutter/lib/web_ui/analysis_options.yaml +++ b/engine/src/flutter/lib/web_ui/analysis_options.yaml @@ -10,6 +10,7 @@ linter: rules: avoid_print: false avoid_setters_without_getters: false + invalid_runtime_check_with_js_interop_types: false library_private_types_in_public_api: false no_default_cases: false prefer_relative_imports: false diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/initialization.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/initialization.dart index 1bb537c56e..bee021d80a 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/initialization.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/initialization.dart @@ -141,7 +141,7 @@ Future initializeEngineServices({ // // This extension does not need to clean-up Dart statics. Those are cleaned // up by the compiler. - developer.registerExtension('ext.flutter.disassemble', (_, __) { + developer.registerExtension('ext.flutter.disassemble', (_, _) { for (final ui.VoidCallback listener in _hotRestartListeners) { listener(); } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/app_lifecycle_state.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/app_lifecycle_state.dart index f78e14f046..da5808c2f7 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/app_lifecycle_state.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/app_lifecycle_state.dart @@ -104,7 +104,7 @@ class _BrowserAppLifecycleState extends AppLifecycleState { } }); - void _onViewCountChanged(_) { + void _onViewCountChanged(int _) { if (_viewManager.views.isEmpty) { onAppLifecycleStateChange(ui.AppLifecycleState.detached); } else { diff --git a/engine/src/flutter/lib/web_ui/test/engine/composition_test.dart b/engine/src/flutter/lib/web_ui/test/engine/composition_test.dart index 2c30a44ad4..e35070a8de 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/composition_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/composition_test.dart @@ -43,7 +43,7 @@ GloballyPositionedTextEditingStrategy _enableEditingStrategy({ owner.debugTextEditingStrategyOverride = editingStrategy; - editingStrategy.enable(owner.configuration!, onChange: onChange ?? (_, __) {}, onAction: (_) {}); + editingStrategy.enable(owner.configuration!, onChange: onChange ?? (_, _) {}, onAction: (_) {}); return editingStrategy; } diff --git a/engine/src/flutter/lib/web_ui/test/engine/pointer_binding/event_position_helper_test.dart b/engine/src/flutter/lib/web_ui/test/engine/pointer_binding/event_position_helper_test.dart index 35ea7b8df4..b326062a05 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/pointer_binding/event_position_helper_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/pointer_binding/event_position_helper_test.dart @@ -111,7 +111,7 @@ void doTests() { textEditing.strategy.enable( InputConfiguration(viewId: view.viewId), - onChange: (_, __) {}, + onChange: (_, _) {}, onAction: (_) {}, ); diff --git a/engine/src/flutter/lib/web_ui/test/engine/semantics/text_field_test.dart b/engine/src/flutter/lib/web_ui/test/engine/semantics/text_field_test.dart index 82270feb5c..617681680f 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/semantics/text_field_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/semantics/text_field_test.dart @@ -153,7 +153,7 @@ void testMain() { int actionCount = 0; strategy.enable( singlelineConfig, - onChange: (_, __) { + onChange: (_, _) { changeCount++; }, onAction: (_) { @@ -218,7 +218,7 @@ void testMain() { }); test('Does not overwrite text value and selection editing state on semantic updates', () { - strategy.enable(singlelineConfig, onChange: (_, __) {}, onAction: (_) {}); + strategy.enable(singlelineConfig, onChange: (_, _) {}, onAction: (_) {}); final textFieldSemantics = createTextFieldSemantics( value: 'hello', @@ -242,7 +242,7 @@ void testMain() { test('Updates editing state when receiving framework messages from the text input channel', () { expect(owner().semanticsHost.ownerDocument?.activeElement, domDocument.body); - strategy.enable(singlelineConfig, onChange: (_, __) {}, onAction: (_) {}); + strategy.enable(singlelineConfig, onChange: (_, _) {}, onAction: (_) {}); final textFieldSemantics = createTextFieldSemantics( value: 'hello', @@ -280,7 +280,7 @@ void testMain() { test('Gives up focus after DOM blur', () { expect(owner().semanticsHost.ownerDocument?.activeElement, domDocument.body); - strategy.enable(singlelineConfig, onChange: (_, __) {}, onAction: (_) {}); + strategy.enable(singlelineConfig, onChange: (_, _) {}, onAction: (_) {}); final textFieldSemantics = createTextFieldSemantics(value: 'hello', isFocused: true); final textField = textFieldSemantics.semanticRole! as SemanticTextField; @@ -294,7 +294,7 @@ void testMain() { }); test('Does not dispose and recreate dom elements in persistent mode', () async { - strategy.enable(singlelineConfig, onChange: (_, __) {}, onAction: (_) {}); + strategy.enable(singlelineConfig, onChange: (_, _) {}, onAction: (_) {}); // It doesn't create a new DOM element. expect(strategy.domElement, isNull); @@ -323,7 +323,7 @@ void testMain() { }); test('Refocuses when setting editing state', () { - strategy.enable(singlelineConfig, onChange: (_, __) {}, onAction: (_) {}); + strategy.enable(singlelineConfig, onChange: (_, _) {}, onAction: (_) {}); createTextFieldSemantics(value: 'hello', isFocused: true); expect(strategy.domElement, isNotNull); @@ -352,7 +352,7 @@ void testMain() { }); test('Works in multi-line mode', () { - strategy.enable(multilineConfig, onChange: (_, __) {}, onAction: (_) {}); + strategy.enable(multilineConfig, onChange: (_, _) {}, onAction: (_) {}); createTextFieldSemantics(value: 'hello', isFocused: true, isMultiline: true); final textArea = strategy.domElement! as DomHTMLTextAreaElement; @@ -360,7 +360,7 @@ void testMain() { expect(owner().semanticsHost.ownerDocument?.activeElement, strategy.domElement); - strategy.enable(singlelineConfig, onChange: (_, __) {}, onAction: (_) {}); + strategy.enable(singlelineConfig, onChange: (_, _) {}, onAction: (_) {}); textArea.blur(); expect(owner().semanticsHost.ownerDocument?.activeElement, domDocument.body); @@ -373,7 +373,7 @@ void testMain() { }); test('multi-line and obscured', () { - strategy.enable(multilineConfig, onChange: (_, __) {}, onAction: (_) {}); + strategy.enable(multilineConfig, onChange: (_, _) {}, onAction: (_) {}); createTextFieldSemantics( value: 'hello', isFocused: true, @@ -391,7 +391,7 @@ void testMain() { }, skip: ui_web.browser.browserEngine == ui_web.BrowserEngine.firefox); test('Does not position or size its DOM element', () { - strategy.enable(singlelineConfig, onChange: (_, __) {}, onAction: (_) {}); + strategy.enable(singlelineConfig, onChange: (_, _) {}, onAction: (_) {}); // Send width and height that are different from semantics values on // purpose. @@ -450,7 +450,7 @@ void testMain() { } test('Changes focus from one text field to another through a semantics update', () { - strategy.enable(singlelineConfig, onChange: (_, __) {}, onAction: (_) {}); + strategy.enable(singlelineConfig, onChange: (_, _) {}, onAction: (_) {}); // Switch between the two fields a few times. for (int i = 0; i < 5; i++) { diff --git a/engine/src/flutter/lib/web_ui/test/engine/window_test.dart b/engine/src/flutter/lib/web_ui/test/engine/window_test.dart index 27fa2f493a..e6d0fe0c7a 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/window_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/window_test.dart @@ -279,7 +279,7 @@ Future testMain() async { final Zone innerZone = Zone.current.fork(); innerZone.runGuarded(() { - void callback(String _, ByteData? __, void Function(ByteData?)? ___) { + void callback(String _, ByteData? _, void Function(ByteData?)? _) { expect(Zone.current, innerZone); }