Skip to content

Commit 837d28b

Browse files
bwilkersoncommit-bot@chromium.org
bwilkerson
authored and
[email protected]
committed
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 <[email protected]> Reviewed-by: Paul Berry <[email protected]> Reviewed-by: Dan Rubel <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]>
1 parent a421022 commit 837d28b

2 files changed

Lines changed: 105 additions & 35 deletions

File tree

‎pkg/nnbd_migration/lib/src/graph_builder.dart‎

Lines changed: 70 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,8 @@ class GraphBuilder extends GeneralizingAstVisitor<DecoratedType> {
145145

146146
@override
147147
DecoratedType visitAsExpression(AsExpression node) {
148-
throw new UnimplementedError('TODO(brianwilkerson)');
148+
// TODO(brianwilkerson)
149+
_unimplemented(node, 'AsExpression');
149150
}
150151

151152
@override
@@ -166,7 +167,8 @@ class GraphBuilder extends GeneralizingAstVisitor<DecoratedType> {
166167
@override
167168
DecoratedType visitAssignmentExpression(AssignmentExpression node) {
168169
if (node.operator.type != TokenType.EQ) {
169-
throw UnimplementedError('TODO(paulberry)');
170+
// TODO(paulberry)
171+
_unimplemented(node, 'Assignment with operator ${node.operator.lexeme}');
170172
}
171173
var leftType = node.leftHandSide.accept(this);
172174
var conditionalNode = _lastConditionalNode;
@@ -227,8 +229,9 @@ class GraphBuilder extends GeneralizingAstVisitor<DecoratedType> {
227229
calleeType.positionalParameters[0], node.rightOperand);
228230
return calleeType.returnType;
229231
default:
230-
assert(false); // TODO(paulberry)
231-
return null;
232+
// TODO(paulberry)
233+
_unimplemented(
234+
node, 'Binary expression with operator ${node.operator.lexeme}');
232235
}
233236
}
234237

@@ -314,13 +317,15 @@ class GraphBuilder extends GeneralizingAstVisitor<DecoratedType> {
314317

315318
@override
316319
DecoratedType visitFunctionExpression(FunctionExpression node) {
317-
throw new UnimplementedError('TODO(brianwilkerson)');
320+
// TODO(brianwilkerson)
321+
_unimplemented(node, 'FunctionExpression');
318322
}
319323

320324
@override
321325
DecoratedType visitFunctionExpressionInvocation(
322326
FunctionExpressionInvocation node) {
323-
throw UnimplementedError('TODO(brianwilkerson)');
327+
// TODO(brianwilkerson)
328+
_unimplemented(node, 'FunctionExpressionInvocation');
324329
}
325330

326331
@override
@@ -369,7 +374,8 @@ class GraphBuilder extends GeneralizingAstVisitor<DecoratedType> {
369374
}
370375
var callee = node.staticElement;
371376
if (callee == null) {
372-
throw new UnimplementedError('TODO(paulberry)');
377+
// TODO(paulberry)
378+
_unimplemented(node, 'Index expression with no static type');
373379
}
374380
var calleeType = getOrComputeElementType(callee, targetType: targetType);
375381
// TODO(paulberry): substitute if necessary
@@ -389,7 +395,8 @@ class GraphBuilder extends GeneralizingAstVisitor<DecoratedType> {
389395
if (callee.enclosingElement.typeParameters.isNotEmpty) {
390396
// If the class has type parameters then we might need to substitute the
391397
// appropriate type arguments.
392-
throw UnimplementedError('TODO(brianwilkerson)');
398+
// TODO(brianwilkerson)
399+
_unimplemented(node, 'Instance creation expression with type arguments');
393400
}
394401
_handleInvocationArguments(node.argumentList, calleeType);
395402
return calleeType.returnType;
@@ -406,9 +413,11 @@ class GraphBuilder extends GeneralizingAstVisitor<DecoratedType> {
406413
if (type is NamedType && type.typeArguments != null) {
407414
// TODO(brianwilkerson) Figure out what constraints we need to add to
408415
// allow the tool to decide whether to make the type arguments nullable.
409-
throw new UnimplementedError('TODO(brianwilkerson)');
416+
// TODO(brianwilkerson)
417+
_unimplemented(node, 'Is expression with type arguments');
410418
} else if (type is GenericFunctionType) {
411-
throw new UnimplementedError('TODO(brianwilkerson)');
419+
// TODO(brianwilkerson)
420+
_unimplemented(node, 'Is expression with GenericFunctionType');
412421
}
413422
node.visitChildren(this);
414423
return DecoratedType(node.staticType, _graph.never);
@@ -421,7 +430,8 @@ class GraphBuilder extends GeneralizingAstVisitor<DecoratedType> {
421430
// TODO(brianwilkerson) We might want to create a fake node in the graph
422431
// to represent the type argument so that we can still create edges from
423432
// the elements to it.
424-
throw new UnimplementedError('TODO(brianwilkerson)');
433+
// TODO(brianwilkerson)
434+
_unimplemented(node, 'List literal with no type arguments');
425435
} else {
426436
var typeArgumentType = _variables.decoratedTypeAnnotation(
427437
_source, node.typeArguments.arguments[0]);
@@ -431,7 +441,8 @@ class GraphBuilder extends GeneralizingAstVisitor<DecoratedType> {
431441
} else {
432442
// Handle spread and control flow elements.
433443
element.accept(this);
434-
throw new UnimplementedError('TODO(brianwilkerson)');
444+
// TODO(brianwilkerson)
445+
_unimplemented(node, 'Spread or control flow element');
435446
}
436447
}
437448
return DecoratedType(listType, _graph.never,
@@ -469,7 +480,8 @@ class GraphBuilder extends GeneralizingAstVisitor<DecoratedType> {
469480
}
470481
var callee = node.methodName.staticElement;
471482
if (callee == null) {
472-
throw UnimplementedError('TODO(paulberry)');
483+
// TODO(paulberry)
484+
_unimplemented(node, 'Unresolved method name');
473485
}
474486
var calleeType = getOrComputeElementType(callee, targetType: targetType);
475487
// TODO(paulberry): substitute if necessary
@@ -518,13 +530,15 @@ $stackTrace''');
518530

519531
@override
520532
DecoratedType visitPostfixExpression(PostfixExpression node) {
521-
throw new UnimplementedError('TODO(brianwilkerson)');
533+
// TODO(brianwilkerson)
534+
_unimplemented(node, 'PostfixExpression');
522535
}
523536

524537
@override
525538
DecoratedType visitPrefixedIdentifier(PrefixedIdentifier node) {
526539
if (node.prefix.staticElement is ImportElement) {
527-
throw new UnimplementedError('TODO(paulberry)');
540+
// TODO(paulberry)
541+
_unimplemented(node, 'PrefixedIdentifier with a prefix');
528542
} else {
529543
return _handlePropertyAccess(node, node.prefix, node.identifier);
530544
}
@@ -538,7 +552,8 @@ $stackTrace''');
538552
return _nonNullableBoolType;
539553
}
540554
// TODO(brianwilkerson) The remaining cases are invocations.
541-
throw new UnimplementedError('TODO(brianwilkerson)');
555+
_unimplemented(
556+
node, 'Prefix expression with operator ${node.operator.lexeme}');
542557
}
543558

544559
@override
@@ -565,7 +580,8 @@ $stackTrace''');
565580
// TODO(brianwilkerson) We might want to create fake nodes in the graph to
566581
// represent the type arguments so that we can still create edges from
567582
// the elements to them.
568-
throw new UnimplementedError('TODO(brianwilkerson)');
583+
// TODO(brianwilkerson)
584+
_unimplemented(node, 'Set or map literal with no type arguments');
569585
} else if (typeArguments.length == 1) {
570586
var elementType =
571587
_variables.decoratedTypeAnnotation(_source, typeArguments[0]);
@@ -575,7 +591,8 @@ $stackTrace''');
575591
} else {
576592
// Handle spread and control flow elements.
577593
element.accept(this);
578-
throw new UnimplementedError('TODO(brianwilkerson)');
594+
// TODO(brianwilkerson)
595+
_unimplemented(node, 'Spread or control flow element');
579596
}
580597
}
581598
return DecoratedType(listType, _graph.never,
@@ -592,13 +609,16 @@ $stackTrace''');
592609
} else {
593610
// Handle spread and control flow elements.
594611
element.accept(this);
595-
throw new UnimplementedError('TODO(brianwilkerson)');
612+
// TODO(brianwilkerson)
613+
_unimplemented(node, 'Spread or control flow element');
596614
}
597615
}
598616
return DecoratedType(listType, _graph.never,
599617
typeArguments: [keyType, valueType]);
600618
} else {
601-
throw new UnimplementedError('TODO(brianwilkerson)');
619+
// TODO(brianwilkerson)
620+
_unimplemented(
621+
node, 'Set or map literal with more than two type arguments');
602622
}
603623
}
604624

@@ -612,7 +632,8 @@ $stackTrace''');
612632
return _nonNullableTypeType;
613633
} else {
614634
// TODO(paulberry)
615-
throw new UnimplementedError('${staticElement.runtimeType}');
635+
_unimplemented(node,
636+
'Simple identifier with a static element of type ${staticElement.runtimeType}');
616637
}
617638
}
618639

