Skip to content

Commit

Permalink
[analyzer] Fix flow analysis of null-aware extension overrides.
Browse files Browse the repository at this point in the history
When an extension override is used in a null-aware fashion (for
example, `E(x)?.foo(...)`, where `E` refers to an extension
declaration), the expression appearing inside the extension override
(`x` in this example) needs to be passed to flow analysis, so that it
can be type promoted if appropriate.

Fixes #59654.

Bug: #59654
Change-Id: I9968f7403eee63f209eb39b12d4904151f7d6914
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/399623
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
  • Loading branch information
stereotype441 authored and Commit Queue committed Dec 15, 2024
1 parent 47269eb commit 2f33461
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 3 deletions.
11 changes: 8 additions & 3 deletions pkg/analyzer/lib/src/generated/resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4261,9 +4261,14 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
case SimpleIdentifier(staticElement: InterfaceElement()):
// `?.` to access static methods is equivalent to `.`, so do nothing.
break;
default:
flow.nullAwareAccess_rightBegin(target,
SharedTypeView(target.staticType ?? typeProvider.dynamicType));
case ExtensionOverride(
argumentList: ArgumentList(arguments: [var expression])
):
case var expression:
flow.nullAwareAccess_rightBegin(
expression,
SharedTypeView(
expression.staticType ?? typeProvider.dynamicType));
_unfinishedNullShorts.add(node.nullShortingTermination);
}
}
Expand Down
27 changes: 27 additions & 0 deletions pkg/analyzer/test/src/dart/resolution/extension_override_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import 'package:analyzer/src/error/codes.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

import 'context_collection_resolution.dart';
import 'node_text_expectations.dart';

main() {
defineReflectiveSuite(() {
defineReflectiveTests(ExtensionOverrideResolutionTest);
defineReflectiveTests(UpdateNodeTextExpectations);
});
}

