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

Override packagemeta using a provider rather than statically #2219

Merged
merged 2 commits into from
May 27, 2020
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
8 changes: 1 addition & 7 deletions bin/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,11 @@ import 'package:dartdoc/options.dart';
/// Analyzes Dart files and generates a representation of included libraries,
/// classes, and members. Uses the current directory to look for libraries.
Future<void> main(List<String> arguments) async {
var config = await parseOptions(arguments);
var config = await parseOptions(pubPackageMetaProvider, arguments);
if (config == null) {
// There was an error while parsing options.
return;
}
// Set the default way to construct [PackageMeta].
PackageMeta.setPackageMetaFactories(
PubPackageMeta.fromElement,
PubPackageMeta.fromFilename,
PubPackageMeta.fromDir,
);
final packageBuilder = PubPackageBuilder(config);
final dartdoc = config.generateDocs
? await Dartdoc.fromContext(config, packageBuilder)
Expand Down
7 changes: 5 additions & 2 deletions lib/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@ Future<List<DartdocOption>> createDartdocProgramOptions() async {
];
}

Future<DartdocProgramOptionContext> parseOptions(List<String> arguments) async {
Future<DartdocProgramOptionContext> parseOptions(
PackageMetaProvider packageMetaProvider,
List<String> arguments,
) async {
var optionSet = await DartdocOptionSet.fromOptionGenerators('dartdoc', [
createDartdocOptions,
() => createDartdocOptions(packageMetaProvider),
createDartdocProgramOptions,
createLoggingOptions,
createGeneratorOptions,
Expand Down
12 changes: 7 additions & 5 deletions lib/src/dartdoc_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1429,7 +1429,9 @@ class DartdocOptionContext extends DartdocOptionContextBase

/// Instantiate dartdoc's configuration file and options parser with the
/// given command line arguments.
Future<List<DartdocOption>> createDartdocOptions() async {
Future<List<DartdocOption>> createDartdocOptions(
PackageMetaProvider packageMetaProvider,
) async {
return <DartdocOption>[
DartdocOptionArgOnly<bool>('allowTools', false,
help: 'Execute user-defined tools to fill in @tool directives.',
Expand Down Expand Up @@ -1562,7 +1564,7 @@ Future<List<DartdocOption>> createDartdocOptions() async {
DartdocOptionSyntheticOnly<PackageMeta>(
'packageMeta',
(DartdocSyntheticOption<PackageMeta> option, Directory dir) {
var packageMeta = PackageMeta.fromDir(dir);
var packageMeta = packageMetaProvider.fromDir(dir);
if (packageMeta == null) {
throw DartdocOptionError(
'Unable to determine package for directory: ${dir.path}');
Expand Down Expand Up @@ -1590,8 +1592,8 @@ Future<List<DartdocOption>> createDartdocOptions() async {
help: "Label categories that aren't documented", negatable: true),
DartdocOptionSyntheticOnly<PackageMeta>('topLevelPackageMeta',
(DartdocSyntheticOption<PackageMeta> option, Directory dir) {
var packageMeta = PackageMeta.fromDir(
Directory(option.parent['inputDir'].valueAt(dir)));
var packageMeta = packageMetaProvider
.fromDir(Directory(option.parent['inputDir'].valueAt(dir)));
if (packageMeta == null) {
throw DartdocOptionError(
'Unable to generate documentation: no package found');
Expand Down Expand Up @@ -1632,7 +1634,7 @@ Future<List<DartdocOption>> createDartdocOptions() async {
// TODO(jcollins-g): refactor so there is a single static "create" for
// each DartdocOptionContext that traverses the inheritance tree itself.
...await createExperimentOptions(),
...await createPackageWarningOptions(),
...await createPackageWarningOptions(packageMetaProvider),
...await createSourceLinkerOptions(),
];
}
5 changes: 4 additions & 1 deletion lib/src/model/library.dart
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,10 @@ class Library extends ModelElement with Categorization, TopLevelContainer {
PackageMeta _packageMeta;

PackageMeta get packageMeta {
_packageMeta ??= PackageMeta.fromElement(element, config.sdkDir);
_packageMeta ??= packageGraph.packageMetaProvider.fromElement(
element,
config.sdkDir,
);
return _packageMeta;
}

Expand Down
12 changes: 9 additions & 3 deletions lib/src/model/package_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ import 'package:dartdoc/src/dartdoc_options.dart';
import 'package:dartdoc/src/io_utils.dart';
import 'package:dartdoc/src/logging.dart';
import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/package_meta.dart' show PackageMeta;
import 'package:dartdoc/src/package_meta.dart'
show PackageMeta, pubPackageMetaProvider;
import 'package:dartdoc/src/render/renderer_factory.dart';
import 'package:dartdoc/src/special_elements.dart';
import 'package:package_config/discovery.dart' as package_config;
Expand Down Expand Up @@ -62,7 +63,12 @@ class PubPackageBuilder implements PackageBuilder {
var rendererFactory = RendererFactory.forFormat(config.format);

var newGraph = PackageGraph.UninitializedPackageGraph(
config, sdk, hasEmbedderSdkFiles, rendererFactory);
config,
sdk,
hasEmbedderSdkFiles,
rendererFactory,
pubPackageMetaProvider,
);
await getLibraries(newGraph);
await newGraph.initializePackageGraph();
return newGraph;
Expand Down Expand Up @@ -238,7 +244,7 @@ class PubPackageBuilder implements PackageBuilder {
Set<PackageMeta> _packageMetasForFiles(Iterable<String> files) {
var metas = <PackageMeta>{};
for (var filename in files) {
metas.add(PackageMeta.fromFilename(filename));
metas.add(pubPackageMetaProvider.fromFilename(filename));
}
return metas;
}
Expand Down
18 changes: 13 additions & 5 deletions lib/src/model/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,21 @@ import 'package:dartdoc/src/dartdoc_options.dart';
import 'package:dartdoc/src/logging.dart';
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;
import 'package:dartdoc/src/package_meta.dart'
show PackageMeta, PackageMetaProvider;
import 'package:dartdoc/src/render/renderer_factory.dart';
import 'package:dartdoc/src/special_elements.dart';
import 'package:dartdoc/src/tuple.dart';
import 'package:dartdoc/src/warnings.dart';

class PackageGraph {
PackageGraph.UninitializedPackageGraph(
this.config, this.sdk, this.hasEmbedderSdk, this.rendererFactory)
: packageMeta = config.topLevelPackageMeta {
this.config,
this.sdk,
this.hasEmbedderSdk,
this.rendererFactory,
this.packageMetaProvider,
) : packageMeta = config.topLevelPackageMeta {
_packageWarningCounter = PackageWarningCounter(this);
// Make sure the default package exists, even if it has no libraries.
// This can happen for packages that only contain embedder SDKs.
Expand All @@ -38,7 +43,7 @@ class PackageGraph {
void addLibraryToGraph(DartDocResolvedLibrary resolvedLibrary) {
assert(!allLibrariesAdded);
var element = resolvedLibrary.element;
var packageMeta = PackageMeta.fromElement(element, config.sdkDir);
var packageMeta = packageMetaProvider.fromElement(element, config.sdkDir);
var lib = Library.fromLibraryResult(
resolvedLibrary, this, Package.fromPackageMeta(packageMeta, this));
packageMap[packageMeta.name].libraries.add(lib);
Expand Down Expand Up @@ -214,6 +219,9 @@ class PackageGraph {
/// Factory for renderers
final RendererFactory rendererFactory;

/// PackageMeta Provider for building [PackageMeta]s.
final PackageMetaProvider packageMetaProvider;

Package _defaultPackage;

Package get defaultPackage {
Expand Down Expand Up @@ -849,7 +857,7 @@ class PackageGraph {
resolvedLibrary,
this,
Package.fromPackageMeta(
PackageMeta.fromElement(elementLibrary, config.sdkDir),
packageMetaProvider.fromElement(elementLibrary, config.sdkDir),
packageGraph));
allLibraries[elementLibrary] = foundLibrary;
return foundLibrary;
Expand Down
62 changes: 22 additions & 40 deletions lib/src/package_meta.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,28 @@ final List<List<String>> __sdkDirFilePathsPosix = [
['lib/core/core.dart'],
];

final PackageMetaProvider pubPackageMetaProvider = PackageMetaProvider(
PubPackageMeta.fromElement,
PubPackageMeta.fromFilename,
PubPackageMeta.fromDir,
);

/// Sets the supported way of constructing [PackageMeta] objects.
///
/// These objects can be constructed from a filename, a directory
/// or a [LibraryElement]. We allow different dartdoc implementations to
/// provide their own [PackageMeta] types.
///
/// By using a different provider, these implementations can control how
/// [PackageMeta] objects is built.
class PackageMetaProvider {
final PackageMeta Function(LibraryElement, String) fromElement;
final PackageMeta Function(String) fromFilename;
final PackageMeta Function(Directory) fromDir;

PackageMetaProvider(this.fromElement, this.fromFilename, this.fromDir);
}

/// Describes a single package in the context of `dartdoc`.
///
/// The primary function of this class is to allow canonicalization of packages
Expand Down Expand Up @@ -104,46 +126,6 @@ abstract class PackageMeta {

@override
String toString() => name;

/// Sets the supported ways of constructing [PackageMeta] objects.
///
/// These objects can be constructed from a filename, a directory
/// or a [LibraryElement]. We allow different dartdoc implementations to
/// provide their own [PackageMeta] types.
///
/// By calling this function, these implementations can control how
/// [PackageMeta] is built.
static void setPackageMetaFactories(
PackageMeta Function(LibraryElement, String) fromElementFactory,
PackageMeta Function(String) fromFilenameFactory,
PackageMeta Function(Directory) fromDirFactory,
) {
assert(fromElementFactory != null);
assert(fromFilenameFactory != null);
assert(fromDirFactory != null);
if (_fromElement == fromElementFactory &&
_fromFilename == fromFilenameFactory &&
_fromDir == fromDirFactory) {
// Nothing to do.
return;
}
if (_fromElement != null || _fromFilename != null || _fromDir != null) {
throw StateError('PackageMeta factories cannot be changed once defined.');
}
_fromElement = fromElementFactory;
_fromFilename = fromFilenameFactory;
_fromDir = fromDirFactory;
}

static PackageMeta Function(LibraryElement, String) _fromElement;
static PackageMeta Function(String) _fromFilename;
static PackageMeta Function(Directory) _fromDir;
static PackageMeta Function(LibraryElement, String) get fromElement =>
_fromElement ?? PubPackageMeta.fromElement;
static PackageMeta Function(String) get fromFilename =>
_fromFilename ?? PubPackageMeta.fromFilename;
static PackageMeta Function(Directory) get fromDir =>
_fromDir ?? PubPackageMeta.fromDir;
}

/// Default implementation of [PackageMeta] depends on pub packages.
Expand Down
16 changes: 12 additions & 4 deletions lib/src/warnings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ abstract class PackageWarningOptionContext implements DartdocOptionContextBase {
bool get verboseWarnings => optionSet['verboseWarnings'].valueAt(context);
}

Future<List<DartdocOption>> createPackageWarningOptions() async {
Future<List<DartdocOption>> createPackageWarningOptions(
PackageMetaProvider packageMetaProvider,
) async {
return <DartdocOption>[
DartdocOptionArgOnly<bool>('allowNonLocalWarnings', false,
negatable: true,
Expand Down Expand Up @@ -79,7 +81,10 @@ Future<List<DartdocOption>> createPackageWarningOptions() async {
.join('\n')),
// Synthetic option uses a factory to build a PackageWarningOptions from all the above flags.
DartdocOptionSyntheticOnly<PackageWarningOptions>(
'packageWarningOptions', PackageWarningOptions.fromOptions),
'packageWarningOptions',
(DartdocSyntheticOption<PackageWarningOptions> option, Directory dir) =>
PackageWarningOptions.fromOptions(option, dir, packageMetaProvider),
),
];
}

Expand Down Expand Up @@ -303,10 +308,13 @@ class PackageWarningOptions {

/// [packageMeta] parameter is for testing.
static PackageWarningOptions fromOptions(
DartdocSyntheticOption<PackageWarningOptions> option, Directory dir) {
DartdocSyntheticOption<PackageWarningOptions> option,
Directory dir,
PackageMetaProvider packageMetaProvider,
) {
// First, initialize defaults.
var newOptions = PackageWarningOptions();
var packageMeta = PackageMeta.fromDir(dir);
var packageMeta = packageMetaProvider.fromDir(dir);

// Interpret errors/warnings/ignore options. In the event of conflict, warning overrides error and
// ignore overrides warning.
Expand Down
2 changes: 1 addition & 1 deletion test/dartdoc_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ void main() {
await subprocessLauncher.runStreamed(Platform.resolvedExecutable, args,
workingDirectory: _testPackagePath,
perLine: (s) => output.writeln(s));
var dartdocMeta = PackageMeta.fromFilename(dartdocPath);
var dartdocMeta = pubPackageMetaProvider.fromFilename(dartdocPath);
expect(output.toString(),
endsWith('dartdoc version: ${dartdocMeta.version}\n'));
});
Expand Down
8 changes: 4 additions & 4 deletions test/package_meta_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ void main() {

setUp(() {
var d = Directory.systemTemp.createTempSync('test_package_not_valid');
p = PackageMeta.fromDir(d);
p = pubPackageMetaProvider.fromDir(d);
});

test('is not valid', () {
Expand All @@ -25,7 +25,7 @@ void main() {
});

group('PackageMeta for the test package', () {
var p = PackageMeta.fromDir(Directory(
var p = pubPackageMetaProvider.fromDir(Directory(
path.join(Directory.current.path, 'testing', 'test_package')));

test('readme with corrupt UTF-8 loads without throwing', () {
Expand All @@ -35,7 +35,7 @@ void main() {
});

group('PackageMeta.fromDir for this package', () {
var p = PackageMeta.fromDir(Directory.current);
var p = pubPackageMetaProvider.fromDir(Directory.current);

test('has a name', () {
expect(p.name, 'dartdoc');
Expand Down Expand Up @@ -82,7 +82,7 @@ void main() {
});

group('PackageMeta.fromSdk', () {
var p = PackageMeta.fromDir(defaultSdkDir);
var p = pubPackageMetaProvider.fromDir(defaultSdkDir);

test('has a name', () {
expect(p.name, 'Dart');
Expand Down
13 changes: 8 additions & 5 deletions test/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ final RegExp observatoryPortRegexp =
RegExp(r'^Observatory listening on http://.*:(\d+)');

Directory sdkDir = defaultSdkDir;
PackageMeta sdkPackageMeta = PackageMeta.fromDir(sdkDir);
PackageMeta sdkPackageMeta = pubPackageMetaProvider.fromDir(sdkDir);

final _testPackageGraphMemo = AsyncMemoizer<PackageGraph>();
Future<PackageGraph> get testPackageGraph => _testPackageGraphMemo.runOnce(() =>
Expand Down Expand Up @@ -98,17 +98,20 @@ final Directory testPackageCustomTemplates =
/// the '--input' flag.
Future<DartdocGeneratorOptionContext> generatorContextFromArgv(
List<String> argv) async {
var optionSet = await DartdocOptionSet.fromOptionGenerators(
'dartdoc', [createDartdocOptions, createGeneratorOptions]);
var optionSet = await DartdocOptionSet.fromOptionGenerators('dartdoc', [
() => createDartdocOptions(pubPackageMetaProvider),
createGeneratorOptions,
]);
optionSet.parseArguments(argv);
return DartdocGeneratorOptionContext(optionSet, null);
}

/// 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 {
var optionSet = await DartdocOptionSet.fromOptionGenerators(
'dartdoc', [createDartdocOptions]);
var optionSet = await DartdocOptionSet.fromOptionGenerators('dartdoc', [
() => createDartdocOptions(pubPackageMetaProvider),
]);
optionSet.parseArguments(argv);
return DartdocOptionContext(optionSet, Directory.current);
}
Expand Down
3 changes: 2 additions & 1 deletion test/warnings_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ library dartdoc.warnings_test;
import 'dart:io';

import 'package:dartdoc/src/dartdoc_options.dart';
import 'package:dartdoc/src/package_meta.dart';
import 'package:dartdoc/src/warnings.dart';
import 'package:path/path.dart' as path;
import 'package:test/test.dart';
Expand Down Expand Up @@ -47,7 +48,7 @@ dartdoc:

setUp(() async {
optionSet = await DartdocOptionSet.fromOptionGenerators(
'dartdoc', [createDartdocOptions]);
'dartdoc', [() => createDartdocOptions(pubPackageMetaProvider)]);
});

test('Verify that options for enabling/disabling packages work', () {
Expand Down
Loading