Skip to content

Fix many library sidebars, and search #3865

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

Merged
merged 2 commits into from
Sep 3, 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
5 changes: 5 additions & 0 deletions lib/src/model/canonicalization.dart
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ final class Canonicalization {
scoredCandidate._alterScore(1.0, _Reason.packageName);
}

// Same idea as the above, for the Dart SDK.
if (library.name == 'dart:core') {
scoredCandidate._alterScore(0.9, _Reason.packageName);
}

// Give a tiny boost for libraries with long names, assuming they're
// more specific (and therefore more likely to be the owner of this symbol).
scoredCandidate._alterScore(
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/container.dart
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ abstract class Container extends ModelElement
String get sidebarPath;

@override
String get aboveSidebarPath => library.sidebarPath;
String get aboveSidebarPath => canonicalLibraryOrThrow.sidebarPath;

@override
String get belowSidebarPath => sidebarPath;
Expand Down
17 changes: 17 additions & 0 deletions lib/src/model/model_element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,22 @@ abstract class ModelElement
!(enclosingElement as Extension).isPublic) {
return false;
}

if (element case LibraryElement(:var identifier, :var source)) {
// Private Dart SDK libraries are not public.
if (identifier.startsWith('dart:_') ||
identifier.startsWith('dart:nativewrappers/') ||
'dart:nativewrappers' == identifier) {
return false;
}
// Package-private libraries are not public.
var elementUri = source.uri;
if (elementUri.scheme == 'package' &&
elementUri.pathSegments[1] == 'src') {
return false;
}
}

return !element.hasPrivateName && !hasNodoc;
}();