@@ -685,7 +706,8 @@ $stackTrace''');
685706
var destinationType = getOrComputeElementType(node.declaredElement);
686707
var initializer = node.initializer;
687708
if (initializer == null) {
688-
throw UnimplementedError('TODO(paulberry)');
709+
// TODO(paulberry)
710+
_unimplemented(node, 'Variable declaration with no initializer');
689711
} else {
690712
_handleAssignment(destinationType, initializer);
691713
}
@@ -769,13 +791,15 @@ $stackTrace''');
769791
var name = expression.name.label.name;
770792
var parameterType = calleeType.namedParameters[name];
771793
if (parameterType == null) {
772-
throw UnimplementedError('TODO(paulberry)');
794+
// TODO(paulberry)
795+
_unimplemented(expression, 'Missing type for named parameter');
773796
}
774797
_handleAssignment(parameterType, expression.expression);
775798
suppliedNamedParameters.add(name);
776799
} else {
777800
if (calleeType.positionalParameters.length <= i) {
778-
throw UnimplementedError('TODO(paulberry)');
801+
// TODO(paulberry)
802+
_unimplemented(argumentList, 'Missing positional parameter at $i');
779803
}
780804
_handleAssignment(calleeType.positionalParameters[i++], expression);
781805
}
@@ -800,7 +824,8 @@ $stackTrace''');
800824
}
801825
var callee = propertyName.staticElement;
802826
if (callee == null) {
803-
throw new UnimplementedError('TODO(paulberry)');
827+
// TODO(paulberry)
828+
_unimplemented(node, 'Unresolved property access');
804829
}
805830
var calleeType = getOrComputeElementType(callee, targetType: targetType);
806831
// TODO(paulberry): substitute if necessary
@@ -837,7 +862,9 @@ $stackTrace''');
837862
case TokenType.QUESTION_PERIOD:
838863
return true;
839864
default:
840-
throw new UnimplementedError('TODO(paulberry)');
865+
// TODO(paulberry)
866+
_unimplemented(
867+
expression, 'Conditional expression with operator ${token.lexeme}');
841868
}
842869
}
843870

