Improve the feedback for unimplemented portions of the migration tooling
Change-Id: I1d4fd85f15c16e74e4dfdf5a618de2a7f4f3ec9a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106680
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Dan Rubel <danrubel@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/nnbd_migration/lib/src/graph_builder.dart b/pkg/nnbd_migration/lib/src/graph_builder.dart
index 4ced40b..0bd79a9 100644
--- a/pkg/nnbd_migration/lib/src/graph_builder.dart
+++ b/pkg/nnbd_migration/lib/src/graph_builder.dart
@@ -145,7 +145,8 @@
@override
DecoratedType visitAsExpression(AsExpression node) {
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'AsExpression');
}
@override
@@ -166,7 +167,8 @@
@override
DecoratedType visitAssignmentExpression(AssignmentExpression node) {
if (node.operator.type != TokenType.EQ) {
- throw UnimplementedError('TODO(paulberry)');
+ // TODO(paulberry)
+ _unimplemented(node, 'Assignment with operator ${node.operator.lexeme}');
}
var leftType = node.leftHandSide.accept(this);
var conditionalNode = _lastConditionalNode;
@@ -227,8 +229,9 @@
calleeType.positionalParameters[0], node.rightOperand);
return calleeType.returnType;
default:
- assert(false); // TODO(paulberry)
- return null;
+ // TODO(paulberry)
+ _unimplemented(
+ node, 'Binary expression with operator ${node.operator.lexeme}');
}
}
@@ -314,13 +317,15 @@
@override
DecoratedType visitFunctionExpression(FunctionExpression node) {
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'FunctionExpression');
}
@override
DecoratedType visitFunctionExpressionInvocation(
FunctionExpressionInvocation node) {
- throw UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'FunctionExpressionInvocation');
}
@override
@@ -369,7 +374,8 @@
}
var callee = node.staticElement;
if (callee == null) {
- throw new UnimplementedError('TODO(paulberry)');
+ // TODO(paulberry)
+ _unimplemented(node, 'Index expression with no static type');
}
var calleeType = getOrComputeElementType(callee, targetType: targetType);
// TODO(paulberry): substitute if necessary
@@ -389,7 +395,8 @@
if (callee.enclosingElement.typeParameters.isNotEmpty) {
// If the class has type parameters then we might need to substitute the
// appropriate type arguments.
- throw UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'Instance creation expression with type arguments');
}
_handleInvocationArguments(node.argumentList, calleeType);
return calleeType.returnType;
@@ -406,9 +413,11 @@
if (type is NamedType && type.typeArguments != null) {
// TODO(brianwilkerson) Figure out what constraints we need to add to
// allow the tool to decide whether to make the type arguments nullable.
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'Is expression with type arguments');
} else if (type is GenericFunctionType) {
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'Is expression with GenericFunctionType');
}
node.visitChildren(this);
return DecoratedType(node.staticType, _graph.never);
@@ -421,7 +430,8 @@
// TODO(brianwilkerson) We might want to create a fake node in the graph
// to represent the type argument so that we can still create edges from
// the elements to it.
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'List literal with no type arguments');
} else {
var typeArgumentType = _variables.decoratedTypeAnnotation(
_source, node.typeArguments.arguments[0]);
@@ -431,7 +441,8 @@
} else {
// Handle spread and control flow elements.
element.accept(this);
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'Spread or control flow element');
}
}
return DecoratedType(listType, _graph.never,
@@ -469,7 +480,8 @@
}
var callee = node.methodName.staticElement;
if (callee == null) {
- throw UnimplementedError('TODO(paulberry)');
+ // TODO(paulberry)
+ _unimplemented(node, 'Unresolved method name');
}
var calleeType = getOrComputeElementType(callee, targetType: targetType);
// TODO(paulberry): substitute if necessary
@@ -518,13 +530,15 @@
@override
DecoratedType visitPostfixExpression(PostfixExpression node) {
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'PostfixExpression');
}
@override
DecoratedType visitPrefixedIdentifier(PrefixedIdentifier node) {
if (node.prefix.staticElement is ImportElement) {
- throw new UnimplementedError('TODO(paulberry)');
+ // TODO(paulberry)
+ _unimplemented(node, 'PrefixedIdentifier with a prefix');
} else {
return _handlePropertyAccess(node, node.prefix, node.identifier);
}
@@ -538,7 +552,8 @@
return _nonNullableBoolType;
}
// TODO(brianwilkerson) The remaining cases are invocations.
- throw new UnimplementedError('TODO(brianwilkerson)');
+ _unimplemented(
+ node, 'Prefix expression with operator ${node.operator.lexeme}');
}
@override
@@ -565,7 +580,8 @@
// TODO(brianwilkerson) We might want to create fake nodes in the graph to
// represent the type arguments so that we can still create edges from
// the elements to them.
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'Set or map literal with no type arguments');
} else if (typeArguments.length == 1) {
var elementType =
_variables.decoratedTypeAnnotation(_source, typeArguments[0]);
@@ -575,7 +591,8 @@
} else {
// Handle spread and control flow elements.
element.accept(this);
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'Spread or control flow element');
}
}
return DecoratedType(listType, _graph.never,
@@ -592,13 +609,16 @@
} else {
// Handle spread and control flow elements.
element.accept(this);
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'Spread or control flow element');
}
}
return DecoratedType(listType, _graph.never,
typeArguments: [keyType, valueType]);
} else {
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(
+ node, 'Set or map literal with more than two type arguments');
}
}
@@ -612,7 +632,8 @@
return _nonNullableTypeType;
} else {
// TODO(paulberry)
- throw new UnimplementedError('${staticElement.runtimeType}');
+ _unimplemented(node,
+ 'Simple identifier with a static element of type ${staticElement.runtimeType}');
}
}
@@ -685,7 +706,8 @@
var destinationType = getOrComputeElementType(node.declaredElement);
var initializer = node.initializer;
if (initializer == null) {
- throw UnimplementedError('TODO(paulberry)');
+ // TODO(paulberry)
+ _unimplemented(node, 'Variable declaration with no initializer');
} else {
_handleAssignment(destinationType, initializer);
}
@@ -769,13 +791,15 @@
var name = expression.name.label.name;
var parameterType = calleeType.namedParameters[name];
if (parameterType == null) {
- throw UnimplementedError('TODO(paulberry)');
+ // TODO(paulberry)
+ _unimplemented(expression, 'Missing type for named parameter');
}
_handleAssignment(parameterType, expression.expression);
suppliedNamedParameters.add(name);
} else {
if (calleeType.positionalParameters.length <= i) {
- throw UnimplementedError('TODO(paulberry)');
+ // TODO(paulberry)
+ _unimplemented(argumentList, 'Missing positional parameter at $i');
}
_handleAssignment(calleeType.positionalParameters[i++], expression);
}
@@ -800,7 +824,8 @@
}
var callee = propertyName.staticElement;
if (callee == null) {
- throw new UnimplementedError('TODO(paulberry)');
+ // TODO(paulberry)
+ _unimplemented(node, 'Unresolved property access');
}
var calleeType = getOrComputeElementType(callee, targetType: targetType);
// TODO(paulberry): substitute if necessary
@@ -837,7 +862,9 @@
case TokenType.QUESTION_PERIOD:
return true;
default:
- throw new UnimplementedError('TODO(paulberry)');
+ // TODO(paulberry)
+ _unimplemented(
+ expression, 'Conditional expression with operator ${token.lexeme}');
}
}
@@ -865,6 +892,21 @@
return false;
}
+ @alwaysThrows
+ void _unimplemented(AstNode node, String message) {
+ CompilationUnit unit = node.root as CompilationUnit;
+ StringBuffer buffer = StringBuffer();
+ buffer.write(message);
+ buffer.write(' in "');
+ buffer.write(node.toSource());
+ buffer.write('" on line ');
+ buffer.write(unit.lineInfo.getLocation(node.offset).lineNumber);
+ buffer.write(' of "');
+ buffer.write(unit.declaredElement.source.fullName);
+ buffer.write('"');
+ throw UnimplementedError(buffer.toString());
+ }
+
void _unionDecoratedTypes(
DecoratedType x, DecoratedType y, EdgeOrigin origin) {
_graph.union(x.node, y.node, origin);
@@ -876,7 +918,8 @@
y.positionalParameters.isNotEmpty ||
x.namedParameters.isNotEmpty ||
y.namedParameters.isNotEmpty) {
- throw UnimplementedError('TODO(paulberry): _unionDecoratedTypes($x, $y)');
+ // TODO(paulberry)
+ throw UnimplementedError('_unionDecoratedTypes($x, $y, $origin)');
}
}
}
diff --git a/pkg/nnbd_migration/lib/src/node_builder.dart b/pkg/nnbd_migration/lib/src/node_builder.dart
index aa833d7..202fe0a 100644
--- a/pkg/nnbd_migration/lib/src/node_builder.dart
+++ b/pkg/nnbd_migration/lib/src/node_builder.dart
@@ -10,6 +10,7 @@
import 'package:analyzer/src/generated/resolver.dart';
import 'package:analyzer/src/generated/source.dart';
import 'package:front_end/src/scanner/token.dart';
+import 'package:meta/meta.dart';
import 'package:nnbd_migration/nnbd_migration.dart';
import 'package:nnbd_migration/src/conditional_discard.dart';
import 'package:nnbd_migration/src/decorated_type.dart';
@@ -69,7 +70,8 @@
if (node.factoryKeyword != null) {
// Factory constructors can return null, but we don't want to propagate a
// null type if we can prove that null is never returned.
- throw UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'Declaration of a factory constructor');
}
_handleExecutableDeclaration(
node.declaredElement, null, node.parameters, node.body, node);
@@ -93,7 +95,8 @@
@override
DecoratedType visitFieldFormalParameter(FieldFormalParameter node) {
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'FieldFormalParameter');
}
@override
@@ -117,13 +120,15 @@
@override
DecoratedType visitFunctionTypeAlias(FunctionTypeAlias node) {
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'FunctionTypeAlias');
}
@override
DecoratedType visitFunctionTypedFormalParameter(
FunctionTypedFormalParameter node) {
- throw new UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(node, 'FunctionTypedFormalParameter');
}
@override
@@ -197,7 +202,8 @@
if (node is GenericFunctionType) {
returnType = decorateType(node.returnType, node);
if (node.typeParameters != null) {
- throw UnimplementedError('TODO(paulberry)');
+ // TODO(paulberry)
+ _unimplemented(node, 'Generic function type with type parameters');
}
positionalParameters = <DecoratedType>[];
namedParameters = <String, DecoratedType>{};
@@ -281,7 +287,9 @@
typeArguments:
type.typeArguments.map(_decorateImplicitTypeArgument).toList());
}
- throw UnimplementedError('TODO(paulberry): ${type.runtimeType}');
+ // TODO(paulberry)
+ throw UnimplementedError(
+ '_decorateImplicitTypeArgument(${type.runtimeType})');
}
/// Common handling of function and method declarations.
@@ -298,11 +306,15 @@
if (declaredElement.isFactory) {
// Factory constructors can return null, but we don't want to propagate
// a null type if we can prove that null is never returned.
- throw UnimplementedError('TODO(brianwilkerson)');
+ // TODO(brianwilkerson)
+ _unimplemented(
+ parameters.parent, 'Declaration of a factory constructor');
}
if (declaredElement.enclosingElement.typeParameters.isNotEmpty) {
// Need to decorate the type parameters appropriately.
- throw new UnimplementedError('TODO(paulberry,brianwilkerson)');
+ // TODO(paulberry,brianwilkerson)
+ _unimplemented(parameters.parent,
+ 'Declaration of a constructor with type parameters');
}
decoratedReturnType = new DecoratedType(
declaredElement.enclosingElement.type, _graph.never);
@@ -327,6 +339,21 @@
}
_variables.recordDecoratedElementType(declaredElement, functionType);
}
+
+ @alwaysThrows
+ void _unimplemented(AstNode node, String message) {
+ CompilationUnit unit = node.root as CompilationUnit;
+ StringBuffer buffer = StringBuffer();
+ buffer.write(message);
+ buffer.write(' in "');
+ buffer.write(node.toSource());
+ buffer.write('" on line ');
+ buffer.write(unit.lineInfo.getLocation(node.offset).lineNumber);
+ buffer.write(' of "');
+ buffer.write(unit.declaredElement.source.fullName);
+ buffer.write('"');
+ throw UnimplementedError(buffer.toString());
+ }
}
/// Repository of constraint variables and decorated types corresponding to the
