From 82c3d74505384fe55052d082d6650fa980156440 Mon Sep 17 00:00:00 2001 From: FMorschel Date: Fri, 7 Mar 2025 10:29:44 -0800 Subject: [PATCH] [DAS] Renaming positional parameters to rename hierarchy equivalent Fixes: https://github.com/dart-lang/sdk/issues/53187 Change-Id: Ie0aaea2e24667ad04a95ce6522fd687710501230 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/412520 Reviewed-by: Konstantin Shcheglov Commit-Queue: Phil Quitslund Auto-Submit: Felipe Morschel Reviewed-by: Phil Quitslund --- .../services/refactoring/legacy/rename.dart | 7 + .../refactoring/legacy/rename_local.dart | 31 +- .../refactoring/legacy/rename_parameter.dart | 74 +- .../lib/src/services/search/hierarchy.dart | 46 ++ .../test/edit/refactoring_test.dart | 2 +- .../refactoring/legacy/rename_local_test.dart | 374 +---------- .../legacy/rename_parameter_test.dart | 631 ++++++++++++++++++ .../services/refactoring/legacy/test_all.dart | 2 + pkg/analyzer/api.txt | 1 + .../lib/dart/analysis/code_style_options.dart | 5 + .../analysis_options/code_style_options.dart | 4 + 11 files changed, 798 insertions(+), 379 deletions(-) create mode 100644 pkg/analysis_server/test/services/refactoring/legacy/rename_parameter_test.dart diff --git a/pkg/analysis_server/lib/src/services/refactoring/legacy/rename.dart b/pkg/analysis_server/lib/src/services/refactoring/legacy/rename.dart index 6ec0203e4b8..984c84208d8 100644 --- a/pkg/analysis_server/lib/src/services/refactoring/legacy/rename.dart +++ b/pkg/analysis_server/lib/src/services/refactoring/legacy/rename.dart @@ -8,7 +8,9 @@ import 'package:analysis_server/src/services/correction/util.dart'; import 'package:analysis_server/src/services/refactoring/legacy/refactoring.dart'; import 'package:analysis_server/src/services/refactoring/legacy/refactoring_internal.dart'; import 'package:analysis_server/src/services/search/search_engine.dart'; +import 'package:analyzer/dart/analysis/code_style_options.dart'; import 'package:analyzer/dart/element/element2.dart'; +import 'package:analyzer/file_system/file_system.dart'; import 'package:analyzer/src/dart/analysis/session_helper.dart'; import 'package:analyzer/src/dart/element/element.dart'; import 'package:analyzer/src/generated/java_core.dart'; @@ -154,6 +156,11 @@ abstract class RenameRefactoringImpl extends RefactoringImpl /// Adds individual edits to [change]. Future fillChange(); + CodeStyleOptions getCodeStyleOptions(File file) => + sessionHelper.session.analysisContext + .getAnalysisOptionsForFile(file) + .codeStyleOptions; + static String _getOldName(Element2 element) { if (element is ConstructorElement2) { var name = element.name3; diff --git a/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_local.dart b/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_local.dart index 24be3f4f0ff..db8f815f012 100644 --- a/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_local.dart +++ b/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_local.dart @@ -17,6 +17,7 @@ import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element2.dart'; import 'package:analyzer/source/source_range.dart'; import 'package:analyzer/src/dart/element/element.dart'; +import 'package:analyzer/src/generated/java_core.dart'; class ConflictValidatorVisitor extends RecursiveAstVisitor { final RefactoringStatus result; @@ -88,7 +89,12 @@ class ConflictValidatorVisitor extends RecursiveAstVisitor { if (_isVisibleWithTarget(declaredElement)) { conflictingLocals.add(declaredElement); var nodeKind = declaredElement.kind.displayName; - var message = "Duplicate $nodeKind '$newName'."; + var message = format( + "Duplicate {0} of name '{1}'{2}.", + nodeKind, + newName, + declaredElement.declarationLocation, + ); result.addError(message, newLocation_fromElement(declaredElement)); return; } @@ -200,3 +206,26 @@ class RenameLocalRefactoringImpl extends RenameRefactoringImpl { processor.addReferenceEdits(references); } } + +extension on Element2 { + String get declarationLocation { + var sourceName = firstFragment.libraryFragment!.source.shortName; + var executable = enclosingElement2; + String className = ''; + String executableName = ''; + if (executable is MethodElement2) { + var namescope = executable.enclosingElement2 as ClassElement2?; + className = namescope?.displayName ?? ''; + if (className.isNotEmpty && executable.displayName.isNotEmpty) { + className += '.'; + } + executableName = executable.displayName; + } else if (executable is TopLevelFunctionElement) { + executableName = executable.displayName; + } + if (executableName.isEmpty) { + return " in '$sourceName'"; + } + return " at $className$executableName in '$sourceName'"; + } +} diff --git a/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_parameter.dart b/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_parameter.dart index 52e7454fef5..286befe5113 100644 --- a/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_parameter.dart +++ b/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_parameter.dart @@ -11,10 +11,12 @@ import 'package:analysis_server/src/services/refactoring/legacy/visible_ranges_c import 'package:analysis_server/src/services/search/hierarchy.dart'; import 'package:analyzer/dart/element/element2.dart'; import 'package:analyzer/src/generated/java_core.dart'; +import 'package:analyzer_plugin/protocol/protocol_common.dart'; /// A [Refactoring] for renaming [FormalParameterElement]s. class RenameParameterRefactoringImpl extends RenameRefactoringImpl { List elements = []; + bool _renameAllPositionalOccurences = false; RenameParameterRefactoringImpl( super.workspace, @@ -33,6 +35,7 @@ class RenameParameterRefactoringImpl extends RenameRefactoringImpl { @override Future checkFinalConditions() async { var result = RefactoringStatus(); + var conflictResult = RefactoringStatus(); await _prepareElements(); for (var element in elements) { if (newName.startsWith('_') && element.isNamed) { @@ -45,16 +48,39 @@ class RenameParameterRefactoringImpl extends RenameRefactoringImpl { break; } var resolvedUnit = await sessionHelper.getResolvedUnitByElement(element); - var unit = resolvedUnit?.unit; - unit?.accept( - ConflictValidatorVisitor( - result, - newName, - element, - VisibleRangesComputer.forNode(unit), + if (resolvedUnit != null) { + // If any of the resolved units have the lint enabled, we should avoid + // renaming method parameters separately from the other implementations. + if (element.isPositional && !_renameAllPositionalOccurences) { + _renameAllPositionalOccurences |= + getCodeStyleOptions( + resolvedUnit.file, + ).avoidRenamingMethodParameters; + } + + var unit = resolvedUnit.unit; + unit.accept( + ConflictValidatorVisitor( + conflictResult, + newName, + element, + VisibleRangesComputer.forNode(unit), + ), + ); + } + } + if (_renameAllPositionalOccurences && elements.length > 1) { + result.addStatus( + _RefactoringStatusExt.from( + 'This will also rename all related positional parameters ' + 'to the same name.', + conflictResult, ), ); } + if (result.problem == null) { + return conflictResult; + } return result; } @@ -69,6 +95,11 @@ class RenameParameterRefactoringImpl extends RenameRefactoringImpl { Future fillChange() async { var processor = RenameProcessor(workspace, sessionHelper, change, newName); for (var element in elements) { + if (element != this.element && + element.isPositional && + !_renameAllPositionalOccurences) { + continue; + } var fieldRenamed = false; if (element is FieldFormalParameterElement2) { var field = element.field2; @@ -104,10 +135,31 @@ class RenameParameterRefactoringImpl extends RenameRefactoringImpl { Future _prepareElements() async { var element = this.element; if (element.isNamed) { - elements = - (await getHierarchyNamedParameters(searchEngine, element)).toList(); - } else { - elements = [element]; + elements = await getHierarchyNamedParameters(searchEngine, element); + } else if (element.isPositional) { + elements = await getHierarchyPositionalParameters(searchEngine, element); } } } + +extension _RefactoringStatusExt on RefactoringStatus { + String get messagesAggregated { + if (problems.isEmpty) { + return ''; + } + if (problems.length == 1) { + return '\n${problems.first.message}'; + } + return '\n${problems.first.message} And ${problems.length - 1} more ' + 'error${problems.length > 1 ? 's' : ''}.'; + } + + static RefactoringStatus from(String message, RefactoringStatus result) { + var constructor = switch (result.severity) { + RefactoringProblemSeverity.ERROR => RefactoringStatus.error, + RefactoringProblemSeverity.FATAL => RefactoringStatus.fatal, + _ => RefactoringStatus.warning, + }; + return constructor(message + result.messagesAggregated); + } +} diff --git a/pkg/analysis_server/lib/src/services/search/hierarchy.dart b/pkg/analysis_server/lib/src/services/search/hierarchy.dart index 332129d21b6..00d01a40a7c 100644 --- a/pkg/analysis_server/lib/src/services/search/hierarchy.dart +++ b/pkg/analysis_server/lib/src/services/search/hierarchy.dart @@ -221,6 +221,37 @@ Future> getHierarchyNamedParameters( return [element]; } +Future> getHierarchyPositionalParameters( + SearchEngine searchEngine, + FormalParameterElement element, +) async { + if (element.isPositional) { + var method = element.enclosingElement2; + if (method is MethodElement2) { + var index = method.parameterIndex(element); + // Should not ever happen but this means we can't find the index. + if (index == null) { + return [element]; + } + var hierarchyParameters = []; + var hierarchyMembers = await getHierarchyMembers(searchEngine, method); + for (var hierarchyMethod in hierarchyMembers) { + if (hierarchyMethod is MethodElement2) { + for (var hierarchyParameter in hierarchyMethod.formalParameters) { + if (hierarchyParameter.isPositional && + hierarchyMethod.parameterIndex(hierarchyParameter) == index) { + hierarchyParameters.add(hierarchyParameter); + break; + } + } + } + } + return hierarchyParameters; + } + } + return [element]; +} + /// Returns non-synthetic members of the given [InterfaceElement2] and its super /// classes. /// @@ -254,3 +285,18 @@ String? _getBaseName(Element2 element) { } return element.name3; } + +extension on MethodElement2 { + int? parameterIndex(FormalParameterElement parameter) { + var index = 0; + for (var positionalParameter in formalParameters.where( + (p) => p.isPositional, + )) { + if (positionalParameter == parameter) { + return index; + } + index++; + } + return null; + } +} diff --git a/pkg/analysis_server/test/edit/refactoring_test.dart b/pkg/analysis_server/test/edit/refactoring_test.dart index 7b85a6e8564..8a95dec94ef 100644 --- a/pkg/analysis_server/test/edit/refactoring_test.dart +++ b/pkg/analysis_server/test/edit/refactoring_test.dart @@ -2739,7 +2739,7 @@ void f() { expect(problems, hasLength(1)); assertResultProblemsError( problems, - "Duplicate local variable 'newName'.", + "Duplicate local variable of name 'newName' at f in 'test.dart'.", ); }); } diff --git a/pkg/analysis_server/test/services/refactoring/legacy/rename_local_test.dart b/pkg/analysis_server/test/services/refactoring/legacy/rename_local_test.dart index 1955f628a4f..14622469a79 100644 --- a/pkg/analysis_server/test/services/refactoring/legacy/rename_local_test.dart +++ b/pkg/analysis_server/test/services/refactoring/legacy/rename_local_test.dart @@ -30,7 +30,7 @@ void f() { assertRefactoringStatus( status, RefactoringProblemSeverity.ERROR, - expectedMessage: "Duplicate function 'newName'.", + expectedMessage: "Duplicate function of name 'newName' in 'test.dart'.", expectedContextSearch: 'newName() => 1', ); } @@ -49,7 +49,7 @@ void f() { assertRefactoringStatus( status, RefactoringProblemSeverity.ERROR, - expectedMessage: "Duplicate function 'newName'.", + expectedMessage: "Duplicate function of name 'newName' in 'test.dart'.", ); } @@ -69,7 +69,9 @@ void f() { assertRefactoringStatus( status, RefactoringProblemSeverity.ERROR, - expectedMessage: "Duplicate local variable 'newName'.", + expectedMessage: + "Duplicate local variable of name 'newName' at f in " + "'test.dart'.", expectedContextSearch: 'newName = 1;', ); } @@ -88,7 +90,9 @@ void f() { assertRefactoringStatus( status, RefactoringProblemSeverity.ERROR, - expectedMessage: "Duplicate local variable 'newName'.", + expectedMessage: + "Duplicate local variable of name 'newName' at f in " + "'test.dart'.", expectedContextSearch: 'newName = 1;', ); } @@ -177,34 +181,6 @@ class A { ); } - Future - test_checkFinalConditions_shadows_classMember_namedParameter() async { - await indexTestUnit(''' -class A { - foo({test = 1}) { // in A - } -} -class B extends A { - var newName = 1; - foo({test = 1}) { - print(newName); - } -} -'''); - createRenameRefactoringAtString('test = 1}) { // in A'); - // check status - refactoring.newName = 'newName'; - var status = await refactoring.checkFinalConditions(); - assertRefactoringStatus( - status, - RefactoringProblemSeverity.ERROR, - expectedMessage: - 'Usage of field "B.newName" declared in "test.dart" ' - 'will be shadowed by renamed parameter.', - expectedContextSearch: 'newName);', - ); - } - Future test_checkFinalConditions_shadows_classMemberOK_qualifiedReference() async { await indexTestUnit(''' @@ -294,47 +270,6 @@ void f() { assertRefactoringStatusOK(refactoring.checkNewName()); } - Future test_checkNewName_ParameterElement() async { - await indexTestUnit(''' -void f(test) { -} -'''); - createRenameRefactoringAtString('test) {'); - // empty - refactoring.newName = ''; - assertRefactoringStatus( - refactoring.checkNewName(), - RefactoringProblemSeverity.FATAL, - expectedMessage: 'Parameter name must not be empty.', - ); - // OK - refactoring.newName = 'newName'; - assertRefactoringStatusOK(refactoring.checkNewName()); - } - - Future test_createChange_closure_parameter() async { - await indexTestUnit(''' -void f(void Function(int) _) {} - -void g() => f((parameter) { - print(parameter); -}); -'''); - // configure refactoring - createRenameRefactoringAtString('parameter) {'); - expect(refactoring.refactoringName, 'Rename Parameter'); - expect(refactoring.elementKindName, 'parameter'); - refactoring.newName = 'newName'; - // validate change - return assertSuccessfulRefactoring(''' -void f(void Function(int) _) {} - -void g() => f((newName) { - print(newName); -}); -'''); - } - Future test_createChange_localFunction() async { await indexTestUnit(''' void f() { @@ -539,299 +474,6 @@ void f() { '''); } - Future test_createChange_parameter_named() async { - await indexTestUnit(''' -myFunction({required int test}) { - test = 1; - test += 2; - print(test); -} -void f() { - myFunction(test: 2); -} -'''); - // configure refactoring - createRenameRefactoringAtString('test}) {'); - expect(refactoring.refactoringName, 'Rename Parameter'); - expect(refactoring.elementKindName, 'parameter'); - refactoring.newName = 'newName'; - // validate change - return assertSuccessfulRefactoring(''' -myFunction({required int newName}) { - newName = 1; - newName += 2; - print(newName); -} -void f() { - myFunction(newName: 2); -} -'''); - } - - Future test_createChange_parameter_named_anywhere() async { - await indexTestUnit(''' -myFunction(int a, int b, {required int test}) { - test = 1; - test += 2; - print(test); -} -void f() { - myFunction(0, test: 2, 1); -} -'''); - // configure refactoring - createRenameRefactoringAtString('test}) {'); - expect(refactoring.refactoringName, 'Rename Parameter'); - expect(refactoring.elementKindName, 'parameter'); - refactoring.newName = 'newName'; - // validate change - return assertSuccessfulRefactoring(''' -myFunction(int a, int b, {required int newName}) { - newName = 1; - newName += 2; - print(newName); -} -void f() { - myFunction(0, newName: 2, 1); -} -'''); - } - - Future test_createChange_parameter_named_inOtherFile() async { - var a = convertPath('$testPackageLibPath/a.dart'); - var b = convertPath('$testPackageLibPath/b.dart'); - - newFile(a, r''' -class A { - A({test}); -} -'''); - newFile(b, r''' -import 'a.dart'; - -void f() { - new A(test: 2); -} -'''); - await analyzeTestPackageFiles(); - - testFilePath = a; - await resolveTestFile(); - - createRenameRefactoringAtString('test});'); - expect(refactoring.refactoringName, 'Rename Parameter'); - refactoring.newName = 'newName'; - - await assertSuccessfulRefactoring(''' -class A { - A({newName}); -} -'''); - assertFileChangeResult(b, ''' -import 'a.dart'; - -void f() { - new A(newName: 2); -} -'''); - } - - Future - test_createChange_parameter_named_ofConstructor_genericClass() async { - await indexTestUnit(''' -class A { - A({required T test}); -} - -void f() { - A(test: 0); -} -'''); - // configure refactoring - createRenameRefactoringAtString('test}'); - expect(refactoring.refactoringName, 'Rename Parameter'); - expect(refactoring.elementKindName, 'parameter'); - refactoring.newName = 'newName'; - // validate change - return assertSuccessfulRefactoring(''' -class A { - A({required T newName}); -} - -void f() { - A(newName: 0); -} -'''); - } - - Future test_createChange_parameter_named_ofMethod_genericClass() async { - await indexTestUnit(''' -class A { - void foo({required T test}) {} -} - -void f(A a) { - a.foo(test: 0); -} -'''); - // configure refactoring - createRenameRefactoringAtString('test}'); - expect(refactoring.refactoringName, 'Rename Parameter'); - expect(refactoring.elementKindName, 'parameter'); - refactoring.newName = 'newName'; - // validate change - return assertSuccessfulRefactoring(''' -class A { - void foo({required T newName}) {} -} - -void f(A a) { - a.foo(newName: 0); -} -'''); - } - - Future test_createChange_parameter_named_super() async { - await indexTestUnit(''' -class A { - A({required int test}); // 0 -} -class B extends A { - B({required super.test}); -} -'''); - // configure refactoring - createRenameRefactoringAtString('test}); // 0'); - expect(refactoring.refactoringName, 'Rename Parameter'); - expect(refactoring.elementKindName, 'parameter'); - refactoring.newName = 'newName'; - // validate change - return assertSuccessfulRefactoring(''' -class A { - A({required int newName}); // 0 -} -class B extends A { - B({required super.newName}); -} -'''); - } - - Future test_createChange_parameter_named_updateHierarchy() async { - await indexUnit('$testPackageLibPath/test2.dart', ''' -library test2; -class A { - void foo({int? test}) { - print(test); - } -} -class B extends A { - void foo({int? test}) { - print(test); - } -} -'''); - await indexTestUnit(''' -import 'test2.dart'; -void f() { - new A().foo(test: 10); - new B().foo(test: 20); - new C().foo(test: 30); -} -class C extends A { - void foo({int? test}) { - print(test); - } -} -'''); - // configure refactoring - createRenameRefactoringAtString('test: 20'); - expect(refactoring.refactoringName, 'Rename Parameter'); - refactoring.newName = 'newName'; - // validate change - await assertSuccessfulRefactoring(''' -import 'test2.dart'; -void f() { - new A().foo(newName: 10); - new B().foo(newName: 20); - new C().foo(newName: 30); -} -class C extends A { - void foo({int? newName}) { - print(newName); - } -} -'''); - assertFileChangeResult('$testPackageLibPath/test2.dart', ''' -library test2; -class A { - void foo({int? newName}) { - print(newName); - } -} -class B extends A { - void foo({int? newName}) { - print(newName); - } -} -'''); - } - - Future test_createChange_parameter_optionalPositional() async { - await indexTestUnit(''' -myFunction([int? test]) { - test = 1; - test += 2; - print(test); -} -void f() { - myFunction(2); -} -'''); - // configure refactoring - createRenameRefactoringAtString('test]) {'); - expect(refactoring.refactoringName, 'Rename Parameter'); - expect(refactoring.elementKindName, 'parameter'); - refactoring.newName = 'newName'; - // validate change - return assertSuccessfulRefactoring(''' -myFunction([int? newName]) { - newName = 1; - newName += 2; - print(newName); -} -void f() { - myFunction(2); -} -'''); - } - - Future test_createChange_parameter_positional_super() async { - await indexTestUnit(''' -class A { - A(int test); // 0 -} -class B extends A { - B(super.test); -} -'''); - - createRenameRefactoringAtString('test); // 0'); - expect(refactoring.refactoringName, 'Rename Parameter'); - expect(refactoring.elementKindName, 'parameter'); - refactoring.newName = 'newName'; - - // The name of the super-formal parameter does not have to be the same. - // So, we don't rename it. - return assertSuccessfulRefactoring(''' -class A { - A(int newName); // 0 -} -class B extends A { - B(super.test); -} -'''); - } - Future test_createChange_patternVariable_declarationStatement() async { await indexTestUnit(''' void f(Object? x) { diff --git a/pkg/analysis_server/test/services/refactoring/legacy/rename_parameter_test.dart b/pkg/analysis_server/test/services/refactoring/legacy/rename_parameter_test.dart new file mode 100644 index 00000000000..29018abe551 --- /dev/null +++ b/pkg/analysis_server/test/services/refactoring/legacy/rename_parameter_test.dart @@ -0,0 +1,631 @@ +// Copyright (c) 2025, 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:analyzer_plugin/protocol/protocol_common.dart'; +import 'package:linter/src/lint_names.dart'; +import 'package:test/test.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import 'abstract_rename.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(RenameNamedParameterTest); + defineReflectiveTests(RenamePositionalParameterTest); + }); +} + +@reflectiveTest +class RenameNamedParameterTest extends RenameRefactoringTest { + Future test_checkFinalConditions_shadows_classMember() async { + await indexTestUnit(''' +class A { + foo({test = 1}) { // in A + } +} +class B extends A { + var newName = 1; + foo({test = 1}) { + print(newName); + } +} +'''); + createRenameRefactoringAtString('test = 1}) { // in A'); + // check status + refactoring.newName = 'newName'; + var status = await refactoring.checkFinalConditions(); + assertRefactoringStatus( + status, + RefactoringProblemSeverity.ERROR, + expectedMessage: + 'Usage of field "B.newName" declared in "test.dart" ' + 'will be shadowed by renamed parameter.', + expectedContextSearch: 'newName);', + ); + } + + Future test_createChange() async { + await indexTestUnit(''' +myFunction({required int test}) { + test = 1; + test += 2; + print(test); +} +void f() { + myFunction(test: 2); +} +'''); + // configure refactoring + createRenameRefactoringAtString('test}) {'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + refactoring.newName = 'newName'; + // validate change + return assertSuccessfulRefactoring(''' +myFunction({required int newName}) { + newName = 1; + newName += 2; + print(newName); +} +void f() { + myFunction(newName: 2); +} +'''); + } + + Future test_createChange_anywhere() async { + await indexTestUnit(''' +myFunction(int a, int b, {required int test}) { + test = 1; + test += 2; + print(test); +} +void f() { + myFunction(0, test: 2, 1); +} +'''); + // configure refactoring + createRenameRefactoringAtString('test}) {'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + refactoring.newName = 'newName'; + // validate change + return assertSuccessfulRefactoring(''' +myFunction(int a, int b, {required int newName}) { + newName = 1; + newName += 2; + print(newName); +} +void f() { + myFunction(0, newName: 2, 1); +} +'''); + } + + Future test_createChange_inOtherFile() async { + var a = convertPath('$testPackageLibPath/a.dart'); + var b = convertPath('$testPackageLibPath/b.dart'); + + newFile(a, r''' +class A { + A({test}); +} +'''); + newFile(b, r''' +import 'a.dart'; + +void f() { + new A(test: 2); +} +'''); + await analyzeTestPackageFiles(); + + testFilePath = a; + await resolveTestFile(); + + createRenameRefactoringAtString('test});'); + expect(refactoring.refactoringName, 'Rename Parameter'); + refactoring.newName = 'newName'; + + await assertSuccessfulRefactoring(''' +class A { + A({newName}); +} +'''); + assertFileChangeResult(b, ''' +import 'a.dart'; + +void f() { + new A(newName: 2); +} +'''); + } + + Future test_createChange_ofConstructor_genericClass() async { + await indexTestUnit(''' +class A { + A({required T test}); +} + +void f() { + A(test: 0); +} +'''); + // configure refactoring + createRenameRefactoringAtString('test}'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + refactoring.newName = 'newName'; + // validate change + return assertSuccessfulRefactoring(''' +class A { + A({required T newName}); +} + +void f() { + A(newName: 0); +} +'''); + } + + Future test_createChange_ofMethod_genericClass() async { + await indexTestUnit(''' +class A { + void foo({required T test}) {} +} + +void f(A a) { + a.foo(test: 0); +} +'''); + // configure refactoring + createRenameRefactoringAtString('test}'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + refactoring.newName = 'newName'; + // validate change + return assertSuccessfulRefactoring(''' +class A { + void foo({required T newName}) {} +} + +void f(A a) { + a.foo(newName: 0); +} +'''); + } + + Future test_createChange_super() async { + await indexTestUnit(''' +class A { + A({required int test}); // 0 +} +class B extends A { + B({required super.test}); +} +'''); + // configure refactoring + createRenameRefactoringAtString('test}); // 0'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + refactoring.newName = 'newName'; + // validate change + return assertSuccessfulRefactoring(''' +class A { + A({required int newName}); // 0 +} +class B extends A { + B({required super.newName}); +} +'''); + } + + Future test_createChange_updateHierarchy() async { + await indexUnit('$testPackageLibPath/test2.dart', ''' +library test2; +class A { + void foo({int? test}) { + print(test); + } +} +class B extends A { + void foo({int? test}) { + print(test); + } +} +'''); + await indexTestUnit(''' +import 'test2.dart'; +void f() { + new A().foo(test: 10); + new B().foo(test: 20); + new C().foo(test: 30); +} +class C extends A { + void foo({int? test}) { + print(test); + } +} +'''); + // configure refactoring + createRenameRefactoringAtString('test: 20'); + expect(refactoring.refactoringName, 'Rename Parameter'); + refactoring.newName = 'newName'; + // validate change + await assertSuccessfulRefactoring(''' +import 'test2.dart'; +void f() { + new A().foo(newName: 10); + new B().foo(newName: 20); + new C().foo(newName: 30); +} +class C extends A { + void foo({int? newName}) { + print(newName); + } +} +'''); + assertFileChangeResult('$testPackageLibPath/test2.dart', ''' +library test2; +class A { + void foo({int? newName}) { + print(newName); + } +} +class B extends A { + void foo({int? newName}) { + print(newName); + } +} +'''); + } +} + +@reflectiveTest +class RenamePositionalParameterTest extends RenameRefactoringTest { + Future test_checkNewName() async { + await indexTestUnit(''' +void f(test) { +} +'''); + createRenameRefactoringAtString('test) {'); + // empty + refactoring.newName = ''; + assertRefactoringStatus( + refactoring.checkNewName(), + RefactoringProblemSeverity.FATAL, + expectedMessage: 'Parameter name must not be empty.', + ); + // OK + refactoring.newName = 'newName'; + assertRefactoringStatusOK(refactoring.checkNewName()); + } + + Future test_createChange_closure() async { + await indexTestUnit(''' +void f(void Function(int) _) {} + +void g() => f((parameter) { + print(parameter); +}); +'''); + // configure refactoring + createRenameRefactoringAtString('parameter) {'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + refactoring.newName = 'newName'; + // validate change + return assertSuccessfulRefactoring(''' +void f(void Function(int) _) {} + +void g() => f((newName) { + print(newName); +}); +'''); + } + + Future test_createChange_optional() async { + await indexTestUnit(''' +myFunction([int? test]) { + test = 1; + test += 2; + print(test); +} +void f() { + myFunction(2); +} +'''); + // configure refactoring + createRenameRefactoringAtString('test]) {'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + refactoring.newName = 'newName'; + // validate change + return assertSuccessfulRefactoring(''' +myFunction([int? newName]) { + newName = 1; + newName += 2; + print(newName); +} +void f() { + myFunction(2); +} +'''); + } + + Future test_createChange_super() async { + await indexTestUnit(''' +class A { + A(int test); // 0 +} +class B extends A { + B(super.test); +} +'''); + + createRenameRefactoringAtString('test); // 0'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + refactoring.newName = 'newName'; + + // The name of the super-formal parameter does not have to be the same. + // So, we don't rename it. + return assertSuccessfulRefactoring(''' +class A { + A(int newName); // 0 +} +class B extends A { + B(super.test); +} +'''); + } + + Future test_function_shadow() async { + await indexTestUnit(''' +void function(int a) { + int b = 0; +} +'''); + createRenameRefactoringAtString('a) {'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + expect(refactoring.oldName, 'a'); + refactoring.newName = 'b'; + var status = await refactoring.checkFinalConditions(); + return assertRefactoringStatus( + status, + RefactoringProblemSeverity.ERROR, + expectedMessage: + "Duplicate local variable of name 'b' at function in " + "'test.dart'.", + ); + } + + Future test_hierarchy_override_lint_shadow_double() async { + createAnalysisOptionsFile( + lints: [LintNames.avoid_renaming_method_parameters], + ); + await indexTestUnit(''' +class C { + void m(int? a) { + int b = 0; + } +} + +class D extends C { + @override + void m(int? a) {} // marker +} + +class E extends C { + @override + void m(int? a) { + int b = 0; + } +} +'''); + createRenameRefactoringAtString('a) {} // marker'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + expect(refactoring.oldName, 'a'); + refactoring.newName = 'b'; + var status = await refactoring.checkFinalConditions(); + return assertRefactoringStatus( + status, + RefactoringProblemSeverity.ERROR, + expectedMessage: + 'This will also rename all related positional parameters to the same ' + "name.\nDuplicate local variable of name 'b' at E.m in 'test.dart'. " + 'And 1 more errors.', + ); + } + + Future test_subclass_override() async { + await indexTestUnit(''' +class C { + void m(int? a) {} +} +class D extends C { + @override + void m(int? a) {} // marker +} +'''); + createRenameRefactoringAtString('a) {} // marker'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + expect(refactoring.oldName, 'a'); + refactoring.newName = 'b'; + return assertSuccessfulRefactoring(''' +class C { + void m(int? a) {} +} +class D extends C { + @override + void m(int? b) {} // marker +} +'''); + } + + Future test_subclass_override_lint() async { + createAnalysisOptionsFile( + lints: [LintNames.avoid_renaming_method_parameters], + ); + await indexTestUnit(''' +class C { + void m(int? a) {} +} +class D extends C { + @override + void m(int? a) {} // marker +} +'''); + createRenameRefactoringAtString('a) {} // marker'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + expect(refactoring.oldName, 'a'); + refactoring.newName = 'b'; + var status = await refactoring.checkFinalConditions(); + assertRefactoringStatus( + status, + RefactoringProblemSeverity.WARNING, + expectedMessage: + 'This will also rename all related positional ' + 'parameters to the same name.', + ); + refactoringChange = await refactoring.createChange(); + assertTestChangeResult(''' +class C { + void m(int? b) {} +} +class D extends C { + @override + void m(int? b) {} // marker +} +'''); + } + + Future test_subclass_override_lint_shadow() async { + createAnalysisOptionsFile( + lints: [LintNames.avoid_renaming_method_parameters], + ); + await indexTestUnit(''' +class C { + void m(int? a) { + int b = 0; + } +} +class D extends C { + @override + void m(int? a) {} // marker +} +'''); + createRenameRefactoringAtString('a) {} // marker'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + expect(refactoring.oldName, 'a'); + refactoring.newName = 'b'; + var status = await refactoring.checkFinalConditions(); + return assertRefactoringStatus( + status, + RefactoringProblemSeverity.ERROR, + expectedMessage: + 'This will also rename all related positional parameters to the same ' + "name.\nDuplicate local variable of name 'b' at C.m in 'test.dart'.", + ); + } + + Future test_superclass_override() async { + await indexTestUnit(''' +class C { + void m(int? a) {} // marker +} +class D extends C { + @override + void m(int? a) {} +} +'''); + createRenameRefactoringAtString('a) {} // marker'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + expect(refactoring.oldName, 'a'); + refactoring.newName = 'b'; + return assertSuccessfulRefactoring(''' +class C { + void m(int? b) {} // marker +} +class D extends C { + @override + void m(int? a) {} +} +'''); + } + + Future test_superclass_override_lint() async { + createAnalysisOptionsFile( + lints: [LintNames.avoid_renaming_method_parameters], + ); + await indexTestUnit(''' +class C { + void m(int? a) {} // marker +} +class D extends C { + @override + void m(int? a) {} +} +'''); + createRenameRefactoringAtString('a) {} // marker'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + expect(refactoring.oldName, 'a'); + refactoring.newName = 'b'; + var status = await refactoring.checkFinalConditions(); + assertRefactoringStatus( + status, + RefactoringProblemSeverity.WARNING, + expectedMessage: + 'This will also rename all related positional parameters to the same ' + 'name.', + ); + refactoringChange = await refactoring.createChange(); + assertTestChangeResult(''' +class C { + void m(int? b) {} // marker +} +class D extends C { + @override + void m(int? b) {} +} +'''); + } + + Future test_superclass_override_lint_shadow() async { + createAnalysisOptionsFile( + lints: [LintNames.avoid_renaming_method_parameters], + ); + await indexTestUnit(''' +class C { + void m(int? a) {} // marker +} +class D extends C { + @override + void m(int? a) { + int b = 0; + } +} +'''); + createRenameRefactoringAtString('a) {} // marker'); + expect(refactoring.refactoringName, 'Rename Parameter'); + expect(refactoring.elementKindName, 'parameter'); + expect(refactoring.oldName, 'a'); + refactoring.newName = 'b'; + var status = await refactoring.checkFinalConditions(); + return assertRefactoringStatus( + status, + RefactoringProblemSeverity.ERROR, + expectedMessage: + 'This will also rename all related positional parameters to the same ' + "name.\nDuplicate local variable of name 'b' at D.m in 'test.dart'.", + ); + } +} diff --git a/pkg/analysis_server/test/services/refactoring/legacy/test_all.dart b/pkg/analysis_server/test/services/refactoring/legacy/test_all.dart index 4b8d30fb029..8d3e1901253 100644 --- a/pkg/analysis_server/test/services/refactoring/legacy/test_all.dart +++ b/pkg/analysis_server/test/services/refactoring/legacy/test_all.dart @@ -20,6 +20,7 @@ import 'rename_import_test.dart' as rename_import_test; import 'rename_label_test.dart' as rename_label_test; import 'rename_library_test.dart' as rename_library_test; import 'rename_local_test.dart' as rename_local_test; +import 'rename_parameter_test.dart' as rename_parameter; import 'rename_type_parameter_test.dart' as rename_type_parameter; import 'rename_unit_member_test.dart' as rename_unit_member_test; @@ -41,6 +42,7 @@ void main() { rename_label_test.main(); rename_library_test.main(); rename_local_test.main(); + rename_parameter.main(); rename_type_parameter.main(); rename_unit_member_test.main(); }, name: 'legacy'); diff --git a/pkg/analyzer/api.txt b/pkg/analyzer/api.txt index 73acf72bf5f..2c702c5993d 100644 --- a/pkg/analyzer/api.txt +++ b/pkg/analyzer/api.txt @@ -54,6 +54,7 @@ package:analyzer/dart/analysis/code_style_options.dart: CodeStyleOptions (class extends Object): new (constructor: CodeStyleOptions Function()) addTrailingCommas (getter: bool) + avoidRenamingMethodParameters (getter: bool) finalInForEach (getter: bool) makeLocalsFinal (getter: bool) preferConstDeclarations (getter: bool) diff --git a/pkg/analyzer/lib/dart/analysis/code_style_options.dart b/pkg/analyzer/lib/dart/analysis/code_style_options.dart index bae080839fc..cc500e8bc41 100644 --- a/pkg/analyzer/lib/dart/analysis/code_style_options.dart +++ b/pkg/analyzer/lib/dart/analysis/code_style_options.dart @@ -13,6 +13,11 @@ abstract class CodeStyleOptions { /// should be inserted in function calls and declarations. bool get addTrailingCommas; + /// Whether the `avoid_renaming_method_parameters` is enabled and method + /// parameters should not be renamed separately from the other + /// implementations. + bool get avoidRenamingMethodParameters; + /// Whether local variables should be `final` inside a for-loop. bool get finalInForEach; diff --git a/pkg/analyzer/lib/src/analysis_options/code_style_options.dart b/pkg/analyzer/lib/src/analysis_options/code_style_options.dart index 95fecedc655..adfdebcc3c6 100644 --- a/pkg/analyzer/lib/src/analysis_options/code_style_options.dart +++ b/pkg/analyzer/lib/src/analysis_options/code_style_options.dart @@ -19,6 +19,10 @@ class CodeStyleOptionsImpl implements CodeStyleOptions { @override bool get addTrailingCommas => _isLintEnabled('require_trailing_commas'); + @override + bool get avoidRenamingMethodParameters => + _isLintEnabled('avoid_renaming_method_parameters'); + @override bool get finalInForEach => _isLintEnabled('prefer_final_in_for_each');