[analysis_server] Bump yaml_edit, remove ignore_diagnostic workaround

This bumps yaml_edit to get a fix from dart-archive/yaml_edit#66 that corrects line endings on Windows.

It removes the workaround that was in the server normalizing these itself.

It also updates the tests to use normalized content to ensure consistent line endings on both platforms.

Change-Id: I096978500b30ca41d38bc9e78dc9bdf3e44474d1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/353521
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Keerti Parthasarathy <[email protected]>
Commit-Queue: Keerti Parthasarathy <[email protected]>

Author: DanTup

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

@@ -8,7 +8,6 @@ import 'package:analysis_server/src/services/correction/util.dart';
 import 'package:analyzer/error/error.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/src/ignore_comments/ignore_info.dart';
-import 'package:analyzer/src/test_utilities/platform.dart';
 import 'package:analyzer/src/workspace/blaze.dart';
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
@@ -156,18 +155,6 @@ class IgnoreDiagnosticInAnalysisOptionsFile extends AbstractIgnoreDiagnostic {
 
       var edit = editor.edits.single;
       var replacement = edit.replacement;
-
-      // TODO(dantup): The YAML editor currently produces inconsistent line
-      //  endings in edits when the source file contains '\r\n'.
-      // https://github.com/dart-lang/yaml_edit/issues/65
-      var analysisOptionsEol = content.contains('\r')
-          ? '\r\n'
-          : content.contains('\n')
-              ? '\n'
-              : platformEol;
-      replacement =
-          replacement.replaceAll('\r', '').replaceAll('\n', analysisOptionsEol);
-
       builder.addSimpleInsertion(edit.offset, replacement);
     });
   }

pkg/analysis_server/test/abstract_context.dart

@@ -15,6 +15,7 @@ import 'package:analyzer/src/dart/analysis/driver_based_analysis_context.dart';
 import 'package:analyzer/src/dart/analysis/experiments.dart';
 import 'package:analyzer/src/generated/engine.dart' show AnalysisEngine;
 import 'package:analyzer/src/test_utilities/mock_sdk.dart';
+import 'package:analyzer/src/test_utilities/platform.dart';
 import 'package:analyzer/src/test_utilities/resource_provider_mixin.dart';
 import 'package:analyzer/src/util/file_paths.dart' as file_paths;
 import 'package:analyzer/src/utilities/extensions/file_system.dart';
@@ -31,6 +32,9 @@ class AbstractContextTest
 
   static final ByteStore _byteStore = MemoryByteStore();
 
+  /// Whether to rewrite line endings in test code based on platform.
+  bool useLineEndingsForPlatform = false;
+
   final Map<String, String> _declaredVariables = {};
   AnalysisContextCollectionImpl? _analysisContextCollection;
 
@@ -161,7 +165,7 @@ class AbstractContextTest
       }
     }
 
-    newFile(analysisOptionsPath, buffer.toString());
+    writeAnalysisOptionsFile(buffer.toString());
   }
 
   /// Returns the existing analysis driver that should be used to analyze the
@@ -191,11 +195,16 @@ class AbstractContextTest
       throw StateError('Only dart files can be changed after analysis.');
     }
 
-    final file = super.newFile(path, content);
+    final file = super.newFile(path, normalizeSource(content));
     _addAnalyzedFileToDrivers(file);
     return file;
   }
 
