[analyzer] Use LinkedHashSet for recording errors to keep order consistent

Fixes https://github.com/Dart-Code/Dart-Code/issues/3934.

Change-Id: I1cd9fd35ddbbd768877e6b2b2984056ed5ec5d9c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/241741
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>

pkg/analysis_server/test/integration/server/bazel_changes_test.dart

@@ -127,7 +127,9 @@ void main() { my_fun(); }
 
     await processedNotification.future;
     expect(errors, isNotEmpty);
-    expect(errors[0].message, contains('generated.dart'));
+    var error = errors.singleWhere((e) => e.code == 'uri_does_not_exist',
+        orElse: () => throw "'uri_does_not_exist' error was not found");
+    expect(error.message, contains('generated.dart'));
 
     // This seems to be necessary (at least when running the test from source),
     // because it takes a while for the watcher isolate to start.

pkg/analysis_server/test/lsp/diagnostic_test.dart

@@ -5,6 +5,7 @@
 import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart' as plugin;
 import 'package:analyzer_plugin/protocol/protocol_generated.dart' as plugin;
+import 'package:linter/src/rules.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
@@ -378,6 +379,71 @@ version: latest
     }
   }
 
+  /// Tests that diagnostic ordering is stable when minor changes are made to
+  /// the file that does not alter the diagnostics besides extending their
+  /// range and adding to their messages.
+  ///
+  /// https://github.com/Dart-Code/Dart-Code/issues/3934
+  Future<void> test_stableOrder() async {
+    /// Helper to pad out the content in a way that has previously triggered
+    /// this issue.
+    String wrappedContent(String content) => '''
+//
+//
+//
+//
+
+void f() {
+  $content
+}
+''';
+
+    registerLintRules();
+    newFile(analysisOptionsPath, '''
+linter:
+  rules:
+    - prefer_typing_uninitialized_variables
+
+analyzer:
+  language:
+    strict-inference: true
+    ''');
+
+    newFile(mainFilePath, '');
+    await initialize();
+    await openFile(mainFileUri, '');
+
+    // Collect the initial set of diagnostic to compare against.
+    var docVersion = 1;
+    final originalDiagnosticsUpdate = waitForDiagnostics(mainFileUri);
+    await replaceFile(docVersion++, mainFileUri, wrappedContent('final bar;'));
+    final originalDiagnostics = await originalDiagnosticsUpdate;
+
+    // Helper to update the content and verify the same diagnostics are returned
+    // in the same order, despite the changes to offset/message altering
+    // hashcodes.
+    Future<void> verifyDiagnostics(String content) async {
+      final diagnosticsUpdate = waitForDiagnostics(mainFileUri);
+      await replaceFile(docVersion++, mainFileUri, wrappedContent(content));
+      final diagnostics = await diagnosticsUpdate;
+      expect(
+        diagnostics!.map((d) => d.code),
+        originalDiagnostics!.map((d) => d.code),
+      );
+    }
+
+    // These changes do not affect the errors being produced (besides offset/
+    // message text) but will cause hashcode changes that previously altered the
+    // returned order.
+    await verifyDiagnostics('final dbar;');
+    await verifyDiagnostics('final dybar;');
+    await verifyDiagnostics('final dynbar;');
+    await verifyDiagnostics('final dynabar;');
+    await verifyDiagnostics('final dynambar;');
+    await verifyDiagnostics('final dynamibar;');
+    await verifyDiagnostics('final dynamicbar;');
+  }
+
   Future<void> test_todos_boolean() async {
     // TODOs only show up if there's also some code in the file.
     const contents = '''

pkg/analyzer/lib/error/listener.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:collection';
-
 import 'package:analyzer/dart/ast/ast.dart'
     show AstNode, ConstructorDeclaration;
 import 'package:analyzer/dart/ast/token.dart';
@@ -257,7 +255,7 @@ class RecordingErrorListener implements AnalysisErrorListener {
 
   @override
   void onError(AnalysisError error) {
-    (_errors ??= HashSet<AnalysisError>()).add(error);
+    (_errors ??= {}).add(error);
   }
 }
 

pkg/analyzer/test/error/error_listener_test.dart

@@ -0,0 +1,46 @@
+// Copyright (c) 2022, 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 'package:analyzer/error/error.dart';
+import 'package:analyzer/error/listener.dart';
+import 'package:test/test.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(RecordingErrorListenerTest);
+  });
+}
+
+@reflectiveTest
+class RecordingErrorListenerTest {
+  test_orderedAsReported() {
+    final listener = RecordingErrorListener();
+    listener.onError(_MockAnalysisError(expectedIndex: 0, hashCode: 1));
+    listener.onError(_MockAnalysisError(expectedIndex: 1, hashCode: 10));
+    listener.onError(_MockAnalysisError(expectedIndex: 2, hashCode: -50));
+    listener.onError(_MockAnalysisError(expectedIndex: 3, hashCode: 20));
+    listener.onError(_MockAnalysisError(expectedIndex: 4, hashCode: 1));
+
+    // Expect the errors are returned in the order they are reported, and not
+    // affected by their hashcodes.
+    expect(
+      listener.errors.cast<_MockAnalysisError>().map((e) => e.expectedIndex),
+      [0, 1, 2, 3, 4],
+    );
+  }
+}
+
+/// An [AnalysisError] that allows setting an explicit hash code.
+class _MockAnalysisError implements AnalysisError {
+  @override
+  int hashCode;
+
+  int expectedIndex;
+
+  _MockAnalysisError({required this.expectedIndex, required this.hashCode});
+
+  @override
+  dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
+}

pkg/analyzer/test/error/test_all.dart

@@ -4,11 +4,13 @@
 
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
+import 'error_listener_test.dart' as error_listener;
 import 'error_reporter_test.dart' as error_reporter;
 import 'error_test.dart' as error_test;
 
 main() {
   defineReflectiveSuite(() {
+    error_listener.main();
     error_reporter.main();
     error_test.main();
   }, name: 'error');