diff --git a/lib/main.dart b/lib/main.dart index 78ee2a4c..5657b7a1 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -3,6 +3,7 @@ import 'package:analysis_server_plugin/registry.dart'; import 'package:solid_lints/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart'; import 'package:solid_lints/src/lints/avoid_global_state/avoid_global_state_rule.dart'; import 'package:solid_lints/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart'; +import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; /// The entry point for the Solid Lints analyser server plugin. /// @@ -26,5 +27,11 @@ class SolidLintsPlugin extends Plugin { registry.registerLintRule( AvoidNonNullAssertionRule(), ); + registry.registerLintRule( + AvoidDebugPrintInReleaseRule(), + ); + registry.registerLintRule( + ProperSuperCallsRule(), + ); } } diff --git a/lib/src/lints/proper_super_calls/proper_super_calls_rule.dart b/lib/src/lints/proper_super_calls/proper_super_calls_rule.dart index 34df0dbc..fb82e8e8 100644 --- a/lib/src/lints/proper_super_calls/proper_super_calls_rule.dart +++ b/lib/src/lints/proper_super_calls/proper_super_calls_rule.dart @@ -1,8 +1,8 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:solid_lints/src/lints/proper_super_calls/visitors/proper_super_calls_visitor.dart'; /// Ensures that `super` calls are made in the correct order for the following /// StatefulWidget methods: @@ -43,130 +43,44 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// super.dispose(); // OK /// } /// ``` -class ProperSuperCallsRule extends SolidLintRule { - /// This lint rule represents - /// the error whether the initState and dispose methods - /// are called in the incorrect order - static const lintName = 'proper_super_calls'; - static const _initState = 'initState'; - static const _dispose = 'dispose'; - static const _override = 'override'; - - /// This lint rule represents - /// the error whether super.initState() should be called first - static const _superInitStateCode = LintCode( - name: lintName, - problemMessage: "super.initState() should be first", +class ProperSuperCallsRule extends AnalysisRule { + /// Lint rule name. + static const String lintName = 'proper_super_calls'; + + /// Error code for invalid `super.initState()` placement. + static const LintCode superInitStateCode = LintCode( + lintName, + 'super.initState() should be first', ); - /// This lint rule represents - /// the error whether super.dispose() should be called last - static const _superDisposeCode = LintCode( - name: lintName, - problemMessage: "super.dispose() should be last", + /// Error code for invalid `super.dispose()` placement. + static const LintCode superDisposeCode = LintCode( + lintName, + 'super.dispose() should be last', ); - ProperSuperCallsRule._(super.config); - - /// Creates a new instance of [ProperSuperCallsRule] - /// based on the lint configuration. - factory ProperSuperCallsRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - name: lintName, - configs: configs, - problemMessage: (_) => 'Proper super calls issue', - ); + /// Creates an instance of [ProperSuperCallsRule]. + ProperSuperCallsRule() + : super( + name: lintName, + description: + 'Ensures proper ordering of Flutter lifecycle super calls.', + ); - return ProperSuperCallsRule._(rule); - } + @override + LintCode get diagnosticCode => superInitStateCode; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addMethodDeclaration( - (node) { - final methodName = node.name.toString(); - final body = node.body; - - if (methodName case _initState || _dispose - when body is BlockFunctionBody) { - final statements = body.block.statements; - - _checkSuperCalls( - node, - methodName, - statements, - reporter, - ); - } - }, + registry.addMethodDeclaration( + this, + ProperSuperCallsVisitor( + rule: this, + context: context, + ), ); } - - /// This method report an error whether `super.initState()` - /// or `super.dispose()` are called incorrect - void _checkSuperCalls( - MethodDeclaration node, - String methodName, - List statements, - DiagnosticReporter reporter, - ) { - final hasOverrideAnnotation = - node.metadata.any((annotation) => annotation.name.name == _override); - - if (!hasOverrideAnnotation) return; - if (methodName == _initState && !_isSuperInitStateCalledFirst(statements)) { - reporter.atNode( - node, - _superInitStateCode, - ); - } - if (methodName == _dispose && !_isSuperDisposeCalledLast(statements)) { - reporter.atNode( - node, - _superDisposeCode, - ); - } - } - - /// Returns `true` if `super.initState()` is called before other code in the - /// `initState` method, `false` otherwise. - bool _isSuperInitStateCalledFirst(List statements) { - if (statements.isEmpty) return false; - final firstStatement = statements.first; - - if (firstStatement is ExpressionStatement) { - final expression = firstStatement.expression; - - final isSuperInitStateCalledFirst = expression is MethodInvocation && - expression.target is SuperExpression && - expression.methodName.toString() == _initState; - - return isSuperInitStateCalledFirst; - } - - return false; - } - - /// Returns `true` if `super.dispose()` is called at the end of the `dispose` - /// method, `false` otherwise. - bool _isSuperDisposeCalledLast(List statements) { - if (statements.isEmpty) return false; - final lastStatement = statements.last; - - if (lastStatement is ExpressionStatement) { - final expression = lastStatement.expression; - - final lastStatementIsSuperDispose = expression is MethodInvocation && - expression.target is SuperExpression && - expression.methodName.toString() == _dispose; - - return lastStatementIsSuperDispose; - } - - return false; - } } diff --git a/lib/src/lints/proper_super_calls/visitors/proper_super_calls_visitor.dart b/lib/src/lints/proper_super_calls/visitors/proper_super_calls_visitor.dart new file mode 100644 index 00000000..6cbe4759 --- /dev/null +++ b/lib/src/lints/proper_super_calls/visitors/proper_super_calls_visitor.dart @@ -0,0 +1,112 @@ +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; + +/// Visitor for [ProperSuperCallsRule]. +class ProperSuperCallsVisitor extends SimpleAstVisitor { + /// The rule associated with this visitor. + final ProperSuperCallsRule rule; + + /// The context associated with this visitor. + final RuleContext context; + + static const _initState = 'initState'; + static const _dispose = 'dispose'; + static const _flutterStateClass = 'State'; + + /// Creates a new instance of [ProperSuperCallsVisitor]. + ProperSuperCallsVisitor({ + required this.rule, + required this.context, + }); + + @override + void visitMethodDeclaration(MethodDeclaration node) { + final methodName = node.name.lexeme; + final body = node.body; + + if ((methodName != _initState && methodName != _dispose) || + body is! BlockFunctionBody) { + return; + } + + if (!_overridesFlutterStateMethod(node)) { + return; + } + + final statements = body.block.statements; + final reporter = context.currentUnit?.diagnosticReporter; + + if (reporter == null) return; + + if (methodName == _initState && + !_isSuperCallFirst(statements, _initState)) { + reporter.atToken( + node.name, + ProperSuperCallsRule.superInitStateCode, + ); + } + + if (methodName == _dispose && !_isSuperCallLast(statements, _dispose)) { + reporter.atToken( + node.name, + ProperSuperCallsRule.superDisposeCode, + ); + } + } + + bool _overridesFlutterStateMethod(MethodDeclaration node) { + final classElement = node.declaredFragment?.element.enclosingElement; + + if (classElement is! ClassElement) { + return false; + } + + final methodName = node.name.lexeme; + + final supertype = classElement.supertype; + + if (supertype == null) { + return false; + } + + final isStateSubclass = _isStateSubclass(supertype); + + if (!isStateSubclass) { + return false; + } + + return methodName == _initState || methodName == _dispose; + } + + bool _isStateSubclass(InterfaceType supertype) { + final isStateSubclass = supertype.element.name == _flutterStateClass || + supertype.allSupertypes.any( + (t) => t.element.name == _flutterStateClass, + ); + return isStateSubclass; + } + + bool _isSuperCallFirst(List statements, String name) { + return statements.isNotEmpty && _isTargetSuperCall(statements.first, name); + } + + bool _isSuperCallLast(List statements, String name) { + return statements.isNotEmpty && _isTargetSuperCall(statements.last, name); + } + + bool _isTargetSuperCall(Statement statement, String name) { + if (statement is! ExpressionStatement) { + return false; + } + + final expression = statement.expression; + + return expression is MethodInvocation && + expression.target is SuperExpression && + expression.methodName.name == name; + } +} diff --git a/lint_test/proper_super_calls_test.dart b/lint_test/proper_super_calls_test.dart deleted file mode 100644 index fc6e4773..00000000 --- a/lint_test/proper_super_calls_test.dart +++ /dev/null @@ -1,82 +0,0 @@ -// ignore_for_file: prefer_match_file_name, no_empty_block - -class Widget {} - -abstract class State { - build(); - dispose() {} - initState() {} -} - -abstract class StatefulWidget extends Widget { - State createState(); - - StatefulWidget(); -} - -/// Check "check super" keyword fail -/// -/// `proper_super_calls` -class ProperSuperCallsTest1 extends StatefulWidget { - @override - State createState() => _ProperSuperCallsTest1State(); - - ProperSuperCallsTest1(); -} - -class _ProperSuperCallsTest1State extends State { - @override - Widget build() { - return Widget(); - } - - // expect_lint: proper_super_calls - @override - void initState() { - print(''); - super.initState(); - } - - // expect_lint: proper_super_calls - @override - void dispose() { - super.dispose(); - print(''); - } -} - -class ProperSuperCallsTest2 extends StatefulWidget { - @override - State createState() => _ProperSuperCallsTest2State(); - - ProperSuperCallsTest2(); -} - -class _ProperSuperCallsTest2State extends State { - @override - Widget build() { - return Widget(); - } - - @override - void initState() { - super.initState(); - print(''); - } - - @override - void dispose() { - print(''); - super.dispose(); - } -} - -class MyClass { - dispose() {} - initState() {} -} - -abstract interface class Disposable { - /// Abstract methods should be omitted by `proper_super_calls` - void dispose(); -} diff --git a/pubspec.yaml b/pubspec.yaml index a7c16536..f85b1309 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -26,3 +26,4 @@ dev_dependencies: args: ^2.6.0 analyzer_testing: ^0.1.9 test_reflective_loader: ^0.3.0 + diff --git a/test/avoid_debug_print_in_release_rule_test.dart b/test/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule_test.dart similarity index 100% rename from test/avoid_debug_print_in_release_rule_test.dart rename to test/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule_test.dart diff --git a/test/avoid_global_state_rule_test.dart b/test/lints/avoid_global_state/avoid_global_state_rule_test.dart similarity index 100% rename from test/avoid_global_state_rule_test.dart rename to test/lints/avoid_global_state/avoid_global_state_rule_test.dart diff --git a/test/avoid_non_null_assertion_rule_test.dart b/test/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule_test.dart similarity index 100% rename from test/avoid_non_null_assertion_rule_test.dart rename to test/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule_test.dart diff --git a/test/lints/proper_super_calls/proper_super_calls_rule_test.dart b/test/lints/proper_super_calls/proper_super_calls_rule_test.dart new file mode 100644 index 00000000..8969e73c --- /dev/null +++ b/test/lints/proper_super_calls/proper_super_calls_rule_test.dart @@ -0,0 +1,170 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(ProperSuperCallsRuleTest); + }); +} + +@reflectiveTest +class ProperSuperCallsRuleTest extends AnalysisRuleTest { + @override + void setUp() { + final flutter = newPackage('flutter'); + flutter.addFile( + 'lib/src/widgets/framework.dart', + r''' +abstract class StatefulWidget {} + +abstract class State { + void initState() {} + void dispose() {} +} +''', + ); + + rule = ProperSuperCallsRule(); + super.setUp(); + } + + @override + String get analysisRule => ProperSuperCallsRule.lintName; + + void test_initState_reports_when_super_not_first() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + @override + void initState() { + print('Bad'); + super.initState(); + } +} +''', + [lint(125, 9)], + ); + } + + void test_dispose_reports_when_super_not_last() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + @override + void dispose() { + super.dispose(); + print('Bad'); + } +} +''', + [lint(125, 7)], + ); + } + + void test_reports_even_without_explicit_override_annotation() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + void initState() { + print('Bad'); + super.initState(); + } +} +''', + [lint(113, 9)], + ); + } + + void test_reports_empty_body_missing_super() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + @override + void initState() {} +} +''', + [lint(125, 9)], + ); + } + + void test_reports_when_wrong_super_method_is_called() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + @override + void initState() { + super.dispose(); + print('Bad'); + } +} +''', + [lint(125, 9)], + ); + } + + void test_reports_in_deep_inheritance() async { + await assertDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +abstract class BaseState extends State {} + +class MyWidgetState extends BaseState { + @override + void dispose() { + super.dispose(); + print('Bad'); + } +} +''', + [lint(172, 7)], + ); + } + + void test_no_report_for_correct_usage() async { + await assertNoDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + @override + void initState() { + super.initState(); + print('Good'); + } + + @override + void dispose() { + print('Good'); + super.dispose(); + } +} +''', + ); + } + + void test_no_report_for_other_methods() async { + await assertNoDiagnostics( + r''' +import 'package:flutter/src/widgets/framework.dart'; + +class MyWidgetState extends State { + void build() { + super.initState(); + } +} +''', + ); + } +}