Enable more lints (flutter#91642)

analysis_options.yaml

@@ -56,10 +56,10 @@ linter:
     - annotate_overrides
     # - avoid_annotating_with_dynamic # conflicts with always_specify_types
     - avoid_bool_literals_in_conditional_expressions
-    # - avoid_catches_without_on_clauses # we do this commonly
-    # - avoid_catching_errors # we do this commonly
+    # - 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_classes_with_only_static_members
-    # - avoid_double_and_int_checks # only useful when targeting JS runtime
+    - avoid_double_and_int_checks
     - avoid_dynamic_calls
     - avoid_empty_else
     - avoid_equals_and_hash_code_on_mutable_classes
@@ -68,7 +68,7 @@ linter:
     - avoid_function_literals_in_foreach_calls
     - avoid_implementing_value_types
     - avoid_init_to_null
-    # - avoid_js_rounded_ints # only useful when targeting JS runtime
+    - avoid_js_rounded_ints
     # - avoid_multiple_declarations_per_line # seems to be a stylistic choice we don't subscribe to
     - avoid_null_checks_in_equality_operators
     # - avoid_positional_boolean_parameters # would have been nice to enable this but by now there's too many places that break it
@@ -81,7 +81,7 @@ linter:
     # - avoid_returning_null # still violated by some pre-nnbd code that we haven't yet migrated
     - avoid_returning_null_for_future
     - avoid_returning_null_for_void
-    # - avoid_returning_this # there are plenty of valid reasons to return this
+    # - avoid_returning_this # there are enough valid reasons to return `this` that this lint ends up with too many false positives
     - avoid_setters_without_getters
     - avoid_shadowing_type_parameters
     - avoid_single_cascade_in_expression_statements
@@ -201,7 +201,7 @@ linter:
     - tighten_type_of_initializing_formals
     # - type_annotate_public_apis # subset of always_specify_types
     - type_init_formals
-    # - unawaited_futures # too many false positives
+    # - unawaited_futures # too many false positives, especially with the way AnimationController works
     - unnecessary_await_in_return
     - unnecessary_brace_in_string_interps
     - unnecessary_const
@@ -216,24 +216,24 @@ linter:
     - unnecessary_nullable_for_final_variable_declarations
     - unnecessary_overrides
     - unnecessary_parenthesis
-    # - unnecessary_raw_strings # not yet tested
+    # - unnecessary_raw_strings # what's "necessary" is a matter of opinion; consistency across strings can help readability more than this lint
     - unnecessary_statements
     - unnecessary_string_escapes
     - unnecessary_string_interpolations
     - unnecessary_this
     - unrelated_type_equality_checks
-    # - unsafe_html # not yet tested
+    - unsafe_html
     - use_build_context_synchronously
     - use_full_hex_values_for_flutter_colors
     - use_function_type_syntax_for_parameters
-    # - use_if_null_to_convert_nulls_to_bools # not yet tested
+    # - use_if_null_to_convert_nulls_to_bools # blocked on https://github.com/dart-lang/sdk/issues/47436
     - use_is_even_rather_than_modulo
     - use_key_in_widget_constructors
     - use_late_for_private_fields_and_variables
     - use_named_constants
     - use_raw_strings
     - use_rethrow_when_possible
-    # - use_setters_to_change_properties # not yet tested
+    - use_setters_to_change_properties
     # - use_string_buffers # has false positives: https://github.com/dart-lang/sdk/issues/34182
     - use_test_throws_matchers
     # - use_to_and_as_if_applicable # has false positives, so we prefer to catch this by code-review

dev/analysis_options.yaml

@@ -2,5 +2,10 @@ include: ../analysis_options.yaml
 
 linter:
   rules:
-    avoid_print: false # We use prints as debugging tools here all the time.
-    only_throw_errors: false # Tests use a less... rigorous style.
+    avoid_print: false # We use prints as debugging tools here a lot.
+
+    # Tests try to throw and catch things in exciting ways all the time, so
+    # we disable these lints for the tests.
+    only_throw_errors: false
+    avoid_catching_errors: false
+    avoid_catches_without_on_clauses: false

dev/bots/analysis_options.yaml

@@ -0,0 +1,5 @@
+include: ../analysis_options.yaml
+
+linter:
+  rules:
+    avoid_js_rounded_ints: false # CLI code doesn't need to worry about JS issues

examples/layers/services/isolate.dart

@@ -46,7 +46,7 @@ class Calculator {
       final List<dynamic> result = decoder.convert(_data) as List<dynamic>;
       final int n = result.length;
       onResultListener('Decoded $n results');
-    } catch (e, stack) {
+    } on FormatException catch (e, stack) {
       debugPrint('Invalid JSON file: $e');
       debugPrint('$stack');
     }

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

@@ -5,7 +5,7 @@
 import 'bitfield.dart' as bitfield;
 
 /// The dart:io implementation of [bitfield.kMaxUnsignedSMI].
-const int kMaxUnsignedSMI = 0x3FFFFFFFFFFFFFFF;
+const int kMaxUnsignedSMI = 0x3FFFFFFFFFFFFFFF; // ignore: avoid_js_rounded_ints, (VM-only code)
 
 /// The dart:io implementation of [bitfield.Bitfield].
 class BitField<T extends dynamic> implements bitfield.BitField<T> {

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

@@ -597,34 +597,27 @@ abstract class BindingBase {
         return Future<void>.delayed(Duration.zero);
       });
 
-      Object? caughtException;
-      StackTrace? caughtStack;
       late Map<String, dynamic> result;
       try {
         result = await callback(parameters);
       } catch (exception, stack) {
-        caughtException = exception;
-        caughtStack = stack;
-      }
-      if (caughtException == null) {
-        result['type'] = '_extensionType';
-        result['method'] = method;
-        return developer.ServiceExtensionResponse.result(json.encode(result));
-      } else {
         FlutterError.reportError(FlutterErrorDetails(
-          exception: caughtException,
-          stack: caughtStack,
+          exception: exception,
+          stack: stack,
           context: ErrorDescription('during a service extension callback for "$method"'),
         ));
         return developer.ServiceExtensionResponse.error(
           developer.ServiceExtensionResponse.extensionError,
           json.encode(<String, String>{
-            'exception': caughtException.toString(),
-            'stack': caughtStack.toString(),
+            'exception': exception.toString(),
+            'stack': stack.toString(),
             'method': method,
           }),
         );
       }
+      result['type'] = '_extensionType';
+      result['method'] = method;
+      return developer.ServiceExtensionResponse.result(json.encode(result));
     });
   }
 

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

