Migration: account for generic bounds when determining whether null checks are necessary.

Previously, if a variable with a generic type was null-checked, and
the migration engine did not add a question mark to the type of the
variable itself, e.g.:

    f<T extends Object?>(T x) {
      if (x == null) { ... }
    }

Then the migration tool would erroneously consider the null check
unnecessary.  It failed to account for the fact that the bound on the
generic type might be nullable.

This change fixes the tool so that it now properly accounts for the
bound on the generic type variable.

Bug: https://b.corp.google.com/issues/247496662
Change-Id: I2b131e75b7e3fc5db0b668f1064340d404442c82
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/264501
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
This commit is contained in:
Paul Berry 2022-10-19 19:44:20 +00:00 committed by Commit Queue
parent b8b81d4d19
commit 5349d8c602
2 changed files with 62 additions and 3 deletions

View file

@ -538,12 +538,22 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
notEqual: notEqual);
void buildNullConditionInfo(NullLiteral nullLiteral,
Expression otherOperand, NullabilityNode? otherNode) {
Expression otherOperand, DecoratedType otherType) {
assert(nullLiteral != otherOperand);
// TODO(paulberry): only set falseChecksNonNull in unconditional
// control flow
// TODO(paulberry): figure out what the rules for isPure should be.
bool isPure = otherOperand is SimpleIdentifier;
var otherNode = otherType.node;
var otherTypeType = otherType.type;
if (otherTypeType is TypeParameterType) {
var boundNullability =
DecoratedTypeParameterBounds.current!.get(otherTypeType.element);
if (boundNullability != null) {
otherNode =
NullabilityNode.forLUB(otherNode, boundNullability.node);
}
}
var conditionInfo = _ConditionInfo(node,
isPure: isPure,
postDominatingIntent: _isReferenceInScope(otherOperand),
@ -553,9 +563,9 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
}
if (rightOperand is NullLiteral) {
buildNullConditionInfo(rightOperand, leftOperand, leftType.node);
buildNullConditionInfo(rightOperand, leftOperand, leftType);
} else if (leftOperand is NullLiteral) {
buildNullConditionInfo(leftOperand, rightOperand, rightType.node);
buildNullConditionInfo(leftOperand, rightOperand, rightType);
}
return _makeNonNullableBoolType(node);

View file

@ -8154,6 +8154,55 @@ void main() {
await _checkSingleFileChanges(content, expected);
}
Future<void> test_nullTestOnGenericType_implicitBound() async {
var content = '''
void f<T>(T x, T y) {
if (x == null) return;
if (y == null) return;
}
g() => f(1, null);
''';
var expected = '''
void f<T>(T x, T y) {
if (x == null) return;
if (y == null) return;
}
g() => f(1, null);
''';
await _checkSingleFileChanges(content, expected);
}
Future<void> test_nullTestOnGenericType_nonNullableBound() async {
var content = '''
void f<T extends Object/*!*/>(T x, T y) {
if (x == null) return;
if (y == null) return;
}
''';
var expected = '''
void f<T extends Object>(T x, T y) {}
''';
await _checkSingleFileChanges(content, expected);
}
Future<void> test_nullTestOnGenericType_nullableBound() async {
var content = '''
void f<T extends Object>(T x, T y) {
if (x == null) return;
if (y == null) return;
}
g() => f(1, null);
''';
var expected = '''
void f<T extends Object?>(T x, T y) {
if (x == null) return;
if (y == null) return;
}
g() => f(1, null);
''';
await _checkSingleFileChanges(content, expected);
}
Future<void> test_operator_eq_with_inferred_parameter_type() async {
var content = '''
class C {