Update ChangeBuilder to not include empty edits (Take 2)

Combination of Brian's change:
https://dart-review.googlesource.com/c/sdk/+/63901

and Devon's change:
https://dart-review.googlesource.com/c/sdk/+/66405

Change-Id: I57e5d1f7d3a653f225aba2d690190fe1ff18a5b1
Reviewed-on: https://dart-review.googlesource.com/67344
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Danny Tuppeny <dantup@google.com>

pkg/analysis_server/lib/protocol/protocol_generated.dart

@@ -8206,7 +8206,7 @@ class EditImportElementsParams implements RequestParams {
  * edit.importElements result
  *
  * {
- *   "edit": SourceFileEdit
+ *   "edit": optional SourceFileEdit
  * }
  *
  * Clients may not extend, implement or mix-in this class.
@@ -8219,7 +8219,8 @@ class EditImportElementsResult implements ResponseResult {
    * accessible. The file to be edited will be the defining compilation unit of
    * the library containing the file specified in the request, which can be
    * different than the file specified in the request if the specified file is
-   * a part file.
+   * a part file. This field will be omitted if there are no edits that need to
+   * be applied.
    */
   SourceFileEdit get edit => _edit;
 
@@ -8228,14 +8229,14 @@ class EditImportElementsResult implements ResponseResult {
    * accessible. The file to be edited will be the defining compilation unit of
    * the library containing the file specified in the request, which can be
    * different than the file specified in the request if the specified file is
-   * a part file.
+   * a part file. This field will be omitted if there are no edits that need to
+   * be applied.
    */
   void set edit(SourceFileEdit value) {
-    assert(value != null);
     this._edit = value;
   }
 
-  EditImportElementsResult(SourceFileEdit edit) {
+  EditImportElementsResult({SourceFileEdit edit}) {
     this.edit = edit;
   }
 
@@ -8249,10 +8250,8 @@ class EditImportElementsResult implements ResponseResult {
       if (json.containsKey("edit")) {
         edit = new SourceFileEdit.fromJson(
             jsonDecoder, jsonPath + ".edit", json["edit"]);
-      } else {
-        throw jsonDecoder.mismatch(jsonPath, "edit");
       }
-      return new EditImportElementsResult(edit);
+      return new EditImportElementsResult(edit: edit);
     } else {
       throw jsonDecoder.mismatch(jsonPath, "edit.importElements result", json);
     }
@@ -8268,7 +8267,9 @@ class EditImportElementsResult implements ResponseResult {
   @override
   Map<String, dynamic> toJson() {
     Map<String, dynamic> result = {};
-    result["edit"] = edit.toJson();
+    if (edit != null) {
+      result["edit"] = edit.toJson();
+    }
     return result;
   }
 

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

@@ -408,11 +408,13 @@ class EditDomainHandler extends AbstractRequestHandler {
     ImportElementsComputer computer =
         new ImportElementsComputer(server.resourceProvider, result);
     SourceChange change = await computer.createEdits(params.elements);
+    List<SourceFileEdit> edits = change.edits;
+    SourceFileEdit edit = edits.isEmpty ? null : edits[0];
     //
     // Send the response.
     //
     server.sendResponse(
-        new EditImportElementsResult(change.edits[0]).toResponse(request.id));
+        new EditImportElementsResult(edit: edit).toResponse(request.id));
   }
 
   Future isPostfixCompletionApplicable(Request request) async {

pkg/analysis_server/test/integration/edit/import_elements_test.dart

@@ -79,9 +79,7 @@ class AnalysisGetImportElementsIntegrationTest
     EditImportElementsResult result =
         await sendEditImportElements(pathname, <ImportedElements>[]);
 
-    SourceFileEdit edit = result.edit;
-    expect(edit, isNotNull);
-    expect(edit.edits, hasLength(0));
+    expect(result.edit, isNull);
   }
 
   Future setUp() async {

pkg/analysis_server/test/integration/support/integration_test_methods.dart

@@ -1763,13 +1763,14 @@ abstract class IntegrationTestMixin {
    *
    * Returns
    *
-   * edit: SourceFileEdit
+   * edit: SourceFileEdit (optional)
    *
    *   The edits to be applied in order to make the specified elements
    *   accessible. The file to be edited will be the defining compilation unit
    *   of the library containing the file specified in the request, which can
    *   be different than the file specified in the request if the specified
-   *   file is a part file.
+   *   file is a part file. This field will be omitted if there are no edits
+   *   that need to be applied.
    */
   Future<EditImportElementsResult> sendEditImportElements(
       String file, List<ImportedElements> elements) async {

pkg/analysis_server/test/integration/support/protocol_matchers.dart

@@ -2329,12 +2329,12 @@ final Matcher isEditImportElementsParams = new LazyMatcher(() =>
  * edit.importElements result
  *
  * {
- *   "edit": SourceFileEdit
+ *   "edit": optional SourceFileEdit
  * }
  */
 final Matcher isEditImportElementsResult = new LazyMatcher(() =>
-    new MatchesJsonObject(
-        "edit.importElements result", {"edit": isSourceFileEdit}));
+    new MatchesJsonObject("edit.importElements result", null,
+        optionalFields: {"edit": isSourceFileEdit}));
 
 /**
  * edit.isPostfixCompletionApplicable params

pkg/analysis_server/test/src/computer/import_elements_computer_test.dart

@@ -34,16 +34,19 @@ class ImportElementsComputerTest extends AbstractContextTest {
   }
 
   void assertNoChanges() {
-    expect(sourceFileEdit.edits, isEmpty);
+    expect(sourceFileEdit, isNull);
   }
 
   Future<Null> computeChanges(List<ImportedElements> importedElements) async {
     SourceChange change = await computer.createEdits(importedElements);
     expect(change, isNotNull);
     List<SourceFileEdit> edits = change.edits;
-    expect(edits, hasLength(1));
-    sourceFileEdit = edits[0];
-    expect(sourceFileEdit, isNotNull);
+    if (edits.length == 1) {
+      sourceFileEdit = edits[0];
+      expect(sourceFileEdit, isNotNull);
+    } else {
+      sourceFileEdit = null;
+    }
   }
 
   Future<Null> createBuilder(String content) async {

pkg/analysis_server/tool/spec/spec_input.html

@@ -2246,14 +2246,13 @@
       </field>
     </params>
     <result>
-      <field name="edit">
+      <field name="edit" optional="true">
         <ref>SourceFileEdit</ref>
         <p>
-          The edits to be applied in order to make the specified elements
-          accessible. The file to be edited will be the defining compilation
-          unit of the library containing the file specified in the request,
-          which can be different than the file specified in the request if the
-          specified file is a part file.
+          The edits to be applied in order to make the specified elements accessible. The file to be edited will be the
+          defining compilation unit of the library containing the file specified in the request, which can be different
+          than the file specified in the request if the specified file is a part file. This field will be omitted if
+          there are no edits that need to be applied.
         </p>
       </field>
     </result>

pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_core.dart

@@ -67,8 +67,10 @@ class ChangeBuilderImpl implements ChangeBuilder {
     await null;
     FileEditBuilderImpl builder = await createFileEditBuilder(path);
     buildFileEdit(builder);
-    _change.addFileEdit(builder.fileEdit);
-    await builder.finalize();
+    if (builder.hasEdits) {
+      _change.addFileEdit(builder.fileEdit);
+      await builder.finalize();
+    }
   }
 
   /**
@@ -275,6 +277,11 @@ class FileEditBuilderImpl implements FileEditBuilder {
   FileEditBuilderImpl(this.changeBuilder, String path, int timeStamp)
       : fileEdit = new SourceFileEdit(path, timeStamp);
 
+  /**
+   * Return `true` if this builder has edits to be applied.
+   */
+  bool get hasEdits => fileEdit.edits.isNotEmpty;
+
   @override
   void addDeletion(SourceRange range) {
     EditBuilderImpl builder = createEditBuilder(range.offset, range.length);

pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart

@@ -1123,6 +1123,9 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl
       : libraryElement = unit.element.library,
         super(changeBuilder, path, timeStamp);
 
+  @override
+  bool get hasEdits => super.hasEdits || librariesToImport.isNotEmpty;
+
   @override
   void addInsertion(int offset, void buildEdit(DartEditBuilder builder)) =>
       super.addInsertion(offset, (builder) => buildEdit(builder));

pkg/analyzer_plugin/test/src/utilities/change_builder/change_builder_core_test.dart

@@ -44,7 +44,19 @@ class ChangeBuilderImplTest {
     expect(builder.sourceChange.selection, position);
   }
 
-  void test_sourceChange_noChanges() {
+  void test_sourceChange_emptyEdit() async {
+    ChangeBuilderImpl builder = new ChangeBuilderImpl();
+    String path = '/test.dart';
+    await builder.addFileEdit(path, (FileEditBuilder builder) {});
+    SourceChange sourceChange = builder.sourceChange;
+    expect(sourceChange, isNotNull);
+    expect(sourceChange.edits, isEmpty);
+    expect(sourceChange.linkedEditGroups, isEmpty);
+    expect(sourceChange.message, isEmpty);
+    expect(sourceChange.selection, isNull);
+  }
+
+  void test_sourceChange_noEdits() {
     ChangeBuilderImpl builder = new ChangeBuilderImpl();
     SourceChange sourceChange = builder.sourceChange;
     expect(sourceChange, isNotNull);
@@ -57,7 +69,9 @@ class ChangeBuilderImplTest {
   test_sourceChange_oneChange() async {
     ChangeBuilderImpl builder = new ChangeBuilderImpl();
     String path = '/test.dart';
-    await builder.addFileEdit(path, (FileEditBuilder builder) {});
+    await builder.addFileEdit(path, (FileEditBuilder builder) {
+      builder.addSimpleInsertion(0, '_');
+    });
     builder.getLinkedEditGroup('a');
     SourceChange sourceChange = builder.sourceChange;
     expect(sourceChange, isNotNull);

pkg/pkg.status

@@ -198,6 +198,9 @@ analysis_server/test/*: Skip # Issue 26813
 analyzer/test/*: Skip # Issue 26813
 analyzer/tool/*: Skip # Issue 26813
 
+[ $builder_tag == pkg_tests_no_preview_dart_2 && $runtime == vm ]
+analysis_server/test/services/correction/assist_test: RuntimeError
+
 # Analyze dev_compiler but only run its tests on the vm
 [ $compiler != dart2analyzer && $runtime != vm ]
 dev_compiler/test/*: Skip

tools/bots/test_matrix.json

@@ -1889,6 +1889,7 @@
         {
           "name": "package unit tests",
           "arguments": [
+            "--builder-tag=pkg_tests_no_preview_dart_2",
             "--checked",
             "--compiler=none",
             "--no-preview-dart-2",