Migration: don't add !s to expressions of type Null.

Previously, we had a rule in the migration tool that we didn't add
`!`s to the literal `null`, because such a change would be guaranteed
to fail at runtime; instead we issued a warning to the user that there
is no valid migration for their code, so user intervention is
required.

This change extends the rule to apply to any expression whose type is
`Null`.  Notably, this makes migration of code using Mockito much
easier, because it prevents the migration tool from trying to add a
`!` to expressions like `any`, `anyNamed(...)`, `argThat(...)`,
`captureThat(...)`, etc. (all of which have a return type of `Null`).

Bug: https://b.corp.google.com/issues/200173910
Change-Id: I70ce5da5002e6254397024da5a9e9f69eb04f21c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214313
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry 2021-09-24 13:25:24 +00:00 committed by commit-bot@chromium.org
parent 22652a3b49
commit faa03308f6
4 changed files with 37 additions and 2 deletions

View file

@ -124,7 +124,8 @@ class NullabilityFixDescription {
/// Informative message: there is no valid migration for `null` in a
/// non-nullable context.
static const noValidMigrationForNull = NullabilityFixDescription._(
appliedMessage: 'No valid migration for `null` in a non-nullable context',
appliedMessage: 'No valid migration for expression with type `Null` in '
'a non-nullable context',
kind: NullabilityFixKind.noValidMigrationForNull);
/// Informative message: a null-aware access won't be necessary in strong

View file

@ -635,7 +635,7 @@ class MigrationResolutionHooksImpl
var resultType =
_fixBuilder!._typeSystem.promoteToNonNull(type as TypeImpl);
_flowAnalysis!.nonNullAssert_end(node);
return node is NullLiteral && hint == null
return type.isDartCoreNull && hint == null
? NoValidMigrationChange(resultType)
: NullCheckChange(resultType, hint: hint);
}

View file

@ -6746,6 +6746,24 @@ void f() {
await _checkSingleFileChanges(content, expected);
}
Future<void> test_null_typed_expression_wiithout_valid_migration() async {
var content = '''
void f(int/*!*/ x) {}
void g() {
f(h());
}
Null h() => null;
''';
var expected = '''
void f(int x) {}
void g() {
f(h());
}
Null h() => null;
''';
await _checkSingleFileChanges(content, expected);
}
Future<void> test_nullable_use_of_typedef() async {
var content = '''
typedef F<T> = int Function(T);

View file

@ -2307,6 +2307,22 @@ _f(bool/*?*/ x) => x && x;
changes: {findNode.simple('x &&'): isNullCheck});
}
Future<void> test_nullExpression_noValidMigration() async {
await analyze('''
int/*!*/ f() => g();
Null g() => null;
''');
var invocation = findNode.methodInvocation('g();');
// Note: in spite of the fact that we leave the method invocation alone, we
// analyze it as though it has type `Never`, because it's in a context where
// `null` doesn't work.
visitSubexpression(invocation, 'Never', changes: {
invocation: isNodeChangeForExpression.havingNoValidMigrationWithInfo(
isInfo(NullabilityFixDescription.noValidMigrationForNull,
{FixReasonTarget.root: TypeMatcher<NullabilityEdge>()}))
});
}
Future<void> test_nullLiteral() async {
await analyze('''
f() => null;