Merge pull request #747 from dart-lang/lrhn-fix-typedef

Lrhn fix typedef
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9946158..867efed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+# 1.2.1-dev
+
+* Add `--fix-function-typedefs` to convert the old typedef syntax for function
+  types to the new preferred syntax.
+
 # 1.2.0
 
 * Add `--stdin-name` to specify name shown when reading from stdin (#739).
diff --git a/lib/src/source_visitor.dart b/lib/src/source_visitor.dart
index 68cec31..e744dd6 100644
--- a/lib/src/source_visitor.dart
+++ b/lib/src/source_visitor.dart
@@ -112,6 +112,14 @@
   /// How many levels deep inside a constant context the visitor currently is.
   int _constNesting = 0;
 
+  /// Whether we are currently fixing a typedef declaration.
+  ///
+  /// Set to `true` while traversing the parameters of a typedef being converted
+  /// to the new syntax. The new syntax does not allow `int foo()` as a
+  /// parameter declaration, so it needs to be converted to `int Function() foo`
+  /// as part of the fix.
+  bool _insideNewTypedefFix = false;
+
   /// A stack that tracks forcing nested collections to split.
   ///
   /// Each entry corresponds to a collection currently being visited and the
@@ -1028,17 +1036,14 @@
 
   visitFieldFormalParameter(FieldFormalParameter node) {
     visitParameterMetadata(node.metadata, () {
-      builder.startLazyRule(new Rule(Cost.parameterType));
-      builder.nestExpression();
-      modifier(node.covariantKeyword);
+      _beginFormalParameter(node);
       token(node.keyword, after: space);
       visit(node.type, after: split);
       token(node.thisKeyword);
       token(node.period);
       visit(node.identifier);
       visit(node.parameters);
-      builder.unnest();
-      builder.endRule();
+      _endFormalParameter(node);
     });
   }
 
@@ -1305,6 +1310,23 @@
   visitFunctionTypeAlias(FunctionTypeAlias node) {
     visitMetadata(node.metadata);
 
+    if (_formatter.fixes.contains(StyleFix.functionTypedefs)) {
+      _simpleStatement(node, () {
+        // Inlined visitGenericTypeAlias
+        _visitGenericTypeAliasHeader(node.typedefKeyword, node.name,
+            node.typeParameters, null, (node.returnType ?? node.name).offset);
+
+        space();
+
+        // Recursively convert function-arguments to Function syntax.
+        _insideNewTypedefFix = true;
+        _visitGenericFunctionType(
+            node.returnType, null, node.name.offset, null, node.parameters);
+        _insideNewTypedefFix = false;
+      });
+      return;
+    }
+
     _simpleStatement(node, () {
       token(node.typedefKeyword);
       space();
@@ -1317,48 +1339,35 @@
 
   visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) {
     visitParameterMetadata(node.metadata, () {
-      modifier(node.covariantKeyword);
-      visit(node.returnType, after: space);
-
-      // Try to keep the function's parameters with its name.
-      builder.startSpan();
-      visit(node.identifier);
-      _visitParameterSignature(node.typeParameters, node.parameters);
-      builder.endSpan();
+      if (!_insideNewTypedefFix) {
+        modifier(node.covariantKeyword);
+        visit(node.returnType, after: space);
+        // Try to keep the function's parameters with its name.
+        builder.startSpan();
+        visit(node.identifier);
+        _visitParameterSignature(node.typeParameters, node.parameters);
+        builder.endSpan();
+      } else {
+        _beginFormalParameter(node);
+        _visitGenericFunctionType(node.returnType, null, node.identifier.offset,
+            node.typeParameters, node.parameters);
+        split();
+        visit(node.identifier);
+        _endFormalParameter(node);
+      }
     });
   }
 
   visitGenericFunctionType(GenericFunctionType node) {
-    builder.startLazyRule();
-    builder.nestExpression();
-
-    visit(node.returnType, after: split);
-    token(node.functionKeyword);
-
-    builder.unnest();
-    builder.endRule();
-
-    _visitParameterSignature(node.typeParameters, node.parameters);
+    _visitGenericFunctionType(node.returnType, node.functionKeyword, null,
+        node.typeParameters, node.parameters);
   }
 
   visitGenericTypeAlias(GenericTypeAlias node) {
     visitNodes(node.metadata, between: newline, after: newline);
     _simpleStatement(node, () {
-      token(node.typedefKeyword);
-      space();
-
-      // If the typedef's type parameters split, split after the "=" too,
-      // mainly to ensure the function's type parameters and parameters get
-      // end up on successive lines with the same indentation.
-      builder.startRule();
-
-      visit(node.name);
-
-      visit(node.typeParameters);
-      split();
-
-      token(node.equals);
-      builder.endRule();
+      _visitGenericTypeAliasHeader(node.typedefKeyword, node.name,
+          node.typeParameters, node.equals, null);
 
       space();
 
@@ -1830,21 +1839,29 @@
 
   visitSimpleFormalParameter(SimpleFormalParameter node) {
     visitParameterMetadata(node.metadata, () {
-      builder.startLazyRule(new Rule(Cost.parameterType));
-      builder.nestExpression();
-      modifier(node.covariantKeyword);
+      _beginFormalParameter(node);
+
       modifier(node.keyword);
+      var hasType = node.type != null;
+      if (_insideNewTypedefFix && !hasType) {
+        // In function declarations and the old typedef syntax, you can have a
+        // parameter name without a type. In the new syntax, you can have a type
+        // without a name. Add "dynamic" in that case.
 
-      visit(node.type);
+        // Ensure comments on the identifier comes before the inserted type.
+        token(node.identifier.token, before: () {
+          _writeText("dynamic", node.identifier.offset);
+          split();
+        });
+      } else {
+        visit(node.type);
 
-      // In function declarations and the old typedef syntax, you can have a
-      // parameter name without a type. In the new syntax, you can have a type
-      // without a name. Handle both cases.
-      if (node.type != null && node.identifier != null) split();
+        if (hasType && node.identifier != null) split();
 
-      visit(node.identifier);
-      builder.unnest();
-      builder.endRule();
+        visit(node.identifier);
+      }
+
+      _endFormalParameter(node);
     });
   }
 
@@ -2584,6 +2601,74 @@
     }
   }
 
+  /// Begins writing a formal parameter of any kind.
+  void _beginFormalParameter(FormalParameter node) {
+    builder.startLazyRule(new Rule(Cost.parameterType));
+    builder.nestExpression();
+    modifier(node.covariantKeyword);
+  }
+
+  /// Ends writing a formal parameter of any kind.
+  void _endFormalParameter(FormalParameter node) {
+    builder.unnest();
+    builder.endRule();
+  }
+
+  /// Writes a `Function` function type.
+  ///
+  /// Used also by a fix, so there may not be a [functionKeyword].
+  /// In that case [functionKeywordPosition] should be the source position
+  /// used for the inserted "Function" text.
+  void _visitGenericFunctionType(
+      AstNode returnType,
+      Token functionKeyword,
+      int functionKeywordPosition,
+      TypeParameterList typeParameters,
+      FormalParameterList parameters) {
+    builder.startLazyRule();
+    builder.nestExpression();
+
+    visit(returnType, after: split);
+    if (functionKeyword != null) {
+      token(functionKeyword);
+    } else {
+      _writeText("Function", functionKeywordPosition);
+    }
+
+    builder.unnest();
+    builder.endRule();
+    _visitParameterSignature(typeParameters, parameters);
+  }
+
+  /// Writes the header of a new-style typedef.
+  ///
+  /// Also used by a fix so there may not be an [equals] token.
+  /// If [equals] is `null`, then [equalsPosition] must be a
+  /// position to use for the inserted text "=".
+  void _visitGenericTypeAliasHeader(Token typedefKeyword, AstNode name,
+      AstNode typeParameters, Token equals, int equalsPosition) {
+    token(typedefKeyword);
+    space();
+
+    // If the typedef's type parameters split, split after the "=" too,
+    // mainly to ensure the function's type parameters and parameters get
+    // end up on successive lines with the same indentation.
+    builder.startRule();
+
+    visit(name);
+
+    visit(typeParameters);
+    split();
+
+    if (equals != null) {
+      token(equals);
+    } else {
+      _writeText("=", equalsPosition);
+    }
+
+    builder.endRule();
+  }
+
   /// Gets the cost to split at an assignment (or `:` in the case of a named
   /// default value) with the given [rightHandSide].
   ///
diff --git a/lib/src/style_fix.dart b/lib/src/style_fix.dart
index 92f832e..f15d7f6 100644
--- a/lib/src/style_fix.dart
+++ b/lib/src/style_fix.dart
@@ -1,10 +1,17 @@
 // Copyright (c) 2018, 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.
 
 /// Enum-like class for the different syntactic fixes that can be applied while
 /// formatting.
 class StyleFix {
+  static const docComments = const StyleFix._(
+      "doc-comments", 'Use triple slash for documentation comments.');
+
+  static const functionTypedefs = const StyleFix._(
+      "function-typedefs", 'Use new syntax for function type typedefs.');
+
   static const namedDefaultSeparator = const StyleFix._(
       "named-default-separator",
       'Use "=" as the separator before named parameter default values.');
@@ -15,14 +22,12 @@
   static const optionalNew =
       const StyleFix._("optional-new", 'Remove "new" keyword.');
 
-  static const docComments = const StyleFix._(
-      "doc-comments", 'Use triple slash for documentation comments.');
-
   static const all = const [
+    docComments,
+    functionTypedefs,
     namedDefaultSeparator,
     optionalConst,
-    optionalNew,
-    docComments,
+    optionalNew
   ];
 
   final String name;
diff --git a/pubspec.yaml b/pubspec.yaml
index 622cf1e..3785aea 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,6 +1,6 @@
 name: dart_style
 # Note: See tool/grind.dart for how to bump the version.
-version: 1.2.0
+version: 1.2.1-dev
 author: Dart Team <misc@dartlang.org>
 description: Opinionated, automatic Dart source code formatter.
 homepage: https://github.com/dart-lang/dart_style
diff --git a/test/fix_test.dart b/test/fix_test.dart
index e712dd7..0cf3964 100644
--- a/test/fix_test.dart
+++ b/test/fix_test.dart
@@ -15,6 +15,7 @@
   testFile(
       "fixes/named_default_separator.unit", [StyleFix.namedDefaultSeparator]);
   testFile("fixes/doc_comments.stmt", [StyleFix.docComments]);
+  testFile("fixes/function_typedefs.unit", [StyleFix.functionTypedefs]);
   testFile("fixes/optional_const.unit", [StyleFix.optionalConst]);
   testFile("fixes/optional_new.stmt", [StyleFix.optionalNew]);
 }
diff --git a/test/fixes/function_typedefs.unit b/test/fixes/function_typedefs.unit
new file mode 100644
index 0000000..ded56be
--- /dev/null
+++ b/test/fixes/function_typedefs.unit
@@ -0,0 +1,251 @@
+40 columns                              |
+>>>
+typedef void A();
+<<<
+typedef A = void Function();
+>>>
+typedef void A(String name);
+<<<
+typedef A = void Function(String name);
+>>>
+typedef bool A(int count);
+<<<
+typedef A = bool Function(int count);
+>>>
+typedef void A<T>();
+<<<
+typedef A<T> = void Function();
+>>>
+typedef void A<T>(T x);
+<<<
+typedef A<T> = void Function(T x);
+>>>
+typedef void A<T>(x);
+<<<
+typedef A<T> = void Function(dynamic x);
+>>>
+typedef V A<K, V>();
+<<<
+typedef A<K, V> = V Function();
+>>>
+typedef V A<K, V>(K key);
+<<<
+typedef A<K, V> = V Function(K key);
+>>>
+typedef K A<K, V>(V value);
+<<<
+typedef A<K, V> = K Function(V value);
+>>>
+typedef F<T> = void Function(T);
+<<<
+typedef F<T> = void Function(T);
+>>>
+@meta typedef void X(y);
+<<<
+@meta
+typedef X = void Function(dynamic y);
+>>> split type parameters
+typedef G = T Function<TypeOne, TypeTwo, TypeThree>();
+<<<
+typedef G = T Function<TypeOne, TypeTwo,
+    TypeThree>();
+>>> split all type parameters
+typedef G = T Function<TypeOne, TypeTwo, TypeThree, TypeFour, TypeFive, TypeSix>();
+<<<
+typedef G = T Function<
+    TypeOne,
+    TypeTwo,
+    TypeThree,
+    TypeFour,
+    TypeFive,
+    TypeSix>();
+>>> split type and value parameters
+typedef G = T Function<TypeOne, TypeTwo, TypeThree>(TypeOne one, TypeTwo two, TypeThree three);
+<<<
+typedef G = T Function<TypeOne, TypeTwo,
+        TypeThree>(TypeOne one,
+    TypeTwo two, TypeThree three);
+>>> generic typedef parameters on one line
+typedef Foo<T, S> = Function();
+<<<
+typedef Foo<T, S> = Function();
+>>> generic typedef parameters that split
+typedef LongfunctionType<First, Second, Third, Fourth, Fifth, Sixth> = Function(First first, Second second, Third third, Fourth fourth);
+<<<
+typedef LongfunctionType<First, Second,
+        Third, Fourth, Fifth, Sixth>
+    = Function(
+        First first,
+        Second second,
+        Third third,
+        Fourth fourth);
+>>> both type parameter lists split
+typedef LongfunctionType<First, Second, Third, Fourth, Fifth, Sixth> = Function<Seventh>(First first, Second second, Third third, Fourth fourth);
+<<<
+typedef LongfunctionType<First, Second,
+        Third, Fourth, Fifth, Sixth>
+    = Function<Seventh>(
+        First first,
+        Second second,
+        Third third,
+        Fourth fourth);
+>>> all three parameter lists split
+typedef LongfunctionType<First, Second, Third, Fourth, Fifth, Sixth> = Function<Seventh, Eighth, Ninth, Tenth, Eleventh, Twelfth, Thirteenth>(First first, Second second, Third third, Fourth fourth);
+<<<
+typedef LongfunctionType<First, Second,
+        Third, Fourth, Fifth, Sixth>
+    = Function<
+            Seventh,
+            Eighth,
+            Ninth,
+            Tenth,
+            Eleventh,
+            Twelfth,
+            Thirteenth>(
+        First first,
+        Second second,
+        Third third,
+        Fourth fourth);
+>>> old generic typedef syntax
+typedef Foo  <  T  ,S  >(T t,S s);
+<<<
+typedef Foo<T, S> = Function(T t, S s);
+>>> non-generic in typedef
+typedef   SomeFunc=ReturnType  Function(int param,   double other);
+<<<
+typedef SomeFunc = ReturnType Function(
+    int param, double other);
+>>> generic in typedef
+typedef Generic = T Function<T>(T param, double other);
+<<<
+typedef Generic = T Function<T>(
+    T param, double other);
+>>> no return type
+typedef SomeFunc = Function();
+<<<
+typedef SomeFunc = Function();
+>>> nested
+typedef SomeFunc = Function(int first, Function(int first, bool second, String third) second, String third);
+<<<
+typedef SomeFunc = Function(
+    int first,
+    Function(int first, bool second,
+            String third)
+        second,
+    String third);
+>>> without param names
+typedef F = Function(int, bool, String);
+<<<
+typedef F = Function(int, bool, String);
+>>> generic
+typedef    Foo < A ,B>=Function ( A a,   B b );
+<<<
+typedef Foo<A, B> = Function(A a, B b);
+>>> generic function
+typedef    Foo  =Function  < A ,B  >   ( A a,B b );
+<<<
+typedef Foo = Function<A, B>(A a, B b);
+>>> simple typedef
+typedef int Foo(int x);
+<<<
+typedef Foo = int Function(int x);
+>>> generic typedef
+typedef S Foo<S extends num>(S x);
+<<<
+typedef Foo<S extends num> = S Function(
+    S x);
+>>> named argument
+typedef int Foo({x});
+<<<
+typedef Foo = int Function({dynamic x});
+>>> optional positional argument
+typedef int Foo([x]);
+<<<
+typedef Foo = int Function([dynamic x]);
+>>> metadata and keywords
+typedef int Foo(@meta v1, final v2, var v3, /*c*/ v4);
+<<<
+typedef Foo = int Function(
+    @meta dynamic v1,
+    final dynamic v2,
+    var dynamic v3,
+    /*c*/ dynamic v4);
+>>> metadata and keywords optional positional
+typedef int Foo([@meta v1, final v2, var v3, /*c*/ v4]);
+<<<
+typedef Foo = int Function(
+    [@meta dynamic v1,
+    final dynamic v2,
+    var dynamic v3,
+    /*c*/ dynamic v4]);
+>>> metadata and keywords named
+typedef int Foo({@meta v1, final v2, var v3, /*c*/ v4});
+<<<
+typedef Foo = int Function(
+    {@meta dynamic v1,
+    final dynamic v2,
+    var dynamic v3,
+    /*c*/ dynamic v4});
+>>> function argument
+typedef int Foo(int callback(int x));
+<<<
+typedef Foo = int Function(
+    int Function(int x) callback);
+>>> nested Function type
+typedef int Foo(int cb(int Function(int) cb2));
+<<<
+typedef Foo = int Function(
+    int Function(int Function(int) cb2)
+        cb);
+>>> nested generic function
+typedef T Foo<T>(T cb<S>(T cb2(S x)));
+<<<
+typedef Foo<T> = T Function(
+    T Function<S>(T Function(S x) cb2)
+        cb);
+>>> new-style function type unchanged
+typedef int Foo(int Function(int) cb);
+<<<
+typedef Foo = int Function(
+    int Function(int) cb);
+>>> don't change correct typedef
+typedef Foo = int Function(int);
+<<<
+typedef Foo = int Function(int);
+>>> simple typedef, no types
+typedef Foo(x);
+<<<
+typedef Foo = Function(dynamic x);
+>>> nested function types, no types
+typedef Foo(foo(x));
+<<<
+typedef Foo = Function(
+    Function(dynamic x) foo);
+>>> block comments in typedef
+typedef/*0*/void/*1*/Foo/*2*/<T>/*3*/(/*4*/T/*5*/foo(/*6*/x));
+<<<
+typedef /*1*/ Foo /*2*/ <T>
+    = /*0*/ void Function /*3*/ (
+        /*4*/ T Function(
+            /*6*/ dynamic x) /*5*/ foo);
+>>> eol comments in typedef
+typedef//0
+void//1
+Foo//2
+<T>//3
+(//4
+T//5
+foo(//6
+x));
+<<<
+typedef //1
+    Foo //2
+    <T>
+    = //0
+    void Function //3
+    (
+        //4
+        T Function(
+                //6
+                dynamic x) //5
+            foo);