Skip to content

git commit -m "Fix provider_parameters false positive for extension t…#4804

Open
agarwalrahul2702 wants to merge 2 commits into
rrousselGit:masterfrom
agarwalrahul2702:master
Open

git commit -m "Fix provider_parameters false positive for extension t…#4804
agarwalrahul2702 wants to merge 2 commits into
rrousselGit:masterfrom
agarwalrahul2702:master

Conversation

@agarwalrahul2702

@agarwalrahul2702 agarwalrahul2702 commented Jun 30, 2026

Copy link
Copy Markdown

Related Issues

fixes #4796

Checklist

  • I have updated the CHANGELOG.md of the relevant packages.
  • If this contains new features or behavior changes,
    I have updated the documentation to match those changes.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed a false positive in the provider_parameters lint when using extension type arguments.
    • Lint checks now handle these cases more accurately without flagging valid code.
  • Tests

    • Expanded lint coverage to include additional extension type argument scenarios.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Modifies the provider_parameters lint in riverpod_lint to skip ==/hashCode validation when the instantiated type is an ExtensionTypeElement, preventing a false positive. Adds a changelog entry and extends test coverage with a new TypeId extension type used across multiple ref.watch assertions.

Changes

Extension Type False-Positive Fix

Layer / File(s) Summary
Lint logic fix
packages/riverpod_lint/lib/src/lints/provider_parameters.dart
Extracts instantiatedType, returns early for ExtensionTypeElement, and computes operatorEqual from instantiatedType.
Test coverage update
packages/riverpod_lint_flutter_test/test/lints/provider_parameters.dart
Adds extension type TypeId(int id) {} and new ref.watch(legacy(TypeId(1))) / ref.watch(generatorProvider(value: TypeId(1))) cases across multiple test sections.
Changelog update
packages/riverpod_lint/CHANGELOG.md
Adds an Unreleased fix entry documenting the false-positive fix.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Suggested reviewers

  • rrousselGit

A bunny hopped through extension lines,
No false alarms, no warning signs,
TypeId tucked in tidy and small,
The lint now knows it needn't call,
Carrots safe, the build is green,
Hop hop hooray — the cleanest scene! 🐰🥕

🚥 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 describes the main fix: provider_parameters false positives for extension types.
Linked Issues check ✅ Passed The code now skips the lint check for extension types, matching the issue's requirement to avoid warnings there.
Out of Scope Changes check ✅ Passed The changelog and test updates support the same extension-type lint fix and do not add unrelated scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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.

@coderabbitai coderabbitai Bot left a comment

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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/riverpod_lint/lib/src/lints/provider_parameters.dart (1)

78-90: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

return inside the loop skips checking remaining arguments.

Line 83's if (instantiatedType is ExtensionTypeElement) return; exits the entire _checkExpression method, not just the current loop iteration. If a provider takes multiple parameters (e.g. a tuple/family with several arguments) and an earlier argument happens to be an extension type, every subsequent argument in arguments.arguments is silently skipped — even ones that should be flagged (e.g. a non-const list literal). This should be continue to only skip the current argument's check.

🐛 Proposed fix
       } else if (value is InstanceCreationExpression && !value.isConst) {
         final instantiatedObject = value.constructorName.element
             ?.applyRedirectedConstructors();
         final instantiatedType = instantiatedObject?.enclosingElement;

-        if (instantiatedType is ExtensionTypeElement) return;
+        if (instantiatedType is ExtensionTypeElement) continue;

         final operatorEqual = instantiatedType?.recursiveGetMethod('==');

The added tests only cover single-argument legacy(TypeId(1)) / generatorProvider(value: TypeId(1)) calls, so this multi-argument regression isn't caught by the new coverage.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/riverpod_lint/lib/src/lints/provider_parameters.dart` around lines
78 - 90, The early exit in `_checkExpression` should not stop evaluation of the
rest of `arguments.arguments` when an `InstanceCreationExpression` resolves to
an `ExtensionTypeElement`. Replace the `return` after the `instantiatedType is
ExtensionTypeElement` check with `continue` so only that argument is skipped,
and keep the existing `operatorEqual`/`rule.reportAtNode` logic running for any
remaining arguments in the loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/riverpod_lint/lib/src/lints/provider_parameters.dart`:
- Around line 78-90: The early exit in `_checkExpression` should not stop
evaluation of the rest of `arguments.arguments` when an
`InstanceCreationExpression` resolves to an `ExtensionTypeElement`. Replace the
`return` after the `instantiatedType is ExtensionTypeElement` check with
`continue` so only that argument is skipped, and keep the existing
`operatorEqual`/`rule.reportAtNode` logic running for any remaining arguments in
the loop.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b92b3c05-ba7a-4473-b72c-a5214d317cd4

📥 Commits

Reviewing files that changed from the base of the PR and between 2fa6ad2 and afb71c1.

📒 Files selected for processing (3)
  • packages/riverpod_lint/CHANGELOG.md
  • packages/riverpod_lint/lib/src/lints/provider_parameters.dart
  • packages/riverpod_lint_flutter_test/test/lints/provider_parameters.dart

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.

bug(riverpod_lint): false positiveprovider_parameters on extension types

1 participant