Skip to content

Commit f98e53c

Browse files
chloestefantsovaCommit Queue
chloestefantsova
authored and
Commit Queue
committed
[analysis_server] Suggest use_null_aware_elements for getters
Closes #61167 Change-Id: I79acda17de5a8844f97f554219d659dd752902c4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444820 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Chloe Stefantsova <[email protected]>
1 parent 235d6cb commit f98e53c

File tree

3 files changed

+260
-67
lines changed

3 files changed

+260
-67
lines changed

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

Lines changed: 95 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'package:analysis_server/src/services/correction/fix.dart';
66
import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
7+
import 'package:analyzer/dart/ast/token.dart';
78
import 'package:analyzer/dart/element/element.dart';
89
import 'package:analyzer/src/dart/ast/ast.dart';
910
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
@@ -36,52 +37,104 @@ class ConvertNullCheckToNullAwareElementOrEntry
3637
)) {
3738
if (node.caseClause == null) {
3839
// An element or entry of the form `if (x != null) ...`.
39-
if (thenElement is! MapLiteralEntry) {
40-
// In case of a list or set element, we simply replace the entire
41-
// element with the then-element prefixed by '?'.
40+
if (thenElement is SimpleIdentifier) {
41+
// In case of a list or set element with a promotable target, we
42+
// simply replace the entire element with the then-element prefixed by
43+
// '?'.
4244
//
43-
// `[if (x != null) x]` ==> `[?x]`
44-
// `{if (x != null) x]` ==> `{?x}`
45+
// `if (x != null) x` is rewritten as `?x`
4546
await builder.addDartFileEdit(file, (builder) {
4647
builder.addSimpleReplacement(
47-
range.startOffsetEndOffset(
48-
node.ifKeyword.offset,
49-
thenElement.offset,
50-
),
48+
range.startStart(node, thenElement),
5149
'?',
5250
);
5351
});
54-
} else {
52+
} else if (thenElement is PostfixExpression) {
53+
// In case of a list or set element with a getter target, we replace
54+
// the entire element with the then-element target identifier prefixed
55+
// by '?'. Note that in the case of a getter target, the null-check
56+
// operator '!' is always present in [thenElement].
57+
//
58+
// `if (x != null) x!` is rewritten as `?x`
59+
await builder.addDartFileEdit(file, (builder) {
60+
builder.addSimpleReplacement(
61+
range.startStart(node, thenElement),
62+
'?',
63+
);
64+
builder.addDeletion(range.endEnd(thenElement.operand, thenElement));
65+
});
66+
} else if (thenElement is MapLiteralEntry) {
5567
// In case of a map entry we need to check if it's the key that's
5668
// promoted to non-nullable or the value.
69+
var thenElementKey = thenElement.key;
70+
var keyCanonicalElement = switch (thenElementKey) {
71+
SimpleIdentifier() => thenElementKey.canonicalElement,
72+
PostfixExpression(:var operand, operator: Token(lexeme: '!')) =>
73+
operand.canonicalElement,
74+
_ => null,
75+
};
76+
5777
var binaryCondition = condition as BinaryExpression;
58-
var keyCanonicalElement = thenElement.key.canonicalElement;
5978
if (keyCanonicalElement != null &&
6079
(binaryCondition.leftOperand.canonicalElement ==
6180
keyCanonicalElement ||
6281
binaryCondition.rightOperand.canonicalElement ==
6382
keyCanonicalElement)) {
64-
// In case the key is promoted, we simply replace everything before
65-
// the key with '?'.
66-
//
67-
// `{if (x != null) x: "value"}` ==> `{?x: "value"}`
68-
await builder.addDartFileEdit(file, (builder) {
69-
builder.addSimpleReplacement(
70-
range.startOffsetEndOffset(node.offset, thenElement.key.offset),
71-
'?',
72-
);
73-
});
83+
if (thenElementKey is SimpleIdentifier) {
84+
// In case the key is null-aware and is promotable, we simply
85+
// replace everything before the key with '?'.
86+
//
87+
// `if (x != null) x: "v"` is rewritten as `?x: "v"`
88+
await builder.addDartFileEdit(file, (builder) {
89+
builder.addSimpleReplacement(
90+
range.startStart(node, thenElement.key),
91+
'?',
92+
);
93+
});
94+
} else if (thenElementKey is PostfixExpression) {
95+
// In case the key is null-aware and is a getter, we replace
96+
// everything before the key with '?' and remove '!' afterwards.
97+
// Note that in the case of a getter, the null-check operator '!'
98+
// is always present in [thenElementKey].
99+
//
100+
// `if (x != null) x!: "v"` is rewritten as `?x: "v"`
101+
await builder.addDartFileEdit(file, (builder) {
102+
builder.addSimpleReplacement(
103+
range.startStart(node, thenElementKey),
104+
'?',
105+
);
106+
builder.addDeletion(
107+
range.endStart(thenElementKey.operand, thenElement.separator),
108+
);
109+
});
110+
}
74111
} else {
75-
// In case the value is promoted, we remove everything before the
76-
// key and insert '?' before the value.
77-
//
78-
// `{if (x != null) "key": x}` ==> `{"key": ?x}`
79-
await builder.addDartFileEdit(file, (builder) {
80-
builder.addDeletion(
81-
range.startOffsetEndOffset(node.offset, thenElement.key.offset),
82-
);
83-
builder.addSimpleInsertion(thenElement.value.offset, '?');
84-
});
112+
var thenElementValue = thenElement.value;
113+
if (thenElementValue is SimpleIdentifier) {
114+
// In case the value is null-aware and is promotable, we remove
115+
// everything before the key and insert '?' before the value.
116+
//
117+
// `if (x != null) "k": x` is rewritten as `"k": ?x`
118+
await builder.addDartFileEdit(file, (builder) {
119+
builder.addDeletion(range.startStart(node, thenElement.key));
120+
builder.addSimpleInsertion(thenElement.value.offset, '?');
121+
});
122+
} else if (thenElementValue is PostfixExpression) {
123+
// In case the value is null-aware and is a getter, we remove
124+
// everything before the key, insert '?' before the value, and
125+
// delete '!' after it. Note that in the case of a getter, the
126+
// null-check operator '!' is always present in
127+
// [thenElementValue].
128+
//
129+
// `if (x != null) "k": x!` is rewritten as `"k": ?x`
130+
await builder.addDartFileEdit(file, (builder) {
131+
builder.addDeletion(range.startStart(node, thenElementKey));
132+
builder.addSimpleInsertion(thenElementValue.offset, '?');
133+
builder.addDeletion(
134+
range.endEnd(thenElementValue.operand, thenElementValue),
135+
);
136+
});
137+
}
85138
}
86139
}
87140
} else {
@@ -90,13 +143,13 @@ class ConvertNullCheckToNullAwareElementOrEntry
90143
// In case of a list or set element, we replace the entire element
91144
// with the expression to the left of 'case', prefixed by '?'.
92145
//
93-
// `[if (x case var y?) y]` ==> `[?x]`
94-
// `{if (x case var y?) y]` ==> `{?x}`
146+
// `if (x case var y?) y` is rewritten as `?x`
95147
await builder.addDartFileEdit(file, (builder) {
96148
builder.addSimpleReplacement(
97-
range.startOffsetEndOffset(node.offset, node.end),
98-
'?${condition.toSource()}',
149+
range.startStart(node, condition),
150+
'?',
99151
);
152+
builder.addDeletion(range.endEnd(condition, node));
100153
});
101154
} else {
102155
// In case of a map entry we need to check if it's the key that's
@@ -111,28 +164,24 @@ class ConvertNullCheckToNullAwareElementOrEntry
111164
// In case the key is promoted, replace everything before ':' with
112165
// the expression before 'case', prefixed by '?'.
113166
//
114-
// `{if (x case var y?) y: "value"}` ==> `{?x: "value"}`
167+
// `if (x case var y?) y: "v"` is rewritten as `?x: "v"`
115168
await builder.addDartFileEdit(file, (builder) {
116169
builder.addSimpleReplacement(
117-
range.startOffsetEndOffset(node.offset, thenElement.key.end),
118-
'?${condition.toSource()}',
170+
range.startStart(node, condition),
171+
'?',
119172
);
173+
builder.addDeletion(range.endEnd(condition, thenElement.key));
120174
});
121175
} else {
122176
// In case the value is promoted, delete everything before the key
123177
// and replace the value with the expression to the left of 'case',
124178
// prefixed by '?'.
125179
//
126-
// `{if (x case var y?) "key": y}` ==> `{"key": ?x}`
180+
// `if (x case var y?) "k": y` is rewritten as `"k": ?x`
127181
await builder.addDartFileEdit(file, (builder) {
128-
builder.addDeletion(
129-
range.startOffsetEndOffset(node.offset, thenElement.key.offset),
130-
);
182+
builder.addDeletion(range.startStart(node, thenElement.key));
131183
builder.addSimpleReplacement(
132-
range.startOffsetEndOffset(
133-
thenElement.value.offset,
134-
thenElement.value.end,
135-
),
184+
range.startEnd(thenElement.value, thenElement.value),
136185
'?${condition.toSource()}',
137186
);
138187
});

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

Lines changed: 120 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,122 @@ class ConvertNullCheckToNullAwareElementOrEntryTest
109109
@override
110110
String get lintCode => LintNames.use_null_aware_elements;
111111

112-
Future<void> test_nullCheck_list() async {
112+
Future<void> test_nullCheck_getter_list() async {
113+
await resolveTestCode('''
114+
abstract class A {
115+
int? get x;
116+
List<int> f() {
117+
return [
118+
if (x != null) x!,
119+
];
120+
}
121+
}
122+
''');
123+
await assertHasFix('''
124+
abstract class A {
125+
int? get x;
126+
List<int> f() {
127+
return [
128+
?x,
129+
];
130+
}
131+
}
132+
''');
133+
}
134+
135+
Future<void> test_nullCheck_getter_mapKey() async {
136+
await resolveTestCode('''
137+
abstract class A {
138+
int? get x;
139+
Map<int, String> f() {
140+
return {
141+
if (x != null) x!: "",
142+
};
143+
}
144+
}
145+
''');
146+
await assertHasFix('''
147+
abstract class A {
148+
int? get x;
149+
Map<int, String> f() {
150+
return {
151+
?x: "",
152+
};
153+
}
154+
}
155+
''');
156+
}
157+
158+
Future<void> test_nullCheck_getter_mapKeyAndValue() async {
159+
await resolveTestCode('''
160+
abstract class A {
161+
int? get x;
162+
Map<int, int> f() {
163+
return {
164+
if (x != null) x!: x!,
165+
};
166+
}
167+
}
168+
''');
169+
await assertHasFix('''
170+
abstract class A {
171+
int? get x;
172+
Map<int, int> f() {
173+
return {
174+
?x: x!,
175+
};
176+
}
177+
}
178+
''');
179+
}
180+
181+
Future<void> test_nullCheck_getter_mapValue() async {
182+
await resolveTestCode('''
183+
abstract class A {
184+
int? get x;
185+
Map<String, int> f() {
186+
return {
187+
if (x != null) "key1": x!,
188+
};
189+
}
190+
}
191+
''');
192+
await assertHasFix('''
193+
abstract class A {
194+
int? get x;
195+
Map<String, int> f() {
196+
return {
197+
"key1": ?x,
198+
};
199+
}
200+
}
201+
''');
202+
}
203+
204+
Future<void> test_nullCheck_getter_set() async {
205+
await resolveTestCode('''
206+
abstract class A {
207+
int? get x;
208+
Set<int> f() {
209+
return {
210+
if (x != null) x!,
211+
};
212+
}
213+
}
214+
''');
215+
await assertHasFix('''
216+
abstract class A {
217+
int? get x;
218+
Set<int> f() {
219+
return {
220+
?x,
221+
};
222+
}
223+
}
224+
''');
225+
}
226+
227+
Future<void> test_nullCheck_promotable_list() async {
113228
await resolveTestCode('''
114229
List<int> f(int? x) {
115230
return [
@@ -126,7 +241,7 @@ List<int> f(int? x) {
126241
''');
127242
}
128243

129-
Future<void> test_nullCheck_mapKey() async {
244+
Future<void> test_nullCheck_promotable_mapKey() async {
130245
await resolveTestCode('''
131246
Map<int, String> f(int? x) {
132247
return {
@@ -143,7 +258,7 @@ Map<int, String> f(int? x) {
143258
''');
144259
}
145260

146-
Future<void> test_nullCheck_mapKeyAndValue() async {
261+
Future<void> test_nullCheck_promotable_mapKeyAndValue() async {
147262
await resolveTestCode('''
148263
Map<int, int> f(int? x) {
149264
return {
@@ -160,7 +275,7 @@ Map<int, int> f(int? x) {
160275
''');
161276
}
162277

163-
Future<void> test_nullCheck_mapValue() async {
278+
Future<void> test_nullCheck_promotable_mapValue() async {
164279
await resolveTestCode('''
165280
Map<String, int> f(int? x) {
166281
return {
@@ -177,7 +292,7 @@ Map<String, int> f(int? x) {
177292
''');
178293
}
179294

180-
Future<void> test_nullCheck_set() async {
295+
Future<void> test_nullCheck_promotable_set() async {
181296
await resolveTestCode('''
182297
Set<int> f(int? x) {
183298
return {

0 commit comments

Comments
 (0)