Skip to content

Commit 4b6664f

Browse files
scheglovCommit Queue
authored andcommitted
Issue 62382. Fixes for reporting member conflicts in extension types.
Bug: #62382 Change-Id: I7989b4172d135cea06c0d52aca58162ffc03cdcc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/475540 Reviewed-by: Paul Berry <paulberry@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
1 parent d0a10e3 commit 4b6664f

7 files changed

Lines changed: 480 additions & 30 deletions

File tree

pkg/analyzer/api.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4007,6 +4007,7 @@ package:analyzer/dart/element/element.dart:
40074007
forSetter (getter: Name)
40084008
hashCode (getter: int)
40094009
isPublic (getter: bool)
4010+
isSetter (getter: bool)
40104011
libraryUri (getter: Uri?)
40114012
name (getter: String)
40124013
== (method: bool Function(Object))

pkg/analyzer/lib/src/dart/element/extensions.dart

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'package:analyzer/source/source.dart';
1010
import 'package:analyzer/source/source_range.dart';
1111
import 'package:analyzer/src/dart/element/element.dart';
1212
import 'package:analyzer/src/dart/element/type.dart';
13+
import 'package:analyzer/src/diagnostic/diagnostic_message.dart';
1314
import 'package:analyzer/src/generated/utilities_dart.dart';
1415
import 'package:meta/meta_meta.dart';
1516

@@ -202,6 +203,24 @@ extension InterfaceTypeExtension on InterfaceType {
202203
}
203204
}
204205

