Skip to content

Commit

Permalink
Refactor how Object and Pragma are discovered and handled (#3929)
Browse files Browse the repository at this point in the history
  • Loading branch information
srawlins authored Nov 15, 2024
1 parent 6bbd3d7 commit fa339a1
Show file tree
Hide file tree
Showing 12 changed files with 46 additions and 171 deletions.
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 {
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

0 comments on commit fa339a1

Please sign in to comment.