Skip to content

Commit

Permalink
Revert "analyzer: Write out qualified extension names in error messages"
Browse files Browse the repository at this point in the history 
This reverts commit 10c1d88.

Reason for revert: g/dart-sdk-rolls/MOLCzv4S-kQ

Original change's description:
> analyzer: Write out qualified extension names in error messages
>
> Fixes #56269
>
> Change-Id: I025966fd4aa3d7c5b71175321f21f95e8c41f086
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/388580
> Reviewed-by: Konstantin Shcheglov <[email protected]>
> Commit-Queue: Samuel Rawlins <[email protected]>
> Reviewed-by: Brian Wilkerson <[email protected]>

Change-Id: I613695e90c96e5ffdb4dc56e729dc34be385f5fe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/388823
Reviewed-by: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Auto-Submit: Alexander Thomas <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
Commit-Queue: Alexander Thomas <[email protected]>
  • Loading branch information
@athomas
athomas authored and Commit Queue committed Oct 8, 2024
1 parent 284e9e9 commit d4aa30a
Show file tree
Hide file tree
Showing 10 changed files with 147 additions and 265 deletions.
4 changes: 1 addition & 3 deletions pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,7 @@ CompileTimeErrorCode.AMBIGUOUS_EXPORT:
status: needsFix
notes: |-
For each exported name, add a fix to hide the name.
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_THREE_OR_MORE:
status: hasFix
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO:
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS:
status: hasFix
CompileTimeErrorCode.AMBIGUOUS_IMPORT:
status: needsFix
Expand Down
5 changes: 1 addition & 4 deletions pkg/analysis_server/lib/src/services/correction/fix_internal.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -747,10 +747,7 @@ final _builtInLintProducers = <LintCode, List<ProducerGenerator>>{
};

