Convert the way function types are displayed to better match the syntax of Dart

Change-Id: Id5068bd52b3a35c70cb0d2ccc735469e2f8a0d3b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/103701
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: bwilkerson

pkg/analysis_server/test/analysis/get_hover_test.dart

@@ -543,7 +543,7 @@ main(A a) {
     expect(hover.elementKind, 'method');
     expect(hover.isDeprecated, isFalse);
     // types
-    expect(hover.staticType, '(int, String) → List<String>');
+    expect(hover.staticType, 'List<String> Function(int, String)');
     expect(hover.propagatedType, isNull);
     // no parameter
     expect(hover.parameter, isNull);
@@ -594,7 +594,7 @@ f(Stream<int> s) {
     expect(hover.isDeprecated, isFalse);
     // types
     expect(hover.staticType,
-        '(StreamTransformer<int, dynamic>) → Stream<dynamic>');
+        'Stream<dynamic> Function(StreamTransformer<int, dynamic>)');
     expect(hover.propagatedType, isNull);
     // no parameter
     expect(hover.parameter, isNull);

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

@@ -582,24 +582,28 @@ foo({String children}) {}
     //
     addTestSource('main() { int.parse("16", ^);}');
     await computeSuggestions();
-    assertSuggestArgumentsAndTypes(
-        namedArgumentsWithTypes: {'radix': 'int', 'onError': '(String) → int'});
+    assertSuggestArgumentsAndTypes(namedArgumentsWithTypes: {
+      'radix': 'int',
+      'onError': 'int Function(String)'
+    });
   }
 
   test_ArgumentList_imported_function_named_param1() async {
     //
     addTestSource('main() { int.parse("16", r^);}');
     await computeSuggestions();
-    assertSuggestArgumentsAndTypes(
-        namedArgumentsWithTypes: {'radix': 'int', 'onError': '(String) → int'});
+    assertSuggestArgumentsAndTypes(namedArgumentsWithTypes: {
+      'radix': 'int',
+      'onError': 'int Function(String)'
+    });
   }
 
   test_ArgumentList_imported_function_named_param2() async {
     //
     addTestSource('main() { int.parse("16", radix: 7, ^);}');
     await computeSuggestions();
     assertSuggestArgumentsAndTypes(
-        namedArgumentsWithTypes: {'onError': '(String) → int'});
+        namedArgumentsWithTypes: {'onError': 'int Function(String)'});
   }
 
   test_ArgumentList_imported_function_named_param2a() async {
@@ -613,9 +617,10 @@ foo({String children}) {}
     //
     addTestSource('main() { int.parse("16", r^: 16);}');
     await computeSuggestions();
-    assertSuggestArgumentsAndTypes(
-        namedArgumentsWithTypes: {'radix': 'int', 'onError': '(String) → int'},
-        includeColon: false);
+    assertSuggestArgumentsAndTypes(namedArgumentsWithTypes: {
+      'radix': 'int',
+      'onError': 'int Function(String)'
+    }, includeColon: false);
   }
 
   test_ArgumentList_imported_function_named_param_label2() async {
@@ -629,8 +634,10 @@ foo({String children}) {}
     //
     addTestSource('main() { int.parse("16", ^: 16);}');
     await computeSuggestions();
-    assertSuggestArgumentsAndTypes(
-        namedArgumentsWithTypes: {'radix': 'int', 'onError': '(String) → int'});
+    assertSuggestArgumentsAndTypes(namedArgumentsWithTypes: {
+      'radix': 'int',
+      'onError': 'int Function(String)'
+    });
   }
 
   test_ArgumentList_local_constructor_named_fieldFormal_documentation() async {
@@ -886,8 +893,10 @@ main() { new A(^);}''');
 f(v,{int radix, int onError(String s)}){}
 main() { f("16", ^);}''');
     await computeSuggestions();
-    assertSuggestArgumentsAndTypes(
-        namedArgumentsWithTypes: {'radix': 'int', 'onError': '(String) → int'});
+    assertSuggestArgumentsAndTypes(namedArgumentsWithTypes: {
+      'radix': 'int',
+      'onError': 'int Function(String)'
+    });
   }
 
   test_ArgumentList_local_function_named_param1() async {
@@ -896,8 +905,10 @@ main() { f("16", ^);}''');
 f(v,{int radix, int onError(String s)}){}
 main() { f("16", r^);}''');
     await computeSuggestions();
