Add a quick fix to add a null check operator

Change-Id: Ie85ab236015057c2cb0255cc1548605e5b01e908
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/156783
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: bwilkerson

pkg/analysis_server/lib/src/services/correction/dart/add_null_check.dart

@@ -0,0 +1,70 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analysis_server/src/services/correction/dart/abstract_producer.dart';
+import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analyzer/dart/analysis/features.dart';
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/precedence.dart';
+import 'package:analyzer/dart/element/type.dart';
+import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+
+class AddNullCheck extends CorrectionProducer {
+  @override
+  FixKind get fixKind => DartFixKind.ADD_NULL_CHECK;
+
+  @override
+  Future<void> compute(ChangeBuilder builder) async {
+    if (!unit.featureSet.isEnabled(Feature.non_nullable)) {
+      // Don't suggest a feature that isn't supported.
+      return;
+    }
+    Expression target;
+    if (coveredNode is Expression) {
+      target = coveredNode;
+    } else {
+      return;
+    }
+
+    var fromType = target.staticType;
+    if (fromType == typeProvider.nullType) {
+      // Adding a null check after an explicit `null` is pointless.
+      return;
+    }
+    DartType toType;
+    var parent = target.parent;
+    if (parent is AssignmentExpression && target == parent.rightHandSide) {
+      toType = parent.leftHandSide.staticType;
+    } else if (parent is VariableDeclaration && target == parent.initializer) {
+      toType = parent.declaredElement.type;
+    } else if (parent is ArgumentList) {
+      toType = target.staticParameterElement.type;
+    } else {
+      return;
+    }
+    if (!typeSystem.isAssignableTo(
+        toType, typeSystem.promoteToNonNull(fromType))) {
+      // The reason that `fromType` can't be assigned to `toType` is more than
+      // just because it's nullable, in which case a null check won't fix the
+      // problem.
+      return;
+    }
+    var needsParentheses = target.precedence < Precedence.postfix;
+    await builder.addDartFileEdit(file, (builder) {
+      if (needsParentheses) {
+        builder.addSimpleInsertion(target.offset, '(');
+      }
+      builder.addInsertion(target.end, (builder) {
+        if (needsParentheses) {
+          builder.write(')');
+        }
+        builder.write('!');
+      });
+    });
+  }
+
+  /// Return an instance of this class. Used as a tear-off in `FixProcessor`.
+  static AddNullCheck newInstance() => AddNullCheck();
+}

pkg/analysis_server/lib/src/services/correction/fix.dart

@@ -176,6 +176,8 @@ class DartFixKind {
       "Add required argument '{0}'");
   static const ADD_NE_NULL = FixKind('dart.fix.add.neNull', 50, 'Add != null',
       appliedTogetherMessage: 'Add != null everywhere in file');
+  static const ADD_NULL_CHECK =
+      FixKind('dart.fix.add.nullCheck', 50, 'Add a null check (!)');
   static const ADD_OVERRIDE =
       FixKind('dart.fix.add.override', 50, "Add '@override' annotation");
   static const ADD_REQUIRED =

pkg/analysis_server/lib/src/services/correction/fix_internal.dart

@@ -21,6 +21,7 @@ import 'package:analysis_server/src/services/correction/dart/add_missing_paramet
 import 'package:analysis_server/src/services/correction/dart/add_missing_parameter_named.dart';
 import 'package:analysis_server/src/services/correction/dart/add_missing_required_argument.dart';
 import 'package:analysis_server/src/services/correction/dart/add_ne_null.dart';
+import 'package:analysis_server/src/services/correction/dart/add_null_check.dart';
 import 'package:analysis_server/src/services/correction/dart/add_override.dart';
 import 'package:analysis_server/src/services/correction/dart/add_required.dart';
 import 'package:analysis_server/src/services/correction/dart/add_required_keyword.dart';
