Skip to content

Commit

Permalink
[DAS] Fixes renaming closure parameter
Browse files Browse the repository at this point in the history
R=scheglov@google.com

Fixes: #60047
Fixes: #60046

Change-Id: I24b62213c6ef81e76603e7bffabe463c9ca96dcf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/408140
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Auto-Submit: Felipe Morschel <git@fmorschel.dev>
  • Loading branch information
FMorschel authored and Commit Queue committed Feb 12, 2025
1 parent 33da540 commit 1a0b1fe
Show file tree
Hide file tree
Showing 32 changed files with 787 additions and 620 deletions.
25 changes: 25 additions & 0 deletions pkg/analysis_server/test/lsp/definition_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,31 @@ class A {
await testContents(contents);
}

Future<void> test_closure_parameter() async {
setLocationLinkSupport();

var code = TestCode.parse('''
void f(void Function(int) _) {}
void g() => f((/*[0*/variable/*0]*/) {
print(/*[1*/^variable/*1]*/);
});
''');

await initialize();
await openFile(mainFileUri, code.code);
var res = await getDefinitionAsLocationLinks(
mainFileUri,
code.position.position,
);

expect(res, hasLength(1));
var loc = res.first;
expect(loc.originSelectionRange, equals(code.ranges.last.range));
expect(loc.targetRange, equals(code.ranges.first.range));
expect(loc.targetSelectionRange, equals(code.ranges.first.range));
}

Future<void> test_comment_adjacentReference() async {
/// Computing Dart navigation locates a node at the provided offset then
/// returns all navigation regions inside it. This test ensures we filter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,29 @@ void f(test) {
assertRefactoringStatusOK(refactoring.checkNewName());
}

