diff --git a/lib/lints/prefer_last/prefer_last_fix.dart b/lib/lints/prefer_last/prefer_last_fix.dart new file mode 100644 index 00000000..9e276350 --- /dev/null +++ b/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 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, + ); + }); + } +} diff --git a/lib/lints/prefer_last/prefer_last_rule.dart b/lib/lints/prefer_last/prefer_last_rule.dart new file mode 100644 index 00000000..f965144a --- /dev/null +++ b/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 getFixes() => [PreferLastFix()]; +} diff --git a/lib/lints/prefer_last/prefer_last_visitor.dart b/lib/lints/prefer_last/prefer_last_visitor.dart new file mode 100644 index 00000000..aa76127f --- /dev/null +++ b/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 { + final _expressions = []; + + /// List of all Iterable access expressions + Iterable 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; + } +} diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index b6711421..f43c00d2 100644 --- a/lib/solid_lints.dart +++ b/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 diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index 406ed6a2..10cd3967 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -51,3 +51,4 @@ custom_lint: - no-magic-number - prefer-conditional-expressions - prefer-first + - prefer-last diff --git a/lint_test/prefer_last_test.dart b/lint_test/prefer_last_test.dart new file mode 100644 index 00000000..0007641d --- /dev/null +++ b/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); +}