Simplify LibraryContainer hierarchy and tests

Author: srawlins

lib/src/model/category.dart

@@ -11,13 +11,12 @@ import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/warnings.dart';
 
 /// A subcategory of a package, containing elements tagged with `{@category}`.
-class Category
+final class Category extends LibraryContainer
     with
         Nameable,
         Warnable,
         CommentReferable,
         MarkdownFileDocumentation,
-        LibraryContainer,
         TopLevelContainer
     implements Documentable {
   /// The package in which this category is contained.
@@ -64,7 +63,8 @@ class Category
   Category(this._name, this.package, this.config)
       : _categoryDefinition =
             config.categories.categoryDefinitions[_name.orDefault] ??
-                CategoryDefinition(_name, null, null);
+                CategoryDefinition(_name, null, null),
+        super(isSdk: false, enclosingName: package.name);
 
   Iterable<ExternalItem> get externalItems => _categoryDefinition.externalItems;
 
@@ -88,9 +88,6 @@ class Category
   @override
   List<String> get containerOrder => config.categoryOrder;
 
-  @override
-  String get enclosingName => package.name;
-
   @override
   PackageGraph get packageGraph => package.packageGraph;
 

lib/src/model/library_container.dart

@@ -5,12 +5,13 @@
 import 'package:collection/collection.dart';
 import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/model_utils.dart' as model_utils;
+import 'package:meta/meta.dart';
 
 /// A set of libraries, initialized after construction by accessing [libraries].
 ///
 /// Do not cache return values of any methods or members excepting [libraries]
 /// and [name] before finishing initialization of a [LibraryContainer].
-abstract mixin class LibraryContainer
+abstract base class LibraryContainer
     implements Nameable, Comparable<LibraryContainer>, Documentable {
   final List<Library> libraries = [];
 
@@ -19,31 +20,40 @@ abstract mixin class LibraryContainer
 
   bool get hasPublicLibraries => libraries.any((e) => e.isPublic);
 
+  LibraryContainer({required this.isSdk, required String enclosingName})
+      : _enclosingName = enclosingName;
+
   /// The name of the container or object that this LibraryContainer is a part
-  /// of.  Used for sorting in [containerOrder].
-  String get enclosingName;
+  /// of.
+  ///
+  /// Used for sorting in [containerOrder].
+  final String _enclosingName;
 
   /// Order by which this container should be sorted.
+  @visibleForOverriding
   List<String> get containerOrder;
 
-  /// Sorting key.  [containerOrder] should contain these.
+  /// Sorting key.
+  ///
+  /// [containerOrder] should contain these.
   String get sortKey => name;
 
-  /// Does this container represent the SDK?  This can be false for containers
-  /// that only represent a part of the SDK.
-  bool get isSdk => false;
+  /// Whether this container represents the Dart SDK.
+  ///
+  /// This can be false for containers that only represent a part of the SDK.
+  final bool isSdk;
 
   /// Returns:
   /// * -1 if this container is listed in [containerOrder].
-  /// * 0 if this container is named the same as the [enclosingName].
+  /// * 0 if this container is named the same as the [_enclosingName].
   /// * 1 if this container represents the SDK.
-  /// * 2 if this group has a name that contains the name [enclosingName].
+  /// * 2 if this group has a name that contains the name [_enclosingName].
   /// * 3 otherwise.
   int get _group {
     if (containerOrder.contains(sortKey)) return -1;
-    if (equalsIgnoreAsciiCase(sortKey, enclosingName)) return 0;
+    if (equalsIgnoreAsciiCase(sortKey, _enclosingName)) return 0;
     if (isSdk) return 1;
-    if (sortKey.toLowerCase().contains(enclosingName.toLowerCase())) return 2;
+    if (sortKey.toLowerCase().contains(_enclosingName.toLowerCase())) return 2;
     return 3;
   }
 

lib/src/model/package.dart

@@ -29,7 +29,7 @@ const String htmlBasePlaceholder = r'%%__HTMLBASE_dartdoc_internal__%%';
 
 /// A [LibraryContainer] that contains [Library] objects related to a particular
 /// package.
-class Package extends LibraryContainer
+final class Package extends LibraryContainer
     with Nameable, Warnable, CommentReferable {
   @override
   final String name;
@@ -70,7 +70,10 @@ class Package extends LibraryContainer
           packageGraph.config,
           packageGraph.resourceProvider.getFolder(packagePath),
           packageGraph.resourceProvider,
-        );
+        ),
+        super(
+            isSdk: packageMeta.isSdk,
+            enclosingName: packageGraph.defaultPackageName);
 
   @override
   bool get isCanonical => true;
@@ -183,9 +186,6 @@ class Package extends LibraryContainer
     return DocumentLocation.missing;
   }();
 
