diff --git a/lib/src/lints/avoid_returning_widgets/visitors/avoid_returning_widgets_visitor.dart b/lib/src/lints/avoid_returning_widgets/visitors/avoid_returning_widgets_visitor.dart index f637acfe..7b9184a4 100644 --- a/lib/src/lints/avoid_returning_widgets/visitors/avoid_returning_widgets_visitor.dart +++ b/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 { 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 { _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( diff --git a/lib/src/utils/node_utils.dart b/lib/src/utils/node_utils.dart index 15562947..880a57e3 100644 --- a/lib/src/utils/node_utils.dart +++ b/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, + }; +} diff --git a/lib/src/utils/types_utils.dart b/lib/src/utils/types_utils.dart index 184387be..59c22378 100644 --- a/lib/src/utils/types_utils.dart +++ b/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); diff --git a/test/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule_test.dart b/test/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule_test.dart index cf822377..f36e64c3 100644 --- a/test/src/lints/avoid_returning_widgets/avoid_returning_widgets_rule_test.dart +++ b/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 get widget => throw 'unimplemented'; +} + +class Color {} + +abstract interface class WidgetStateProperty {} + +class WidgetStateColor extends Color implements WidgetStateProperty {} + class DecoratedBox extends Widget { const DecoratedBox({required this.decoration}); @@ -265,6 +275,112 @@ class NotExcludeWidget extends StatelessWidget { ${expectLint('Widget excludeWidgetMethod() => const SizedBox();')} } +'''); + } + + Future test_does_not_report_on_collections() async { + await assertNoDiagnostics(''' +$_importFlutterWidgets + +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + List buildList() => [const SizedBox()]; + + @override + Widget build(BuildContext context) { + return const SizedBox(); + } +} +'''); + } + + Future 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 test_does_not_report_on_abstract_methods() async { + await assertNoDiagnostics(''' +$_importFlutterWidgets + +abstract class BaseStrategy { + Widget buildHeader(BuildContext context); +} +'''); + } + + Future test_does_not_report_on_state_widget_getters() async { + await assertNoDiagnostics(''' +$_importFlutterWidgets + +class TargetWidget extends StatefulWidget { + const TargetWidget({super.key}); +} + +class _TargetWidgetState extends State { + TargetWidget get widget => super.widget as TargetWidget; + TargetWidget get parenthesizedWidget => ((super.widget as TargetWidget)); + TargetWidget get blockWidget { + return super.widget as TargetWidget; + } +} +'''); + } + + Future 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 test_reports_on_non_widget_state_accessors() async { + await assertAutoDiagnostics(''' +$_importFlutterWidgets + +class OtherState extends State { + ${expectLint('Widget get someWidget => const SizedBox();')} +} + +class _TargetWidgetState extends State { + late final OtherState otherState; + + ${expectLint('Widget get customWidget => otherState.someWidget;')} +} +'''); + } + + Future test_does_not_report_on_local_non_flutter_widget_class() async { + await assertNoDiagnostics(''' +class Widget {} + +class CustomService { + Widget createCustomWidget() => Widget(); +} '''); } }