Change in dart/sdk[master]: [analyzer] Diagnostics for invalid Future.value and Completer.complete
Simon Binder (Gerrit)
Attention is currently required from: Brian Wilkerson.
Simon Binder would like Brian Wilkerson to review this change.
[analyzer] Diagnostics for invalid Future.value and Completer.complete
When Future.value and Completer.complete are used with a non-nullable
type argument and an argument that is absent or null, a runtime error
occurs.
This adds a diagnostic detecting those invalid usages.
Closes: https://github.com/dart-lang/sdk/issues/45319
Change-Id: Ifd9af70e002c351bf145f4a623105476feb213d9
---
M pkg/analyzer/lib/error/error.dart
M pkg/analyzer/lib/src/dart/error/hint_codes.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
A pkg/analyzer/lib/src/error/null_safe_api_verifier.dart
A pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart
M pkg/analyzer/test/src/diagnostics/test_all.dart
6 files changed, 185 insertions(+), 0 deletions(-)
diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart
index c3bba78..bcc84b7 100644
--- a/pkg/analyzer/lib/error/error.dart
+++ b/pkg/analyzer/lib/error/error.dart
@@ -564,6 +564,7 @@
HintCode.MUST_CALL_SUPER,
HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR,
HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR_USING_NEW,
+ HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE,
HintCode.NULL_AWARE_BEFORE_OPERATOR,
HintCode.NULL_AWARE_IN_CONDITION,
HintCode.NULL_AWARE_IN_LOGICAL_OPERATOR,
diff --git a/pkg/analyzer/lib/src/dart/error/hint_codes.dart b/pkg/analyzer/lib/src/dart/error/hint_codes.dart
index 05e7152..e9dccf7 100644
--- a/pkg/analyzer/lib/src/dart/error/hint_codes.dart
+++ b/pkg/analyzer/lib/src/dart/error/hint_codes.dart
@@ -1661,6 +1661,16 @@
);
/**
+ * Users should not use `Future.value` or `Completer.complete` with a null
+ * argument if the type argument is non-nullable.
+ */
+ static const HintCode NULL_ARGUMENT_TO_NON_NULL_TYPE = HintCode(
+ 'NULL_ARGUMENT_TO_NON_NULL_TYPE',
+ "'{0}' should not be called with a null argument for the non-nullable "
+ "type argument '{1}'",
+ correction: 'Try adding a non-null argument.');
+
+ /**
* When the left operand of a binary expression uses '?.' operator, it can be
* `null`.
*/
diff --git a/pkg/analyzer/lib/src/error/best_practices_verifier.dart b/pkg/analyzer/lib/src/error/best_practices_verifier.dart
index 285d5f7..333cd43 100644
--- a/pkg/analyzer/lib/src/error/best_practices_verifier.dart
+++ b/pkg/analyzer/lib/src/error/best_practices_verifier.dart
@@ -27,6 +27,7 @@
import 'package:analyzer/src/error/deprecated_member_use_verifier.dart';
import 'package:analyzer/src/error/error_handler_verifier.dart';
import 'package:analyzer/src/error/must_call_super_verifier.dart';
+import 'package:analyzer/src/error/null_safe_api_verifier.dart';
import 'package:analyzer/src/generated/constant.dart';
import 'package:analyzer/src/generated/engine.dart';
import 'package:analyzer/src/generated/resolver.dart';
@@ -76,6 +77,8 @@
final ErrorHandlerVerifier _errorHandlerVerifier;
+ final NullSafeApiVerifier _nullSafeApiVerifier;
+
/// The [WorkspacePackage] in which [_currentLibrary] is declared.
final WorkspacePackage? _workspacePackage;
@@ -115,6 +118,7 @@
_mustCallSuperVerifier = MustCallSuperVerifier(_errorReporter),
_errorHandlerVerifier =
ErrorHandlerVerifier(_errorReporter, typeProvider, typeSystem),
+ _nullSafeApiVerifier = NullSafeApiVerifier(_errorReporter, typeSystem),
_workspacePackage = workspacePackage {
_deprecatedVerifier.pushInDeprecatedValue(_currentLibrary.hasDeprecated);
_inDoNotStoreMember = _currentLibrary.hasDoNotStore;
@@ -539,6 +543,7 @@
@override
void visitInstanceCreationExpression(InstanceCreationExpression node) {
_deprecatedVerifier.instanceCreationExpression(node);
+ _nullSafeApiVerifier.instanceCreation(node);
_checkForLiteralConstructorUse(node);
super.visitInstanceCreationExpression(node);
}
@@ -612,6 +617,7 @@
_deprecatedVerifier.methodInvocation(node);
_checkForNullAwareHints(node, node.operator);
_errorHandlerVerifier.verifyMethodInvocation(node);
+ _nullSafeApiVerifier.methodInvocation(node);
super.visitMethodInvocation(node);
}
diff --git a/pkg/analyzer/lib/src/error/null_safe_api_verifier.dart b/pkg/analyzer/lib/src/error/null_safe_api_verifier.dart
new file mode 100644
index 0000000..41cb02b
--- /dev/null
+++ b/pkg/analyzer/lib/src/error/null_safe_api_verifier.dart
@@ -0,0 +1,76 @@
+// Copyright (c) 2021, 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.
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/element/type.dart';
+import 'package:analyzer/error/listener.dart';
+import 'package:analyzer/src/dart/element/type_system.dart';
+import 'package:analyzer/src/dart/error/hint_codes.dart';
+
+/// Verifies usages of `Future.value` and `Completer.complete` when null-safety
+/// is enabled.
+///
+/// `Future.value` and `Completer.complete` both accept a `FutureOr<T>?` as an
+/// optional argument but throw an exception when `T` is non-nullable and `null`
+/// is passed as an argument.
+///
+/// This verifier detects and reports those scenarios.
+class NullSafeApiVerifier {
+ final ErrorReporter _errorReporter;
+ final TypeSystemImpl _typeSystem;
+
+ NullSafeApiVerifier(this._errorReporter, this._typeSystem);
+
+ void _checkTypes(
+ Expression node, String memberName, DartType type, ArgumentList args) {
+ if (args.arguments.length > 1) return;
+
+ final argument = args.arguments.isEmpty ? null : args.arguments.single;
+
+ final typeIsNonNullable = _typeSystem.isNonNullable(type);
+ final argumentIsNull =
+ argument == null || _typeSystem.isNull(argument.staticType!);
+
+ if (typeIsNonNullable && argumentIsNull) {
+ _errorReporter.reportErrorForNode(
+ HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE,
+ argument ?? node,
+ [memberName, type.getDisplayString(withNullability: true)]);
+ }
+ }
+
+ /// Reports an error if the expression creates a `Future<T>.value` with a non-
+ /// nullable value `T` and an argument that is effectively `null`.
+ void instanceCreation(InstanceCreationExpression expression) {
+ if (!_typeSystem.isNonNullableByDefault) return;
+
+ final constructor = expression.constructorName.staticElement;
+ if (constructor == null) return;
+
+ final type = constructor.returnType;
+ final isFutureValue = type.isDartAsyncFuture && constructor.name == 'value';
+
+ if (isFutureValue) {
+ _checkTypes(expression, 'Future.value', type.typeArguments.single,
+ expression.argumentList);
+ }
+ }
+
+ /// Reports an error if `Completer<T>.complete` is invoked with a non-nullable
+ /// `T` and an argument that is effectively `null`.
+ void methodInvocation(MethodInvocation node) {
+ if (!_typeSystem.isNonNullableByDefault) return;
+
+ final targetType = node.realTarget?.staticType;
+ final targetClass = targetType?.element;
+ if (targetClass == null || targetType is! InterfaceType) return;
+
+ if (targetClass.library?.isDartAsync == true &&
+ targetClass.name == 'Completer' &&
+ node.methodName.name == 'complete') {
+ _checkTypes(node, 'Completer.complete', targetType.typeArguments.single,
+ node.argumentList);
+ }
+ }
+}
diff --git a/pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart b/pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart
new file mode 100644
index 0000000..4463e90
--- /dev/null
+++ b/pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart
@@ -0,0 +1,89 @@
+// Copyright (c) 2021, 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.
+
+import 'package:analyzer/src/dart/error/hint_codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+void main() {
+ defineReflectiveSuite(() {
+ defineReflectiveTests(NullArgumentToNonNullTypeTest);
+ });
+}
+
+@reflectiveTest
+class NullArgumentToNonNullTypeTest extends PubPackageResolutionTest {
+ test_completerComplete_nonNullable_absent() async {
+ await assertErrorsInCode('''
+import 'dart:async';
+void f() => Completer<int>().complete();
+''', [
+ error(HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE, 33, 27),
+ ]);
+ }
+
+ test_completerComplete_nonNullable_null() async {
+ await assertErrorsInCode('''
+import 'dart:async';
+void f() => Completer<int>().complete(null);
+''', [
+ error(HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE, 59, 4),
+ ]);
+ }
+
+ test_dynamic() async {
+ await assertNoErrorsInCode('''
+import 'dart:async';
+void f() {
+ Future<int>.value(null as dynamic);
+ Completer<int>().complete(null as dynamic);
+}
+''');
+ }
+
+ test_futureValue_nonNullable_absent() async {
+ await assertErrorsInCode('''
+void foo() => Future<int>.value();
+''', [
+ error(HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE, 14, 19),
+ ]);
+ }
+
+ test_futureValue_nonNullable_null() async {
+ await assertErrorsInCode('''
+void foo() => Future<int>.value(null);
+''', [
+ error(HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE, 32, 4),
+ ]);
+ }
+
+ test_legacy() async {
+ await assertNoErrorsInCode('''
+// @dart=2.9
+import 'dart:async';
+
+void f() {
+ final c = Completer<int>();
+ Future<int>.value();
+ Future<int>.value(null);
+ c.complete();
+ c.complete(null);
+}
+''');
+ }
+
+ test_nullable() async {
+ await assertNoErrorsInCode('''
+import 'dart:async';
+void f() {
+ final c = Completer<int?>();
+ Future<int?>.value();
+ Future<int?>.value(null);
+ c.complete();
+ c.complete(null);
+}
+''');
+ }
+}
diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart
index ef8d0f6..f1a667f 100644
--- a/pkg/analyzer/test/src/diagnostics/test_all.dart
+++ b/pkg/analyzer/test/src/diagnostics/test_all.dart
@@ -496,6 +496,8 @@
import 'not_iterable_spread_test.dart' as not_iterable_spread;
import 'not_map_spread_test.dart' as not_map_spread;
import 'not_null_aware_null_spread_test.dart' as not_null_aware_null_spread;
+import 'null_argument_to_non_null_type_test.dart'
+ as null_argument_to_non_null_type;
import 'null_aware_before_operator_test.dart' as null_aware_before_operator;
import 'null_aware_in_condition_test.dart' as null_aware_in_condition;
import 'null_aware_in_logical_operator_test.dart'
@@ -1031,6 +1033,7 @@
not_iterable_spread.main();
not_map_spread.main();
not_null_aware_null_spread.main();
+ null_argument_to_non_null_type.main();
null_aware_before_operator.main();
null_aware_in_condition.main();
null_aware_in_logical_operator.main();
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Simon Binder (Gerrit)
Attention is currently required from: Brian Wilkerson.
Brian Wilkerson (Gerrit)
Attention is currently required from: Simon Binder.
5 comments:
Patchset:
Thanks!
File pkg/analyzer/lib/src/error/null_safe_api_verifier.dart:
Patch Set #2, Line 31:
typeIsNonNullableConsider exiting early rather than waiting to test the result of this test, something like
```
if (!_typeSystem.isNonNullable(type)) return;
```
Doing so would be slightly more efficient.Patch Set #2, Line 33:
argument.staticType!The `staticType` can be `null` if the code is incomplete or invalid, which is the common case when users are editing code. Therefore, having a null check here will result in exceptions occasionally being thrown. This needs to exit without generating a diagnostic if the static type is not currently known, something like
```
var argumentType = argument.staticType;
if (argumentType == null) return;
```
File pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart:
Patch Set #2, Line 18:
completerCompleteConsider splitting this into two classes, one for tests related to `Completer.complete` and one for those related to `Future.value`.
Patch Set #2, Line 36:
test_dynamicWhether we want to ignore `dynamic` is an interesting question, but I'm fine with doing so for now. We can always change our minds later.
It isn't as clear to me, though, that we want to ignore _all_ nullable types. What about the following:
```
void f(String? s) {
Completer<String>().complete(s);
}
```
Shouldn't the fact that `s` is nullable cause the hint to fire?Or what about this:
```
void f(Null n) {
Completer<String>().complete(n);
}
```
where the only possible value for `n` is `null`?(However we answer those questions, I'd like to have tests to make the decision explicit.)
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Simon Binder (Gerrit)
Attention is currently required from: Brian Wilkerson.
4 comments:
File pkg/analyzer/lib/src/error/null_safe_api_verifier.dart:
Patch Set #2, Line 31:
typeIsNonNullable
Consider exiting early rather than waiting to test the result of this test, something like […]
Done
Patch Set #2, Line 33:
argument.staticType!
The `staticType` can be `null` if the code is incomplete or invalid, which is the common case when u […]
Done
File pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart:
Patch Set #2, Line 18:
completerComplete
Consider splitting this into two classes, one for tests related to `Completer. […]
Done
Patch Set #2, Line 36:
test_dynamic
Whether we want to ignore `dynamic` is an interesting question, but I'm fine with doing so for now. […]
My understanding from https://github.com/dart-lang/linter/issues/2686#issuecomment-852158721 was that diagnostics are for things that can't have false-positives.
Your example with the nullable String is not a guaranteed runtime error, as it may as well be non-null too.
Lasse argued that we should encourage users to add the null assertion in their code and flag all nullable arguments (except for dynamic I guess). If that's the decision we want to make I'm happy to flag all nullable types here.Regardless, we should definitely flag everything that is guaranteed to be null where it's not supposed to. That includes the `Null` type, for which I've added a test.
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Brian Wilkerson (Gerrit)
Attention is currently required from: Simon Binder.
Patch set 3:Commit-Queue +1
1 comment:
File pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart:
Patch Set #2, Line 36:
test_dynamic... diagnostics are for things that can't have false-positives.
Correct.
I was thinking of this as being analogous to something like
```
void f(String? s) {
String s2 = s;
}
```
where we flag the assignment despite the fact that `s` might be non-nullable and so the assignment might be valid. But the reason we can do that is because the spec makes this invalid code. The spec doesn't make either the `Completer.complete` or `Future.value` cases invalid, so I agree that we shouldn't flag them. Thanks for helping me think through it.
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Dart CI (Gerrit)
Attention is currently required from: Simon Binder.
go/dart-cbuild result: FAILURE (REGRESSIONS DETECTED)
Details: https://goto.google.com/dart-cbuild/find/e222d569657bf4b837fd501fb4281d6b7ad8eff6
Bugs: go/dart-cbuild-bug/e222d569657bf4b837fd501fb4281d6b7ad8eff6
Brian Wilkerson (Gerrit)
Attention is currently required from: Simon Binder.
1 comment:
Patchset:
There are a couple of items blocking this from landing.
- The file `analyzer/lib/src/error/null_safe_api_verifier.dart` needs to be sorted (using the "Sort Members in Dart File" menu item).
- There are two uses of `Future.value` without an argument in deprecated code that should probably be marked with an 'ignore' comment:
info - lib/src/dart/analysis/driver.dart:871:14 - 'Future.value' should not be called with a null argument for the non-nullable type argument 'ResolvedLibraryResult' Try adding a non-null argument. - null_argument_to_non_null_type
info - lib/src/dart/analysis/driver.dart:1140:14 - 'Future.value' should not be called with a null argument for the non-nullable type argument 'String' Try adding a non-null argument. - null_argument_to_non_null_type- There are two uses of `Future.value` in Flutter. I don't know what the appropriate resolution of those issues are:
info • 'Future.value' should not be called with a null argument for the non-nullable type argument 'ByteData' • packages/flutter/test/services/default_binary_messenger_test.dart:54:72 • null_argument_to_non_null_type
info - test/store_kit_wrappers/sk_methodchannel_apis_test.dart:177:53 - 'Future.value' should not be called with a null argument for the non-nullable type argument 'Map<String, dynamic>' Try adding a non-null argument. - null_argument_to_non_null_type
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Simon Binder (Gerrit)
1 comment:
Patchset:
There are a couple of items blocking this from landing. […]
I've fixed the issues in the driver and sorted the members.
I've opened two PRs to fix the usages in Flutter (https://github.com/flutter/flutter/pull/84138 and https://github.com/flutter/plugins/pull/4027). I'll ping you once they got through.
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Simon Binder (Gerrit)
Attention is currently required from: Brian Wilkerson.
1 comment:
Patchset:
The usages in Flutter have been fixed, could you start another dry run? Thanks!
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Dart CI (Gerrit)
Attention is currently required from: Brian Wilkerson.
go/dart-cbuild result: FAILURE (REGRESSIONS DETECTED)
Details: https://goto.google.com/dart-cbuild/find/4c6e116d5e64a6a0ea586584356b113b793c3c60
Bugs: go/dart-cbuild-bug/4c6e116d5e64a6a0ea586584356b113b793c3c60
Simon Binder (Gerrit)
1 comment:
Patchset:
Did you have a chance to look at the internal regressions yet? Please let me know if there's anything I can do about them.
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Brian Wilkerson (Gerrit)
Attention is currently required from: Simon Binder.
1 comment:
Patchset:
Sorry, I thought I had fixed the issue, but I missed the notification of the "Dart CI" results telling me that I hadn't. I'll dig some more.
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Dart CI (Gerrit)
Attention is currently required from: Simon Binder.
go/dart-cbuild result: FAILURE (REGRESSIONS DETECTED)
Details: https://goto.google.com/dart-cbuild/find/4c6e116d5e64a6a0ea586584356b113b793c3c60
Bugs: go/dart-cbuild-bug/4c6e116d5e64a6a0ea586584356b113b793c3c60
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Brian Wilkerson (Gerrit)
Attention is currently required from: Simon Binder.
1 comment:
Patchset:
This is still waiting for internal changes before it can land, but hasn't been forgotten.
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Simon Binder (Gerrit)
Patch set 4:Auto-Submit +1
1 comment:
Patchset:
Thanks for resolving the internal usages. Can we land this now?
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Dart CI (Gerrit)
Attention is currently required from: Simon Binder.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/9137086f0effd24f287b88931843d4a999f62294
Brian Wilkerson (Gerrit)
Attention is currently required from: Simon Binder.
Patch set 5:Commit-Queue +2
1 comment:
Patchset:
Thanks for resolving the internal usages.
I had a little trouble getting the internal tests to be re-run, but they just completed successfully, so yes.
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Brian Wilkerson (Gerrit)
Attention is currently required from: Simon Binder.
Patch set 5:Commit-Queue +2
1 comment:
Patchset:
Thanks for your patience while I learned a part of the system I wasn't familiar with.
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Brian Wilkerson (Gerrit)
Attention is currently required from: Simon Binder.
Patch set 5:Code-Review +1Commit-Queue +2
commit-bot@chromium.org (Gerrit)
commi...@chromium.org submitted this change.
[analyzer] Diagnostics for invalid Future.value and Completer.complete
When Future.value and Completer.complete are used with a non-nullable
type argument and an argument that is absent or null, a runtime error
occurs.
This adds a diagnostic detecting those invalid usages.
Closes: https://github.com/dart-lang/sdk/issues/45319
Change-Id: Ifd9af70e002c351bf145f4a623105476feb213d9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202622
Reviewed-by: Brian Wilkerson <brianwi...@google.com>
Commit-Queue: Brian Wilkerson <brianwi...@google.com>
Auto-Submit: Simon Binder <o...@simonbinder.eu>
---
M pkg/analyzer/lib/error/error.dart
M pkg/analyzer/lib/src/dart/analysis/driver.dart
M pkg/analyzer/lib/src/dart/error/hint_codes.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
A pkg/analyzer/lib/src/error/null_safe_api_verifier.dart
A pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart
M pkg/analyzer/test/src/diagnostics/test_all.dart
7 files changed, 237 insertions(+), 1 deletion(-)
diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart
index 9595fbe..52d52dc 100644
--- a/pkg/analyzer/lib/error/error.dart
+++ b/pkg/analyzer/lib/error/error.dart
@@ -567,6 +567,7 @@
HintCode.MUST_CALL_SUPER,
HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR,
HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR_USING_NEW,
+ HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE,
HintCode.NULL_AWARE_BEFORE_OPERATOR,
HintCode.NULL_AWARE_IN_CONDITION,
HintCode.NULL_AWARE_IN_LOGICAL_OPERATOR,
diff --git a/pkg/analyzer/lib/src/dart/analysis/driver.dart b/pkg/analyzer/lib/src/dart/analysis/driver.dart
index 66cfda4..cedb70b 100644
--- a/pkg/analyzer/lib/src/dart/analysis/driver.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/driver.dart
@@ -869,6 +869,7 @@
var result = await getResolvedLibrary2(path);
if (result is NotPathOfUriResult) {
+ // ignore: null_argument_to_non_null_type
return Future.value(); // bug?
}
@@ -1138,7 +1139,7 @@
Future<String> getUnitElementSignature(String path) {
_throwIfNotAbsolutePath(path);
if (!_fsState.hasUri(path)) {
- return Future.value();
+ return Future.value(); // ignore: null_argument_to_non_null_type
}
var completer = Completer<String>();
_unitElementSignatureFiles
diff --git a/pkg/analyzer/lib/src/dart/error/hint_codes.dart b/pkg/analyzer/lib/src/dart/error/hint_codes.dart
index 14038d6..878e7c7 100644
--- a/pkg/analyzer/lib/src/dart/error/hint_codes.dart
+++ b/pkg/analyzer/lib/src/dart/error/hint_codes.dart
@@ -1683,6 +1683,16 @@
);
/**
+ * Users should not use `Future.value` or `Completer.complete` with a null
+ * argument if the type argument is non-nullable.
+ */
+ static const HintCode NULL_ARGUMENT_TO_NON_NULL_TYPE = HintCode(
+ 'NULL_ARGUMENT_TO_NON_NULL_TYPE',
+ "'{0}' should not be called with a null argument for the non-nullable "
+ "type argument '{1}'",
+ correction: 'Try adding a non-null argument.');
+
+ /**
* When the left operand of a binary expression uses '?.' operator, it can be
* `null`.
*/
diff --git a/pkg/analyzer/lib/src/error/best_practices_verifier.dart b/pkg/analyzer/lib/src/error/best_practices_verifier.dart
index 0dcc2f6..a1d8739 100644
--- a/pkg/analyzer/lib/src/error/best_practices_verifier.dart
+++ b/pkg/analyzer/lib/src/error/best_practices_verifier.dart
@@ -27,6 +27,7 @@
import 'package:analyzer/src/error/deprecated_member_use_verifier.dart';
import 'package:analyzer/src/error/error_handler_verifier.dart';
import 'package:analyzer/src/error/must_call_super_verifier.dart';
+import 'package:analyzer/src/error/null_safe_api_verifier.dart';
import 'package:analyzer/src/generated/constant.dart';
import 'package:analyzer/src/generated/engine.dart';
import 'package:analyzer/src/generated/resolver.dart';
@@ -76,6 +77,8 @@
final ErrorHandlerVerifier _errorHandlerVerifier;
+ final NullSafeApiVerifier _nullSafeApiVerifier;
+
/// The [WorkspacePackage] in which [_currentLibrary] is declared.
final WorkspacePackage? _workspacePackage;
@@ -115,6 +118,7 @@
_mustCallSuperVerifier = MustCallSuperVerifier(_errorReporter),
_errorHandlerVerifier =
ErrorHandlerVerifier(_errorReporter, typeProvider, typeSystem),
+ _nullSafeApiVerifier = NullSafeApiVerifier(_errorReporter, typeSystem),
_workspacePackage = workspacePackage {
_deprecatedVerifier.pushInDeprecatedValue(_currentLibrary.hasDeprecated);
_inDoNotStoreMember = _currentLibrary.hasDoNotStore;
@@ -567,6 +571,7 @@
@override
void visitInstanceCreationExpression(InstanceCreationExpression node) {
_deprecatedVerifier.instanceCreationExpression(node);
+ _nullSafeApiVerifier.instanceCreation(node);
_checkForLiteralConstructorUse(node);
super.visitInstanceCreationExpression(node);
}
@@ -644,6 +649,7 @@
_deprecatedVerifier.methodInvocation(node);
_checkForNullAwareHints(node, node.operator);
_errorHandlerVerifier.verifyMethodInvocation(node);
+ _nullSafeApiVerifier.methodInvocation(node);
super.visitMethodInvocation(node);
}
diff --git a/pkg/analyzer/lib/src/error/null_safe_api_verifier.dart b/pkg/analyzer/lib/src/error/null_safe_api_verifier.dart
new file mode 100644
index 0000000..a0308f1
--- /dev/null
+++ b/pkg/analyzer/lib/src/error/null_safe_api_verifier.dart
@@ -0,0 +1,81 @@
+// Copyright (c) 2021, 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.
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/element/type.dart';
+import 'package:analyzer/error/listener.dart';
+import 'package:analyzer/src/dart/element/type_system.dart';
+import 'package:analyzer/src/dart/error/hint_codes.dart';
+
+/// Verifies usages of `Future.value` and `Completer.complete` when null-safety
+/// is enabled.
+///
+/// `Future.value` and `Completer.complete` both accept a `FutureOr<T>?` as an
+/// optional argument but throw an exception when `T` is non-nullable and `null`
+/// is passed as an argument.
+///
+/// This verifier detects and reports those scenarios.
+class NullSafeApiVerifier {
+ final ErrorReporter _errorReporter;
+ final TypeSystemImpl _typeSystem;
+
+ NullSafeApiVerifier(this._errorReporter, this._typeSystem);
+
+ void _checkTypes(
+ Expression node, String memberName, DartType type, ArgumentList args) {
+ // If there's more than one argument, something else is wrong (and will
+ // generate another diagnostic). Also, only check the argument type if we
+ // expect a non-nullable type in the first place.
+ if (args.arguments.length > 1 || !_typeSystem.isNonNullable(type)) return;
+
+ final argument = args.arguments.isEmpty ? null : args.arguments.single;
+ final argumentType = argument?.staticType;
+ // Skip if the type is not currently resolved.
+ if (argument != null && argumentType == null) return;
+
+ final argumentIsNull =
+ argument == null || _typeSystem.isNull(argumentType!);
+
+ if (argumentIsNull) {
+ _errorReporter.reportErrorForNode(
+ HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE,
+ argument ?? node,
+ [memberName, type.getDisplayString(withNullability: true)]);
+ }
+ }
+}
diff --git a/pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart b/pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart
new file mode 100644
index 0000000..5e9409d
--- /dev/null
+++ b/pkg/analyzer/test/src/diagnostics/null_argument_to_non_null_type_test.dart
@@ -0,0 +1,134 @@
+// Copyright (c) 2021, 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.
+
+import 'package:analyzer/src/dart/error/hint_codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+void main() {
+ defineReflectiveSuite(() {
+ defineReflectiveTests(NullArgumentToNonNullCompleterCompleteTest);
+ defineReflectiveTests(NullArgumentToNonNullFutureValueTest);
+ });
+}
+
+@reflectiveTest
+class NullArgumentToNonNullCompleterCompleteTest
+ extends PubPackageResolutionTest {
+ test_absent() async {
+ await assertErrorsInCode('''
+import 'dart:async';
+void f() => Completer<int>().complete();
+''', [
+ error(HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE, 33, 27),
+ ]);
+ }
+
+ test_dynamic() async {
+ await assertNoErrorsInCode('''
+import 'dart:async';
+void f() {
+ Completer<int>().complete(null as dynamic);
+}
+''');
+ }
+
+ test_legacy() async {
+ await assertNoErrorsInCode('''
+// @dart=2.9
+import 'dart:async';
+
+void f() {
+ final c = Completer<int>();
+ c.complete();
+ c.complete(null);
+}
+''');
+ }
+
+ test_null() async {
+ await assertErrorsInCode('''
+import 'dart:async';
+void f() => Completer<int>().complete(null);
+''', [
+ error(HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE, 59, 4),
+ ]);
+ }
+
+ test_nullable() async {
+ await assertNoErrorsInCode('''
+import 'dart:async';
+void f() {
+ final c = Completer<int?>();
+ c.complete();
+ c.complete(null);
+}
+''');
+ }
+
+ test_nullType() async {
+ await assertErrorsInCode('''
+import 'dart:async';
+void f(Null a) => Completer<int>().complete(a);
+''', [
+ error(HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE, 65, 1),
+ ]);
+ }
+}
+
+@reflectiveTest
+class NullArgumentToNonNullFutureValueTest extends PubPackageResolutionTest {
+ test_absent() async {
+ await assertErrorsInCode('''
+void foo() => Future<int>.value();
+''', [
+ error(HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE, 14, 19),
+ ]);
+ }
+
+ test_dynamic() async {
+ await assertNoErrorsInCode('''
+import 'dart:async';
+void f() {
+ Future<int>.value(null as dynamic);
+}
+''');
+ }
+
+ test_legacy() async {
+ await assertNoErrorsInCode('''
+// @dart=2.9
+void f() {
+ Future<int>.value();
+ Future<int>.value(null);
+}
+''');
+ }
+
+ test_null() async {
+ await assertErrorsInCode('''
+void foo() => Future<int>.value(null);
+''', [
+ error(HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE, 32, 4),
+ ]);
+ }
+
+ test_nullable() async {
+ await assertNoErrorsInCode('''
+void f() {
+ Future<int?>.value();
+ Future<int?>.value(null);
+}
+''');
+ }
+
+ test_nullType() async {
+ await assertErrorsInCode('''
+void foo(Null a) => Future<int>.value(a);
+''', [
+ error(HintCode.NULL_ARGUMENT_TO_NON_NULL_TYPE, 38, 1),
+ ]);
+ }
+}
diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart
index 188a40a..8af03ed 100644
--- a/pkg/analyzer/test/src/diagnostics/test_all.dart
+++ b/pkg/analyzer/test/src/diagnostics/test_all.dart
@@ -501,6 +501,8 @@
import 'not_iterable_spread_test.dart' as not_iterable_spread;
import 'not_map_spread_test.dart' as not_map_spread;
import 'not_null_aware_null_spread_test.dart' as not_null_aware_null_spread;
+import 'null_argument_to_non_null_type_test.dart'
+ as null_argument_to_non_null_type;
import 'null_aware_before_operator_test.dart' as null_aware_before_operator;
import 'null_aware_in_condition_test.dart' as null_aware_in_condition;
import 'null_aware_in_logical_operator_test.dart'
@@ -1039,6 +1041,7 @@
not_iterable_spread.main();
not_map_spread.main();
not_null_aware_null_spread.main();
+ null_argument_to_non_null_type.main();
null_aware_before_operator.main();
null_aware_in_condition.main();
null_aware_in_logical_operator.main();
To view, visit change 202622. To unsubscribe, or for help writing mail filters, visit settings.
Dart CI (Gerrit)
go/dart-cbuild result: FAILURE (NO REGRESSIONS DETECTED)
Details: https://goto.google.com/dart-cbuild/find/8fbc2ad8c048717e97dfa5dec400c925db912b57