Completion. Issue 53016. Suggestion CompletionSuggestionKind.IDENTIFIER when factory redirect.

Replaces NamedConstructorContributor with new style implementation.

Bug: https://github.com/dart-lang/sdk/issues/53016
Change-Id: I254c139a0c08eae8a256e9589b09481997f40e75
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356141
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

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

@@ -67,11 +67,16 @@ final class ConstructorSuggestion extends ImportableSuggestion {
   /// site. That is, whether we are completing after a period.
   final bool hasClassName;
 
+  /// Whether a tear-off should be suggested, not an invocation.
+  final bool isTearOff;
+
   /// Initialize a newly created candidate suggestion to suggest the [element].
-  ConstructorSuggestion(
-      {required super.importData,
-      required this.element,
-      required this.hasClassName});
+  ConstructorSuggestion({
+    required super.importData,
+    required this.element,
+    required this.hasClassName,
+    required this.isTearOff,
+  });
 
   @override
   String get completion => '$completionPrefix${element.displayName}';
@@ -593,8 +598,15 @@ extension SuggestionBuilderExtension on SuggestionBuilder {
             includeTrailingComma: suggestion.includeTrailingComma);
       case ConstructorSuggestion():
         libraryUriStr = suggestion.libraryUriStr;
-        suggestConstructor(suggestion.element,
-            hasClassName: suggestion.hasClassName, prefix: suggestion.prefix);
+        suggestConstructor(
+          suggestion.element,
+          hasClassName: suggestion.hasClassName,
+          kind: suggestion.isTearOff
+              ? CompletionSuggestionKind.IDENTIFIER
+              : CompletionSuggestionKind.INVOCATION,
+          tearOff: suggestion.isTearOff,
+          prefix: suggestion.prefix,
+        );
         libraryUriStr = null;
       case EnumSuggestion():
         libraryUriStr = suggestion.libraryUriStr;

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

@@ -13,7 +13,6 @@ import 'package:analysis_server/src/services/completion/dart/feature_computer.da
 import 'package:analysis_server/src/services/completion/dart/in_scope_completion_pass.dart';
 import 'package:analysis_server/src/services/completion/dart/library_member_contributor.dart';
 import 'package:analysis_server/src/services/completion/dart/library_prefix_contributor.dart';
-import 'package:analysis_server/src/services/completion/dart/named_constructor_contributor.dart';
 import 'package:analysis_server/src/services/completion/dart/not_imported_contributor.dart';
 import 'package:analysis_server/src/services/completion/dart/record_literal_contributor.dart';
 import 'package:analysis_server/src/services/completion/dart/suggestion_builder.dart';
@@ -129,7 +128,6 @@ class DartCompletionManager {
       ExtensionMemberContributor(request, builder),
       LibraryMemberContributor(request, builder),
       LibraryPrefixContributor(request, builder),
-      NamedConstructorContributor(request, builder),
       RecordLiteralContributor(request, builder),
       if (enableUriContributor) UriContributor(request, builder),
       VariableNameContributor(request, builder),

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

@@ -119,6 +119,16 @@ class DeclarationHelper {
     }
   }
 
+  /// Add suggestions for all constructors of [element].
+  void addConstructorNamesForElement({
+    required InterfaceElement element,
+  }) {
+    var constructors = element.augmented?.constructors ?? element.constructors;
+    for (var constructor in constructors) {
+      _suggestConstructor(constructor, hasClassName: true, importData: null);
+    }
+  }
+
   /// Add suggestions for all of the named constructors in the [type]. If
   /// [exclude] is not `null` it is the name of a constructor that should be
   /// omitted from the list, typically because suggesting it would result in an
@@ -974,8 +984,7 @@ class DeclarationHelper {
       var allowNonFactory =
           containingElement is ClassElement && !containingElement.isAbstract;
       for (var constructor in constructors) {
-        if (constructor.name.isNotEmpty &&
-            constructor.isVisibleIn(request.libraryElement) &&
+        if (constructor.isVisibleIn(request.libraryElement) &&
             (allowNonFactory || constructor.isFactory)) {
           _suggestConstructor(constructor,
               hasClassName: true, importData: null);
@@ -1072,8 +1081,17 @@ class DeclarationHelper {
     if (mustBeAssignable || (mustBeConstant && !element.isConst)) {
       return;
     }
+
+    if (!element.isVisibleIn(request.libraryElement)) {
+      return;
+    }
+
     var suggestion = ConstructorSuggestion(
-        importData: importData, element: element, hasClassName: hasClassName);
+      importData: importData,
+      element: element,
+      hasClassName: hasClassName,
+      isTearOff: preferNonInvocation,
+    );
     collector.addSuggestion(suggestion);
   }
 

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

@@ -11,6 +11,7 @@ import 'package:analysis_server/src/services/completion/dart/override_helper.dar
 import 'package:analysis_server/src/services/completion/dart/suggestion_collector.dart';
 import 'package:analysis_server/src/services/completion/dart/visibility_tracker.dart';
 import 'package:analysis_server/src/utilities/extensions/ast.dart';
+import 'package:analysis_server/src/utilities/extensions/completion_request.dart';
 import 'package:analysis_server/src/utilities/flutter.dart';
 import 'package:analyzer/dart/analysis/features.dart';
 import 'package:analyzer/dart/ast/ast.dart';
@@ -615,6 +616,40 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
     }
   }
 
+  @override
+  void visitConstructorName(ConstructorName node) {
+    if (node.parent is ConstructorReference) {
+      var element = node.type.element;
+      if (element is InterfaceElement) {
+        declarationHelper(
+          preferNonInvocation: true,
+        ).addConstructorNamesForElement(element: element);
+      }
+    } else {
+      var type = node.type.type;
+      if (type is InterfaceType) {
+        // Suggest factory redirects.
+        if (node.parent case ConstructorDeclaration factoryConstructor) {
+          if (factoryConstructor.factoryKeyword != null &&
+              factoryConstructor.redirectedConstructor == node) {
+            declarationHelper(
+              mustBeConstant: factoryConstructor.constKeyword != null,
+              preferNonInvocation: true,
+            ).addConstructorNamesForType(
+              type: type,
+              exclude: factoryConstructor.name?.lexeme,
+            );
+            return;
+          }
+        }
+        // Suggest invocations.
+        declarationHelper().addConstructorNamesForType(type: type);
+      }
+    }
+
+    super.visitConstructorName(node);
+  }
+
   @override
   void visitConstructorReference(ConstructorReference node) {
     _forExpression(node);
@@ -1679,8 +1714,11 @@ class InScopeCompletionPass extends SimpleAstVisitor<void> {
           var parent = node.parent;
           var mustBeAssignable =
               parent is AssignmentExpression && node == parent.leftHandSide;
-          declarationHelper(mustBeAssignable: mustBeAssignable)
-              .addStaticMembersOfElement(element);
+          declarationHelper(
+            mustBeAssignable: mustBeAssignable,
+            preferNonInvocation: element is InterfaceElement &&
+                state.request.shouldSuggestTearOff(element),
+          ).addStaticMembersOfElement(element);
         }
       }
     }

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

@@ -1,68 +0,0 @@
-// Copyright (c) 2015, 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:analysis_server/src/protocol_server.dart' as protocol;
-import 'package:analysis_server/src/provisional/completion/dart/completion_dart.dart';
-import 'package:analysis_server/src/utilities/extensions/completion_request.dart';
-import 'package:analyzer/dart/ast/ast.dart';
-import 'package:analyzer/dart/element/element.dart';
-import 'package:analyzer/dart/element/type.dart';
-import 'package:analyzer/src/util/performance/operation_performance.dart';
-
-/// A contributor that produces suggestions based on the named constructors
-/// defined on a given class. More concretely, this class produces suggestions
-/// for expressions of the form `C.^` or `C<E>.^`, where `C` is the name of a
-/// class.
-class NamedConstructorContributor extends DartCompletionContributor {
-  NamedConstructorContributor(super.request, super.builder);
-
-  @override
-  Future<void> computeSuggestions({
-    required OperationPerformanceImpl performance,
-  }) async {
-    var node = request.target.containingNode;
-    if (node is ConstructorName) {
-      if (node.parent is ConstructorReference) {
-        var element = node.type.element;
-        if (element is InterfaceElement) {
-          _buildSuggestions(element);
-        }
-      } else {
-        var type = node.type.type;
-        if (type is InterfaceType) {
-          var element = type.element;
-          _buildSuggestions(element);
-        }
-      }
-    } else if (node is PrefixedIdentifier) {
-      var element = node.prefix.staticElement;
-      if (element is ClassElement) {
-        _buildSuggestions(element);
-      }
-    }
-  }
-
-  void _buildSuggestions(InterfaceElement element) {
-    if (element is! ClassElement) {
-      return;
-    }
-
-    var tearOff = request.shouldSuggestTearOff(element);
-    var isLocalClassDecl = element.library == request.libraryElement;
-    for (var constructor in element.constructors) {
-      if (isLocalClassDecl || !constructor.isPrivate) {
-        if (!element.isAbstract || constructor.isFactory) {
-          builder.suggestConstructor(
-            constructor,
-            hasClassName: true,
-            kind: tearOff
-                ? protocol.CompletionSuggestionKind.IDENTIFIER
-                : protocol.CompletionSuggestionKind.INVOCATION,
-            tearOff: tearOff,
-          );
-        }
-      }
-    }
-  }
-}

pkg/analysis_server/test/services/completion/dart/completion_printer.dart

@@ -89,7 +89,7 @@ class CompletionResponsePrinter {
       } else if (elementKind == ElementKind.CLASS) {
         return 'class';
       } else if (elementKind == ElementKind.CONSTRUCTOR) {
-        return 'constructorInvocation';
+        return 'constructor';
       } else if (elementKind == ElementKind.ENUM) {
         return 'enum';
       } else if (elementKind == ElementKind.ENUM_CONSTANT) {

pkg/analysis_server/test/services/completion/dart/completion_test.dart

@@ -8062,7 +8062,7 @@ suggestions
   int
     kind: class
   int.fromEnvironment
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -8128,7 +8128,7 @@ suggestions
   int
     kind: class
   int.fromEnvironment
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -8192,7 +8192,7 @@ suggestions
   int
     kind: class
   int.fromEnvironment
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -8261,7 +8261,7 @@ suggestions
   int
     kind: class
   int.fromEnvironment
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -8330,7 +8330,7 @@ suggestions
   int
     kind: class
   int.fromEnvironment
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -8408,15 +8408,15 @@ suggestions
   String
     kind: class
   String.fromCharCode
-    kind: constructorInvocation
+    kind: constructor
   String.fromCharCodes
-    kind: constructorInvocation
+    kind: constructor
   String.fromEnvironment
-    kind: constructorInvocation
+    kind: constructor
   int
     kind: class
   int.fromEnvironment
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -8443,15 +8443,15 @@ suggestions
   String
     kind: class
   String.fromCharCode
-    kind: constructorInvocation
+    kind: constructor
   String.fromCharCodes
-    kind: constructorInvocation
+    kind: constructor
   String.fromEnvironment
-    kind: constructorInvocation
+    kind: constructor
   int
     kind: class
   int.fromEnvironment
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -9540,9 +9540,9 @@ void f(p) {
     assertResponse(r'''
 suggestions
   myFuncDouble
-    kind: methodInvocation
+    kind: method
   myFuncInt
-    kind: methodInvocation
+    kind: method
 ''');
   }
 
@@ -9561,7 +9561,7 @@ void f(p) {
     assertResponse(r'''
 suggestions
   myFunc
-    kind: methodInvocation
+    kind: method
 ''');
   }
 
@@ -9579,7 +9579,7 @@ void f(p) {
     assertResponse(r'''
 suggestions
   myFunc
-    kind: methodInvocation
+    kind: method
 ''');
   }
 

pkg/analysis_server/test/services/completion/dart/declaration/constructor_test.dart

@@ -58,9 +58,9 @@ void f() {
     assertResponse(r'''
 suggestions
   b0
-    kind: constructorInvocation
+    kind: constructor
   f0
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -141,9 +141,9 @@ void f() {
     assertResponse(r'''
 suggestions
   named
-    kind: constructorInvocation
+    kind: constructor
   new
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -163,7 +163,7 @@ replacement
   left: 2
 suggestions
   named
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -182,9 +182,9 @@ void f() {
     assertResponse(r'''
 suggestions
   named
-    kind: constructorInvocation
+    kind: constructor
   new
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -205,9 +205,9 @@ void f() {
     assertResponse(r'''
 suggestions
   named
-    kind: constructorInvocation
+    kind: constructor
   new
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -228,7 +228,7 @@ replacement
   left: 2
 suggestions
   named
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -250,9 +250,9 @@ void f() {
     assertResponse(r'''
 suggestions
   named
-    kind: constructorInvocation
+    kind: constructor
   new
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -270,7 +270,7 @@ void f() {
     assertResponse(r'''
 suggestions
   named
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 
@@ -386,7 +386,7 @@ void f() {
     assertResponse(r'''
 suggestions
   named
-    kind: constructorInvocation
+    kind: constructor
 ''');
   }
 

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

@@ -17,6 +17,40 @@ class ConstructorDeclarationTest extends AbstractCompletionDriverTest
     with ConstructorDeclarationTestCases {}
 
 mixin ConstructorDeclarationTestCases on AbstractCompletionDriverTest {
+  Future<void> test_factory_redirectedConstructor_afterName() async {
+    await computeSuggestions('''
+class A {
+  A.n01();
+  const A.n02();
+
+  factory A.n03() = A.^
+}
+''');
+    assertResponse(r'''
+suggestions
+  n01
+    kind: constructor
+  n02
+    kind: constructor
+''');
+  }
+
+  Future<void> test_factory_redirectedConstructor_afterName_const() async {
+    await computeSuggestions('''
+class A {
+  A.n01();
+  const A.n02();
+
+  const factory A.n03() = A.^
+}
+''');
+    assertResponse(r'''
+suggestions
+  n02
+    kind: constructor
+''');
+  }
+
   Future<void> test_initializers_beforeInitializer() async {
     await computeSuggestions('''
 class A {