Add a fix to inline a typedef

I needed to fix AST nodes for parameters to know about the required
token when computing the beginning token and child entities.

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

pkg/analysis_server/lib/src/services/correction/dart/inline_typedef.dart

@@ -0,0 +1,146 @@
+// Copyright (c) 2020, 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:_fe_analyzer_shared/src/scanner/token.dart';
+import 'package:analysis_server/src/services/correction/dart/abstract_producer.dart';
+import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:analyzer_plugin/utilities/range_factory.dart';
+
+class InlineTypedef extends CorrectionProducer {
+  String name;
+
+  @override
+  List<Object> get fixArguments => [name];
+
+  @override
+  FixKind get fixKind => DartFixKind.INLINE_TYPEDEF;
+
+  @override
+  Future<void> compute(DartChangeBuilder builder) async {
+    //
+    // Extract the information needed to build the edit.
+    //
+    TypeAnnotation returnType;
+    TypeParameterList typeParameters;
+    List<FormalParameter> parameters;
+    if (node is FunctionTypeAlias) {
+      var typedef = node as FunctionTypeAlias;
+      returnType = typedef.returnType;
+      name = typedef.name.name;
+      typeParameters = typedef.typeParameters;
+      parameters = typedef.parameters.parameters;
+    } else if (node is GenericTypeAlias) {
+      var typedef = node as GenericTypeAlias;
+      if (typedef.typeParameters != null) {
+        return;
+      }
+      var functionType = typedef.functionType;
+      returnType = functionType.returnType;
+      name = typedef.name.name;
+      typeParameters = functionType.typeParameters;
+      parameters = functionType.parameters.parameters;
+    } else {
+      return;
+    }
+    // TODO(brianwilkerson) Handle parts.
+    var finder = _ReferenceFinder(name);
+    resolvedResult.unit.accept(finder);
+    if (finder.count != 1) {
+      return;
+    }
+    //
+    // Build the edit.
+    //
+    await builder.addFileEdit(file, (DartFileEditBuilder builder) {
+      builder.addDeletion(utils.getLinesRange(range.node(node)));
+      builder.addReplacement(range.node(finder.reference),
+          (DartEditBuilder builder) {
+        if (returnType != null) {
+          builder.write(utils.getNodeText(returnType));
+          builder.write(' ');
+        }
+        builder.write('Function');
+        if (typeParameters != null) {
+          builder.write(utils.getNodeText(typeParameters));
+        }
+        String groupEnd;
+        builder.write('(');
+        for (int i = 0; i < parameters.length; i++) {
+          var parameter = parameters[i];
+          if (i > 0) {
+            // This intentionally drops any trailing comma in order to improve
+            // formatting.
+            builder.write(', ');
+          }
+          if (parameter is DefaultFormalParameter) {
+            if (groupEnd == null) {
+              if (parameter.isNamed) {
+                groupEnd = '}';
+                builder.write('{');
+              } else {
+                groupEnd = ']';
+                builder.write('[');
+              }
+            }
+            parameter = (parameter as DefaultFormalParameter).parameter;
+          }
+          if (parameter is FunctionTypedFormalParameter) {
+            builder.write(utils.getNodeText(parameter));
+          } else if (parameter is SimpleFormalParameter) {
+            if (parameter.metadata.isNotEmpty) {
+              builder
+                  .write(utils.getRangeText(range.nodes(parameter.metadata)));
+            }
+            if (parameter.requiredKeyword != null) {
+              builder.write('required ');
+            }
+            if (parameter.covariantKeyword != null) {
+              builder.write('covariant ');
+            }
+            var keyword = parameter.keyword;
+            if (keyword != null && keyword.type != Keyword.VAR) {
+              builder.write(keyword.lexeme);
+            }
+            if (parameter.type == null) {
+              builder.write('dynamic');
+            } else {
+              builder.write(utils.getNodeText(parameter.type));
+            }
+            if (parameter.isNamed) {
+              builder.write(' ');
+              builder.write(parameter.identifier.name);
+            }
+          }
+        }
+        if (groupEnd != null) {
+          builder.write(groupEnd);
+        }
+        builder.write(')');
+      });
+    });
+  }
+}
+
+class _ReferenceFinder extends RecursiveAstVisitor {
+  final String typeName;
+
+  TypeName reference;
+
+  int count = 0;
+
+  _ReferenceFinder(this.typeName);
+
+  @override
+  void visitTypeName(TypeName node) {
+    if (node.name.name == typeName) {
+      reference ??= node;
+      count++;
+    }
+    super.visitTypeName(node);
+  }
+}

pkg/analysis_server/lib/src/services/correction/fix.dart

