A value from Angular's 'injector.get' in setUp signals non-null intent.
For b/254330485
Change-Id: I5a3993753dfe559426ce0517c1527eba27221759
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/265120
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
diff --git a/pkg/nnbd_migration/lib/instrumentation.dart b/pkg/nnbd_migration/lib/instrumentation.dart
index 270028e..6734bd8 100644
--- a/pkg/nnbd_migration/lib/instrumentation.dart
+++ b/pkg/nnbd_migration/lib/instrumentation.dart
@@ -252,6 +252,7 @@
alwaysNullableType,
angularAnnotation,
argumentErrorCheckNotNull,
+ assignmentFromAngularInjectorGet,
builtValueNullableAnnotation,
callTearOff,
compoundAssignment,
diff --git a/pkg/nnbd_migration/lib/src/edge_builder.dart b/pkg/nnbd_migration/lib/src/edge_builder.dart
index 6988917..3d1f378 100644
--- a/pkg/nnbd_migration/lib/src/edge_builder.dart
+++ b/pkg/nnbd_migration/lib/src/edge_builder.dart
@@ -489,11 +489,14 @@
}
}
+ var isInjectorGetAssignment = _isInjectorGetCall(node.rightHandSide);
+
var expressionType = _handleAssignment(node.rightHandSide,
assignmentExpression: node,
compoundOperatorInfo: isCompound ? node : null,
questionAssignNode: isQuestionAssign ? node : null,
- sourceIsSetupCall: sourceIsSetupCall);
+ sourceIsSetupCall: sourceIsSetupCall,
+ isInjectorGetAssignment: isInjectorGetAssignment);
var conditionalNode = _conditionalNodes[node.leftHandSide];
if (conditionalNode != null) {
expressionType = expressionType!.withNode(
@@ -2461,14 +2464,15 @@
/// proper time.
///
/// Set [wrapFuture] to true to handle assigning Future<flatten(T)> to R.
- DecoratedType? _handleAssignment(Expression? expression,
+ DecoratedType? _handleAssignment(Expression expression,
{DecoratedType? destinationType,
AssignmentExpression? assignmentExpression,
AssignmentExpression? compoundOperatorInfo,
AssignmentExpression? questionAssignNode,
bool fromDefaultValue = false,
bool wrapFuture = false,
- bool sourceIsSetupCall = false}) {
+ bool sourceIsSetupCall = false,
+ bool isInjectorGetAssignment = false}) {
assert(
(assignmentExpression == null) != (destinationType == null),
'Either assignmentExpression or destinationType should be supplied, '
@@ -2490,6 +2494,14 @@
destinationType = _dispatch(destinationExpression);
}
}
+ if (sourceIsSetupCall &&
+ isInjectorGetAssignment &&
+ destinationType != null) {
+ _graph.makeNonNullable(
+ destinationType.node,
+ AssignmentFromAngularInjectorGetOrigin(
+ source, assignmentExpression!.leftHandSide as SimpleIdentifier));
+ }
if (questionAssignNode != null) {
_guards.add(destinationType!.node);
@@ -2504,7 +2516,7 @@
}
if (sourceType == null) {
throw StateError('No type computed for ${expression.runtimeType} '
- '(${expression!.toSource()}) offset=${expression.offset}');
+ '(${expression.toSource()}) offset=${expression.offset}');
}
EdgeOrigin edgeOrigin = _makeEdgeOrigin(sourceType, expression,
isSetupAssignment: sourceIsSetupCall);
@@ -2525,7 +2537,7 @@
_checkAssignment(edgeOrigin, FixReasonTarget.root,
source: sourceType,
destination: compoundOperatorType.positionalParameters![0],
- hard: _shouldUseHardEdge(expression!),
+ hard: _shouldUseHardEdge(expression),
sourceIsFunctionLiteral: expression is FunctionExpression);
sourceType = _fixNumericTypes(compoundOperatorType.returnType!,
compoundOperatorInfo.staticType);
@@ -2544,7 +2556,7 @@
expression, edgeOrigin, sourceType, destinationType!)) {
// Nothing further to do.
} else {
- var hard = _shouldUseHardEdge(expression!,
+ var hard = _shouldUseHardEdge(expression,
isConditionallyExecuted: questionAssignNode != null);
_checkAssignment(edgeOrigin, FixReasonTarget.root,
source: sourceType,
@@ -3090,7 +3102,7 @@
var suppliedNamedParameters = <String>{};
for (var argument in arguments) {
String? name;
- Expression? expression;
+ Expression expression;
if (argument is NamedExpression) {
name = argument.name.label.name;
expression = argument.expression;
@@ -3099,7 +3111,7 @@
name = argument.name!.lexeme;
}
// TODO(scheglov) This is a hack.
- expression = (argument as FormalParameterImpl).identifierForMigration;
+ expression = (argument as FormalParameterImpl).identifierForMigration!;
} else {
expression = argument as Expression;
}
@@ -3364,7 +3376,7 @@
}
}
- EdgeOrigin _makeEdgeOrigin(DecoratedType sourceType, Expression? expression,
+ EdgeOrigin _makeEdgeOrigin(DecoratedType sourceType, Expression expression,
{bool isSetupAssignment = false}) {
if (sourceType.type!.isDynamic) {
return DynamicAssignmentOrigin(source, expression);
@@ -3373,7 +3385,7 @@
source, expression, ExpressionChecks(),
isSetupAssignment: isSetupAssignment);
_variables.recordExpressionChecks(
- source, expression!, expressionChecksOrigin);
+ source, expression, expressionChecksOrigin);
return expressionChecksOrigin;
}
}
@@ -3645,6 +3657,38 @@
return null;
}
}
+
+ /// Whether `expression` is a call to Angular's [Injector.get], with exactly
+ /// one argument.
+ static bool _isInjectorGetCall(Expression expression) {
+ if (expression is! MethodInvocation) {
+ return false;
+ }
+
+ if (expression.methodName.name != 'get') {
+ return false;
+ }
+ if (expression.argumentList.arguments.length != 1) {
+ // If a second argument is passed, which may indicate that a non-null
+ // value isn't necessarily expected, don't count this call.
+ return false;
+ }
+
+ var target = expression.target;
+ if (target is! Identifier) {
+ return false;
+ }
+
+ var receiver = target.staticType?.element;
+ if (receiver?.name != 'Injector') {
+ return false;
+ }
+
+ var uri = receiver?.library?.source.uri;
+ return uri != null &&
+ uri.isScheme('package') &&
+ uri.path.startsWith('angular/');
+ }
}
/// Implementation of [_checkAssignment] for [EdgeBuilder].
diff --git a/pkg/nnbd_migration/lib/src/edge_origin.dart b/pkg/nnbd_migration/lib/src/edge_origin.dart
index 0db2344..9e94536 100644
--- a/pkg/nnbd_migration/lib/src/edge_origin.dart
+++ b/pkg/nnbd_migration/lib/src/edge_origin.dart
@@ -96,6 +96,31 @@
EdgeOriginKind get kind => EdgeOriginKind.argumentErrorCheckNotNull;
}
+/// Edge origin resulting from an assignment from a call to Angular's
+/// `Injector.get` in a test `setUp` callback.
+///
+/// For example, in the following code snippet:
+///
+/// Foo foo;
+/// setUp(() {
+/// foo = injector.get(Foo);
+/// });
+///
+/// this class is used for the edge connecting the type of the `foo` variable to
+/// `never`, due to the assignment from `injector.get`. While `Injector.get` has
+/// a return type of `dynamic`, it's use in a test `setUp` callback is more than
+/// likely intended to yield a non-null value.
+class AssignmentFromAngularInjectorGetOrigin extends EdgeOrigin {
+ AssignmentFromAngularInjectorGetOrigin(
+ super.source, SimpleIdentifier super.node);
+
+ @override
+ String get description => 'value retrieved from Injector.get in test setUp';
+
+ @override
+ EdgeOriginKind get kind => EdgeOriginKind.assignmentFromAngularInjectorGet;
+}
+
/// Edge origin resulting from a use of built_value's `@nullable` annotation.
class BuiltValueNullableOrigin extends EdgeOrigin {
BuiltValueNullableOrigin(super.source, AstNode super.node);
diff --git a/pkg/nnbd_migration/test/abstract_context.dart b/pkg/nnbd_migration/test/abstract_context.dart
index b30e6a0..445d194 100644
--- a/pkg/nnbd_migration/test/abstract_context.dart
+++ b/pkg/nnbd_migration/test/abstract_context.dart
@@ -71,6 +71,9 @@
class ViewChildren {
const ViewChildren(Object selector, {Object? read});
}
+class Injector {
+ dynamic get(Object token, [Object? notFoundValue]) => null;
+}
''');
if (internalUris) {
addPackageFile('angular', 'angular.dart', '''
diff --git a/pkg/nnbd_migration/test/api_test.dart b/pkg/nnbd_migration/test/api_test.dart
index 170f10f..7aa818d 100644
--- a/pkg/nnbd_migration/test/api_test.dart
+++ b/pkg/nnbd_migration/test/api_test.dart
@@ -9151,6 +9151,92 @@
await _checkSingleFileChanges(content, expected);
}
+ Future<void> test_testVariable_assignedInjectorGet() async {
+ addAngularPackage();
+ addTestCorePackage();
+ var content = '''
+import 'package:angular/angular.dart';
+import 'package:test/test.dart';
+void main() {
+ int i;
+ setUp(() {
+ var injector = Injector();
+ i = injector.get(int);
+ });
+ test('a', () {
+ i.isEven;
+ });
+}
+''';
+ var expected = '''
+import 'package:angular/angular.dart';
+import 'package:test/test.dart';
+void main() {
+ late int i;
+ setUp(() {
+ var injector = Injector();
+ i = injector.get(int);
+ });
+ test('a', () {
+ i.isEven;
+ });
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ Future<void> test_testVariable_assignedInjectorGet_inDeclaration() async {
+ addAngularPackage();
+ addTestCorePackage();
+ var content = '''
+import 'package:angular/angular.dart';
+import 'package:test/test.dart';
+void main() {
+ setUp(() {
+ var injector = Injector();
+ int i = injector.get(int);
+ i.isEven;
+ });
+}
+''';
+ var expected = '''
+import 'package:angular/angular.dart';
+import 'package:test/test.dart';
+void main() {
+ setUp(() {
+ var injector = Injector();
+ int i = injector.get(int);
+ i.isEven;
+ });
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ Future<void> test_testVariable_assignedInjectorGet_outsideSetup() async {
+ addAngularPackage();
+ addTestCorePackage();
+ var content = '''
+import 'package:angular/angular.dart';
+void main() {
+ int i;
+ var injector = Injector();
+ i = injector.get(int);
+ i.isEven;
+}
+''';
+ var expected = '''
+import 'package:angular/angular.dart';
+void main() {
+ int? i;
+ var injector = Injector();
+ i = injector.get(int);
+ i!.isEven;
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
Future<void> test_testVariable_assignedNullableValue() async {
addTestCorePackage();
var content = '''
