Prevent crashes/corruption from inability to detect special objects in the right order. (#1729)
* dartfmt
* Fix crash in angular, rearrange special object handling, and test angular doc builds
* dartfmt
* Pin to .69 until other changes go through
* Pin appveyor too
* Fix CI typos
* Fix problem with embeddeders that don't provide Interceptor (mark it optional)
* Fix Flutter analysis errors from forced-load Interceptor attempt
* Check for null embedder sdk
diff --git a/.travis.yml b/.travis.yml
index 794905e..d4b915b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,11 +1,10 @@
language: dart
sudo: false
dart:
- - "dev/raw/latest"
+ - "dev/raw/2.0.0-dev.69.0"
env:
- DARTDOC_BOT=main
- # TODO(devoncarew): add angulardart support
- #- DARTDOC_BOT=angular
+ - DARTDOC_BOT=packages
- DARTDOC_BOT=flutter
- DARTDOC_BOT=sdk-docs
script: ./tool/travis.sh
diff --git a/analysis_options.yaml b/analysis_options.yaml
index ee60de8..83921c5 100644
--- a/analysis_options.yaml
+++ b/analysis_options.yaml
@@ -1,7 +1,5 @@
analyzer:
- strong-mode: true
language:
- enableGenericMethods: true
enablePreviewDart2: true
enableSuperMixins: true
exclude:
diff --git a/appveyor.yml b/appveyor.yml
index d4bd319..8cd0e15 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -3,7 +3,7 @@
# BSD-style license that can be found in the LICENSE file.
install:
- - ps: wget https://storage.googleapis.com/dart-archive/channels/dev/raw/latest/sdk/dartsdk-windows-x64-release.zip -OutFile dart-sdk.zip
+ - ps: wget https://storage.googleapis.com/dart-archive/channels/dev/raw/2.0.0-dev.69.0/sdk/dartsdk-windows-x64-release.zip -OutFile dart-sdk.zip
- cmd: echo "Unzipping dart-sdk..."
- cmd: 7z x dart-sdk.zip -o"C:\tools" -y > nul
- set PATH=%PATH%;C:\tools\dart-sdk\bin
diff --git a/lib/src/model.dart b/lib/src/model.dart
index 530a9d6..7ed5a5d 100644
--- a/lib/src/model.dart
+++ b/lib/src/model.dart
@@ -44,6 +44,7 @@
import 'package:dartdoc/src/markdown_processor.dart' show Documentation;
import 'package:dartdoc/src/model_utils.dart';
import 'package:dartdoc/src/package_meta.dart' show PackageMeta, FileContents;
+import 'package:dartdoc/src/special_elements.dart';
import 'package:dartdoc/src/utils.dart';
import 'package:dartdoc/src/warnings.dart';
import 'package:front_end/src/byte_store/byte_store.dart';
@@ -180,18 +181,17 @@
List<Class> get inheritance {
List<Class> inheritance = [];
inheritance.addAll((enclosingElement as Class).inheritanceChain);
+ Class object = packageGraph.specialClasses[SpecialClass.object];
if (!inheritance.contains(definingEnclosingElement) &&
definingEnclosingElement != null) {
- assert(definingEnclosingElement == packageGraph.objectElement);
+ assert(definingEnclosingElement == object);
}
// Unless the code explicitly extends dart-core's Object, we won't get
// an entry here. So add it.
- if (inheritance.last != packageGraph.objectElement &&
- packageGraph.objectElement != null) {
- inheritance.add(packageGraph.objectElement);
+ if (inheritance.last != object && object != null) {
+ inheritance.add(object);
}
- assert(
- inheritance.where((e) => e == packageGraph.objectElement).length == 1);
+ assert(inheritance.where((e) => e == object).length == 1);
return inheritance;
}
}
@@ -1629,7 +1629,7 @@
buffer.write('${getter.oneLineDoc}');
}
if (hasPublicSetter && setter.oneLineDoc.isNotEmpty) {
- buffer.write('${getterSetterBothAvailable ? "": setter.oneLineDoc}');
+ buffer.write('${getterSetterBothAvailable ? "" : setter.oneLineDoc}');
}
_oneLineDoc = buffer.toString();
}
@@ -2587,21 +2587,6 @@
if (e is ClassElement) {
if (!e.isEnum) {
newModelElement = new Class(e, library, packageGraph);
- if (newModelElement.name == 'Object' &&
- newModelElement.library.name == 'dart:core') {
- // We've found Object. This is an important object, so save it in the package.
- assert(
- newModelElement.library.packageGraph._objectElement == null);
- newModelElement.library.packageGraph._objectElement =
- newModelElement;
- }
- if (newModelElement.name == 'Interceptor' &&
- newModelElement.library.name == 'dart:_interceptors') {
- // We've found Interceptor. Another important object.
- assert(!newModelElement.library.packageGraph._interceptorUsed);
- newModelElement.library.packageGraph.interceptor =
- newModelElement;
- }
} else {
newModelElement = new Enum(e, library, packageGraph);
}
@@ -4017,8 +4002,19 @@
// TODO(jcollins-g): This constructor is convoluted. Clean this up by
// building Libraries and adding them to Packages, then adding Packages
// to this graph.
- PackageGraph(Iterable<LibraryElement> libraryElements, this.config,
- this.packageMeta, this._packageWarningOptions, this.driver, this.sdk) {
+
+ /// Construct a package graph.
+ /// [libraryElements] - Libraries to be documented.
+ /// [specialLibraryElements] - Any libraries that may not be documented, but
+ /// contain required [SpecialClass]es.
+ PackageGraph(
+ Iterable<LibraryElement> libraryElements,
+ Iterable<LibraryElement> specialLibraryElements,
+ this.config,
+ this.packageMeta,
+ this._packageWarningOptions,
+ this.driver,
+ this.sdk) {
assert(_allConstructedModelElements.isEmpty);
assert(allLibraries.isEmpty);
_packageWarningCounter = new PackageWarningCounter(_packageWarningOptions);
@@ -4042,11 +4038,19 @@
new Package.fromPackageMeta(packageMeta, this);
allLibrariesAdded = true;
+ // [findOrCreateLibraryFor] already adds to the proper structures.
+ specialLibraryElements.forEach((element) {
+ findOrCreateLibraryFor(element);
+ });
+
// Go through docs of every ModelElement in package to pre-build the macros
// index.
allLocalModelElements.forEach((m) => m.documentationLocal);
_macrosAdded = true;
+ // Scan all model elements to insure that interceptor and other special
+ // objects are found.
+ specialClasses = new SpecialClasses(this);
// After the allModelElements traversal to be sure that all packages
// are picked up.
documentedPackages.toList().forEach((package) {
@@ -4059,6 +4063,8 @@
allImplementorsAdded = true;
}
+ SpecialClasses specialClasses;
+
/// It is safe to cache values derived from the _implementors table if this
/// is true.
bool allImplementorsAdded = false;
@@ -4497,31 +4503,6 @@
return _localPublicLibraries;
}
- // Written from ModelElement.from.
- ModelElement _objectElement;
-
- // Return the element for "Object".
- ModelElement get objectElement {
- assert(_objectElement != null);
- return _objectElement;
- }
-
- // Don't let this be used for canonicalization before we find it.
- bool _interceptorUsed = false;
- Class _interceptor;
-
- /// Return the element for "Interceptor", a Dart implementation class intended
- /// to function the same as Object.
- Class get interceptor {
- _interceptorUsed = true;
- return _interceptor;
- }
-
- set interceptor(Class newInterceptor) {
- assert(_interceptorUsed == false);
- _interceptor = newInterceptor;
- }
-
// Return the set of [Class]es objects should inherit through if they
// show up in the inheritance chain. Do not call before interceptorElement is
// found. Add classes here if they are similar to Interceptor in that they
@@ -4531,7 +4512,7 @@
Set<Class> get inheritThrough {
if (_inheritThrough == null) {
_inheritThrough = new Set();
- _inheritThrough.add(interceptor);
+ _inheritThrough.add(specialClasses[SpecialClass.interceptor]);
}
return _inheritThrough;
}
@@ -4746,16 +4727,16 @@
return foundLibrary;
}
- List<ModelElement> _allModelElements;
+ List<ModelElement> _allLocalModelElements;
Iterable<ModelElement> get allLocalModelElements {
assert(allLibrariesAdded);
- if (_allModelElements == null) {
- _allModelElements = [];
+ if (_allLocalModelElements == null) {
+ _allLocalModelElements = [];
this.localLibraries.forEach((library) {
- _allModelElements.addAll(library.allModelElements);
+ _allLocalModelElements.addAll(library.allModelElements);
});
}
- return _allModelElements;
+ return _allLocalModelElements;
}
List<ModelElement> _allCanonicalModelElements;
@@ -5512,9 +5493,16 @@
if (packageMeta.needsPubGet) {
packageMeta.runPubGet();
}
- Set<LibraryElement> libraries = await getLibraries(getFiles);
- return new PackageGraph(libraries, config, config.topLevelPackageMeta,
- getWarningOptions(), driver, sdk);
+ Set<LibraryElement> libraries = new Set();
+ Set<LibraryElement> specialLibraries = new Set();
+ DartSdk findSpecialsSdk = sdk;
+ if (embedderSdk != null && embedderSdk.urlMappings.isNotEmpty) {
+ findSpecialsSdk = embedderSdk;
+ }
+ await getLibraries(libraries, specialLibraries, getFiles,
+ specialLibraryFiles(findSpecialsSdk).toSet());
+ return new PackageGraph(libraries, specialLibraries, config,
+ config.topLevelPackageMeta, getWarningOptions(), driver, sdk);
}
DartSdk _sdk;
@@ -5711,7 +5699,8 @@
return metas;
}
- Future<List<LibraryElement>> _parseLibraries(Set<String> files) async {
+ Future<List<LibraryElement>> _parseLibraries(Set<String> files,
+ {bool throwErrors = true}) async {
Set<LibraryElement> libraries = new Set();
Set<Source> originalSources;
Set<Source> sources = new Set<Source>();
@@ -5724,8 +5713,8 @@
driver.addFile(filename);
addedFiles.add(filename);
});
- await Future
- .wait(files.map((f) => processLibrary(f, libraries, sources)));
+ await Future.wait(
+ files.map((f) => processLibrary(f, libraries, sources)));
/// We don't care about upstream analysis errors, so save the first
/// source list.
@@ -5747,7 +5736,7 @@
}
} while (!lastPass.containsAll(current));
- await logAnalysisErrors(originalSources);
+ if (throwErrors) await logAnalysisErrors(originalSources);
return libraries.toList();
}
@@ -5824,9 +5813,18 @@
return new Set.from(files.map((s) => new File(s).absolute.path));
}
- Future<Set<LibraryElement>> getLibraries(Set<String> files) async {
- Set<LibraryElement> libraries = new Set();
+ Future<void> getLibraries(
+ Set<LibraryElement> libraries,
+ Set<LibraryElement> specialLibraries,
+ Set<String> files,
+ Set<String> specialFiles) async {
libraries.addAll(await _parseLibraries(files));
+
+ /// Flutter doesn't seem to like being given the Interceptor library.
+ /// But it doesn't need it, either. So just skip reporting errors here.
+ specialLibraries.addAll(await _parseLibraries(
+ specialFiles.difference(files),
+ throwErrors: false));
if (config.include.isNotEmpty) {
Iterable knownLibraryNames = libraries.map((l) => l.name);
Set notFound = new Set.from(config.include)
@@ -5838,7 +5836,6 @@
}
libraries.removeWhere((lib) => !config.include.contains(lib.name));
}
- return libraries;
}
/// If [dir] contains both a `lib` directory and a `pubspec.yaml` file treat
diff --git a/lib/src/special_elements.dart b/lib/src/special_elements.dart
new file mode 100644
index 0000000..ec72722
--- /dev/null
+++ b/lib/src/special_elements.dart
@@ -0,0 +1,126 @@
+// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
+// 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.
+
+/// Handling for special elements within Dart. When identified these
+/// may alter the interpretation and documentation generated for other
+/// [ModelElement]s.
+///
+/// Finding these must not depend on canonicalization.
+library dartdoc.special_elements;
+
+import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/src/generated/sdk.dart';
+import 'package:dartdoc/src/model.dart';
+
+/// Which of the [SpecialClasses] to get.
+enum SpecialClass {
+ /// From dart:core, Object
+ object,
+
+ /// From dart:_interceptors, Interceptor
+ interceptor,
+}
+
+/// A declaration of a special [Class] and how to find it.
+class _SpecialClassDefinition {
+ /// Which specialElement this is.
+ final SpecialClass specialClass;
+
+ /// Name of the ModelElement.
+ final String name;
+
+ /// The library name for the [LibraryElement] in which this [ModelElement]
+ /// can be found.
+ final String libraryName;
+
+ /// The package name in which this [ModelElement] can be found.
+ final String packageName;
+
+ /// The URI for the library in which this [ModelElement] is defined.
+ final String specialFileUri;
+
+ /// If true, require this element to exist in the packageGraph when
+ /// calling the [SpecialClasses] constructor.
+ final bool required;
+ _SpecialClassDefinition(
+ this.specialClass, this.name, this.libraryName, this.specialFileUri,
+ {this.required = true, this.packageName = 'Dart'});
+
+ /// Get the filename for the Dart Library where this [specialClass] is
+ /// declared.
+ String getSpecialFilename(DartSdk sdk) =>
+ sdk.mapDartUri(specialFileUri)?.fullName;
+
+ bool matchesClass(Class modelClass) {
+ return modelClass.name == name &&
+ modelClass.library.element.name == libraryName &&
+ modelClass.package.name == packageName;
+ }
+}
+
+/// List all special classes we need to find here.
+final List<_SpecialClassDefinition> _specialClassDefinitions = [
+ new _SpecialClassDefinition(
+ SpecialClass.object, 'Object', 'dart.core', 'dart:core'),
+ new _SpecialClassDefinition(SpecialClass.interceptor, 'Interceptor',
+ '_interceptors', 'dart:_interceptors',
+ required: false),
+];
+
+/// Given a SDK, resolve URIs for the libraries containing our special
+/// clases.
+Set<String> specialLibraryFiles(DartSdk sdk) => _specialClassDefinitions
+ .map((_SpecialClassDefinition d) => d.getSpecialFilename(sdk))
+ .where((String s) => s != null)
+ .toSet();
+
+Set<String> __specialLibraryNames;
+
+/// These library names can be checked against the [LibraryElement] names
+/// to avoid traversing libraries we don't need to.
+Set<String> get _specialLibraryNames {
+ if (__specialLibraryNames == null) {
+ __specialLibraryNames = _specialClassDefinitions
+ .map((_SpecialClassDefinition d) => d.libraryName)
+ .toSet();
+ }
+ return __specialLibraryNames;
+}
+
+/// Class for managing special [Class] objects inside Dartdoc.
+class SpecialClasses {
+ final PackageGraph packageGraph;
+ final Map<SpecialClass, Class> _specialClass = {};
+
+ SpecialClasses(this.packageGraph) {
+ Set<LibraryElement> doneKeys = new Set();
+ Set<LibraryElement> keysToDo = new Set.from(packageGraph.allLibraries.keys);
+ // Loops because traversing the libraries can instantiate additional
+ // libraries, and does so in this manner to avoid running into iterable
+ // modification exceptions.
+ while (keysToDo.isNotEmpty) {
+ keysToDo.forEach((LibraryElement e) {
+ if (_specialLibraryNames.contains(e.name)) {
+ packageGraph.allLibraries[e].allClasses.forEach((Class aClass) {
+ _specialClassDefinitions.forEach((_SpecialClassDefinition d) {
+ if (d.matchesClass(aClass)) {
+ assert(!_specialClass.containsKey(d.specialClass) ||
+ _specialClass[d.specialClass] == aClass);
+ _specialClass[d.specialClass] = aClass;
+ }
+ });
+ });
+ }
+ doneKeys.add(e);
+ });
+ keysToDo = new Set.from(packageGraph.allLibraries.keys
+ .where((LibraryElement e) => !doneKeys.contains(e)));
+ }
+ _specialClassDefinitions.forEach((_SpecialClassDefinition d) {
+ if (d.required) assert(_specialClass.containsKey(d.specialClass));
+ });
+ }
+
+ Class operator [](SpecialClass specialClass) => _specialClass[specialClass];
+}
diff --git a/pubspec.lock b/pubspec.lock
index d00fc3b..990285f 100644
--- a/pubspec.lock
+++ b/pubspec.lock
@@ -492,4 +492,4 @@
source: hosted
version: "2.1.13"
sdks:
- dart: ">=2.0.0-dev.59.0 <=2.0.0-dev.63.0.flutter-4c9689c1d2"
+ dart: ">=2.0.0-dev.59.0 <=2.0.0-dev.69.0"
diff --git a/test/compare_output_test.dart b/test/compare_output_test.dart
index 3fcb718..b29a583 100644
--- a/test/compare_output_test.dart
+++ b/test/compare_output_test.dart
@@ -200,8 +200,7 @@
fail('dartdoc failed');
}
- var jsonValues = LineSplitter
- .split(result.stdout)
+ var jsonValues = LineSplitter.split(result.stdout)
.map((j) => json.decode(j) as Map<String, dynamic>)
.toList();
diff --git a/test/dartdoc_options_test.dart b/test/dartdoc_options_test.dart
index a3b4fdd..917cbea 100644
--- a/test/dartdoc_options_test.dart
+++ b/test/dartdoc_options_test.dart
@@ -498,7 +498,14 @@
errorMessage,
equals(
'Field dartdoc.fileOptionList from ${pathLib.canonicalize(dartdocOptionsTwo.path)}, set to [existing.dart, thing/that/does/not/exist], resolves to missing path: '
- '"${pathLib.joinAll([pathLib.canonicalize(secondDir.path), 'thing', 'that', 'does', 'not', 'exist'])}"'));
+ '"${pathLib.joinAll([
+ pathLib.canonicalize(secondDir.path),
+ 'thing',
+ 'that',
+ 'does',
+ 'not',
+ 'exist'
+ ])}"'));
// It doesn't matter that this fails.
expect(
dartdocOptionSetFiles['nonCriticalFileOption'].valueAt(firstDir),
@@ -519,7 +526,10 @@
errorMessage,
equals(
'Field dartdoc.fileOption from ${pathLib.canonicalize(dartdocOptionsTwo.path)}, set to not existing, resolves to missing path: '
- '"${pathLib.joinAll([pathLib.canonicalize(secondDir.path), "not existing"])}"'));
+ '"${pathLib.joinAll([
+ pathLib.canonicalize(secondDir.path),
+ "not existing"
+ ])}"'));
});
test('DartdocOptionSetFile works for directory options', () {
diff --git a/test/model_test.dart b/test/model_test.dart
index bd3077e..ca93b77 100644
--- a/test/model_test.dart
+++ b/test/model_test.dart
@@ -8,6 +8,7 @@
import 'package:dartdoc/dartdoc.dart';
import 'package:dartdoc/src/model.dart';
+import 'package:dartdoc/src/special_elements.dart';
import 'package:dartdoc/src/warnings.dart';
import 'package:path/path.dart' as pathLib;
import 'package:test/test.dart';
@@ -305,7 +306,8 @@
htmlLibrary.allClasses.singleWhere((c) => c.name == 'EventTarget');
Field hashCode = EventTarget.allPublicInstanceProperties
.singleWhere((f) => f.name == 'hashCode');
- Class objectModelElement = sdkAsPackageGraph.objectElement;
+ Class objectModelElement =
+ sdkAsPackageGraph.specialClasses[SpecialClass.object];
// If this fails, EventTarget might have been changed to no longer
// inherit from Interceptor. If that's true, adjust test case to
// another class that does.
@@ -1693,7 +1695,7 @@
new File(pathLib.join(utils.testPackageDir.path, "crossdart.json"));
crossDartFile.writeAsStringSync("""
{"$fakePath":
- {"references":[{"offset":${offset},"end":${offset+3},"remotePath":"http://www.example.com/fake.dart"}]}}
+ {"references":[{"offset":${offset},"end":${offset + 3},"remotePath":"http://www.example.com/fake.dart"}]}}
""");
// Indirectly load the file.
crossdartPackageGraph.crossdartJson;
diff --git a/test/src/utils.dart b/test/src/utils.dart
index a2dc71d..688677c 100644
--- a/test/src/utils.dart
+++ b/test/src/utils.dart
@@ -44,8 +44,8 @@
/// Convenience factory to build a [DartdocOptionContext] and associate it with a
/// [DartdocOptionSet] based on the current working directory.
Future<DartdocOptionContext> contextFromArgv(List<String> argv) async {
- DartdocOptionSet optionSet = await DartdocOptionSet
- .fromOptionGenerators('dartdoc', [createDartdocOptions]);
+ DartdocOptionSet optionSet = await DartdocOptionSet.fromOptionGenerators(
+ 'dartdoc', [createDartdocOptions]);
optionSet.parseArguments(argv);
return new DartdocOptionContext(optionSet, Directory.current);
}
diff --git a/tool/doc_packages.dart b/tool/doc_packages.dart
index 0c31e56..ade349f 100644
--- a/tool/doc_packages.dart
+++ b/tool/doc_packages.dart
@@ -196,8 +196,7 @@
{String cwd,
bool quiet: false,
Duration timeout: const Duration(seconds: 60)}) {
- return Process
- .start(command, args, workingDirectory: cwd)
+ return Process.start(command, args, workingDirectory: cwd)
.then((Process process) {
if (!quiet) {
process.stdout.listen((bytes) => _log(utf8.decode(bytes)));
diff --git a/tool/grind.dart b/tool/grind.dart
index 3cb6104..99d3b1a 100644
--- a/tool/grind.dart
+++ b/tool/grind.dart
@@ -586,7 +586,8 @@
}
/// Returns the directory in which we generated documentation.
-Future<String> _buildPubPackageDocs(String pubPackageName,
+Future<String> _buildPubPackageDocs(
+ String pubPackageName, List<String> dartdocParameters,
[String version, String label]) async {
Map<String, String> env = _createThrowawayPubCache();
var launcher = new SubprocessLauncher(
@@ -609,19 +610,24 @@
pathLib.join(Directory.current.absolute.path, 'bin', 'dartdoc.dart'),
'--json',
'--show-progress',
- ]..addAll(extraDartdocParameters),
+ ]..addAll(dartdocParameters),
workingDirectory: pubPackageDir.absolute.path);
return pathLib.join(pubPackageDir.absolute.path, 'doc', 'api');
}
@Task(
- 'Serve an arbitrary pub package based on PACKAGE_NAME and PACKAGE_VERSION environment variables')
-servePubPackage() async {
+ 'Build an arbitrary pub package based on PACKAGE_NAME and PACKAGE_VERSION environment variables')
+Future<String> buildPubPackage() async {
assert(Platform.environment.containsKey('PACKAGE_NAME'));
String packageName = Platform.environment['PACKAGE_NAME'];
String version = Platform.environment['PACKAGE_VERSION'];
- _serveDocsFrom(await _buildPubPackageDocs(packageName, version), 9000,
- 'serve-pub-package');
+ return _buildPubPackageDocs(packageName, extraDartdocParameters, version);
+}
+
+@Task(
+ 'Serve an arbitrary pub package based on PACKAGE_NAME and PACKAGE_VERSION environment variables')
+servePubPackage() async {
+ _serveDocsFrom(await buildPubPackage(), 9000, 'serve-pub-package');
}
@Task('Checks that CHANGELOG mentions current version')
diff --git a/tool/travis.sh b/tool/travis.sh
index 6f4cceb..1445fb0 100755
--- a/tool/travis.sh
+++ b/tool/travis.sh
@@ -20,6 +20,9 @@
elif [ "$DARTDOC_BOT" = "flutter" ]; then
echo "Running flutter dartdoc bot"
pub run grinder validate-flutter-docs
+elif [ "$DARTDOC_BOT" = "packages" ]; then
+ echo "Running packages dartdoc bot"
+ PACKAGE_NAME=angular PACKAGE_VERSION=">=5.0.0-beta" DARTDOC_PARAMS="--include=angular,angular.security" pub run grinder build-pub-package
else
echo "Running main dartdoc bot"
pub run grinder buildbot