Expand Down Expand Up @@ -518,6 +534,7 @@ abstract class ModelElement
final candidateLibraries = thisAndExported.where((l) {
if (!l.isPublic) return false;
if (l.package.documentedWhere == DocumentLocation.missing) return false;
if (this is Library) return true;
var lookup = l.element.exportNamespace.definedNames[topLevelElementName];
return topLevelElement ==
(lookup is PropertyAccessorElement ? lookup.variable2 : lookup);
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/model_function.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class ModelFunctionTyped extends ModelElement with TypeParameters {
String get filePath => '${canonicalLibrary?.dirName}/$fileName';

@override
String get aboveSidebarPath => enclosingElement.sidebarPath;
String get aboveSidebarPath => canonicalLibraryOrThrow.sidebarPath;

@override
String? get belowSidebarPath => null;
Expand Down
3 changes: 3 additions & 0 deletions lib/src/model/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ class PackageGraph with CommentReferable, Nameable {
// that might not _have_ a canonical [ModelElement], too.
if (element.isCanonical ||
element.canonicalModelElement == null ||
element is Library ||
element.enclosingElement!.isCanonical) {
for (var d in element.documentationFrom
.where((d) => d.hasDocumentationComment)) {
Expand Down Expand Up @@ -771,6 +772,8 @@ class PackageGraph with CommentReferable, Nameable {
if (canonicalClass != null) preferredClass = canonicalClass;
}
var lib = modelElement.canonicalLibrary;
if (modelElement is Library) return lib;

if (lib == null && preferredClass != null) {
lib = preferredClass.canonicalLibrary;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/top_level_variable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class TopLevelVariable extends ModelElement
String get filePath => '${canonicalLibraryOrThrow.dirName}/$fileName';

@override
String get aboveSidebarPath => enclosingElement.sidebarPath;
String get aboveSidebarPath => canonicalLibraryOrThrow.sidebarPath;

@override
String? get belowSidebarPath => null;
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/typedef.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ abstract class Typedef extends ModelElement
String get filePath => '${canonicalLibraryOrThrow.dirName}/$fileName';

@override
String get aboveSidebarPath => enclosingElement.sidebarPath;
String get aboveSidebarPath => canonicalLibraryOrThrow.sidebarPath;

@override
String? get belowSidebarPath => null;
Expand Down
13 changes: 0 additions & 13 deletions lib/src/model_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,19 +72,6 @@ extension ElementExtension on Element {
return true;
}
}
if (self is LibraryElement) {
if (self.identifier.startsWith('dart:_') ||
self.identifier.startsWith('dart:nativewrappers/') ||
'dart:nativewrappers' == self.identifier) {
return true;
}
var elementUri = self.source.uri;
// TODO(jcollins-g): Implement real cross package detection.
if (elementUri.scheme == 'package' &&
elementUri.pathSegments[1] == 'src') {
return true;
}
}
return false;
}
}
Expand Down
20 changes: 12 additions & 8 deletions test/dartdoc_test_base.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ abstract class DartdocTestBase {

String get linkPrefix => '$placeholder$libraryName';

String get dartAsyncUrlPrefix =>
'https://api.dart.dev/stable/3.2.0/dart-async';

String get dartCoreUrlPrefix => 'https://api.dart.dev/stable/3.2.0/dart-core';

String get sdkConstraint => '>=3.3.0 <4.0.0';
Expand Down Expand Up @@ -99,20 +102,21 @@ analyzer:
/// Creates a single library named [libraryName], with optional preamble
/// [libraryPreamble]. Optionally, pass [extraFiles] such as
/// `dartdoc_options.yaml`.
Future<Library> bootPackageWithLibrary(String libraryContent,
{String libraryPreamble = '',
Iterable<d.Descriptor> extraFiles = const [],
List<String> additionalArguments = const []}) async {
Future<Library> bootPackageWithLibrary(
String libraryContent, {
String libraryFilePath = 'lib/library.dart',
String libraryPreamble = '',
Iterable<d.Descriptor> extraFiles = const [],
List<String> additionalArguments = const [],
}) async {
return (await bootPackageFromFiles([
d.dir('lib', [
d.file('lib.dart', '''
d.file(libraryFilePath, '''
$libraryPreamble
library $libraryName;

$libraryContent
'''),
]),
...extraFiles
...extraFiles,
], additionalArguments: additionalArguments))
.libraries
.named(libraryName);
Expand Down
42 changes: 40 additions & 2 deletions test/libraries_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,19 @@
import 'package:analyzer/file_system/memory_file_system.dart';
import 'package:dartdoc/src/model/model.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

import 'dartdoc_test_base.dart';
import 'src/test_descriptor_utils.dart' as d;
import 'src/utils.dart';

// TODO(srawlins): Migrate to test_reflective_loader tests.

void main() {
defineReflectiveSuite(() {
defineReflectiveTests(LibrariesTest);
});

// TODO(srawlins): Migrate to test_reflective_loader tests.

test('A named library', () async {
var packageMetaProvider = testPackageMetaProvider;

Expand Down Expand Up @@ -169,3 +175,35 @@ A doc comment.
'${htmlBasePlaceholder}dart-async/dart-async-library.html');
}, onPlatform: {'windows': Skip('Test does not work on Windows (#2446)')});
}

@reflectiveTest
class LibrariesTest extends DartdocTestBase {
@override
String get libraryName => 'libraries';

void test_publicLibrary() async {
var library = await bootPackageWithLibrary(
'var x = 1;',
libraryFilePath: 'lib/library.dart',
);

expect(library.qualifiedName, 'libraries');
expect(library.href, '${placeholder}libraries/libraries-library.html');
}

void test_exportedLibrary() async {
var library = await bootPackageWithLibrary(
'var x = 1;',
libraryFilePath: 'lib/src/library.dart',
extraFiles: [
d.dir('lib', [
d.file('public.dart', '''
export 'src/library.dart';
''')
]),
],
);
expect(library.qualifiedName, 'libraries');
expect(library.href, '${placeholder}public/public-library.html');
}
}
13 changes: 8 additions & 5 deletions test/prefixes_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,14 @@ int x = 0;
''',
additionalArguments: ['--link-to-remote'],
);
var f = library.properties.named('x');
// There is no link, but also no wrong link or crash.
expect(f.documentationAsHtml, '<p>Text <code>async</code>.</p>');
var x = library.properties.named('x');
expect(
x.documentationAsHtml,
'<p>Text <a href="$dartAsyncUrlPrefix/dart-async-library.html">async</a>.</p>',
);
}

@FailingTest(issue: 'https://github.com/dart-lang/dartdoc/issues/3769')
void test_referenced_wildcard() async {
var library = await bootPackageWithLibrary(
'''
Expand All @@ -44,8 +47,8 @@ int x = 0;
''',
additionalArguments: ['--link-to-remote'],
);
var f = library.properties.named('x');
var x = library.properties.named('x');
// There is no link, but also no wrong link or crash.
expect(f.documentationAsHtml, '<p>Text <code>_</code>.</p>');
expect(x.documentationAsHtml, '<p>Text <code>_</code>.</p>');
}
}
3 changes: 2 additions & 1 deletion test/src/test_descriptor_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ extension on d.FileDescriptor {
Future<String> createInMemory(
MemoryResourceProvider resourceProvider, String parent) async {
var content = await readAsBytes().transform(utf8.decoder).join('');
var fullPath = resourceProvider.pathContext.join(parent, name);
var fullPath = resourceProvider
.convertPath(resourceProvider.pathContext.join(parent, name));
resourceProvider.newFile(fullPath, content);
return fullPath;
}
Expand Down