Extract availability analysis from selection analysis.

Danny gave a good idea that checking for availability of a refactoring
should be cheap. Change method signature refactoring mostly satisfied
this, with one exception - we cannot compute formal parameters for
an ExecutableElement, because we need resolved AST to do this, and
the invoked method can be declared in a different file. We cannot
afford resolving other files while checking.

So, this CL separates availability checking, and preparing formal
parameters, postponing expensive opertions until the time when the
action is invoked.

Change-Id: Ic703d70717e41de304c9dbcef66cadb22c139ad8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310041
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

pkg/analysis_server/lib/src/services/refactoring/agnostic/change_method_signature.dart

@@ -26,12 +26,22 @@ import 'package:analyzer_plugin/utilities/range_factory.dart';
 import 'package:collection/collection.dart';
 
 /// Analyzes the selection in [refactoringContext], and either returns
+/// a [Available], or [NotAvailable].
+Availability analyzeAvailability({
+  required AbstractRefactoringContext refactoringContext,
+}) {
+  return _AvailabilityAnalyzer(
+    refactoringContext: refactoringContext,
+  ).analyze();
+}
+
+/// Continues analysis of the selection in [available], and returns either
 /// a [ValidSelectionState], or one of [ErrorSelectionState] subtypes.
 Future<SelectionState> analyzeSelection({
-  required AbstractRefactoringContext refactoringContext,
+  required Available available,
 }) async {
   return _SelectionAnalyzer(
-    refactoringContext: refactoringContext,
+    available: available,
   ).analyze();
 }
 
@@ -53,6 +63,16 @@ Future<ChangeStatus> computeSourceChange({
   );
 }
 
+sealed class Availability {}
+
+sealed class Available extends Availability {
+  final AbstractRefactoringContext refactoringContext;
+
+  Available({
+    required this.refactoringContext,
+  });
+}
+
 /// The supertype return types from [computeSourceChange].
 sealed class ChangeStatus {}
 
@@ -163,6 +183,8 @@ final class NoExecutableElementSelectionState extends ErrorSelectionState {
   const NoExecutableElementSelectionState();
 }
 
+final class NotAvailable extends Availability {}
+
 /// The supertype for all results of [analyzeSelection].
 sealed class SelectionState {
   const SelectionState();
@@ -209,102 +231,34 @@ final class ValidSelectionState extends SelectionState {
   });
 }
 