-  @override
-  String get enclosingName => packageGraph.defaultPackageName;
-
   String get filePath => 'index.html';
 
   @override
@@ -364,9 +364,6 @@ class Package extends LibraryContainer
       packageGraph.localPackages.isNotEmpty &&
       identical(packageGraph.localPackages.first, this);
 
-  @override
-  bool get isSdk => packageMeta.isSdk;
-
   final String packagePath;
 
   String get version => packageMeta.version;

lib/src/package_config_provider.dart

@@ -25,6 +25,8 @@ class FakePackageConfigProvider implements PackageConfigProvider {
   /// A mapping of package config search locations to configured packages.
   final _packageConfigData = <String, List<package_config.Package>>{};
 
+  /// Adds the package named [name] at [root] to the package config for
+  /// [location].
   void addPackageToConfigFor(String location, String name, Uri root) {
     _packageConfigData
         .putIfAbsent(location, () => [])

test/end2end/model_test.dart

@@ -9,7 +9,6 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/source/line_info.dart';
 import 'package:async/async.dart';
 import 'package:collection/src/iterable_extensions.dart';
-import 'package:dartdoc/src/dartdoc_options.dart';
 import 'package:dartdoc/src/element_type.dart';
 import 'package:dartdoc/src/matching_link_result.dart';
 import 'package:dartdoc/src/model/attribute.dart';
@@ -36,66 +35,6 @@ Future<PackageGraph> get testPackageGraph async =>
         excludeLibraries: ['css', 'code_in_comments'],
         additionalArguments: ['--no-link-to-remote']));
 
-/// For testing sort behavior.
-class TestLibraryContainer extends LibraryContainer with Nameable {
-  @override
-  final List<String> containerOrder;
-  @override
-  String enclosingName;
-  @override
-  final String name;
-
-  @override
-  bool get isSdk => false;
-  @override
-  PackageGraph get packageGraph => throw UnimplementedError();
-
-  @override
-  Package get package => throw UnimplementedError();
-
-  TestLibraryContainer(
-      this.name, this.containerOrder, LibraryContainer? enclosingContainer)
-      : enclosingName = enclosingContainer?.name ?? '';
-
-  @override
-  DartdocOptionContext get config => throw UnimplementedError();
-
-  @override
-  String? get documentation => throw UnimplementedError();
-
-  @override
-  String get documentationAsHtml => throw UnimplementedError();
-
-  @override
-  bool get hasDocumentation => throw UnimplementedError();
-
-  @override
-  String? get href => throw UnimplementedError();
-
-  @override
-  bool get isDocumented => throw UnimplementedError();
-
-  @override
-  Kind get kind => throw UnimplementedError();
-
-  @override
-  String get oneLineDoc => throw UnimplementedError();
-
-  @override
-  String? get aboveSidebarPath => null;
-
-  @override
-  String? get belowSidebarPath => null;
-}
-
-class TestLibraryContainerSdk extends TestLibraryContainer {
-  TestLibraryContainerSdk(super.name, super.containerOrder,
-      LibraryContainer super.enclosingContainer);
-
-  @override
-  bool get isSdk => true;
-}
-
 void main() async {
   final packageGraph = await testPackageGraph;
   late final Library exLibrary;
@@ -737,64 +676,6 @@ void main() async {
     expect(category.redirectFilePath, 'topics/Superb-topic.html');
   });
 
