Skip to content

fix: avoid returning widgets - #371

Merged
solid-illiaaihistov merged 4 commits into
solid-software:masterfrom
solid-illiaaihistov:fix-avoid-returning-widgets
Sep 1, 2026
Merged

fix: avoid returning widgets#371
solid-illiaaihistov merged 4 commits into
solid-software:masterfrom
solid-illiaaihistov:fix-avoid-returning-widgets

Conversation

@solid-illiaaihistov

@solid-illiaaihistov solid-illiaaihistov commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added ignored_types configuration support for suppressing selected lint diagnostics.
    • Supports ignored types provided as a list, map, or single value.
    • Expanded “avoid returning widgets” handling for inherited and provider-based widget types.
  • Bug Fixes
    • Improved detection of ignored types across declared and expression return types.
  • Documentation
    • Added configuration examples for ignored widget types.
  • Tests
    • Added coverage for parsing and ignored-type lint behavior.

@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: fb383d49-eb00-4870-8396-727d3016af6d

📥 Commits

Reviewing files that changed from the base of the PR and between 4ee8098 and 3e0132c.

📒 Files selected for processing (4)
  • lib/src/common/parameters/ignored_types_list_parameter.dart
  • lib/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart
  • lib/src/lints/avoid_returning_widgets/visitors/avoid_returning_widgets_visitor.dart
  • lib/src/utils/node_utils.dart

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change adds a shared ignored-type parameter model, integrates it into three lint rules, and updates avoid-returning-widgets to inspect declared and single-expression return types. Documentation and tests cover supported configuration formats and widget-return filtering.

Changes

Ignored type configuration

Layer / File(s) Summary
Shared ignored-type parameter model
lib/src/common/parameters/ignored_types_list_parameter.dart, test/src/common/parameters/parameters_parsing_test.dart
Adds IgnoredTypesListParameter with multi-format JSON parsing, null-safe type checks, Equatable value semantics, and parsing tests.
Lint parameter and visitor integration
lib/src/lints/avoid_late_keyword/..., lib/src/lints/avoid_non_null_assertion/...
Updates both lint parameter models and visitors to use IgnoredTypesListParameter.shouldIgnore.
Returning-widgets filtering and validation
lib/src/lints/avoid_returning_widgets/..., lib/src/utils/node_utils.dart, lib/src/utils/types_utils.dart, test/src/lints/avoid_returning_widgets/...
Adds ignored-type configuration to avoid-returning-widgets, evaluates declared and single-expression return types, replaces widget classification usage, removes obsolete helpers, and adds documentation and behavior tests.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 3e013

This change only adjusts which widget-return diagnostics a static analysis rule reports and does not affect runtime behavior, permissions, data, or deployment; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant Declaration
  participant AvoidReturningWidgetsVisitor
  participant IgnoredTypesListParameter
  participant isWidgetOrSubclass
  Declaration->>AvoidReturningWidgetsVisitor: Visit declaration
  AvoidReturningWidgetsVisitor->>isWidgetOrSubclass: Check return type
  isWidgetOrSubclass-->>AvoidReturningWidgetsVisitor: Return widget classification
  AvoidReturningWidgetsVisitor->>IgnoredTypesListParameter: Check declared and expression types
  IgnoredTypesListParameter-->>AvoidReturningWidgetsVisitor: Return ignore decision
  AvoidReturningWidgetsVisitor-->>Declaration: Emit or suppress diagnostic
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: fixing the avoid-returning-widgets lint rule. It is concise and related to the pull request changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (4 skipped: 4 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread lib/src/utils/types_utils.dart Outdated
bool isWidgetType(DartType type) =>
isWidgetOrSubclass(type) &&
!(_isMultiProvider(type) || _isSubclassOfInheritedProvider(type));
bool isWidgetType(DartType type) => isWidgetOrSubclass(type);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool isWidgetType(DartType type) => isWidgetOrSubclass(type);

I'm not sure if we need both? the only difference is the name and parameter type, but I don't feel like it's reflected in the name change or needed?

and if we use the null accepting one, we can do

    if (!isWidgetOrSubclass(returnType)) return;

instead of

    if (returnType == null) return;

    if (!isWidgetType(returnType)) return;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Comment thread lib/analysis_options.yaml Outdated
Comment on lines +51 to +54
ignored_types:
- MultiProvider
- InheritedProvider
- InheritedTheme

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we want to make these default

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Comment on lines +46 to +56
@override
bool operator ==(Object other) =>
identical(this, other) ||
other is IgnoredTypesListParameter &&
const SetEquality<String>().equals(
other.ignoredTypes,
ignoredTypes,
);

@override
int get hashCode => const SetEquality<String>().hash(ignoredTypes);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can use Equatable for this

Suggested change
@override
bool operator ==(Object other) =>
identical(this, other) ||
other is IgnoredTypesListParameter &&
const SetEquality<String>().equals(
other.ignoredTypes,
ignoredTypes,
);
@override
int get hashCode => const SetEquality<String>().hash(ignoredTypes);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Comment on lines +60 to +64
_parameters.ignoredTypes.shouldIgnore(returnType) ||
_parameters.ignoredTypes.shouldIgnore(
node.singleReturnExpression?.staticType,
) ||
_parameters.exclude.shouldIgnore(node);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_parameters.ignoredTypes.shouldIgnore(returnType) ||
_parameters.ignoredTypes.shouldIgnore(
node.singleReturnExpression?.staticType,
) ||
_parameters.exclude.shouldIgnore(node);
_parameters.shouldIgnore(node)

I'd rather extract this to AvoidReturningWidgetsParameters

      ignoredTypes.shouldIgnoreAny([
        node.returnType,
        node.singleReturnExpression?.staticType,
      ]) ||
      exclude.shouldIgnore(node);
}

extension on Declaration {
  DartType? get returnType => switch (this) {
    MethodDeclaration(:final declaredFragment?) =>
      declaredFragment.element.returnType,
    FunctionDeclaration(:final declaredFragment?) =>
      declaredFragment.element.returnType,
    _ => null,
  };
}

and

  bool shouldIgnoreAny(List<DartType?> types) => types.any(shouldIgnore);

Also we probably can join all of the if (...) return;s into a single one

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Thank you!

bool _isOverridden(Declaration node) {
if (node is MethodDeclaration &&
node.metadata.any((m) => m.name.name == 'override')) {
if (node is MethodDeclaration && isOverride(node.metadata)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be moved into the switch - MethodDeclaration() when isOverride(...) => true

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@solid-illiaaihistov
solid-illiaaihistov merged commit f815a49 into solid-software:master Sep 1, 2026
2 checks passed
@solid-illiaaihistov
solid-illiaaihistov deleted the fix-avoid-returning-widgets branch September 1, 2026 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants