diff --git a/CHANGELOG.md b/CHANGELOG.md index a21c1cd..072f379 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ - Added `avoid_using_debug_print` rule - Fixed an issue with no_magic_number lint +- Improvement for `avoid_returning_widget` lint: + - ignores methods that override ones that return widget (build() for example) + - no longer allows returning widgets from methods/functions named build ## 0.1.4 diff --git a/lib/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart b/lib/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart index dcc4f9b..7d68d00 100644 --- a/lib/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart +++ b/lib/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart @@ -1,4 +1,5 @@ import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/error/listener.dart'; import 'package:custom_lint_builder/custom_lint_builder.dart'; import 'package:solid_lints/src/models/rule_config.dart'; @@ -10,6 +11,9 @@ import 'package:solid_lints/src/utils/types_utils.dart'; /// Using functions instead of Widget subclasses for decomposing Widget trees /// may cause unexpected behavior and performance issues. /// +/// Exceptions: +/// - overriden methods +/// /// More details: https://github.com/flutter/flutter/issues/19269 /// /// ### Example @@ -20,7 +24,8 @@ import 'package:solid_lints/src/utils/types_utils.dart'; /// Widget avoidReturningWidgets() => const SizedBox(); // LINT /// /// class MyWidget extends StatelessWidget { -/// Widget _test1() => const SizedBox(); // LINT +/// Widget get box => SizedBox(); // LINT +/// Widget test1() => const SizedBox(); //LINT /// Widget get _test3 => const SizedBox(); // LINT /// } /// ``` @@ -29,7 +34,14 @@ import 'package:solid_lints/src/utils/types_utils.dart'; /// #### GOOD: /// /// ```dart -/// class MyWidget extends StatelessWidget { +/// class MyWidget extends MyWidget { +/// +/// @override +/// Widget test1() => const SizedBox(); +/// +/// @override +/// Widget get box => ColoredBox(color: Colors.pink); +/// /// @override /// Widget build(BuildContext context) { /// return const SizedBox(); @@ -51,7 +63,8 @@ class AvoidReturningWidgetsRule extends SolidLintRule { name: lintName, problemMessage: (_) => 'Returning a widget from a function is considered an anti-pattern. ' - 'Extract your widget to a separate class.', + 'Unless you are overriding an existing method, ' + 'consider extracting your widget to a separate class.', ); return AvoidReturningWidgetsRule._(rule); @@ -64,15 +77,24 @@ class AvoidReturningWidgetsRule extends SolidLintRule { CustomLintContext context, ) { context.registry.addDeclaration((node) { - final isWidgetReturned = switch (node) { + // Check if declaration is function or method, + // simultaneously checks if return type is [DartType] + final DartType? returnType = switch (node) { FunctionDeclaration(returnType: TypeAnnotation(:final type?)) || MethodDeclaration(returnType: TypeAnnotation(:final type?)) => - hasWidgetType(type), - _ => false, + type, + _ => null, }; - // `build` methods return widgets by nature - if (isWidgetReturned && node.declaredElement?.name != "build") { + if (returnType == null) { + return; + } + + final isWidgetReturned = hasWidgetType(returnType); + + final isOverriden = node.declaredElement?.hasOverride ?? false; + + if (isWidgetReturned && !isOverriden) { reporter.reportErrorForNode(code, node); } }); diff --git a/lint_test/avoid_returning_widget_test.dart b/lint_test/avoid_returning_widget_test.dart index ed366e1..cc7a5fa 100644 --- a/lint_test/avoid_returning_widget_test.dart +++ b/lint_test/avoid_returning_widget_test.dart @@ -9,7 +9,25 @@ import 'package:flutter/material.dart'; // expect_lint: avoid_returning_widgets Widget avoidReturningWidgets() => const SizedBox(); -class MyWidget extends StatelessWidget { +class BaseWidget extends StatelessWidget { + const BaseWidget({super.key}); + + // Not allowed even though overriding it is alllowed + // expect_lint: avoid_returning_widgets + Widget get box => SizedBox(); + + // expect_lint: avoid_returning_widgets + Widget decoratedBox() { + return DecoratedBox(decoration: BoxDecoration()); + } + + @override + Widget build(BuildContext context) { + return const Placeholder(); + } +} + +class MyWidget extends BaseWidget { const MyWidget({super.key}); // expect_lint: avoid_returning_widgets @@ -25,8 +43,24 @@ class MyWidget extends StatelessWidget { // expect_lint: avoid_returning_widgets Widget get _test3 => const SizedBox(); + // Allowed + @override + Widget decoratedBox() { + return super.decoratedBox(); + } + + // Allowed + @override + Widget get box => ColoredBox(color: Colors.pink); + + // Allowed @override Widget build(BuildContext context) { return const SizedBox(); } } + +// expect_lint: avoid_returning_widgets +Widget build() { + return Offstage(); +} diff --git a/lint_test/no_magic_number_test.dart b/lint_test/no_magic_number_test.dart index 575b817..4bdc69e 100644 --- a/lint_test/no_magic_number_test.dart +++ b/lint_test/no_magic_number_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: unused_local_variable +// ignore_for_file: unused_local_variable, avoid_returning_widgets // ignore_for_file: prefer_match_file_name // ignore_for_file: avoid_unused_parameters // ignore_for_file: no_empty_block