Convert one more KeywordContributor method

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

pkg/analysis_server/lib/src/services/completion/dart/in_scope_completion_pass.dart

@@ -11,6 +11,7 @@ import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/syntactic_entity.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/src/dart/ast/extensions.dart';
 import 'package:analyzer/src/dart/ast/token.dart';
 
 /// A completion pass that will create candidate suggestions based on the
@@ -335,6 +336,8 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
     if (defaultValue is Expression && defaultValue.coversOffset(offset)) {
       collector.completionLocation = 'DefaultFormalParameter_defaultValue';
       _forExpression(defaultValue);
+    } else {
+      node.parameter.accept(this);
     }
   }
 
@@ -537,8 +540,18 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
       var parent = node.parent;
       if (parent is FunctionExpression) {
         visitFunctionExpression(parent);
+        return;
       }
     }
+    var parameters = node.parameters;
+    var precedingParameter = parameters.elementBefore(offset);
+    if (precedingParameter != null && precedingParameter.isIncomplete) {
+      precedingParameter.accept(this);
+      return;
+    }
+
+    keywordHelper.addFormalParameterKeywords(node);
+    _forTypeAnnotation();
   }
 
   @override
@@ -637,6 +650,17 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
     }
   }
 
+  @override
+  void visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) {
+    var returnType = node.returnType;
+    if (returnType != null && offset <= returnType.end) {
+      keywordHelper.addFormalParameterKeywords(node.parentFormalParameterList);
+      _forTypeAnnotation();
+    } else if (returnType == null && offset < node.name.offset) {
+      _forTypeAnnotation();
+    }
+  }
+
   @override
   void visitGenericTypeAlias(GenericTypeAlias node) {
     if (node.typedefKeyword.coversOffset(offset)) {
@@ -1036,9 +1060,29 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
 
   @override
   void visitSimpleFormalParameter(SimpleFormalParameter node) {
+    var name = node.name;
+    if (name != null && node.isSingleIdentifier) {
+      if (name.isKeyword) {
+        if (name.keyword == Keyword.REQUIRED && node.covariantKeyword == null) {
+          keywordHelper.addKeyword(Keyword.COVARIANT);
+        }
+        _forTypeAnnotation();
+        return;
+      } else if (name.isSynthetic) {
+        keywordHelper
+            .addFormalParameterKeywords(node.parentFormalParameterList);
+        _forTypeAnnotation();
+      } else {
+        keywordHelper
+            .addFormalParameterKeywords(node.parentFormalParameterList);
+        _forTypeAnnotation();
+      }
+    }
     var type = node.type;
     if (type != null) {
       if (type.beginToken.coversOffset(offset)) {
+        keywordHelper
+            .addFormalParameterKeywords(node.parentFormalParameterList);
         _forTypeAnnotation();
       } else if (type is GenericFunctionType &&
           offset < type.functionKeyword.offset &&
@@ -1640,7 +1684,7 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
 }
 
 extension on AstNode {
-  /// Return `true` if all of the tokens in this node are synthetic.
+  /// Whether all of the tokens in this node are synthetic.
   bool get isFullySynthetic {
     var current = beginToken;
     var stop = endToken.next!;
@@ -1745,7 +1789,7 @@ extension on AstNode {
 }
 
 extension on ClassDeclaration {
-  /// Return `true` if this class declaration doesn't have a body.
+  /// Whether this class declaration doesn't have a body.
   bool get hasNoBody {
     return leftBracket.isSynthetic && rightBracket.isSynthetic;
   }
@@ -1810,7 +1854,7 @@ extension on CompilationUnit {
 }
 
 extension on ExpressionStatement {
-  /// Return `true` if this statement consists of a single identifier.
+  /// Whether this statement consists of a single identifier.
   bool get isSingleIdentifier {
     var first = beginToken;
     var last = endToken;
@@ -1821,14 +1865,14 @@ extension on ExpressionStatement {
 }
 
 extension on ExtensionTypeDeclaration {
-  /// Return `true` if this class declaration doesn't have a body.
+  /// Whether this class declaration doesn't have a body.
   bool get hasNoBody {
     return leftBracket.isSynthetic && rightBracket.isSynthetic;
   }
 }
 
 extension on FieldDeclaration {
-  /// Return `true` if this field declaration consists of a single identifier.
+  /// Whether this field declaration consists of a single identifier.
   bool get isSingleIdentifier {
     var first = beginToken;
     var last = endToken;
@@ -1838,8 +1882,35 @@ extension on FieldDeclaration {
   }
 }
 
+extension on FormalParameter {
+  /// Whether this formal parameter declaration is incomplete.
+  bool get isIncomplete {
+    final name = this.name;
+    if (name == null || name.isKeyword) {
+      return true;
+    }
+    var self = this;
+    if (self is DefaultFormalParameter && self.separator != null) {
+      var defaultValue = self.defaultValue;
+      if (defaultValue == null || defaultValue.isSynthetic) {
+        // The `defaultValue` won't be `null` if the separator is non-`null`,
+        // but the condition is necessary because the type system can't express
+        // that constraint.
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /// Whether this formal parameter declaration consists of a single identifier.
+  bool get isSingleIdentifier {
+    final beginToken = this.beginToken;
+    return beginToken == endToken && beginToken.isKeywordOrIdentifier;
+  }
+}
+
 extension on GuardedPattern {
-  /// Return `true` if this pattern has, or might have, a `when` keyword.
+  /// Whether this pattern has, or might have, a `when` keyword.
   bool get hasWhen {
     if (whenClause != null) {
       return true;
@@ -1883,7 +1954,7 @@ extension on SyntacticEntity? {
 }
 
 extension on TopLevelVariableDeclaration {
-  /// Return `true` if this top level variable declaration consists of a single
+  /// Whether this top level variable declaration consists of a single
   /// identifier.
   bool get isSingleIdentifier {
     var first = beginToken;
@@ -1896,7 +1967,7 @@ extension on TopLevelVariableDeclaration {
 }
 
 extension on TypeAnnotation? {
-  /// Return `true` if this type annotation consists of a single identifier.
+  /// Whether this type annotation consists of a single identifier.
   bool get isSingleIdentifier {
     var self = this;
     return self is NamedType &&

pkg/analysis_server/lib/src/services/completion/dart/keyword_contributor.dart

@@ -11,7 +11,6 @@ import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/syntactic_entity.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:analyzer/src/dart/ast/extensions.dart';
 import 'package:analyzer/src/util/performance/operation_performance.dart';
 import 'package:analyzer_plugin/src/utilities/completion/optype.dart';
 
@@ -103,87 +102,6 @@ class _KeywordVisitor extends SimpleAstVisitor<void> {
     }
   }
 
-  @override
-  void visitFormalParameterList(FormalParameterList node) {
-    var constructorDeclaration =
-        node.thisOrAncestorOfType<ConstructorDeclaration>();
-    if (constructorDeclaration != null) {
-      if (request.featureSet.isEnabled(Feature.super_parameters)) {
-        _addSuggestion(Keyword.SUPER);
-      }
-      _addSuggestion(Keyword.THIS);
-    }
-    final entity = this.entity;
-    if (entity is Token) {
-      FormalParameter? lastParameter() {
-        var parameters = node.parameters;
-        if (parameters.isNotEmpty) {
-          return parameters.last.notDefault;
-        }
-        return null;
-      }
-
-      bool hasCovariant() {
-        var last = lastParameter();
-        return last != null &&
-            (last.covariantKeyword != null || last.name?.lexeme == 'covariant');
-      }
-
-      bool hasRequired() {
-        var last = lastParameter();
-        return last != null &&
-            (last.requiredKeyword != null || last.name?.lexeme == 'required');
-      }
-
-      var tokenType = entity.type;
-      if (tokenType == TokenType.CLOSE_PAREN) {
-        _addSuggestion(Keyword.DYNAMIC);
-        _addSuggestion(Keyword.VOID);
-        if (!hasCovariant()) {
-          _addSuggestion(Keyword.COVARIANT);
-        }
-      } else if (tokenType == TokenType.CLOSE_CURLY_BRACKET) {
-        _addSuggestion(Keyword.DYNAMIC);
-        _addSuggestion(Keyword.VOID);
-        if (!hasCovariant()) {
-          _addSuggestion(Keyword.COVARIANT);
-          if (request.featureSet.isEnabled(Feature.non_nullable) &&
-              !hasRequired()) {
-            _addSuggestion(Keyword.REQUIRED);
-          }
-        }
-      } else if (tokenType == TokenType.CLOSE_SQUARE_BRACKET) {
-        _addSuggestion(Keyword.DYNAMIC);
-        _addSuggestion(Keyword.VOID);
-        if (!hasCovariant()) {
-          _addSuggestion(Keyword.COVARIANT);
-        }
-      }
-    } else if (entity is FormalParameter) {
-      var beginToken = entity.beginToken;
-      var offset = request.target.offset;
-      if (offset <= beginToken.end) {
-        _addSuggestion(Keyword.COVARIANT);
-        _addSuggestion(Keyword.DYNAMIC);
-        _addSuggestion(Keyword.VOID);
-        if (entity.isNamed &&
-            !entity.isRequired &&
-            request.featureSet.isEnabled(Feature.non_nullable)) {
-          _addSuggestion(Keyword.REQUIRED);
-        }
-      } else if (entity is FunctionTypedFormalParameter) {
-        _addSuggestion(Keyword.COVARIANT);
-        _addSuggestion(Keyword.DYNAMIC);
-        _addSuggestion(Keyword.VOID);
-        if (entity.isNamed &&
-            !entity.isRequired &&
-            request.featureSet.isEnabled(Feature.non_nullable)) {
-          _addSuggestion(Keyword.REQUIRED);
-        }
-      }
-    }
-  }
-
   void _addExpressionKeywords(AstNode node) {
     _addSuggestions([
       Keyword.FALSE,

pkg/analysis_server/lib/src/services/completion/dart/keyword_helper.dart

@@ -375,6 +375,23 @@ class KeywordHelper {
     }
   }
 
+  /// Add the keywords that are appropriate when the selection is at the start
+  /// of a formal parameter in the given [parameterList].
+  void addFormalParameterKeywords(FormalParameterList parameterList) {
+    addKeyword(Keyword.COVARIANT);
+    addKeyword(Keyword.FINAL);
+    if (parameterList.inNamedGroup(offset)) {
+      addKeyword(Keyword.REQUIRED);
+    }
+    var parent = parameterList.parent;
+    if (parent is ConstructorDeclaration) {
+      if (featureSet.isEnabled(Feature.super_parameters)) {
+        addKeyword(Keyword.SUPER);
+      }
+      addKeyword(Keyword.THIS);
+    }
+  }
+
   /// Add the keywords that are appropriate when the selection is before the `{`
   /// or `=>` in a function body. The [body] is used to determine which keywords
   /// are appropriate.
@@ -585,6 +602,19 @@ extension on CollectionElement? {
   }
 }
 
+extension on FormalParameterList {
+  bool inNamedGroup(int offset) {
+    final leftDelimiter = this.leftDelimiter;
+    if (leftDelimiter == null ||
+        leftDelimiter.type != TokenType.OPEN_CURLY_BRACKET) {
+      return false;
+    }
+    var left = leftDelimiter.end;
+    var right = rightDelimiter?.offset ?? rightParenthesis.offset;
+    return left <= offset && offset <= right;
+  }
+}
+
 extension on NodeList<ConstructorInitializer> {
   ConstructorInitializer get lastNonSynthetic {
     final last = this.last;

pkg/analysis_server/test/services/completion/dart/location/named_type_test.dart

@@ -131,6 +131,8 @@ suggestions
     kind: keyword
   dynamic
     kind: keyword
+  final
+    kind: keyword
   super
     kind: keyword
   this
@@ -162,6 +164,8 @@ suggestions
     kind: keyword
   dynamic
     kind: keyword
+  final
+    kind: keyword
   void
     kind: keyword
 ''');

pkg/analysis_server/test/services/completion/dart/location/parameter_list_test.dart

@@ -8,15 +8,33 @@ import '../../../../client/completion_driver_test.dart';
 
 void main() {
   defineReflectiveSuite(() {
-    defineReflectiveTests(ParameterListTest1);
-    defineReflectiveTests(ParameterListTest2);
+    defineReflectiveTests(ParameterListInConstructorTest1);
+    defineReflectiveTests(ParameterListInConstructorTest2);
+    defineReflectiveTests(ParameterListInFunctionTest1);
+    defineReflectiveTests(ParameterListInFunctionTest2);
+    defineReflectiveTests(ParameterListInMethodTest1);
+    defineReflectiveTests(ParameterListInMethodTest2);
   });
 }
 
+@reflectiveTest
+class ParameterListInConstructorTest1 extends AbstractCompletionDriverTest
+    with ParameterListInConstructorTestCases {
+  @override
+  TestingCompletionProtocol get protocol => TestingCompletionProtocol.version1;
+}
+
+@reflectiveTest
+class ParameterListInConstructorTest2 extends AbstractCompletionDriverTest
+    with ParameterListInConstructorTestCases {
+  @override
+  TestingCompletionProtocol get protocol => TestingCompletionProtocol.version2;
+}
+
 mixin ParameterListInConstructorTestCases on AbstractCompletionDriverTest {
   Future<void> test_afterLeftParen_beforeFunctionType() async {
     await computeSuggestions('''
-class A { A(^ Function(){}) {}}
+class A { A(^ Function() f) {}}
 ''');
     assertResponse(r'''
 suggestions
@@ -24,6 +42,8 @@ suggestions
     kind: keyword
   dynamic
     kind: keyword
+  final
+    kind: keyword
   super
     kind: keyword
   this
@@ -43,6 +63,8 @@ suggestions
     kind: keyword
   dynamic
     kind: keyword
+  final
+    kind: keyword
   super
     kind: keyword
   this
@@ -65,6 +87,8 @@ suggestions
     kind: keyword
   dynamic
     kind: keyword
+  final
+    kind: keyword
   this
     kind: keyword
   void
@@ -93,6 +117,8 @@ suggestions
     kind: keyword
   dynamic
     kind: keyword
+  final
+    kind: keyword
   super
     kind: keyword
   this
@@ -104,6 +130,20 @@ suggestions
   }
 }
 
+@reflectiveTest
+class ParameterListInFunctionTest1 extends AbstractCompletionDriverTest
+    with ParameterListInFunctionTestCases {
+  @override
+  TestingCompletionProtocol get protocol => TestingCompletionProtocol.version1;
+}
+
+@reflectiveTest
+class ParameterListInFunctionTest2 extends AbstractCompletionDriverTest
+    with ParameterListInFunctionTestCases {
+  @override
+  TestingCompletionProtocol get protocol => TestingCompletionProtocol.version2;
+}
+
 mixin ParameterListInFunctionTestCases on AbstractCompletionDriverTest {
   Future<void> test_afterLeftParen_beforeFunctionType_partial() async {
     await computeSuggestions('''
@@ -117,12 +157,28 @@ suggestions
     kind: keyword
   dynamic
     kind: keyword
+  final
+    kind: keyword
   void
     kind: keyword
 ''');
   }
 }
 
+@reflectiveTest
+class ParameterListInMethodTest1 extends AbstractCompletionDriverTest
+    with ParameterListInMethodTestCases {
+  @override
+  TestingCompletionProtocol get protocol => TestingCompletionProtocol.version1;
+}
+
+@reflectiveTest
+class ParameterListInMethodTest2 extends AbstractCompletionDriverTest
+    with ParameterListInMethodTestCases {
+  @override
+  TestingCompletionProtocol get protocol => TestingCompletionProtocol.version2;
+}
+
 mixin ParameterListInMethodTestCases on AbstractCompletionDriverTest {
   Future<void> test_afterColon_beforeRightBrace() async {
     await computeSuggestions('''
@@ -234,6 +290,8 @@ suggestions
     kind: keyword
   dynamic
     kind: keyword
+  final
+    kind: keyword
   void
     kind: keyword
 ''');
@@ -249,6 +307,8 @@ suggestions
     kind: keyword
   dynamic
     kind: keyword
+  final
+    kind: keyword
   void
     kind: keyword
 ''');
@@ -273,29 +333,11 @@ suggestions
     kind: keyword
   dynamic
     kind: keyword
+  final
+    kind: keyword
   void
     kind: keyword
 ''');
     }
   }
 }
-
-@reflectiveTest
-class ParameterListTest1 extends AbstractCompletionDriverTest
-    with
-        ParameterListInConstructorTestCases,
-        ParameterListInFunctionTestCases,
-        ParameterListInMethodTestCases {
-  @override
-  TestingCompletionProtocol get protocol => TestingCompletionProtocol.version1;
-}
-
-@reflectiveTest
-class ParameterListTest2 extends AbstractCompletionDriverTest
-    with
-        ParameterListInConstructorTestCases,
-        ParameterListInFunctionTestCases,
-        ParameterListInMethodTestCases {
-  @override
-  TestingCompletionProtocol get protocol => TestingCompletionProtocol.version2;
-}