Revert "[CP-beta] Enable the avoid_final_parameters lint. (#185216)"#186270
Conversation
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
There was a problem hiding this comment.
Code Review
This pull request updates analysis_options.yaml to disable the avoid_final_parameters lint and applies the final keyword to parameters in various files across the repository. Review feedback points out that these changes conflict with the Flutter style guide's recommendation to avoid final for parameters to reduce visual noise. The feedback also highlights inconsistencies in function signatures where final was applied only to a subset of parameters and notes that the justification provided in the configuration file is misleading regarding lint compatibility.
| - avoid_field_initializers_in_const_classes | ||
| # TODO(kallentu): Remove this lint once the Dart SDK in Flutter is on version 3.13. | ||
| - avoid_final_parameters | ||
| # - avoid_final_parameters # incompatible with prefer_final_parameters |
There was a problem hiding this comment.
The comment added here is misleading. avoid_final_parameters is not incompatible with prefer_final_parameters in a way that justifies disabling it if the goal is to follow the Flutter style guide. In fact, the Flutter style guide (referenced in the Repository Style Guide) explicitly recommends avoiding final for parameters. If this lint must be disabled for SDK compatibility reasons on this branch, the comment should clarify that instead of referencing an unused lint rule.
References
- The Flutter style guide recommends avoiding the use of 'final' for parameters and local variables to reduce visual noise and maintain a consistent style across the repository. (link)
| final ValueChanged<DatePickerEntryMode>? onDatePickerModeChange, | ||
| final Icon? switchToInputEntryModeIcon, | ||
| final Icon? switchToCalendarEntryModeIcon, | ||
| final CalendarDelegate<DateTime> calendarDelegate = const GregorianCalendarDelegate(), |
There was a problem hiding this comment.
This change introduces an inconsistency in the function signature. Parameters on lines 219-221 are not marked final, while these are. Additionally, the Flutter style guide recommends avoiding final for parameters. It is better to omit the final keyword here to maintain consistency and adhere to the repository's style guidelines.
ValueChanged<DatePickerEntryMode>? onDatePickerModeChange,
Icon? switchToInputEntryModeIcon,
Icon? switchToCalendarEntryModeIcon,
CalendarDelegate<DateTime> calendarDelegate = const GregorianCalendarDelegate(),References
- The Flutter style guide recommends avoiding the use of 'final' for parameters and local variables. (link)
| final Icon? switchToInputEntryModeIcon, | ||
| final Icon? switchToCalendarEntryModeIcon, |
There was a problem hiding this comment.
Inconsistent use of final for parameters in this function signature (compare with lines 1190-1192 and 1195). Following the Flutter style guide, final should be omitted for parameters.
Icon? switchToInputEntryModeIcon,
Icon? switchToCalendarEntryModeIcon,References
- The Flutter style guide recommends avoiding the use of 'final' for parameters and local variables. (link)
|
|
||
| void validatePropertyJsonSerializationHelper( | ||
| Map<String, Object?> json, | ||
| final Map<String, Object?> json, |
There was a problem hiding this comment.
Inconsistent use of final for parameters in this function signature (json is final, but property is not). Per the Flutter style guide, final should be avoided for parameters.
| final Map<String, Object?> json, | |
| Map<String, Object?> json, |
References
- The Flutter style guide recommends avoiding the use of 'final' for parameters and local variables. (link)
| final Key textFieldKey = UniqueKey(); | ||
|
|
||
| Widget builder(int? maxLines, String hintMsg) { | ||
| Widget builder(int? maxLines, final String hintMsg) { |
There was a problem hiding this comment.
Inconsistent use of final for parameters in the builder function (maxLines is not final, but hintMsg is). Per the Flutter style guide, final should be avoided for parameters.
Widget builder(int? maxLines, String hintMsg) {References
- The Flutter style guide recommends avoiding the use of 'final' for parameters and local variables. (link)
| String fileName, | ||
| String header, | ||
| LocaleInfo locale, | ||
| final LocaleInfo locale, |
There was a problem hiding this comment.
Inconsistent use of final for parameters. Only locale is marked final while the other parameters in this method are not. Following the Flutter style guide, final should be omitted.
| final LocaleInfo locale, | |
| LocaleInfo locale, |
References
- The Flutter style guide recommends avoiding the use of 'final' for parameters and local variables. (link)
Reverts #186199. These lint changes aren't necessary for the intended dart version