Expand Down Expand Up @@ -1105,6 +1107,31 @@ BinaryExpression
''');
}

test_promotion() async {
await assertNoErrorsInCode(r'''
class C {
int f() => 0;
}
extension E on C {
int g(dynamic d) => 0;
}
void test(C? c) {
E(c)?.g(c); // `c` is promoted to `C` on the RHS of `?.`
}
''');
var node = findNode.simple('c);');
assertResolvedNodeText(node, r'''
SimpleIdentifier
token: c
parameter: <testLibraryFragment>::@extension::E::@method::g::@parameter::d
staticElement: <testLibraryFragment>::@function::test::@parameter::c
element: <testLibraryFragment>::@function::test::@parameter::c#element
staticType: C
''');
}

test_propertyAccess_getter_nullAware() async {
await assertNoErrorsInCode('''
extension E on int {
Expand Down
71 changes: 71 additions & 0 deletions tests/language/extension_methods/null_aware_promotion_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

// This test verifies that a null-aware extension method invocation properly
// promotes its target to non-nullable.

import '../static_type_helper.dart';

class B {
final C? _c;
B(this._c);
}

class C {
void method(Object? o) {}
C operator +(Object? other) => this;
}

extension E on C {
C extensionMethod(Object? o) => this;
C get extensionProperty => this;
set extensionProperty(Object? value) {}
C? get nullableExtensionProperty => this;
set nullableExtensionProperty(Object? value) {}
C operator [](Object? index) => this;
operator []=(Object? index, Object? value) {}
}

extension E2 on C {
C? operator [](Object? index) => this;
operator []=(Object? index, Object? value) {}
}

testVariable(C? c) {
E(c)?.extensionMethod(c..expectStaticType<Exactly<C>>());
E(c)?.extensionProperty.method(c..expectStaticType<Exactly<C>>());
E(c)?.extensionProperty = c..expectStaticType<Exactly<C>>();
E(c)?.extensionProperty += c..expectStaticType<Exactly<C>>();
E(c)?.nullableExtensionProperty ??= c..expectStaticType<Exactly<C>>();
E(c)?[c
..expectStaticType<Exactly<C>>()].method(c..expectStaticType<Exactly<C>>());
E(c)?[c..expectStaticType<Exactly<C>>()] = c..expectStaticType<Exactly<C>>();
E(c)?[c..expectStaticType<Exactly<C>>()] += c..expectStaticType<Exactly<C>>();
E2(c)?[c..expectStaticType<Exactly<C>>()] ??=
c..expectStaticType<Exactly<C>>();
}

testProperty(B b) {
E(b._c)?.extensionMethod(b._c..expectStaticType<Exactly<C>>());
E(b._c)?.extensionProperty.method(b._c..expectStaticType<Exactly<C>>());
E(b._c)?.extensionProperty = b._c..expectStaticType<Exactly<C>>();
E(b._c)?.extensionProperty += b._c..expectStaticType<Exactly<C>>();
E(b._c)?.nullableExtensionProperty ??= b._c..expectStaticType<Exactly<C>>();
E(b._c)?[b._c..expectStaticType<Exactly<C>>()].method(
b._c..expectStaticType<Exactly<C>>(),
);
E(b._c)?[b._c..expectStaticType<Exactly<C>>()] =
b._c..expectStaticType<Exactly<C>>();
E(b._c)?[b._c..expectStaticType<Exactly<C>>()] +=
b._c..expectStaticType<Exactly<C>>();
E2(b._c)?[b._c..expectStaticType<Exactly<C>>()] ??=
b._c..expectStaticType<Exactly<C>>();
}

main() {
for (var value in [null, C()]) {
testVariable(value);
testProperty(B(value));
}
}
79 changes: 79 additions & 0 deletions tests/language/extension_methods/null_aware_reachability_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

// This test verifies that a null-aware extension method invocation is properly
// treated as unreachable when the target has type `Null`.

import '../static_type_helper.dart';

class C {
void method(Object? o) {}
C operator +(Object? other) => this;
}

extension E on Null {
C extensionMethod(Object? o) => C();
C get extensionProperty => C();
set extensionProperty(Object? value) {}
C? get nullableExtensionProperty => C();
set nullableExtensionProperty(Object? value) {}
C operator [](Object? index) => C();
operator []=(Object? index, Object? value) {}
}

extension E2 on C {
C? operator [](Object? index) => C();
operator []=(Object? index, Object? value) {}
}

testLiteralNull() {
int? i = 0; // Promotes to non-null.
i.expectStaticType<Exactly<int>>();
E(null)?.extensionMethod(i = null);
i.expectStaticType<Exactly<int>>();
E(null)?.extensionProperty.method(i = null);
i.expectStaticType<Exactly<int>>();
E(null)?.extensionProperty = i = null;
i.expectStaticType<Exactly<int>>();
E(null)?.extensionProperty += i = null;
i.expectStaticType<Exactly<int>>();
E(null)?.nullableExtensionProperty ??= i = null;
i.expectStaticType<Exactly<int>>();
E(null)?[i = null].method(i = null);
i.expectStaticType<Exactly<int>>();
E(null)?[i = null] = i = null;
i.expectStaticType<Exactly<int>>();
E(null)?[i = null] += i = null;
i.expectStaticType<Exactly<int>>();
E2(null)?[i = null] ??= i = null;
i.expectStaticType<Exactly<int>>();
}

testNullVariable(Null n) {
int? i = 0; // Promotes to non-null.
i.expectStaticType<Exactly<int>>();
E(n)?.extensionMethod(i = null);
i.expectStaticType<Exactly<int>>();
E(n)?.extensionProperty.method(i = null);
i.expectStaticType<Exactly<int>>();
E(n)?.extensionProperty = i = null;
i.expectStaticType<Exactly<int>>();
E(n)?.extensionProperty += i = null;
i.expectStaticType<Exactly<int>>();
E(n)?.nullableExtensionProperty ??= i = null;
i.expectStaticType<Exactly<int>>();
E(n)?[i = null].method(i = null);
i.expectStaticType<Exactly<int>>();
E(n)?[i = null] = i = null;
i.expectStaticType<Exactly<int>>();
E(n)?[i = null] += i = null;
i.expectStaticType<Exactly<int>>();
E2(n)?[i = null] ??= i = null;
i.expectStaticType<Exactly<int>>();
}

main() {
testLiteralNull();
testNullVariable(null);
}

0 comments on commit 2f33461

Please sign in to comment.