Reland "analyzer: Write out qualified extension names in error messages"

This is a reland of commit 10c1d88

This includes a fix in listener.dart to not null-assert on an
element's name (`element.name!`). This fixes a problem in google
internal code.

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: I6c20cf023af26b2d3b0fe9d8193f5e067719da8a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/389587
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -151,7 +151,9 @@ CompileTimeErrorCode.AMBIGUOUS_EXPORT:
   status: needsFix
   notes: |-
     For each exported name, add a fix to hide the name.
-CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS:
+CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_THREE_OR_MORE:
+  status: hasFix
+CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO:
   status: hasFix
 CompileTimeErrorCode.AMBIGUOUS_IMPORT:
   status: needsFix

pkg/analysis_server/lib/src/services/correction/fix_internal.dart

@@ -748,7 +748,10 @@ final _builtInLintProducers = <LintCode, List<ProducerGenerator>>{
 };
 
 final _builtInNonLintMultiProducers = {
-  CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS: [
+  CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO: [
+    AddExtensionOverride.new,
+  ],
+  CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_THREE_OR_MORE: [
     AddExtensionOverride.new,
   ],
   CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE: [

pkg/analysis_server/test/src/services/correction/fix/add_extension_override_test.dart

@@ -44,7 +44,7 @@ f() {
 }
 ''', expectedNumberOfFixesForKind: 1, errorFilter: (error) {
       return error.errorCode ==
-          CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS;
+          CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO;
     });
   }
 

pkg/analyzer/lib/error/listener.dart

@@ -11,8 +11,10 @@ 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';
 
@@ -168,7 +170,19 @@ class ErrorReporter {
       return;
     }
 
-    _convertElements(arguments);
+    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(', ')}');
+      }
+    }
+
     contextMessages ??= [];
     contextMessages.addAll(_convertTypeNames(arguments));
     _errorListener.onError(
@@ -334,92 +348,82 @@ class ErrorReporter {
     );
   }
 
-  /// 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
+  /// 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
   /// clarify the message.
   List<DiagnosticMessage> _convertTypeNames(List<Object?>? arguments) {
-    var messages = <DiagnosticMessage>[];
     if (arguments == null) {
-      return messages;
+      return const [];
     }
 
-    Map<String, List<_TypeToConvert>> typeGroups = {};
-    for (int i = 0; i < arguments.length; i++) {
+    var typeGroups = <String, List<_ToConvert>>{};
+    for (var i = 0; i < arguments.length; i++) {
       var argument = arguments[i];
       if (argument is TypeImpl) {
-        String displayName = argument.getDisplayString(
-          preferTypeAlias: true,
-        );
-        List<_TypeToConvert> types =
-            typeGroups.putIfAbsent(displayName, () => <_TypeToConvert>[]);
+        var displayName = argument.getDisplayString(preferTypeAlias: true);
+        var types = typeGroups.putIfAbsent(displayName, () => []);
         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));
       }
     }
-    for (List<_TypeToConvert> typeGroup in typeGroups.values) {
+
+    var messages = <DiagnosticMessage>[];
+    for (var typeGroup in typeGroups.values) {
       if (typeGroup.length == 1) {
-        _TypeToConvert typeToConvert = typeGroup[0];
+        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.
         arguments[typeToConvert.index] = typeToConvert.displayName;
-      } 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);
-          }
+        continue;
+      }
+
+      const unnamedExtension = '<unnamed extension>';
+      const unnamed = '<unnamed>';
+      var nameToElementMap = <String, Set<Element>>{};
+      for (var typeToConvert in typeGroup) {
+        for (var element in typeToConvert.allElements) {
+          var name = element.name;
+          name ??= element is ExtensionElement ? unnamedExtension : unnamed;
+
+          var elements = nameToElementMap.putIfAbsent(name, () => {});
+          elements.add(element);
         }
-        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}');
-            }
-            messages.add(DiagnosticMessageImpl(
-                filePath: element.source!.fullName,
-                length: element.nameLength,
-                message: '$name is defined in ${element.source!.fullName}',
-                offset: element.nameOffset,
-                url: null));
-          }
+      }
 
-          if (buffer != null) {
-            arguments[typeToConvert.index] =
-                '${typeToConvert.displayName} ($buffer)';
-          } else {
-            arguments[typeToConvert.index] = typeToConvert.displayName;
+      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;
+          name ??= element is ExtensionElement ? unnamedExtension : unnamed;
+          var sourcePath = element.source!.fullName;
+          if (nameToElementMap[name]!.length > 1) {
+            if (buffer == null) {
+              buffer = StringBuffer();
+              buffer.write('where ');
+            } else {
+              buffer.write(', ');
+            }
+            buffer.write('$name is defined in $sourcePath');
           }
+          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;
       }
     }
     return messages;
@@ -453,6 +457,22 @@ 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
@@ -461,42 +481,61 @@ class _NullErrorListener implements AnalysisErrorListener {
   }
 }
 
-/// 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;
+/// 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;
 
-  List<Element>? _allElements;
+  /// The argument's display string, to replace the argument in the argument
+  /// list.
+  String get displayName;
 
-  _TypeToConvert(this.index, this.type, this.displayName);
+  /// The index of the argument in the argument list.
+  int get index;
+}
 
-  List<Element> allElements() {
-    if (_allElements == null) {
-      Set<Element> elements = <Element>{};
+/// 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
+  final int index;
 
-      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);
-            }
+  final DartType _type;
+
+  @override
+  final String 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);
           }
         }
       }
-
-      addElementsFrom(type);
-      _allElements = elements.where((element) {
-        var name = element.name;
-        return name != null && name.isNotEmpty;
-      }).toList();
     }
-    return _allElements!;
-  }
+
+    addElementsFrom(_type);
+    return elements.where((element) {
+      var name = element.name;
+      return name != null && name.isNotEmpty;
+    });
+  }();
+
+  _TypeToConvert(this.index, this._type, this.displayName);
 }

pkg/analyzer/lib/src/dart/resolver/extension_member_resolver.dart

@@ -117,21 +117,33 @@ class ExtensionMemberResolver {
     }
 
     // The most specific extension is ambiguous.
-    _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,
-      ],
-    );
+    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,
+        ],
+      );
+    }
     return ResolutionResult.ambiguous;
   }