Skip to content

Commit 3f299c9

Browse files
oprypinCommit Queue
oprypin
authored and
Commit Queue
committed
Handle more cases in convert_to_null_aware fixer
This is done mainly by copying significant parts of the code from the prefer_null_aware_operators lint itself Change-Id: I13fb7f52ee003924fd5e2f336cfde2bac6b6011a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/340080 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Oleh Prypin <[email protected]>
1 parent 8356cfe commit 3f299c9

File tree

3 files changed

+168
-29
lines changed

3 files changed

+168
-29
lines changed

‎pkg/analysis_server/lib/src/services/correction/dart/convert_to_null_aware.dart

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import 'package:analysis_server/src/services/correction/dart/abstract_producer.d
77
import 'package:analysis_server/src/services/correction/fix.dart';
88
import 'package:analyzer/dart/ast/ast.dart';
99
import 'package:analyzer/dart/ast/token.dart';
10-
import 'package:analyzer/dart/element/element.dart';
10+
import 'package:analyzer/source/source_range.dart';
1111
import 'package:analyzer_plugin/utilities/assist/assist.dart';
1212
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
1313
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
@@ -43,29 +43,24 @@ class ConvertToNullAware extends ResolvedCorrectionProducer {
4343
return;
4444
}
4545
var condition = targetNode.condition.unParenthesized;
46-
SimpleIdentifier identifier;
46+
String conditionText;
4747
Expression nullExpression;
4848
Expression nonNullExpression;
49-
int periodOffset;
5049

