From ad920340822743fcf431ccd9f725760e14055e68 Mon Sep 17 00:00:00 2001
From: David Morgan <davidmorgan@google.com>
Date: Thu, 10 Oct 2024 10:26:33 +0200
Subject: [PATCH] Address review comments.

---
 pkgs/dart_model/lib/src/dart_model.dart | 40 +++++++++++++++++--------
 pkgs/dart_model/test/model_test.dart    | 16 +++++-----
 2 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/pkgs/dart_model/lib/src/dart_model.dart b/pkgs/dart_model/lib/src/dart_model.dart
index f603ed15..cf4257b7 100644
--- a/pkgs/dart_model/lib/src/dart_model.dart
+++ b/pkgs/dart_model/lib/src/dart_model.dart
@@ -24,20 +24,36 @@ extension ModelExtension on Model {
   /// types of node we want this functionality exposed for.
   /// TODO(davidmorgan): a list of path segments is probably more useful than
   /// `String`.
-  String? pathToMember(Member member) => _pathTo(member.node);
+  QualifiedName? qualifiedNameOfMember(Member member) =>
+      _qualifiedNameOf(member.node);
 
-  /// Returns the path in the model to [node], or `null` if [node] is not in this [Model].
-  String? _pathTo(Map<String, Object?> node) {
-    if (node == this.node) return '';
-    final parent = _getParent(node);
-    if (parent == null) return null;
-    for (final entry in parent.entries) {
-      if (entry.value == node) {
-        final parentPath = _pathTo(parent);
-        return parentPath == null ? null : '$parentPath/${entry.key}';
-      }
+  /// Returns the [QualifiedName] in the model to [node], or `null` if [node] is not in this [Model].
+  QualifiedName? _qualifiedNameOf(Map<String, Object?> node) {
+    final members = _getParent(node);
+    if (members == null) return null;
+    final interface = _getParent(members);
+    if (interface == null) return null;
+    final scopes = _getParent(interface);
+    if (scopes == null) return null;
+    final library = _getParent(scopes);
+    if (library == null) return null;
+    final libraries = _getParent(library);
+    if (libraries == null) return null;
+
+    final uri = _keyOf(library, libraries);
+    final name = _keyOf(interface, scopes);
+
+    return QualifiedName(uri: uri, name: name);
+  }
+
+  /// Returns the key of [value] in [map].
+  ///
+  /// Throws if [value] is not in [map].
+  String _keyOf(Object value, Map<String, Object?> map) {
+    for (final entry in map.entries) {
+      if (entry.value == value) return entry.key;
     }
-    return null;
+    throw ArgumentError('Value not in map: $value, $map');
   }
 
   /// Gets the `Map` that contains [node], or `null` if there isn't one.
diff --git a/pkgs/dart_model/test/model_test.dart b/pkgs/dart_model/test/model_test.dart
index 8898a7e8..7ffbbf6f 100644
--- a/pkgs/dart_model/test/model_test.dart
+++ b/pkgs/dart_model/test/model_test.dart
@@ -111,16 +111,16 @@ void main() {
     test('can give the path to Members in buffer backed maps', () {
       final member = model.uris['package:dart_model/dart_model.dart']!
           .scopes['JsonData']!.members['_root']!;
-      expect(model.pathToMember(member),
-          '/uris/package:dart_model/dart_model.dart/scopes/JsonData/members/_root');
+      expect(model.qualifiedNameOfMember(member)!.asString,
+          'package:dart_model/dart_model.dart#JsonData');
     });
 
     test('can give the path to Members in SDK maps', () {
       final copiedModel = Model.fromJson(_copyMap(model.node));
       final member = copiedModel.uris['package:dart_model/dart_model.dart']!
           .scopes['JsonData']!.members['_root']!;
-      expect(copiedModel.pathToMember(member),
-          '/uris/package:dart_model/dart_model.dart/scopes/JsonData/members/_root');
+      expect(copiedModel.qualifiedNameOfMember(member)!.asString,
+          'package:dart_model/dart_model.dart#JsonData');
     });
 
     test('path to Members throws on cycle', () {
@@ -129,7 +129,7 @@ void main() {
       (copiedModel.node['uris'] as Map<String, Object?>)['loop'] = copiedModel;
       final member = copiedModel.uris['package:dart_model/dart_model.dart']!
           .scopes['JsonData']!.members['_root']!;
-      expect(() => copiedModel.pathToMember(member), throwsStateError);
+      expect(() => copiedModel.qualifiedNameOfMember(member), throwsStateError);
     });
 
     test('path to Members throws on reused node', () {
@@ -139,7 +139,7 @@ void main() {
           copiedModel.uris['package:dart_model/dart_model.dart']!;
       final member = copiedModel.uris['package:dart_model/dart_model.dart']!
           .scopes['JsonData']!.members['_root']!;
-      expect(() => copiedModel.pathToMember(member), throwsStateError);
+      expect(() => copiedModel.qualifiedNameOfMember(member), throwsStateError);
     });
 
     test('path to Member returns null for Member in wrong Map', () {
@@ -151,8 +151,8 @@ void main() {
           .scopes['JsonData']!
           .members['_root']!
           .node));
-      expect(copiedModel.pathToMember(member), null);
-      expect(model.pathToMember(copiedMember), null);
+      expect(copiedModel.qualifiedNameOfMember(member), null);
+      expect(model.qualifiedNameOfMember(copiedMember), null);
     });
   });