applied code review suggestions

Author: Islam-Shaaban-Ibrahim

lib/src/lints/proper_super_calls/proper_super_calls_rule.dart

@@ -44,49 +44,43 @@ import 'package:solid_lints/src/lints/proper_super_calls/visitors/proper_super_c
 /// }
 /// ```
 class ProperSuperCallsRule extends AnalysisRule {
-  /// This lint rule name.
+  /// Lint rule name.
   static const String lintName = 'proper_super_calls';
 
-  /// Error code for when super.initState() is not the first statement.
-  static const _superInitStateCode = LintCode(
+  /// Error code for invalid `super.initState()` placement.
+  static const LintCode superInitStateCode = LintCode(
     lintName,
-    "super.initState() should be first",
+    'super.initState() should be first',
   );
 
-  /// Error code for when super.dispose() is not the last statement.
-  static const _superDisposeCode = LintCode(
+  /// Error code for invalid `super.dispose()` placement.
+  static const LintCode superDisposeCode = LintCode(
     lintName,
-    "super.dispose() should be last",
+    'super.dispose() should be last',
   );
 
-  /// Creates a new instance of [ProperSuperCallsRule].
+  /// Creates an instance of [ProperSuperCallsRule].
   ProperSuperCallsRule()
       : super(
           name: lintName,
           description:
-              'Ensures that `super` calls are made in the correct order ',
+              'Ensures proper ordering of Flutter lifecycle super calls.',
         );
 
   @override
-  LintCode get diagnosticCode => _superInitStateCode;
+  LintCode get diagnosticCode => superInitStateCode;
 
   @override
   void registerNodeProcessors(
     RuleVisitorRegistry registry,
     RuleContext context,
   ) {
-    final visitor = ProperSuperCallsVisitor(
-      onViolation: (nameToken, {required bool isInitState}) {
-        // Access the reporter from the currentUnit
-        final reporter = context.currentUnit?.diagnosticReporter;
-
-        reporter?.atToken(
-          nameToken,
-          isInitState ? _superInitStateCode : _superDisposeCode,
-        );
-      },
+    registry.addMethodDeclaration(
+      this,
+      ProperSuperCallsVisitor(
+        rule: this,
+        context: context,
+      ),
     );
-
-    registry.addMethodDeclaration(this, visitor);
   }
 }

lib/src/lints/proper_super_calls/visitors/proper_super_calls_visitor.dart

@@ -1,65 +1,112 @@
+import 'package:analyzer/analysis_rule/rule_context.dart';
 import 'package:analyzer/dart/ast/ast.dart';
-import 'package:analyzer/dart/ast/token.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 the proper [ProperSuperCallsRule].
+/// Visitor for [ProperSuperCallsRule].
 class ProperSuperCallsVisitor extends SimpleAstVisitor<void> {
-  /// Callback to report violations.
-  final void Function(Token name, {required bool isInitState}) onViolation;
+  /// 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 _override = 'override';
+  static const _flutterStateClass = 'State';
 
   /// Creates a new instance of [ProperSuperCallsVisitor].
-  ProperSuperCallsVisitor({required this.onViolation});
+  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) {
-      final hasOverride = node.metadata.any(
-        (annotation) => annotation.name.name == _override,
+    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,
       );
-      if (!hasOverride) return;
+    }
+  }
 
-      final statements = body.block.statements;
+  bool _overridesFlutterStateMethod(MethodDeclaration node) {
+    final classElement = node.declaredFragment?.element.enclosingElement;
 
-      // Logic for initState: super.initState() must be the very first statement.
-      if (methodName == _initState &&
-          !_isSuperCallFirst(statements, _initState)) {
-        onViolation(node.name, isInitState: true);
-      }
+    if (classElement is! ClassElement) {
+      return false;
+    }
+
+    final methodName = node.name.lexeme;
+
+    final supertype = classElement.supertype;
+
+    if (supertype == null) {
+      return false;
+    }
 
-      // Logic for dispose: super.dispose() must be the very last statement.
-      if (methodName == _dispose && !_isSuperCallLast(statements, _dispose)) {
-        onViolation(node.name, isInitState: 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;
   }
 
-  /// Returns true if the first statement is the expected super call.
   bool _isSuperCallFirst(List<Statement> statements, String name) {
     return statements.isNotEmpty && _isTargetSuperCall(statements.first, name);
   }
 
-  /// Returns true if the last statement is the expected super call.
   bool _isSuperCallLast(List<Statement> statements, String name) {
     return statements.isNotEmpty && _isTargetSuperCall(statements.last, name);
   }
 
-  /// Checks if a statement is a [MethodInvocation] on [SuperExpression].
   bool _isTargetSuperCall(Statement statement, String name) {
-    if (statement is ExpressionStatement) {
-      final expr = statement.expression;
-      return expr is MethodInvocation &&
-          expr.target is SuperExpression &&
-          expr.methodName.name == name;
+    if (statement is! ExpressionStatement) {
+      return false;
     }
-    return false;
+
+    final expression = statement.expression;
+
+    return expression is MethodInvocation &&
+        expression.target is SuperExpression &&
+        expression.methodName.name == name;
   }
 }

test/proper_super_calls_rule_test.dart

@@ -12,59 +12,131 @@ void main() {
 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<T extends StatefulWidget> {
+  void initState() {}
+  void dispose() {}
+}
+''',
+    );
+
     rule = ProperSuperCallsRule();
     super.setUp();
   }
 
   @override
   String get analysisRule => ProperSuperCallsRule.lintName;
 
-  /// Mocks the Flutter framework parts relevant to this lint.
-  String get _flutterBase => r'''
-abstract class StatefulWidget {}
-abstract class State<T extends StatefulWidget> {
-  void initState() {}
-  void dispose() {}
+  void test_initState_reports_when_super_not_first() async {
+    await assertDiagnostics(
+      r'''
+import 'package:flutter/src/widgets/framework.dart';
+
+class MyWidgetState extends State<StatefulWidget> {
+  @override
+  void initState() {
+    print('Bad');
+    super.initState();
+  }
 }
-''';
+''',
+      [lint(125, 9)],
+    );
+  }
 
-  void test_initState_reports_when_not_first() async {
+  void test_dispose_reports_when_super_not_last() async {
     await assertDiagnostics(
-        _flutterBase +
-            r'''
+      r'''
+import 'package:flutter/src/widgets/framework.dart';
+
 class MyWidgetState extends State<StatefulWidget> {
   @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<StatefulWidget> {
   void initState() {
     print('Bad');
     super.initState();
   }
 }
 ''',
-        [
-          lint(197, 9), // Highlights '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<StatefulWidget> {
+  @override
+  void initState() {}
+}
+''',
+      [lint(125, 9)],
+    );
   }
 
-  void test_dispose_reports_when_not_last() async {
+  void test_reports_when_wrong_super_method_is_called() async {
     await assertDiagnostics(
-        _flutterBase +
-            r'''
+      r'''
+import 'package:flutter/src/widgets/framework.dart';
+
 class MyWidgetState extends State<StatefulWidget> {
+  @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<StatefulWidget> {}
+
+class MyWidgetState extends BaseState {
   @override
   void dispose() {
     super.dispose();
     print('Bad');
   }
 }
 ''',
-        [
-          lint(197, 7), // Highlights 'dispose'
-        ]);
+      [lint(172, 7)],
+    );
   }
 
-  void test_no_report_for_correct_flutter_usage() async {
-    await assertNoDiagnostics(_flutterBase +
-        r'''
+  void test_no_report_for_correct_usage() async {
+    await assertNoDiagnostics(
+      r'''
+import 'package:flutter/src/widgets/framework.dart';
+
 class MyWidgetState extends State<StatefulWidget> {
   @override
   void initState() {
@@ -78,6 +150,21 @@ class MyWidgetState extends State<StatefulWidget> {
     super.dispose();
   }
 }
-''');
+''',
+    );
+  }
+
+  void test_no_report_for_other_methods() async {
+    await assertNoDiagnostics(
+      r'''
+import 'package:flutter/src/widgets/framework.dart';
+
+class MyWidgetState extends State<StatefulWidget> {
+  void build() {
+    super.initState();   
+  }
+}
+''',
+    );
   }
 }