Migration: update FixBuilder to handle simple statements.

We now handle expression statements and variable declaration
statements where the variable type is an interface type.

Change-Id: I00f53a326495bcbeeba785b25c46abf1f3e9ab49
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/121260
Reviewed-by: Mike Fairhurst <mfairhurst@google.com>

Author: stereotype441

pkg/nnbd_migration/lib/src/fix_builder.dart

@@ -14,6 +14,7 @@ import 'package:analyzer/src/dart/element/type_algebra.dart';
 import 'package:analyzer/src/dart/element/type_provider.dart';
 import 'package:analyzer/src/dart/resolver/flow_analysis_visitor.dart';
 import 'package:analyzer/src/generated/resolver.dart';
+import 'package:analyzer/src/generated/source.dart';
 import 'package:front_end/src/fasta/flow_analysis/flow_analysis.dart';
 import 'package:meta/meta.dart';
 import 'package:nnbd_migration/src/decorated_class_hierarchy.dart';
@@ -91,11 +92,18 @@ abstract class FixBuilder extends GeneralizingAstVisitor<DartType> {
   /// inference.  This is used to determine when `!` needs to be inserted.
   DartType _contextType;
 
-  FixBuilder(this._decoratedClassHierarchy, TypeProvider typeProvider,
-      this._typeSystem, this._variables)
+  /// The file being analyzed.
+  final Source source;
+
+  FixBuilder(this.source, this._decoratedClassHierarchy,
+      TypeProvider typeProvider, this._typeSystem, this._variables)
       : _typeProvider = (typeProvider as TypeProviderImpl)
             .withNullability(NullabilitySuffix.none);
 
+  /// Called whenever a type annotation is found for which a `?` needs to be
+  /// inserted.
+  void addNullable(TypeAnnotation node);
+
   /// Called whenever an expression is found for which a `!` needs to be
   /// inserted.
   void addNullCheck(Expression subexpression);
@@ -225,9 +233,9 @@ abstract class FixBuilder extends GeneralizingAstVisitor<DartType> {
   }
 
   @override
-  DartType visitExpression(Expression node) {
-    // Every expression type needs its own visit method.
-    throw UnimplementedError('No visit method for ${node.runtimeType}');
+  DartType visitExpressionStatement(ExpressionStatement node) {
+    visitSubexpression(node.expression, UnknownInferredType.instance);
+    return null;
   }
 
   @override
@@ -243,6 +251,12 @@ abstract class FixBuilder extends GeneralizingAstVisitor<DartType> {
         .withNullability(NullabilitySuffix.none);
   }
 
+  @override
+  DartType visitNode(AstNode node) {
+    // Every node type needs its own visit method.
+    throw UnimplementedError('No visit method for ${node.runtimeType}');
+  }
+
   @override
   DartType visitNullLiteral(NullLiteral node) {
     _flowAnalysis.nullLiteral(node);
@@ -287,6 +301,57 @@ abstract class FixBuilder extends GeneralizingAstVisitor<DartType> {
     }
   }
 
+  @override
+  DartType visitTypeName(TypeName node) {
+    var decoratedType = _variables.decoratedTypeAnnotation(source, node);
+    assert(decoratedType != null);
+    List<DartType> arguments = [];
+    if (node.typeArguments != null) {
+      for (var argument in node.typeArguments.arguments) {
+        arguments.add(argument.accept(this));
+      }
+    }
+    if (decoratedType.type.isDynamic || decoratedType.type.isVoid) {
+      // Already nullable.  Nothing to do.
+      return decoratedType.type;
+    } else {
+      var element = decoratedType.type.element as ClassElement;
+      bool isNullable = decoratedType.node.isNullable;
+      if (isNullable) {
+        addNullable(node);
+      }
+      return InterfaceTypeImpl.explicit(element, arguments,
+          nullabilitySuffix:
+              isNullable ? NullabilitySuffix.question : NullabilitySuffix.none);
+    }
+  }
+
+  @override
+  DartType visitVariableDeclarationList(VariableDeclarationList node) {
+    node.metadata.accept(this);
+    DartType contextType;
+    var typeAnnotation = node.type;
+    if (typeAnnotation != null) {
+      contextType = typeAnnotation.accept(this);
+      assert(contextType != null);
+    } else {
+      contextType = UnknownInferredType.instance;
+    }
+    for (var variable in node.variables) {
+      if (variable.initializer != null) {
+        visitSubexpression(variable.initializer, contextType);
+      }
+    }
+    return null;
+  }
+
+  @override
+  DartType visitVariableDeclarationStatement(
+      VariableDeclarationStatement node) {
+    node.variables.accept(this);
+    return null;
+  }
+
   /// Computes the type that [element] will have after migration.
   ///
   /// If [targetType] is present, and [element] is a class member, it is the

pkg/nnbd_migration/test/fix_builder_test.dart

@@ -8,6 +8,7 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/type_provider.dart';
 import 'package:analyzer/src/generated/resolver.dart';
+import 'package:analyzer/src/generated/source.dart';
 import 'package:analyzer/src/generated/type_system.dart';
 import 'package:nnbd_migration/src/decorated_class_hierarchy.dart';
 import 'package:nnbd_migration/src/fix_builder.dart';
@@ -522,6 +523,16 @@ f() => 1.0;
     visitSubexpression(findNode.doubleLiteral('1.0'), 'double');
   }
 
+  test_expressionStatement() async {
+    await analyze('''
+_f(int/*!*/ x, int/*?*/ y) {
+  x = y;
+}
+''');
+    visitStatement(findNode.statement('x = y'),
+        nullChecked: {findNode.simple('y;')});
+  }
+
   test_integerLiteral() async {
     await analyze('''
 f() => 1;
@@ -649,6 +660,73 @@ f() => #foo;
     visitSubexpression(findNode.symbolLiteral('#foo'), 'Symbol');
   }
 
+  test_typeName_dynamic() async {
+    await analyze('''
+void _f() {
+  dynamic d = null;
+}
+''');
+    visitTypeAnnotation(findNode.typeAnnotation('dynamic'), 'dynamic');
+  }
+
+  test_typeName_generic_nonNullable() async {
+    await analyze('''
+void _f() {
+  List<int> i = [0];
+}
+''');
+    visitTypeAnnotation(findNode.typeAnnotation('List<int>'), 'List<int>');
+  }
+
+  test_typeName_generic_nullable() async {
+    await analyze('''
+void _f() {
+  List<int> i = null;
+}
+''');
+    var listIntAnnotation = findNode.typeAnnotation('List<int>');
+    visitTypeAnnotation(listIntAnnotation, 'List<int>?',
+        nullable: {listIntAnnotation});
+  }
+
+  test_typeName_generic_nullable_arg() async {
+    await analyze('''
+void _f() {
+  List<int> i = [null];
+}
+''');
+    visitTypeAnnotation(findNode.typeAnnotation('List<int>'), 'List<int?>',
+        nullable: {findNode.typeAnnotation('int')});
+  }
+
+  test_typeName_simple_nonNullable() async {
+    await analyze('''
+void _f() {
+  int i = 0;
+}
+''');
+    visitTypeAnnotation(findNode.typeAnnotation('int'), 'int');
+  }
+
+  test_typeName_simple_nullable() async {
+    await analyze('''
+void _f() {
+  int i = null;
+}
+''');
+    var intAnnotation = findNode.typeAnnotation('int');
+    visitTypeAnnotation((intAnnotation), 'int?', nullable: {intAnnotation});
+  }
+
+  test_typeName_void() async {
+    await analyze('''
+void _f() {
+  void v;
+}
+''');
+    visitTypeAnnotation(findNode.typeAnnotation('void v'), 'void');
+  }
+
   test_use_of_dynamic() async {
     // Use of `dynamic` in a context requiring non-null is not explicitly null
     // checked.
@@ -658,6 +736,62 @@ bool _f(dynamic d, bool b) => d && b;
     visitSubexpression(findNode.binary('&&'), 'bool');
   }
 
+  test_variableDeclaration_typed_initialized_nonNullable() async {
+    await analyze('''
+void _f() {
+  int x = 0;
+}
+''');
+    visitStatement(findNode.statement('int x'));
+  }
+
+  test_variableDeclaration_typed_initialized_nullable() async {
+    await analyze('''
+void _f() {
+  int x = null;
+}
+''');
+    visitStatement(findNode.statement('int x'),
+        nullable: {findNode.typeAnnotation('int')});
+  }
+
+  test_variableDeclaration_typed_uninitialized() async {
+    await analyze('''
+void _f() {
+  int x;
+}
+''');
+    visitStatement(findNode.statement('int x'));
+  }
+
+  test_variableDeclaration_untyped_initialized() async {
+    await analyze('''
+void _f() {
+  var x = 0;
+}
+''');
+    visitStatement(findNode.statement('var x'));
+  }
+
+  test_variableDeclaration_untyped_uninitialized() async {
+    await analyze('''
+void _f() {
+  var x;
+}
+''');
+    visitStatement(findNode.statement('var x'));
+  }
+
+  test_variableDeclaration_visit_initializer() async {
+    await analyze('''
+void _f(bool/*?*/ x, bool/*?*/ y) {
+  bool z = x && y;
+}
+''');
+    visitStatement(findNode.statement('bool z'),
+        nullChecked: {findNode.simple('x &&'), findNode.simple('y;')});
+  }
+
   void visitAssignmentTarget(
       Expression node, String expectedReadType, String expectedWriteType,
       {Set<Expression> nullChecked = const <Expression>{},
@@ -672,21 +806,47 @@ bool _f(dynamic d, bool b) => d && b;
     expect(fixBuilder.problems, problems);
   }
 
+  void visitStatement(Statement node,
+      {Set<Expression> nullChecked = const <Expression>{},
+      Map<AstNode, Set<Problem>> problems = const <AstNode, Set<Problem>>{},
+      Set<TypeAnnotation> nullable = const <TypeAnnotation>{}}) {
+    _FixBuilder fixBuilder = _createFixBuilder(node);
+    var type = node.accept(fixBuilder);
+    expect(type, null);
+    expect(fixBuilder.nullCheckedExpressions, nullChecked);
+    expect(fixBuilder.problems, problems);
+    expect(fixBuilder.nullable, nullable);
+  }
+
   void visitSubexpression(Expression node, String expectedType,
       {DartType contextType,
       Set<Expression> nullChecked = const <Expression>{},
-      Map<AstNode, Set<Problem>> problems = const <AstNode, Set<Problem>>{}}) {
+      Map<AstNode, Set<Problem>> problems = const <AstNode, Set<Problem>>{},
+      Set<TypeAnnotation> nullable = const <TypeAnnotation>{}}) {
     contextType ??= dynamicType;
     _FixBuilder fixBuilder = _createFixBuilder(node);
     var type = fixBuilder.visitSubexpression(node, contextType);
     expect((type as TypeImpl).toString(withNullability: true), expectedType);
     expect(fixBuilder.nullCheckedExpressions, nullChecked);
     expect(fixBuilder.problems, problems);
+    expect(fixBuilder.nullable, nullable);
+  }
+
+  void visitTypeAnnotation(TypeAnnotation node, String expectedType,
+      {Set<Expression> nullChecked = const <Expression>{},
+      Map<AstNode, Set<Problem>> problems = const <AstNode, Set<Problem>>{},
+      Set<TypeAnnotation> nullable = const <TypeAnnotation>{}}) {
+    _FixBuilder fixBuilder = _createFixBuilder(node);
+    var type = node.accept(fixBuilder);
+    expect((type as TypeImpl).toString(withNullability: true), expectedType);
+    expect(fixBuilder.nullCheckedExpressions, nullChecked);
+    expect(fixBuilder.problems, problems);
+    expect(fixBuilder.nullable, nullable);
   }
 
-  _FixBuilder _createFixBuilder(Expression node) {
-    var fixBuilder = _FixBuilder(
-        decoratedClassHierarchy, typeProvider, typeSystem, variables);
+  _FixBuilder _createFixBuilder(AstNode node) {
+    var fixBuilder = _FixBuilder(testSource, decoratedClassHierarchy,
+        typeProvider, typeSystem, variables);
     var body = node.thisOrAncestorOfType<FunctionBody>();
     var declaration = body.thisOrAncestorOfType<Declaration>();
     fixBuilder.createFlowAnalysis(declaration, null);
@@ -697,11 +857,20 @@ bool _f(dynamic d, bool b) => d && b;
 class _FixBuilder extends FixBuilder {
   final Set<Expression> nullCheckedExpressions = {};
 
+  final Set<TypeAnnotation> nullable = {};
+
   final Map<AstNode, Set<Problem>> problems = {};
 
-  _FixBuilder(DecoratedClassHierarchy decoratedClassHierarchy,
+  _FixBuilder(Source source, DecoratedClassHierarchy decoratedClassHierarchy,
       TypeProvider typeProvider, TypeSystem typeSystem, Variables variables)
-      : super(decoratedClassHierarchy, typeProvider, typeSystem, variables);
+      : super(source, decoratedClassHierarchy, typeProvider, typeSystem,
+            variables);
+
+  @override
+  void addNullable(TypeAnnotation node) {
+    var newlyAdded = nullable.add(node);
+    expect(newlyAdded, true);
+  }
 
   @override
   void addNullCheck(Expression subexpression) {