Skip to content

Commit 624ffd9

Browse files
committed
fix(#2): Denying the definition of hooks within a class.
1 parent 9a779b6 commit 624ffd9

File tree

13 files changed

+153
-35
lines changed

13 files changed

+153
-35
lines changed

packages/flutter_hooks_lint/lib/flutter_hooks_lint.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ library;
55

66
import 'package:custom_lint_builder/custom_lint_builder.dart';
77
import 'package:flutter_hooks_lint/src/rules/hooks_avoid_nesting_rule.dart';
8+
import 'package:flutter_hooks_lint/src/rules/hooks_avoid_within_class_rule.dart';
89
import 'package:flutter_hooks_lint/src/rules/hooks_callback_consideration_rule.dart';
910
import 'package:flutter_hooks_lint/src/rules/hooks_extends_rule.dart';
1011
import 'package:flutter_hooks_lint/src/rules/hooks_memoized_consideration_rule.dart';
@@ -20,6 +21,7 @@ class _FlutterHooksLinter extends PluginBase {
2021
const HooksExtendsRule(),
2122
const HooksUnuseWidgetRule(),
2223
const HooksAvoidNestingRule(),
24+
const HooksAvoidWithinClassRule(),
2325
const HooksNameConventionRule(),
2426
const HooksMemoizedConsiderationRule(),
2527
const HooksCallbackConsiderationRule(),
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import 'package:analyzer/dart/ast/ast.dart';
2+
import 'package:analyzer/dart/ast/visitor.dart';
3+
import 'package:analyzer/error/error.dart';
4+
import 'package:analyzer/error/listener.dart';
5+
import 'package:custom_lint_builder/custom_lint_builder.dart';
6+
import 'package:flutter_hooks_lint/src/visitors/hooks_method_visitor.dart';
7+
8+
class HooksAvoidWithinClassRule extends DartLintRule {
9+
const HooksAvoidWithinClassRule() : super(code: _code);
10+
11+
static const _code = LintCode(
12+
name: 'hooks_avoid_within_class',
13+
problemMessage: 'Hooks must not be defined within the class',
14+
errorSeverity: ErrorSeverity.WARNING,
15+
);
16+
17+
@override
18+
void run(
19+
CustomLintResolver resolver,
20+
ErrorReporter reporter,
21+
CustomLintContext context,
22+
) {
23+
context.registry.addClassDeclaration((declaration) {
24+
final collector = _MethodCollector();
25+
declaration.visitChildren(collector);
26+
27+
for (final methodDeclaration in collector.methods) {
28+
// TODO(nikaera): should only detect when the build function name is inherited from the Widget class.
29+
if ('build' == methodDeclaration.name.lexeme) continue;
30+
31+
methodDeclaration.visitChildren(
32+
HooksMethodVisitor(
33+
onVisitMethodInvocation: (node) {
34+
reporter.reportErrorForNode(code, methodDeclaration);
35+
},
36+
),
37+
);
38+
}
39+
});
40+
}
41+
}
42+
43+
class _MethodCollector extends GeneralizingAstVisitor<void> {
44+
final List<MethodDeclaration> methods = [];
45+
46+
@override
47+
void visitMethodDeclaration(MethodDeclaration node) {
48+
methods.add(node);
49+
super.visitMethodDeclaration(node);
50+
}
51+
}

packages/flutter_hooks_lint/lib/src/rules/hooks_name_convention_rule.dart

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
1+
import 'package:analyzer/dart/ast/ast.dart';
12
import 'package:analyzer/error/error.dart';
23
import 'package:analyzer/error/listener.dart';
34
import 'package:custom_lint_builder/custom_lint_builder.dart';
5+
import 'package:flutter_hooks_lint/src/helpers/hooks_helper.dart';
46
import 'package:flutter_hooks_lint/src/visitors/hooks_method_visitor.dart';
57

8+
final _regexp = RegExp('^use[A-Z]{1}');
9+
610
class HooksNameConventionRule extends DartLintRule {
711
const HooksNameConventionRule() : super(code: _code);
812

@@ -19,13 +23,24 @@ class HooksNameConventionRule extends DartLintRule {
1923
ErrorReporter reporter,
2024
CustomLintContext context,
2125
) {
22-
context.registry.addMethodDeclaration((classNode) {
23-
final regexp = RegExp('^use[A-Z]{1}');
26+
context.registry.addMethodDeclaration((node) {
27+
final classDeclaration = node.thisOrAncestorOfType<ClassDeclaration>();
28+
final extendsClause = classDeclaration?.extendsClause;
29+
if (extendsClause == null) {
30+
return;
31+
}
32+
final extendsElement = extendsClause.superclass.element;
33+
if (extendsElement == null) {
34+
return;
35+
}
36+
if (!HooksHelper.isHooksElement(extendsElement)) {
37+
return;
38+
}
2439

25-
classNode.visitChildren(
40+
node.visitChildren(
2641
HooksMethodVisitor(
2742
onVisitMethodInvocation: (node) {
28-
if (!regexp.hasMatch(node.methodName.name)) {
43+
if (!_regexp.hasMatch(node.methodName.name)) {
2944
reporter.reportErrorForNode(code, node);
3045
}
3146
},

packages/flutter_hooks_lint/lib/src/visitors/hooks_method_visitor.dart

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,56 +2,69 @@ import 'package:analyzer/dart/analysis/analysis_context_collection.dart';
22
import 'package:analyzer/dart/analysis/results.dart';
33
import 'package:analyzer/dart/ast/ast.dart';
44
import 'package:analyzer/dart/ast/visitor.dart';
5-
import 'package:analyzer/dart/element/element.dart';
65
import 'package:analyzer/file_system/physical_file_system.dart';
76
import 'package:flutter_hooks_lint/src/helpers/hooks_helper.dart';
87

8+
final _regexp = RegExp('^use[A-Z]{1}');
9+
910
class HooksMethodVisitor extends RecursiveAstVisitor<void> {
1011
const HooksMethodVisitor({
1112
required this.onVisitMethodInvocation,
13+
this.caller,
1214
});
15+
1316
final void Function(MethodInvocation node) onVisitMethodInvocation;
17+
final MethodInvocation? caller;
1418

19+
/*
20+
TODO(nikaera): I would like to recursively verify whether a function accurately utilizes
21+
the hooks package using getResolvedUnit. However, the execution time becomes
22+
excessively long. Hence, I am simplifying the process by examining if the
23+
function, which I aim to explore using getParsedUnit, employs the hooks
24+
package from the AST of a single file where the function is defined.
25+
*/
1526
@override
1627
void visitMethodInvocation(MethodInvocation node) {
1728
final element = node.methodName.staticElement;
1829
if (element == null) {
19-
return;
30+
if (caller != null && _regexp.hasMatch(node.methodName.name)) {
31+
onVisitMethodInvocation(caller!);
32+
}
33+
return super.visitMethodInvocation(node);
2034
}
2135

2236
final methodName = node.methodName.name;
37+
2338
final isDartCoreMethod = element.library?.isDartCore ?? false;
2439
final isInSdkMethod = element.library?.isInSdk ?? false;
2540

2641
if (HooksHelper.isHooksElement(element)) {
2742
onVisitMethodInvocation(node);
2843
} else if (!isDartCoreMethod && !isInSdkMethod) {
29-
try {
30-
final filePath = element.librarySource?.fullName;
31-
if (filePath != null) {
32-
final collection = AnalysisContextCollection(
33-
includedPaths: [filePath],
34-
resourceProvider: PhysicalResourceProvider.INSTANCE,
35-
);
44+
final filePath = element.librarySource?.fullName;
45+
if (filePath != null) {
46+
final collection = AnalysisContextCollection(
47+
includedPaths: [filePath],
48+
resourceProvider: PhysicalResourceProvider.INSTANCE,
49+
);
3650

37-
final context = collection.contextFor(filePath);
38-
final result = context.currentSession.getParsedUnit(filePath);
51+
final context = collection.contextFor(filePath);
52+
final result = context.currentSession.getParsedUnit(filePath);
53+
result as ParsedUnitResult;
3954

40-
if (result is ParsedUnitResult) {
41-
final AstNode rootNode = result.unit;
42-
rootNode.visitChildren(
43-
_SpecificFunctionDeclarationVisitor(
44-
functionName: methodName,
45-
onVisitFunctionDeclaration: (node) {
46-
node.visitChildren(this);
47-
},
48-
),
49-
);
50-
}
51-
onVisitMethodInvocation(node);
52-
}
53-
} catch (e) {
54-
print('$methodName: $e');
55+
result.unit.visitChildren(
56+
_SpecificFunctionDeclarationVisitor(
57+
functionName: methodName,
58+
onVisitFunctionDeclaration: (childNode) {
59+
childNode.visitChildren(
60+
HooksMethodVisitor(
61+
caller: caller ?? node,
62+
onVisitMethodInvocation: onVisitMethodInvocation,
63+
),
64+
);
65+
},
66+
),
67+
);
5568
}
5669
}
5770

packages/flutter_hooks_lint_flutter_test/test/golden.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import 'package:analyzer/dart/analysis/results.dart';
99
import 'package:analyzer/dart/analysis/utilities.dart';
1010

1111
@Deprecated('Do not commit')
12-
var goldenWrite = false;
12+
final goldenWrite = false;
1313

1414
File writeToTemporaryFile(String content) {
1515
final tempDir = Directory.systemTemp.createTempSync();
@@ -31,7 +31,6 @@ void testGolden(
3131
}) {
3232
test(description, () async {
3333
final file = File(sourcePath).absolute;
34-
3534
final result = await resolveFile2(path: file.path);
3635
result as ResolvedUnitResult;
3736

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import 'dart:io';
2+
3+
import 'package:flutter_hooks_lint/src/rules/hooks_avoid_within_class_rule.dart';
4+
import 'package:test/test.dart';
5+
6+
void main() {
7+
test('Check nested using of flutter_hooks methods', () async {
8+
final file =
9+
File('test/hooks_avoid_within_class/hooks_avoid_within_class.dart')
10+
.absolute;
11+
final errors = await HooksAvoidWithinClassRule().testAnalyzeAndRun(file);
12+
13+
expect(errors, hasLength(1));
14+
}, retry: 0);
15+
}

packages/flutter_hooks_lint_flutter_test/test/hooks_extends/hooks_extends.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,13 @@ class RequiresConsumerHookWidget extends ConsumerWidget {
1717
return Text(state.value.toString());
1818
}
1919
}
20+
21+
final _textProvider = Provider((ref) => "");
22+
23+
class RiverpodWidget extends ConsumerWidget {
24+
@override
25+
Widget build(BuildContext context, WidgetRef ref) {
26+
final text = ref.watch(_textProvider);
27+
return Text(text);
28+
}
29+
}

packages/flutter_hooks_lint_flutter_test/test/hooks_name_convention/hooks_name_convention.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import "package:flutter/material.dart";
22
import "package:flutter_hooks/flutter_hooks.dart";
33

4-
import "custom_hooks.dart";
4+
import "src/custom_hooks.dart";
55

66
class CorrectMethodWidget extends HookWidget {
77
@override

packages/flutter_hooks_lint_flutter_test/test/hooks_unuse_widget/hooks_unuse_widget.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import 'package:flutter/material.dart';
22
import 'package:flutter_hooks/flutter_hooks.dart';
33
import 'package:hooks_riverpod/hooks_riverpod.dart';
4-
import 'custom_hooks.dart';
4+
import 'src/custom_hooks.dart';
55

66
class UnuseHookWidget extends HookWidget {
77
@override

packages/flutter_hooks_lint_flutter_test/test/hooks_unuse_widget/hooks_unuse_widget.diff

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ Message: `Convert to StatelessWidget`
22
Priority: 30
33
Diff for file `test/hooks_unuse_widget/hooks_unuse_widget.dart:6`:
44
```
5-
import 'custom_hooks.dart';
5+
import 'src/custom_hooks.dart';
66

77
- class UnuseHookWidget extends HookWidget {
88
+ class UnuseHookWidget extends StatelessWidget {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import 'package:flutter_hooks/flutter_hooks.dart';
2+
3+
void useEffectOnce(Dispose? Function() effect) {
4+
useEffect(effect, const []);
5+
}
6+
7+
class TestHelper {
8+
const TestHelper._();
9+
10+
static void useEffectOnce(Dispose? Function() effect) {
11+
useEffect(effect, const []);
12+
}
13+
}

0 commit comments

Comments
 (0)