@@ -865,6 +892,21 @@ $stackTrace''');
865892
return false;
866893
}
867894

895+
@alwaysThrows
896+
void _unimplemented(AstNode node, String message) {
897+
CompilationUnit unit = node.root as CompilationUnit;
898+
StringBuffer buffer = StringBuffer();
899+
buffer.write(message);
900+
buffer.write(' in "');
901+
buffer.write(node.toSource());
902+
buffer.write('" on line ');
903+
buffer.write(unit.lineInfo.getLocation(node.offset).lineNumber);
904+
buffer.write(' of "');
905+
buffer.write(unit.declaredElement.source.fullName);
906+
buffer.write('"');
907+
throw UnimplementedError(buffer.toString());
908+
}
909+
868910
void _unionDecoratedTypes(
869911
DecoratedType x, DecoratedType y, EdgeOrigin origin) {
870912
_graph.union(x.node, y.node, origin);
@@ -876,7 +918,8 @@ $stackTrace''');
876918
y.positionalParameters.isNotEmpty ||
877919
x.namedParameters.isNotEmpty ||
878920
y.namedParameters.isNotEmpty) {
879-
throw UnimplementedError('TODO(paulberry): _unionDecoratedTypes($x, $y)');
921+
// TODO(paulberry)
922+
throw UnimplementedError('_unionDecoratedTypes($x, $y, $origin)');
880923
}
881924
}
882925
}

