Skip to content
This repository was archived by the owner on Nov 20, 2024. It is now read-only.

Commit 1e603e1

Browse files
authored
Fix cascade_invocations false positive when examining complicated expressions (#1328)
Fix cascade_invocations bug
1 parent cc95903 commit 1e603e1

File tree

2 files changed

+76
-7
lines changed

2 files changed

+76
-7
lines changed

lib/src/rules/cascade_invocations.dart

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,8 @@ Element _getPrefixElementFromExpression(Expression rawExpression) {
7979
return DartTypeUtilities.getCanonicalElementFromIdentifier(
8080
expression.prefix);
8181
} else if (expression is PropertyAccess &&
82-
_isInvokedWithoutNullAwareOperator(expression.operator)) {
82+
_isInvokedWithoutNullAwareOperator(expression.operator) &&
83+
expression.target is SimpleIdentifier) {
8384
return DartTypeUtilities.getCanonicalElementFromIdentifier(
8485
expression.target);
8586
}
@@ -121,7 +122,14 @@ class _CascadableExpression {
121122
null, [],
122123
canJoin: false, canReceive: false, canBeCascaded: false);
123124

125+
/// Whether this expression can be joined with a previous expression via a
126+
/// cascade operation.
124127
final bool canJoin;
128+
129+
/// Whether this expression can receive an additional expression with a
130+
/// cascade operation.
131+
///
132+
/// For example, `a.b = 1` can receive, but `a = 1` cannot receive.
125133
final bool canReceive;
126134
final bool canBeCascaded;
127135

@@ -178,7 +186,7 @@ class _CascadableExpression {
178186
isCritical: true);
179187
}
180188
// setters
181-
final variable = _getPrefixElementFromExpression(node.leftHandSide);
189+
final variable = _getPrefixElementFromExpression(leftExpression);
182190
final canReceive = node.operator.type != TokenType.QUESTION_QUESTION_EQ &&
183191
variable is VariableElement &&
184192
!variable.isStatic;
@@ -212,17 +220,44 @@ class _CascadableExpression {
212220
DartTypeUtilities.getCanonicalElementFromIdentifier(node.prefix), [],
213221
canJoin: true, canReceive: true, canBeCascaded: true);
214222

215-
factory _CascadableExpression._fromPropertyAccess(PropertyAccess node) =>
216-
new _CascadableExpression._internal(
217-
DartTypeUtilities.getCanonicalElementFromIdentifier(node.target), [],
218-
canJoin: true, canReceive: true, canBeCascaded: true);
223+
factory _CascadableExpression._fromPropertyAccess(PropertyAccess node) {
224+
var targetIsSimple = node.target is SimpleIdentifier;
225+
return new _CascadableExpression._internal(
226+
DartTypeUtilities.getCanonicalElementFromIdentifier(node.target), [],
227+
// If the target is something like `(a + b).x`, then node can neither
228+
// join, nor receive.
229+
//
230+
// This restriction is quite limiting. It means that this rule cannot
231+
// report on the following code:
232+
//
233+
// b1.a
234+
// ..f = 1
235+
// ..g = 1;
236+
// b2.a.f = 2; // Could report here.
237+
//
238+
// But it may be difficult to accurately know if complicated
239+
// expressions are cascadable. Just because two expressions are "equal"
240+
// or look the same, does not mean they can be cascaded; for example:
241+
//
242+
// (b1 + b2).a
243+
// ..f = 1
244+
// ..g = 1;
245+
// (b1 + b2).a.f = 2; // Should not report here.
246+
//
247+
// TODO(srawlins): Look into whether this is possible.
248+
canJoin: targetIsSimple,
249+
canReceive: targetIsSimple,
250+
canBeCascaded: true);
251+
}
219252

220253
_CascadableExpression._internal(this.element, this.criticalNodes,
221254
{this.canJoin,
222255
this.canReceive,
223256
this.canBeCascaded,
224257
this.isCritical = false});
225258

259+
/// Whether [this] is compatible to be joined with [expressionBox] with a
260+
/// cascade operation.
226261
bool compatibleWith(_CascadableExpression expressionBox) =>
227262
element != null &&
228263
expressionBox.canReceive &&

test/rules/cascade_invocations.dart

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ void cascadeStatic() {
109109
StaticFoo.decrement();
110110
}
111111

112-
// Bug 339
112+
// https://github.com/dart-lang/linter/issues/339
113113
class Identifier339 {
114114
String value;
115115
String system;
@@ -194,3 +194,37 @@ void function694() {
194194
Bug694.bar = 2;
195195
Bug694.foo = 3; // OK
196196
}
197+
198+
class A {
199+
int f;
200+
int g;
201+
202+
int get p => 7;
203+
int get q => 6;
204+
}
205+
206+
class B {
207+
final A a = A();
208+
209+
B operator +(other) {
210+
return B();
211+
}
212+
}
213+
214+
// https://github.com/dart-lang/linter/issues/1323
215+
void bug1323() {
216+
final B b1 = B();
217+
final B b2 = B();
218+
219+
b1.a
220+
..f = 1
221+
..g = 1;
222+
b2.a.f = 2; // OK
223+
224+
print('buffer statement.');
225+
226+
b1.a
227+
..p
228+
..q;
229+
b2.a.p; // OK
230+
}

0 commit comments

Comments
 (0)