Migrate avoid unnecessary return variable rule and its tests

lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart

@@ -1,11 +1,8 @@
-import 'package:analyzer/dart/ast/ast.dart';
-import 'package:analyzer/dart/element/element.dart';
-import 'package:analyzer/error/listener.dart';
-import 'package:custom_lint_builder/custom_lint_builder.dart';
-import 'package:solid_lints/src/common/visitors/select_expression_identifiers_visitor.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/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart';
-import 'package:solid_lints/src/models/rule_config.dart';
-import 'package:solid_lints/src/models/solid_lint_rule.dart';
 
 /// An `avoid_unnecessary_return_variable` rule which forbids returning
 /// an immutable variable if it can be rewritten in return statement itself.
@@ -32,109 +29,37 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart';
 /// }
 /// ```
 ///
-class AvoidUnnecessaryReturnVariableRule extends SolidLintRule {
-  /// This lint rule represents the error
-  /// when unnecessary return variable statement is found
+class AvoidUnnecessaryReturnVariableRule extends AnalysisRule {
+  /// The name of the lint rule.
   static const lintName = 'avoid_unnecessary_return_variable';
 
-  AvoidUnnecessaryReturnVariableRule._(super.config);
-
-  /// Creates a new instance of [AvoidUnnecessaryReturnVariableRule]
-  /// based on the lint configuration.
-  factory AvoidUnnecessaryReturnVariableRule.createRule(
-    CustomLintConfigs configs,
-  ) {
-    final rule = RuleConfig(
-      configs: configs,
-      name: lintName,
-      problemMessage: (_) => """
+  /// The message shown when the lint is triggered.
+  static const String lintMessage = """
 Avoid creating unnecessary variable only for return.
-Rewrite the variable evaluation into return statement instead.""",
-    );
+Rewrite the variable evaluation into return statement instead.""";
 
-    return AvoidUnnecessaryReturnVariableRule._(rule);
-  }
+  /// Lint code.
+  static const LintCode _code = LintCode(
+    lintName,
+    lintMessage,
+  );
 
-  @override
-  void run(
-    CustomLintResolver resolver,
-    DiagnosticReporter reporter,
-    CustomLintContext context,
-  ) {
-    context.registry.addReturnStatement(
-      (statement) {
-        _checkReturnStatement(
-          statement: statement,
-          reporter: reporter,
-          context: context,
+  /// Creates a new instance of [AvoidUnnecessaryReturnVariableRule]
+  AvoidUnnecessaryReturnVariableRule()
+      : super(
+          name: lintName,
+          description: lintMessage,
         );
-      },
-    );
-  }
-
-  void _checkReturnStatement({
-    required ReturnStatement statement,
-    required DiagnosticReporter reporter,
-    required CustomLintContext context,
-  }) {
-    final expr = statement.expression;
-    if (expr is! SimpleIdentifier) return;
-
-    final element = expr.element;
-    if (element is! LocalVariableElement) return;
-    final returnVariableElement = element;
-
-    if (!returnVariableElement.isFinal && !returnVariableElement.isConst) {
-      return;
-    }
-
-    final blockBody = statement.parent;
-    if (blockBody == null) return;
-
-    final visitor = AvoidUnnecessaryReturnVariableVisitor(
-      returnVariableElement,
-      statement,
-    );
-    blockBody.visitChildren(visitor);
-
-    if (!visitor.hasBadStatementCount()) return;
 
-    //it is 100% bad if return statement follows declaration
-    if (!visitor.foundTokensBetweenDeclarationAndReturn) {
-      reporter.atNode(statement, code);
-      return;
-    }
-
-    final declaration = visitor.variableDeclaration;
-
-    //check if immutable
-    final initializer = declaration?.initializer;
-    if (initializer == null) return;
-
-    if (!_isExpressionImmutable(initializer)) return;
-
-    reporter.atNode(statement, code);
-  }
-
-  bool _isExpressionImmutable(Expression expr) {
-    final visitor = SelectExpressionIdentifiersVisitor();
-    expr.accept(visitor);
-
-    return visitor.identifiers.every(_isSimpleIdentifierImmutable);
-  }
-
-  bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) {
-    switch (identifier.element) {
-      case final VariableElement variable:
-        return variable.isFinal || variable.isConst;
-
-      case ClassElement _:
-        return true;
-
-      case GetterElement(:final PropertyInducingElement variable):
-        return variable.isFinal || variable.isConst;
-    }
+  @override
+  LintCode get diagnosticCode => _code;
 
-    return false;
+  @override
+  void registerNodeProcessors(
+    RuleVisitorRegistry registry,
+    RuleContext context,
+  ) {
+    final visitor = AvoidUnnecessaryReturnVariableVisitor(this);
+    registry.addReturnStatement(this, visitor);
   }
 }

lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart

@@ -1,79 +1,79 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element.dart';
-import 'package:collection/collection.dart';
+import 'package:solid_lints/src/common/visitors/select_expression_identifiers_visitor.dart';
+import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart';
+import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart';
 
-/// This visitor is searches all uses of a single local variable,
-/// including variable declaration.
-class AvoidUnnecessaryReturnVariableVisitor extends RecursiveAstVisitor<void> {
-  /// The problem expects that exactly 1 mention of return variable.
-  /// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier.
-  /// Any other amount of variable mentions implies that it is used somewhere
-  /// except return, so its existence is justified.
-  static const _badStatementCount = 1;
+/// Visitor for [AvoidUnnecessaryReturnVariableRule].
+class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor<void> {
+  /// The rule associated with this visitor.
+  final AvoidUnnecessaryReturnVariableRule rule;
 
-  final LocalVariableElement _returnVariableElement;
+  /// Creates a new instance of [AvoidUnnecessaryReturnVariableVisitor].
+  AvoidUnnecessaryReturnVariableVisitor(this.rule);
 
-  bool _foundTokensBetweenDeclarationAndReturn = false;
-  VariableDeclaration? _variableDeclaration;
-  int _variableStatementCounter = 0;
+  @override
+  void visitReturnStatement(ReturnStatement node) {
+    final expr = node.expression;
+    if (expr is! SimpleIdentifier) {
+      return;
+    }
 
-  final ReturnStatement _returnStatement;
+    final element = expr.element;
+    if (element is! LocalVariableElement) {
+      return;
+    }
 
-  /// After visiting holds info about whether there are any tokens
-  /// between variable declaration and return statement
-  bool get foundTokensBetweenDeclarationAndReturn =>
-      _foundTokensBetweenDeclarationAndReturn;
+    if (!element.isFinal && !element.isConst) {
+      return;
+    }
 
-  /// Returns statement of local variable declaration
-  VariableDeclaration? get variableDeclaration => _variableDeclaration;
+    final block = node.parent;
+    if (block == null) return;
 
-  /// Creates a new instance of [AvoidUnnecessaryReturnVariableVisitor].
-  AvoidUnnecessaryReturnVariableVisitor(
-    this._returnVariableElement,
-    this._returnStatement,
-  );
+    final returnVariableUsageVisitor =
+        ReturnVariableUsageVisitor(node, element);
 
-  /// Defines whether the variables is used in return statement only.
-  bool hasBadStatementCount() =>
-      _variableStatementCounter == _badStatementCount;
+    block.visitChildren(returnVariableUsageVisitor);
+    if (!returnVariableUsageVisitor.hasBadStatementCount()) return;
 
-  @override
-  void visitVariableDeclarationStatement(VariableDeclarationStatement node) {
-    if (_collectVariableDeclaration(node)) {
-      _checkTokensInBetween(node, _returnStatement);
+    //it is 100% bad if return statement follows declaration
+    if (!returnVariableUsageVisitor.foundTokensBetweenDeclarationAndReturn) {
+      rule.reportAtNode(node);
+      return;
     }
-    super.visitVariableDeclarationStatement(node);
-  }
 
-  @override
-  void visitSimpleIdentifier(SimpleIdentifier node) {
-    if (node.element?.id == _returnVariableElement.id) {
-      _variableStatementCounter++;
-    }
+    final declaration = returnVariableUsageVisitor.variableDeclaration;
+
+    //check if immutable
+    final initializer = declaration?.initializer;
+    if (initializer == null) return;
 
-    super.visitSimpleIdentifier(node);
+    if (!_isExpressionImmutable(initializer)) return;
+
+    rule.reportAtNode(node);
   }
 
-  bool _collectVariableDeclaration(VariableDeclarationStatement node) {
-    final targetVariable = node.variables.variables.firstWhereOrNull(
-      (v) => v.declaredFragment?.element.id == _returnVariableElement.id,
-    );
-    if (targetVariable == null) return false;
+  bool _isExpressionImmutable(Expression expr) {
+    final visitor = SelectExpressionIdentifiersVisitor();
+    expr.accept(visitor);
 
-    _variableDeclaration = targetVariable;
-    return true;
+    return visitor.identifiers.every(_isSimpleIdentifierImmutable);
   }
 
-  void _checkTokensInBetween(
-    VariableDeclarationStatement variableDeclaration,
-    ReturnStatement returnStatement,
-  ) {
-    final tokenBeforeReturn =
-        _returnStatement.findPrevious(_returnStatement.beginToken);
+  bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) {
+    switch (identifier.element) {
+      case final VariableElement variable:
+        return variable.isFinal || variable.isConst;
+
+      case ClassElement _:
+        return true;
 
-    if (tokenBeforeReturn != variableDeclaration.endToken) {
-      _foundTokensBetweenDeclarationAndReturn = true;
+      case GetterElement(:final PropertyInducingElement variable):
+        return variable.isFinal || variable.isConst;
     }
+
+    return false;
   }
 }

lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart

@@ -0,0 +1,76 @@
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:collection/collection.dart';
+import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart';
+
+/// Visitor for [AvoidUnnecessaryReturnVariableRule] which checks whether the
+/// return variable is used somewhere except return statement and
+/// whether it is immutable.
+class ReturnVariableUsageVisitor extends RecursiveAstVisitor<void> {
+  final ReturnStatement _returnStatement;
+  final LocalVariableElement _returnVariableElement;
+
+  /// After visiting holds the declaration of return variable
+  VariableDeclaration? variableDeclaration;
+
+  /// After visiting holds info about whether there are any tokens
+  bool foundTokensBetweenDeclarationAndReturn = false;
+
+  /// The problem expects that exactly 1 mention of return variable.
+  /// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier.
+  /// Any other amount of variable mentions implies that it is used somewhere
+  /// except return, so its existence is justified.
+  static const _badStatementCount = 1;
+
+  int _variableStatementCounter = 0;
+
+  /// Defines whether the variables is used in return statement only.
+  bool hasBadStatementCount() =>
+      _variableStatementCounter == _badStatementCount;
+
+  /// Creates a new instance of [ReturnVariableUsageVisitor].
+  ReturnVariableUsageVisitor(
+    this._returnStatement,
+    this._returnVariableElement,
+  );
+
+  @override
+  void visitVariableDeclarationStatement(VariableDeclarationStatement node) {
+    if (_collectVariableDeclaration(node)) {
+      _checkTokensInBetween(node, _returnStatement);
+    }
+    super.visitVariableDeclarationStatement(node);
+  }
+
+  @override
+  void visitSimpleIdentifier(SimpleIdentifier node) {
+    if (node.element?.id == _returnVariableElement.id) {
+      _variableStatementCounter++;
+    }
+
+    super.visitSimpleIdentifier(node);
+  }
+
+  bool _collectVariableDeclaration(VariableDeclarationStatement node) {
+    final targetVariable = node.variables.variables.firstWhereOrNull(
+      (v) => v.declaredFragment?.element.id == _returnVariableElement.id,
+    );
+    if (targetVariable == null) return false;
+    variableDeclaration = targetVariable;
+
+    return true;
+  }
+
+  void _checkTokensInBetween(
+    VariableDeclarationStatement variableDeclaration,
+    ReturnStatement returnStatement,
+  ) {
+    final tokenBeforeReturn =
+        _returnStatement.findPrevious(_returnStatement.beginToken);
+
+    if (tokenBeforeReturn != variableDeclaration.endToken) {
+      foundTokensBetweenDeclarationAndReturn = true;
+    }
+  }
+}