@@ -591,6 +592,7 @@ class FixProcessor extends BaseProcessor {
       MakeVariableNotFinal.newInstance,
     ],
     CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE: [
+      AddNullCheck.newInstance,
       WrapInText.newInstance,
     ],
     CompileTimeErrorCode.ASYNC_FOR_IN_WRONG_CONTEXT: [
@@ -673,6 +675,7 @@ class FixProcessor extends BaseProcessor {
     ],
     CompileTimeErrorCode.INVALID_ASSIGNMENT: [
       AddExplicitCast.newInstance,
+      AddNullCheck.newInstance,
       ChangeTypeAnnotation.newInstance,
     ],
     CompileTimeErrorCode.INVOCATION_OF_NON_FUNCTION_EXPRESSION: [

pkg/analysis_server/test/src/services/correction/fix/add_null_check_test.dart

@@ -0,0 +1,116 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analyzer/src/dart/analysis/experiments.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import 'fix_processor.dart';
+
+void main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(AddNullCheckTest);
+  });
+}
+
+@reflectiveTest
+class AddNullCheckTest extends FixProcessorTest {
+  @override
+  List<String> get experiments => [EnableString.non_nullable];
+
+  @override
+  FixKind get kind => DartFixKind.ADD_NULL_CHECK;
+
+  Future<void> test_argument() async {
+    await resolveTestUnit('''
+void f(int x) {}
+void g(int? y) {
+  f(y);
+}
+''');
+    await assertHasFix('''
+void f(int x) {}
+void g(int? y) {
+  f(y!);
+}
+''');
+  }
+
+  Future<void> test_argument_differByMoreThanNullability() async {
+    await resolveTestUnit('''
+void f(int x) {}
+void g(String y) {
+  f(y);
+}
+''');
+    await assertNoFix();
+  }
+
+  Future<void> test_assignment() async {
+    await resolveTestUnit('''
+void f(int x, int? y) {
+  x = y;
+}
+''');
+    await assertHasFix('''
+void f(int x, int? y) {
+  x = y!;
+}
+''');
+  }
+
+  Future<void> test_assignment_differByMoreThanNullability() async {
+    await resolveTestUnit('''
+void f(int x, String y) {
+  x = y;
+}
+''');
+    await assertNoFix();
+  }
+
+  Future<void> test_assignment_needsParens() async {
+    await resolveTestUnit('''
+void f(A x) {
+  x = x + x;
+}
+class A {
+  A? operator +(A a) => null;
+}
+''');
+    await assertHasFix('''
+void f(A x) {
+  x = (x + x)!;
+}
+class A {
+  A? operator +(A a) => null;
+}
+''');
+  }
+
+  Future<void> test_initializer() async {
+    await resolveTestUnit('''
+void f(int? x) {
+  int y = x;
+  print(y);
+}
+''');
+    await assertHasFix('''
+void f(int? x) {
+  int y = x!;
+  print(y);
+}
+''');
+  }
+
+  Future<void> test_initializer_differByMoreThanNullability() async {
+    await resolveTestUnit('''
+void f(String x) {
+  int y = x;
+  print(y);
+}
+''');
+    await assertNoFix();
+  }
+}

pkg/analysis_server/test/src/services/correction/fix/test_all.dart

@@ -23,6 +23,7 @@ import 'add_missing_parameter_required_test.dart'
 import 'add_missing_required_argument_test.dart'
     as add_missing_required_argument;
 import 'add_ne_null_test.dart' as add_ne_null;
+import 'add_null_check_test.dart' as add_null_check;
 import 'add_override_test.dart' as add_override;
 import 'add_required_test.dart' as add_required;
 import 'add_return_type_test.dart' as add_return_type;
@@ -181,6 +182,7 @@ void main() {
     add_missing_parameter_required.main();
     add_missing_required_argument.main();
     add_ne_null.main();
+    add_null_check.main();
     add_override.main();
     add_required.main();
     add_return_type.main();