Allow non-nullable usage to imply non-null intent.
If a method parameter is used in unconditional control flow in a way
that a `null` value would directly lead to an exception (i.e. by
dereferencing it, or by passing it to a method that requires a
non-nullable value), this is treated as implying that the method
parameter is intended to be non-nullable.
Change-Id: I4f55e4c95b3cfaee0a2ba9367b47d51083e0b7b1
Reviewed-on: https://dart-review.googlesource.com/c/93363
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart b/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
index 417bf77..4beb041 100644
--- a/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
+++ b/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
@@ -441,6 +441,14 @@
} finally {
_guards.removeLast();
}
+ if (!_inConditionalControlFlow && destinationType.nullable == null) {
+ // The destination type can never be nullable so this demonstrates
+ // non-null intent.
+ var nonNullIntent = sourceType.nonNullIntent;
+ if (nonNullIntent != null) {
+ _recordFact(nonNullIntent);
+ }
+ }
}
// TODO(paulberry): it's a cheat to pass in expression=null for the
// recursive checks. Really we want to unify all the checks in a single
diff --git a/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart b/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
index d01508f..1f045cc 100644
--- a/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
+++ b/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
@@ -648,6 +648,19 @@
checkExpression('c.m').nullCheck);
}
+ test_methodInvocation_target_demonstrates_non_null_intent() async {
+ await analyze('''
+class C {
+ void m() {}
+}
+void test(C c) {
+ c.m();
+}
+''');
+
+ assertConstraint([], decoratedTypeAnnotation('C c').nonNullIntent);
+ }
+
test_parenthesizedExpression() async {
await analyze('''
int f() {
diff --git a/pkg/analysis_server/test/src/nullability/provisional_api_test.dart b/pkg/analysis_server/test/src/nullability/provisional_api_test.dart
index cf56fef..2fc92fd 100644
--- a/pkg/analysis_server/test/src/nullability/provisional_api_test.dart
+++ b/pkg/analysis_server/test/src/nullability/provisional_api_test.dart
@@ -108,6 +108,58 @@
await _checkSingleFileChanges(content, expected);
}
+ test_conditional_dereference_does_not_imply_non_null_intent() async {
+ var content = '''
+void f(bool b, int i) {
+ if (b) i.abs();
+}
+void g(bool b, int i) {
+ if (b) f(b, i);
+}
+main() {
+ g(false, null);
+}
+''';
+ var expected = '''
+void f(bool b, int? i) {
+ if (b) i!.abs();
+}
+void g(bool b, int? i) {
+ if (b) f(b, i);
+}
+main() {
+ g(false, null);
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ test_conditional_non_null_usage_does_not_imply_non_null_intent() async {
+ var content = '''
+void f(bool b, int i, int j) {
+ if (b) i.gcd(j);
+}
+void g(bool b, int i, int j) {
+ if (b) f(b, i, j);
+}
+main() {
+ g(false, 0, null);
+}
+''';
+ var expected = '''
+void f(bool b, int i, int? j) {
+ if (b) i.gcd(j!);
+}
+void g(bool b, int i, int? j) {
+ if (b) f(b, i, j);
+}
+main() {
+ g(false, 0, null);
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
test_data_flow_generic_inward() async {
var content = '''
class C<T> {
@@ -631,6 +683,58 @@
''';
await _checkSingleFileChanges(content, expected);
}
+
+ test_unconditional_dereference_implies_non_null_intent() async {
+ var content = '''
+void f(int i) {
+ i.abs();
+}
+void g(bool b, int i) {
+ if (b) f(i);
+}
+main() {
+ g(false, null);
+}
+''';
+ var expected = '''
+void f(int i) {
+ i.abs();
+}
+void g(bool b, int? i) {
+ if (b) f(i!);
+}
+main() {
+ g(false, null);
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ test_unconditional_non_null_usage_implies_non_null_intent() async {
+ var content = '''
+void f(int i, int j) {
+ i.gcd(j);
+}
+void g(bool b, int i, int j) {
+ if (b) f(i, j);
+}
+main() {
+ g(false, 0, null);
+}
+''';
+ var expected = '''
+void f(int i, int j) {
+ i.gcd(j);
+}
+void g(bool b, int i, int? j) {
+ if (b) f(i, j!);
+}
+main() {
+ g(false, 0, null);
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
}
@reflectiveTest
diff --git a/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart b/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
index 94f6ed8..8aa9bd3 100644
--- a/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
+++ b/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
@@ -218,6 +218,8 @@
int operator ~();
+ int gcd(int other);
+
external static int parse(String source,
{int radix, int onError(String source)});
}
