Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add prefer last rule #61

67 changes: 67 additions & 0 deletions lib/lints/prefer_last/prefer_last_fix.dart
Original file line number Diff line number Diff line change
@@ -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,
);
});
}
}
48 changes: 48 additions & 0 deletions lib/lints/prefer_last/prefer_last_rule.dart
Original file line number Diff line number Diff line change
@@ -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()];
}
74 changes: 74 additions & 0 deletions lib/lints/prefer_last/prefer_last_visitor.dart
Original file line number Diff line number Diff line change
@@ -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;
}
}
3 changes: 3 additions & 0 deletions lib/solid_lints.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -51,6 +53,7 @@ class _SolidLints extends PluginBase {
NoMagicNumberRule.createRule(configs),
PreferConditionalExpressionsRule.createRule(configs),
PreferFirstRule.createRule(configs),
PreferLastRule.createRule(configs),
];

// Return only enabled rules
Expand Down
1 change: 1 addition & 0 deletions lint_test/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,4 @@ custom_lint:
- no-magic-number
- prefer-conditional-expressions
- prefer-first
- prefer-last
25 changes: 25 additions & 0 deletions lint_test/prefer_last_test.dart
Original file line number Diff line number Diff line change
@@ -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);
}