@@ -268,6 +268,8 @@ class DartFixKind {
       FixKind('IMPORT_LIBRARY_SHOW', 55, "Update library '{0}' import");
   static const INLINE_INVOCATION =
       FixKind('INLINE_INVOCATION', 30, "Inline invocation of '{0}'");
+  static const INLINE_TYPEDEF =
+      FixKind('INLINE_TYPEDEF', 30, "Inline the definition of '{0}'");
   static const INSERT_SEMICOLON = FixKind('INSERT_SEMICOLON', 50, "Insert ';'");
   static const MAKE_CLASS_ABSTRACT =
       FixKind('MAKE_CLASS_ABSTRACT', 50, "Make class '{0}' abstract");

pkg/analysis_server/lib/src/services/correction/fix_internal.dart

@@ -18,6 +18,7 @@ import 'package:analysis_server/src/services/correction/dart/convert_to_map_lite
 import 'package:analysis_server/src/services/correction/dart/convert_to_null_aware.dart';
 import 'package:analysis_server/src/services/correction/dart/convert_to_set_literal.dart';
 import 'package:analysis_server/src/services/correction/dart/convert_to_where_type.dart';
+import 'package:analysis_server/src/services/correction/dart/inline_typedef.dart';
 import 'package:analysis_server/src/services/correction/dart/remove_dead_if_null.dart';
 import 'package:analysis_server/src/services/correction/dart/remove_if_null_operator.dart';
 import 'package:analysis_server/src/services/correction/dart/replace_with_eight_digit_hex.dart';
@@ -4717,7 +4718,9 @@ class FixProcessor extends BaseProcessor {
       await compute(RemoveDeadIfNull());
     } else if (errorCode is LintCode) {
       String name = errorCode.name;
-      if (name == LintNames.avoid_returning_null_for_future) {
+      if (name == LintNames.avoid_private_typedef_functions) {
+        await compute(InlineTypedef());
+      } else if (name == LintNames.avoid_returning_null_for_future) {
         await compute(WrapInFuture());
       } else if (name == LintNames.prefer_collection_literals) {
         await compute(ConvertToListLiteral());

pkg/analysis_server/lib/src/services/linter/lint_names.dart

@@ -19,6 +19,8 @@ class LintNames {
   static const String avoid_relative_lib_imports = 'avoid_relative_lib_imports';
   static const String avoid_return_types_on_setters =
       'avoid_return_types_on_setters';
+  static const String avoid_private_typedef_functions =
+      'avoid_private_typedef_functions';
   static const String avoid_returning_null_for_future =
       'avoid_returning_null_for_future';
   static const String avoid_types_on_closure_parameters =

pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart

@@ -27,13 +27,17 @@ abstract class FixProcessorLintTest extends FixProcessorTest {
   /// The offset of the lint marker in the code being analyzed.
   int lintOffset = -1;
 
+  /// Return a list of the experiments that are to be enabled for tests in this
+  /// class, or `null` if there are no experiments that should be enabled.
+  List<String> get experiments => null;
+
   /// Return the lint code being tested.
   String get lintCode;
 
   @override
   void setUp() {
     super.setUp();
-    createAnalysisOptionsFile(lints: [lintCode]);
+    createAnalysisOptionsFile(experiments: experiments, lints: [lintCode]);
   }
 
   /// Find the error that is to be fixed by computing the errors in the file,

pkg/analysis_server/test/src/services/correction/fix/inline_typedef_test.dart

@@ -0,0 +1,162 @@
+// Copyright (c) 2020, 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/services/correction/fix.dart';
+import 'package:analysis_server/src/services/linter/lint_names.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import 'fix_processor.dart';
+
+void main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(InlineTypedefTest);
+    defineReflectiveTests(InlineTypedefWithNNBDTest);
+  });
+}
+
+@reflectiveTest
+class InlineTypedefTest extends FixProcessorLintTest {
+  @override
+  FixKind get kind => DartFixKind.INLINE_TYPEDEF;
+
+  @override
+  String get lintCode => LintNames.avoid_private_typedef_functions;
+
+  Future<void> test_generic_parameter_optionalNamed() async {
+    await resolveTestUnit('''
+typedef _F = Function({int i});
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function({int i}) f) {}
+''');
+  }
+
+  Future<void> test_generic_parameter_optionalPositional_withName() async {
+    await resolveTestUnit('''
+typedef _F = Function([int i]);
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function([int]) f) {}
+''');
+  }
+
+  Future<void> test_generic_parameter_optionalPositional_withoutName() async {
+    await resolveTestUnit('''
+typedef _F = Function([int]);
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function([int]) f) {}
+''');
+  }
+
+  Future<void> test_generic_parameter_requiredPositional_withName() async {
+    await resolveTestUnit('''
+typedef _F = Function(int i);
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function(int) f) {}
+''');
+  }
+
+  Future<void> test_generic_parameter_requiredPositional_withoutName() async {
+    await resolveTestUnit('''
+typedef _F = Function(int);
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function(int) f) {}
+''');
+  }
+
+  Future<void> test_generic_returnType() async {
+    await resolveTestUnit('''
+typedef _F = void Function();
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(void Function() f) {}
+''');
+  }
+
+  Future<void> test_generic_typeParameters() async {
+    await resolveTestUnit('''
+typedef _F = Function<T>(T);
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function<T>(T) f) {}
+''');
+  }
+
+  Future<void> test_nonGeneric_parameter_requiredPositional_typed() async {
+    await resolveTestUnit('''
+typedef _F(int i);
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function(int) f) {}
+''');
+  }
+
+  Future<void> test_nonGeneric_parameter_requiredPositional_untyped() async {
+    await resolveTestUnit('''
+typedef _F(i);
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function(dynamic) f) {}
+''');
+  }
+
+  Future<void> test_nonGeneric_returnType() async {
+    await resolveTestUnit('''
+typedef void _F();
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(void Function() f) {}
+''');
+  }
+
+  Future<void> test_nonGeneric_typeParameters() async {
+    await resolveTestUnit('''
+typedef _F<T>(T t);
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function<T>(T) f) {}
+''');
+  }
+}
+
+@reflectiveTest
+class InlineTypedefWithNNBDTest extends InlineTypedefTest {
+  @override
+  List<String> get experiments => ['non-nullable'];
+
+  Future<void> test_generic_parameter_requiredNamed() async {
+    await resolveTestUnit('''
+typedef _F = Function({required int i});
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function({required int i}) f) {}
+''');
+  }
+
+  Future<void> test_nonGeneric_parameter_requiredNamed() async {
+    await resolveTestUnit('''
+typedef _F({required int i});
+void g(_F f) {}
+''');
+    await assertHasFix('''
+void g(Function({required int i}) f) {}
+''');
+  }
+}

pkg/analysis_server/test/src/services/correction/fix/test_all.dart

@@ -82,6 +82,7 @@ import 'import_library_project_test.dart' as import_library_project;
 import 'import_library_sdk_test.dart' as import_library_sdk;
 import 'import_library_show_test.dart' as import_library_show;
 import 'inline_invocation_test.dart' as inline_invocation;
+import 'inline_typedef_test.dart' as inline_typedef;
 import 'insert_semicolon_test.dart' as insert_semicolon;
 import 'make_class_abstract_test.dart' as make_class_abstract;
 import 'make_field_not_final_test.dart' as make_field_not_final;
@@ -221,6 +222,7 @@ void main() {
     import_library_sdk.main();
     import_library_show.main();
     inline_invocation.main();
+    inline_typedef.main();
     insert_semicolon.main();
     make_class_abstract.main();
     make_field_not_final.main();

pkg/analyzer/lib/src/dart/ast/ast.dart

@@ -1393,14 +1393,14 @@ class ChildEntities
   @override
   Iterator<SyntacticEntity> get iterator => _entities.iterator;
 
-  /// Add an AST node or token as the next child entity, if it is not null.
+  /// Add an AST node or token as the next child entity, if it is not `null`.
   void add(SyntacticEntity entity) {
     if (entity != null) {
       _entities.add(entity);
     }
   }
 
-  /// Add the given items as the next child entities, if [items] is not null.
+  /// Add the given items as the next child entities, if [items] is not `null`.
   void addAll(Iterable<SyntacticEntity> items) {
     if (items != null) {
       _entities.addAll(items);
@@ -4067,6 +4067,8 @@ class FieldFormalParameterImpl extends NormalFormalParameterImpl
     NodeList<Annotation> metadata = this.metadata;
     if (metadata.isNotEmpty) {
       return metadata.beginToken;
+    } else if (requiredKeyword != null) {
+      return requiredKeyword;
     } else if (covariantKeyword != null) {
       return covariantKeyword;
     } else if (keyword != null) {
@@ -5166,11 +5168,19 @@ class FunctionTypedFormalParameterImpl extends NormalFormalParameterImpl
   }
 
   @override
-  Token get beginToken =>
-      this.metadata.beginToken ??
-      covariantKeyword ??
-      _returnType?.beginToken ??
-      identifier?.beginToken;
+  Token get beginToken {
+    NodeList<Annotation> metadata = this.metadata;
+    if (metadata.isNotEmpty) {
+      return metadata.beginToken;
+    } else if (requiredKeyword != null) {
+      return requiredKeyword;
+    } else if (covariantKeyword != null) {
+      return covariantKeyword;
+    } else if (_returnType != null) {
+      return _returnType.beginToken;
+    }
+    return identifier?.beginToken;
+  }
 
   @override
   Iterable<SyntacticEntity> get childEntities =>
@@ -7691,9 +7701,7 @@ abstract class NormalFormalParameterImpl extends FormalParameterImpl
     } else {
       result.addAll(sortedCommentAndAnnotations);
     }
-    if (covariantKeyword != null) {
-      result.add(covariantKeyword);
-    }
+    result..add(requiredKeyword)..add(covariantKeyword);
     return result;
   }
 
@@ -8725,6 +8733,8 @@ class SimpleFormalParameterImpl extends NormalFormalParameterImpl
     NodeList<Annotation> metadata = this.metadata;
     if (metadata.isNotEmpty) {
       return metadata.beginToken;
+    } else if (requiredKeyword != null) {
+      return requiredKeyword;
     } else if (covariantKeyword != null) {
       return covariantKeyword;
     } else if (keyword != null) {