Skip to content

Commit 5e3bc83

Browse files
feat: handle cascade & null aware expressions
1 parent 8cbbc84 commit 5e3bc83

2 files changed

Lines changed: 98 additions & 20 deletions

File tree

lib/src/lints/prefer_last/visitors/prefer_last_visitor.dart

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import 'package:solid_lints/src/utils/types_utils.dart';
77
/// The AST visitor that will collect all Iterable access expressions
88
/// which can be replaced with .last
99
class PreferLastVisitor extends RecursiveAstVisitor<void> {
10+
static const _lengthGetterName = 'length';
11+
1012
final PreferLastRule _rule;
1113

1214
/// Creates a new instance of [PreferLastVisitor]
@@ -24,8 +26,7 @@ class PreferLastVisitor extends RecursiveAstVisitor<void> {
2426

2527
final arg = node.argumentList.arguments.firstOrNull;
2628

27-
if (arg is BinaryExpression &&
28-
_isLastElementAccess(arg, target.toString())) {
29+
if (arg is BinaryExpression && _isLastElementAccess(arg, target)) {
2930
_rule.reportAtNode(node);
3031
}
3132
}
@@ -40,38 +41,72 @@ class PreferLastVisitor extends RecursiveAstVisitor<void> {
4041

4142
final index = node.index;
4243

43-
if (index is BinaryExpression &&
44-
_isLastElementAccess(index, target.toString())) {
44+
if (index is BinaryExpression && _isLastElementAccess(index, target)) {
4545
_rule.reportAtNode(node);
4646
}
4747
}
4848

49-
bool _isLastElementAccess(BinaryExpression expression, String targetName) {
49+
bool _isLastElementAccess(BinaryExpression expression, Expression? target) {
5050
final right = expression.rightOperand;
5151
if (right is! IntegerLiteral || right.value != 1) return false;
5252

53-
final isMinusExpression = expression.operator.type == TokenType.MINUS;
54-
if (!isMinusExpression) return false;
53+
if (expression.operator.type != TokenType.MINUS) return false;
54+
if (target == null) return false;
5555

56-
final left = expression.leftOperand;
57-
final leftName = _getLeftOperandName(left);
56+
return _isLengthOfTarget(expression.leftOperand, target);
57+
}
5858

59-
return leftName == '$targetName.length';
59+
bool _isLengthOfTarget(Expression expression, Expression target) {
60+
final lengthReceiver = switch (expression) {
61+
PropertyAccess(
62+
propertyName: SimpleIdentifier(name: _lengthGetterName),
63+
operator: Token(type: TokenType.PERIOD),
64+
:final target,
65+
) =>
66+
target,
67+
PrefixedIdentifier(
68+
prefix: final prefix,
69+
identifier: SimpleIdentifier(name: _lengthGetterName),
70+
) =>
71+
prefix,
72+
_ => null,
73+
};
74+
75+
if (lengthReceiver == null) return false;
76+
77+
return _referencesSameTarget(lengthReceiver, target);
6078
}
6179

62-
String? _getLeftOperandName(Expression expression) {
63-
if (expression is PrefixedIdentifier) {
64-
return expression.name;
80+
bool _referencesSameTarget(
81+
Expression lengthReceiver,
82+
Expression accessTarget,
83+
) {
84+
final normalizedLengthReceiver = _unwrapExpression(lengthReceiver);
85+
final normalizedAccessTarget = _unwrapExpression(accessTarget);
86+
87+
if (normalizedLengthReceiver is Identifier &&
88+
normalizedAccessTarget is Identifier) {
89+
return normalizedLengthReceiver.element?.id ==
90+
normalizedAccessTarget.element?.id;
6591
}
6692

67-
/// Access target like map.keys.length is being reported as PropertyAccess
68-
/// expression this case will handle such cases
69-
if (expression is PropertyAccess) {
70-
if (expression.operator.type != TokenType.PERIOD) return null;
93+
return normalizedLengthReceiver.toString() ==
94+
normalizedAccessTarget.toString();
95+
}
96+
97+
Expression _unwrapExpression(Expression expression) {
98+
switch (expression) {
99+
case PostfixExpression(
100+
:final operand,
101+
operator: Token(type: TokenType.BANG),
102+
):
103+
return _unwrapExpression(operand);
71104

72-
return expression.toString();
73-
}
105+
case ParenthesizedExpression(:final expression):
106+
return _unwrapExpression(expression);
74107

75-
return null;
108+
default:
109+
return expression;
110+
}
76111
}
77112
}

test/prefer_last/prefer_last_rule_test.dart

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,47 @@ void main () {
102102
[lint(104, 39), lint(234, 29), lint(267, 39), lint(310, 43)],
103103
);
104104
}
105+
106+
void test_reports_on_cascade_element_at_access_with_length_minus_one() async {
107+
await assertDiagnostics(
108+
r'''
109+
void main () {
110+
final list2 = [1, 0, 2, 3];
111+
list2..elementAt(list2.length - 1);
112+
}
113+
''',
114+
[lint(52, 29)],
115+
);
116+
}
117+
118+
void test_reports_on_null_aware_index_access_with_length_minus_one() async {
119+
await assertDiagnostics(
120+
r'''
121+
List<int>? list1 = [0, 1, 2, 3];
122+
var a = list1?[list1!.length - 1];
123+
124+
void test(List<int>? list2) {
125+
list1?[list1!.length - 1];
126+
list2?[list2.length - 1];
127+
}
128+
''',
129+
[lint(41, 25), lint(101, 25), lint(130, 24)],
130+
);
131+
}
132+
133+
void
134+
test_reports_on_null_aware_element_at_access_with_length_minus_one() async {
135+
await assertDiagnostics(
136+
r'''
137+
List<int>? list1 = [0, 1, 2, 3];
138+
var a = list1?.elementAt(list1!.length - 1);
139+
140+
void test(List<int>? list2) {
141+
list1?.elementAt(list1!.length - 1);
142+
list2?.elementAt(list2.length - 1);
143+
}
144+
''',
145+
[lint(41, 35), lint(111, 35), lint(150, 34)],
146+
);
147+
}
105148
}

0 commit comments

Comments
 (0)