-    assertSuggestArgumentsAndTypes(
-        namedArgumentsWithTypes: {'radix': 'int', 'onError': '(String) → int'});
+    assertSuggestArgumentsAndTypes(namedArgumentsWithTypes: {
+      'radix': 'int',
+      'onError': 'int Function(String)'
+    });
   }
 
   test_ArgumentList_local_function_named_param2() async {
@@ -907,7 +918,7 @@ f(v,{int radix, int onError(String s)}){}
 main() { f("16", radix: 7, ^);}''');
     await computeSuggestions();
     assertSuggestArgumentsAndTypes(
-        namedArgumentsWithTypes: {'onError': '(String) → int'});
+        namedArgumentsWithTypes: {'onError': 'int Function(String)'});
   }
 
   test_ArgumentList_local_function_named_param2a() async {

pkg/analyzer/CHANGELOG.md

@@ -7,6 +7,12 @@
 * Deprecated the `abstract` setter in `ClassElementImpl`, `EnumElementImpl`,
   `MethodElementImpl`, and `PropertyAccessorElementImpl`.  `isAbstract` should
   be used instead.
+* Changed the way function types are displayed from e.g. `(int) -> void` to
+  `void Function(int)`. This is more consistent with the syntax of Dart, and it
+  will avoid ambiguities when nullability is added to the type system. This
+  impacts to value returned by both `FunctionType.displayName` and
+  `FunctionType.toString`. Client code might be broken if it depends on the
+  content of the returned value.
 
 ## 0.36.3
 * Deprecated `AstFactory.compilationUnit`.  In a future analyzer release, this

pkg/analyzer/lib/dart/element/type.dart

@@ -29,6 +29,9 @@ import 'package:analyzer/src/generated/type_system.dart' show TypeSystem;
 abstract class DartType {
   /// Return the name of this type as it should appear when presented to users
   /// in contexts such as error messages.
+  ///
+  /// Clients should not depend on the content of the returned value as it will
+  /// be changed if doing so would improve the UX.
   String get displayName;
 
   /// Return the element representing the declaration of this type, or `null` if

pkg/analyzer/lib/src/dart/element/type.dart

@@ -275,6 +275,12 @@ class CircularFunctionTypeImpl extends DynamicTypeImpl
   @override
   TypeImpl withNullability(NullabilitySuffix nullabilitySuffix) => this;
 
+  @override
+  void _appendToWithTypeParameters(StringBuffer buffer,
+      Set<TypeImpl> visitedTypes, bool withNullability, String typeParameters) {
+    throw StateError('We should never get here.');
+  }
+
   @override
   void _forEachParameterType(
       ParameterKind kind, callback(String name, DartType type)) {
@@ -714,11 +720,11 @@ abstract class FunctionTypeImpl extends TypeImpl implements FunctionType {
   @override
   void appendTo(StringBuffer buffer, Set<TypeImpl> visitedTypes,
       {bool withNullability = false}) {
-    // TODO(paulberry): update to use the new "Function" syntax to avoid
-    // ambiguity with NNBD, and eliminate code duplication with
+    // TODO(paulberry): eliminate code duplication with
     // _ElementWriter.writeType.  See issue #35818.
     if (visitedTypes.add(this)) {
       if (typeFormals.isNotEmpty) {
+        StringBuffer typeParametersBuffer = StringBuffer();
         // To print a type with type variables, first make sure we have unique
         // variable names to print.
         Set<TypeParameterType> freeVariables = new HashSet<TypeParameterType>();
@@ -733,10 +739,10 @@ abstract class FunctionTypeImpl extends TypeImpl implements FunctionType {
 
         List<DartType> instantiateTypeArgs = <DartType>[];
         List<DartType> variables = <DartType>[];
-        buffer.write("<");
+        typeParametersBuffer.write('<');
         for (TypeParameterElement e in typeFormals) {
           if (e != typeFormals[0]) {
-            buffer.write(",");
+            typeParametersBuffer.write(',');
           }
           String name = e.name;
           int counter = 0;
@@ -751,89 +757,27 @@ abstract class FunctionTypeImpl extends TypeImpl implements FunctionType {
           }
           TypeParameterTypeImpl t =
               new TypeParameterTypeImpl(new TypeParameterElementImpl(name, -1));
-          t.appendTo(buffer, visitedTypes, withNullability: withNullability);
+          t.appendTo(typeParametersBuffer, visitedTypes,
+              withNullability: withNullability);
           instantiateTypeArgs.add(t);
           variables.add(e.type);
           if (e.bound != null) {
-            buffer.write(" extends ");
+            typeParametersBuffer.write(' extends ');
             TypeImpl renamed =
                 e.bound.substitute2(instantiateTypeArgs, variables);
-            renamed.appendTo(buffer, visitedTypes);
+            renamed.appendTo(typeParametersBuffer, visitedTypes);
           }
         }
-        buffer.write(">");
-
-        // Instantiate it and print the resulting type. After instantiation, it
-        // will no longer have typeFormals, so we will continue below.
-        this
-            .instantiate(instantiateTypeArgs)
-            .appendTo(buffer, visitedTypes, withNullability: withNullability);
-        return;
-      }
-
-      List<DartType> normalParameterTypes = this.normalParameterTypes;
-      List<DartType> optionalParameterTypes = this.optionalParameterTypes;
-      Map<String, DartType> namedParameterTypes = this.namedParameterTypes;
-      DartType returnType = this.returnType;
-
-      bool needsComma = false;
-      void writeSeparator() {
-        if (needsComma) {
-          buffer.write(", ");
-        } else {
-          needsComma = true;
-        }
-      }
-
-      void startOptionalParameters() {
-        if (needsComma) {
-          buffer.write(", ");
-          needsComma = false;
-        }
-      }
-
-      buffer.write("(");
-      if (normalParameterTypes.isNotEmpty) {
-        for (DartType type in normalParameterTypes) {
-          writeSeparator();
-          (type as TypeImpl)
-              .appendTo(buffer, visitedTypes, withNullability: withNullability);
-        }
-      }
-      if (optionalParameterTypes.isNotEmpty) {
-        startOptionalParameters();
-        buffer.write("[");
-        for (DartType type in optionalParameterTypes) {
-          writeSeparator();
-          (type as TypeImpl)
-              .appendTo(buffer, visitedTypes, withNullability: withNullability);
-        }
-        buffer.write("]");
-        needsComma = true;
-      }
-      if (namedParameterTypes.isNotEmpty) {
-        startOptionalParameters();
-        buffer.write("{");
-        namedParameterTypes.forEach((String name, DartType type) {
-          writeSeparator();
-          buffer.write(name);
-          buffer.write(": ");
-          (type as TypeImpl)
-              .appendTo(buffer, visitedTypes, withNullability: withNullability);
-        });
-        buffer.write("}");
-        needsComma = true;
-      }
-      buffer.write(")");
-      buffer.write(ElementImpl.RIGHT_ARROW);
-      if (returnType == null) {
-        buffer.write("null");
+        typeParametersBuffer.write('>');
+
+        // Instantiate it and print the resulting type.
+        this.instantiate(instantiateTypeArgs)._appendToWithTypeParameters(
+            buffer,
+            visitedTypes,
+            withNullability,
+            typeParametersBuffer.toString());
       } else {
-        (returnType as TypeImpl)
-            .appendTo(buffer, visitedTypes, withNullability: withNullability);
-      }
-      if (withNullability) {
-        _appendNullability(buffer);
+        _appendToWithTypeParameters(buffer, visitedTypes, withNullability, '');
       }
       visitedTypes.remove(this);
     } else {
@@ -936,6 +880,76 @@ abstract class FunctionTypeImpl extends TypeImpl implements FunctionType {
   FunctionTypeImpl substitute3(List<DartType> argumentTypes) =>
       substitute2(argumentTypes, typeArguments);
 
+  void _appendToWithTypeParameters(StringBuffer buffer,
+      Set<TypeImpl> visitedTypes, bool withNullability, String typeParameters) {
+    List<DartType> normalParameterTypes = this.normalParameterTypes;
+    List<DartType> optionalParameterTypes = this.optionalParameterTypes;
+    Map<String, DartType> namedParameterTypes = this.namedParameterTypes;
+    DartType returnType = this.returnType;
+
+    if (returnType == null) {
+      buffer.write('null');
+    } else {
+      (returnType as TypeImpl)
+          .appendTo(buffer, visitedTypes, withNullability: withNullability);
+    }
+    buffer.write(' Function');
+    buffer.write(typeParameters);
+    bool needsComma = false;
+
+    void writeSeparator() {
+      if (needsComma) {
+        buffer.write(', ');
+      } else {
+        needsComma = true;
+      }
+    }
+
+    void startOptionalParameters() {
+      if (needsComma) {
+        buffer.write(', ');
+        needsComma = false;
+      }
+    }
+
+    buffer.write('(');
+    if (normalParameterTypes.isNotEmpty) {
+      for (DartType type in normalParameterTypes) {
+        writeSeparator();
+        (type as TypeImpl)
+            .appendTo(buffer, visitedTypes, withNullability: withNullability);
+      }
+    }
+    if (optionalParameterTypes.isNotEmpty) {
+      startOptionalParameters();
+      buffer.write('[');
+      for (DartType type in optionalParameterTypes) {
+        writeSeparator();
+        (type as TypeImpl)
+            .appendTo(buffer, visitedTypes, withNullability: withNullability);
+      }
+      buffer.write(']');
+      needsComma = true;
+    }
+    if (namedParameterTypes.isNotEmpty) {
+      startOptionalParameters();
+      buffer.write('{');
+      namedParameterTypes.forEach((String name, DartType type) {
+        writeSeparator();
+        buffer.write(name);
+        buffer.write(': ');
+        (type as TypeImpl)
+            .appendTo(buffer, visitedTypes, withNullability: withNullability);
+      });
+      buffer.write('}');
+      needsComma = true;
+    }
+    buffer.write(')');
+    if (withNullability) {
+      _appendNullability(buffer);
+    }
+  }
+
   /**
    * Invokes [callback] for each parameter of [kind] with the parameter's [name]
    * and type after any type parameters have been applied.

pkg/analyzer/test/generated/simple_resolver_test.dart

@@ -993,7 +993,7 @@ main() {
     await resolveTestFile();
 
     var node = findNode.simple('myVar(42)');
-    assertType(node, '(int) → String');
+    assertType(node, 'String Function(int)');
   }
 
   test_metadata_class() async {