-/// The target method declaration.
-class _Declaration {
-  final ExecutableElement element;
-  final AstNode node;
-  final List<FormalParameter> selected;
-
-  _Declaration({
-    required this.element,
-    required this.node,
-    required this.selected,
-  });
-}
-
-/// Formal parameters of a declaration that match the selection.
-final class _DeclarationFormalParameters {
-  final List<FormalParameter> positional;
-  final Map<String, FormalParameter> named;
-
-  _DeclarationFormalParameters({
-    required this.positional,
-    required this.named,
-  });
-}
-
-/// The class that implements [analyzeSelection].
-class _SelectionAnalyzer {
+class _AvailabilityAnalyzer {
   final AbstractRefactoringContext refactoringContext;
 
-  _SelectionAnalyzer({
+  _AvailabilityAnalyzer({
     required this.refactoringContext,
   });
 
-  Future<SelectionState> analyze() async {
-    final declaration = await _declaration();
-    if (declaration == null) {
-      return const NoExecutableElementSelectionState();
-    }
-
-    final parameterNodeList = declaration.node.formalParameterList;
-    if (parameterNodeList == null) {
-      return const NoExecutableElementSelectionState();
-    }
-
-    final formalParameterStateList = <FormalParameterState>[];
-    var formalParameterId = 0;
-    var positionalIndex = 0;
-    for (final parameterNode in parameterNodeList.parameters) {
-      final nameToken = parameterNode.name;
-      if (nameToken == null) {
-        return const UnexpectedSelectionState();
-      }
-
-      FormalParameterKind kind;
-      if (parameterNode.isRequiredPositional) {
-        kind = FormalParameterKind.requiredPositional;
-      } else if (parameterNode.isOptionalPositional) {
-        kind = FormalParameterKind.optionalPositional;
-      } else if (parameterNode.isRequiredNamed) {
-        kind = FormalParameterKind.requiredNamed;
-      } else if (parameterNode.isOptionalNamed) {
-        kind = FormalParameterKind.optionalNamed;
-      } else {
-        // This branch is never reached.
-        return const UnexpectedSelectionState();
-      }
-
-      // TODO(scheglov) Rework this when adding support for constructors.
-      TypeAnnotation? typeNode;
-      final notDefault = parameterNode.notDefault;
-      if (notDefault is SimpleFormalParameter) {
-        typeNode = notDefault.type;
-      }
-      if (typeNode == null) {
-        return const UnexpectedSelectionState();
-      }
-
-      formalParameterStateList.add(
-        FormalParameterState(
-          id: formalParameterId++,
-          kind: kind,
-          positionalIndex: kind.isPositional ? positionalIndex++ : null,
-          name: nameToken.lexeme,
-          typeStr: refactoringContext.utils.getNodeText(typeNode),
-          isSelected: declaration.selected.contains(parameterNode),
-        ),
+  Availability analyze() {
+    final declaration = _declaration();
+    if (declaration != null) {
+      return _AvailableWithDeclaration(
+        refactoringContext: refactoringContext,
+        declaration: declaration,
       );
     }
 
-    return ValidSelectionState(
-      refactoringContext: refactoringContext,
-      element: declaration.element,
-      formalParameters: formalParameterStateList,
-    );
+    final executableElement = _executableElement();
+    if (executableElement != null) {
+      return _AvailableWithExecutableElement(
+        refactoringContext: refactoringContext,
+        element: executableElement,
+      );
+    }
+
+    return NotAvailable();
   }
 
-  Future<_Declaration?> _declaration() async {
+  _Declaration? _declaration() {
     final coveringNode = refactoringContext.coveringNode;
 
     switch (coveringNode) {
@@ -314,40 +268,11 @@ class _SelectionAnalyzer {
         return _declarationFormalParameterList(coveringNode);
     }
 
-    final atExecutable = _declarationExecutable(
+    return _declarationExecutable(
       node: coveringNode,
       anyLocation: false,
       selected: const [],
     );
-    if (atExecutable != null) {
-      return atExecutable;
-    }
-
-    Element? element;
-    if (coveringNode is SimpleIdentifier) {
-      final invocation = coveringNode.parent;
-      if (invocation is MethodInvocation &&
-          invocation.methodName == coveringNode) {
-        element = invocation.methodName.staticElement;
-      }
-    }
-    if (element is! ExecutableElement) {
-      return null;
-    }
-
-    // TODO(scheglov) Check that element is in an editable library.
-    final declarationResult =
-        await refactoringContext.sessionHelper.getElementDeclaration(element);
-    final node = declarationResult?.node;
-    if (node == null) {
-      return null;
-    }
-
-    return _Declaration(
-      element: element,
-      node: node,
-      selected: const [],
-    );
   }
 
   _Declaration? _declarationExecutable({
@@ -430,6 +355,169 @@ class _SelectionAnalyzer {
       selected: selected,
     );
   }
+
+  ExecutableElement? _executableElement() {
+    final coveringNode = refactoringContext.coveringNode;
+
+    Element? element;
+    if (coveringNode is SimpleIdentifier) {
+      final invocation = coveringNode.parent;
+      if (invocation is MethodInvocation &&
+          invocation.methodName == coveringNode) {
+        element = invocation.methodName.staticElement;
+      }
+    }
+
+    if (element is! ExecutableElement) {
+      return null;
+    }
+
+    // TODO(scheglov) Check that element is in an editable library.
+    return element;
+  }
+}
+
+final class _AvailableWithDeclaration extends Available {
+  final _Declaration declaration;
+
+  _AvailableWithDeclaration({
+    required super.refactoringContext,
+    required this.declaration,
+  });
+}
+
+final class _AvailableWithExecutableElement extends Available {
+  final ExecutableElement element;
+
+  _AvailableWithExecutableElement({
+    required super.refactoringContext,
+    required this.element,
+  });
+}
+
+/// The target method declaration.
+class _Declaration {
+  final ExecutableElement element;
+  final AstNode node;
+  final List<FormalParameter> selected;
+
+  _Declaration({
+    required this.element,
+    required this.node,
+    required this.selected,
+  });
+}
+
+/// Formal parameters of a declaration that match the selection.
+final class _DeclarationFormalParameters {
+  final List<FormalParameter> positional;
+  final Map<String, FormalParameter> named;
+
+  _DeclarationFormalParameters({
+    required this.positional,
+    required this.named,
+  });
+}
+
+/// The class that implements [analyzeSelection].
+class _SelectionAnalyzer {
+  final Available available;
+
+  _SelectionAnalyzer({
+    required this.available,
+  });
+
+  AbstractRefactoringContext get refactoringContext {
+    return available.refactoringContext;
+  }
+
+  Future<SelectionState> analyze() async {
+    final declaration = await _declaration();
+    if (declaration == null) {
+      return const NoExecutableElementSelectionState();
+    }
+
+    final parameterNodeList = declaration.node.formalParameterList;
+    if (parameterNodeList == null) {
+      return const NoExecutableElementSelectionState();
+    }
+
+    final formalParameterStateList = <FormalParameterState>[];
+    var formalParameterId = 0;
+    var positionalIndex = 0;
+    for (final parameterNode in parameterNodeList.parameters) {
+      final nameToken = parameterNode.name;
+      if (nameToken == null) {
+        return const UnexpectedSelectionState();
+      }
+
+      FormalParameterKind kind;
+      if (parameterNode.isRequiredPositional) {
+        kind = FormalParameterKind.requiredPositional;
+      } else if (parameterNode.isOptionalPositional) {
+        kind = FormalParameterKind.optionalPositional;
+      } else if (parameterNode.isRequiredNamed) {
+        kind = FormalParameterKind.requiredNamed;
+      } else if (parameterNode.isOptionalNamed) {
+        kind = FormalParameterKind.optionalNamed;
+      } else {
+        // This branch is never reached.
+        return const UnexpectedSelectionState();
+      }
+
+      // TODO(scheglov) Rework this when adding support for constructors.
+      TypeAnnotation? typeNode;
+      final notDefault = parameterNode.notDefault;
+      if (notDefault is SimpleFormalParameter) {
+        typeNode = notDefault.type;
+      }
+      if (typeNode == null) {
+        return const UnexpectedSelectionState();
+      }
+
+      formalParameterStateList.add(
+        FormalParameterState(
+          id: formalParameterId++,
+          kind: kind,
+          positionalIndex: kind.isPositional ? positionalIndex++ : null,
+          name: nameToken.lexeme,
+          typeStr: refactoringContext.utils.getNodeText(typeNode),
+          isSelected: declaration.selected.contains(parameterNode),
+        ),
+      );
+    }
+
+    return ValidSelectionState(
+      refactoringContext: refactoringContext,
+      element: declaration.element,
+      formalParameters: formalParameterStateList,
+    );
+  }
+
+  /// Converts [available] into a [_Declaration].
+  Future<_Declaration?> _declaration() async {
+    switch (available) {
+      case _AvailableWithDeclaration(:final declaration):
+        return declaration;
+      case _AvailableWithExecutableElement(:final element):
+        final node = await _elementDeclaration(element);
+        if (node == null) {
+          return null;
+        }
+
+        return _Declaration(
+          element: element,
+          node: node,
+          selected: const [],
+        );
+    }
+  }
+
+  Future<AstNode?> _elementDeclaration(ExecutableElement element) async {
+    final helper = refactoringContext.sessionHelper;
+    final nodeResult = await helper.getElementDeclaration(element);
+    return nodeResult?.node;
+  }
 }
 
 /// The class that implements [computeSourceChange].
@@ -786,7 +874,7 @@ class _SignatureUpdater {
   /// For example, it is not allowed to have both optional positional, and
   /// any named formal parameters.
   ///
-  /// TODO(scheglov) check no required positional arter optional
+  /// TODO(scheglov) check no required positional after optional
   ChangeStatus validateFormalParameterUpdates() {
     final updates = signatureUpdate.formalParameters;
 

pkg/analysis_server/lib/src/services/refactoring/convert_formal_parameters.dart

@@ -33,9 +33,16 @@ class ConvertFormalParametersToNamed extends RefactoringProducer {
     List<Object?> commandArguments,
     ChangeBuilder builder,
   ) async {
-    final selection = await analyzeSelection(
+    final availability = analyzeAvailability(
       refactoringContext: refactoringContext,
     );
+    if (availability is! Available) {
+      return;
+    }
+
+    final selection = await analyzeSelection(
+      available: availability,
+    );
 
     if (selection is! ValidSelectionState) {
       return;
@@ -64,12 +71,10 @@ class ConvertFormalParametersToNamed extends RefactoringProducer {
   }
 
   @override
-  Future<bool> isAvailable() async {
-    final selection = await analyzeSelection(
+  bool isAvailable() {
+    final availability = analyzeAvailability(
       refactoringContext: refactoringContext,
     );
-    // TODO(scheglov) This is bad implementation.
-    // We should not recompute selection.
-    return selection is ValidSelectionState;
+    return availability is Available;
   }
 }

pkg/analysis_server/lib/src/services/refactoring/framework/refactoring_processor.dart

@@ -37,7 +37,7 @@ class RefactoringProcessor {
         continue;
       }
 
-      final isAvailable = await producer.isAvailable();
+      final isAvailable = producer.isAvailable();
       if (!isAvailable) {
         continue;
       }

pkg/analysis_server/lib/src/services/refactoring/framework/refactoring_producer.dart

@@ -86,7 +86,7 @@ abstract class RefactoringProducer {
   Future<void> compute(List<Object?> commandArguments, ChangeBuilder builder);
 
   /// Return `true` if this refactoring is available in the given context.
-  Future<bool> isAvailable();
+  bool isAvailable();
 
   /// Return `true` if the selection is inside the given [token].
   bool selectionIsInToken(Token? token) => _context.selectionIsInToken(token);

pkg/analysis_server/lib/src/services/refactoring/move_top_level_to_file.dart

@@ -166,7 +166,7 @@ class MoveTopLevelToFile extends RefactoringProducer {
   }
 
   @override
-  Future<bool> isAvailable() async {
+  bool isAvailable() {
     if (supportsFileCreation) {
       var members = _membersToMove();
       if (members != null) {

pkg/analysis_server/test/services/refactoring/agnostic/change_method_signature_test.dart

@@ -46,9 +46,17 @@ class AbstractChangeMethodSignatureTest extends AbstractContextTest {
       testCode: testCode,
     );
 
-    selectionState = await analyzeSelection(
+    final availability = analyzeAvailability(
       refactoringContext: refactoringContext,
     );
+    if (availability is! Available) {
+      selectionState = NoExecutableElementSelectionState();
+      return;
+    }
+
+    selectionState = await analyzeSelection(
+      available: availability,
+    );
   }
 
   Future<void> _analyzeValidSelection(String rawCode) async {