[analysis_server] Change some LSP base test classes to mixins

This changes two classes that were previously used as base classes for tests to mixins, which allows both of them to be applied to the same class (which is required for some upcoming shared Code Action tests, since one of the base classes provides the shared interface and the other provides CodeActions helpers).

As part of this, I also inlined the setUp() method from the original base class and reduced it to just what's required for each testing, including removing `failTestOnErrorDiagnostic = false` from some tests, and fixing up some unintended errors in them.

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

Author: DanTup

pkg/analysis_server/test/lsp/code_actions_assists_test.dart

@@ -8,6 +8,7 @@ import 'package:analysis_server/lsp_protocol/protocol.dart';
 import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analysis_server/src/lsp/extensions/code_action.dart';
+import 'package:analysis_server/src/services/correction/assist_internal.dart';
 import 'package:analyzer/src/test_utilities/test_code_format.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart' as plugin;
 import 'package:analyzer_plugin/protocol/protocol_generated.dart' as plugin;
@@ -16,7 +17,8 @@ import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import '../utils/lsp_protocol_extensions.dart';
 import '../utils/test_code_extensions.dart';
-import 'code_actions_abstract.dart';
+import 'code_actions_mixin.dart';
+import 'server_abstract.dart';
 
 void main() {
   defineReflectiveSuite(() {
@@ -25,12 +27,19 @@ void main() {
 }
 
 @reflectiveTest
-class AssistsCodeActionsTest extends AbstractCodeActionsTest {
+class AssistsCodeActionsTest extends AbstractLspAnalysisServerTest
+    with CodeActionsTestMixin {
   @override
   void setUp() {
     super.setUp();
-    writeTestPackageConfig(flutter: true);
+
+    setApplyEditSupport();
+    setDocumentChangesSupport();
     setSupportedCodeActionKinds([CodeActionKind.Refactor]);
+
+    registerBuiltInAssistGenerators();
+
+    writeTestPackageConfig(flutter: true);
   }
 
   Future<void> test_appliesCorrectEdits_withDocumentChangesSupport() async {
@@ -256,6 +265,8 @@ Future? f;
   }
 
   Future<void> test_plugin() async {
+    failTestOnErrorDiagnostic = false;
+
     if (!AnalysisServer.supportsPlugins) return;
     // This code should get an assist to replace 'foo' with 'bar'.'
     const content = '''

pkg/analysis_server/test/lsp/code_actions_fixes_test.dart

@@ -5,6 +5,7 @@
 import 'package:analysis_server/lsp_protocol/protocol.dart';
 import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/lsp/extensions/code_action.dart';
+import 'package:analysis_server/src/services/correction/fix_internal.dart';
 import 'package:analyzer/src/dart/error/lint_codes.dart';
 import 'package:analyzer/src/lint/linter.dart';
 import 'package:analyzer/src/lint/registry.dart';
@@ -16,7 +17,8 @@ import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import '../utils/test_code_extensions.dart';
-import 'code_actions_abstract.dart';
+import 'code_actions_mixin.dart';
+import 'server_abstract.dart';
 
 void main() {
   defineReflectiveSuite(() {
@@ -46,7 +48,8 @@ class DeprecatedCamelCaseTypes extends LintRule {
 }
 
 @reflectiveTest
-class FixesCodeActionsTest extends AbstractCodeActionsTest {
+class FixesCodeActionsTest extends AbstractLspAnalysisServerTest
+    with CodeActionsTestMixin {
   /// Helper to check plugin fixes for [filePath].
   ///
   /// Used to ensure that both Dart and non-Dart files fixes are returned.
@@ -108,7 +111,15 @@ bar
   @override
   void setUp() {
     super.setUp();
+
+    // Fix tests are likely to have diagnostics that need fixing.
+    failTestOnErrorDiagnostic = false;
+
+    setApplyEditSupport();
+    setDocumentChangesSupport();
     setSupportedCodeActionKinds([CodeActionKind.QuickFix]);
+
+    registerBuiltInFixGenerators();
   }
 
   Future<void> test_addImport_noPreference() async {

pkg/analysis_server/test/lsp/code_actions_abstract.dart renamed to pkg/analysis_server/test/lsp/code_actions_mixin.dart

@@ -5,8 +5,6 @@
 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:analysis_server/src/services/correction/assist_internal.dart';
-import 'package:analysis_server/src/services/correction/fix_internal.dart';
 import 'package:analyzer/src/test_utilities/test_code_format.dart';
 import 'package:collection/collection.dart';
 import 'package:test/test.dart';
@@ -16,7 +14,7 @@ import '../utils/test_code_extensions.dart';
 import 'change_verifier.dart';
 import 'server_abstract.dart';
 
-abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
+mixin CodeActionsTestMixin on AbstractLspAnalysisServerTest {
   /// Initializes the server with some basic configuration and expects to find
   /// a [CodeAction] with [kind]/[command]/[title].
   Future<CodeAction> expectCodeAction(
@@ -236,21 +234,6 @@ abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
     return null;
   }
 
-  @override
-  void setUp() {
-    super.setUp();
-
-    // Fix tests are likely to have diagnostics that need fixing.
-    failTestOnErrorDiagnostic = false;
-
-    // Some defaults that most tests use. Tests can opt-out by overwriting these
-    // before initializing.
-    setApplyEditSupport();
-    setDocumentChangesSupport();
-    registerBuiltInAssistGenerators();
-    registerBuiltInFixGenerators();
-  }
-
   /// Verifies that executing the given Code Action (either via a command or
   /// an inline edit) results in the files matching the expected content.
   Future<LspChangeVerifier> verifyCodeActionEdits(

pkg/analysis_server/test/lsp/code_actions_refactor_test.dart

@@ -13,8 +13,9 @@ import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import '../tool/lsp_spec/matchers.dart';
 import '../utils/test_code_extensions.dart';
-import 'code_actions_abstract.dart';
+import 'code_actions_mixin.dart';
 import 'request_helpers_mixin.dart';
+import 'server_abstract.dart';
 
 void main() {
   defineReflectiveSuite(() {
@@ -222,7 +223,7 @@ void f() {
     try {
       // Send an edit request immediately after the refactor request.
       var req1 = executeCommand(codeAction.command!);
-      var req2 = replaceFile(100, mainFileUri, 'new test content');
+      var req2 = replaceFile(100, mainFileUri, '// new test content');
       completer.complete();
 
       // Expect the first to fail because of the modified content.
@@ -264,22 +265,22 @@ void f() {
 
   Future<void> test_generatesNames() async {
     const content = '''
-Object F() {
+Object? F() {
   return Container([!Text('Test!')!]);
 }
 
-Object Container(Object text) => null;
-Object Text(Object text) => null;
+Object? Container(Object? text) => null;
+Object? Text(Object? text) => null;
 ''';
     const expectedContent = '''
-Object F() {
+Object? F() {
   return Container(text());
 }
 
-Object text() => Text('Test!');
+Object? text() => Text('Test!');
 
-Object Container(Object text) => null;
-Object Text(Object text) => null;
+Object? Container(Object? text) => null;
+Object? Text(Object? text) => null;
 ''';
 
     await verifyCodeActionLiteralEdits(
@@ -308,7 +309,7 @@ void f() {}
     const content = '''
 import 'dart:io' as io;
 
-i^o.File a;
+i^o.File? a;
 ''';
 
     await expectNoAction(
@@ -977,10 +978,14 @@ void foo2() {
   }
 }
 
-abstract class RefactorCodeActionsTest extends AbstractCodeActionsTest {
+abstract class RefactorCodeActionsTest extends AbstractLspAnalysisServerTest
+    with CodeActionsTestMixin {
   @override
   void setUp() {
     super.setUp();
+
+    setApplyEditSupport();
+    setDocumentChangesSupport();
     setSupportedCodeActionKinds([CodeActionKind.Refactor]);
   }
 }

pkg/analysis_server/test/lsp/code_actions_source_test.dart

@@ -6,12 +6,14 @@ import 'dart:async';
 
 import 'package:analysis_server/lsp_protocol/protocol.dart';
 import 'package:analysis_server/src/lsp/constants.dart';
+import 'package:analysis_server/src/services/correction/fix_internal.dart';
 import 'package:linter/src/rules.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import '../tool/lsp_spec/matchers.dart';
-import 'code_actions_abstract.dart';
+import 'code_actions_mixin.dart';
+import 'server_abstract.dart';
 
 void main() {
   defineReflectiveSuite(() {
@@ -21,7 +23,9 @@ void main() {
   });
 }
 
-abstract class AbstractSourceCodeActionsTest extends AbstractCodeActionsTest {
+abstract class AbstractSourceCodeActionsTest
+    extends AbstractLspAnalysisServerTest
+    with CodeActionsTestMixin {
   /// For convenience since source code actions do not rely on a position (but
   /// one must be provided), uses [startOfDocPos] to avoid every test needing
   /// to include a '^' marker.
@@ -46,12 +50,25 @@ abstract class AbstractSourceCodeActionsTest extends AbstractCodeActionsTest {
   @override
   void setUp() {
     super.setUp();
+
+    setApplyEditSupport();
+    setDocumentChangesSupport();
     setSupportedCodeActionKinds([CodeActionKind.Source]);
   }
 }
 
 @reflectiveTest
 class FixAllSourceCodeActionsTest extends AbstractSourceCodeActionsTest {
+  @override
+  void setUp() {
+    super.setUp();
+
+    // Fix tests are likely to have diagnostics that need fixing.
+    failTestOnErrorDiagnostic = false;
+
+    registerBuiltInFixGenerators();
+  }
+
   Future<void> test_appliesCorrectEdits() async {
     const analysisOptionsContent = '''
 linter:
@@ -351,14 +368,14 @@ import 'dart:math';
 import 'dart:async';
 import 'dart:convert';
 
-Completer foo;
+Completer? foo;
 int minified(int x, int y) => min(x, y);
 ''';
     const expectedContent = '''
 import 'dart:async';
 import 'dart:math';
 
-Completer foo;
+Completer? foo;
 int minified(int x, int y) => min(x, y);
 ''';
 
@@ -375,14 +392,14 @@ import 'dart:math';
 import 'dart:async';
 import 'dart:convert';
 
-Completer foo;
+Completer? foo;
 int minified(int x, int y) => min(x, y);
 ''';
     const expectedContent = '''
 import 'dart:async';
 import 'dart:math';
 
-Completer foo;
+Completer? foo;
 int minified(int x, int y) => min(x, y);
 ''';
 
@@ -414,6 +431,7 @@ int minified(int x, int y) => min(x, y);
   }
 
   Future<void> test_fileHasErrors_failsSilentlyForAutomatic() async {
+    failTestOnErrorDiagnostic = false;
     var content = 'invalid dart code';
 
     var codeAction = await expectCodeActionLiteral(
@@ -429,6 +447,7 @@ int minified(int x, int y) => min(x, y);
   }
 
   Future<void> test_fileHasErrors_failsWithErrorForManual() async {
+    failTestOnErrorDiagnostic = false;
     var content = 'invalid dart code';
 
     var codeAction = await expectCodeActionLiteral(
@@ -464,7 +483,7 @@ int minified(int x, int y) => min(x, y);
 import 'dart:async';
 import 'dart:math';
 
-Completer foo;
+Completer? foo;
 int minified(int x, int y) => min(x, y);
 ''';
 
@@ -500,12 +519,12 @@ int minified(int x, int y) => min(x, y);
 class SortMembersSourceCodeActionsTest extends AbstractSourceCodeActionsTest {
   Future<void> test_appliesCorrectEdits_withDocumentChangesSupport() async {
     const content = '''
-String b;
-String a;
+String? b;
+String? a;
 ''';
     const expectedContent = '''
-String a;
-String b;
+String? a;
+String? b;
 ''';
 
     await verifyCodeActionLiteralEdits(
@@ -517,12 +536,12 @@ String b;
 
   Future<void> test_appliesCorrectEdits_withoutDocumentChangesSupport() async {
     const content = '''
-String b;
-String a;
+String? b;
+String? a;
 ''';
     const expectedContent = '''
-String a;
-String b;
+String? a;
+String? b;
 ''';
 
     setDocumentChangesSupport(false);
@@ -554,8 +573,8 @@ String b;
 
   Future<void> test_failsIfClientDoesntApplyEdits() async {
     const content = '''
-String b;
-String a;
+String? b;
+String? a;
 ''';
 
     var codeAction = await expectCodeActionLiteral(
@@ -591,6 +610,7 @@ String a;
   }
 
   Future<void> test_fileHasErrors_failsSilentlyForAutomatic() async {
+    failTestOnErrorDiagnostic = false;
     var content = 'invalid dart code';
 
     var codeAction = await expectCodeActionLiteral(
@@ -606,6 +626,7 @@ String a;
   }
 
   Future<void> test_fileHasErrors_failsWithErrorForManual() async {
+    failTestOnErrorDiagnostic = false;
     var content = 'invalid dart code';
 
     var codeAction = await expectCodeActionLiteral(