@@ -2832,6 +2832,8 @@ class DiagnosticsProperty<T> extends DiagnosticsNode {
     try {
       _value = _computeValue!();
     } catch (exception) {
+      // The error is reported to inspector; rethrowing would destroy the
+      // debugging experience.
       _exception = exception;
       _value = null;
     }

packages/flutter/lib/src/rendering/editable.dart

@@ -1091,6 +1091,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
   /// The prompt rectangle will only be requested on non-web iOS applications.
   ///
   /// When set to null, the currently displayed prompt rectangle (if any) will be dismissed.
+  // ignore: use_setters_to_change_properties, (API predates enforcing the lint)
   void setPromptRectRange(TextRange? newRange) {
     _autocorrectHighlightPainter.highlightedRange = newRange;
   }

packages/flutter/lib/src/rendering/error.dart

@@ -49,7 +49,9 @@ class RenderErrorBox extends RenderBox {
         _paragraph = null;
       }
     } catch (error) {
-      // Intentionally left empty.
+      // If an error happens here we're in a terrible state, so we really should
+      // just forget about it and let the developer deal with the already-reported
+      // errors. It's unlikely that these errors are going to help with that.
     }
   }
 
@@ -161,8 +163,10 @@ class RenderErrorBox extends RenderBox {
         }
         context.canvas.drawParagraph(_paragraph!, offset + Offset(left, top));
       }
-    } catch (e) {
-      // Intentionally left empty.
+    } catch (error) {
+      // If an error happens here we're in a terrible state, so we really should
+      // just forget about it and let the developer deal with the already-reported
+      // errors. It's unlikely that these errors are going to help with that.
     }
   }
 }

packages/flutter/lib/src/rendering/sliver_persistent_header.dart

@@ -640,6 +640,7 @@ abstract class RenderSliverFloatingPersistentHeader extends RenderSliverPersiste
   }
 
   /// Update the last known ScrollDirection when scrolling began.
+  // ignore: use_setters_to_change_properties, (API predates enforcing the lint)
   void updateScrollStartDirection(ScrollDirection direction) {
     _lastStartedScrollDirection = direction;
   }

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

