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

feat(riverpod_lint): added avoid use ref inside dispose #2701

Merged
merged 18 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from 17 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
20 changes: 20 additions & 0 deletions packages/riverpod_lint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ Riverpod_lint adds various warnings with quick fixes and refactoring options, su
- [unsupported\_provider\_value (riverpod\_generator only)](#unsupported_provider_value-riverpod_generator-only)
- [stateless\_ref (riverpod\_generator only)](#stateless_ref-riverpod_generator-only)
- [generator\_class\_extends (riverpod\_generator only)](#generator_class_extends-riverpod_generator-only)
- [avoid\_ref\_inside\_state\_dispose](#avoid_ref_inside_state_dispose)
- [All assists](#all-assists)
- [Wrap widgets with a `Consumer`](#wrap-widgets-with-a-consumer)
- [Wrap widgets with a `ProviderScope`](#wrap-widgets-with-a-providerscope)
Expand Down Expand Up @@ -531,6 +532,25 @@ class Example extends Anything {
}
```

### avoid_ref_inside_state_dispose

Avoid using `Ref` in the dispose method.

**Bad**:

```dart
class _MyWidgetState extends ConsumerState<MyWidget> {
@override
void dispose() {
// Do not use 'ref' in the dispose method
ref.read(provider).doSomething();
super.dispose();
}

// ...
}
```

## All assists

### Wrap widgets with a `Consumer`
Expand Down
2 changes: 2 additions & 0 deletions packages/riverpod_lint/lib/riverpod_lint.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'src/assists/wrap_with_consumer.dart';
import 'src/assists/wrap_with_provider_scope.dart';
import 'src/lints/avoid_manual_providers_as_generated_provider_dependency.dart';
import 'src/lints/avoid_public_notifier_properties.dart';
import 'src/lints/avoid_use_ref_inside_dispose.dart';
import 'src/lints/generator_class_extends.dart';
import 'src/lints/missing_provider_scope.dart';
import 'src/lints/provider_dependencies.dart';
Expand All @@ -31,6 +32,7 @@ class _RiverpodPlugin extends PluginBase {
const AvoidManualProvidersAsGeneratedProviderDependency(),
const ScopedProvidersShouldSpecifyDependencies(),
const UnsupportedProviderValue(),
const AvoidUseRefInsideDispose(),
// const AvoidDynamicProviders(),
// // "Avoid passing providers as parameter to objects"
// const AvoidExposingProviderRef(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/error/listener.dart';
import 'package:custom_lint_builder/custom_lint_builder.dart';
import 'package:riverpod_analyzer_utils/riverpod_analyzer_utils.dart';

import '../riverpod_custom_lint.dart';

const disposeMethod = 'dispose';

class AvoidUseRefInsideDispose extends RiverpodLintRule {
Copy link
Owner

@rrousselGit rrousselGit Jul 25, 2023

Choose a reason for hiding this comment

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

Could you rename the class and file name to match the lint code? avoid_ref_inside_state_dispose.dart and AvoidRefInsideStateDispose

const AvoidUseRefInsideDispose() : super(code: _code);

static const _code = LintCode(
name: 'avoid_ref_inside_state_dispose',
problemMessage: "Avoid using 'Ref' inside State.dispose.",
);

@override
void run(
CustomLintResolver resolver,
ErrorReporter reporter,
CustomLintContext context,
) {
context.registry.addMethodInvocation((node) {
final targetType = node.realTarget?.staticType;
if (targetType == null) return;
if (!widgetRefType.isAssignableFromType(targetType)) return;

final ancestor = node.thisOrAncestorMatching((method) {
final isDisposeMethod =
method is MethodDeclaration && method.name.lexeme == disposeMethod;
if (!isDisposeMethod) return false;

return _findConsumerStateClass(node) != null;
});

if (ancestor != null) {
reporter.reportErrorForNode(_code, node);
}
});
}

/// Looking for the ConsumerState class ancestor
/// into the [node] parent.
AstNode? _findConsumerStateClass(AstNode node) {
return node.parent?.thisOrAncestorMatching((node) {
if (node is! ClassDeclaration) return false;

/// Looking for the class which is a [ConsumerState]
final extendsClause = node.extendsClause;
if (extendsClause == null) return false;
final extendsType = extendsClause.superclass.type;
if (extendsType == null) return false;

return consumerStateType.isExactlyType(extendsType);
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import 'package:flutter/widgets.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';

final provider = Provider((ref) => 0);

class MyWidget extends ConsumerStatefulWidget {
rrousselGit marked this conversation as resolved.
Show resolved Hide resolved
const MyWidget({super.key});

@override
ConsumerState<ConsumerStatefulWidget> createState() => _MyWidgetState();
}

class _MyWidgetState extends ConsumerState<MyWidget> {
@override
void dispose() {
// expect_lint: avoid_ref_inside_state_dispose
ref.read(provider);
// expect_lint: avoid_ref_inside_state_dispose
ref.watch(provider);

super.dispose();
}

@override
Widget build(BuildContext context) {
return Container();
}
}

class PlainClass with ChangeNotifier {
PlainClass(this.ref);

final WidgetRef ref;

@override
void dispose() {
ref.read(provider);
ref.watch(provider);

super.dispose();
}
}
Loading