206+
extension InternalExecutableElementExtension on InternalExecutableElement {
207+
/// Return a diagnostic message pointing at the first fragment.
208+
DiagnosticMessageImpl diagnosticMessage({
209+
required String message,
210+
String? url,
211+
}) {
212+
var baseElement = nonSynthetic.baseElement as ElementImpl;
213+
var location = baseElement.firstFragmentLocation;
214+
return DiagnosticMessageImpl(
215+
filePath: location.libraryFragment!.source.fullName,
216+
message: message,
217+
offset: location.nameOffset ?? -1,
218+
length: location.name?.length ?? 0,
219+
url: url,
220+
);
221+
}
222+
}
223+
205224
extension LibraryExtension2 on LibraryElement? {
206225
bool get hasWildcardVariablesFeatureEnabled =>
207226
this?.featureSet.isEnabled(Feature.wildcard_variables) ?? false;

pkg/analyzer/lib/src/dart/element/inheritance_manager3.dart

Lines changed: 115 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,33 @@ class Conflict {
3030
Conflict({required this.name});
3131
}
3232

33+
/// The extension type inherits a method and a setter with the same name,
34+
/// and does not preclude them.
35+
class ExtensionTypeConflictingInheritedMethodAndSetterConflict
36+
extends Conflict {
37+
final InternalMethodElement method;
38+
final InternalSetterElement setter;
39+
40+
ExtensionTypeConflictingInheritedMethodAndSetterConflict({
41+
required super.name,
42+
required this.method,
43+
required this.setter,
44+
});
45+
}
46+
47+
/// The extension type has a static member that conflicts with an inherited
48+
/// instance member.
49+
class ExtensionTypeConflictingStaticAndInstanceConflict extends Conflict {
50+
final InternalExecutableElement declared;
51+
final InternalExecutableElement inherited;
52+
53+
ExtensionTypeConflictingStaticAndInstanceConflict({
54+
required super.name,
55+
required this.declared,
56+
required this.inherited,
57+
});
58+
}
59+
3360
/// Failure because of a getter and a method from direct superinterfaces.
3461
class GetterMethodConflict extends Conflict {
3562
final InternalExecutableElement getter;
@@ -840,6 +867,87 @@ class InheritanceManager3 {
840867
implemented[name] = combinedSignature;
841868
}
842869

870+
// Check for conflicting static and instance members.
871+
for (var member in [
872+
...element.methods,
873+
...element.getters,
874+
...element.setters,
875+
]) {
876+
if (!member.isStatic) {
877+
continue;
878+
}
879+
880+
var getterName = Name.forElement(member)?.forGetter;
881+
if (getterName == null) {
882+
continue;
883+
}
884+
885+
if (redeclared[getterName]?.firstOrNull case var inherited?) {
886+
conflicts.add(
887+
ExtensionTypeConflictingStaticAndInstanceConflict(
888+
name: getterName,
889+
declared: member,
890+
inherited: inherited,
891+
),
892+
);
893+
continue;
894+
}
895+
896+
var setterName = getterName.forSetter;
897+
if (redeclared[setterName]?.firstOrNull case var inherited?) {
898+
conflicts.add(
899+
ExtensionTypeConflictingStaticAndInstanceConflict(
900+
name: setterName,
901+
declared: member,
902+
inherited: inherited,
903+
),
904+
);
905+
}
906+
}
907+
908+
// Inherited method and setter with the same name.
909+
for (var redeclaredEntry in redeclared.entries) {
910+
var methodName = redeclaredEntry.key;
911+
if (methodName.isSetter) {
912+
continue;
913+
}
914+
915+
// We are looking for a conflict between an inherited method and an
916+
// inherited setter. If either is precluded by a declaration in the
917+
// extension type, there is no conflict.
918+
var setterName = methodName.forSetter;
919+
if (precludedNames.contains(methodName) ||
920+
precludedMethods.contains(methodName) ||
921+
precludedNames.contains(setterName) ||
922+
precludedSetters.contains(setterName)) {
923+
continue;
924+
}
925+
926+
// Choose any method.
927+
var methodCandidates = redeclaredEntry.value
928+
.whereType<InternalMethodElement>();
929+
var method = methodCandidates.firstOrNull;
930+
if (method == null) {
931+
continue;
932+
}
933+
934+
// Choose any corresponding setter.
935+
var setterCandidates = redeclared[setterName]
936+
?.whereType<InternalSetterElement>();
937+
var setter = setterCandidates?.firstOrNull;
938+
if (setter == null) {
939+
continue;
940+
}
941+
942+
conflicts.add(
943+
ExtensionTypeConflictingInheritedMethodAndSetterConflict(
944+
name: methodName,
945+
method: method,
946+
setter: setter,
947+
),
948+
);
949+
}
950+
843951
var uniqueRedeclared = <Name, List<InternalExecutableElement>>{};
844952
for (var entry in redeclared.entries) {
845953
var name = entry.key;
@@ -1357,7 +1465,7 @@ class Name {
13571465
Name._internal(this.libraryUri, this.name, this.isPublic, this.hashCode);
13581466

13591467
Name get forGetter {
1360-
if (name.endsWith('=')) {
1468+
if (isSetter) {
13611469
var getterName = name.substring(0, name.length - 1);
13621470
return Name(libraryUri, getterName);
13631471
} else {
@@ -1366,13 +1474,18 @@ class Name {
13661474
}
13671475

13681476
Name get forSetter {
1369-
if (name.endsWith('=')) {
1477+
if (isSetter) {
13701478
return this;
13711479
} else {
13721480
return Name(libraryUri, '$name=');
13731481
}
13741482
}
13751483

1484+
bool get isSetter {
1485+
return name.endsWith('=') &&
1486+
!const {'[]=', '==', '<=', '>='}.contains(name);
1487+
}
1488+
13761489
@override
13771490
bool operator ==(Object other) {
13781491
return other is Name &&

pkg/analyzer/lib/src/error/redeclare_verifier.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
import 'package:analyzer/dart/ast/ast.dart';
65
import 'package:analyzer/dart/ast/visitor.dart';
76
import 'package:analyzer/dart/element/element.dart';
7+
import 'package:analyzer/src/dart/ast/ast.dart';
8+
import 'package:analyzer/src/dart/element/element.dart';
89
import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag;
910
import 'package:analyzer/src/error/listener.dart';
1011

@@ -16,12 +17,14 @@ class RedeclareVerifier extends RecursiveAstVisitor<void> {
1617
final DiagnosticReporter _errorReporter;
1718

1819
/// The current extension type.
19-
InterfaceElement? _currentExtensionType;
20+
ExtensionTypeElementImpl? _currentExtensionType;
2021

2122
RedeclareVerifier(this._errorReporter);
2223

2324
@override
24-
void visitExtensionTypeDeclaration(ExtensionTypeDeclaration node) {
25+
void visitExtensionTypeDeclaration(
26+
covariant ExtensionTypeDeclarationImpl node,
27+
) {
2528
_currentExtensionType = node.declaredFragment!.element;
2629
super.visitExtensionTypeDeclaration(node);
2730
_currentExtensionType = null;

pkg/analyzer/lib/src/generated/error_verifier.dart

Lines changed: 67 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2354,6 +2354,70 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
23542354
}
23552355

23562356
Uri libraryUri = _currentLibrary.uri;
2357+
2358+
if (enclosingClass is ExtensionTypeElementImpl) {
2359+
var conflicts = _inheritanceManager
2360+
.getInterface(enclosingClass)
2361+
.conflicts;
2362+
for (var conflict in conflicts) {
2363+
switch (conflict) {
2364+
case ExtensionTypeConflictingStaticAndInstanceConflict():
2365+
var declared = conflict.declared;
2366+
if (declared.firstFragment.libraryFragment != _currentUnit) {
2367+
continue;
2368+
}
2369+
diagnosticReporter.report(
2370+
diag.conflictingStaticAndInstance
2371+
.withArguments(
2372+
className: enclosingClass.displayName,
2373+
memberName: declared.displayName,
2374+
conflictingClassName:
2375+
conflict.inherited.enclosingElement!.displayName,
2376+
)
2377+
.atSourceRange(declared.diagnosticRange(_currentUnit.source)),
2378+
);
2379+
case ExtensionTypeConflictingInheritedMethodAndSetterConflict():
2380+
if (fragment.isAugmentation) {
2381+
continue;
2382+
}
2383+
var method = conflict.method;
2384+
var setter = conflict.setter;
2385+
diagnosticReporter.report(
2386+
diag.conflictingInheritedMethodAndSetter
2387+
.withArguments(
2388+
enclosingElementKind: enclosingClass.kind.displayName,
2389+
enclosingElementName: enclosingClass.displayName,
2390+
memberName: conflict.name.name,
2391+
)
2392+
.withContextMessages([
2393+
method.diagnosticMessage(
2394+
message: formatList(
2395+
"The method is inherited from the {0} '{1}'.",
2396+
[
2397+
method.enclosingElement!.kind.displayName,
2398+
method.enclosingElement!.name,
2399+
],
2400+
),
2401+
),
2402+
setter.diagnosticMessage(
2403+
message: formatList(
2404+
"The setter is inherited from the {0} '{1}'.",
2405+
[
2406+
setter.enclosingElement.kind.displayName,
2407+
setter.enclosingElement.name,
2408+
],
2409+
),
2410+
),
2411+
])
2412+
.atSourceRange(
2413+
enclosingClass.diagnosticRange(_currentUnit.source),
2414+
),
2415+
);
2416+
}
2417+
}
2418+
return;
2419+
}
2420+
23572421
var conflictingDeclaredNames = <String>{};
23582422

23592423
// method declared in the enclosing class vs. inherited getter/setter
@@ -2400,11 +2464,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
24002464
}
24012465
}
24022466

2403-
// Extension type methods preclude accessors.
2404-
if (enclosingClass is ExtensionTypeElementImpl) {
2405-
continue;
2406-
}
2407-
24082467
void reportFieldConflict(InternalPropertyAccessorElement inherited) {
24092468
diagnosticReporter.report(
24102469
diag.conflictingMethodAndField
@@ -2458,10 +2517,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
24582517
);
24592518
conflictingDeclaredNames.add(name);
24602519
} else if (inherited is InternalMethodElement) {
2461-
// Extension type accessors preclude inherited accessors/methods.
2462-
if (enclosingClass is ExtensionTypeElementImpl) {
2463-
continue;
2464-
}
24652520
diagnosticReporter.report(
24662521
diag.conflictingFieldAndMethod
24672522
.withArguments(
@@ -2488,7 +2543,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
24882543
}
24892544
var setterName = methodName.forSetter;
24902545
var setter = inherited[setterName];
2491-
if (setter is InternalPropertyAccessorElement) {
2546+
if (setter is InternalSetterElement) {
24922547
diagnosticReporter.report(
24932548
diag.conflictingInheritedMethodAndSetter
24942549
.withArguments(
@@ -2497,33 +2552,23 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
24972552
memberName: methodName.name,
24982553
)
24992554
.withContextMessages([
2500-
DiagnosticMessageImpl(
2501-
filePath:
2502-
method.firstFragment.libraryFragment.source.fullName,
2555+
method.diagnosticMessage(
25032556
message: formatList(
25042557
"The method is inherited from the {0} '{1}'.",
25052558
[
25062559
method.enclosingElement!.kind.displayName,
25072560
method.enclosingElement!.name,
25082561
],
25092562
),
2510-
offset: method.firstFragment.nameOffset!,
2511-
length: method.firstFragment.name!.length,
2512-
url: null,
25132563
),
2514-
DiagnosticMessageImpl(
2515-
filePath:
2516-
setter.firstFragment.libraryFragment.source.fullName,
2564+
setter.diagnosticMessage(
25172565
message: formatList(
25182566
"The setter is inherited from the {0} '{1}'.",
25192567
[
25202568
setter.enclosingElement.kind.displayName,
25212569
setter.enclosingElement.name,
25222570
],
25232571
),
2524-
offset: setter.firstFragment.nameOffset!,
2525-
length: setter.firstFragment.name!.length,
2526-
url: null,
25272572
),
25282573
])
25292574
.atSourceRange(

pkg/analyzer/test/src/dart/element/inheritance_manager3_test.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'package:collection/collection.dart';
1313
import 'package:test/test.dart';
1414
import 'package:test_reflective_loader/test_reflective_loader.dart';
1515

16+
import '../../../util/diff.dart';
1617
import '../../../util/element_printer.dart';
1718
import '../../summary/elements_base.dart';
1819
import '../resolution/context_collection_resolution.dart';
@@ -3104,9 +3105,9 @@ class _InheritanceManager3Base2 extends ElementsBaseTest {
31043105
void assertInterfaceText(InterfaceElementImpl element, String expected) {
31053106
var actual = _interfaceText(element);
31063107
if (actual != expected) {
3107-
print('-------- Actual --------');
3108-
print('$actual------------------------');
31093108
NodeTextExpectationsCollector.add(actual);
3109+
printPrettyDiff(expected, actual);
3110+
fail('See the difference above.');
31103111
}
31113112
expect(actual, expected);
31123113
}

0 commit comments

Comments
 (0)