feat: change avoid-returning-widgets

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 * Add static code diagnostics `always-remove-listener` and `avoid-unnecessary-setstate`.
+* Improve static code diagnostic `avoid-returning-widgets`.
 * Remove deprecated `Lines of Executable Code` metric, use `Source lines of Code` instead.
 * Changed the supported `analyzer` version to `^1.7.0`.
 

README.md

@@ -339,7 +339,7 @@ Rules configuration is [described here](#configuring-a-rules-entry).
 
 ### Flutter specific
 
-- [always-remove-listener](https://github.com/dart-code-checker/dart-code-metrics/blob/master/doc/rules/always_remove_listener.md)
+- [always-remove-listener](https://github.com/dart-code-checker/dart-code-metrics/blob/master/doc/rules/always-remove-listener.md)
 - [avoid-returning-widgets](https://github.com/dart-code-checker/dart-code-metrics/blob/master/doc/rules/avoid-returning-widgets.md)   [![Configurable](https://img.shields.io/badge/-configurable-informational)](https://github.com/dart-code-checker/dart-code-metrics/blob/master/doc/rules/avoid-returning-widgets.md#config-example)
 - [avoid-unnecessary-setstate](https://github.com/dart-code-checker/dart-code-metrics/blob/master/doc/rules/avoid-unnecessary-setstate.md)
 

doc/rules/avoid-returning-widgets.md

@@ -8,9 +8,12 @@ avoid-returning-widgets
 
 ## Description
 
-Warns when a method or function returns a Widget or subclass of a Widget. The following patterns will not trigger the rule:
+Warns when a method, function or getter returns a Widget or subclass of a Widget.
+
+The following patterns will not trigger the rule:
 
 - Widget `build` method overrides.
+- Class method that is passed to a builder.
 - Functions with [functional_widget](https://pub.dev/packages/functional_widget) package annotations.
 
 Extracting widgets to a method is considered as a Flutter anti-pattern, because when Flutter rebuilds widget tree, it calls the function all the time, making more processor time for the operations.
@@ -19,12 +22,16 @@ Consider creating a separate widget instead of a function or method.
 
 Additional resources:
 
-* <https://github.com/flutter/flutter/issues/19269>
-* <https://flutter.dev/docs/perf/rendering/best-practices#controlling-build-cost>
-* <https://www.reddit.com/r/FlutterDev/comments/avhvco/extracting_widgets_to_a_function_is_not_an/>
-* <https://medium.com/flutter-community/splitting-widgets-to-methods-is-a-performance-antipattern-16aa3fb4026c>
+- <https://github.com/flutter/flutter/issues/19269>
+- <https://flutter.dev/docs/perf/rendering/best-practices#controlling-build-cost>
+- <https://www.reddit.com/r/FlutterDev/comments/avhvco/extracting_widgets_to_a_function_is_not_an/>
+- <https://medium.com/flutter-community/splitting-widgets-to-methods-is-a-performance-antipattern-16aa3fb4026c>
+
+Use `ignored-names` configuration, if you want to ignore a function or method name.
+
+Use `ignored-annotations` configuration, if you want to override default ignored annotation list.
 
-Use `ignored-names` configuration, if you want to ignore a function or method name. Use `ignored-annotations` configuration, if you want to override default ignored annotation list. For example:
+For example:
 
 ### Config example
 
@@ -51,6 +58,8 @@ class MyWidget extends StatelessWidget {
   const MyWidget();
 
   // LINT
+  Widget _getWidget() => Container();
+
   Widget _buildShinyWidget() {
     return Container(
       child: Column(
@@ -68,7 +77,7 @@ class MyWidget extends StatelessWidget {
       children: [
         Text('Text!'),
         ...
-        _buildShinyWidget(),
+        _buildShinyWidget(), // LINT
       ],
     );
   }
@@ -109,3 +118,20 @@ class _MyShinyWidget extends StatelessWidget {
   }
 }
 ```
+
+Good:
+
+```dart
+class MyWidget extends StatelessWidget {
+  Widget _buildMyWidget(BuildContext context) {
+    return Container();
+  }
+
+  @override
+  Widget build(BuildContext context) {
+    return Builder(
+      builder: _buildMyWidget,
+    );
+  }
+}
+```

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/avoid_returning_widgets.dart

@@ -14,11 +14,15 @@ import '../../rule_utils.dart';
 
 part 'config_parser.dart';
 part 'visitor.dart';
+part 'visit_declaration.dart';
 
 class AvoidReturningWidgetsRule extends Rule {
   static const String ruleId = 'avoid-returning-widgets';
 
   static const _warningMessage = 'Avoid returning widgets from a function.';
+  static const _getterWarningMessage = 'Avoid returning widgets from a getter.';
+  static const _globalFunctionWarningMessage =
+      'Avoid returning widgets from a global function.';
 
   final Iterable<String> _ignoredNames;
   final Iterable<String> _ignoredAnnotations;
@@ -43,16 +47,34 @@ class AvoidReturningWidgetsRule extends Rule {
 
     source.unit.visitChildren(_visitor);
 
-    return _visitor.declarations
-        .map((declaration) => createIssue(
-              rule: this,
-              location: nodeLocation(
-                node: declaration,
-                source: source,
-                withCommentOrMetadata: true,
-              ),
-              message: _warningMessage,
-            ))
-        .toList(growable: false);
+    return [
+      ..._visitor.invocations.map((invocation) => createIssue(
+            rule: this,
+            location: nodeLocation(
+              node: invocation,
+              source: source,
+              withCommentOrMetadata: true,
+            ),
+            message: _warningMessage,
+          )),
+      ..._visitor.getters.map((getter) => createIssue(
+            rule: this,
+            location: nodeLocation(
+              node: getter,
+              source: source,
+              withCommentOrMetadata: true,
+            ),
+            message: _getterWarningMessage,
+          )),
+      ..._visitor.globalFunctions.map((globalFunction) => createIssue(
+            rule: this,
+            location: nodeLocation(
+              node: globalFunction,
+              source: source,
+              withCommentOrMetadata: true,
+            ),
+            message: _globalFunctionWarningMessage,
+          )),
+    ];
   }
 }

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/visit_declaration.dart

@@ -0,0 +1,71 @@
+part of 'avoid_returning_widgets.dart';
+
+Declaration? _visitDeclaration(
+  Declaration node,
+  SimpleIdentifier name,
+  TypeAnnotation? returnType,
+  Iterable<String> ignoredNames,
+  Iterable<String> ignoredAnnotations, {
+  required bool isSetter,
+}) {
+  final hasIgnoredAnnotation = node.metadata.any(
+    (node) =>
+        ignoredAnnotations.contains(node.name.name) &&
+        node.atSign.type == TokenType.AT,
+  );
+
+  if (!hasIgnoredAnnotation &&
+      !isSetter &&
+      !_isIgnored(name.name, ignoredNames)) {
+    final type = returnType?.type;
+    if (type != null && _hasWidgetType(type)) {
+      return node;
+    }
+  }
+
+  return null;
+}
+
+bool _hasWidgetType(DartType type) =>
+    _isWidgetOrSubclass(type) ||
+    _isIterable(type) ||
+    _isList(type) ||
+    _isFuture(type);
+
+bool _isIterable(DartType type) =>
+    type.isDartCoreIterable &&
+    type is InterfaceType &&
+    _isWidgetOrSubclass(type.typeArguments.firstOrNull);
+
+bool _isList(DartType type) =>
+    type.isDartCoreList &&
+    type is InterfaceType &&
+    _isWidgetOrSubclass(type.typeArguments.firstOrNull);
+
+bool _isFuture(DartType type) =>
+    type.isDartAsyncFuture &&
+    type is InterfaceType &&
+    _isWidgetOrSubclass(type.typeArguments.firstOrNull);
+
+bool _isIgnored(
+  String name,
+  Iterable<String> ignoredNames,
+) =>
+    name == 'build' || ignoredNames.contains(name);
+
+bool _isWidgetOrSubclass(DartType? type) =>
+    _isWidget(type) || _isSubclassOfWidget(type);
+
+bool _isWidget(DartType? type) =>
+    type?.getDisplayString(withNullability: false) == 'Widget';
+
+bool _isSubclassOfWidget(DartType? type) =>
+    type is InterfaceType && type.allSupertypes.any(_isWidget);
+
+bool _isStateOrSubclass(DartType? type) =>
+    _isWidgetState(type) || _isSubclassOfWidgetState(type);
+
+bool _isWidgetState(DartType? type) => type?.element?.displayName == 'State';
+
+bool _isSubclassOfWidgetState(DartType? type) =>
+    type is InterfaceType && type.allSupertypes.any(_isWidgetState);