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

Refactor how Object and Pragma are discovered and handled #3929

Merged
merged 2 commits into from
Nov 15, 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
11 changes: 9 additions & 2 deletions lib/src/generator/templates.runtime_renderers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2885,6 +2885,13 @@ class _Renderer_Container extends RendererBase<Container> {
_render_Operator(e, ast, r.template, sink, parent: r));
},
),
'isDartCoreObject': Property(
getValue: (CT_ c) => c.isDartCoreObject,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(c, remainingNames, 'bool'),
getBool: (CT_ c) => c.isDartCoreObject,
),
'isEnum': Property(
getValue: (CT_ c) => c.isEnum,
renderVariable: (CT_ c, Property<CT_> self,
Expand Down Expand Up @@ -16371,6 +16378,7 @@ const _invisibleGetters = {
'localPackages',
'localPublicLibraries',
'name',
'objectClass',
'packageGraph',
'packageMap',
'packageMeta',
Expand All @@ -16383,8 +16391,7 @@ const _invisibleGetters = {
'referenceParents',
'resourceProvider',
'runtimeType',
'sdkLibrarySources',
'specialClasses'
'sdkLibrarySources'
},
'PackageMeta': {
'dir',
Expand Down
6 changes: 5 additions & 1 deletion lib/src/model/class.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ class Class extends InheritingContainer with Constructable, MixedInTypes {

Class(this.element, Library library, PackageGraph packageGraph)
: super(library, packageGraph) {
packageGraph.specialClasses.addSpecial(this);
if (element.name == 'Object' &&
library.element.name == 'dart.core' &&
package.name == 'Dart') {
packageGraph.objectClass = this;
}
}

@override
Expand Down
4 changes: 4 additions & 0 deletions lib/src/model/container.dart
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ abstract class Container extends ModelElement
/// Whether this is a mixin.
bool get isMixin => element is MixinElement;

/// Whether this container represents the Object class from 'dart:core'.
bool get isDartCoreObject =>
element.name == 'Object' && element.library?.name == 'dart.core';

/// The model elements of all of the members of this container, including
/// declared and inherited ones.
Iterable<ModelElement> get allModelElements => [
Expand Down
8 changes: 3 additions & 5 deletions lib/src/model/inheritable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import 'package:collection/collection.dart' show IterableExtension;
import 'package:dartdoc/src/model/attribute.dart';
import 'package:dartdoc/src/model/comment_referable.dart';
import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/special_elements.dart';

/// Mixin for subclasses of [ModelElement] representing elements that can be
/// inherited from one class to another.
Expand Down Expand Up @@ -136,10 +135,9 @@ mixin Inheritable on ContainerMember {
var inheritance = [
...(enclosingElement as InheritingContainer).inheritanceChain,
];
var object = packageGraph.specialClasses[SpecialClass.object]!;

assert(
definingEnclosingContainer == object ||
definingEnclosingContainer.isDartCoreObject ||
inheritance.contains(definingEnclosingContainer), () {
var inheritanceDescriptions = inheritance
.map((e) =>
Expand All @@ -153,8 +151,8 @@ mixin Inheritable on ContainerMember {
}());
// Unless the code explicitly extends dart:core's Object, we won't get
// an entry here. So add it.
if (inheritance.last != object) {
inheritance.add(object);
if (!inheritance.last.isDartCoreObject) {
inheritance.add(packageGraph.objectClass);
}
return inheritance;
}
Expand Down
4 changes: 1 addition & 3 deletions lib/src/model/mixin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:dartdoc/src/model/comment_referable.dart';
import 'package:dartdoc/src/model/kind.dart';
import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/model_utils.dart' as model_utils;
import 'package:dartdoc/src/special_elements.dart';
import 'package:meta/meta.dart';

class Mixin extends InheritingContainer {
Expand All @@ -20,8 +19,7 @@ class Mixin extends InheritingContainer {
...element.superclassConstraints
.map((InterfaceType i) =>
getTypeFor(i, library) as ParameterizedElementType)
.where((t) =>
t.modelElement != packageGraph.specialClasses[SpecialClass.object])
.where((t) => t.modelElement != packageGraph.objectClass)
];

@override
Expand Down
23 changes: 17 additions & 6 deletions lib/src/model/model_element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import 'package:analyzer/source/line_info.dart';
// ignore: implementation_imports
import 'package:analyzer/src/dart/element/member.dart'
show ExecutableMember, Member, ParameterMember;
import 'package:collection/collection.dart';
import 'package:dartdoc/src/dartdoc_options.dart';
import 'package:dartdoc/src/model/annotation.dart';
import 'package:dartdoc/src/model/attribute.dart';
Expand All @@ -27,7 +26,6 @@ import 'package:dartdoc/src/model_utils.dart';
import 'package:dartdoc/src/render/parameter_renderer.dart';
import 'package:dartdoc/src/runtime_stats.dart';
import 'package:dartdoc/src/source_linker.dart';
import 'package:dartdoc/src/special_elements.dart';
import 'package:dartdoc/src/type_utils.dart';
import 'package:dartdoc/src/warnings.dart';
import 'package:meta/meta.dart';
Expand Down Expand Up @@ -385,10 +383,7 @@ abstract class ModelElement
/// invalid code from analyzer's perspective, some are present in `sky_engine`
/// (`@Native`) so we don't want to crash here.
late final List<Annotation> annotations = element.metadata
.whereNot((m) =>
m.element == null ||
packageGraph.specialClasses[SpecialClass.pragma]!.element.constructors
.contains(m.element))
.where((m) => m.isVisibleAnnotation)
.map((m) => Annotation(m, library, packageGraph))
.toList(growable: false);

Expand Down Expand Up @@ -791,3 +786,19 @@ abstract class ModelElement

String get linkedObjectType => _packageGraph.dartCoreObject;
}

extension on ElementAnnotation {
/// Whether this annotation should be displayed in documentation.
///
/// At the moment, `pragma` is the only invisible annotation.
bool get isVisibleAnnotation {
srawlins marked this conversation as resolved.
Show resolved Hide resolved
if (element == null) return false;

if (element case ConstructorElement(:var enclosingElement3)) {
return !(enclosingElement3.name == 'pragma' &&
enclosingElement3.library.name == 'dart.core');
}

return true;
}
}
14 changes: 1 addition & 13 deletions lib/src/model/package_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import 'package:dartdoc/src/package_config_provider.dart';
import 'package:dartdoc/src/package_meta.dart'
show PackageMeta, PackageMetaProvider;
import 'package:dartdoc/src/runtime_stats.dart';
import 'package:dartdoc/src/special_elements.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as p show Context;

Expand Down Expand Up @@ -479,13 +478,7 @@ class PubPackageBuilder implements PackageBuilder {
/// Adds all libraries with documentable elements to
/// [uninitializedPackageGraph].
Future<void> _getLibraries(PackageGraph uninitializedPackageGraph) async {
var embedderSdk = _embedderSdk;
var findSpecialsSdk = switch (embedderSdk) {
EmbedderSdk(:var urlMappings) when urlMappings.isNotEmpty => embedderSdk,
_ => _sdk,
};
var files = await _getFilesToDocument();
var specialFiles = specialLibraryFiles(findSpecialsSdk);

logInfo('Discovering libraries...');
var foundLibraries = <LibraryElement>{};
Expand All @@ -495,12 +488,7 @@ class PubPackageBuilder implements PackageBuilder {
files,
);
_checkForMissingIncludedFiles(foundLibraries);
await _discoverLibraries(
uninitializedPackageGraph.addSpecialLibraryToGraph,
foundLibraries,
specialFiles.difference(files),
addingSpecials: true,
);
uninitializedPackageGraph.allLibrariesAdded = true;
}

/// Throws an exception if any configured-to-be-included files were not found
Expand Down
40 changes: 4 additions & 36 deletions lib/src/model/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/model_utils.dart' as utils;
import 'package:dartdoc/src/package_meta.dart'
show PackageMeta, PackageMetaProvider;
import 'package:dartdoc/src/special_elements.dart';
import 'package:dartdoc/src/tool_definition.dart';
import 'package:dartdoc/src/tool_runner.dart';
import 'package:dartdoc/src/warnings.dart';
Expand Down Expand Up @@ -118,29 +117,6 @@ class PackageGraph with CommentReferable, Nameable {
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.
///
/// Call during initialization to add a library possibly containing
/// special/non-documented elements to this [PackageGraph]. Must be called
/// after any normal libraries.
void addSpecialLibraryToGraph(DartDocResolvedLibrary resolvedLibrary) {
allLibrariesAdded = true;
assert(!_localDocumentationBuilt);
final libraryElement = resolvedLibrary.element.library;
_allLibraries.putIfAbsent(
libraryElement.source.fullName,
() => Library.fromLibraryResult(
resolvedLibrary,
this,
Package.fromPackageMeta(
packageMetaProvider.fromElement(libraryElement, config.sdkDir)!,
packageGraph),
),
);
}

/// Call after all libraries are added.
Future<void> initializePackageGraph() async {
assert(!_localDocumentationBuilt);
Expand Down Expand Up @@ -170,9 +146,6 @@ class PackageGraph with CommentReferable, Nameable {
}
allImplementersAdded = true;
allExtensionsAdded = true;

// We should have found all special classes by now.
specialClasses.assertSpecials();
}

/// Generate a list of futures for any docs that actually require precaching.
Expand Down Expand Up @@ -230,8 +203,8 @@ class PackageGraph with CommentReferable, Nameable {
// more than once for them.
final Map<Element, ModelNode> _modelNodes = {};

/// The collection of "special" classes for which we need some special access.
final specialClasses = SpecialClasses();
/// The Object class declared in the Dart SDK's 'dart:core' library.
late InheritingContainer objectClass;

/// Populate's [_modelNodes] with elements in [resolvedLibrary].
///
Expand Down Expand Up @@ -706,14 +679,9 @@ class PackageGraph with CommentReferable, Nameable {
?.linkedName ??
'Object';

/// The set of [Class] objects that are similar to 'pragma' in that we should
/// never count them as documentable annotations.
late final Set<Class> _invisibleAnnotations = {
if (specialClasses[SpecialClass.pragma] case var pragma?) pragma,
};

bool isAnnotationVisible(Class class_) =>
!_invisibleAnnotations.contains(class_);
class_.element.name == 'pragma' &&
class_.element.library.name == 'dart.core';

@override
String toString() {
Expand Down
84 changes: 0 additions & 84 deletions lib/src/special_elements.dart

This file was deleted.

4 changes: 1 addition & 3 deletions test/classes_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:dartdoc/src/special_elements.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

Expand Down Expand Up @@ -194,9 +193,8 @@ class C implements D {}
class D implements Object {}
''');

var object = library.packageGraph.specialClasses[SpecialClass.object]!;
var toString = library.classes.named('A').instanceMethods.named('toString');
expect(toString.canonicalEnclosingContainer, object);
expect(toString.canonicalEnclosingContainer!.isDartCoreObject, isTrue);
}

// TODO(srawlins): Test everything else about classes.
Expand Down
5 changes: 1 addition & 4 deletions test/end2end/model_special_cases_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/model_utils.dart';
import 'package:dartdoc/src/package_config_provider.dart';
import 'package:dartdoc/src/package_meta.dart';
import 'package:dartdoc/src/special_elements.dart';
import 'package:html/parser.dart' as html;
import 'package:test/test.dart';

Expand Down Expand Up @@ -412,8 +411,6 @@ void main() {
htmlLibrary.classes.singleWhere((c) => c.name == 'EventTarget');
var hashCode = eventTarget.instanceFields.wherePublic
.singleWhere((f) => f.name == 'hashCode');
var objectModelElement =
sdkAsPackageGraph.specialClasses[SpecialClass.object];
expect(
eventTarget.superChain,
contains(isA<ParameterizedElementType>()
Expand All @@ -429,7 +426,7 @@ void main() {
"EventTarget appears to have an explicit override of 'hashCode', "
'which makes this test case invalid.',
);
expect(hashCode.canonicalEnclosingContainer, equals(objectModelElement));
expect(hashCode.canonicalEnclosingContainer!.isDartCoreObject, isTrue);
expect(
eventTarget.publicSuperChainReversed
.any((et) => et.name == 'Interceptor'),
Expand Down
Loading