Skip to content

Commit

Permalink
Fix scoped_providers_should_specify_dependencies incorrectly triggeri…
Browse files Browse the repository at this point in the history
…ng on functions other than "main"

fixes #2340
  • Loading branch information
rrousselGit committed Apr 6, 2023
1 parent cba5d99 commit 65ea983
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 12 deletions.
1 change: 1 addition & 0 deletions packages/riverpod_lint/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## Unreleased fix

- Disable unsupported_provider_value when a notifier returns "this"
- Fix scoped_providers_should_specify_dependencies incorrectly triggering on functions other than "main"

## 1.1.7 - 2023-04-06

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import 'package:collection/collection.dart';
import 'package:custom_lint_builder/custom_lint_builder.dart';
import 'package:riverpod_analyzer_utils/riverpod_analyzer_utils.dart';

import 'scoped_providers_should_specify_dependencies.dart';

class MissingProviderScope extends DartLintRule {
const MissingProviderScope() : super(code: _code);

Expand All @@ -22,7 +24,7 @@ class MissingProviderScope extends DartLintRule {
CustomLintContext context,
) {
context.registry.addMethodInvocation((node) {
if (node.methodName.name != 'runApp') return;
if (!node.methodName.isFlutterRunApp) return;
final function = node.function;
if (function is! SimpleIdentifier) return;
final functionElement = function.staticElement;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@ import 'package:riverpod_analyzer_utils/riverpod_analyzer_utils.dart';

import '../riverpod_custom_lint.dart';

extension SimpleIdentifierX on SimpleIdentifier {
bool get isFlutterRunApp {
if (name != 'runApp') return false;

final library = staticElement?.library;
if (library == null) return false;
final libraryUri = Uri.tryParse(library.identifier);
if (libraryUri == null) return false;

return libraryUri.scheme == 'package' &&
libraryUri.pathSegments.first == 'flutter';
}
}

class ScopedProvidersShouldSpecifyDependencies extends RiverpodLintRule {
const ScopedProvidersShouldSpecifyDependencies() : super(code: _code);

Expand Down Expand Up @@ -62,18 +76,20 @@ class ScopedProvidersShouldSpecifyDependencies extends RiverpodLintRule {
bool isProviderScopeScoped(
ProviderScopeInstanceCreationExpression expression,
) {
final hasParent = expression.node.argumentList.arguments
final hasParentParameter = expression.node.argumentList.arguments
.whereType<NamedExpression>()
// TODO handle parent:null.
// This might be doable by checking that the expression's
// static type is non-nullable
.any((e) => e.name.label.name == 'parent');
if (hasParent) return true;
if (hasParentParameter) return true;

final enclosingFunction =
expression.node.thisOrAncestorOfType<FunctionDeclaration>();
// in runApp(ProviderScope(..)) the direct parent of the ProviderScope
// is an ArgumentList.
final enclosingExpression = expression.node.parent?.parent;

// Any ProviderScope without parent outside of the main are considered "scoped".
return enclosingFunction == null || enclosingFunction.name.lexeme != 'main';
// If the ProviderScope isn't directly as a child of runApp, it is scoped
return enclosingExpression is! MethodInvocation ||
!enclosingExpression.methodName.isFlutterRunApp;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/dart/element/type_system.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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@ void main() {
);
}

void definitelyNotAMain() {
// expect_lint: missing_provider_scope
runApp(
MyApp(),
);
runApp(ProviderScope(child: MyApp()));
runApp(
UncontrolledProviderScope(
container: ProviderContainer(),
child: MyApp(),
),
);
}

class MyApp extends StatelessWidget {
const MyApp({super.key});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import 'package:flutter/material.dart';
// ignore_for_file: unused_local_variable

import 'package:flutter/material.dart' as flutter;
import 'package:flutter/material.dart' hide runApp;
import 'package:hooks_riverpod/hooks_riverpod.dart';
import 'package:riverpod_annotation/riverpod_annotation.dart';

Expand All @@ -13,6 +16,9 @@ external int unimplementedScoped();
@riverpod
int root(RootRef ref) => 0;

// A fake runApp to check that we lint only on the official Flutter's runApp
void runApp(Widget widget) {}

void main() {
final rootContainer = ProviderContainer(
overrides: [
Expand All @@ -22,7 +28,6 @@ void main() {
],
);

// ignore: unused_local_variable
final scopedContainer = ProviderContainer(
parent: rootContainer,
overrides: [
Expand All @@ -33,7 +38,7 @@ void main() {
],
);

runApp(
flutter.runApp(
ProviderScope(
overrides: [
scopedProvider.overrideWith((ref) => 0),
Expand All @@ -45,6 +50,64 @@ void main() {
);

runApp(
ProviderScope(
overrides: [
scopedProvider.overrideWith((ref) => 0),
unimplementedScopedProvider.overrideWith((ref) => 0),
// This is not a Flutter's runApp, so the ProviderScope is considered scoped
// expect_lint: scoped_providers_should_specify_dependencies
rootProvider.overrideWith((ref) => 0),
],
child: Container(),
),
);

flutter.runApp(
ProviderScope(
parent: rootContainer,
overrides: [
scopedProvider.overrideWith((ref) => 0),
unimplementedScopedProvider.overrideWith((ref) => 0),
// expect_lint: scoped_providers_should_specify_dependencies
rootProvider.overrideWith((ref) => 0),
],
child: Container(),
),
);
}

// Regression tests for https://github.com/rrousselGit/riverpod/issues/2340
void definitelyNotAMain() {
final rootContainer = ProviderContainer(
overrides: [
scopedProvider.overrideWith((ref) => 0),
unimplementedScopedProvider.overrideWith((ref) => 0),
rootProvider.overrideWith((ref) => 0),
],
);

final scopedContainer = ProviderContainer(
parent: rootContainer,
overrides: [
scopedProvider.overrideWith((ref) => 0),
unimplementedScopedProvider.overrideWith((ref) => 0),
// expect_lint: scoped_providers_should_specify_dependencies
rootProvider.overrideWith((ref) => 0),
],
);

flutter.runApp(
ProviderScope(
overrides: [
scopedProvider.overrideWith((ref) => 0),
unimplementedScopedProvider.overrideWith((ref) => 0),
rootProvider.overrideWith((ref) => 0),
],
child: Container(),
),
);

flutter.runApp(
ProviderScope(
parent: rootContainer,
overrides: [
Expand Down

0 comments on commit 65ea983

Please sign in to comment.