‎pkg/nnbd_migration/lib/src/node_builder.dart‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'package:analyzer/src/dart/element/type.dart';
1010
import 'package:analyzer/src/generated/resolver.dart';
1111
import 'package:analyzer/src/generated/source.dart';
1212
import 'package:front_end/src/scanner/token.dart';
13+
import 'package:meta/meta.dart';
1314
import 'package:nnbd_migration/nnbd_migration.dart';
1415
import 'package:nnbd_migration/src/conditional_discard.dart';
1516
import 'package:nnbd_migration/src/decorated_type.dart';
@@ -69,7 +70,8 @@ class NodeBuilder extends GeneralizingAstVisitor<DecoratedType> {
6970
if (node.factoryKeyword != null) {
7071
// Factory constructors can return null, but we don't want to propagate a
7172
// null type if we can prove that null is never returned.
72-
throw UnimplementedError('TODO(brianwilkerson)');
73+
// TODO(brianwilkerson)
74+
_unimplemented(node, 'Declaration of a factory constructor');
7375
}
7476
_handleExecutableDeclaration(
7577
node.declaredElement, null, node.parameters, node.body, node);
@@ -93,7 +95,8 @@ class NodeBuilder extends GeneralizingAstVisitor<DecoratedType> {
9395

9496
@override
9597
DecoratedType visitFieldFormalParameter(FieldFormalParameter node) {
96-
throw new UnimplementedError('TODO(brianwilkerson)');
98+
// TODO(brianwilkerson)
99+
_unimplemented(node, 'FieldFormalParameter');
97100
}
98101

99102
@override
@@ -117,13 +120,15 @@ class NodeBuilder extends GeneralizingAstVisitor<DecoratedType> {
117120

118121
@override
119122
DecoratedType visitFunctionTypeAlias(FunctionTypeAlias node) {
120-
throw new UnimplementedError('TODO(brianwilkerson)');
123+
// TODO(brianwilkerson)
124+
_unimplemented(node, 'FunctionTypeAlias');
121125
}
122126

123127
@override
124128
DecoratedType visitFunctionTypedFormalParameter(
125129
FunctionTypedFormalParameter node) {
126-
throw new UnimplementedError('TODO(brianwilkerson)');
130+
// TODO(brianwilkerson)
131+
_unimplemented(node, 'FunctionTypedFormalParameter');
127132
}
128133

129134
@override
@@ -197,7 +202,8 @@ $stackTrace''');
197202
if (node is GenericFunctionType) {
198203
returnType = decorateType(node.returnType, node);
199204
if (node.typeParameters != null) {
200-
throw UnimplementedError('TODO(paulberry)');
205+
// TODO(paulberry)
206+
_unimplemented(node, 'Generic function type with type parameters');
201207
}
202208
positionalParameters = <DecoratedType>[];
203209
namedParameters = <String, DecoratedType>{};
@@ -281,7 +287,9 @@ $stackTrace''');
281287
typeArguments:
282288
type.typeArguments.map(_decorateImplicitTypeArgument).toList());
283289
}
284-
throw UnimplementedError('TODO(paulberry): ${type.runtimeType}');
290+
// TODO(paulberry)
291+
throw UnimplementedError(
292+
'_decorateImplicitTypeArgument(${type.runtimeType})');
285293
}
286294

287295
/// Common handling of function and method declarations.
@@ -298,11 +306,15 @@ $stackTrace''');
298306
if (declaredElement.isFactory) {
299307
// Factory constructors can return null, but we don't want to propagate
300308
// a null type if we can prove that null is never returned.
301-
throw UnimplementedError('TODO(brianwilkerson)');
309+
// TODO(brianwilkerson)
310+
_unimplemented(
311+
parameters.parent, 'Declaration of a factory constructor');
302312
}
303313
if (declaredElement.enclosingElement.typeParameters.isNotEmpty) {
304314
// Need to decorate the type parameters appropriately.
305-
throw new UnimplementedError('TODO(paulberry,brianwilkerson)');
315+
// TODO(paulberry,brianwilkerson)
316+
_unimplemented(parameters.parent,
317+
'Declaration of a constructor with type parameters');
306318
}
307319
decoratedReturnType = new DecoratedType(
308320
declaredElement.enclosingElement.type, _graph.never);
@@ -327,6 +339,21 @@ $stackTrace''');
327339
}
328340
_variables.recordDecoratedElementType(declaredElement, functionType);
329341
}
342+
343+
@alwaysThrows
344+
void _unimplemented(AstNode node, String message) {
345+
CompilationUnit unit = node.root as CompilationUnit;
346+
StringBuffer buffer = StringBuffer();
347+
buffer.write(message);
348+
buffer.write(' in "');
349+
buffer.write(node.toSource());
350+
buffer.write('" on line ');
351+
buffer.write(unit.lineInfo.getLocation(node.offset).lineNumber);
352+
buffer.write(' of "');
353+
buffer.write(unit.declaredElement.source.fullName);
354+
buffer.write('"');
355+
throw UnimplementedError(buffer.toString());
356+
}
330357
}
331358

332359
/// Repository of constraint variables and decorated types corresponding to the

0 commit comments

Comments
 (0)