first cut adding simple non-nullable migration to dartfix

... and address comments in https://dart-review.googlesource.com/c/sdk/+/88640

Change-Id: I26e0f70f7c112d9ac184e10475e519afd8118a1d
Reviewed-on: https://dart-review.googlesource.com/c/89046
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Dan Rubel <danrubel@google.com>

pkg/analysis_server/lib/src/edit/edit_dartfix.dart

@@ -6,6 +6,7 @@ import 'package:analysis_server/plugin/edit/fix/fix_core.dart';
 import 'package:analysis_server/protocol/protocol.dart';
 import 'package:analysis_server/protocol/protocol_generated.dart';
 import 'package:analysis_server/src/analysis_server.dart';
+import 'package:analysis_server/src/edit/fix/non_nullable_fix.dart';
 import 'package:analysis_server/src/edit/fix/prefer_int_literals_fix.dart';
 import 'package:analysis_server/src/edit/fix/prefer_mixin_fix.dart';
 import 'package:analysis_server/src/services/correction/change_workspace.dart';
@@ -43,7 +44,8 @@ class EditDartFix {
 
   EditDartFix(this.server, this.request);
 
-  void addFix(String description, Location location, SourceChange change) {
+  void addSourceChange(
+      String description, Location location, SourceChange change) {
     suggestions.add(new DartFixSuggestion(description, location: location));
     for (SourceFileEdit fileEdit in change.edits) {
       for (SourceEdit sourceEdit in fileEdit.edits) {
@@ -52,6 +54,14 @@ class EditDartFix {
     }
   }
 
+  void addSourceFileEdit(
+      String description, Location location, SourceFileEdit fileEdit) {
+    suggestions.add(new DartFixSuggestion(description, location: location));
+    for (SourceEdit sourceEdit in fileEdit.edits) {
+      sourceChange.addEdit(fileEdit.file, fileEdit.fileStamp, sourceEdit);
+    }
+  }
+
   void addRecommendation(String description, [Location location]) {
     otherSuggestions
         .add(new DartFixSuggestion(description, location: location));
@@ -89,6 +99,7 @@ class EditDartFix {
 
     final preferIntLiterals = lintRules['prefer_int_literals'];
     final preferIntLiteralsFix = new PreferIntLiteralsFix(this);
+    final nonNullableFix = new NonNullableFix(this);
     preferIntLiterals?.reporter = preferIntLiteralsFix;
 
     // Setup
@@ -163,6 +174,9 @@ class EditDartFix {
             for (LinterFix fix in fixes) {
               await fix.applyLocalFixes(result);
             }
+            if (isIncluded(source.fullName)) {
+              nonNullableFix.applyLocalFixes(result);
+            }
           }
           break;
         } on InconsistentAnalysisException catch (_) {
@@ -219,7 +233,7 @@ class EditDartFix {
     Fix fix = await processor.computeFix();
     final location = locationFor(result, error.offset, error.length);
     if (fix != null) {
-      addFix(fix.change.message, location, fix.change);
+      addSourceChange(fix.change.message, location, fix.change);
     } else {
       // TODO(danrubel): Determine why the fix could not be applied
       // and report that in the description.

pkg/analysis_server/lib/src/edit/fix/non_nullable_fix.dart

@@ -0,0 +1,82 @@
+import 'package:analysis_server/src/edit/edit_dartfix.dart';
+import 'package:analyzer/dart/analysis/results.dart';
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/src/generated/engine.dart';
+import 'package:analyzer/src/generated/source.dart';
+import 'package:analyzer_plugin/protocol/protocol_common.dart';
+
+class NonNullableFix {
+  final EditDartFix dartFix;
+
+  /// The current source being "fixed"
+  Source source;
+
+  /// The source file change or `null` if none
+  SourceFileEdit fileEdit;
+
+  int firstOffset;
+  int firstLength;
+
+  NonNullableFix(this.dartFix);
+
+  void addEdit(int offset, int length, String replacementText) {
+    fileEdit ??= new SourceFileEdit(source.fullName, 0);
+    fileEdit.edits.add(new SourceEdit(offset, length, replacementText));
+  }
+
+  /// Update the source to be non-nullable by
+  /// 1) adding trailing '?' to type references of nullable variables, and
+  /// 2) removing trailing '?' from type references of non-nullable variables.
+  void applyLocalFixes(ResolvedUnitResult result) {
+    final context = result.session.analysisContext;
+    AnalysisOptionsImpl options = context.analysisOptions;
+    if (!options.experimentStatus.non_nullable) {
+      return;
+    }
+
+    final unit = result.unit;
+    source = unit.declaredElement.source;
+
+    // find and fix types
+    unit.accept(new _NonNullableTypeVisitor(this));
+
+    // add source changes to the collection of fixes
+    source = null;
+    if (fileEdit != null) {
+      dartFix.addSourceFileEdit('Update non-nullable type references',
+          dartFix.locationFor(result, firstOffset, firstLength), fileEdit);
+    }
+  }
+}
+
+class _NonNullableTypeVisitor extends RecursiveAstVisitor<void> {
+  final NonNullableFix fix;
+
+  _NonNullableTypeVisitor(this.fix);
+
+  @override
+  void visitConstructorName(ConstructorName node) {
+    // skip the type name associated with the constructor
+    node.type?.typeArguments?.accept(this);
+  }
+
+  @override
+  void visitTypeName(TypeName node) {
+    // TODO(danrubel): Replace this braindead implementation
+    // with something that determines whether or not the type should be nullable
+    // and adds or removes the trailing `?` to match.
+    if (node.question == null) {
+      final identifier = node.name;
+      if (identifier is SimpleIdentifier) {
+        if (identifier.name == 'void') {
+          return;
+        }
+      }
+      fix.addEdit(node.end, 0, '?');
+      fix.firstOffset ??= node.offset;
+      fix.firstLength ??= node.length;
+    }
+    super.visitTypeName(node);
+  }
+}

pkg/analysis_server/lib/src/edit/fix/prefer_int_literals_fix.dart

@@ -34,8 +34,10 @@ class PreferIntLiteralsFix extends LinterFix {
           dartFix.locationFor(result, literal.offset, literal.length);
       if (assists.isNotEmpty) {
         for (Assist assist in assists) {
-          dartFix.addFix('Replace a double literal with an int literal',
-              location, assist.change);
+          dartFix.addSourceChange(
+              'Replace a double literal with an int literal',
+              location,
+              assist.change);
         }
       } else {
         // TODO(danrubel): If assists is empty, then determine why

pkg/analysis_server/lib/src/edit/fix/prefer_mixin_fix.dart

@@ -50,8 +50,8 @@ class PreferMixinFix extends LinterFix {
             dartFix.locationFor(result, elem.nameOffset, elem.nameLength);
         if (assists.isNotEmpty) {
           for (Assist assist in assists) {
-            dartFix.addFix('Convert ${elem.displayName} to a mixin', location,
-                assist.change);
+            dartFix.addSourceChange('Convert ${elem.displayName} to a mixin',
+                location, assist.change);
           }
         } else {
           // TODO(danrubel): If assists is empty, then determine why

pkg/analysis_server/test/domain_edit_dartfix_test.dart

@@ -111,6 +111,55 @@ main() {
     ''');
   }
 
+  test_dartfix_non_nullable() async {
+    // Add analysis options to enable non-nullable analysis
+    newFile('/project/analysis_options.yaml', content: '''
+analyzer:
+  enable-experiment:
+    - non-nullable
+''');
+
+    createProject();
+    addTestFile('''
+main() {
+  functionWithNullableParam(new List<Object>(1));
+  functionWithNullableParam(null);
+}
+
+void functionWithNullableParam(Object object) {
+  if (object == null) {
+    print('object is null');
+  } else {
+    print('object is not-null');
+  }
+  List<Object> list = null;
+  list = <Object>[];
+  list.add(object);
+}
+''');
+    EditDartfixResult result = await performFix();
+    expect(result.suggestions, hasLength(1));
+    expect(result.hasErrors, isFalse);
+    expectSuggestion(result.suggestions[0], 'non-nullable', 46, 6);
+    expectEdits(result.edits, '''
+main() {
+  functionWithNullableParam(new List<Object?>(1));
+  functionWithNullableParam(null);
+}
+
+void functionWithNullableParam(Object? object) {
+  if (object == null) {
+    print('object is null');
+  } else {
+    print('object is not-null');
+  }
+  List<Object?>? list = null;
+  list = <Object?>[];
+  list.add(object);
+}
+''');
+  }
+
   test_dartfix_excludedSource() async {
     // Add analysis options to exclude the lib directory then reanalyze
     newFile('/project/analysis_options.yaml', content: '''

pkg/analyzer/test/generated/parser_fasta_test.dart

@@ -113,8 +113,9 @@ class ComplexParserTest_Fasta extends FastaParserTestCase
   void test_conditionalExpression_precedence_nullableType_as2() {
     ExpressionStatement statement = parseStatement('x as bool? ? (x + y) : z;');
     ConditionalExpression expression = statement.expression;
-    Expression condition = expression.condition;
-    expect(condition, isAsExpression);
+    AsExpression asExpression = expression.condition;
+    TypeName type = asExpression.type;
+    expect(type.question.lexeme, '?');
     Expression thenExpression = expression.thenExpression;
     expect(thenExpression, isParenthesizedExpression);
     Expression elseExpression = expression.elseExpression;
@@ -127,9 +128,10 @@ class ComplexParserTest_Fasta extends FastaParserTestCase
     ExpressionStatement statement =
         parseStatement('(x as bool?) ? (x + y) : z;');
     ConditionalExpression expression = statement.expression;
-    Expression condition = expression.condition;
-    expect(condition, isParenthesizedExpression);
-    expect((condition as ParenthesizedExpression).expression, isAsExpression);
+    ParenthesizedExpression condition = expression.condition;
+    AsExpression asExpression = condition.expression;
+    TypeName type = asExpression.type;
+    expect(type.question.lexeme, '?');
     Expression thenExpression = expression.thenExpression;
     expect(thenExpression, isParenthesizedExpression);
     Expression elseExpression = expression.elseExpression;
@@ -142,8 +144,9 @@ class ComplexParserTest_Fasta extends FastaParserTestCase
     ExpressionStatement statement =
         parseStatement('x is String? ? (x + y) : z;');
     ConditionalExpression expression = statement.expression;
-    Expression condition = expression.condition;
-    expect(condition, isIsExpression);
+    IsExpression isExpression = expression.condition;
+    TypeName type = isExpression.type;
+    expect(type.question.lexeme, '?');
     Expression thenExpression = expression.thenExpression;
     expect(thenExpression, isParenthesizedExpression);
     Expression elseExpression = expression.elseExpression;
@@ -156,9 +159,10 @@ class ComplexParserTest_Fasta extends FastaParserTestCase
     ExpressionStatement statement =
         parseStatement('(x is String?) ? (x + y) : z;');
     ConditionalExpression expression = statement.expression;
-    Expression condition = expression.condition;
-    expect(condition, isParenthesizedExpression);
-    expect((condition as ParenthesizedExpression).expression, isIsExpression);
+    ParenthesizedExpression condition = expression.condition;
+    IsExpression isExpression = condition.expression;
+    TypeName type = isExpression.type;
+    expect(type.question.lexeme, '?');
     Expression thenExpression = expression.thenExpression;
     expect(thenExpression, isParenthesizedExpression);
     Expression elseExpression = expression.elseExpression;

pkg/front_end/lib/src/fasta/parser/type_info_impl.dart

@@ -397,7 +397,9 @@ class ComplexTypeInfo implements TypeInfo {
   /// Type arguments were seen during analysis.
   final TypeParamOrArgInfo typeArguments;
 
-  /// The token before the trailing question mark or `null` if none.
+  /// The token before the trailing question mark or `null` if either
+  /// 1) there is no trailing question mark, or
+  /// 2) the trailing question mark is not part of the type reference.
   Token beforeQuestionMark;
 
   /// The last token in the type reference.