Skip to content

Make PackageGraph.allLibraries private #3792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Make PackageGraph.allLibraries private #3792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions lib/src/model/package_graph.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class PackageGraph with CommentReferable, Nameable {
@override
String get breadcrumbName => throw UnimplementedError();

/// Adds [resolvedLibrary] to the package graph, adding it to [allLibraries],
/// Adds [resolvedLibrary] to the package graph, adding it to [_allLibraries],
/// and to the [Package] which is created from the [PackageMeta] for the
/// library.
///
Expand All @@ -108,11 +108,11 @@ class PackageGraph with CommentReferable, Nameable {
var package = Package.fromPackageMeta(packageMeta, this);
var lib = Library.fromLibraryResult(resolvedLibrary, this, package);
package.libraries.add(lib);
allLibraries[libraryElement.source.fullName] = lib;
_allLibraries[libraryElement.source.fullName] = lib;
}

/// Adds [resolvedLibrary] as a special library to the package graph, which
/// adds the library to [allLibraries], but does not add it to any [Package]'s
/// adds the library to [_allLibraries], but does not add it to any [Package]'s
/// list of libraries.
///
/// Call during initialization to add a library possibly containing
Expand All @@ -122,7 +122,7 @@ class PackageGraph with CommentReferable, Nameable {
allLibrariesAdded = true;
assert(!_localDocumentationBuilt);
final libraryElement = resolvedLibrary.element.library;
allLibraries.putIfAbsent(
_allLibraries.putIfAbsent(
libraryElement.source.fullName,
() => Library.fromLibraryResult(
resolvedLibrary,
Expand Down Expand Up @@ -312,18 +312,18 @@ class PackageGraph with CommentReferable, Nameable {
return _extensions;
}

/// All library objects related to this package; a superset of [libraries].
/// All library objects related to this package graph; a superset of
/// [libraries].
///
/// Keyed by `LibraryElement.Source.fullName` to resolve different URIs, which
/// refer to the same location, to the same [Library]. This isn't how Dart
/// Keyed by `LibraryElement.source.fullName` to resolve different URIs
/// referring to the same location, to the same [Library]. This isn't how Dart
/// works internally, but Dartdoc pretends to avoid documenting or duplicating
/// data structures for the same "library" on disk based on how it is
/// referenced. We can't use [Source] as a key due to differences in the
/// [TimestampedData] timestamps.
///
/// This mapping must be complete before [initializePackageGraph] is called.
@visibleForTesting
final Map<String, Library> allLibraries = {};
final Map<String, Library> _allLibraries = {};

/// All [ModelElement]s constructed for this package; a superset of
/// the elements gathered in [_gatherModelElements].
Expand Down Expand Up @@ -533,8 +533,8 @@ class PackageGraph with CommentReferable, Nameable {
/// which is created if it is not yet populated.
Map<LibraryElement, Set<Library>> get libraryExports {
// Table must be reset if we're still in the middle of adding libraries.
if (allLibraries.keys.length != _lastSizeOfAllLibraries) {
_lastSizeOfAllLibraries = allLibraries.keys.length;
if (_allLibraries.keys.length != _lastSizeOfAllLibraries) {
_lastSizeOfAllLibraries = _allLibraries.keys.length;
_libraryExports = {};
for (var library in publicLibraries) {
_tagExportsFor(library, library.element);
Expand Down Expand Up @@ -565,7 +565,7 @@ class PackageGraph with CommentReferable, Nameable {
hrefMap.putIfAbsent(href, () => {}).add(modelElement);
}

for (final library in allLibraries.values) {
for (final library in _allLibraries.values) {
final href = library.href;
if (href == null) continue;
hrefMap.putIfAbsent(href, () => {}).add(library);
Expand Down Expand Up @@ -862,7 +862,7 @@ class PackageGraph with CommentReferable, Nameable {
/// set of canonical Libraries).
Library? findButDoNotCreateLibraryFor(Element e) {
// This is just a cache to avoid creating lots of libraries over and over.
return allLibraries[e.library?.source.fullName];
return _allLibraries[e.library?.source.fullName];
}

/// Gathers all of the model elements found in all of the libraries of all
Expand Down Expand Up @@ -894,7 +894,9 @@ class PackageGraph with CommentReferable, Nameable {
return allElements;
}

/// Glob lookups can be expensive. Cache per filename.
/// Cache of 'nodoc' configurations.
///
/// Glob lookups can be expensive, so cache per filename.
final _configSetsNodocFor = HashMap<String, bool>();

/// Given an element's [fullName], look up the nodoc configuration data and
Expand Down
42 changes: 17 additions & 25 deletions test/end2end/model_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,8 +839,7 @@ void main() async {

setUpAll(() {
anonLib = packageGraph.libraries.named('anonymous_library');

someLib = packageGraph.allLibraries.values.named('reexport.somelib');
someLib = packageGraph.libraries.named('reexport.somelib');
reexportOneLib = packageGraph.libraries.named('reexport_one');
reexportTwoLib = packageGraph.libraries.named('reexport_two');
reexportThreeLib = packageGraph.libraries.named('reexport_three');
Expand Down Expand Up @@ -2240,12 +2239,11 @@ void main() async {
late Constructor aConstructor;

setUp(() {
base = packageGraph.allLibraries.values.named('two_exports.src.base');
extending =
packageGraph.allLibraries.values.named('two_exports.src.extending');
local_scope = packageGraph.allLibraries.values
.named('two_exports.src.local_scope');
two_exports = packageGraph.allLibraries.values.named('two_exports');
base = packageGraph.libraries.named('two_exports.src.base');
extending = packageGraph.libraries.named('two_exports.src.extending');
local_scope =
packageGraph.libraries.named('two_exports.src.local_scope');
two_exports = packageGraph.libraries.named('two_exports');

BaseWithMembers = base.classes.named('BaseWithMembers');
aStaticField = BaseWithMembers.staticFields.named('aStaticField');
Expand Down Expand Up @@ -2469,23 +2467,19 @@ void main() async {
initViaFieldFormal;

setUpAll(() async {
mylibpub = packageGraph.allLibraries.values.named('mylibpub');
mylibpub = packageGraph.libraries.named('mylibpub');
aFunctionUsingRenamedLib =
fakeLibrary.functions.named('aFunctionUsingRenamedLib');
Dart = packageGraph.allLibraries.values.named('Dart');
Dart = packageGraph.libraries.named('Dart');
DartPackage = packageGraph.packages.firstWhere((p) => p.name == 'Dart');
nameWithTwoUnderscores =
fakeLibrary.constants.named('NAME_WITH_TWO_UNDERSCORES');
nameWithSingleUnderscore =
fakeLibrary.constants.named('NAME_SINGLEUNDERSCORE');
string = packageGraph.allLibraries.values
.named('dart:core')
.classes
.named('String');
metaUseResult = packageGraph.allLibraries.values
.named('meta')
.classes
.named('UseResult');
string =
packageGraph.libraries.named('dart:core').classes.named('String');
metaUseResult =
packageGraph.libraries.named('meta').classes.named('UseResult');
baseForDocComments = fakeLibrary.classes.named('BaseForDocComments');
aNonDefaultConstructor = baseForDocComments.constructors
.named('BaseForDocComments.aNonDefaultConstructor');
Expand All @@ -2512,11 +2506,11 @@ void main() async {
.named('csspub')
.properties
.named('theOnlyThingInTheLibrary');
doesStuff = packageGraph.allLibraries.values
doesStuff = packageGraph.libraries
.named('anonymous_library')
.functions
.named('doesStuff');
BaseClass = packageGraph.allLibraries.values
BaseClass = packageGraph.libraries
.named('two_exports.src.base')
.classes
.named('BaseClass');
Expand All @@ -2531,7 +2525,7 @@ void main() async {
.named('ImplicitProperties')
.allFields
.named('forInheriting');
action = packageGraph.allLibraries.values
action = packageGraph.libraries
.named('reexport.somelib')
.classes
.named('BaseReexported')
Expand Down Expand Up @@ -2845,10 +2839,8 @@ void main() async {
reexportOneLib.extensions.named('DocumentThisExtensionOnce');
documentOnceReexportTwo =
reexportTwoLib.extensions.named('DocumentThisExtensionOnce');
string = packageGraph.allLibraries.values
.named('dart:core')
.classes
.named('String');
string =
packageGraph.libraries.named('dart:core').classes.named('String');
apple = exLibrary.classes.named('Apple');
ext = exLibrary.extensions.named('AppleExtension');
extensionReferencer = exLibrary.classes.named('ExtensionReferencer');
Expand Down