Add prefer last rule (#61)

* Add prefer-conditional-expressions rule and fix

* Add tests for prefer-conditional-expressions rule

* fix nested test plugin path

* Fix tests after merge

* Add prefer-first rule and fix

* Add tests for prefer-first rule

* Add prefer-last rule & fix

* Add tests for prefer-last rule

---------

Co-authored-by: Denis Bogatirov <[email protected]>
Co-authored-by: Yurii Prykhodko <[email protected]>

lib/lints/prefer_last/prefer_last_fix.dart

@@ -0,0 +1,67 @@
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/error/error.dart';
+import 'package:analyzer/source/source_range.dart';
+import 'package:custom_lint_builder/custom_lint_builder.dart';
+
+/// A Quick fix for `prefer-last` rule
+/// Suggests to replace iterable access expressions
+class PreferLastFix extends DartFix {
+  static const _replaceComment = "Replace with 'last'.";
+
+  @override
+  void run(
+    CustomLintResolver resolver,
+    ChangeReporter reporter,
+    CustomLintContext context,
+    AnalysisError analysisError,
+    List<AnalysisError> others,
+  ) {
+    context.registry.addMethodInvocation((node) {
+      if (analysisError.sourceRange.intersects(node.sourceRange)) {
+        final correction = _createCorrection(node);
+
+        _addReplacement(reporter, node, correction);
+      }
+    });
+
+    context.registry.addIndexExpression((node) {
+      if (analysisError.sourceRange.intersects(node.sourceRange)) {
+        final correction = _createCorrection(node);
+
+        _addReplacement(reporter, node, correction);
+      }
+    });
+  }
+
+  String _createCorrection(Expression expression) {
+    if (expression is MethodInvocation) {
+      return expression.isCascaded
+          ? '..last'
+          : '${expression.target ?? ''}.last';
+    } else if (expression is IndexExpression) {
+      return expression.isCascaded
+          ? '..last'
+          : '${expression.target ?? ''}.last';
+    } else {
+      return '.last';
+    }
+  }
+
+  void _addReplacement(
+    ChangeReporter reporter,
+    Expression node,
+    String correction,
+  ) {
+    final changeBuilder = reporter.createChangeBuilder(
+      message: _replaceComment,
+      priority: 1,
+    );
+
+    changeBuilder.addDartFileEdit((builder) {
+      builder.addSimpleReplacement(
+        SourceRange(node.offset, node.length),
+        correction,
+      );
+    });
+  }
+}

lib/lints/prefer_last/prefer_last_rule.dart

@@ -0,0 +1,48 @@
+import 'package:analyzer/error/listener.dart';
+import 'package:custom_lint_builder/custom_lint_builder.dart';
+import 'package:solid_lints/lints/prefer_last/prefer_last_fix.dart';
+import 'package:solid_lints/lints/prefer_last/prefer_last_visitor.dart';
+import 'package:solid_lints/models/rule_config.dart';
+import 'package:solid_lints/models/solid_lint_rule.dart';
+
+/// A `prefer-last` rule which warns about
+/// usage of iterable[length-1] or iterable.elementAt(length-1)
+class PreferLastRule extends SolidLintRule {
+  /// The [LintCode] of this lint rule that represents the error if iterable
+  /// access can be simplified.
+  static const lintName = 'prefer-last';
+
+  PreferLastRule._(super.config);
+
+  /// Creates a new instance of [PreferLastRule]
+  /// based on the lint configuration.
+  factory PreferLastRule.createRule(CustomLintConfigs configs) {
+    final config = RuleConfig(
+      configs: configs,
+      name: lintName,
+      problemMessage: (value) =>
+          'Use last instead of accessing the last element by index.',
+    );
+
+    return PreferLastRule._(config);
+  }
+
+  @override
+  void run(
+    CustomLintResolver resolver,
+    ErrorReporter reporter,
+    CustomLintContext context,
+  ) {
+    context.registry.addCompilationUnit((node) {
+      final visitor = PreferLastVisitor();
+      node.accept(visitor);
+
+      for (final element in visitor.expressions) {
+        reporter.reportErrorForNode(code, element);
+      }
+    });
+  }
+
+  @override
+  List<Fix> getFixes() => [PreferLastFix()];
+}

lib/lints/prefer_last/prefer_last_visitor.dart

@@ -0,0 +1,74 @@
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:solid_lints/utils/types_utils.dart';
+
+/// The AST visitor that will collect all Iterable access expressions
+/// which can be replaced with .last
+class PreferLastVisitor extends RecursiveAstVisitor<void> {
+  final _expressions = <Expression>[];
+
+  /// List of all Iterable access expressions
+  Iterable<Expression> get expressions => _expressions;
+
+  @override
+  void visitMethodInvocation(MethodInvocation node) {
+    super.visitMethodInvocation(node);
+
+    final target = node.realTarget;
+
+    if (isIterableOrSubclass(target?.staticType) &&
+        node.methodName.name == 'elementAt') {
+      final arg = node.argumentList.arguments.first;
+
+      if (arg is BinaryExpression &&
+          _isLastElementAccess(arg, target.toString())) {
+        _expressions.add(node);
+      }
+    }
+  }
+
+  @override
+  void visitIndexExpression(IndexExpression node) {
+    super.visitIndexExpression(node);
+
+    final target = node.realTarget;
+
+    if (isListOrSubclass(target.staticType)) {
+      final index = node.index;
+
+      if (index is BinaryExpression &&
+          _isLastElementAccess(index, target.toString())) {
+        _expressions.add(node);
+      }
+    }
+  }
+
+  bool _isLastElementAccess(BinaryExpression expression, String targetName) {
+    final left = expression.leftOperand;
+    final right = expression.rightOperand;
+    final leftName = _getLeftOperandName(left);
+
+    if (right is! IntegerLiteral) return false;
+    if (right.value != 1) return false;
+    if (expression.operator.type != TokenType.MINUS) return false;
+
+    return leftName == '$targetName.length';
+  }
+
+  String? _getLeftOperandName(Expression expression) {
+    if (expression is PrefixedIdentifier) {
+      return expression.name;
+    }
+
+    /// Access target like map.keys.length is being reported as PropertyAccess
+    /// expression this case will handle such cases
+    if (expression is PropertyAccess) {
+      if (expression.operator.type != TokenType.PERIOD) return null;
+
+      return expression.toString();
+    }
+
+    return null;
+  }
+}

lib/solid_lints.dart

@@ -21,6 +21,8 @@ import 'package:solid_lints/lints/no_magic_number/no_magic_number_rule.dart';
 import 'package:solid_lints/lints/number_of_parameters/number_of_parameters_metric.dart';
 import 'package:solid_lints/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart';
 import 'package:solid_lints/lints/prefer_first/prefer_first_rule.dart';
+import 'package:solid_lints/lints/prefer_last/prefer_last_rule.dart';
+
 import 'package:solid_lints/models/solid_lint_rule.dart';
 
 /// Creates a plugin for our custom linter
@@ -51,6 +53,7 @@ class _SolidLints extends PluginBase {
       NoMagicNumberRule.createRule(configs),
       PreferConditionalExpressionsRule.createRule(configs),
       PreferFirstRule.createRule(configs),
+      PreferLastRule.createRule(configs),
     ];
 
     // Return only enabled rules

lint_test/analysis_options.yaml

@@ -51,3 +51,4 @@ custom_lint:
     - no-magic-number
     - prefer-conditional-expressions
     - prefer-first
+    - prefer-last

lint_test/prefer_last_test.dart

@@ -0,0 +1,25 @@
+/// Check the `prefer-first` rule
+void fun() {
+  final list = [0, 1, 2, 3];
+  final length = list.length - 1;
+  final set = {0, 1, 2, 3};
+  final map = {0: 0, 1: 1, 2: 2, 3: 3};
+
+  // expect_lint: prefer-last
+  list[list.length - 1];
+
+  list[length - 1];
+
+  // expect_lint: prefer-last
+  list.elementAt(list.length - 1);
+  list.elementAt(length - 1);
+
+  // expect_lint: prefer-last
+  set.elementAt(set.length - 1);
+
+  // expect_lint: prefer-last
+  map.keys.elementAt(map.keys.length - 1);
+
+  // expect_lint: prefer-last
+  map.values.elementAt(map.values.length - 1);
+}