Avoid returning widgets lint improvement (#139)

* commit * raw version * Improvement for avoid_returning_widget lint * doc improvement * changelog and version * Update CHANGELOG.md Co-authored-by: Illia Romanenko <[email protected]> --------- Co-authored-by: Yurii Prykhodko <[email protected]> Co-authored-by: Illia Romanenko <[email protected]>
solid-software · Mar 14, 2024 · b8d126d · b8d126d
1 parent 46026f7
commit b8d126d
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 - Added `avoid_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
@@ -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
@@ -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
@@ -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