Issue 52131. Find implicit constructor invocations from other constructors.

Bug: https://github.com/dart-lang/sdk/issues/52131
Change-Id: I7f28e4664f18196f978cb9dd38f878b1240d46a1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/297080
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Konstantin Shcheglov 2023-04-21 22:04:16 +00:00 committed by Commit Queue
parent 29472dbb73
commit 529adda42e
4 changed files with 217 additions and 0 deletions

View file

@ -10,6 +10,7 @@ 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/refactoring/legacy/rename.dart';
import 'package:analysis_server/src/services/search/hierarchy.dart';
import 'package:analysis_server/src/utilities/selection.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/generated/java_core.dart';
@ -50,6 +51,24 @@ class RenameConstructorRefactoringImpl extends RenameRefactoringImpl {
var references = getSourceReferences(matches);
// update references
for (var reference in references) {
// Handle implicit references.
var coveringNode = await _nodeCoveringReference(reference);
var coveringParent = coveringNode?.parent;
if (coveringNode is ClassDeclaration) {
_addDefaultConstructorToClass(
reference: reference,
classDeclaration: coveringNode,
);
continue;
} else if (coveringParent is ConstructorDeclaration &&
coveringParent.returnType.offset == reference.range.offset) {
_addSuperInvocationToConstructor(
reference: reference,
constructor: coveringParent,
);
continue;
}
String replacement;
if (newName.isNotEmpty) {
replacement = '.$newName';
@ -76,6 +95,38 @@ class RenameConstructorRefactoringImpl extends RenameRefactoringImpl {
}
}
void _addDefaultConstructorToClass({
required SourceReference reference,
required ClassDeclaration classDeclaration,
}) {
final className = classDeclaration.name.lexeme;
_replaceInReferenceFile(
reference: reference,
range: range.endLength(classDeclaration.leftBracket, 0),
replacement: '\n $className() : super.$newName();',
);
}
void _addSuperInvocationToConstructor({
required SourceReference reference,
required ConstructorDeclaration constructor,
}) {
final initializers = constructor.initializers;
if (initializers.lastOrNull case final last?) {
_replaceInReferenceFile(
reference: reference,
range: range.endLength(last, 0),
replacement: ', super.$newName()',
);
} else {
_replaceInReferenceFile(
reference: reference,
range: range.endLength(constructor.parameters, 0),
replacement: ' : super.$newName()',
);
}
}
void _analyzePossibleConflicts(RefactoringStatus result) {
var parentClass = element.enclosingElement;
// Check if the "newName" is the name of the enclosing class.
@ -101,6 +152,26 @@ class RenameConstructorRefactoringImpl extends RenameRefactoringImpl {
}
}
Future<AstNode?> _nodeCoveringReference(SourceReference reference) async {
var element = reference.element;
var unitResult = await sessionHelper.getResolvedUnitByElement(element);
return unitResult?.unit
.select(offset: reference.range.offset, length: 0)
?.coveringNode;
}
void _replaceInReferenceFile({
required SourceReference reference,
required SourceRange range,
required String replacement,
}) {
doSourceChange_addElementEdit(
change,
reference.element,
newSourceEdit_range(range, replacement),
);
}
Future<void> _replaceSynthetic() async {
var classElement = element.enclosingElement;

View file

@ -210,6 +210,95 @@ void f() {
''');
}
Future<void>
test_createChange_implicitlyInvoked_hasConstructor_hasInitializers() async {
await indexTestUnit('''
class A {
A();
}
class B extends A {
var field;
B() : field = 0;
}
''');
// configure refactoring
_createConstructorDeclarationRefactoring('A();');
expect(refactoring.refactoringName, 'Rename Constructor');
expect(refactoring.elementKindName, 'constructor');
expect(refactoring.oldName, '');
// validate change
refactoring.newName = 'newName';
return assertSuccessfulRefactoring('''
class A {
A.newName();
}
class B extends A {
var field;
B() : field = 0, super.newName();
}
''');
}
Future<void>
test_createChange_implicitlyInvoked_hasConstructor_noInitializers() async {
await indexTestUnit('''
class A {
A();
}
class B extends A {
B();
}
''');
// configure refactoring
_createConstructorDeclarationRefactoring('A();');
expect(refactoring.refactoringName, 'Rename Constructor');
expect(refactoring.elementKindName, 'constructor');
expect(refactoring.oldName, '');
// validate change
refactoring.newName = 'newName';
return assertSuccessfulRefactoring('''
class A {
A.newName();
}
class B extends A {
B() : super.newName();
}
''');
}
Future<void> test_createChange_implicitlyInvoked_noConstructor() async {
await indexTestUnit('''
class A {
A();
}
class B extends A {
void foo() {}
}
''');
// configure refactoring
_createConstructorDeclarationRefactoring('A();');
expect(refactoring.refactoringName, 'Rename Constructor');
expect(refactoring.elementKindName, 'constructor');
expect(refactoring.oldName, '');
// validate change
refactoring.newName = 'newName';
return assertSuccessfulRefactoring('''
class A {
A.newName();
}
class B extends A {
B() : super.newName();
void foo() {}
}
''');
}
Future<void> test_createChange_lint_sortConstructorsFirst() async {
createAnalysisOptionsFile(lints: [LintNames.sort_constructors_first]);
await indexTestUnit('''

View file

@ -589,6 +589,21 @@ class _IndexContributor extends GeneralizingAstVisitor {
node.name.offset, 0, true);
}
recordIsAncestorOf(declaredElement);
// If the class has only a synthetic default constructor, then it
// implicitly invokes the default super constructor. Associate the
// invocation with the name of the class.
final defaultConstructor = declaredElement.constructors.singleOrNull;
if (defaultConstructor is ConstructorElementImpl &&
defaultConstructor.isSynthetic) {
defaultConstructor.isDefaultConstructor;
final superConstructor = defaultConstructor.superConstructor;
if (superConstructor != null) {
recordRelation(
superConstructor, IndexRelationKind.IS_INVOKED_BY, node.name, true);
}
}
super.visitClassDeclaration(node);
}
@ -629,6 +644,24 @@ class _IndexContributor extends GeneralizingAstVisitor {
return super.visitCommentReference(node);
}
@override
visitConstructorDeclaration(ConstructorDeclaration node) {
// If the constructor does not have an explicit `super` constructor
// invocation, it implicitly invokes the unnamed constructor.
if (node.initializers.none((e) => e is SuperConstructorInvocation)) {
final element = node.declaredElement as ConstructorElementImpl;
final superConstructor = element.superConstructor;
if (superConstructor != null) {
final offset = node.returnType.offset;
final end = (node.name ?? node.returnType).end;
recordRelationOffset(superConstructor, IndexRelationKind.IS_INVOKED_BY,
offset, end - offset, true);
}
}
super.visitConstructorDeclaration(node);
}
@override
void visitConstructorFieldInitializer(ConstructorFieldInitializer node) {
var fieldName = node.fieldName;

View file

@ -45,6 +45,30 @@ class IndexTest extends PubPackageResolutionTest with _IndexMixin {
expect(actual, expected);
}
test_class_constructorElement_unnamed_implicitInvocation() async {
await _indexTestUnit('''
class A {
A();
}
class B extends A {
B();
B.named();
factory B.foo() = A;
}
class C extends A {}
''');
final element = findElement.unnamedConstructor('A');
assertElementIndexText(element, r'''
42 6:3 |B| IS_INVOKED_BY qualified
49 7:3 |B.named| IS_INVOKED_BY qualified
81 8:22 || IS_REFERENCED_BY qualified
92 11:7 |C| IS_INVOKED_BY qualified
''');
}
test_fieldFormalParameter_noSuchField() async {
await _indexTestUnit('''
class B<T> {