From 27ed75b7bcbcbbee01d9ccbe119b0f0fcd01cfbf Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Tue, 5 May 2026 14:50:40 -0700 Subject: [PATCH] [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 --- .../correction/dart/remove_type_name.dart | 59 +++++++++++++++--- .../correction/fix/remove_type_name_test.dart | 60 +++++++++++++++++-- 2 files changed, 107 insertions(+), 12 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/remove_type_name.dart b/pkg/analysis_server/lib/src/services/correction/dart/remove_type_name.dart index 37aa52876f1..da93f8dc710 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/remove_type_name.dart +++ b/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); diff --git a/pkg/analysis_server/test/src/services/correction/fix/remove_type_name_test.dart b/pkg/analysis_server/test/src/services/correction/fix/remove_type_name_test.dart index 3edbce5c7ed..f561d1e1825 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/remove_type_name_test.dart +++ b/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 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 test_factory_unnamed() async { await resolveTestCode(''' class C { @@ -73,7 +91,24 @@ class C { '''); await assertHasFix(''' class C { - factory () => C._(); + factory() => C._(); + + new _(); +} +'''); + } + + Future 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 test_new_named() async { + await resolveTestCode(''' +class C { + new C.name(); +} +'''); + await assertHasFix( + ''' +class C { + new name(); +} +''', + filter: (diagnostic) => + diagnostic.diagnosticCode == diag.unnecessaryTypeNameInConstructor, + ); + } }