-  group('LibraryContainer', () {
-    late final TestLibraryContainer topLevel;
-    var sortOrderBasic = ['theFirst', 'second', 'fruit'];
-    var containerNames = [
-      'moo',
-      'woot',
-      'theFirst',
-      'topLevel Things',
-      'toplevel',
-      'fruit'
-    ];
-
-    setUpAll(() {
-      topLevel = TestLibraryContainer('topLevel', [], null);
-    });
-
-    test('multiple containers with specified sort order', () {
-      var containers = <LibraryContainer>[];
-      for (var i = 0; i < containerNames.length; i++) {
-        var name = containerNames[i];
-        containers.add(TestLibraryContainer(name, sortOrderBasic, topLevel));
-      }
-      containers.add(TestLibraryContainerSdk('SDK', sortOrderBasic, topLevel));
-      containers.sort();
-      expect(
-          containers.map((c) => c.name),
-          orderedEquals([
-            'theFirst',
-            'fruit',
-            'toplevel',
-            'SDK',
-            'topLevel Things',
-            'moo',
-            'woot'
-          ]));
-    });
-
-    test('multiple containers, no specified sort order', () {
-      var containers = <LibraryContainer>[];
-      for (var name in containerNames) {
-        containers.add(TestLibraryContainer(name, [], topLevel));
-      }
-      containers.add(TestLibraryContainerSdk('SDK', [], topLevel));
-      containers.sort();
-      expect(
-          containers.map((c) => c.name),
-          orderedEquals([
-            'toplevel',
-            'SDK',
-            'topLevel Things',
-            'fruit',
-            'moo',
-            'theFirst',
-            'woot'
-          ]));
-    });
-  });
-
   group('Library', () {
     late final Library anonLib,
         isDeprecated,

test/packages_test.dart

@@ -277,8 +277,9 @@ library script;
 class Script {}
 ''');
 
-        packageTwoRoot =
-            utils.writePackage('two', resourceProvider, packageConfigProvider);
+        packageTwoRoot = utils.writePackage(
+            'two', resourceProvider, packageConfigProvider,
+            dependencies: ['one']);
         packageConfigProvider.addPackageToConfigFor(
             packageTwoRoot.path, 'one', Uri.file('${packageOneRoot.path}/'));
         packageTwoRoot
@@ -456,6 +457,64 @@ int x;
         expect(packageGraph.localPackages.first.categories, isEmpty);
       });
     });
+
+    group('ordering', () {
+      late Folder defaultRoot;
+
+      Folder bootBasicPackages(List<String> names) {
+        var defaultPackageRoot = utils.writePackage(
+            'default', resourceProvider, packageConfigProvider,
+            dependencies: names);
+
+        for (var name in names) {
+          var root =
+              utils.writePackage(name, resourceProvider, packageConfigProvider);
+          root
+              .getChildAssumingFolder('lib')
+              .getChildAssumingFile('$name.dart')
+              .writeAsStringSync('var a = 1;');
+          packageConfigProvider.addPackageToConfigFor(
+              defaultPackageRoot.path, name, Uri.file('${root.path}/'));
+          defaultPackageRoot
+              .getChildAssumingFolder('lib')
+              .getChildAssumingFile('$name.dart')
+              .writeAsStringSync('''
+import 'package:$name/$name.dart';
+var b = a;
+''');
+        }
+
+        return defaultPackageRoot;
+      }
+
+      test('default package precedes SDK which precedes dependencies',
+          () async {
+        defaultRoot = bootBasicPackages(['bbb', 'aaa']);
+        var packageGraph = await utils.bootBasicPackage(
+            defaultRoot.path, packageMetaProvider, packageConfigProvider,
+            additionalArguments: [
+              '--auto-include-dependencies',
+            ]);
+
+        var sortedPackages = [...packageGraph.localPackages]..sort();
+        expect(sortedPackages.map((c) => c.name),
+            orderedEquals(['default', 'Dart', 'aaa', 'bbb']));
+      });
+
+      test('packages named in --package-order option come first', () async {
+        defaultRoot = bootBasicPackages(['aaa', 'bbb', 'ccc']);
+        var packageGraph = await utils.bootBasicPackage(
+            defaultRoot.path, packageMetaProvider, packageConfigProvider,
+            additionalArguments: [
+              '--auto-include-dependencies',
+              '--package-order=bbb,aaa',
+            ]);
+
+        var sortedPackages = [...packageGraph.localPackages]..sort();
+        expect(sortedPackages.map((c) => c.name),
+            orderedEquals(['bbb', 'aaa', 'default', 'Dart', 'ccc']));
+      });
+    });
   });
 }