Skip to content
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.

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

Correct a regression for a package with a specified set of included libraries #3825

Merged
merged 2 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion lib/src/model/category.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class Category
TopLevelContainer,
Indexable
implements Documentable {
/// All libraries in [libraries] must come from [_package].
/// All libraries in [libraries] must come from [package].
@override
final Package package;

Expand Down
14 changes: 8 additions & 6 deletions lib/src/model/library.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/scope.dart';
import 'package:analyzer/source/line_info.dart';
// ignore: implementation_imports
import 'package:analyzer/src/generated/sdk.dart' show SdkLibrary;
import 'package:dartdoc/src/model/comment_referable.dart';
import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/package_meta.dart' show PackageMeta;
Expand Down Expand Up @@ -107,13 +105,17 @@ class Library extends ModelElement
CompilationUnitElement get compilationUnitElement =>
element.definingCompilationUnit;

SdkLibrary? get _sdkLib =>
packageGraph.sdkLibrarySources[element.librarySource];

@override

/// Whether this library is considered "public."
///
/// A library is considered public if it:
/// * is an SDK library and it is documented and it is not internal, or
/// * is found in a package's top-level 'lib' directory, and
/// not found in it's 'lib/src' directory, and it is not excluded.
bool get isPublic {
if (!super.isPublic) return false;
final sdkLib = _sdkLib;
final sdkLib = packageGraph.sdkLibrarySources[element.librarySource];
if (sdkLib != null && (sdkLib.isInternal || !sdkLib.isDocumented)) {
return false;
}
Expand Down
3 changes: 1 addition & 2 deletions lib/src/model/nameable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ mixin Nameable {
///
/// A "package-public" element satisfies the following requirements:
/// * is not documented with the `@nodoc` directive,
/// * for a library, is found in a package's top-level 'lib' directory, and
/// not found in it's 'lib/src' directory,
/// * for a library, adheres to the documentation for [Library.isPublic],
/// * for a library member, is in a _public_ library's exported namespace, and
/// is not privately named, nor an unnamed extension,
/// * for a container (class, enum, extension, extension type, mixin) member,
Expand Down
10 changes: 2 additions & 8 deletions lib/src/model/package_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,8 @@ class PubPackageBuilder implements PackageBuilder {
if (processedLibraries.contains(resolvedLibrary.element)) {
continue;
}
if (addingSpecials || _shouldIncludeLibrary(resolvedLibrary.element)) {
addLibrary(resolvedLibrary);
processedLibraries.add(resolvedLibrary.element);
}
addLibrary(resolvedLibrary);
processedLibraries.add(resolvedLibrary.element);
}
files.addAll(newFiles);
if (!addingSpecials) {
Expand Down Expand Up @@ -329,10 +327,6 @@ class PubPackageBuilder implements PackageBuilder {
}
}

/// Whether [libraryElement] should be included in the libraries-to-document.
bool _shouldIncludeLibrary(LibraryElement libraryElement) =>
_config.include.isEmpty || _config.include.contains(libraryElement.name);

/// Returns all top level library files in the 'lib/' directory of the given
/// package root directory.
///
Expand Down
33 changes: 22 additions & 11 deletions lib/src/model/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,17 @@ class PackageGraph with CommentReferable, Nameable {
'that `$dartOrFlutter analyze` completes without errors.');
}
var package = Package.fromPackageMeta(packageMeta, this);
var lib = Library.fromLibraryResult(resolvedLibrary, this, package);
package.libraries.add(lib);
_allLibraries[libraryElement.source.fullName] = lib;
var library = Library.fromLibraryResult(resolvedLibrary, this, package);
if (_shouldIncludeLibrary(libraryElement)) {
package.libraries.add(library);
}
_allLibraries[libraryElement.source.fullName] = library;
}

/// Whether [libraryElement] should be included in the libraries-to-document.
bool _shouldIncludeLibrary(LibraryElement libraryElement) =>
config.include.isEmpty || config.include.contains(libraryElement.name);

/// 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
/// list of libraries.
Expand Down Expand Up @@ -346,10 +352,10 @@ class PackageGraph with CommentReferable, Nameable {
/// [libraries].
///
/// 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
/// referring to the same location, and hence 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.
Expand Down Expand Up @@ -556,14 +562,19 @@ class PackageGraph with CommentReferable, Nameable {
}
}

int _lastSizeOfAllLibraries = 0;
int _previousSizeOfAllLibraries = 0;

/// A mapping from a [LibraryElement] to all of the [Library]s that export it,
/// 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;
// The map must be reset if we're still in the middle of adding libraries
// (though this shouldn't happen).
if (_allLibraries.keys.length != _previousSizeOfAllLibraries) {
assert(
_previousSizeOfAllLibraries == 0,
'Re-entered `libraryExports` while building all libraries',
);
_previousSizeOfAllLibraries = _allLibraries.keys.length;
_libraryExports = {};
for (var library in publicLibraries) {
_tagExportsFor(library, library.element);
Expand Down
8 changes: 1 addition & 7 deletions test/element_type_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,9 @@ void main() async {
late FakePackageConfigProvider packageConfigProvider;
late String packagePath;

Future<void> setUpPackage(
String name, {
String? pubspec,
String? analysisOptions,
}) async {
Future<void> setUpPackage(String name) async {
packagePath = await d.createPackage(
name,
pubspec: pubspec,
analysisOptions: analysisOptions,
resourceProvider: resourceProvider,
);

Expand Down
47 changes: 47 additions & 0 deletions test/options_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ class OptionsTest extends DartdocTestBase {
static const packageName = 'test_package';

Future<void> createPackage({
String? pubspec,
String? dartdocOptions,
List<d.Descriptor> libFiles = const [],
List<d.Descriptor> files = const [],
}) async {
packagePath = await d.createPackage(
packageName,
pubspec: pubspec,
dartdocOptions: dartdocOptions,
libFiles: libFiles,
files: files,
Expand Down Expand Up @@ -181,6 +183,51 @@ class Baz {}
orderedEquals(['library_1', 'library_2']));
}

void test_includeOption_referringToNotIncluded() async {
await createPackage(
pubspec: '''
name: options
version: 0.0.1
environment:
sdk: '>=3.4.0 <4.0.0'
dependencies:
http:
path: vendor/http
''',
dartdocOptions: '''
dartdoc:
include: ["library_1"]
''',
libFiles: [
d.file('library_1.dart', '''
library library_1;
import 'package:http/http.dart' as http;
class Foo {
http.Client? client;
}
'''),
],
files: [
d.dir('vendor', [
d.dir('http', [
d.dir('lib', [d.file('http.dart', 'class Client {}')])
])
])
],
);
final packageGraph = await bootBasicPackage(
packagePath,
packageMetaProvider,
packageConfigProvider,
);
expect(packageGraph.localPublicLibraries.map((l) => l.name),
orderedEquals(['library_1']));
final foo =
packageGraph.localPackages.first.libraries.first.classes.named('Foo');
// The name is not linked, but also does not error.
expect(foo.allFields.first.modelType.linkedName, 'Client?');
}

void test_includeCommandLineOption_overridesOptionsFileOption() async {
await createPackage(
dartdocOptions: '''
Expand Down
5 changes: 4 additions & 1 deletion test/src/test_descriptor_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ Future<String> createPackage(
final dependencies = parsedYaml['dependencies'] as Map;
for (var dep in dependencies.keys) {
// This only accepts 'path' deps.
final depConfig = dependencies[dep] as Map;
final depConfig = dependencies[dep];
if (depConfig is! Map) {
throw StateError('dep in pubspec must be a Map, but is: "$depConfig"');
}
final pathDep = depConfig['path'];

packagesInfo.writeln(''',{
Expand Down