[analysis_server] Bump yaml_edit, remove ignore_diagnostic workaround
This bumps yaml_edit to get a fix from https://github.com/dart-lang/yaml_edit/pull/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 <brianwilkerson@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/ignore_diagnostic.dart b/pkg/analysis_server/lib/src/services/correction/dart/ignore_diagnostic.dart
index 805dc76..720599c 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/ignore_diagnostic.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/ignore_diagnostic.dart
@@ -8,7 +8,6 @@
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 @@
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);
});
}
diff --git a/pkg/analysis_server/test/abstract_context.dart b/pkg/analysis_server/test/abstract_context.dart
index 0fd5f2e..f8f4304 100644
--- a/pkg/analysis_server/test/abstract_context.dart
+++ b/pkg/analysis_server/test/abstract_context.dart
@@ -15,6 +15,7 @@
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 @@
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 @@
}
}
- newFile(analysisOptionsPath, buffer.toString());
+ writeAnalysisOptionsFile(buffer.toString());
}
/// Returns the existing analysis driver that should be used to analyze the
@@ -191,11 +195,16 @@
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 @@
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()) {
diff --git a/pkg/analysis_server/test/abstract_single_unit.dart b/pkg/analysis_server/test/abstract_single_unit.dart
index e119408..e10b0d0 100644
--- a/pkg/analysis_server/test/abstract_single_unit.dart
+++ b/pkg/analysis_server/test/abstract_single_unit.dart
@@ -10,7 +10,6 @@
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';
@@ -18,9 +17,6 @@
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 @@
late FindElement findElement;
void addTestSource(String code) {
- code = normalizeSource(code);
testCode = code;
newFile(testFile.path, code);
}
@@ -81,17 +76,6 @@
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);
diff --git a/pkg/analysis_server/test/src/services/correction/fix/ignore_diagnostic_test.dart b/pkg/analysis_server/test/src/services/correction/fix/ignore_diagnostic_test.dart
index c739414..99eb1215 100644
--- a/pkg/analysis_server/test/src/services/correction/fix/ignore_diagnostic_test.dart
+++ b/pkg/analysis_server/test/src/services/correction/fix/ignore_diagnostic_test.dart
@@ -21,6 +21,12 @@
@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,18 +50,19 @@
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,
);
}
@@ -63,12 +70,10 @@
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 @@
// 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 @@
'''
analyzer:
errors:
- unused_local_variable: ignore''',
+ unused_local_variable: ignore
+''',
target: analysisOptionsPath,
);
}
@@ -151,9 +157,12 @@
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 @@
analyzer:
errors:
unused_local_variable: ignore
-include: package:lints/recommended.yaml''',
+include: package:lints/recommended.yaml
+''',
target: analysisOptionsPath,
);
}
@@ -183,6 +193,14 @@
''');
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
