-
Notifications
You must be signed in to change notification settings - Fork 6k
Remove unnecessary exceptions from analysis_options.yaml #35054
Conversation
lib/ui/painting.dart
Outdated
| ImmutableBuffer buffer, { | ||
| required int width, | ||
| required int height, | ||
| int? rowBytes, | ||
| required PixelFormat pixelFormat, | ||
| }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undo whitespace changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching that. Fixed.
dnfield
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nit
@yjbanov may want a chance to look at this
|
|
||
| final CkSurface skiaSurface; | ||
| final SubmitCallback submitCallback; | ||
| final bool _submitted; // ignore: unnecessary_null_comparison |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this // ignore: unnecessary_null_comparison referring to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like dart fix messed up the comments. In general, unnecessary_null_comparison is ignored on a global level, so individual ignores are no longer needed. I will remove them here.
| : _submitted = false, | ||
| assert(skiaSurface != null), // ignore: unnecessary_null_comparison | ||
| assert(submitCallback != null); // ignore: unnecessary_null_comparison | ||
| assert(submitCallback != null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like // ignore: unnecessary_null_comparison is still needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed, see other comment.
| const _ParagraphCommand.addPlaceholder( | ||
| _CkParagraphPlaceholder placeholderStyle) | ||
| : this._( | ||
| _ParagraphCommandType.addPlaceholder, null, null, placeholderStyle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate constructors from the fields by an empty line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
| // Skia supports inverse winding as part of path fill type. | ||
| // For Flutter inverse is always false. | ||
| bool _isInverseFillType = false; | ||
| final bool _isInverseFillType = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A final bool field seems like an unnecessary variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed and updated code that was using it.
| late RecordingCanvas _canvas; // ignore: unused_field | ||
|
|
||
| bool _isRecording = true; // ignore: unused_field | ||
| final bool _isRecording = true; // ignore: unused_field |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like RawRecordingCanvas class isn't used anywhere. I think it's dead code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted.
yjbanov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.

Inspired by Dan's comment on #34986 I went through the other declared differences in analyzer options between the
flutter/flutterand theflutter/enginerepository. Turns out, some of these exceptions (namelyimport_internal_library,prefer_final_fields, andsort_constructors_first) are no longer necessary. The stated reason for why they are different fromflutter/flutterdon't appear to apply anymore.This reduces the effective differences between
flutter/flutterandflutter/enginedown to three.This PR also removes the special treatment of
todo,missing_required_param, andmissing_returnas those are also no longer necessary as explained in flutter/flutter#108747.