From a8a3f3f77e973ae66b4b91069738eff212427d2d Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Thu, 22 Jun 2023 14:38:38 +0000 Subject: [PATCH] [CMSR] Improve 'get invocation' implementation. Change-Id: I871eddc1727ea6a51e169f4031d55ba3bc93527d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310860 Reviewed-by: Brian Wilkerson Commit-Queue: Konstantin Shcheglov --- .../agnostic/change_method_signature.dart | 64 +++++---- .../lib/src/utilities/selection.dart | 9 ++ .../change_method_signature_test.dart | 135 ++++++++++++++++++ .../lib/utilities/range_factory.dart | 6 + 4 files changed, 190 insertions(+), 24 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/refactoring/agnostic/change_method_signature.dart b/pkg/analysis_server/lib/src/services/refactoring/agnostic/change_method_signature.dart index 2be14b25c9a..4dea006621a 100644 --- a/pkg/analysis_server/lib/src/services/refactoring/agnostic/change_method_signature.dart +++ b/pkg/analysis_server/lib/src/services/refactoring/agnostic/change_method_signature.dart @@ -895,7 +895,7 @@ class _SignatureUpdater { } ArgumentList argumentList; - final invocation = selection.coveringNode.invocation; + final invocation = selection.invocation; switch (invocation) { case ConstructorDeclaration constructor: return ChangeStatusFailureSuperFormalParameter( @@ -905,6 +905,8 @@ class _SignatureUpdater { argumentList = instanceCreation.argumentList; case MethodInvocation invocation: argumentList = invocation.argumentList; + case RedirectingConstructorInvocation invocation: + argumentList = invocation.argumentList; case SuperConstructorInvocation invocation: argumentList = invocation.argumentList; default: @@ -1027,15 +1029,7 @@ class _SignatureUpdater { } } -extension on FormalParameterList { - bool get hasTrailingComma { - final last = parameters.lastOrNull; - final nextToken = last?.endToken.next; - return nextToken != null && nextToken.type == TokenType.COMMA; - } -} - -extension on AstNode { +extension _AstNodeExtension on AstNode { AstNode? get declaration { final self = this; if (self is FunctionExpression) { @@ -1076,27 +1070,49 @@ extension on AstNode { } return null; } +} +extension _FormalParameterListExtension on FormalParameterList { + bool get hasTrailingComma { + final last = parameters.lastOrNull; + final nextToken = last?.endToken.next; + return nextToken != null && nextToken.type == TokenType.COMMA; + } +} + +extension _SelectionExtension on Selection { AstNode? get invocation { - AstNode? self = this; - if (self is ArgumentList) { - self = self.parent; - } else if (self is NamedType && self.parent is ConstructorName) { - self = self.parent?.parent; + final node = coveringNode; + switch (node) { + case RedirectingConstructorInvocation(): + case SuperConstructorInvocation(): + return node; } - switch (self) { - case SimpleIdentifier(): - final parent = self.parent; - if (parent is ConstructorDeclaration) { - return parent; - } else if (parent is MethodInvocation) { + + final parent = node.parent; + switch (parent) { + case MethodInvocation(): + if (isCoveredByNode(parent.methodName)) { + return parent; + } + case RedirectingConstructorInvocation(): + if (isCoveredByToken(parent.thisKeyword)) { return parent; } - case InstanceCreationExpression(): - case MethodInvocation(): case SuperConstructorInvocation(): - return self; + if (isCoveredByToken(parent.superKeyword)) { + return parent; + } } + + final parent2 = parent?.parent; + switch (parent2) { + case InstanceCreationExpression(): + if (isCoveredByNode(parent2.constructorName)) { + return parent2; + } + } + return null; } } diff --git a/pkg/analysis_server/lib/src/utilities/selection.dart b/pkg/analysis_server/lib/src/utilities/selection.dart index 7bb11e00bc8..e0c75bcd16a 100644 --- a/pkg/analysis_server/lib/src/utilities/selection.dart +++ b/pkg/analysis_server/lib/src/utilities/selection.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/source/source_range.dart'; import 'package:analyzer/src/utilities/extensions/ast.dart'; @@ -25,6 +26,14 @@ class Selection { Selection( {required this.offset, required this.length, required this.coveringNode}); + bool isCoveredByNode(AstNode node) { + return node.offset <= offset && offset + length <= node.end; + } + + bool isCoveredByToken(Token token) { + return token.offset <= offset && offset + length <= token.end; + } + /// Returns the contiguous subset of [coveringNode] children that are at /// least partially covered by the selection. Touching is not enough. /// diff --git a/pkg/analysis_server/test/services/refactoring/agnostic/change_method_signature_test.dart b/pkg/analysis_server/test/services/refactoring/agnostic/change_method_signature_test.dart index 1fe89f6950d..8b3469cf537 100644 --- a/pkg/analysis_server/test/services/refactoring/agnostic/change_method_signature_test.dart +++ b/pkg/analysis_server/test/services/refactoring/agnostic/change_method_signature_test.dart @@ -944,6 +944,68 @@ void f() { '''); } + Future + test_classConstructor_redirectingConstructorInvocation_named() async { + await _analyzeValidSelection(r''' +class A { + final int a; + ^A.named(int a); + A() : this.named(0); +} +'''); + + final signatureUpdate = MethodSignatureUpdate( + formalParameters: [ + FormalParameterUpdate( + id: 0, + kind: FormalParameterKind.requiredNamed, + ), + ], + formalParametersTrailingComma: TrailingComma.ifPresent, + argumentsTrailingComma: ArgumentsTrailingComma.ifPresent, + ); + + await _assertUpdate(signatureUpdate, r''' +>>>>>>> /home/test/lib/test.dart +class A { + final int a; + A.named({required int a}); + A() : this.named(a: 0); +} +'''); + } + + Future + test_classConstructor_redirectingConstructorInvocation_unnamed() async { + await _analyzeValidSelection(r''' +class A { + final int a; + ^A(int a); + A.named() : this(0); +} +'''); + + final signatureUpdate = MethodSignatureUpdate( + formalParameters: [ + FormalParameterUpdate( + id: 0, + kind: FormalParameterKind.requiredNamed, + ), + ], + formalParametersTrailingComma: TrailingComma.ifPresent, + argumentsTrailingComma: ArgumentsTrailingComma.ifPresent, + ); + + await _assertUpdate(signatureUpdate, r''' +>>>>>>> /home/test/lib/test.dart +class A { + final int a; + A({required int a}); + A.named() : this(a: 0); +} +'''); + } + Future test_classConstructor_requiredNamed_reorder() async { await _analyzeValidSelection(r''' class A { @@ -1158,6 +1220,79 @@ void f() { '''); } + Future test_classConstructor_superConstructorInvocation_named() async { + await _analyzeValidSelection(r''' +class A { + final int a; + ^A.named(int a); +} + +class B extends A { + B() : super.named(0); +} +'''); + + final signatureUpdate = MethodSignatureUpdate( + formalParameters: [ + FormalParameterUpdate( + id: 0, + kind: FormalParameterKind.requiredNamed, + ), + ], + formalParametersTrailingComma: TrailingComma.ifPresent, + argumentsTrailingComma: ArgumentsTrailingComma.ifPresent, + ); + + await _assertUpdate(signatureUpdate, r''' +>>>>>>> /home/test/lib/test.dart +class A { + final int a; + A.named({required int a}); +} + +class B extends A { + B() : super.named(a: 0); +} +'''); + } + + Future + test_classConstructor_superConstructorInvocation_unnamed() async { + await _analyzeValidSelection(r''' +class A { + final int a; + ^A(int a); +} + +class B extends A { + B() : super(0); +} +'''); + + final signatureUpdate = MethodSignatureUpdate( + formalParameters: [ + FormalParameterUpdate( + id: 0, + kind: FormalParameterKind.requiredNamed, + ), + ], + formalParametersTrailingComma: TrailingComma.ifPresent, + argumentsTrailingComma: ArgumentsTrailingComma.ifPresent, + ); + + await _assertUpdate(signatureUpdate, r''' +>>>>>>> /home/test/lib/test.dart +class A { + final int a; + A({required int a}); +} + +class B extends A { + B() : super(a: 0); +} +'''); + } + Future test_classMethod_optionalNamed_reorder_less() async { await _analyzeValidSelection(r''' class A { diff --git a/pkg/analyzer_plugin/lib/utilities/range_factory.dart b/pkg/analyzer_plugin/lib/utilities/range_factory.dart index 6ed5dc99e84..e61c04ddd44 100644 --- a/pkg/analyzer_plugin/lib/utilities/range_factory.dart +++ b/pkg/analyzer_plugin/lib/utilities/range_factory.dart @@ -198,6 +198,12 @@ class RangeFactory { return SourceRange(startOffset, length); } + /// Return a source range that starts at the given [startOffset], and has + /// the given [length]. + SourceRange startOffsetLength(int startOffset, int length) { + return SourceRange(startOffset, length); + } + /// Return a source range that starts at the start of [leftEntity] and ends at /// the start of [rightEntity]. SourceRange startStart(