[analysis_server] Ensure LspChangeVerifier always normalizes source + expects normalised source files

This removes `useLineEndingsForPlatform = false` from a few more tests, and as part of fixing them, ensures that `LspChangeVerifier` always normalises the expected text, and asserts that all modifies files were modified.

This highlighted many more tests that were not normalizing their content (but didn't use `useLineEndingsForPlatform` because they didn't previously call any shared methods that required/asserting normalization. Those tests were also updated.

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

Author: DanTup

pkg/analysis_server/lib/src/analysis_server.dart

@@ -421,6 +421,10 @@ abstract class AnalysisServer {
     return sessions;
   }
 
+  /// The default line terminator that should be used by the server when there
+  /// is no existing EOL to copy.
+  String get defaultEol => io.Platform.lineTerminator;
+
   /// A table mapping [Folder]s to the [AnalysisDriver]s associated with them.
   Map<Folder, analysis.AnalysisDriver> get driverMap =>
       contextManager.driverMap;

pkg/analysis_server/lib/src/handler/legacy/edit_get_fixes.dart

@@ -297,6 +297,7 @@ error.errorCode: ${diagnostic.diagnosticCode}
         diagnostic,
         content,
         node,
+        defaultEol: server.defaultEol,
       );
       var fixes = await generator.computeFixes();
       if (fixes.isNotEmpty) {

pkg/analysis_server/lib/src/lsp/handlers/code_actions/pubspec.dart

@@ -82,6 +82,7 @@ class PubspecCodeActionsProducer extends AbstractCodeActionsProducer {
         error,
         content,
         node,
+        defaultEol: server.defaultEol,
       );
       var fixes = await generator.computeFixes();
       if (fixes.isEmpty) {

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

@@ -963,6 +963,7 @@ class BulkFixProcessor {
         errors[0],
         contents,
         node,
+        defaultEol: builder.defaultEol,
       );
       return await generator.computeFixes();
     }

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

@@ -17,6 +17,7 @@ import 'package:analyzer/src/generated/java_core.dart';
 import 'package:analyzer/src/pubspec/pubspec_warning_code.dart';
 import 'package:analyzer/src/pubspec/validators/missing_dependency_validator.dart';
 import 'package:analyzer/src/util/yaml.dart';
+import 'package:analyzer_plugin/src/utilities/extensions/string_extension.dart';
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 import 'package:yaml/yaml.dart';
@@ -51,35 +52,19 @@ class PubspecFixGenerator {
   /// The fixes that were generated.
   final List<Fix> fixes = <Fix>[];
 
-  /// The end-of-line marker used in the `pubspec.yaml` file.
-  String? _endOfLine;
+  /// The end-of-line marker to be used in this `pubspec.yaml` file.
+  final String endOfLine;
 
   PubspecFixGenerator(
     this.resourceProvider,
     this.diagnostic,
     this.content,
-    this.node,
-  ) : diagnosticOffset = diagnostic.offset,
-      diagnosticLength = diagnostic.length,
-      lineInfo = LineInfo.fromContent(content);
-
-  /// Returns the end-of-line marker to use for the `pubspec.yaml` file.
-  String get endOfLine {
-    // TODO(brianwilkerson): Share this with CorrectionUtils, probably by
-    //  creating a subclass of CorrectionUtils containing utilities that are
-    //  only dependent on knowing the content of the file. Also consider moving
-    //  this kind of utility into the ChangeBuilder API directly.
-    var endOfLine = _endOfLine;
-    if (endOfLine != null) {
-      return endOfLine;
-    }
-
-    if (content.contains('\r\n')) {
-      return _endOfLine = '\r\n';
-    } else {
-      return _endOfLine = '\n';
-    }
-  }
+    this.node, {
+    required String defaultEol,
+  }) : diagnosticOffset = diagnostic.offset,
+       diagnosticLength = diagnostic.length,
+       lineInfo = LineInfo.fromContent(content),
+       endOfLine = content.endOfLine ?? defaultEol;
 
   /// Return the absolute, normalized path to the file in which the error was
   /// reported.
@@ -332,10 +317,12 @@ class PubspecFixGenerator {
     var section = node[sectionName];
     var buffer = StringBuffer();
     if (section == null) {
-      buffer.writeln('$sectionName:');
+      buffer.write('$sectionName:');
+      buffer.write(endOfLine);
     }
     for (var name in packageNames) {
-      buffer.writeln('  $name: any');
+      buffer.write('  $name: any');
+      buffer.write(endOfLine);
     }
 
     var offset =

pkg/analysis_server/test/abstract_context.dart

@@ -2,8 +2,6 @@
 // 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:io';
-
 import 'package:analysis_server/src/protocol_server.dart';
 import 'package:analysis_server/src/services/correction/assist_internal.dart';
 import 'package:analysis_server/src/services/correction/fix_internal.dart';
@@ -311,7 +309,7 @@ class AbstractContextTest
   }) {
     for (var fileEdit in sourceChange.edits) {
       var file = getFile(fileEdit.file);
-      buffer.write('>>>>>>>>>> ${file.posixPath}${Platform.lineTerminator}');
+      buffer.write('>>>>>>>>>> ${file.posixPath}$testEol');
       var current = file.readAsStringSync();
       var updated = SourceEdit.applySequence(current, fileEdit.edits);
       buffer.write(updated);

pkg/analysis_server/test/abstract_single_unit.dart

@@ -44,8 +44,8 @@ class AbstractSingleUnitTest extends AbstractContextTest {
 
   String get testCode => parsedTestCode.code;
   set testCode(String value) {
-    parsedTestCode = TestCode.parse(
-      normalizeSource(value),
+    parsedTestCode = TestCode.parseNormalized(
+      value,
       positionShorthand: allowTestCodeShorthand,
       rangeShorthand: allowTestCodeShorthand,
     );

pkg/analysis_server/test/lsp/change_verifier.dart

@@ -2,14 +2,22 @@
 // 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:analyzer/src/test_utilities/platform.dart';
+import 'package:analyzer_plugin/src/utilities/extensions/string_extension.dart';
 import 'package:collection/collection.dart';
 import 'package:test/test.dart';
 
 import 'request_helpers_mixin.dart';
 
 /// Applies LSP [WorkspaceEdit]s to produce a flattened string describing the
 /// new file contents and any create/rename/deletes to use in test expectations.
+///
+/// Expects all file content to use [testEol] and automatically
+/// normalizes expectations to use the same. Does not normalize edits, as line
+/// ending differences in edits indicate a bug in the edit creation.
 class LspChangeVerifier {
   /// Marks that signifies the start of an edit description.
   static final editMarkerStart = '>>>>>>>>>>';
@@ -27,11 +35,16 @@ class LspChangeVerifier {
   /// The [WorkspaceEdit] being applied/verified.
   final WorkspaceEdit edit;
 
+  /// The line terminator to use in the output.
+  final String endOfLine = testEol;
+
   LspChangeVerifier(this.editHelpers, this.edit) {
     _applyEdit();
   }
 
   void verifyFiles(String expected, {Map<Uri, int>? expectedVersions}) {
+    expected = normalizeNewlinesForPlatform(expected);
+
     var actual = _toChangeString();
     if (actual != expected) {
       print('-' * 64);
@@ -182,6 +195,18 @@ class LspChangeVerifier {
   String _applyTextEdits(String content, List<TextEdit> changes) =>
       editHelpers.applyTextEdits(content, changes);
 
+  /// Assert that [input] uses the current platforms line endings.
+  void _assertLineEnding(String input) {
+    var actualLineEnding = input.endOfLine;
+    if (actualLineEnding != null) {
+      assert(
+        actualLineEnding == endOfLine,
+        'Expected line ending ${jsonEncode(endOfLine)} '
+        'but string uses ${jsonEncode(actualLineEnding)}',
+      );
+    }
+  }
+
   _Change _change(Uri fileUri) => _changes.putIfAbsent(
     fileUri,
     () => _Change(_getCurrentFileContent(fileUri)),
@@ -197,8 +222,13 @@ class LspChangeVerifier {
     expect(edit.textDocument.version, equals(expectedVersion));
   }
 
-  String? _getCurrentFileContent(Uri uri) =>
-      editHelpers.getCurrentFileContent(uri);
+  String? _getCurrentFileContent(Uri uri) {
+    var content = editHelpers.getCurrentFileContent(uri);
+    if (content != null) {
+      _assertLineEnding(content);
+    }
+    return content;
+  }
 
   String _relativeUri(Uri uri) => editHelpers.relativeUri(uri);
 
@@ -220,7 +250,7 @@ class LspChangeVerifier {
       if (content?.isEmpty ?? false) {
         buffer.write(' empty');
       }
-      buffer.writeln();
+      buffer.write(endOfLine);
 
       // Write any annotations.
       if (annotations.isNotEmpty) {
@@ -231,7 +261,7 @@ class LspChangeVerifier {
             buffer.write(' (${annotation.description})');
           }
           buffer.write(': ${operations.join(', ')}');
-          buffer.writeln();
+          buffer.write(endOfLine);
         }
       }
 
@@ -242,7 +272,8 @@ class LspChangeVerifier {
         // If the content didn't end with a newline we need to add one, but
         // add a marked so it's clear there was no trailing newline.
         if (content.isNotEmpty && !content.endsWith('\n')) {
-          buffer.writeln(editMarkerEnd);
+          buffer.write(editMarkerEnd);
+          buffer.write(endOfLine);
         }
       }
     }

pkg/analysis_server/test/lsp/code_actions_mixin.dart

@@ -5,6 +5,7 @@
 import 'package:analysis_server/lsp_protocol/protocol.dart';
 import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analysis_server/src/lsp/extensions/code_action.dart';
+import 'package:analyzer/src/test_utilities/platform.dart';
 import 'package:analyzer/src/test_utilities/test_code_format.dart';
 import 'package:collection/collection.dart';
 import 'package:test/test.dart';
@@ -80,6 +81,8 @@ mixin CodeActionsTestMixin
     String? filePath,
     bool openTargetFile = false,
   }) async {
+    content = normalizeNewlinesForPlatform(content);
+
     var action = await expectCodeAction(
       TestCode.parse(content),
       kind: kind,
@@ -335,6 +338,9 @@ ${LspChangeVerifier.editMarkerStart} ${relativePath(filePath)}
 $expected''';
     }
 
+    content = normalizeNewlinesForPlatform(content);
+    expected = normalizeNewlinesForPlatform(expected);
+
     var action = await expectCodeActionLiteral(
       filePath: filePath,
       content,
@@ -375,6 +381,9 @@ ${LspChangeVerifier.editMarkerStart} ${relativePath(filePath)}
 $expected''';
     }
 
+    content = normalizeNewlinesForPlatform(content);
+    expected = normalizeNewlinesForPlatform(expected);
+
     var commandAction = await expectCommandCodeAction(
       filePath: filePath,
       content,

pkg/analysis_server/test/lsp/completion_dart_test.dart

@@ -64,7 +64,7 @@ typedef ExpectedLabel =
 abstract class AbstractCompletionTest extends AbstractLspAnalysisServerTest
     with CompletionTestMixin {
   late String content;
-  late final code = TestCode.parseNormalized(content);
+  late final TestCode code = TestCode.parseNormalized(content);
 
   AbstractCompletionTest() {
     defaultInitializationOptions = {
@@ -5384,7 +5384,7 @@ stle^
 abstract class SnippetCompletionTest extends AbstractLspAnalysisServerTest
     with CompletionTestMixin {
   late String content;
-  late final code = TestCode.parseNormalized(content);
+  late final TestCode code = TestCode.parseNormalized(content);
 
   /// Expect that there is a snippet for [prefix] at [position] with the label
   /// [label] and return the results of applying it to [content].