final _builtInNonLintMultiProducers = {
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO: [
AddExtensionOverride.new,
],
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_THREE_OR_MORE: [
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS: [
AddExtensionOverride.new,
],
CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE: [
Expand Down
2 changes: 1 addition & 1 deletion pkg/analysis_server/test/src/services/correction/fix/add_extension_override_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ f() {
}
''', expectedNumberOfFixesForKind: 1, errorFilter: (error) {
return error.errorCode ==
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO;
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS;
});
}

Expand Down
235 changes: 101 additions & 134 deletions pkg/analyzer/lib/error/listener.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,8 @@ import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/diagnostic/diagnostic.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/source/source.dart';
import 'package:analyzer/src/dart/element/extensions.dart';
import 'package:analyzer/src/dart/element/type.dart';
import 'package:analyzer/src/diagnostic/diagnostic.dart';
import 'package:analyzer/src/utilities/extensions/collection.dart';
import 'package:meta/meta.dart';
import 'package:source_span/source_span.dart';

Expand Down Expand Up @@ -170,19 +168,7 @@ class ErrorReporter {
return;
}

if (arguments != null) {
var invalid = arguments
.whereNotType<String>()
.whereNotType<DartType>()
.whereNotType<Element>()
.whereNotType<int>()
.whereNotType<Uri>();
if (invalid.isNotEmpty) {
throw ArgumentError('Tried to format an error using '
'${invalid.map((e) => e.runtimeType).join(', ')}');
}
}

_convertElements(arguments);
contextMessages ??= [];
contextMessages.addAll(_convertTypeNames(arguments));
_errorListener.onError(
Expand Down Expand Up @@ -348,76 +334,92 @@ class ErrorReporter {
);
}

/// Given an array of [arguments] that may contain [DartType]s and [Element]s,
/// converts the types and elements into strings by using the display names of
/// each, unless there are two or more types or elements with the same display
/// names, in which case the extended display names will be used in order to
/// Convert all [Element]s in the [arguments] into their display strings.
void _convertElements(List<Object>? arguments) {
if (arguments == null) {
return;
}

for (var i = 0; i < arguments.length; i++) {
var argument = arguments[i];
if (argument is Element) {
arguments[i] = argument.getDisplayString();
} else if (!(argument is String ||
argument is DartType ||
argument is int ||
argument is Uri)) {
throw ArgumentError(
'Tried to format an error using ${argument.runtimeType}');
}
}
}

/// Given an array of [arguments] that is expected to contain two or more
/// types, convert the types into strings by using the display names of the
/// types, unless there are two or more types with the same names, in which
/// case the extended display names of the types will be used in order to
/// clarify the message.
List<DiagnosticMessage> _convertTypeNames(List<Object?>? arguments) {
var messages = <DiagnosticMessage>[];
if (arguments == null) {
return const [];
return messages;
}

var typeGroups = <String, List<_ToConvert>>{};
for (var i = 0; i < arguments.length; i++) {
Map<String, List<_TypeToConvert>> typeGroups = {};
for (int i = 0; i < arguments.length; i++) {
var argument = arguments[i];
if (argument is TypeImpl) {
var displayName = argument.getDisplayString(preferTypeAlias: true);
var types = typeGroups.putIfAbsent(displayName, () => []);
String displayName = argument.getDisplayString(
preferTypeAlias: true,
);
List<_TypeToConvert> types =
typeGroups.putIfAbsent(displayName, () => <_TypeToConvert>[]);
types.add(_TypeToConvert(i, argument, displayName));
} else if (argument is Element) {
var displayName = argument.getDisplayString();
var types = typeGroups.putIfAbsent(displayName, () => []);
types.add(_ElementToConvert(i, argument, displayName));
}
}

var messages = <DiagnosticMessage>[];
for (var typeGroup in typeGroups.values) {
for (List<_TypeToConvert> typeGroup in typeGroups.values) {
if (typeGroup.length == 1) {
var typeToConvert = typeGroup[0];
// If the display name of a type is unambiguous, just replace the type
// in the arguments list with its display name.
_TypeToConvert typeToConvert = typeGroup[0];
arguments[typeToConvert.index] = typeToConvert.displayName;
continue;
}

var nameToElementMap = <String, Set<Element>>{};
for (var typeToConvert in typeGroup) {
for (var element in typeToConvert.allElements) {
var elements = nameToElementMap.putIfAbsent(element.name!, () => {});
elements.add(element);
} else {
Map<String, Set<Element>> nameToElementMap = {};
for (_TypeToConvert typeToConvert in typeGroup) {
for (Element element in typeToConvert.allElements()) {
Set<Element> elements =
nameToElementMap.putIfAbsent(element.name!, () => <Element>{});
elements.add(element);
}
}
}

for (var typeToConvert in typeGroup) {
// TODO(brianwilkerson): When clients do a better job of displaying
// context messages, remove the extra text added to the buffer.
StringBuffer? buffer;
for (var element in typeToConvert.allElements) {
var name = element.name!;
var sourcePath = element.source!.fullName;
if (nameToElementMap[name]!.length > 1) {
if (buffer == null) {
buffer = StringBuffer();
buffer.write('where ');
} else {
buffer.write(', ');
for (_TypeToConvert typeToConvert in typeGroup) {
// TODO(brianwilkerson): When clients do a better job of displaying
// context messages, remove the extra text added to the buffer.
StringBuffer? buffer;
for (Element element in typeToConvert.allElements()) {
String name = element.name!;
if (nameToElementMap[name]!.length > 1) {
if (buffer == null) {
buffer = StringBuffer();
buffer.write('where ');
} else {
buffer.write(', ');
}
buffer.write('$name is defined in ${element.source!.fullName}');
}
buffer.write('$name is defined in $sourcePath');
messages.add(DiagnosticMessageImpl(
filePath: element.source!.fullName,
length: element.nameLength,
message: '$name is defined in ${element.source!.fullName}',
offset: element.nameOffset,
url: null));
}
messages.add(DiagnosticMessageImpl(
filePath: element.source!.fullName,
length: element.nameLength,
message: '$name is defined in $sourcePath',
offset: element.nameOffset,
url: null,
));
}

arguments[typeToConvert.index] = buffer != null
? '${typeToConvert.displayName} ($buffer)'
: typeToConvert.displayName;
if (buffer != null) {
arguments[typeToConvert.index] =
'${typeToConvert.displayName} ($buffer)';
} else {
arguments[typeToConvert.index] = typeToConvert.displayName;
}
}
}
}
return messages;
Expand Down Expand Up @@ -451,22 +453,6 @@ class RecordingErrorListener implements AnalysisErrorListener {
}
}

/// Used by [ErrorReporter._convertTypeNames] to keep track of an error argument
/// that is an [Element], that is being converted to a display string.
class _ElementToConvert implements _ToConvert {
@override
final int index;

@override
final String displayName;

@override
final Iterable<Element> allElements;

_ElementToConvert(this.index, Element element, this.displayName)
: allElements = [element];
}

/// An [AnalysisErrorListener] that ignores error.
class _NullErrorListener implements AnalysisErrorListener {
@override
Expand All @@ -475,61 +461,42 @@ class _NullErrorListener implements AnalysisErrorListener {
}
}

/// Used by [ErrorReporter._convertTypeNames] to keep track of an argument that
/// is being converted to a display string.
abstract class _ToConvert {
/// A list of all elements involved in the [DartType] or [Element]'s display
/// string.
Iterable<Element> get allElements;

/// The argument's display string, to replace the argument in the argument
/// list.
String get displayName;

/// The index of the argument in the argument list.
int get index;
}

/// Used by [ErrorReporter._convertTypeNames] to keep track of an error argument
/// that is a [DartType], that is being converted to a display string.
class _TypeToConvert implements _ToConvert {
@override
/// Used by `ErrorReporter._convertTypeNames` to keep track of a type that is
/// being converted.
class _TypeToConvert {
final int index;
final DartType type;
final String displayName;

final DartType _type;
List<Element>? _allElements;

@override
final String displayName;
_TypeToConvert(this.index, this.type, this.displayName);

@override
late final Iterable<Element> allElements = () {
var elements = <Element>{};

void addElementsFrom(DartType type) {
if (type is FunctionType) {
addElementsFrom(type.returnType);
for (var parameter in type.parameters) {
addElementsFrom(parameter.type);
}
} else if (type is RecordType) {
for (var parameter in type.fields) {
addElementsFrom(parameter.type);
}
} else if (type is InterfaceType) {
if (elements.add(type.element)) {
for (var typeArgument in type.typeArguments) {
addElementsFrom(typeArgument);
List<Element> allElements() {
if (_allElements == null) {
Set<Element> elements = <Element>{};

void addElementsFrom(DartType type) {
if (type is FunctionType) {
addElementsFrom(type.returnType);
for (ParameterElement parameter in type.parameters) {
addElementsFrom(parameter.type);
}
} else if (type is InterfaceType) {
if (elements.add(type.element)) {
for (DartType typeArgument in type.typeArguments) {
addElementsFrom(typeArgument);
}
}
}
}
}

addElementsFrom(_type);
return elements.where((element) {
var name = element.name;
return name != null && name.isNotEmpty;
});
}();

_TypeToConvert(this.index, this._type, this.displayName);
addElementsFrom(type);
_allElements = elements.where((element) {
var name = element.name;
return name != null && name.isNotEmpty;
}).toList();
}
return _allElements!;
}
}
42 changes: 15 additions & 27 deletions pkg/analyzer/lib/src/dart/resolver/extension_member_resolver.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,33 +117,21 @@ class ExtensionMemberResolver {
}

// The most specific extension is ambiguous.
if (mostSpecific.length == 2) {
_errorReporter.atEntity(
nameEntity,
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO,
arguments: [
name.name,
mostSpecific[0].extension,
mostSpecific[1].extension,
],
);
} else {
_errorReporter.atEntity(
nameEntity,
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_THREE_OR_MORE,
arguments: [
name.name,
mostSpecific.map((e) {
var name = e.extension.name;
if (name != null) {
return "extension '$name'";
}
var type = e.extension.extendedType.getDisplayString();
return "unnamed extension on '$type'";
}).commaSeparatedWithAnd,
],
);
}
_errorReporter.atEntity(
nameEntity,
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS,
arguments: [
name.name,
mostSpecific.map((e) {
var name = e.extension.name;
if (name != null) {
return "extension '$name'";
}
var type = e.extension.extendedType.getDisplayString();
return "unnamed extension on '$type'";
}).commaSeparatedWithAnd,
],
);
return ResolutionResult.ambiguous;
}

Expand Down
Loading

0 comments on commit d4aa30a

Please sign in to comment.