[analysis_server] Add explicit test for generating minimal edits

This is a minor refactor that adds an explicit test for generating minimal edits without going through the formatter.

There's no change in behaviour - this is just to reduce the noise in a future CL for inserted/deleted commas to be supported in computing minimal diffs.

Change-Id: I3d254bd7a90ccc7b11700b54110ece040e837363
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329601
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>

Author: DanTup

pkg/analysis_server/lib/src/lsp/source_edits.dart

@@ -16,7 +16,7 @@ import 'package:analyzer/src/generated/source.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart' as plugin;
 import 'package:dart_style/dart_style.dart';
 
-DartFormatter formatter = DartFormatter();
+DartFormatter _formatter = DartFormatter();
 
 /// Transforms a sequence of LSP document change events to a sequence of source
 /// edits used by analysis plugins.
@@ -84,10 +84,10 @@ ErrorOr<List<TextEdit>?> generateEditsForFormatting(
   SourceCode formattedResult;
   try {
     // If the lineLength has changed, recreate the formatter with the new setting.
-    if (lineLength != formatter.pageWidth) {
-      formatter = DartFormatter(pageWidth: lineLength);
+    if (lineLength != _formatter.pageWidth) {
+      _formatter = DartFormatter(pageWidth: lineLength);
     }
-    formattedResult = formatter.formatSource(code);
+    formattedResult = _formatter.formatSource(code);
   } on FormatterException {
     // If the document fails to parse, just return no edits to avoid the
     // use seeing edits on every save with invalid code (if LSP gains the
@@ -101,10 +101,10 @@ ErrorOr<List<TextEdit>?> generateEditsForFormatting(
     return success(null);
   }
 
-  return _generateMinimalEdits(result, formattedSource, range: range);
+  return generateMinimalEdits(result, formattedSource, range: range);
 }
 
-List<TextEdit> _generateFullEdit(
+List<TextEdit> generateFullEdit(
     LineInfo lineInfo, String unformattedSource, String formattedSource) {
   final end = lineInfo.getLocation(unformattedSource.length);
   return [
@@ -116,15 +116,15 @@ List<TextEdit> _generateFullEdit(
   ];
 }
 
-/// Generates edits that modify the minimum amount of code (only whitespace) to
-/// change [unformatted] to [formatted].
+/// Generates edits that modify the minimum amount of code (if only whitespace,
+/// commas and comments) to change the source of [result] to [formatted].
 ///
 /// This allows editors to more easily track important locations (such as
 /// breakpoints) without needing to do their own diffing.
 ///
-/// If [range] is supplied, only whitespace edits that fall entirely inside this
-/// range will be included in the results.
-ErrorOr<List<TextEdit>> _generateMinimalEdits(
+/// If [range] is supplied, only edits that fall entirely inside this range will
+/// be included in the results.
+ErrorOr<List<TextEdit>> generateMinimalEdits(
   ParsedUnitResult result,
   String formatted, {
   Range? range,
@@ -146,7 +146,7 @@ ErrorOr<List<TextEdit>> _generateMinimalEdits(
   final parsedFormatted = _parse(formatted, result.unit.featureSet);
   final parsedUnformatted = _parse(unformatted, result.unit.featureSet);
   if (parsedFormatted == null || parsedUnformatted == null) {
-    return success(_generateFullEdit(lineInfo, unformatted, formatted));
+    return success(generateFullEdit(lineInfo, unformatted, formatted));
   }
 
   final unformattedTokens = _iterateAllTokens(parsedUnformatted).iterator;
@@ -270,7 +270,7 @@ ErrorOr<List<TextEdit>> _generateMinimalEdits(
       // If the token lexemes do not match, there is a difference in the parsed
       // token streams (this should not ordinarily happen) so fall back to a
       // full edit.
-      return success(_generateFullEdit(lineInfo, unformatted, formatted));
+      return success(generateFullEdit(lineInfo, unformatted, formatted));
     }
 
     addEditFor(
@@ -293,7 +293,7 @@ ErrorOr<List<TextEdit>> _generateMinimalEdits(
   // If we got here and either of the streams still have tokens, something
   // did not match so fall back to a full edit.
   if (unformattedHasMore || formattedHasMore) {
-    return success(_generateFullEdit(lineInfo, unformatted, formatted));
+    return success(generateFullEdit(lineInfo, unformatted, formatted));
   }
 
   // Finally, handle any whitespace that was after the last token.

pkg/analysis_server/test/integration/lsp/handle_test.dart

@@ -32,7 +32,7 @@ void main() {
 /// functionality are in `test/lsp`.
 @reflectiveTest
 class LspOverLegacyTest extends AbstractAnalysisServerIntegrationTest
-    with LspRequestHelpersMixin {
+    with LspRequestHelpersMixin, LspEditHelpersMixin {
   late final testFile = sourcePath('lib/test.dart');
 
   Uri get testFileUri => Uri.file(testFile);

pkg/analysis_server/test/integration/lsp_server/integration_tests.dart

@@ -21,6 +21,7 @@ abstract class AbstractLspAnalysisServerIntegrationTest
     with
         ClientCapabilitiesHelperMixin,
         LspRequestHelpersMixin,
+        LspEditHelpersMixin,
         LspAnalysisServerTestMixin {
   final List<String> vmArgs = [];
   LspServerClient? client;

pkg/analysis_server/test/lsp/request_helpers_mixin.dart

@@ -15,26 +15,7 @@ import 'package:test/test.dart' hide expect;
 
 import 'change_verifier.dart';
 
-/// Helpers to simplify building LSP requests for use in tests.
-///
-/// The actual sending of requests must be supplied by the implementing class
-/// via [expectSuccessfulResponseTo].
-///
-/// These helpers can be used by in-process tests and out-of-process integration
-/// tests and by both the native LSP server and using LSP over the legacy
-/// protocol.
-mixin LspRequestHelpersMixin {
-  int _id = 0;
-
-  final startOfDocPos = Position(line: 0, character: 0);
-
-  final startOfDocRange = Range(
-      start: Position(line: 0, character: 0),
-      end: Position(line: 0, character: 0));
-
-  /// Whether to include 'clientRequestTime' fields in outgoing messages.
-  bool includeClientRequestTime = false;
-
+mixin LspEditHelpersMixin {
   String applyTextEdit(String content, TextEdit edit) {
     final startPos = edit.range.start;
     final endPos = edit.range.end;
@@ -93,6 +74,27 @@ mixin LspRequestHelpersMixin {
     indexedEdits.sort(TextEditWithIndex.compare);
     return indexedEdits.map((e) => e.edit).fold(content, applyTextEdit);
   }
+}
+
+/// Helpers to simplify building LSP requests for use in tests.
+///
+/// The actual sending of requests must be supplied by the implementing class
+/// via [expectSuccessfulResponseTo].
+///
+/// These helpers can be used by in-process tests and out-of-process integration
+/// tests and by both the native LSP server and using LSP over the legacy
+/// protocol.
+mixin LspRequestHelpersMixin {
+  int _id = 0;
+
+  final startOfDocPos = Position(line: 0, character: 0);
+
+  final startOfDocRange = Range(
+      start: Position(line: 0, character: 0),
+      end: Position(line: 0, character: 0));
+
+  /// Whether to include 'clientRequestTime' fields in outgoing messages.
+  bool includeClientRequestTime = false;
 
   Future<List<CallHierarchyIncomingCall>?> callHierarchyIncoming(
       CallHierarchyItem item) {

pkg/analysis_server/test/lsp/server_abstract.dart

@@ -45,6 +45,7 @@ abstract class AbstractLspAnalysisServerTest
         ResourceProviderMixin,
         ClientCapabilitiesHelperMixin,
         LspRequestHelpersMixin,
+        LspEditHelpersMixin,
         LspAnalysisServerTestMixin,
         ConfigurationFilesMixin {
   late MockLspServerChannel channel;
@@ -807,7 +808,8 @@ mixin ConfigurationFilesMixin on ResourceProviderMixin {
   }
 }
 
-mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin
+mixin LspAnalysisServerTestMixin
+    on LspRequestHelpersMixin, LspEditHelpersMixin
     implements ClientCapabilitiesHelperMixin {
   static const positionMarker = '^';
   static const rangeMarkerStart = '[[';

pkg/analysis_server/test/lsp/source_edits_test.dart

@@ -0,0 +1,107 @@
+// Copyright (c) 2023, 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 'dart:convert';
+
+import 'package:analysis_server/lsp_protocol/protocol.dart';
+import 'package:analysis_server/src/lsp/source_edits.dart';
+import 'package:test/test.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../abstract_single_unit.dart';
+import 'request_helpers_mixin.dart';
+
+void main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(SourceEditsTest);
+  });
+}
+
+@reflectiveTest
+class SourceEditsTest extends AbstractSingleUnitTest with LspEditHelpersMixin {
+  Future<void> test_minimalEdits() async {
+    const startContent = '''
+void   f(){}
+''';
+    const endContent = '''
+void f() {
+}
+''';
+    const expectedEdits = r'''
+Delete 1:6-1:8
+Insert " " at 1:11
+Insert "\n" at 1:12
+''';
+
+    await _assertMinimalEdits(startContent, endContent, expectedEdits);
+  }
+
+  /// Assert that computing minimal edits to convert [start] to [end] produces
+  /// the set of edits described in [expected].
+  ///
+  /// Edits will be automatically applied and verified. [expected] is to ensure
+  /// the edits are minimal and we didn't accidentally produces a single edit
+  /// replacing the entire file.
+  Future<void> _assertMinimalEdits(
+    String start,
+    String end,
+    String expected,
+  ) async {
+    start = start.trim();
+    end = end.trim();
+    expected = expected.trim();
+    await parseTestCode(start);
+    final edits = generateMinimalEdits(testParsedResult, end);
+    expect(edits.toText().trim(), expected);
+    expect(applyTextEdits(start, edits.result), end);
+  }
+}
+
+/// Helpers for building simple text representations of edits to verify that
+/// minimal diffs were produced.
+///
+/// Does not include actual content - resulting content should be verified
+/// separately.
+extension on List<TextEdit> {
+  String toText() => map((edit) => edit.toText()).join('\n');
+}
+
+/// Helpers for building simple text representations of edits to verify that
+/// minimal diffs were produced.
+///
+/// Does not include actual content - resulting content should be verified
+/// separately.
+extension on TextEdit {
+  String toText() {
+    return range.start == range.end
+        ? 'Insert ${jsonEncode(newText)} at ${range.start.toText()}'
+        : newText.isEmpty
+            ? 'Delete ${range.toText()}'
+            : 'Replace ${range.toText()} with ${jsonEncode(newText)}';
+  }
+}
+
+/// Helpers for building simple text representations of edits to verify that
+/// minimal diffs were produced.
+extension on Range {
+  String toText() => '${start.toText()}-${end.toText()}';
+}
+
+/// Helpers for building simple text representations of edits to verify that
+/// minimal diffs were produced.
+extension on Position {
+  String toText() => '${line + 1}:${character + 1}';
+}
+
+/// Helpers for building simple text representations of edits to verify that
+/// minimal diffs were produced.
+///
+/// Does not include actual content - resulting content should be verified
+/// separately.
+extension on ErrorOr<List<TextEdit>> {
+  String toText() => map(
+        (error) => 'Error: ${error.message}',
+        (result) => result.toText(),
+      );
+}

pkg/analysis_server/test/lsp/test_all.dart

@@ -45,6 +45,7 @@ import 'semantic_tokens_test.dart' as semantic_tokens;
 import 'server_test.dart' as server;
 import 'signature_help_test.dart' as signature_help;
 import 'snippets_test.dart' as snippets;
+import 'source_edits_test.dart' as source_edits;
 import 'super_test.dart' as get_super;
 import 'temporary_overlay_operation_test.dart' as temporary_overlay_operation;
 import 'type_definition_test.dart' as type_definition;
@@ -96,6 +97,7 @@ void main() {
     server.main();
     signature_help.main();
     snippets.main();
+    source_edits.main();
     temporary_overlay_operation.main();
     type_definition.main();
     type_hierarchy.main();

pkg/analysis_server/test/lsp_over_legacy/format_test.dart

@@ -5,6 +5,7 @@
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
+import '../lsp/request_helpers_mixin.dart';
 import 'abstract_lsp_over_legacy.dart';
 
 void main() {
@@ -14,7 +15,7 @@ void main() {
 }
 
 @reflectiveTest
-class FormatTest extends LspOverLegacyTest {
+class FormatTest extends LspOverLegacyTest with LspEditHelpersMixin {
   Future<void> test_format() async {
     const content = 'void     main() {}';
     const expectedContent = 'void main() {}';