diff --git a/lib/src/model/nameable.dart b/lib/src/model/nameable.dart index 3e8818caae..761ea0150c 100644 --- a/lib/src/model/nameable.dart +++ b/lib/src/model/nameable.dart @@ -35,5 +35,10 @@ abstract class Nameable { String toString() => name; } -int byName(Nameable a, Nameable b) => - compareAsciiLowerCaseNatural(a.name, b.name); +int byName(Nameable a, Nameable b) { + var stringCompare = compareAsciiLowerCaseNatural(a.name, b.name); + if (stringCompare == 0) { + return a.hashCode.compareTo(b.hashCode); + } + return stringCompare; +} diff --git a/lib/src/model/package.dart b/lib/src/model/package.dart index e1f4113964..327207a6b8 100644 --- a/lib/src/model/package.dart +++ b/lib/src/model/package.dart @@ -406,7 +406,7 @@ class Package extends LibraryContainer Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren.addEntries(allLibraries.generateEntries()); + _referenceChildren.addEntries(publicLibrariesSorted.generateEntries()); // Do not override any preexisting data, and insert based on the // public library sort order. // TODO(jcollins-g): warn when results require package-global diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index 62b501dbdf..d088ec260f 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -1030,19 +1030,26 @@ class PackageGraph with CommentReferable, Nameable { Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; + // We have to use a stable order or otherwise references depending + // on ambiguous resolution (see below) will change where they + // resolve based on internal implementation details. + var sortedPackages = packages.toList()..sort(byName); + var sortedDocumentedPackages = documentedPackages.toList()..sort(byName); // Packages are the top priority. - _referenceChildren.addEntries(packages.generateEntries()); + _referenceChildren.addEntries(sortedPackages.generateEntries()); // Libraries are next. // TODO(jcollins-g): Warn about directly referencing libraries out of - // scope? - _referenceChildren.addEntriesIfAbsent(documentedPackages + // scope? Doing this is always going to be ambiguous and potentially + // confusing. + _referenceChildren.addEntriesIfAbsent(sortedDocumentedPackages .expand((p) => p.publicLibrariesSorted) .generateEntries()); // TODO(jcollins-g): Warn about directly referencing top level items - // out of scope? - _referenceChildren.addEntriesIfAbsent(documentedPackages + // out of scope? Doing this will be even more ambiguous and + // potentially confusing than doing so with libraries. + _referenceChildren.addEntriesIfAbsent(sortedDocumentedPackages .expand((p) => p.publicLibrariesSorted) .expand((l) => [ ...l.publicConstants, diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 66552dc856..f1134d9e7b 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -4970,6 +4970,16 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, expect(byName(b, a), 1); }); } + + test('sort order is stable when necessary', () { + var a = StringNameHashCode('a', 12); + var b = StringNameHashCode('b', 12); + var aa = StringNameHashCode('a', 14); + expect(byName(a, aa), -1); + expect(byName(a, b), -1); + expect(byName(b, a), 1); + expect(byName(aa, b), -1); + }); }); } @@ -4982,3 +4992,18 @@ class StringName extends Nameable { @override String toString() => name; } + +class StringNameHashCode extends Nameable { + @override + final String name; + @override + final int hashCode; + + StringNameHashCode(this.name, this.hashCode); + + @override + String toString() => name; + + @override + bool operator ==(Object other) => super == other; +}