[analysis server] Fix extra space when switching to new constructor syntax.

Adds logic to the `RemoveTypeName` correction producer to ensure that
after the correction is applied, the spacing matches what `dart
format` would do. For example, `C()` is changed to `new()` rather than
`new ()`, and `factory C()` is changed to `factory()` rather than
`factory ()`.

This will make it easier for me to visually inspect intermediate
results when transitioning the SDK and google3 to language version
3.13. It also should provide a (marginally) nicer user experience.

Change-Id: I30babde38e58438ea39279ed0d5859ce6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500880
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

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

@@ -2,9 +2,11 @@
 // 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/base/source_range.dart';
 import 'package:analysis_server/src/services/correction/fix.dart';
 import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
 import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 import 'package:analyzer_plugin/utilities/range_factory.dart';
@@ -31,14 +33,55 @@ class RemoveTypeName extends ResolvedCorrectionProducer {
       var typeName = node.typeName;
       if (typeName == null) return;
       var name = node.name;
-      var replacementRange = name == null
-          ? range.node(typeName)
-          : (name.lexeme == 'new'
-                ? range.startEnd(typeName, name)
-                : range.startStart(typeName, name));
-      var replacement = '';
-      if (node.newKeyword == null && node.factoryKeyword == null) {
-        replacement = 'new ';
+      var newOrFactoryKeyword = node.newKeyword ?? node.factoryKeyword;
+      var replacementBeginToken = typeName.beginToken;
+      Token replacementEndToken;
+      if (name == null) {
+        replacementEndToken = typeName.endToken;
+      } else if (name.lexeme == 'new') {
+        replacementEndToken = name;
+      } else {
+        var period = node.period;
+        if (period == null) {
+          // Name but no `.`. This can only happen in the event of a syntax
+          // error, so for safety, don't produce any correction.
+          return;
+        }
+        replacementEndToken = period;
+      }
+      SourceRange replacementRange;
+      String replacement;
+      if (newOrFactoryKeyword != null) {
+        if (name == null &&
+            newOrFactoryKeyword.end + 1 == replacementBeginToken.offset) {
+          // Changing e.g., `factory ClassName(...)` to `factory(...)`, so
+          // delete the space after `factory`.
+          replacementRange = range.endEnd(
+            newOrFactoryKeyword,
+            replacementEndToken,
+          );
+        } else {
+          replacementRange = range.startEnd(
+            replacementBeginToken,
+            replacementEndToken,
+          );
+        }
+        replacement = '';
+      } else {
+        replacementRange = range.startEnd(
+          replacementBeginToken,
+          replacementEndToken,
+        );
+        if (replacementEndToken.isIdentifier) {
+          // The text we are replacing ends in an identifier, so if a space is
+          // needed after `new`, it is already present.
+          replacement = 'new';
+        } else {
+          // The text we are replacing is a typeName followed by `.`. Typically
+          // this is followed immediately by the constructor name, so insert a
+          // space after `new`.
+          replacement = 'new ';
+        }
       }
       await builder.addDartFileEdit(file, (builder) {
         builder.addSimpleReplacement(replacementRange, replacement);

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

@@ -4,6 +4,7 @@
 
 import 'package:analysis_server/src/services/correction/fix.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:linter/src/diagnostic.dart' as diag;
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import 'fix_processor.dart';
@@ -31,7 +32,7 @@ class C {
     await assertHasFix(r'''
 class C {
   new name();
-  new ();
+  new();
   factory f() => C();
 }
 ''');
@@ -63,6 +64,23 @@ class C {
 ''');
   }
 
+  Future<void> test_factory_named_withInterveningComment() async {
+    await resolveTestCode('''
+class C {
+  factory /* comment */ C.name() => C._();
+
+  new _();
+}
+''');
+    await assertHasFix('''
+class C {
+  factory /* comment */ name() => C._();
+
+  new _();
+}
+''');
+  }
+
   Future<void> test_factory_unnamed() async {
     await resolveTestCode('''
 class C {
@@ -73,7 +91,24 @@ class C {
 ''');
     await assertHasFix('''
 class C {
-  factory () => C._();
+  factory() => C._();
+
+  new _();
+}
+''');
+  }
+
+  Future<void> test_factory_unnamed_withInterveningComment() async {
+    await resolveTestCode('''
+class C {
+  factory /* comment */ C() => C._();
+
+  new _();
+}
+''');
+    await assertHasFix('''
+class C {
+  factory /* comment */ () => C._();
 
   new _();
 }
@@ -88,7 +123,7 @@ class C {
 ''');
     await assertHasFix('''
 class C {
-  new ();
+  new();
 }
 ''');
   }
@@ -114,8 +149,25 @@ class C {
 ''');
     await assertHasFix('''
 class C {
-  new ();
+  new();
 }
 ''');
   }
+
+  Future<void> test_new_named() async {
+    await resolveTestCode('''
+class C {
+  new C.name();
+}
+''');
+    await assertHasFix(
+      '''
+class C {
+  new name();
+}
+''',
+      filter: (diagnostic) =>
+          diagnostic.diagnosticCode == diag.unnecessaryTypeNameInConstructor,
+    );
+  }
 }