Skip to content

Commit 5d78579

Browse files
stereotype441Commit Queue
authored andcommitted
[analyzer] Fix dead code reporting for null-aware accesses.
A null-aware access can result in dead code if the target has static type `Null`. The analyzer wasn't properly accounting for this, resulting in some confusing ranges reported for the DEAD_CODE warning. This change causes the following expressions to report dead code for the code ranges indiced by `^`: Null myNullVar = null; myNullVar?[index]; // ^^^^^^ DEAD_CODE myNullVar?[index] = value; // ^^^^^^^^^^^^^^ DEAD_CODE myNullVar?.method(); // ^^^^^^^^ DEAD_CODE myNullVar?.property; // ^^^^^^^^ DEAD_CODE myNullVar?.property = value; // ^^^^^^^^^^^^^^^^ DEAD_CODE Note that the bug was confined solely to the logic that reports the DEAD_CODE warning; there is no change to the reachability inferred by flow analysis (and hence, this is a non-breaking change). Fixes #60364. Bug: #60364 Change-Id: I068826282fba6b9057e9c27d1d9310c65714e203 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416723 Auto-Submit: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>
1 parent 5d1f487 commit 5d78579

File tree

3 files changed

+68
-1
lines changed

3 files changed

+68
-1
lines changed

pkg/_fe_analyzer_shared/test/flow_analysis/reachability/data/unreachable_via_never.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ void nonNullAssert(Null Function() f) {
5050
}
5151

5252
void nullAwareAccess(Null Function() f, Object? Function() g) {
53-
f()?.extensionMethod(/*unreachable*/ 1);
53+
f()?./*analyzer.unreachable*/extensionMethod(/*unreachable*/ 1);
5454
g()?.extensionMethod(2);
5555
}
5656

pkg/analyzer/lib/src/generated/resolver.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
12391239
do {
12401240
_unfinishedNullShorts.removeLast();
12411241
flowAnalysis.flow!.nullAwareAccess_end();
1242+
nullSafetyDeadCodeVerifier.flowEnd(node);
12421243
} while (identical(_unfinishedNullShorts.last, node));
12431244
if (node is! CascadeExpression) {
12441245
// Make the static type of `node` (or whatever it was rewritten to)
@@ -1404,6 +1405,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
14041405

14051406
if (node.isNullAware) {
14061407
_startNullAwareAccess(node, node.target);
1408+
nullSafetyDeadCodeVerifier.visitNode(node.index);
14071409
}
14081410

14091411
var result = _propertyElementResolver.resolveIndexExpression(
@@ -1457,6 +1459,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
14571459
}
14581460
if (node.isNullAware) {
14591461
_startNullAwareAccess(node, node.target);
1462+
nullSafetyDeadCodeVerifier.visitNode(node.propertyName);
14601463
}
14611464

14621465
inferenceLogWriter?.exitLValue(node);
@@ -2838,6 +2841,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
28382841

28392842
if (node.isNullAware) {
28402843
_startNullAwareAccess(node, node.target);
2844+
nullSafetyDeadCodeVerifier.visitNode(node.index);
28412845
}
28422846

28432847
var result = _propertyElementResolver.resolveIndexExpression(
@@ -3055,6 +3059,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
30553059

30563060
if (node.isNullAware) {
30573061
_startNullAwareAccess(node, target);
3062+
nullSafetyDeadCodeVerifier.visitNode(node.methodName);
30583063
}
30593064

30603065
node.typeArguments?.accept(this);
@@ -3945,6 +3950,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
39453950
PropertyAccessImpl node, TypeImpl contextType) {
39463951
if (node.isNullAware) {
39473952
_startNullAwareAccess(node, node.target);
3953+
nullSafetyDeadCodeVerifier.visitNode(node.propertyName);
39483954
}
39493955

39503956
var result = _propertyElementResolver.resolvePropertyAccess(
@@ -4009,6 +4015,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
40094015

40104016
if (function is PropertyAccessImpl && function.isNullAware) {
40114017
_startNullAwareAccess(function, function.target);
4018+
nullSafetyDeadCodeVerifier.visitNode(node.argumentList);
40124019
}
40134020

40144021
_functionExpressionInvocationResolver.resolve(node, whyNotPromotedArguments,

pkg/analyzer/test/src/diagnostics/dead_code_test.dart

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,66 @@ void f() {
202202
]);
203203
}
204204

205+
test_nullAwareIndexedRead() async {
206+
await assertErrorsInCode(r'''
207+
void f(Null n, int i) {
208+
n?[i];
209+
print('reached');
210+
}
211+
''', [
212+
// Dead range: `i]`
213+
error(WarningCode.DEAD_CODE, 29, 2)
214+
]);
215+
}
216+
217+
test_nullAwareIndexedWrite() async {
218+
await assertErrorsInCode(r'''
219+
void f(Null n, int i, int j) {
220+
n?[i] = j;
221+
print('reached');
222+
}
223+
''', [
224+
// Dead range: `i] = j`
225+
error(WarningCode.DEAD_CODE, 36, 6)
226+
]);
227+
}
228+
229+
test_nullAwareMethodInvocation() async {
230+
await assertErrorsInCode(r'''
231+
void f(Null n, int i) {
232+
n?.foo(i);
233+
print('reached');
234+
}
235+
''', [
236+
// Dead range: `foo(i)`
237+
error(WarningCode.DEAD_CODE, 29, 6)
238+
]);
239+
}
240+
241+
test_nullAwarePropertyRead() async {
242+
await assertErrorsInCode(r'''
243+
void f(Null n) {
244+
n?.p;
245+
print('reached');
246+
}
247+
''', [
248+
// Dead range: `p`
249+
error(WarningCode.DEAD_CODE, 22, 1)
250+
]);
251+
}
252+
253+
test_nullAwarePropertyWrite() async {
254+
await assertErrorsInCode(r'''
255+
void f(Null n, int i) {
256+
n?.p = i;
257+
print('reached');
258+
}
259+
''', [
260+
// Dead range: `p = i`
261+
error(WarningCode.DEAD_CODE, 29, 5)
262+
]);
263+
}
264+
205265
test_objectPattern_neverTypedGetter() async {
206266
await assertErrorsInCode(r'''
207267
class A {

0 commit comments

Comments
 (0)