@@ -319,6 +319,7 @@ mixin ServicesBinding on BindingBase, SchedulerBinding {
   ///
   ///   * [SystemChrome.setEnabledSystemUIMode], which specifies the
   ///     [SystemUiMode] to have visible when the application is running.
+  // ignore: use_setters_to_change_properties, (API predates enforcing the lint)
   void setSystemUiChangeCallback(SystemUiChangeCallback? callback) {
     _systemUiChangeCallback = callback;
   }
@@ -387,7 +388,6 @@ class _DefaultBinaryMessenger extends BinaryMessenger {
         try {
           response = await handler(data);
         } catch (exception, stack) {
-
           FlutterError.reportError(FlutterErrorDetails(
             exception: exception,
             stack: stack,

packages/flutter/lib/src/services/message_codecs.dart

@@ -376,7 +376,7 @@ class StandardMessageCodec implements MessageCodec<Object?> {
                                    // decoding because we use tags to detect the type of value.
       buffer.putUint8(_valueFloat64);
       buffer.putFloat64(value);
-    } else if (value is int) {
+    } else if (value is int) { // ignore: avoid_double_and_int_checks, JS code always goes through the `double` path above
       if (-0x7fffffff - 1 <= value && value <= 0x7fffffff) {
         buffer.putUint8(_valueInt32);
         buffer.putInt32(value);

packages/flutter/lib/src/services/platform_channel.dart

@@ -410,8 +410,8 @@ class MethodChannel {
       );
     } on MissingPluginException {
       return null;
-    } catch (e) {
-      return codec.encodeErrorEnvelope(code: 'error', message: e.toString());
+    } catch (error) {
+      return codec.encodeErrorEnvelope(code: 'error', message: error.toString());
     }
   }
 

packages/flutter/lib/src/services/restoration.dart

@@ -1006,7 +1006,9 @@ bool debugIsSerializableForRestoration(Object? object) {
     try {
       const StandardMessageCodec().encodeMessage(object);
       result = true;
-    } catch (_) {
+    } catch (error) {
+      // This is only used in asserts, so reporting the exception isn't
+      // particularly useful, since the assert itself will likely fail.
       result = false;
     }
     return true;

packages/flutter/lib/src/widgets/actions.dart

@@ -160,6 +160,7 @@ abstract class Action<T extends Intent> with Diagnosticable {
   final ObserverList<ActionListenerCallback> _listeners = ObserverList<ActionListenerCallback>();
 
   Action<T>? _currentCallingAction;
+  // ignore: use_setters_to_change_properties, (code predates enabling of this lint)
   void _updateCallingAction(Action<T>? value) {
     _currentCallingAction = value;
   }

packages/flutter/lib/src/widgets/editable_text.dart

@@ -2342,6 +2342,7 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
   }
 
   Rect? _currentCaretRect;
+  // ignore: use_setters_to_change_properties, (this is used as a callback, can't be a setter)
   void _handleCaretChanged(Rect caretRect) {
     _currentCaretRect = caretRect;
   }

packages/flutter/lib/src/widgets/form.dart

@@ -421,6 +421,7 @@ class FormFieldState<T> extends State<FormField<T>> with RestorationMixin {
   /// the value should be set by a call to [didChange], which ensures that
   /// `setState` is called.
   @protected
+  // ignore: use_setters_to_change_properties, (API predates enforcing the lint)
   void setValue(T? value) {
     _value = value;
   }

packages/flutter/lib/src/widgets/framework.dart

@@ -4453,8 +4453,10 @@ class ErrorWidget extends LeafRenderObjectWidget {
   static String _stringify(Object? exception) {
     try {
       return exception.toString();
-    } catch (e) {
-      // intentionally left empty.
+    } catch (error) {
+      // If we get here, it means things have really gone off the rails, and we're better
+      // off just returning a simple string and letting the developer find out what the
+      // root cause of all their problems are by looking at the console logs.
     }
     return 'Error';
   }
@@ -5426,6 +5428,9 @@ abstract class RenderObjectElement extends Element {
       if (badAncestors.isNotEmpty) {
         badAncestors.insert(0, result);
         try {
+          // We explicitly throw here (even though we immediately redirect the
+          // exception elsewhere) so that debuggers will notice it when they
+          // have "break on exception" enabled.
           throw FlutterError.fromParts(<DiagnosticsNode>[
             ErrorSummary('Incorrect use of ParentDataWidget.'),
             ErrorDescription('The following ParentDataWidgets are providing parent data to the same RenderObject:'),
@@ -5763,9 +5768,10 @@ abstract class RenderObjectElement extends Element {
           ]);
         }
       } on FlutterError catch (e) {
-        // Catching the exception directly to avoid activating the ErrorWidget.
-        // Since the tree is in a broken state, adding the ErrorWidget would
-        // cause more exceptions.
+        // We catch the exception directly to avoid activating the ErrorWidget,
+        // while still allowing debuggers to break on exception. Since the tree
+        // is in a broken state, adding the ErrorWidget would likely cause more
+        // exceptions, which is not good for the debugging experience.
         _debugReportException(ErrorSummary('while applying parent data.'), e, e.stackTrace);
       }
       return true;
@@ -6036,6 +6042,7 @@ abstract class RootRenderObjectElement extends RenderObjectElement {
   /// to [runApp]. The binding is responsible for driving the build pipeline by
   /// calling the build owner's [BuildOwner.buildScope] method. See
   /// [WidgetsBinding.drawFrame].
+  // ignore: use_setters_to_change_properties, (API predates enforcing the lint)
   void assignOwner(BuildOwner owner) {
     _owner = owner;
   }

packages/flutter/lib/src/widgets/navigator.dart

@@ -175,6 +175,7 @@ abstract class Route<T> {
     }
   }
 
+  // ignore: use_setters_to_change_properties, (setters can't be private)
   void _updateRestorationId(String? restorationId) {
     _restorationScopeId.value = restorationId;
   }

packages/flutter/lib/src/widgets/scroll_position.dart

@@ -320,6 +320,7 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
   ///    middle of layout and applying the new position immediately.
   ///  * [animateTo], which is like [jumpTo] but animating to the
   ///    destination offset.
+  // ignore: use_setters_to_change_properties, (API is intended to discourage setting value)
   void correctPixels(double value) {
     _pixels = value;
   }

packages/flutter/lib/src/widgets/scrollbar.dart

@@ -1277,7 +1277,10 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
     assert (() {
       try {
         scrollController!.position;
-      } catch (_) {
+      } catch (error) {
+        if (scrollController == null || scrollController.positions.length <= 1) {
+          rethrow;
+        }
         throw FlutterError.fromParts(<DiagnosticsNode>[
           ErrorSummary(
             'The $controllerForError is currently attached to more than one '

packages/flutter/lib/src/widgets/sliver.dart

@@ -204,6 +204,7 @@ abstract class SliverChildDelegate {
       if (children != null)
         description.add('estimated child count: $children');
     } catch (e) {
+      // The exception is forwarded to widget inspector.
       description.add('estimated child count: EXCEPTION (${e.runtimeType})');
     }
   }

packages/flutter/lib/src/widgets/text_selection.dart

@@ -1587,7 +1587,13 @@ class ClipboardStatusNotifier extends ValueNotifier<ClipboardStatus> with Widget
     final bool hasStrings;
     try {
       hasStrings = await Clipboard.hasStrings();
-    } catch (stacktrace) {
+    } catch (exception, stack) {
+      FlutterError.reportError(FlutterErrorDetails(
+        exception: exception,
+        stack: stack,
+        library: 'widget library',
+        context: ErrorDescription('while checking if the clipboard has strings'),
+      ));
       // In the case of an error from the Clipboard API, set the value to
       // unknown so that it will try to update again later.
       if (_disposed || value == ClipboardStatus.unknown) {

packages/flutter/lib/src/widgets/widget_inspector.dart

@@ -1962,6 +1962,8 @@ mixin WidgetInspectorService {
         FlutterErrorDetails(
           exception: exception,
           stack: stack,
+          library: 'widget inspector library',
+          context: ErrorDescription('while tracking widget repaints'),
         ),
       );
     }

packages/flutter/test/analysis_options.yaml

@@ -2,4 +2,8 @@ include: ../analysis_options.yaml
 
 linter:
   rules:
-    only_throw_errors: false # We are more flexible for tests.
+    # Tests try to throw and catch things in exciting ways all the time, so
+    # we disable these lints for the tests.
+    only_throw_errors: false
+    avoid_catches_without_on_clauses: false
+    avoid_catching_errors: false