+  /// Convenience function to normalize newlines in [code] for the current
+  /// platform if [useLineEndingsForPlatform] is `true`.
+  String normalizeSource(String code) =>
+      useLineEndingsForPlatform ? normalizeNewlinesForPlatform(code) : code;
+
   Future<AnalysisSession> sessionFor(File file) async {
     var analysisContext = _contextFor(file);
     await analysisContext.applyPendingFileChanges();
@@ -237,6 +246,11 @@ class AbstractContextTest
 
   void verifyCreatedCollection() {}
 
+  /// Writes string content as an analysis options file.
+  void writeAnalysisOptionsFile(String content) {
+    newFile(analysisOptionsPath, content);
+  }
+
   void _addAnalyzedFilesToDrivers() {
     for (var analysisContext in _analysisContextCollection!.contexts) {
       for (var path in analysisContext.contextRoot.analyzedFiles()) {

pkg/analysis_server/test/abstract_single_unit.dart

@@ -10,17 +10,13 @@ import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/src/error/codes.g.dart';
 import 'package:analyzer/src/test_utilities/find_element.dart';
 import 'package:analyzer/src/test_utilities/find_node.dart';
-import 'package:analyzer/src/test_utilities/platform.dart';
 import 'package:test/test.dart';
 
 import 'abstract_context.dart';
 
 class AbstractSingleUnitTest extends AbstractContextTest {
   bool verifyNoTestUnitErrors = true;
 
-  /// Whether to rewrite line endings in test code based on platform.
-  bool useLineEndingsForPlatform = false;
-
   late String testCode;
   late ParsedUnitResult testParsedResult;
   late ResolvedUnitResult testAnalysisResult;
@@ -31,7 +27,6 @@ class AbstractSingleUnitTest extends AbstractContextTest {
   late FindElement findElement;
 
   void addTestSource(String code) {
-    code = normalizeSource(code);
     testCode = code;
     newFile(testFile.path, code);
   }
@@ -81,17 +76,6 @@ class AbstractSingleUnitTest extends AbstractContextTest {
     return result;
   }
 
-  @override
-  File newFile(String path, String content) {
-    content = normalizeSource(content);
-    return super.newFile(path, content);
-  }
-
-  /// Convenient function to normalize newlines in [code] for the current
-  /// platform if [useLineEndingsForPlatform] is `true`.
-  String normalizeSource(String code) =>
-      useLineEndingsForPlatform ? normalizeNewlinesForPlatform(code) : code;
-
   Future<void> parseTestCode(String code) async {
     addTestSource(code);
     await getParsedUnit(testFile);

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

@@ -21,6 +21,12 @@ class IgnoreDiagnosticAnaylsisOptionFileTest extends FixProcessorTest {
   @override
   FixKind get kind => DartFixKind.IGNORE_ERROR_ANALYSIS_FILE;
 
+  @override
+  void setUp() {
+    useLineEndingsForPlatform = true;
+    super.setUp();
+  }
+
   Future<void> test_addFixToExistingErrorMap() async {
     createAnalysisOptionsFile(
       errors: {'unused_label': 'ignore'},
@@ -44,31 +50,30 @@ analyzer:
 
   Future<void> test_emptyAnalysisOptionsFile() async {
     // This overwrites the file created by `super.setUp` method.
-    resourceProvider.getFile(analysisOptionsPath).writeAsStringSync('');
+    writeBlankAnalysisOptionsFile();
 
     await resolveTestCode('''
-  void f() {
-    var a = 1;
-  }
-  ''');
+void f() {
+  var a = 1;
+}
+''');
     await assertHasFix(
       '''
 analyzer:
   errors:
-    unused_local_variable: ignore''',
+    unused_local_variable: ignore
+''',
       target: analysisOptionsPath,
     );
   }
 
   Future<void> test_invalidAnalysisOptionsFormat() async {
     // This overwrites the file created by `super.setUp` method.
     // Note: a label without a value is an `invalid_section_format` for dart.
-    resourceProvider.getFile(analysisOptionsPath).writeAsStringSync(
-      '''
+    writeAnalysisOptionsFile('''
 analyzer:
   linter:
-''',
-    );
+''');
 
     await resolveTestCode('''
   void f() {
@@ -88,15 +93,15 @@ analyzer:
     // This deletes the file created by `super.setUp` method.
     resourceProvider.getFile(analysisOptionsPath).delete();
     await resolveTestCode('''
-  void f() {
-    var a = 1;
-  }
-  ''');
+void f() {
+  var a = 1;
+}
+''');
     await assertNoFix();
   }
 
   Future<void> test_noAnalyzerLabel() async {
-    createAnalysisOptionsFile();
+    writeBlankAnalysisOptionsFile();
 
     await resolveTestCode('''
 void f() {
@@ -107,7 +112,8 @@ void f() {
       '''
 analyzer:
   errors:
-    unused_local_variable: ignore''',
+    unused_local_variable: ignore
+''',
       target: analysisOptionsPath,
     );
   }
@@ -151,9 +157,12 @@ void f() {
 
   Future<void> test_onlyIncludeLabel() async {
     // This overwrites the file created by `super.setUp` method.
-    resourceProvider.getFile(analysisOptionsPath).writeAsStringSync(
-          'include: package:lints/recommended.yaml',
-        );
+    // Having a newline is important because yaml_edit copies existing
+    // newlines and we want to test the current platforms EOLs.
+    // The content is normalized in newFile().
+    writeAnalysisOptionsFile('''
+include: package:lints/recommended.yaml
+''');
 
     await resolveTestCode('''
   void f() {
@@ -165,7 +174,8 @@ void f() {
 analyzer:
   errors:
     unused_local_variable: ignore
-include: package:lints/recommended.yaml''',
+include: package:lints/recommended.yaml
+''',
       target: analysisOptionsPath,
     );
   }
@@ -183,6 +193,14 @@ void f() {
 ''');
     await assertNoFix();
   }
+
+  void writeBlankAnalysisOptionsFile() {
+    // Include a newline because yaml_edit will copy existing newlines and
+    // this newline will be normalized for the current platform. Without it,
+    // yaml_edit will produce new content using \n on Windows which may not
+    // match the expectations here depending on Git EOL settings.
+    writeAnalysisOptionsFile('\n');
+  }
 }
 
 @reflectiveTest