Skip to content

Commit

Permalink
fix(riverpod_lint): correctly detects nested ref invocations
Browse files Browse the repository at this point in the history
RefInvocations that take as a parameter another RefInvocation are now correctly detected by the analyzer
  • Loading branch information
josh-burton committed Oct 23, 2024
1 parent 5a513c5 commit 5ef007a
Show file tree
Hide file tree
Showing 8 changed files with 267 additions and 11 deletions.
5 changes: 5 additions & 0 deletions packages/riverpod_analyzer_utils/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## Unreleased fix

- Fixed analyzer to correctly detect nested RefInvocations when used as parameters (e.g., ref.watch(provider(ref.watch(...)))). This improves the accuracy of the analyzer's error detection for complex provider compositions. (thanks to @josh-burton)


## 0.5.6 - 2024-10-22

- Support analyzer >=6.7.0 <7.0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ mixin _ParseRefInvocationMixin on RecursiveAstVisitor<void> {
final refInvocation = RefInvocation._parse(node, superCall: superCall);
if (refInvocation != null) {
visitRefInvocation(refInvocation);

for (final argument in refInvocation.node.argumentList.arguments
.whereType<InvocationExpression>()) {
argument.accept(this);
}

// Don't call super as RefInvocation should already be recursive
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,49 @@ final provider = Provider<int>((ref) {
);
});

testSource('Decodes nested ref.read invocations with family providers',
runGenerator: true, source: '''
import 'package:riverpod/riverpod.dart';
import 'package:riverpod_annotation/riverpod_annotation.dart';
part 'foo.g.dart';
final dep = FutureProvider((ref) => 0);
final dep2 = FutureProvider.family((ref, int arg) => 0);
final provider = Provider<int>((ref) {
ref.read(dep2(ref.read(dep)));
return 0;
});
''', (resolver) async {
final result = await resolver.resolveRiverpodAnalysisResult();

expect(result.refReadInvocations, hasLength(2));
expect(result.refInvocations, result.refReadInvocations);

expect(
result.refReadInvocations[0].node.toSource(),
'ref.read(dep2(ref.read(dep)))',
);
expect(result.refReadInvocations[0].function.toSource(), 'read');
expect(
result.refReadInvocations[0].provider.providerElement,
same(
result.legacyProviderDeclarations.findByName('dep2').providerElement,
),
);

expect(result.refReadInvocations[1].node.toSource(), 'ref.read(dep)');
expect(result.refReadInvocations[1].function.toSource(), 'read');
expect(
result.refReadInvocations[1].provider.providerElement,
same(
result.legacyProviderDeclarations.findByName('dep').providerElement,
),
);
});

testSource('Decodes unknown ref usages', source: '''
import 'package:riverpod/riverpod.dart';
import 'package:riverpod_annotation/riverpod_annotation.dart';
Expand Down
4 changes: 4 additions & 0 deletions packages/riverpod_lint/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## Unreleased fix

- provider_dependencies now correctly detects nested ref invocations where a dependency is used as a parameter of another dependency (thanks to @josh-burton)

## 2.6.1 - 2024-10-22

- Support analyzer >=6.7.0 <7.0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ int dep(Ref ref) => 0;
@Riverpod(dependencies: [])
int dep2(Ref ref) => 0;

@Riverpod(dependencies: [])
int dep3(Ref ref, int parameter) => 0;

////////////
// expect_lint: provider_dependencies
Expand Down Expand Up @@ -70,6 +73,18 @@ int extraDep(Ref ref) {
return 0;
}

@Riverpod(
keepAlive: false,
// expect_lint: provider_dependencies
dependencies: [
dep3,
],
)
int onFamilyDep(Ref ref) {
ref.read(dep3Provider(ref.read(dep2Provider)));
return 0;
}

@Riverpod(
keepAlive: false,
dependencies: [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Message: `Specify "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:15`:
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:18`:
```

// expect_lint: provider_dependencies
Expand All @@ -12,7 +12,7 @@ int plainAnnotation(Ref ref) {
---
Message: `Specify "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:22`:
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:25`:
```

// expect_lint: provider_dependencies
Expand All @@ -24,7 +24,7 @@ int customAnnotation(Ref ref) {
---
Message: `Specify "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:30`:
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:33`:
```
// expect_lint: provider_dependencies
@Riverpod(
Expand All @@ -36,7 +36,7 @@ int customAnnotationWithTrailingComma(
---
Message: `Update "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:42`:
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:45`:
```
keepAlive: false,
// expect_lint: provider_dependencies
Expand All @@ -48,7 +48,7 @@ int existingDep(Ref ref) {
---
Message: `Update "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:52`:
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:55`:
```
keepAlive: false,
// expect_lint: provider_dependencies
Expand All @@ -60,7 +60,7 @@ int multipleDeps(Ref ref) {
---
Message: `Update "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:62`:
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:65`:
```
@Riverpod(
keepAlive: false,
Expand All @@ -74,9 +74,23 @@ Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:62`:
int extraDep(Ref ref) {
```
---
Message: `Update "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:79`:
```
keepAlive: false,
// expect_lint: provider_dependencies
- dependencies: [
- dep3,
- ],
+ dependencies: [dep3, dep2],
)
int onFamilyDep(Ref ref) {
```
---
Message: `Remove "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:75`:
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:90`:
```
@Riverpod(
keepAlive: false,
Expand All @@ -92,7 +106,7 @@ int noDep(Ref ref) {
---
Message: `Remove "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:85`:
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:100`:
```

@Riverpod(
Expand All @@ -108,7 +122,7 @@ int dependenciesFirstThenKeepAlive(Ref ref) {
---
Message: `Remove "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:95`:
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:110`:
```
}

Expand All @@ -125,7 +139,7 @@ int noDepNoParam(Ref ref) {
---
Message: `Remove "dependencies"`
Priority: 100
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:106`:
Diff for file `test/lints/provider_dependencies/provider_dependencies.dart:121`:
```

// expect_lint: provider_dependencies
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 5ef007a

Please sign in to comment.