Future<void> 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<void> test_createChange_localFunction() async {
await indexTestUnit('''
void f() {
Expand Down
42 changes: 33 additions & 9 deletions pkg/analyzer/lib/src/dart/element/element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,8 @@ class ClassElementImpl extends ClassOrMixinElementImpl
implicitParameter = DefaultParameterElementImpl(
name: superParameter.name,
nameOffset: -1,
name2: superParameter.name.nullIfEmpty,
nameOffset2: null,
// ignore: deprecated_member_use_from_same_package
parameterKind: superParameter.parameterKind,
)..constantInitializer = constVariable.constantInitializer;
Expand All @@ -592,6 +594,8 @@ class ClassElementImpl extends ClassOrMixinElementImpl
implicitParameter = ParameterElementImpl(
name: superParameter.name,
nameOffset: -1,
name2: superParameter.name.nullIfEmpty,
nameOffset2: null,
// ignore: deprecated_member_use_from_same_package
parameterKind: superParameter.parameterKind,
);
Expand Down Expand Up @@ -1844,6 +1848,8 @@ class DefaultFieldFormalParameterElementImpl
DefaultFieldFormalParameterElementImpl({
required super.name,
required super.nameOffset,
required super.name2,
required super.nameOffset2,
required super.parameterKind,
});

Expand All @@ -1861,6 +1867,8 @@ class DefaultParameterElementImpl extends ParameterElementImpl
DefaultParameterElementImpl({
required super.name,
required super.nameOffset,
required super.name2,
required super.nameOffset2,
required super.parameterKind,
});

Expand All @@ -1877,6 +1885,8 @@ class DefaultSuperFormalParameterElementImpl
DefaultSuperFormalParameterElementImpl({
required super.name,
required super.nameOffset,
required super.name2,
required super.nameOffset2,
required super.parameterKind,
});

Expand Down Expand Up @@ -4491,8 +4501,10 @@ class FieldFormalParameterElementImpl extends ParameterElementImpl
/// Initialize a newly created parameter element to have the given [name] and
/// [nameOffset].
FieldFormalParameterElementImpl({
required String super.name,
required super.name,
required super.nameOffset,
required super.name2,
required super.nameOffset2,
required super.parameterKind,
});

Expand Down Expand Up @@ -9358,7 +9370,7 @@ abstract class NonParameterVariableElementImpl extends VariableElementImpl
with _HasLibraryMixin {
/// Initialize a newly created variable element to have the given [name] and
/// [offset].
NonParameterVariableElementImpl(String super.name, super.offset);
NonParameterVariableElementImpl(super.name, super.offset);

@override
Element get enclosingElement3 =>
Expand Down Expand Up @@ -9414,17 +9426,23 @@ class ParameterElementImpl extends VariableElementImpl
/// Initialize a newly created parameter element to have the given [name] and
/// [nameOffset].
ParameterElementImpl({
required String? name,
required String name,
required int nameOffset,
required this.name2,
required this.nameOffset2,
required this.parameterKind,
}) : super(name, nameOffset);
}) : assert(nameOffset2 == null || nameOffset2 >= 0),
assert(name2 == null || name2.isNotEmpty),
super(name, nameOffset);

/// Creates a synthetic parameter with [name], [type] and [parameterKind].
/// Creates a synthetic parameter with [name2], [type] and [parameterKind].
factory ParameterElementImpl.synthetic(
String? name, DartType type, ParameterKind parameterKind) {
String? name2, DartType type, ParameterKind parameterKind) {
var element = ParameterElementImpl(
name: name,
name: name2 ?? '',
nameOffset: -1,
name2: name2,
nameOffset2: null,
parameterKind: parameterKind,
);
// TODO(paulberry): remove this cast by changing the type of the `type`
Expand Down Expand Up @@ -9567,6 +9585,10 @@ class ParameterElementImpl_ofImplicitSetter extends ParameterElementImpl {
: super(
name: considerCanonicalizeString('_${setter.variable2.name}'),
nameOffset: -1,
name2: setter.variable2.name == ''
? null
: considerCanonicalizeString('_${setter.variable2.name}'),
nameOffset2: null,
parameterKind: ParameterKind.REQUIRED,
) {
enclosingElement3 = setter;
Expand Down Expand Up @@ -10712,8 +10734,10 @@ class SuperFormalParameterElementImpl extends ParameterElementImpl
/// Initialize a newly created parameter element to have the given [name] and
/// [nameOffset].
SuperFormalParameterElementImpl({
required String super.name,
required super.name,
required super.nameOffset,
required super.name2,
required super.nameOffset2,
required super.parameterKind,
});

Expand Down Expand Up @@ -11741,7 +11765,7 @@ abstract class VariableElementImpl extends ElementImpl

/// Initialize a newly created variable element to have the given [name] and
/// [offset].
VariableElementImpl(super.name, super.offset);
VariableElementImpl(String super.name, super.offset);

/// If this element represents a constant variable, and it has an initializer,
/// a copy of the initializer for the constant. Otherwise `null`.
Expand Down
3 changes: 2 additions & 1 deletion pkg/analyzer/lib/src/dart/element/extensions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/src/dart/element/element.dart';
import 'package:analyzer/src/dart/element/type.dart';
import 'package:analyzer/src/generated/utilities_dart.dart';
import 'package:analyzer/src/utilities/extensions/string.dart';
import 'package:meta/meta_meta.dart';

extension DartTypeExtension on DartType {
Expand Down Expand Up @@ -249,7 +250,7 @@ extension ParameterElementExtension on ParameterElement {
bool? isCovariant,
}) {
return ParameterElementImpl.synthetic(
name,
name.nullIfEmpty,
type ?? this.type,
// ignore: deprecated_member_use_from_same_package
kind ?? parameterKind,
Expand Down
33 changes: 25 additions & 8 deletions pkg/analyzer/lib/src/dart/resolver/resolution_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import 'package:analyzer/src/generated/element_walker.dart';
import 'package:analyzer/src/generated/utilities_dart.dart';
import 'package:analyzer/src/utilities/extensions/collection.dart';
import 'package:analyzer/src/utilities/extensions/element.dart';
import 'package:analyzer/src/utilities/extensions/string.dart';

class ElementHolder {
final ElementImpl _element;
Expand Down Expand Up @@ -446,27 +447,33 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
if (_elementWalker != null) {
element = _elementWalker!.getParameter();
} else {
var name = nameToken?.lexeme ?? '';
var nameOffset = nameToken?.offset ?? -1;
var name2 = nameToken?.lexeme.nullIfEmpty;
var nameOffset2 = nameToken?.offset;
if (node.parameter is FieldFormalParameter) {
// Only for recovery, this should not happen in valid code.
element = DefaultFieldFormalParameterElementImpl(
name: name,
nameOffset: nameOffset,
name: name2 ?? '',
nameOffset: nameOffset2 ?? -1,
parameterKind: node.kind,
name2: name2,
nameOffset2: nameOffset2,
)..constantInitializer = node.defaultValue;
} else if (node.parameter is SuperFormalParameter) {
// Only for recovery, this should not happen in valid code.
element = DefaultSuperFormalParameterElementImpl(
name: name,
nameOffset: nameOffset,
name: name2 ?? '',
nameOffset: nameOffset2 ?? -1,
parameterKind: node.kind,
name2: name2,
nameOffset2: nameOffset2,
)..constantInitializer = node.defaultValue;
} else {
element = DefaultParameterElementImpl(
name: name,
nameOffset: nameOffset,
name: name2 ?? '',
nameOffset: nameOffset2 ?? -1,
parameterKind: node.kind,
name2: name2,
nameOffset2: nameOffset2,
)..constantInitializer = node.defaultValue;
}
_elementHolder.addParameter(element);
Expand Down Expand Up @@ -630,6 +637,8 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
element = FieldFormalParameterElementImpl(
name: nameToken.lexeme,
nameOffset: nameToken.offset,
name2: nameToken.lexeme.nullIfEmpty,
nameOffset2: nameToken.offset.nullIfNegative,
parameterKind: node.kind,
);
_elementHolder.enclose(element);
Expand Down Expand Up @@ -838,6 +847,8 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
element = ParameterElementImpl(
name: nameToken.lexeme,
nameOffset: nameToken.offset,
name2: nameToken.lexeme.nullIfEmpty,
nameOffset2: nameToken.offset.nullIfNegative,
parameterKind: node.kind,
);
_elementHolder.addParameter(element);
Expand Down Expand Up @@ -1214,12 +1225,16 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
element = ParameterElementImpl(
name: nameToken.lexeme,
nameOffset: nameToken.offset,
name2: nameToken.lexeme.nullIfEmpty,
nameOffset2: nameToken.offset.nullIfNegative,
parameterKind: node.kind,
);
} else {
element = ParameterElementImpl(
name: '',
nameOffset: -1,
name2: null,
nameOffset2: null,
parameterKind: node.kind,
);
}
Expand Down Expand Up @@ -1270,6 +1285,8 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
element = SuperFormalParameterElementImpl(
name: nameToken.lexeme,
nameOffset: nameToken.offset,
name2: nameToken.lexeme.nullIfEmpty,
nameOffset2: nameToken.offset.nullIfNegative,
parameterKind: node.kind,
);
_elementHolder.enclose(element);
Expand Down
17 changes: 17 additions & 0 deletions pkg/analyzer/lib/src/generated/testing/element_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import 'package:analyzer/src/generated/engine.dart';
import 'package:analyzer/src/generated/source.dart' show NonExistingSource;
import 'package:analyzer/src/generated/utilities_dart.dart';
import 'package:analyzer/src/summary2/reference.dart';
import 'package:analyzer/src/utilities/extensions/string.dart';
import 'package:collection/collection.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart';
Expand Down Expand Up @@ -173,6 +174,8 @@ class ElementFactory {
FieldFormalParameterElementImpl(
name: name.name,
nameOffset: name.offset,
name2: name.name.nullIfEmpty,
nameOffset2: name.offset.nullIfNegative,
parameterKind: ParameterKind.REQUIRED,
);

Expand Down Expand Up @@ -272,6 +275,8 @@ class ElementFactory {
return ParameterElementImpl(
name: name,
nameOffset: 0,
name2: name,
nameOffset2: 0,
parameterKind: ParameterKind.NAMED,
);
}
Expand All @@ -280,6 +285,8 @@ class ElementFactory {
var parameter = ParameterElementImpl(
name: name,
nameOffset: 0,
name2: name,
nameOffset2: 0,
parameterKind: ParameterKind.NAMED,
);
parameter.type = type;
Expand All @@ -290,6 +297,8 @@ class ElementFactory {
return ParameterElementImpl(
name: name,
nameOffset: 0,
name2: name,
nameOffset2: 0,
parameterKind: ParameterKind.POSITIONAL,
);
}
Expand All @@ -298,6 +307,8 @@ class ElementFactory {
var parameter = ParameterElementImpl(
name: name,
nameOffset: 0,
name2: name,
nameOffset2: 0,
parameterKind: ParameterKind.POSITIONAL,
);
parameter.type = type;
Expand All @@ -310,6 +321,8 @@ class ElementFactory {
return ParameterElementImpl(
name: name,
nameOffset: 0,
name2: name,
nameOffset2: 0,
parameterKind: ParameterKind.REQUIRED,
);
}
Expand All @@ -318,6 +331,8 @@ class ElementFactory {
var parameter = ParameterElementImpl(
name: name,
nameOffset: 0,
name2: name,
nameOffset2: 0,
parameterKind: ParameterKind.REQUIRED,
);
parameter.type = type;
Expand Down Expand Up @@ -372,6 +387,8 @@ class ElementFactory {
var parameter = ParameterElementImpl(
name: 'a$index',
nameOffset: index,
name2: 'a$index',
nameOffset2: index,
parameterKind: ParameterKind.REQUIRED,
);
parameter.type = type;
Expand Down
4 changes: 4 additions & 0 deletions pkg/analyzer/lib/src/summary2/ast_binary_reader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,8 @@ class AstBinaryReader {
var element = DefaultParameterElementImpl(
name: nonDefaultElement.name,
nameOffset: nonDefaultElement.nameOffset,
name2: nonDefaultElement.name2,
nameOffset2: nonDefaultElement.nameOffset2,
parameterKind: kind,
);
if (parameter is SimpleFormalParameterImpl) {
Expand Down Expand Up @@ -1219,6 +1221,8 @@ class AstBinaryReader {
var element = ParameterElementImpl(
name: name?.lexeme ?? '',
nameOffset: -1,
name2: name?.lexeme,
nameOffset2: null,
parameterKind: node.kind,
);
element.type = actualType;
Expand Down
Loading

0 comments on commit 1a0b1fe

Please sign in to comment.