Migration: produce diagnostic information for assignments.
We produce a diagnostic under the following conditions: - For compound assignments, if the type read from the LHS is nullable (this is illegal after NNBD, and requires user intervention to fix). - For compound assignments, if the type returned from the combiner is not assignable to the LHS (this was required prior to NNBD, but it is a stricter condition after migration, both because the LHS might have a non-nullable type, and because implicit downcasts are not allowed). - For null-aware assignments, if the type read from the LHS is non-nullable (this indicates that once strong mode is enabled, the assignment will be dead code). Bug: https://github.com/dart-lang/sdk/issues/38676 Change-Id: Icb242ba36437e38364ada069880831eb05e3a513 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/145664 Reviewed-by: Mike Fairhurst <mfairhurst@google.com>
This commit is contained in:
committed by
commit-bot@chromium.org
parent
170d047b20
commit
bf56b2ca2c
@@ -45,6 +45,21 @@ class NullabilityFixDescription {
|
||||
kind: NullabilityFixKind.checkExpressionDueToHint,
|
||||
);
|
||||
|
||||
/// A compound assignment's combiner operator returns a type that isn't
|
||||
/// assignable to the LHS of the assignment.
|
||||
static const compoundAssignmentHasBadCombinedType =
|
||||
const NullabilityFixDescription._(
|
||||
appliedMessage: 'Compound assignment has bad combined type',
|
||||
kind: NullabilityFixKind.compoundAssignmentHasBadCombinedType,
|
||||
);
|
||||
|
||||
/// A compound assignment's LHS has a nullable type.
|
||||
static const compoundAssignmentHasNullableSource =
|
||||
const NullabilityFixDescription._(
|
||||
appliedMessage: 'Compound assignment has nullable source',
|
||||
kind: NullabilityFixKind.compoundAssignmentHasNullableSource,
|
||||
);
|
||||
|
||||
/// Informative message: a condition of an if-test or conditional expression
|
||||
/// will always evaluate to `false` in strong checking mode.
|
||||
static const conditionFalseInStrongMode = const NullabilityFixDescription._(
|
||||
@@ -99,6 +114,14 @@ class NullabilityFixDescription {
|
||||
'Null-aware access will be unnecessary in strong checking mode',
|
||||
kind: NullabilityFixKind.nullAwarenessUnnecessaryInStrongMode);
|
||||
|
||||
/// Informative message: a null-aware assignment won't be necessary in strong
|
||||
/// checking mode.
|
||||
static const nullAwareAssignmentUnnecessaryInStrongMode =
|
||||
const NullabilityFixDescription._(
|
||||
appliedMessage:
|
||||
'Null-aware assignment will be unnecessary in strong checking mode',
|
||||
kind: NullabilityFixKind.nullAwareAssignmentUnnecessaryInStrongMode);
|
||||
|
||||
static const otherCastExpression = const NullabilityFixDescription._(
|
||||
appliedMessage: 'Added a cast to an expression (non-downcast)',
|
||||
kind: NullabilityFixKind.otherCastExpression,
|
||||
@@ -217,12 +240,15 @@ enum NullabilityFixKind {
|
||||
addType,
|
||||
checkExpression,
|
||||
checkExpressionDueToHint,
|
||||
compoundAssignmentHasNullableSource,
|
||||
compoundAssignmentHasBadCombinedType,
|
||||
conditionFalseInStrongMode,
|
||||
conditionTrueInStrongMode,
|
||||
downcastExpression,
|
||||
makeTypeNullable,
|
||||
makeTypeNullableDueToHint,
|
||||
nullAwarenessUnnecessaryInStrongMode,
|
||||
nullAwareAssignmentUnnecessaryInStrongMode,
|
||||
otherCastExpression,
|
||||
removeAs,
|
||||
removeDeadCode,
|
||||
|
||||
@@ -392,6 +392,20 @@ class EditPlanner {
|
||||
return (plan as NodeProducingEditPlan)._getChanges(false);
|
||||
}
|
||||
|
||||
/// Creates a new edit plan that adds an informative message to the given
|
||||
/// [token].
|
||||
///
|
||||
/// The created edit plan should be inserted into the list of inner plans for
|
||||
/// a pass-through plan targeted at the [containingNode]. See [passThrough].
|
||||
EditPlan informativeMessageForToken(AstNode containingNode, Token token,
|
||||
{AtomicEditInfo info}) {
|
||||
return _TokenChangePlan(containingNode, {
|
||||
token.offset: [
|
||||
AtomicEdit.delete(token.lexeme.length, info: info, isInformative: true)
|
||||
]
|
||||
});
|
||||
}
|
||||
|
||||
/// Creates a new edit plan that inserts the text indicated by [edits] at the
|
||||
/// given [offset].
|
||||
///
|
||||
|
||||
@@ -301,6 +301,70 @@ class NodeChangeForAsExpression extends NodeChangeForExpression<AsExpression> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Implementation of [NodeChange] specialized for operating on
|
||||
/// [AssignmentExpression] nodes.
|
||||
class NodeChangeForAssignment
|
||||
extends NodeChangeForExpression<AssignmentExpression> {
|
||||
/// Indicates whether the user should be warned that the assignment is a
|
||||
/// compound assignment with a bad combined type (the return type of the
|
||||
/// combiner isn't assignable to the the write type of the LHS).
|
||||
bool isCompoundAssignmentWithBadCombinedType = false;
|
||||
|
||||
/// Indicates whether the user should be warned that the assignment is a
|
||||
/// compound assignment with a nullable source type.
|
||||
bool isCompoundAssignmentWithNullableSource = false;
|
||||
|
||||
/// Indicates whether the user should be warned that the assignment is a
|
||||
/// null-aware assignment that will have no effect when strong checking is
|
||||
/// enabled.
|
||||
bool isWeakNullAware = false;
|
||||
|
||||
@override
|
||||
Iterable<String> get _toStringParts => [
|
||||
...super._toStringParts,
|
||||
if (isCompoundAssignmentWithBadCombinedType)
|
||||
'isCompoundAssignmentWithBadCombinedType',
|
||||
if (isCompoundAssignmentWithNullableSource)
|
||||
'isCompoundAssignmentWithNullableSource',
|
||||
if (isWeakNullAware) 'isWeakNullAware'
|
||||
];
|
||||
|
||||
@override
|
||||
NodeProducingEditPlan _apply(
|
||||
AssignmentExpression node, FixAggregator aggregator) {
|
||||
var lhsPlan = aggregator.planForNode(node.leftHandSide);
|
||||
EditPlan operatorPlan;
|
||||
if (isCompoundAssignmentWithNullableSource) {
|
||||
operatorPlan = aggregator.planner.informativeMessageForToken(
|
||||
node, node.operator,
|
||||
info: AtomicEditInfo(
|
||||
NullabilityFixDescription.compoundAssignmentHasNullableSource,
|
||||
const {}));
|
||||
} else if (isCompoundAssignmentWithBadCombinedType) {
|
||||
operatorPlan = aggregator.planner.informativeMessageForToken(
|
||||
node, node.operator,
|
||||
info: AtomicEditInfo(
|
||||
NullabilityFixDescription.compoundAssignmentHasBadCombinedType,
|
||||
const {}));
|
||||
} else if (isWeakNullAware) {
|
||||
operatorPlan = aggregator.planner.informativeMessageForToken(
|
||||
node, node.operator,
|
||||
info: AtomicEditInfo(
|
||||
NullabilityFixDescription
|
||||
.nullAwareAssignmentUnnecessaryInStrongMode,
|
||||
const {}));
|
||||
}
|
||||
var rhsPlan = aggregator.planForNode(node.rightHandSide);
|
||||
var innerPlans = <EditPlan>[
|
||||
lhsPlan,
|
||||
if (operatorPlan != null) operatorPlan,
|
||||
rhsPlan
|
||||
];
|
||||
return _applyExpression(aggregator,
|
||||
aggregator.planner.passThrough(node, innerPlans: innerPlans));
|
||||
}
|
||||
}
|
||||
|
||||
/// Implementation of [NodeChange] specialized for operating on
|
||||
/// [CompilationUnit] nodes.
|
||||
class NodeChangeForCompilationUnit extends NodeChange<CompilationUnit> {
|
||||
@@ -832,6 +896,10 @@ class _NodeChangeVisitor extends GeneralizingAstVisitor<NodeChange<AstNode>> {
|
||||
NodeChange visitAsExpression(AsExpression node) =>
|
||||
NodeChangeForAsExpression();
|
||||
|
||||
@override
|
||||
NodeChange visitAssignmentExpression(AssignmentExpression node) =>
|
||||
NodeChangeForAssignment();
|
||||
|
||||
@override
|
||||
NodeChange visitCompilationUnit(CompilationUnit node) =>
|
||||
NodeChangeForCompilationUnit();
|
||||
|
||||
@@ -25,6 +25,7 @@ import 'package:analyzer/src/generated/migration.dart';
|
||||
import 'package:analyzer/src/generated/resolver.dart';
|
||||
import 'package:analyzer/src/generated/source.dart';
|
||||
import 'package:analyzer/src/generated/utilities_dart.dart';
|
||||
import 'package:analyzer/src/task/strong/checker.dart';
|
||||
import 'package:nnbd_migration/fix_reason_target.dart';
|
||||
import 'package:nnbd_migration/instrumentation.dart';
|
||||
import 'package:nnbd_migration/nnbd_migration.dart';
|
||||
@@ -288,6 +289,9 @@ class MigrationResolutionHooksImpl implements MigrationResolutionHooks {
|
||||
|
||||
final Expando<bool> _shouldStayNullAware = Expando();
|
||||
|
||||
final Map<AssignmentExpression, _AssignmentExpressionHandler>
|
||||
_assignmentExpressionHandlers = {};
|
||||
|
||||
FlowAnalysis<AstNode, Statement, Expression, PromotableElement, DartType>
|
||||
_flowAnalysis;
|
||||
|
||||
@@ -423,37 +427,14 @@ class MigrationResolutionHooksImpl implements MigrationResolutionHooks {
|
||||
@override
|
||||
DartType modifyExpressionType(Expression node, DartType type) =>
|
||||
_wrapExceptions(node, () => type, () {
|
||||
var hint =
|
||||
_fixBuilder._variables.getNullCheckHint(_fixBuilder.source, node);
|
||||
if (hint != null) {
|
||||
type = _addNullCheck(node, type,
|
||||
info: AtomicEditInfo(
|
||||
NullabilityFixDescription.checkExpressionDueToHint,
|
||||
{
|
||||
FixReasonTarget.root:
|
||||
FixReason_NullCheckHint(CodeReference.fromAstNode(node))
|
||||
},
|
||||
hintComment: hint),
|
||||
hint: hint);
|
||||
var parent = node.parent;
|
||||
if (parent is AssignmentExpression) {
|
||||
return (_assignmentExpressionHandlers[parent] ??=
|
||||
_AssignmentExpressionHandler(parent))
|
||||
.modifySubexpressionType(this, node, type);
|
||||
} else {
|
||||
return _modifyRValueType(node, type);
|
||||
}
|
||||
if (type.isDynamic) return type;
|
||||
var ancestor = _findNullabilityContextAncestor(node);
|
||||
DartType context = _getNullabilityContext(ancestor);
|
||||
if (!_fixBuilder._typeSystem.isSubtypeOf(type, context)) {
|
||||
// Either a cast or a null check is needed. We prefer to do a null
|
||||
// check if we can.
|
||||
var nonNullType = _fixBuilder._typeSystem.promoteToNonNull(type);
|
||||
if (_fixBuilder._typeSystem.isSubtypeOf(nonNullType, context)) {
|
||||
return _addNullCheck(node, type);
|
||||
} else {
|
||||
return _addCast(node, type, context);
|
||||
}
|
||||
}
|
||||
if (!_fixBuilder._typeSystem.isNullable(type)) return type;
|
||||
if (_needsNullCheckDueToStructure(ancestor)) {
|
||||
return _addNullCheck(node, type);
|
||||
}
|
||||
return type;
|
||||
});
|
||||
|
||||
@override
|
||||
@@ -519,33 +500,40 @@ class MigrationResolutionHooksImpl implements MigrationResolutionHooks {
|
||||
}
|
||||
}
|
||||
|
||||
DartType _getNullabilityContext(Expression node) {
|
||||
var parent = node.parent;
|
||||
if (parent is AssignmentExpression) {
|
||||
var lhs = parent.leftHandSide;
|
||||
if (lhs is SimpleIdentifier) {
|
||||
var lhsElement = lhs.staticElement;
|
||||
if (lhsElement is PromotableElement) {
|
||||
var operatorType = parent.operator.type;
|
||||
switch (operatorType) {
|
||||
case TokenType.EQ:
|
||||
case TokenType.QUESTION_QUESTION_EQ:
|
||||
// When visiting an assignment to a local variable, if the
|
||||
// variable type is promoted, the resolver uses the promoted type
|
||||
// of the variable as the inference context, but it's ok to assign
|
||||
// a different type to the variable (un-doing the promotion). So
|
||||
// for migration purposes, we need to consider the context type to
|
||||
// be the unpromoted type. See
|
||||
// https://github.com/dart-lang/sdk/issues/41411.
|
||||
return lhsElement.type;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
}
|
||||
DartType _modifyRValueType(Expression node, DartType type,
|
||||
{DartType context}) {
|
||||
var hint =
|
||||
_fixBuilder._variables.getNullCheckHint(_fixBuilder.source, node);
|
||||
if (hint != null) {
|
||||
type = _addNullCheck(node, type,
|
||||
info: AtomicEditInfo(
|
||||
NullabilityFixDescription.checkExpressionDueToHint,
|
||||
{
|
||||
FixReasonTarget.root:
|
||||
FixReason_NullCheckHint(CodeReference.fromAstNode(node))
|
||||
},
|
||||
hintComment: hint),
|
||||
hint: hint);
|
||||
}
|
||||
if (type.isDynamic) return type;
|
||||
var ancestor = _findNullabilityContextAncestor(node);
|
||||
context ??=
|
||||
InferenceContext.getContext(ancestor) ?? DynamicTypeImpl.instance;
|
||||
if (!_fixBuilder._typeSystem.isSubtypeOf(type, context)) {
|
||||
// Either a cast or a null check is needed. We prefer to do a null
|
||||
// check if we can.
|
||||
var nonNullType = _fixBuilder._typeSystem.promoteToNonNull(type);
|
||||
if (_fixBuilder._typeSystem.isSubtypeOf(nonNullType, context)) {
|
||||
return _addNullCheck(node, type);
|
||||
} else {
|
||||
return _addCast(node, type, context);
|
||||
}
|
||||
}
|
||||
var context = InferenceContext.getContext(node) ?? DynamicTypeImpl.instance;
|
||||
return context;
|
||||
if (!_fixBuilder._typeSystem.isNullable(type)) return type;
|
||||
if (_needsNullCheckDueToStructure(ancestor)) {
|
||||
return _addNullCheck(node, type);
|
||||
}
|
||||
return type;
|
||||
}
|
||||
|
||||
bool _needsNullCheckDueToStructure(Expression node) {
|
||||
@@ -639,6 +627,110 @@ class NonNullableUnnamedOptionalParameter implements Problem {
|
||||
/// Common supertype for problems reported by [FixBuilder._addProblem].
|
||||
abstract class Problem {}
|
||||
|
||||
/// Data structure keeping track of intermediate results when the fix builder
|
||||
/// is handling an assignment expression.
|
||||
class _AssignmentExpressionHandler {
|
||||
/// The assignment expression in question.
|
||||
final AssignmentExpression node;
|
||||
|
||||
/// For compound and null-aware assignments, the type read from the LHS.
|
||||
/*late final*/ DartType readType;
|
||||
|
||||
/// The type that may be written to the LHS.
|
||||
/*late final*/ DartType writeType;
|
||||
|
||||
/// The type that should be used as a context type when inferring the RHS.
|
||||
DartType rhsContextType;
|
||||
|
||||
_AssignmentExpressionHandler(this.node);
|
||||
|
||||
/// Called after visiting the RHS of the assignment, to verify that for
|
||||
/// compound assignments, the return value of the assignment is assignable to
|
||||
/// [writeType].
|
||||
void handleAssignmentRhs(
|
||||
MigrationResolutionHooksImpl hooks, DartType rhsType) {
|
||||
MethodElement combiner = node.staticElement;
|
||||
if (combiner != null) {
|
||||
var fixBuilder = hooks._fixBuilder;
|
||||
var combinerReturnType =
|
||||
fixBuilder._typeSystem.refineBinaryExpressionType(
|
||||
readType,
|
||||
node.operator.type,
|
||||
rhsType,
|
||||
combiner.returnType,
|
||||
);
|
||||
if (!fixBuilder._typeSystem.isSubtypeOf(combinerReturnType, writeType)) {
|
||||
(fixBuilder._getChange(node) as NodeChangeForAssignment)
|
||||
.isCompoundAssignmentWithBadCombinedType = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Called after visiting the LHS of the assignment. Records the [readType],
|
||||
/// [writeType], and [rhsContextType]. Also verifies that for compound
|
||||
/// assignments, the [readType] is non-nullable, and that for null-aware
|
||||
/// assignments, the [readType] is nullable.
|
||||
void handleLValueType(MigrationResolutionHooksImpl hooks,
|
||||
TokenType operatorType, DartType resolvedType) {
|
||||
assert(resolvedType.nullabilitySuffix != NullabilitySuffix.star);
|
||||
// Provisionally store the resolved type as the type of the lhs, so that
|
||||
// getReadType can fall back on it if necessary.
|
||||
var lhs = node.leftHandSide;
|
||||
lhs.staticType = resolvedType;
|
||||
// The type passed in by the resolver for the LHS of an assignment is the
|
||||
// "write type".
|
||||
var writeType = resolvedType;
|
||||
if (lhs is SimpleIdentifier) {
|
||||
var element = lhs.staticElement;
|
||||
if (element is PromotableElement) {
|
||||
// However, if the LHS is a reference to a local variable that has
|
||||
// been promoted, the resolver passes in the promoted type. We
|
||||
// want to use the variable element's type, so that we consider it
|
||||
// ok to assign a value to the variable that un-does the
|
||||
// promotion. See https://github.com/dart-lang/sdk/issues/41411.
|
||||
writeType = element.type;
|
||||
}
|
||||
}
|
||||
assert(writeType.nullabilitySuffix != NullabilitySuffix.star);
|
||||
this.writeType = writeType;
|
||||
var fixBuilder = hooks._fixBuilder;
|
||||
if (operatorType == TokenType.EQ) {
|
||||
rhsContextType = writeType;
|
||||
} else {
|
||||
readType = getReadType(lhs);
|
||||
assert(readType.nullabilitySuffix != NullabilitySuffix.star);
|
||||
if (operatorType == TokenType.QUESTION_QUESTION_EQ) {
|
||||
rhsContextType = writeType;
|
||||
if (fixBuilder._typeSystem.isNonNullable(readType)) {
|
||||
(fixBuilder._getChange(node) as NodeChangeForAssignment)
|
||||
.isWeakNullAware = true;
|
||||
}
|
||||
} else {
|
||||
if (!readType.isDynamic &&
|
||||
fixBuilder._typeSystem.isPotentiallyNullable(readType)) {
|
||||
(fixBuilder._getChange(node) as NodeChangeForAssignment)
|
||||
.isCompoundAssignmentWithNullableSource = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Called after visiting the LHS or the RHS of the assignment.
|
||||
DartType modifySubexpressionType(MigrationResolutionHooksImpl hooks,
|
||||
Expression subexpression, DartType type) {
|
||||
if (identical(subexpression, node.leftHandSide)) {
|
||||
handleLValueType(hooks, node.operator.type, type);
|
||||
return type;
|
||||
} else {
|
||||
assert(identical(subexpression, node.rightHandSide));
|
||||
type =
|
||||
hooks._modifyRValueType(subexpression, type, context: rhsContextType);
|
||||
handleAssignmentRhs(hooks, type);
|
||||
return type;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Visitor that computes additional migrations on behalf of [FixBuilder] that
|
||||
/// should be run after resolution
|
||||
class _FixBuilderPostVisitor extends GeneralizingAstVisitor<void>
|
||||
|
||||
@@ -186,13 +186,18 @@ class InfoBuilder {
|
||||
// We could add an edit to add a `/*?*/` hint, but the offset is a
|
||||
// little tricky.
|
||||
break;
|
||||
case NullabilityFixKind.nullAwarenessUnnecessaryInStrongMode:
|
||||
case NullabilityFixKind.conditionTrueInStrongMode:
|
||||
case NullabilityFixKind.conditionFalseInStrongMode:
|
||||
case NullabilityFixKind.conditionTrueInStrongMode:
|
||||
case NullabilityFixKind.nullAwarenessUnnecessaryInStrongMode:
|
||||
case NullabilityFixKind.nullAwareAssignmentUnnecessaryInStrongMode:
|
||||
// We don't offer any edits around weak-only code.
|
||||
// TODO(paulberry): offer edits to delete the code that would be dead in
|
||||
// strong mode (https://github.com/dart-lang/sdk/issues/41554).
|
||||
break;
|
||||
case NullabilityFixKind.compoundAssignmentHasBadCombinedType:
|
||||
case NullabilityFixKind.compoundAssignmentHasNullableSource:
|
||||
// We don't offer any edits around bad compound assignments.
|
||||
break;
|
||||
}
|
||||
return edits;
|
||||
}
|
||||
|
||||
@@ -23,9 +23,12 @@ class UnitRenderer {
|
||||
/// "proposed edits" area, in the order in which they should be displayed.
|
||||
@visibleForTesting
|
||||
static const List<NullabilityFixKind> kindPriorityOrder = [
|
||||
NullabilityFixKind.compoundAssignmentHasBadCombinedType,
|
||||
NullabilityFixKind.compoundAssignmentHasNullableSource,
|
||||
NullabilityFixKind.removeDeadCode,
|
||||
NullabilityFixKind.conditionTrueInStrongMode,
|
||||
NullabilityFixKind.conditionFalseInStrongMode,
|
||||
NullabilityFixKind.nullAwareAssignmentUnnecessaryInStrongMode,
|
||||
NullabilityFixKind.nullAwarenessUnnecessaryInStrongMode,
|
||||
NullabilityFixKind.otherCastExpression,
|
||||
NullabilityFixKind.checkExpression,
|
||||
@@ -265,6 +268,12 @@ class UnitRenderer {
|
||||
return '$count null check$s added';
|
||||
case NullabilityFixKind.checkExpressionDueToHint:
|
||||
return '$count null check hint$s converted to null check$s';
|
||||
case NullabilityFixKind.compoundAssignmentHasBadCombinedType:
|
||||
return '$count compound assignment$s could not be migrated (bad '
|
||||
'combined type)';
|
||||
case NullabilityFixKind.compoundAssignmentHasNullableSource:
|
||||
return '$count compound assignment$s could not be migrated (nullable '
|
||||
'source)';
|
||||
case NullabilityFixKind.conditionTrueInStrongMode:
|
||||
return '$count condition$s will be true in strong checking mode';
|
||||
break;
|
||||
@@ -278,6 +287,9 @@ class UnitRenderer {
|
||||
case NullabilityFixKind.nullAwarenessUnnecessaryInStrongMode:
|
||||
return '$count null-aware access$es will be unnecessary in strong '
|
||||
'checking mode';
|
||||
case NullabilityFixKind.nullAwareAssignmentUnnecessaryInStrongMode:
|
||||
return '$count null-aware assignment$s will be unnecessary in strong '
|
||||
'checking mode';
|
||||
case NullabilityFixKind.removeAs:
|
||||
return '$count cast$s now unnecessary';
|
||||
case NullabilityFixKind.removeDeadCode:
|
||||
|
||||
@@ -535,6 +535,25 @@ class EditPlanTest extends AbstractSingleUnitTest {
|
||||
'var x = 0; var y = 0;');
|
||||
}
|
||||
|
||||
Future<void> test_informativeMessageForToken() async {
|
||||
await analyze('f(x) => x + 1;');
|
||||
var sum = findNode.binary('+');
|
||||
var info = _MockInfo();
|
||||
var changes = checkPlan(
|
||||
planner.passThrough(sum, innerPlans: [
|
||||
planner.informativeMessageForToken(sum, sum.operator, info: info)
|
||||
]),
|
||||
'f(x) => x + 1;',
|
||||
expectedIncludingInformative: 'f(x) => x 1;');
|
||||
var expectedOffset = sum.operator.offset;
|
||||
expect(changes.keys, unorderedEquals([expectedOffset]));
|
||||
expect(changes[expectedOffset], hasLength(1));
|
||||
expect(changes[expectedOffset][0].length, '+'.length);
|
||||
expect(changes[expectedOffset][0].replacement, '');
|
||||
expect(changes[expectedOffset][0].isInformative, isTrue);
|
||||
expect(changes[expectedOffset][0].info, same(info));
|
||||
}
|
||||
|
||||
Future<void> test_insertText() async {
|
||||
await analyze('final x = 1;');
|
||||
var variableDeclarationList = findNode.variableDeclarationList('final');
|
||||
@@ -1581,6 +1600,10 @@ g(a, c) => a..b = throw (c..d);
|
||||
}
|
||||
}
|
||||
|
||||
class _MockInfo implements AtomicEditInfo {
|
||||
noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
|
||||
}
|
||||
|
||||
class _PrecedenceChecker extends UnifyingAstVisitor<void> {
|
||||
final EditPlanner planner;
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import 'package:analyzer/dart/element/type.dart';
|
||||
import 'package:analyzer/src/dart/element/element.dart';
|
||||
import 'package:analyzer/src/dart/element/type.dart';
|
||||
import 'package:analyzer/src/dart/element/type_provider.dart';
|
||||
import 'package:nnbd_migration/nnbd_migration.dart';
|
||||
import 'package:nnbd_migration/src/decorated_type.dart';
|
||||
import 'package:nnbd_migration/src/edit_plan.dart';
|
||||
import 'package:nnbd_migration/src/fix_aggregator.dart';
|
||||
@@ -54,6 +55,95 @@ class FixAggregatorTest extends FixAggregatorTestBase {
|
||||
expect(previewInfo.applyTo(code), 'f(a, b) => (a! + b!)!;');
|
||||
}
|
||||
|
||||
Future<void> test_assignment_add_null_check() async {
|
||||
var content = 'f(int x, int y) => x += y;';
|
||||
await analyze(content);
|
||||
var previewInfo = run({
|
||||
findNode.assignment('+='): NodeChangeForAssignment()..addNullCheck(null)
|
||||
});
|
||||
expect(previewInfo.applyTo(code), 'f(int x, int y) => (x += y)!;');
|
||||
}
|
||||
|
||||
Future<void> test_assignment_change_lhs() async {
|
||||
var content = 'f(List<int> x, int y) => x[0] += y;';
|
||||
await analyze(content);
|
||||
var previewInfo = run({
|
||||
findNode.assignment('+='): NodeChangeForAssignment(),
|
||||
findNode.index('[0]').target: NodeChangeForExpression()
|
||||
..addNullCheck(null)
|
||||
});
|
||||
expect(previewInfo.applyTo(code), 'f(List<int> x, int y) => x![0] += y;');
|
||||
}
|
||||
|
||||
Future<void> test_assignment_change_rhs() async {
|
||||
var content = 'f(int x, int y) => x += y;';
|
||||
await analyze(content);
|
||||
var assignment = findNode.assignment('+=');
|
||||
var previewInfo = run({
|
||||
assignment: NodeChangeForAssignment(),
|
||||
assignment.rightHandSide: NodeChangeForExpression()..addNullCheck(null)
|
||||
});
|
||||
expect(previewInfo.applyTo(code), 'f(int x, int y) => x += y!;');
|
||||
}
|
||||
|
||||
Future<void> test_assignment_compound_with_bad_combined_type() async {
|
||||
var content = 'f(int x, int y) => x += y;';
|
||||
await analyze(content);
|
||||
var previewInfo = run({
|
||||
findNode.assignment('+='): NodeChangeForAssignment()
|
||||
..isCompoundAssignmentWithBadCombinedType = true
|
||||
});
|
||||
expect(previewInfo.applyTo(code), content);
|
||||
expect(previewInfo, hasLength(1));
|
||||
var edit = previewInfo[content.indexOf('+=')].single;
|
||||
expect(edit.info.description,
|
||||
NullabilityFixDescription.compoundAssignmentHasBadCombinedType);
|
||||
expect(edit.isInformative, isTrue);
|
||||
expect(edit.length, '+='.length);
|
||||
}
|
||||
|
||||
Future<void> test_assignment_compound_with_nullable_source() async {
|
||||
var content = 'f(int x, int y) => x += y;';
|
||||
await analyze(content);
|
||||
var previewInfo = run({
|
||||
findNode.assignment('+='): NodeChangeForAssignment()
|
||||
..isCompoundAssignmentWithNullableSource = true
|
||||
});
|
||||
expect(previewInfo.applyTo(code), content);
|
||||
expect(previewInfo, hasLength(1));
|
||||
var edit = previewInfo[content.indexOf('+=')].single;
|
||||
expect(edit.info.description,
|
||||
NullabilityFixDescription.compoundAssignmentHasNullableSource);
|
||||
expect(edit.isInformative, isTrue);
|
||||
expect(edit.length, '+='.length);
|
||||
}
|
||||
|
||||
Future<void> test_assignment_introduce_as() async {
|
||||
var content = 'f(int x, int y) => x += y;';
|
||||
await analyze(content);
|
||||
var previewInfo = run({
|
||||
findNode.assignment('+='): NodeChangeForAssignment()
|
||||
..introduceAs(nnbdTypeProvider.intType, null)
|
||||
});
|
||||
expect(previewInfo.applyTo(code), 'f(int x, int y) => (x += y) as int;');
|
||||
}
|
||||
|
||||
Future<void> test_assignment_weak_null_aware() async {
|
||||
var content = 'f(int x, int y) => x ??= y;';
|
||||
await analyze(content);
|
||||
var previewInfo = run({
|
||||
findNode.assignment('??='): NodeChangeForAssignment()
|
||||
..isWeakNullAware = true
|
||||
}, warnOnWeakCode: true);
|
||||
expect(previewInfo.applyTo(code), content);
|
||||
expect(previewInfo, hasLength(1));
|
||||
var edit = previewInfo[content.indexOf('??=')].single;
|
||||
expect(edit.info.description,
|
||||
NullabilityFixDescription.nullAwareAssignmentUnnecessaryInStrongMode);
|
||||
expect(edit.isInformative, isTrue);
|
||||
expect(edit.length, '??='.length);
|
||||
}
|
||||
|
||||
Future<void> test_eliminateDeadIf_changesInKeptCode() async {
|
||||
await analyze('''
|
||||
f(int i, int/*?*/ j) {
|
||||
|
||||
@@ -47,6 +47,18 @@ class FixBuilderTest extends EdgeBuilderTestBase {
|
||||
TypeMatcher<NodeChangeForDefaultFormalParameter>()
|
||||
.having((c) => c.addRequiredKeyword, 'addRequiredKeyword', true);
|
||||
|
||||
static final isCompoundAssignmentNullableSource =
|
||||
TypeMatcher<NodeChangeForAssignment>().having(
|
||||
(c) => c.isCompoundAssignmentWithNullableSource,
|
||||
'isCompoundAssignmentWithNullableSource',
|
||||
true);
|
||||
|
||||
static final isCompoundAssignmentBadCombinedType =
|
||||
TypeMatcher<NodeChangeForAssignment>().having(
|
||||
(c) => c.isCompoundAssignmentWithBadCombinedType,
|
||||
'isCompoundAssignmentWithBadCombinedType',
|
||||
true);
|
||||
|
||||
static final isMakeNullable = TypeMatcher<NodeChangeForTypeAnnotation>()
|
||||
.having((c) => c.makeNullable, 'makeNullable', true)
|
||||
.having((c) => c.nullabilityHint, 'nullabilityHint', isNull);
|
||||
@@ -84,6 +96,10 @@ class FixBuilderTest extends EdgeBuilderTestBase {
|
||||
TypeMatcher<NodeChangeForAnnotation>().having(
|
||||
(c) => c.changeToRequiredKeyword, 'changeToRequiredKeyword', true);
|
||||
|
||||
static final isWeakNullAwareAssignment =
|
||||
TypeMatcher<NodeChangeForAssignment>()
|
||||
.having((c) => c.isWeakNullAware, 'isWeakNullAware', true);
|
||||
|
||||
DartType get dynamicType => postMigrationTypeProvider.dynamicType;
|
||||
|
||||
DartType get objectType => postMigrationTypeProvider.objectType;
|
||||
@@ -349,7 +365,8 @@ _f(bool/*?*/ x, bool/*?*/ y) => x != null && (x ??= y) != null;
|
||||
// On the RHS of the `&&`, `x` is promoted to non-nullable, but it is still
|
||||
// considered to be a nullable assignment target, so no null check is
|
||||
// generated for `y`.
|
||||
visitSubexpression(findNode.binary('&&'), 'bool');
|
||||
visitSubexpression(findNode.binary('&&'), 'bool',
|
||||
changes: {findNode.assignment('??='): isWeakNullAwareAssignment});
|
||||
}
|
||||
|
||||
Future<void>
|
||||
@@ -1047,6 +1064,48 @@ f() => true;
|
||||
visitSubexpression(findNode.booleanLiteral('true'), 'bool');
|
||||
}
|
||||
|
||||
Future<void> test_compound_assignment_nullable_result_bad() async {
|
||||
await analyze('''
|
||||
abstract class C {
|
||||
C/*?*/ operator+(int i);
|
||||
}
|
||||
f(C c) {
|
||||
c += 1;
|
||||
}
|
||||
''');
|
||||
var assignment = findNode.assignment('+=');
|
||||
visitSubexpression(assignment, 'C?',
|
||||
changes: {assignment: isCompoundAssignmentBadCombinedType});
|
||||
}
|
||||
|
||||
Future<void> test_compound_assignment_nullable_result_ok() async {
|
||||
await analyze('''
|
||||
abstract class C {
|
||||
C/*?*/ operator+(int i);
|
||||
}
|
||||
abstract class D {
|
||||
void set x(C/*?*/ value);
|
||||
C/*!*/ get x;
|
||||
f() {
|
||||
x += 1;
|
||||
}
|
||||
}
|
||||
''');
|
||||
var assignment = findNode.assignment('+=');
|
||||
visitSubexpression(assignment, 'C?');
|
||||
}
|
||||
|
||||
Future<void> test_compound_assignment_nullable_source() async {
|
||||
await analyze('''
|
||||
_f(int/*?*/ x) {
|
||||
x += 1;
|
||||
}
|
||||
''');
|
||||
var assignment = findNode.assignment('+=');
|
||||
visitSubexpression(assignment, 'int',
|
||||
changes: {assignment: isCompoundAssignmentNullableSource});
|
||||
}
|
||||
|
||||
Future<void> test_conditionalExpression_dead_else_remove() async {
|
||||
await analyze('_f(int x, int/*?*/ y) => x != null ? x + 1 : y + 1.0;');
|
||||
var expression = findNode.conditionalExpression('x != null');
|
||||
@@ -1814,6 +1873,49 @@ _f(_C/*?*/ c) => c.toString();
|
||||
visitSubexpression(findNode.methodInvocation('c.toString'), 'String');
|
||||
}
|
||||
|
||||
Future<void> test_null_aware_assignment_non_nullable_source() async {
|
||||
await analyze('''
|
||||
abstract class C {
|
||||
int/*!*/ f();
|
||||
g(int/*!*/ x) {
|
||||
x ??= f();
|
||||
}
|
||||
}
|
||||
''');
|
||||
var assignment = findNode.assignment('??=');
|
||||
visitSubexpression(assignment, 'int',
|
||||
changes: {assignment: isWeakNullAwareAssignment});
|
||||
}
|
||||
|
||||
Future<void> test_null_aware_assignment_nullable_rhs_needs_check() async {
|
||||
await analyze('''
|
||||
abstract class C {
|
||||
void set x(int/*!*/ value);
|
||||
int/*?*/ get x;
|
||||
int/*?*/ f();
|
||||
g() {
|
||||
x ??= f();
|
||||
}
|
||||
}
|
||||
''');
|
||||
var assignment = findNode.assignment('??=');
|
||||
visitSubexpression(assignment, 'int',
|
||||
changes: {assignment.rightHandSide: isNullCheck});
|
||||
}
|
||||
|
||||
Future<void> test_null_aware_assignment_nullable_rhs_ok() async {
|
||||
await analyze('''
|
||||
abstract class C {
|
||||
int/*?*/ f();
|
||||
g(int/*?*/ x) {
|
||||
x ??= f();
|
||||
}
|
||||
}
|
||||
''');
|
||||
var assignment = findNode.assignment('??=');
|
||||
visitSubexpression(assignment, 'int?');
|
||||
}
|
||||
|
||||
Future<void> test_nullAssertion_promotes() async {
|
||||
await analyze('''
|
||||
_f(bool/*?*/ x) => x && x;
|
||||
|
||||
@@ -364,6 +364,56 @@ void main() {
|
||||
kind: NullabilityFixKind.addLateDueToTestSetup);
|
||||
}
|
||||
|
||||
Future<void> test_compound_assignment_nullable_result() async {
|
||||
var unit = await buildInfoForSingleTestFile('''
|
||||
abstract class C {
|
||||
C/*?*/ operator+(int i);
|
||||
}
|
||||
void f(C/*!*/ a, int b) {
|
||||
a += b;
|
||||
}
|
||||
''', migratedContent: '''
|
||||
abstract class C {
|
||||
C/*?*/ operator+(int i);
|
||||
}
|
||||
void f(C/*!*/ a, int b) {
|
||||
a += b;
|
||||
}
|
||||
''');
|
||||
var operator = '+=';
|
||||
var operatorOffset = unit.content.indexOf(operator);
|
||||
var region =
|
||||
unit.regions.where((region) => region.offset == operatorOffset).single;
|
||||
assertRegion(
|
||||
region: region,
|
||||
length: operator.length,
|
||||
explanation: 'Compound assignment has bad combined type',
|
||||
kind: NullabilityFixKind.compoundAssignmentHasBadCombinedType,
|
||||
edits: isEmpty);
|
||||
}
|
||||
|
||||
Future<void> test_compound_assignment_nullable_source() async {
|
||||
var unit = await buildInfoForSingleTestFile('''
|
||||
void f(int/*?*/ a, int b) {
|
||||
a += b;
|
||||
}
|
||||
''', migratedContent: '''
|
||||
void f(int/*?*/ a, int b) {
|
||||
a += b;
|
||||
}
|
||||
''');
|
||||
var operator = '+=';
|
||||
var operatorOffset = unit.content.indexOf(operator);
|
||||
var region =
|
||||
unit.regions.where((region) => region.offset == operatorOffset).single;
|
||||
assertRegion(
|
||||
region: region,
|
||||
length: operator.length,
|
||||
explanation: 'Compound assignment has nullable source',
|
||||
kind: NullabilityFixKind.compoundAssignmentHasNullableSource,
|
||||
edits: isEmpty);
|
||||
}
|
||||
|
||||
Future<void> test_conditionFalseInStrongMode_expression() async {
|
||||
var unit = await buildInfoForSingleTestFile(
|
||||
'int f(String s) => s == null ? 0 : s.length;',
|
||||
|
||||
Reference in New Issue
Block a user