fix: resolve false positives in avoid_returning_widgets (#338)

* fix: resolve false positives in avoid_returning_widgets

* refactor: use _isFlutterType helper for accuracy and update avoid_returning_widgets rule to target State.widget accessors

* fix: avoid_returning_widgets restrict state widget casting exclusion to super/this receivers

---------

Co-authored-by: Illia Aihistov <illia.aihistov-us@solid.software>

Author: solid-illiaaihistov

lib/src/lints/avoid_returning_widgets/visitors/avoid_returning_widgets_visitor.dart

@@ -1,9 +1,9 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element.dart';
-import 'package:analyzer/dart/element/type.dart';
 import 'package:solid_lints/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart';
 import 'package:solid_lints/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart';
+import 'package:solid_lints/src/utils/node_utils.dart';
 import 'package:solid_lints/src/utils/types_utils.dart';
 
 /// A visitor that reports on functions that return widgets.
@@ -40,20 +40,23 @@ class AvoidReturningWidgetsVisitor extends RecursiveAstVisitor<void> {
       return;
     }
 
+    if (node is MethodDeclaration &&
+        (node.isAbstract ||
+            node.body is EmptyFunctionBody ||
+            (node.isGetter && _isStateWidgetCastingGetter(node)))) {
+      return;
+    }
+
     final returnType = switch (node) {
-      Declaration(
-        declaredFragment: ExecutableFragment(
-          element: ExecutableElement(type: FunctionType(:final returnType)),
-        ),
-      ) =>
-        returnType,
-      MethodDeclaration(returnType: TypeAnnotation(:final type)) => type,
-      FunctionDeclaration(returnType: TypeAnnotation(:final type)) => type,
+      MethodDeclaration(:final declaredFragment?) =>
+        declaredFragment.element.returnType,
+      FunctionDeclaration(:final declaredFragment?) =>
+        declaredFragment.element.returnType,
       _ => null,
     };
     if (returnType == null) return;
 
-    final isWidgetReturned = hasWidgetType(returnType);
+    final isWidgetReturned = isWidgetType(returnType);
     if (!isWidgetReturned) return;
 
     final isIgnored = _parameters.exclude.shouldIgnore(node);
@@ -64,7 +67,33 @@ class AvoidReturningWidgetsVisitor extends RecursiveAstVisitor<void> {
     _rule.reportAtNode(node);
   }
 
+  bool _isStateWidgetCastingGetter(MethodDeclaration node) {
+    final enclosingElement = node.declaredFragment?.element.enclosingElement;
+    if (enclosingElement is! InterfaceElement ||
+        !isWidgetStateOrSubclass(enclosingElement.thisType)) {
+      return false;
+    }
+
+    final unwrapped = node.singleReturnExpression.unwrapTarget;
+    if (unwrapped?.targetExpression.isThisOrSuperOrNull != true) {
+      return false;
+    }
+
+    final element = unwrapped?.memberElement;
+    final enclosing = element?.enclosingElement;
+
+    return element is PropertyAccessorElement &&
+        element.name == 'widget' &&
+        enclosing is InterfaceElement &&
+        isWidgetStateOrSubclass(enclosing.thisType);
+  }
+
   bool _isOverridden(Declaration node) {
+    if (node is MethodDeclaration &&
+        node.metadata.any((m) => m.name.name == 'override')) {
+      return true;
+    }
+
     return switch (node) {
       Declaration(
         declaredFragment: Fragment(

lib/src/utils/node_utils.dart

@@ -273,6 +273,7 @@ extension ExpressionExtension on Expression {
   /// Returns the member element referenced or operated on by this expression,
   /// or null if none.
   Element? get memberElement => switch (this) {
+    SimpleIdentifier(:final element) => element,
     MethodInvocation(:final methodName) => methodName.element,
     PropertyAccess(:final propertyName) => propertyName.element,
     AssignmentExpression(:final writeElement, :final readElement) ||
@@ -314,3 +315,17 @@ extension ExpressionNullableExtension on Expression? {
   /// Returns `true` if this expression is `this` or `super`.
   bool get isThisOrSuper => this is ThisExpression || this is SuperExpression;
 }
+
+/// Extension on [MethodDeclaration] to provide AST helper getters.
+extension MethodDeclarationExtension on MethodDeclaration {
+  /// Returns the single return expression of a method, or null if the
+  /// method body has multiple statements or no return expression.
+  Expression? get singleReturnExpression => switch (body) {
+    ExpressionFunctionBody(:final expression) => expression,
+    BlockFunctionBody(
+      block: Block(statements: [ReturnStatement(:final expression?)]),
+    ) =>
+      expression,
+    _ => null,
+  };
+}

lib/src/utils/types_utils.dart

@@ -26,7 +26,6 @@ import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
-import 'package:collection/collection.dart';
 import 'package:solid_lints/src/utils/named_type_utils.dart';
 
 extension Subtypes on DartType {
@@ -144,16 +143,9 @@ extension InterfaceElementExt on InterfaceElement {
   }
 }
 
-bool hasWidgetType(DartType type) =>
-    (isWidgetOrSubclass(type) ||
-        _isIterable(type) ||
-        _isList(type) ||
-        _isFuture(type)) &&
-    !(_isMultiProvider(type) ||
-        _isSubclassOfInheritedProvider(type) ||
-        _isIterableInheritedProvider(type) ||
-        _isListInheritedProvider(type) ||
-        _isFutureInheritedProvider(type));
+bool isWidgetType(DartType type) =>
+    isWidgetOrSubclass(type) &&
+    !(_isMultiProvider(type) || _isSubclassOfInheritedProvider(type));
 
 bool isIterable(DartType? type) =>
     _checkSelfOrSupertypes(type, (t) => t?.isDartCoreIterable ?? false);
@@ -205,52 +197,43 @@ bool _checkSelfOrSupertypes(
     predicate(type) ||
     (type is InterfaceType && type.allSupertypes.any(predicate));
 
-bool _isWidget(DartType? type) => type?.getDisplayString() == 'Widget';
+bool _isWidget(DartType? type) => _isFlutterType(type, 'Widget');
 
 bool _isSubclassOfWidget(DartType? type) =>
     type is InterfaceType && type.allSupertypes.any(_isWidget);
 
-// ignore: deprecated_member_use
-bool _isWidgetState(DartType? type) => type?.element?.displayName == 'State';
+bool _isWidgetState(DartType? type) => _isFlutterType(type, 'State');
 
 bool _isSubclassOfWidgetState(DartType? type) =>
     type is InterfaceType && type.allSupertypes.any(_isWidgetState);
 
-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 _isListenable(DartType type) => type.getDisplayString() == 'Listenable';
+bool _isListenable(DartType? type) => _isFlutterType(type, 'Listenable');
 
-bool _isRenderObject(DartType? type) =>
-    type?.getDisplayString() == 'RenderObject';
+bool _isRenderObject(DartType? type) => _isFlutterType(type, 'RenderObject');
 
 bool _isSubclassOfRenderObject(DartType? type) =>
     type is InterfaceType && type.allSupertypes.any(_isRenderObject);
 
 bool _isRenderObjectWidget(DartType? type) =>
-    type?.getDisplayString() == 'RenderObjectWidget';
+    _isFlutterType(type, 'RenderObjectWidget');
 
 bool _isSubclassOfRenderObjectWidget(DartType? type) =>
     type is InterfaceType && type.allSupertypes.any(_isRenderObjectWidget);
 
 bool _isRenderObjectElement(DartType? type) =>
-    type?.getDisplayString() == 'RenderObjectElement';
+    _isFlutterType(type, 'RenderObjectElement');
 
 bool _isSubclassOfRenderObjectElement(DartType? type) =>
     type is InterfaceType && type.allSupertypes.any(_isRenderObjectElement);
 
+bool _isFlutterType(DartType? type, String name) =>
+    type is InterfaceType &&
+    type.element.name == name &&
+    _isFlutterLibrary(type.element.library);
+
+bool _isFlutterLibrary(LibraryElement library) =>
+    library.uri.scheme == 'package' && library.uri.path.startsWith('flutter/');
+
 bool _isMultiProvider(DartType? type) =>
     type?.getDisplayString() == 'MultiProvider';
 
@@ -260,21 +243,6 @@ bool _isSubclassOfInheritedProvider(DartType? type) =>
 bool _isInheritedProvider(DartType? type) =>
     type != null && type.getDisplayString().startsWith('InheritedProvider<');
 
-bool _isIterableInheritedProvider(DartType type) =>
-    type.isDartCoreIterable &&
-    type is InterfaceType &&
-    _isSubclassOfInheritedProvider(type.typeArguments.firstOrNull);
-
-bool _isListInheritedProvider(DartType type) =>
-    type.isDartCoreList &&
-    type is InterfaceType &&
-    _isSubclassOfInheritedProvider(type.typeArguments.firstOrNull);
-
-bool _isFutureInheritedProvider(DartType type) =>
-    type.isDartAsyncFuture &&
-    type is InterfaceType &&
-    _isSubclassOfInheritedProvider(type.typeArguments.firstOrNull);
-
 bool isIterableOrSubclass(DartType? type) =>
     _checkSelfOrSupertypes(type, (t) => t?.isDartCoreIterable ?? false);
 

test/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule_test.dart

@@ -62,6 +62,16 @@ class BoxDecoration extends Widget {
   Widget build(BuildContext context) => throw 'unimplemented';
 }
 
+abstract class State<T extends StatefulWidget> {
+  T get widget => throw 'unimplemented';
+}
+
+class Color {}
+
+abstract interface class WidgetStateProperty<T> {}
+
+class WidgetStateColor extends Color implements WidgetStateProperty<Color> {}
+
 class DecoratedBox extends Widget {
   const DecoratedBox({required this.decoration});
 
@@ -265,6 +275,112 @@ class NotExcludeWidget extends StatelessWidget {
 
   ${expectLint('Widget excludeWidgetMethod() => const SizedBox();')}
 }
+''');
+  }
+
+  Future<void> test_does_not_report_on_collections() async {
+    await assertNoDiagnostics('''
+$_importFlutterWidgets
+
+class MyWidget extends StatelessWidget {
+  const MyWidget({super.key});
+
+  List<Widget> buildList() => [const SizedBox()];
+
+  @override
+  Widget build(BuildContext context) {
+    return const SizedBox();
+  }
+}
+''');
+  }
+
+  Future<void> test_does_not_report_on_non_widget_types() async {
+    await assertNoDiagnostics('''
+$_importFlutterWidgets
+
+class MyWidget extends StatelessWidget {
+  const MyWidget({super.key});
+
+  WidgetStateColor getColor() => WidgetStateColor();
+
+  @override
+  Widget build(BuildContext context) {
+    return const SizedBox();
+  }
+}
+''');
+  }
+
+  Future<void> test_does_not_report_on_abstract_methods() async {
+    await assertNoDiagnostics('''
+$_importFlutterWidgets
+
+abstract class BaseStrategy {
+  Widget buildHeader(BuildContext context);
+}
+''');
+  }
+
+  Future<void> test_does_not_report_on_state_widget_getters() async {
+    await assertNoDiagnostics('''
+$_importFlutterWidgets
+
+class TargetWidget extends StatefulWidget {
+  const TargetWidget({super.key});
+}
+
+class _TargetWidgetState extends State<StatefulWidget> {
+  TargetWidget get widget => super.widget as TargetWidget;
+  TargetWidget get parenthesizedWidget => ((super.widget as TargetWidget));
+  TargetWidget get blockWidget {
+    return super.widget as TargetWidget;
+  }
+}
+''');
+  }
+
+  Future<void> test_does_not_report_on_inline_builder_callbacks() async {
+    await assertNoDiagnostics('''
+$_importFlutterWidgets
+
+void acceptBuilder(Widget Function(BuildContext) builder) {}
+
+class MyWidget extends StatelessWidget {
+  const MyWidget({super.key});
+
+  @override
+  Widget build(BuildContext context) {
+    acceptBuilder((ctx) => const SizedBox());
+    return const SizedBox();
+  }
+}
+''');
+  }
+
+  Future<void> test_reports_on_non_widget_state_accessors() async {
+    await assertAutoDiagnostics('''
+$_importFlutterWidgets
+
+class OtherState extends State<StatefulWidget> {
+  ${expectLint('Widget get someWidget => const SizedBox();')}
+}
+
+class _TargetWidgetState extends State<StatefulWidget> {
+  late final OtherState otherState;
+
+  ${expectLint('Widget get customWidget => otherState.someWidget;')}
+}
+''');
+  }
+
+  Future<void> test_does_not_report_on_local_non_flutter_widget_class() async {
+    await assertNoDiagnostics('''
+class Widget {}
+
+class CustomService {
+  Widget createCustomWidget() => Widget();
+}
 ''');
   }
 }