5150
if (condition is BinaryExpression) {
5251
//
5352
// Identify the variable being compared to `null`, or return if the
54-
// condition isn't a simple comparison of `null` to a variable's value.
53+
// condition isn't a simple comparison of `null` to another expression.
5554
//
5655
var leftOperand = condition.leftOperand;
5756
var rightOperand = condition.rightOperand;
58-
if (leftOperand is NullLiteral && rightOperand is SimpleIdentifier) {
59-
identifier = rightOperand;
60-
} else if (rightOperand is NullLiteral &&
61-
leftOperand is SimpleIdentifier) {
62-
identifier = leftOperand;
57+
if (leftOperand is NullLiteral && rightOperand is! NullLiteral) {
58+
conditionText = rightOperand.toString();
59+
} else if (rightOperand is NullLiteral && leftOperand is! NullLiteral) {
60+
conditionText = leftOperand.toString();
6361
} else {
6462
return;
6563
}
66-
if (identifier.staticElement is! LocalElement) {
67-
return;
68-
}
6964
//
7065
// Identify the expression executed when the variable is `null` and when
7166
// it is non-`null`. Return if the `null` expression isn't a null literal
@@ -84,30 +79,51 @@ class ConvertToNullAware extends ResolvedCorrectionProducer {
8479
if (nullExpression.unParenthesized is! NullLiteral) {
8580
return;
8681
}
87-
var unwrappedExpression = nonNullExpression.unParenthesized;
88-
Expression? target;
82+
Expression? resultExpression = nonNullExpression.unParenthesized;
8983
Token? operator;
90-
if (unwrappedExpression is MethodInvocation) {
91-
target = unwrappedExpression.target;
92-
operator = unwrappedExpression.operator;
93-
} else if (unwrappedExpression is PrefixedIdentifier) {
94-
target = unwrappedExpression.prefix;
95-
operator = unwrappedExpression.period;
96-
} else {
97-
return;
84+
while (true) {
85+
switch (resultExpression) {
86+
case PrefixedIdentifier():
87+
operator = resultExpression.period;
88+
resultExpression = resultExpression.prefix;
89+
case MethodInvocation():
90+
operator = resultExpression.operator;
91+
resultExpression = resultExpression.target;
92+
case PostfixExpression()
93+
when resultExpression.operator.type == TokenType.BANG:
94+
// (Operator remains unaffected.)
95+
resultExpression = resultExpression.operand;
96+
case PropertyAccess():
97+
operator = resultExpression.operator;
98+
resultExpression = resultExpression.target;
99+
default:
100+
return;
101+
}
102+
if (resultExpression.toString() == conditionText) {
103+
break;
104+
}
98105
}
99-
if (operator == null || operator.type != TokenType.PERIOD) {
106+
if (resultExpression == null) {
100107
return;
101108
}
102-
if (!(target is SimpleIdentifier &&
103-
target.staticElement == identifier.staticElement)) {
109+
110+
SourceRange operatorRange;
111+
var optionalQuestionMark = '';
112+
if (operator != null) {
113+
if (operator.type == TokenType.PERIOD) {
114+
optionalQuestionMark = '?';
115+
}
116+
operatorRange = range.endStart(resultExpression, operator);
117+
} else if (resultExpression.parent is PostfixExpression) {
118+
// The case where the expression is just `foo!`.
119+
operatorRange =
120+
range.endEnd(resultExpression, resultExpression.parent!);
121+
} else {
104122
return;
105123
}
106-
periodOffset = operator.offset;
107-
108124
await builder.addDartFileEdit(file, (builder) {
109125
builder.addDeletion(range.startStart(targetNode, nonNullExpression));
110-
builder.addSimpleInsertion(periodOffset, '?');
126+
builder.addSimpleReplacement(operatorRange, optionalQuestionMark);
111127
builder.addDeletion(range.endEnd(nonNullExpression, targetNode));
112128
});
113129
}

‎pkg/analysis_server/test/src/services/correction/assist/convert_to_null_aware_test.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,12 @@ abstract class A {
7878
}
7979
int? f(A? a1) => a1 == null ? null : a1?.m();
8080
''');
81-
await assertNoAssistAt('? n');
81+
await assertHasAssistAt('? n', '''
82+
abstract class A {
83+
int m();
84+
}
85+
int? f(A? a1) => a1?.m();
86+
''');
8287
}
8388

8489
Future<void> test_equal_nullOnLeft() async {

‎pkg/analysis_server/test/src/services/correction/fix/convert_to_null_aware_test.dart

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,124 @@ abstract class A {
5858
int m();
5959
}
6060
int? f(A? a) => a?.m();
61+
''');
62+
}
63+
64+
Future<void> test_equal_targetAlreadyNullAware() async {
65+
await resolveTestCode('''
66+
class A {
67+
late int? value;
68+
}
69+
70+
void f(A bar) {
71+
final foo = bar.value == null
72+
? null
73+
: bar.value?.sign;
74+
print(foo);
75+
}
76+
''');
77+
await assertHasFix('''
78+
class A {
79+
late int? value;
80+
}
81+
82+
void f(A bar) {
83+
final foo = bar.value?.sign;
84+
print(foo);
85+
}
86+
''');
87+
}
88+
89+
Future<void> test_equal_targetTrailingNonNullAssert() async {
90+
await resolveTestCode('''
91+
class A {
92+
int? x;
93+
}
94+
class C {
95+
late A foo;
96+
int? bar;
97+
98+
void f() {
99+
bar = foo.x == null
100+
? null
101+
: foo.x!;
102+
}
103+
}
104+
''');
105+
await assertHasFix('''
106+
class A {
107+
int? x;
108+
}
109+
class C {
110+
late A foo;
111+
int? bar;
112+
113+
void f() {
114+
bar = foo.x;
115+
}
116+
}
117+
''');
118+
}
119+
120+
Future<void> test_notEqual_targetChained() async {
121+
await resolveTestCode('''
122+
void f(int? bar) {
123+
final foo = bar != null
124+
? bar.sign.remainder(5)
125+
: null;
126+
print(foo);
127+
}
128+
''');
129+
await assertHasFix('''
130+
void f(int? bar) {
131+
final foo = bar?.sign.remainder(5);
132+
print(foo);
133+
}
134+
''');
135+
}
136+
137+
Future<void> test_notEqual_targetIndexAndNonNullAssert() async {
138+
await resolveTestCode('''
139+
void f(List a, int i) {
140+
print(a[i] != null
141+
? a[i]!.test()
142+
: null);
143+
}
144+
''');
145+
await assertHasFix('''
146+
void f(List a, int i) {
147+
print(a[i]?.test());
148+
}
149+
''');
150+
}
151+
152+
Future<void> test_notEqual_targetNonNullAssertChained() async {
153+
await resolveTestCode('''
154+
abstract class A {
155+
int? x;
156+
int? f() => x != null ? x!.remainder(5) : null;
157+
}
158+
''');
159+
await assertHasFix('''
160+
abstract class A {
161+
int? x;
162+
int? f() => x?.remainder(5);
163+
}
164+
''');
165+
}
166+
167+
Future<void> test_notEqual_targetTrailingNonNullAssertSingle() async {
168+
await resolveTestCode('''
169+
abstract class A {
170+
String? bar;
171+
String? f() => bar != null ? bar! : null;
172+
}
173+
''');
174+
await assertHasFix('''
175+
abstract class A {
176+
String? bar;
177+
String? f() => bar;
178+
}
61179
''');
62180
}
63181
}

0 commit comments

Comments
 (0)