Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid returning widgets lint improvement #139

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
Expand All @@ -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
/// }
/// ```
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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);
}
});
Expand Down
36 changes: 35 additions & 1 deletion lint_test/avoid_returning_widget_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
}
2 changes: 1 addition & 1 deletion lint_test/no_magic_number_test.dart
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading