From 29ed2b6b8d5a20bc00e3adace047aa8b42371205 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 12 Apr 2018 12:07:49 -0700 Subject: [PATCH 1/6] Squash commits --- lib/src/config.dart | 1 + lib/src/dartdoc_options.dart | 690 +++++++++++++++++++++++++++++++++ test/dartdoc_options_test.dart | 442 +++++++++++++++++++++ 3 files changed, 1133 insertions(+) diff --git a/lib/src/config.dart b/lib/src/config.dart index f32c4cade4..1af09e4a94 100644 --- a/lib/src/config.dart +++ b/lib/src/config.dart @@ -2,6 +2,7 @@ // 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. +/// Legacy dartdoc configuration library. TODO(jcollins-g): merge with [DartdocOption]. library dartdoc.config; import 'dart:io'; diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart index 95d8108d9b..dd225be6c5 100644 --- a/lib/src/dartdoc_options.dart +++ b/lib/src/dartdoc_options.dart @@ -9,17 +9,707 @@ /// It searches parent directories until it finds an analysis_options.yaml file, /// and uses built-in defaults if one is not found. /// +/// The classes here manage both the dartdoc_options.yaml loading and command +/// line arguments. +/// library dartdoc.dartdoc_options; import 'dart:io'; +import 'package:args/args.dart'; +import 'package:dartdoc/dartdoc.dart'; import 'package:path/path.dart' as pathLib; import 'package:yaml/yaml.dart'; import 'logging.dart'; +/// Constants to help with type checking, because T is int and so forth +/// don't work in Dart. +const String _kStringVal = ''; +const List _kListStringVal = const []; +const Map _kMapStringVal = const {}; +const int _kIntVal = 0; +const double _kDoubleVal = 0.0; +const bool _kBoolVal = true; + +class DartdocOptionError extends DartDocFailure { + DartdocOptionError(String details) : super(details); +} + +class DartdocFileMissing extends DartdocOptionError { + DartdocFileMissing(String details) : super(details); +} + +/// A container class to keep track of where our yaml data came from. +class _YamlFileData { + /// The map from the yaml file. + final Map data; + + /// The path to the directory containing the yaml file. + final String canonicalDirectoryPath; + + _YamlFileData(this.data, this.canonicalDirectoryPath); +} + +/// Some DartdocOption subclasses need to keep track of where they +/// got the value from; this class contains those intermediate results +/// so that error messages can be more useful. +class _OptionValueWithContext { + /// The value of the option at canonicalDirectoryPath. + final T value; + + /// A canonical path to the directory where this value came from. May + /// be different from [DartdocOption.valueAt]'s `dir` parameter. + String canonicalDirectoryPath; + + /// If non-null, the basename of the configuration file the value came from. + String definingFile; + + /// A [pathLib.Context] variable initialized with canonicalDirectoryPath. + pathLib.Context pathContext; + + /// Build a _OptionValueWithContext. + /// [path] is the path where this value came from (not required to be canonical) + _OptionValueWithContext(this.value, String path, {String definingFile}) { + this.definingFile = definingFile; + canonicalDirectoryPath = pathLib.canonicalize(path); + pathContext = new pathLib.Context(current: canonicalDirectoryPath); + } + + /// Assume value is a path, and attempt to resolve it. Throws [UnsupportedError] + /// if [T] isn't a [String] or [List]. + T get resolvedValue { + if (value is List) { + return (value as List).map((v) => pathContext.canonicalize(v)) + as T; + } else if (value is String) { + return pathContext.canonicalize(value as String) as T; + } else { + throw new UnsupportedError("Type $T is not supported for resolvedValue"); + } + } +} + +/// An abstract class for interacting with dartdoc options. +/// +/// This class and its implementations allow Dartdoc to declare options +/// that are both defined in a configuration file and specified via the +/// command line, with searching the directory tree for a proper file +/// and overriding file options with the command line built-in. A number +/// of sanity checks are also built in to these classes so that file existence +/// can be verified, types constrained, and defaults provided. +/// +/// Use via implementations [DartdocOptionSet], [DartdocOptionBoth], +/// [DartdocOptionArgOnly], and [DartdocOptionFileOnly]. +abstract class DartdocOption { + /// This is the value returned if we couldn't find one otherwise. + final T defaultsTo; + + /// Text string for help passed on in command line options. + final String help; + + /// The name of this option, not including the names of any parents. + final String name; + + /// Set to true if this option represents the name of a directory. + final bool isDir; + + /// Set to true if this option represents the name of a file. + final bool isFile; + + /// Set to true if DartdocOption subclasses should validate that the + /// directory or file exists. Does not imply validation of [defaultsTo], + /// and requires that one of [isDir] or [isFile] is set. + final bool mustExist; + + DartdocOption._(this.name, this.defaultsTo, this.help, this.isDir, + this.isFile, this.mustExist) { + assert(!(isDir && isFile)); + if (isDir || isFile) assert(_isString || _isListString); + if (mustExist) { + assert(isDir || isFile); + } + } + + // The choice not to use reflection means there's some ugly type checking, + // somewhat more ugly than we'd have to do anyway to automatically convert + // command line arguments and yaml data to real types. + // + // Condense the ugly all in one place, this set of getters. + + bool get _isString => _kStringVal is T; + bool get _isListString => _kListStringVal is T; + bool get _isMapString => _kMapStringVal is T; + bool get _isBool => _kBoolVal is T; + bool get _isInt => _kIntVal is T; + bool get _isDouble => _kDoubleVal is T; + + DartdocOption _parent; + + /// The parent of this DartdocOption, or null if this is the root. + DartdocOption get parent => _parent; + + final Map __yamlAtCanonicalPathCache = {}; + + /// Implementation detail for [DartdocOptionFileOnly]. Make sure we use + /// the root node's cache. + Map get _yamlAtCanonicalPathCache => + root.__yamlAtCanonicalPathCache; + + final ArgParser __argParser = new ArgParser(); + ArgParser get argParser => root.__argParser; + + ArgResults __argResults; + + /// Parse these as string arguments (from argv) with the argument parser. + /// Call before calling [valueAt] for any [DartdocOptionArgOnly] or + /// [DartdocOptionBoth] in this tree. + void _parseArguments(List arguments) { + __argResults = argParser.parse(arguments); + } + + /// Throw [DartdocFileMissing] with a detailed error message indicating where + /// the error came from when a file or directory option is missing. + void _onMissing( + _OptionValueWithContext valueWithContext, String missingFilename); + + /// Call [_onMissing] for every path that does not exist. Returns true if + /// all paths exist or [mustExist] == false. + void _validatePaths(_OptionValueWithContext valueWithContext) { + if (!mustExist) return; + assert(isDir || isFile); + List resolvedPaths; + if (valueWithContext.value is String) { + resolvedPaths = [valueWithContext.resolvedValue]; + } else { + resolvedPaths = valueWithContext.resolvedValue.toList(); + } + for (String path in resolvedPaths) { + FileSystemEntity f = isDir ? new Directory(path) : new File(path); + if (!f.existsSync()) { + _onMissing(valueWithContext, path); + } + } + } + + /// For a [List] or [String] value, if [isDir] or [isFile] is set, + /// resolve paths in value relative to canonicalPath. + T _handlePathsInContext(_OptionValueWithContext valueWithContext) { + if (valueWithContext?.value == null || !(isDir || isFile)) + return valueWithContext?.value; + _validatePaths(valueWithContext); + return valueWithContext.resolvedValue; + } + + /// Call this with argv to set up the argument overrides. Applies to all + /// children. + void parseArguments(List arguments) => + root._parseArguments(arguments); + ArgResults get _argResults => root.__argResults; + + /// Set the parent of this [DartdocOption]. Do not call more than once. + set parent(DartdocOption newParent) { + assert(_parent == null); + _parent = newParent; + } + + /// The root [DartdocOption] containing this object, or [this] if the object + /// has no parent. + DartdocOption get root { + DartdocOption p = this; + while (p.parent != null) { + p = p.parent; + } + return p; + } + + /// All object names starting at the root. + Iterable get keys { + List keyList = []; + DartdocOption option = this; + while (option?.name != null) { + keyList.add(option.name); + option = option.parent; + } + return keyList.reversed; + } + + /// Direct children of this node, mapped by name. + final Map _children = {}; + + /// Return the calculated value of this option, given the directory as context. + /// + /// If [isFile] or [isDir] is set, the returned value will be transformed + /// into a canonical path relative to the current working directory + /// (for arguments) or the config file from which the value was derived. + /// + /// May throw [DartdocOptionError] if a command line argument is of the wrong + /// type. If [mustExist] is true, will throw [DartdocFileMissing] for command + /// line parameters and file paths in config files that don't point to + /// corresponding files or directories. + T valueAt(Directory dir); + + /// Adds a DartdocOption to the children of this DartdocOption. + void add(DartdocOption option) { + if (_children.containsKey(option.name)) + throw new DartdocOptionError( + 'Tried to add two children with the same name: ${option.name}'); + _children[option.name] = option; + option.parent = this; + option.traverse((option) => option._onAdd()); + } + + /// This method is guaranteed to be called when [this] or any parent is added. + void _onAdd() {} + + /// Adds a list of dartdoc options to the children of this DartdocOption. + void addAll(Iterable options) => + options.forEach((o) => add(o)); + + /// Get the immediate child of this node named [name]. + DartdocOption operator [](String name) { + return _children[name]; + } + + /// Apply the function [visit] to [this] and all children. + void traverse(void visit(DartdocOption)) { + visit(this); + _children.values.forEach((d) => d.traverse(visit)); + } +} + +/// A [DartdocOption] that only contains other [DartdocOption]s and is not an option itself. +class DartdocOptionSet extends DartdocOption { + DartdocOptionSet(String name) + : super._(name, null, null, false, false, false); + + /// [DartdocOptionSet] always has the null value. + @override + Null valueAt(Directory dir) => null; + + /// Since we have no value, [_onMissing] does nothing. + @override + void _onMissing( + _OptionValueWithContext valueWithContext, String missingFilename) {} + + @override + + /// Traverse skips this node, because it doesn't represent a real configuration object. + void traverse(void visitor(DartdocOption)) { + _children.values.forEach((d) => d.traverse(visitor)); + } +} + +/// A [DartdocOption] that only exists as a command line argument. --help would +/// be a good example. +class DartdocOptionArgOnly extends DartdocOption + with _DartdocArgOption { + String _abbr; + bool _hide; + bool _negatable; + bool _splitCommas; + + DartdocOptionArgOnly(String name, T defaultsTo, + {String abbr, + bool mustExist = false, + help: '', + bool hide, + bool isDir = false, + bool isFile = false, + bool negatable, + bool splitCommas}) + : super._(name, defaultsTo, help, isDir, isFile, mustExist) { + _hide = hide; + _negatable = negatable; + _splitCommas = splitCommas; + _abbr = abbr; + } + + @override + String get abbr => _abbr; + @override + bool get hide => _hide; + @override + bool get negatable => _negatable; + @override + bool get splitCommas => _splitCommas; +} + +/// A [DartdocOption] that works with command line arguments and dartdoc_options files. +class DartdocOptionBoth extends DartdocOption + with _DartdocArgOption, _DartdocFileOption { + String _abbr; + bool _hide; + bool _negatable; + bool _parentDirOverridesChild; + bool _splitCommas; + + DartdocOptionBoth(String name, T defaultsTo, + {String abbr, + bool mustExist = false, + String help: '', + bool hide, + bool isDir = false, + bool isFile = false, + bool negatable, + bool parentDirOverridesChild: false, + bool splitCommas}) + : super._(name, defaultsTo, help, isDir, isFile, mustExist) { + _abbr = abbr; + _hide = hide; + _negatable = negatable; + _parentDirOverridesChild = parentDirOverridesChild; + _splitCommas = splitCommas; + } + + @override + void _onMissing( + _OptionValueWithContext valueWithContext, String missingPath) { + String dartdocYaml; + String description; + if (valueWithContext.definingFile != null) { + dartdocYaml = pathLib.canonicalize(pathLib.join( + valueWithContext.canonicalDirectoryPath, + valueWithContext.definingFile)); + description = "Field ${fieldName} from ${dartdocYaml}"; + } else { + description = "Argument --${argName}"; + } + throw new DartdocFileMissing( + '$description, set to ${valueWithContext.value}, resolves to missing path: "${missingPath}"'); + } + + @override + + /// Try to find an explicit argument setting this value, but if not, fall back to files + /// finally, the default. + T valueAt(Directory dir) { + T value = _valueAtFromArgs(); + if (value == null) value = _valueAtFromFiles(dir); + if (value == null) value = defaultsTo; + return value; + } + + @override + String get abbr => _abbr; + @override + bool get hide => _hide; + @override + bool get negatable => _negatable; + @override + bool get parentDirOverridesChild => _parentDirOverridesChild; + @override + bool get splitCommas => _splitCommas; +} + +class DartdocOptionFileOnly extends DartdocOption + with _DartdocFileOption { + bool _parentDirOverridesChild; + DartdocOptionFileOnly(String name, T defaultsTo, + {bool mustExist = false, + String help: '', + bool isDir = false, + bool isFile = false, + bool parentDirOverridesChild: false}) + : super._(name, defaultsTo, help, isDir, isFile, mustExist) { + _parentDirOverridesChild = parentDirOverridesChild; + } + + @override + void _onMissing( + _OptionValueWithContext valueWithContext, String missingPath) { + String dartdocYaml = pathLib.canonicalize(pathLib.join( + valueWithContext.canonicalDirectoryPath, + valueWithContext.definingFile)); + throw new DartdocFileMissing( + 'Field ${fieldName} from ${dartdocYaml}, set to ${valueWithContext.value}, resolves to missing path: "${missingPath}"'); + } + + @override + bool get parentDirOverridesChild => _parentDirOverridesChild; +} + +/// Implements checking for options +abstract class _DartdocFileOption implements DartdocOption { + /// If true, the parent directory's value overrides the child's. Otherwise, the child's + /// value overrides values in parents. + bool get parentDirOverridesChild; + + /// The name of the option, with nested options joined by [.]. For example: + /// + /// ```yaml + /// dartdoc: + /// stuff: + /// things: + /// ``` + /// would have the name `things` and the fieldName `dartdoc.stuff.things`. + String get fieldName => keys.join('.'); + + @override + void _onMissing( + _OptionValueWithContext valueWithContext, String missingPath) { + String dartdocYaml = pathLib.join( + valueWithContext.canonicalDirectoryPath, valueWithContext.definingFile); + throw new DartdocFileMissing( + 'Field ${fieldName} from ${dartdocYaml}, set to ${valueWithContext.value}, resolves to missing path: "${missingPath}"'); + } + + @override + + /// Searches for a value in configuration files relative to [dir], and if not + /// found, returns [defaultsTo]. + T valueAt(Directory dir) { + return _valueAtFromFiles(dir) ?? defaultsTo; + } + + T _valueAtFromFiles(Directory dir) { + _OptionValueWithContext valueWithContext; + if (parentDirOverridesChild) { + valueWithContext = _valueAtFromFilesLastFound(dir); + } else { + valueWithContext = _valueAtFromFilesFirstFound(dir); + } + return _handlePathsInContext(valueWithContext); + } + + /// Searches all dartdoc_options files through parent directories, + /// starting at [dir], for the option and returns one once + /// found. + _OptionValueWithContext _valueAtFromFilesFirstFound(Directory dir) { + _OptionValueWithContext value; + while (true) { + value = _valueAtFromFile(dir); + if (value != null || pathLib.equals(dir.parent.path, dir.path)) break; + dir = dir.parent; + } + return value; + } + + /// Searches all dartdoc_options files for the option, and returns the + /// value in the top-most parent directory dartdoc_options.yaml file it is + /// mentioned in. + _OptionValueWithContext _valueAtFromFilesLastFound(Directory dir) { + _OptionValueWithContext value; + while (true) { + _OptionValueWithContext tmpValue = _valueAtFromFile(dir); + if (tmpValue != null) value = tmpValue; + if (pathLib.equals(dir.parent.path, dir.path)) break; + dir = dir.parent; + } + return value; + } + + /// Returns null if not set in the yaml file in this directory (or its + /// parents). + _OptionValueWithContext _valueAtFromFile(Directory dir) { + _YamlFileData yamlFileData = _yamlAtDirectory(dir); + String contextPath = yamlFileData.canonicalDirectoryPath; + var yamlData = yamlFileData.data; + for (String key in keys) { + if (!yamlData.containsKey(key)) return null; + yamlData = yamlData[key]; + } + + var returnData; + if (_isListString) { + if (yamlData is YamlList) { + returnData = []; + for (var item in yamlData as YamlList) { + returnData.add(item.toString()); + } + } + } else if (_isMapString) { + if (yamlData is YamlMap) { + returnData = {}; + for (MapEntry entry in yamlData.entries) { + returnData[entry.key.toString()] = entry.value.toString(); + } + } + } else if (_isDouble) { + if (yamlData is num) { + returnData = (yamlData as num).toDouble(); + } + } else if (_isInt || _isString || _isBool) { + if (yamlData is T) { + returnData = yamlData; + } + } else { + throw new UnsupportedError("Type ${T} is not supported"); + } + return new _OptionValueWithContext(returnData as T, contextPath, + definingFile: 'dartdoc_options.yaml'); + } + + _YamlFileData _yamlAtDirectory(Directory dir) { + List canonicalPaths = [pathLib.canonicalize(dir.path)]; + if (!_yamlAtCanonicalPathCache.containsKey(canonicalPaths.first)) { + _YamlFileData yamlData = new _YamlFileData( + new Map(), pathLib.canonicalize(Directory.current.path)); + if (dir.existsSync()) { + File dartdocOptionsFile; + + while (true) { + dartdocOptionsFile = + new File(pathLib.join(dir.path, 'dartdoc_options.yaml')); + if (dartdocOptionsFile.existsSync() || + pathLib.equals(dir.parent.path, dir.path)) break; + dir = dir.parent; + canonicalPaths.add(pathLib.canonicalize(dir.path)); + } + if (dartdocOptionsFile.existsSync()) { + yamlData = new _YamlFileData( + loadYaml(dartdocOptionsFile.readAsStringSync()), + pathLib.canonicalize(dir.path)); + } + } + canonicalPaths.forEach((p) => _yamlAtCanonicalPathCache[p] = yamlData); + } + return _yamlAtCanonicalPathCache[canonicalPaths.first]; + } +} + +/// Mixin class implementing command-line arguments for [DartdocOption]. +abstract class _DartdocArgOption implements DartdocOption { + /// For [ArgsParser], set to true if the argument can be negated with --no on the command line. + bool get negatable; + + /// For [ArgsParser], set to true if a single string argument will be broken into a list on commas. + bool get splitCommas; + + /// For [ArgsParser], set to true to hide this from the help menu. + bool get hide; + + /// For [ArgsParser], set to a single character to have a short version of the command line argument. + String get abbr; + + /// valueAt for arguments ignores the [dir] parameter and only uses command + /// line arguments and the current working directory to resolve the result. + @override + T valueAt(Directory dir) => _valueAtFromArgs() ?? defaultsTo; + + /// For passing in to [int.parse] and [double.parse] `onError'. + _throwErrorForTypes(String value) { + String example; + if (defaultsTo is Map) { + example = "key::value"; + } else if (_isInt) { + example = "32"; + } else if (_isDouble) { + example = "0.76"; + } + throw new DartdocOptionError( + 'Invalid argument value: --${argName}, set to "${value}", must be a ${T}. Example: --${argName} ${example}'); + } + + /// Returns null if no argument was given on the command line. + T _valueAtFromArgs() { + _OptionValueWithContext valueWithContext = _valueAtFromArgsWithContext(); + return _handlePathsInContext(valueWithContext); + } + + @override + void _onMissing( + _OptionValueWithContext valueWithContext, String missingPath) { + throw new DartdocFileMissing( + 'Argument --${argName}, set to ${valueWithContext.value}, resolves to missing path: "${missingPath}"'); + } + + /// Generates an _OptionValueWithContext using the value of the argument from + /// the [argParser] and the working directory from [Directory.current]. + /// + /// Throws [UnsupportedError] if [T] is not a supported type. + _OptionValueWithContext _valueAtFromArgsWithContext() { + if (!_argResults.wasParsed(argName)) return null; + + T retval; + // Unlike in _DartdocFileOption, we throw here on inputs being invalid rather + // than silently proceeding. TODO(jcollins-g): throw on input formatting for + // files too? + if (_isBool || _isListString || _isString) { + retval = _argResults[argName]; + } else if (_isInt) { + retval = + int.parse(_argResults[argName], onError: _throwErrorForTypes) as T; + } else if (_isDouble) { + retval = double.parse(_argResults[argName], _throwErrorForTypes) as T; + } else if (_isMapString) { + retval = {} as T; + for (String pair in _argResults[argName]) { + List pairList = pair.split('::'); + if (pairList.length != 2) { + _throwErrorForTypes(pair); + } + assert(pairList.length == 2); + (retval as Map)[pairList.first] = pairList.last; + } + } else { + throw UnsupportedError('Type ${T} is not supported'); + } + return new _OptionValueWithContext(retval, Directory.current.path); + } + + /// The name of this option as a command line argument. + String get argName => _keysToArgName(keys); + + /// Turns ['foo', 'somethingBar', 'over_the_hill'] into + /// 'something-bar-over-the-hill' (with default skip). + /// This allows argument names to reflect nested structure. + static String _keysToArgName(Iterable keys, [int skip = 1]) { + String argName = "${keys.skip(skip).join('-')}"; + argName = argName.replaceAll('_', '-'); + // Do not consume the lowercase character after the uppercase one, to handle + // two character words. + final camelCaseRegexp = new RegExp(r'([a-z])([A-Z])(?=([a-z]))'); + argName = argName.replaceAllMapped(camelCaseRegexp, (Match m) { + String before = m.group(1); + String after = m.group(2).toLowerCase(); + return "${before}-${after}"; + }); + return argName; + } + + /// If this argument is added to a larger tree of DartdocOptions, call + /// [ArgParser.addFlag], [ArgParser.addOption], or [ArgParser.addMultiOption] + /// as appropriate for [T]. + @override + void _onAdd() { + if (_isBool) { + argParser.addFlag(argName, + abbr: abbr, + defaultsTo: defaultsTo as bool, + help: help, + hide: hide, + negatable: negatable); + } else if (_isInt || _isDouble || _isString) { + argParser.addOption(argName, + abbr: abbr, + defaultsTo: defaultsTo.toString(), + help: help, + hide: hide); + } else if (_isListString || _isMapString) { + List defaultsToList = []; + if (_isListString) { + defaultsToList = defaultsTo as List; + } else { + defaultsToList.addAll((defaultsTo as Map) + .entries + .map((m) => "${m.key}::${m.value}")); + } + argParser.addMultiOption(argName, + abbr: abbr, + defaultsTo: defaultsToList, + help: help, + hide: hide, + splitCommas: splitCommas); + } else { + throw new UnsupportedError('Type ${T} is not supported'); + } + } +} + final Map _dartdocOptionsCache = {}; +/// Legacy dartdoc options class. TODO(jcollins-g): merge with [DartdocOption]. abstract class DartdocOptions { DartdocOptions(); diff --git a/test/dartdoc_options_test.dart b/test/dartdoc_options_test.dart index c878c130c9..03802cd09e 100644 --- a/test/dartdoc_options_test.dart +++ b/test/dartdoc_options_test.dart @@ -11,6 +11,9 @@ import 'package:path/path.dart' as pathLib; import 'package:test/test.dart'; void main() { + DartdocOptionSet dartdocOptionSetFiles; + DartdocOptionSet dartdocOptionSetArgs; + DartdocOptionSet dartdocOptionSetBoth; Directory tempDir; Directory firstDir; Directory secondDir; @@ -20,13 +23,87 @@ void main() { File dartdocOptionsOne; File dartdocOptionsTwo; File dartdocOptionsTwoFirstSub; + File firstExisting; setUpAll(() { + dartdocOptionSetFiles = new DartdocOptionSet('dartdoc'); + dartdocOptionSetFiles + .add(new DartdocOptionFileOnly>('categoryOrder', [])); + dartdocOptionSetFiles.add(new DartdocOptionFileOnly('double', 3.0)); + dartdocOptionSetFiles + .add(new DartdocOptionFileOnly('mySpecialInteger', 42)); + dartdocOptionSetFiles.add(new DartdocOptionFileOnly>( + 'mapOption', {'hello': 'world'})); + dartdocOptionSetFiles.add(new DartdocOptionFileOnly>( + 'fileOptionList', [], + isFile: true, mustExist: true)); + dartdocOptionSetFiles.add(new DartdocOptionFileOnly( + 'fileOption', null, + isFile: true, mustExist: true)); + dartdocOptionSetFiles.add(new DartdocOptionFileOnly( + 'parentOverride', 'oops', + parentDirOverridesChild: true)); + dartdocOptionSetFiles.add(new DartdocOptionFileOnly( + 'nonCriticalFileOption', null, + isFile: true)); + dartdocOptionSetFiles.add(new DartdocOptionSet('nestedOption') + ..addAll([new DartdocOptionFileOnly('flag', false)])); + dartdocOptionSetFiles.add(new DartdocOptionFileOnly( + 'dirOption', null, + isDir: true, mustExist: true)); + dartdocOptionSetFiles.add(new DartdocOptionFileOnly( + 'nonCriticalDirOption', null, + isDir: true)); + + dartdocOptionSetArgs = new DartdocOptionSet('dartdoc'); + dartdocOptionSetArgs + .add(new DartdocOptionArgOnly('cauliflowerSystem', false)); + dartdocOptionSetArgs + .add(new DartdocOptionArgOnly('extraSpecial', 'something')); + dartdocOptionSetArgs.add( + new DartdocOptionArgOnly>('excludeFiles', ['one', 'two'])); + dartdocOptionSetArgs + .add(new DartdocOptionArgOnly('number_of_heads', 3, abbr: 'h')); + dartdocOptionSetArgs + .add(new DartdocOptionArgOnly('respawnProbability', 0.2)); + dartdocOptionSetArgs.add(new DartdocOptionSet('warn') + ..addAll( + [new DartdocOptionArgOnly('unrecognizedVegetable', false)])); + dartdocOptionSetArgs.add(new DartdocOptionArgOnly>( + 'aFancyMapVariable', {'hello': 'map world'}, + splitCommas: true)); + dartdocOptionSetArgs.add(new DartdocOptionArgOnly>( + 'filesFlag', [], + isFile: true, mustExist: true)); + dartdocOptionSetArgs.add(new DartdocOptionArgOnly( + 'singleFile', 'hello', + isFile: true, mustExist: true)); + dartdocOptionSetArgs.add(new DartdocOptionArgOnly( + 'unimportantFile', 'whatever', + isFile: true)); + + dartdocOptionSetBoth = new DartdocOptionSet('dartdoc'); + dartdocOptionSetBoth + .add(new DartdocOptionBoth>('categoryOrder', [])); + dartdocOptionSetBoth + .add(new DartdocOptionBoth('mySpecialInteger', 91)); + dartdocOptionSetBoth.add(new DartdocOptionSet('warn') + ..addAll([new DartdocOptionBoth('unrecognizedVegetable', false)])); + dartdocOptionSetBoth.add(new DartdocOptionBoth>( + 'mapOption', {'hi': 'there'})); + dartdocOptionSetBoth + .add(new DartdocOptionBoth('notInAnyFile', 'so there')); + dartdocOptionSetBoth.add(new DartdocOptionBoth('fileOption', null, + isFile: true, mustExist: true)); + tempDir = Directory.systemTemp.createTempSync('options_test'); firstDir = new Directory(pathLib.join(tempDir.path, 'firstDir')) ..createSync(); + firstExisting = new File(pathLib.join(firstDir.path, 'existing.dart')) + ..createSync(); secondDir = new Directory(pathLib.join(tempDir.path, 'secondDir')) ..createSync(); + new File(pathLib.join(secondDir.path, 'existing.dart'))..createSync(); secondDirFirstSub = new Directory(pathLib.join(secondDir.path, 'firstSub')) ..createSync(); @@ -43,14 +120,31 @@ void main() { dartdocOptionsOne.writeAsStringSync(''' dartdoc: categoryOrder: ['options_one'] + mySpecialInteger: 30 + nestedOption: + flag: true + mapOption: + firstThing: yes + secondThing: stuff + fileOption: "existing.dart" + dirOption: "notHere" + nonCriticalFileOption: "whatever" + double: 3.3 '''); dartdocOptionsTwo.writeAsStringSync(''' dartdoc: categoryOrder: ['options_two'] + parentOverride: 'parent' + dirOption: 'firstSub' + fileOptionList: ['existing.dart', 'thing/that/does/not/exist'] + mySpecialInteger: 11 + fileOption: "not existing" '''); dartdocOptionsTwoFirstSub.writeAsStringSync(''' dartdoc: categoryOrder: ['options_two_first_sub'] + parentOverride: 'child' + nonCriticalDirOption: 'not_existing' '''); }); @@ -58,6 +152,354 @@ dartdoc: tempDir.deleteSync(recursive: true); }); + group('new style dartdoc both file and argument options', () { + test( + 'validate argument with wrong file throws error even if dartdoc_options is right', + () { + dartdocOptionSetBoth + .parseArguments(['--file-option', 'override-not-existing.dart']); + String errorMessage; + try { + dartdocOptionSetBoth['fileOption'].valueAt(firstDir); + } on DartdocFileMissing catch (e) { + errorMessage = e.message; + } + expect( + errorMessage, + equals( + 'Argument --file-option, set to override-not-existing.dart, resolves to missing path: "${pathLib.join(Directory.current.absolute.path, "override-not-existing.dart")}"')); + }); + + test('validate argument can override missing file', () { + dartdocOptionSetBoth + .parseArguments(['--file-option', firstExisting.absolute.path]); + expect(dartdocOptionSetBoth['fileOption'].valueAt(secondDir), + equals(firstExisting.absolute.path)); + }); + + test('File errors still get passed through', () { + dartdocOptionSetBoth.parseArguments([]); + String errorMessage; + try { + dartdocOptionSetBoth['fileOption'].valueAt(secondDir); + } on DartdocFileMissing catch (e) { + errorMessage = e.message; + } + expect( + errorMessage, + equals( + 'Field dartdoc.fileOption from ${dartdocOptionsTwo.absolute.path}, set to not existing, resolves to missing path: ' + '"${pathLib.join(secondDir.absolute.path, "not existing")}"')); + }); + + test('validate override behavior basic', () { + dartdocOptionSetBoth.parseArguments( + ['--not-in-any-file', 'aha', '--map-option', 'over::theSea']); + expect(dartdocOptionSetBoth['mapOption'].valueAt(tempDir), + equals({'over': 'theSea'})); + expect(dartdocOptionSetBoth['mapOption'].valueAt(firstDir), + equals({'over': 'theSea'})); + expect(dartdocOptionSetBoth['notInAnyFile'].valueAt(firstDir), + equals('aha')); + expect(dartdocOptionSetBoth['mySpecialInteger'].valueAt(firstDir), + equals(30)); + }); + + test('validate override behavior for parent directories', () { + dartdocOptionSetBoth.parseArguments(['--my-special-integer', '14']); + expect( + dartdocOptionSetBoth['mySpecialInteger'].valueAt(secondDirFirstSub), + equals(14)); + }); + + test('validate arg defaults do not override file', () { + dartdocOptionSetBoth.parseArguments([]); + expect(dartdocOptionSetBoth['mySpecialInteger'].valueAt(secondDir), + equals(11)); + }); + + test( + 'validate setting the default manually in an argument overrides the file', + () { + dartdocOptionSetBoth.parseArguments(['--my-special-integer', '91']); + expect(dartdocOptionSetBoth['mySpecialInteger'].valueAt(secondDir), + equals(91)); + }); + }); + + group('new style dartdoc arg-only options', () { + test('DartdocOptionArgOnly loads arg defaults', () { + dartdocOptionSetArgs.parseArguments([]); + expect( + dartdocOptionSetArgs['cauliflowerSystem'].valueAt(tempDir), isFalse); + expect(dartdocOptionSetArgs['extraSpecial'].valueAt(tempDir), + equals('something')); + expect(dartdocOptionSetArgs['excludeFiles'].valueAt(tempDir), + orderedEquals(['one', 'two'])); + expect( + dartdocOptionSetArgs['number_of_heads'].valueAt(tempDir), equals(3)); + expect(dartdocOptionSetArgs['respawnProbability'].valueAt(tempDir), + equals(0.2)); + expect( + dartdocOptionSetArgs['warn']['unrecognizedVegetable'] + .valueAt(tempDir), + equals(false)); + expect(dartdocOptionSetArgs['aFancyMapVariable'].valueAt(tempDir), + equals({'hello': 'map world'})); + expect( + dartdocOptionSetArgs['singleFile'].valueAt(tempDir), equals('hello')); + }); + + test('DartdocOptionArgOnly checks file existence', () { + String errorMessage; + dartdocOptionSetArgs.parseArguments(['--single-file', 'not_found.txt']); + try { + dartdocOptionSetArgs['singleFile'].valueAt(tempDir); + } on DartdocFileMissing catch (e) { + errorMessage = e.message; + } + expect( + errorMessage, + equals( + 'Argument --single-file, set to not_found.txt, resolves to missing path: ' + '"${Directory.current.absolute.path}/not_found.txt"')); + }); + + test('DartdocOptionArgOnly checks file existence on multi-options', () { + String errorMessage; + dartdocOptionSetArgs.parseArguments([ + '--files-flag', + firstExisting.absolute.path, + '--files-flag', + 'other_not_found.txt' + ]); + try { + dartdocOptionSetArgs['filesFlag'].valueAt(tempDir); + } on DartdocFileMissing catch (e) { + errorMessage = e.message; + } + expect( + errorMessage, + equals( + 'Argument --files-flag, set to [${firstExisting.absolute.path}, other_not_found.txt], resolves to missing path: ' + '"${pathLib.join(Directory.current.absolute.path, "other_not_found.txt")}"')); + }); + + test( + 'DartdocOptionArgOnly does not check for file existence when mustExist is false', + () { + dartdocOptionSetArgs + .parseArguments(['--unimportant-file', 'this-will-never-exist']); + expect( + dartdocOptionSetArgs['unimportantFile'].valueAt(tempDir), + equals(pathLib.join( + Directory.current.absolute.path, 'this-will-never-exist'))); + }); + + test('DartdocOptionArgOnly has correct flag names', () { + dartdocOptionSetArgs.parseArguments([]); + expect( + (dartdocOptionSetArgs['cauliflowerSystem'] as DartdocOptionArgOnly) + .argName, + equals('cauliflower-system')); + expect( + (dartdocOptionSetArgs['number_of_heads'] as DartdocOptionArgOnly) + .argName, + equals('number-of-heads')); + expect( + (dartdocOptionSetArgs['warn']['unrecognizedVegetable'] + as DartdocOptionArgOnly) + .argName, + equals('warn-unrecognized-vegetable')); + }); + + test('DartdocOptionArgOnly abbreviations work', () { + dartdocOptionSetArgs.parseArguments(['-h', '125']); + expect(dartdocOptionSetArgs['number_of_heads'].valueAt(tempDir), + equals(125)); + }); + + test('DartdocOptionArgOnly correctly handles some arguments', () { + dartdocOptionSetArgs.parseArguments([ + '--cauliflower-system', + '--exclude-files', + 'one', + '--exclude-files', + 'three', + '--number-of-heads', + '14', + '--warn-unrecognized-vegetable', + '--extra-special', + 'whatever', + '--respawn-probability', + '0.123', + '--a-fancy-map-variable', + 'aKey::aValue,another key::another value', + ]); + expect( + dartdocOptionSetArgs['cauliflowerSystem'].valueAt(tempDir), isTrue); + expect( + dartdocOptionSetArgs['number_of_heads'].valueAt(tempDir), equals(14)); + expect(dartdocOptionSetArgs['respawnProbability'].valueAt(tempDir), + equals(0.123)); + expect(dartdocOptionSetArgs['excludeFiles'].valueAt(tempDir), + equals(['one', 'three'])); + expect( + dartdocOptionSetArgs['warn']['unrecognizedVegetable'] + .valueAt(tempDir), + isTrue); + expect(dartdocOptionSetArgs['extraSpecial'].valueAt(tempDir), + equals('whatever')); + expect(dartdocOptionSetArgs['aFancyMapVariable'].valueAt(tempDir), + equals({'aKey': 'aValue', 'another key': 'another value'})); + }); + + test('DartdocOptionArgOnly throws on double type mismatch', () { + dartdocOptionSetArgs.parseArguments(['--respawn-probability', 'unknown']); + String errorMessage; + try { + dartdocOptionSetArgs['respawnProbability'].valueAt(tempDir); + } on DartdocOptionError catch (e) { + errorMessage = e.message; + } + expect( + errorMessage, + equals( + 'Invalid argument value: --respawn-probability, set to "unknown", must be a double. Example: --respawn-probability 0.76')); + }); + + test('DartdocOptionArgOnly throws on integer type mismatch', () { + dartdocOptionSetArgs.parseArguments(['--number-of-heads', '3.6']); + expect(() => dartdocOptionSetArgs['number_of_heads'].valueAt(tempDir), + throwsA(const isInstanceOf())); + String errorMessage; + try { + dartdocOptionSetArgs['number_of_heads'].valueAt(tempDir); + } on DartdocOptionError catch (e) { + errorMessage = e.message; + } + expect( + errorMessage, + equals( + 'Invalid argument value: --number-of-heads, set to "3.6", must be a int. Example: --number-of-heads 32')); + }); + + test('DartdocOptionArgOnly throws on a map type mismatch', () { + dartdocOptionSetArgs + .parseArguments(['--a-fancy-map-variable', 'not a map']); + String errorMessage; + try { + dartdocOptionSetArgs['aFancyMapVariable'].valueAt(tempDir); + } on DartdocOptionError catch (e) { + errorMessage = e.message; + } + expect( + errorMessage, + equals( + 'Invalid argument value: --a-fancy-map-variable, set to "not a map", must be a Map. Example: --a-fancy-map-variable key::value')); + }); + }); + + group('new style dartdoc file-only options', () { + test('DartdocOptionSetFile checks file existence when appropriate', () { + String errorMessage; + try { + dartdocOptionSetFiles['fileOptionList'].valueAt(secondDir); + } on DartdocFileMissing catch (e) { + errorMessage = e.message; + } + expect( + errorMessage, + equals( + 'Field dartdoc.fileOptionList from ${dartdocOptionsTwo.absolute.path}, set to [existing.dart, thing/that/does/not/exist], resolves to missing path: ' + '"${pathLib.joinAll([secondDir.absolute.path, 'thing', 'that', 'does', 'not', 'exist'])}"')); + // It doesn't matter that this fails. + expect(dartdocOptionSetFiles['nonCriticalFileOption'].valueAt(firstDir), + equals(pathLib.joinAll([firstDir.absolute.path, 'whatever']))); + }); + + test( + 'DartdocOptionSetFile resolves paths for files relative to where they are declared', + () { + String errorMessage; + try { + dartdocOptionSetFiles['fileOption'].valueAt(secondDirFirstSub); + } on DartdocFileMissing catch (e) { + errorMessage = e.message; + } + expect( + errorMessage, + equals( + 'Field dartdoc.fileOption from ${dartdocOptionsTwo.absolute.path}, set to not existing, resolves to missing path: ' + '"${pathLib.joinAll([secondDir.absolute.path, "not existing"])}"')); + }); + + test('DartdocOptionSetFile works for directory options', () { + expect( + dartdocOptionSetFiles['nonCriticalDirOption'] + .valueAt(secondDirFirstSub), + equals( + pathLib.join(secondDirFirstSub.absolute.path, 'not_existing'))); + }); + + test('DartdocOptionSetFile checks errors for directory options', () { + expect(dartdocOptionSetFiles['dirOption'].valueAt(secondDir), + equals(pathLib.join(secondDir.absolute.path, 'firstSub'))); + String errorMessage; + try { + dartdocOptionSetFiles['dirOption'].valueAt(firstDir); + } on DartdocFileMissing catch (e) { + errorMessage = e.message; + } + expect( + errorMessage, + equals( + 'Field dartdoc.dirOption from ${dartdocOptionsOne.absolute.path}, set to notHere, resolves to missing path: ' + '"${pathLib.joinAll([firstDir.absolute.path, "notHere"])}"')); + }); + + test('DartdocOptionSetFile loads defaults', () { + expect(dartdocOptionSetFiles['categoryOrder'].valueAt(tempDir), isEmpty); + expect(dartdocOptionSetFiles['nestedOption']['flag'].valueAt(tempDir), + equals(false)); + expect(dartdocOptionSetFiles['double'].valueAt(tempDir), equals(3.0)); + expect(dartdocOptionSetFiles['parentOverride'].valueAt(tempDir), + equals('oops')); + expect(dartdocOptionSetFiles['mySpecialInteger'].valueAt(tempDir), + equals(42)); + expect(dartdocOptionSetFiles['mapOption'].valueAt(tempDir), + equals({'hello': 'world'})); + }); + + test('DartdocOptionSetFile loads a file', () { + expect(dartdocOptionSetFiles['categoryOrder'].valueAt(firstDir), + orderedEquals(['options_one'])); + expect(dartdocOptionSetFiles['nestedOption']['flag'].valueAt(firstDir), + equals(true)); + expect(dartdocOptionSetFiles['double'].valueAt(firstDir), equals(3.3)); + expect(dartdocOptionSetFiles['mySpecialInteger'].valueAt(firstDir), + equals(30)); + expect(dartdocOptionSetFiles['mapOption'].valueAt(firstDir), + equals({'firstThing': 'yes', 'secondThing': 'stuff'})); + }); + + test('DartdocOptionSetFile loads a file in parent directories', () { + expect(dartdocOptionSetFiles['categoryOrder'].valueAt(secondDirSecondSub), + orderedEquals(['options_two'])); + }); + + test('DartdocOptionSetFile loads the override file instead of parents', () { + expect(dartdocOptionSetFiles['categoryOrder'].valueAt(secondDirFirstSub), + orderedEquals(['options_two_first_sub'])); + }); + + test('DartdocOptionSetFile lets parents override children when appropriate', + () { + expect(dartdocOptionSetFiles['parentOverride'].valueAt(secondDirFirstSub), + equals('parent')); + }); + }); + group('dartdoc options', () { group('options file finding and loading', () { test('DartdocOptions loads defaults', () { From 15e21da65b6eb4576c17eb176f664587798d64bd Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 12 Apr 2018 13:33:09 -0700 Subject: [PATCH 2/6] Windows drive letters :-( --- test/dartdoc_options_test.dart | 48 ++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/test/dartdoc_options_test.dart b/test/dartdoc_options_test.dart index 03802cd09e..9757a260e6 100644 --- a/test/dartdoc_options_test.dart +++ b/test/dartdoc_options_test.dart @@ -167,14 +167,14 @@ dartdoc: expect( errorMessage, equals( - 'Argument --file-option, set to override-not-existing.dart, resolves to missing path: "${pathLib.join(Directory.current.absolute.path, "override-not-existing.dart")}"')); + 'Argument --file-option, set to override-not-existing.dart, resolves to missing path: "${pathLib.join(pathLib.canonicalize(Directory.current.path), "override-not-existing.dart")}"')); }); test('validate argument can override missing file', () { - dartdocOptionSetBoth - .parseArguments(['--file-option', firstExisting.absolute.path]); + dartdocOptionSetBoth.parseArguments( + ['--file-option', pathLib.canonicalize(firstExisting.path)]); expect(dartdocOptionSetBoth['fileOption'].valueAt(secondDir), - equals(firstExisting.absolute.path)); + equals(pathLib.canonicalize(firstExisting.path))); }); test('File errors still get passed through', () { @@ -188,8 +188,8 @@ dartdoc: expect( errorMessage, equals( - 'Field dartdoc.fileOption from ${dartdocOptionsTwo.absolute.path}, set to not existing, resolves to missing path: ' - '"${pathLib.join(secondDir.absolute.path, "not existing")}"')); + 'Field dartdoc.fileOption from ${pathLib.canonicalize(dartdocOptionsTwo.path)}, set to not existing, resolves to missing path: ' + '"${pathLib.join(pathLib.canonicalize(secondDir.path), "not existing")}"')); }); test('validate override behavior basic', () { @@ -262,7 +262,7 @@ dartdoc: errorMessage, equals( 'Argument --single-file, set to not_found.txt, resolves to missing path: ' - '"${Directory.current.absolute.path}/not_found.txt"')); + '"${pathLib.canonicalize(Directory.current.path)}/not_found.txt"')); }); test('DartdocOptionArgOnly checks file existence on multi-options', () { @@ -282,7 +282,7 @@ dartdoc: errorMessage, equals( 'Argument --files-flag, set to [${firstExisting.absolute.path}, other_not_found.txt], resolves to missing path: ' - '"${pathLib.join(Directory.current.absolute.path, "other_not_found.txt")}"')); + '"${pathLib.join(pathLib.canonicalize(Directory.current.path), "other_not_found.txt")}"')); }); test( @@ -292,8 +292,8 @@ dartdoc: .parseArguments(['--unimportant-file', 'this-will-never-exist']); expect( dartdocOptionSetArgs['unimportantFile'].valueAt(tempDir), - equals(pathLib.join( - Directory.current.absolute.path, 'this-will-never-exist'))); + equals(pathLib.join(pathLib.canonicalize(Directory.current.path), + 'this-will-never-exist'))); }); test('DartdocOptionArgOnly has correct flag names', () { @@ -411,11 +411,13 @@ dartdoc: expect( errorMessage, equals( - 'Field dartdoc.fileOptionList from ${dartdocOptionsTwo.absolute.path}, set to [existing.dart, thing/that/does/not/exist], resolves to missing path: ' - '"${pathLib.joinAll([secondDir.absolute.path, 'thing', 'that', 'does', 'not', 'exist'])}"')); + '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'])}"')); // It doesn't matter that this fails. - expect(dartdocOptionSetFiles['nonCriticalFileOption'].valueAt(firstDir), - equals(pathLib.joinAll([firstDir.absolute.path, 'whatever']))); + expect( + dartdocOptionSetFiles['nonCriticalFileOption'].valueAt(firstDir), + equals(pathLib + .joinAll([pathLib.canonicalize(firstDir.path), 'whatever']))); }); test( @@ -430,21 +432,23 @@ dartdoc: expect( errorMessage, equals( - 'Field dartdoc.fileOption from ${dartdocOptionsTwo.absolute.path}, set to not existing, resolves to missing path: ' - '"${pathLib.joinAll([secondDir.absolute.path, "not existing"])}"')); + 'Field dartdoc.fileOption from ${pathLib.canonicalize(dartdocOptionsTwo.path)}, set to not existing, resolves to missing path: ' + '"${pathLib.joinAll([pathLib.canonicalize(secondDir.path), "not existing"])}"')); }); test('DartdocOptionSetFile works for directory options', () { expect( dartdocOptionSetFiles['nonCriticalDirOption'] .valueAt(secondDirFirstSub), - equals( - pathLib.join(secondDirFirstSub.absolute.path, 'not_existing'))); + equals(pathLib.join( + pathLib.canonicalize(secondDirFirstSub.path), 'not_existing'))); }); test('DartdocOptionSetFile checks errors for directory options', () { - expect(dartdocOptionSetFiles['dirOption'].valueAt(secondDir), - equals(pathLib.join(secondDir.absolute.path, 'firstSub'))); + expect( + dartdocOptionSetFiles['dirOption'].valueAt(secondDir), + equals( + pathLib.join(pathLib.canonicalize(secondDir.path), 'firstSub'))); String errorMessage; try { dartdocOptionSetFiles['dirOption'].valueAt(firstDir); @@ -454,8 +458,8 @@ dartdoc: expect( errorMessage, equals( - 'Field dartdoc.dirOption from ${dartdocOptionsOne.absolute.path}, set to notHere, resolves to missing path: ' - '"${pathLib.joinAll([firstDir.absolute.path, "notHere"])}"')); + 'Field dartdoc.dirOption from ${pathLib.canonicalize(dartdocOptionsOne.path)}, set to notHere, resolves to missing path: ' + '"${pathLib.joinAll([pathLib.canonicalize(firstDir.path), "notHere"])}"')); }); test('DartdocOptionSetFile loads defaults', () { From ad739e51d2001e10a9cea38aaade82dada94e7e5 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 12 Apr 2018 13:34:39 -0700 Subject: [PATCH 3/6] Forgot a slash --- test/dartdoc_options_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dartdoc_options_test.dart b/test/dartdoc_options_test.dart index 9757a260e6..ea6b79ed8c 100644 --- a/test/dartdoc_options_test.dart +++ b/test/dartdoc_options_test.dart @@ -262,7 +262,7 @@ dartdoc: errorMessage, equals( 'Argument --single-file, set to not_found.txt, resolves to missing path: ' - '"${pathLib.canonicalize(Directory.current.path)}/not_found.txt"')); + '"${pathLib.join(pathLib.canonicalize(Directory.current.path), 'not_found.txt')}"')); }); test('DartdocOptionArgOnly checks file existence on multi-options', () { From 996b4b73dc657b9625f4e971168bd58e1d9bedc7 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 12 Apr 2018 13:36:29 -0700 Subject: [PATCH 4/6] Canonicalization missed one spot. --- test/dartdoc_options_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dartdoc_options_test.dart b/test/dartdoc_options_test.dart index ea6b79ed8c..4c4647df9b 100644 --- a/test/dartdoc_options_test.dart +++ b/test/dartdoc_options_test.dart @@ -448,7 +448,7 @@ dartdoc: expect( dartdocOptionSetFiles['dirOption'].valueAt(secondDir), equals( - pathLib.join(pathLib.canonicalize(secondDir.path), 'firstSub'))); + pathLib.canonicalize(pathLib.join(secondDir.path, 'firstSub')))); String errorMessage; try { dartdocOptionSetFiles['dirOption'].valueAt(firstDir); From fec2bda1f68260d2fb22381a02cd227d936cc13b Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 12 Apr 2018 13:38:59 -0700 Subject: [PATCH 5/6] One more capitalization problem --- test/dartdoc_options_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dartdoc_options_test.dart b/test/dartdoc_options_test.dart index 4c4647df9b..6c22d20f92 100644 --- a/test/dartdoc_options_test.dart +++ b/test/dartdoc_options_test.dart @@ -459,7 +459,7 @@ dartdoc: errorMessage, equals( 'Field dartdoc.dirOption from ${pathLib.canonicalize(dartdocOptionsOne.path)}, set to notHere, resolves to missing path: ' - '"${pathLib.joinAll([pathLib.canonicalize(firstDir.path), "notHere"])}"')); + '"${pathLib.canonicalize(pathLib.join(firstDir.path, "notHere"))}"')); }); test('DartdocOptionSetFile loads defaults', () { From 0d97cadef1bb31a87ef3d45d542d9781ff0753c9 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Fri, 13 Apr 2018 10:35:41 -0700 Subject: [PATCH 6/6] Review comments --- bin/dartdoc.dart | 10 ++++----- lib/dartdoc.dart | 40 +++++++++++++++++----------------- lib/src/config.dart | 16 +++++++------- lib/src/dartdoc_options.dart | 37 ++++++++++++++++--------------- lib/src/model.dart | 8 +++---- lib/src/package_meta.dart | 4 ++-- pubspec.lock | 2 +- test/dartdoc_options_test.dart | 2 +- test/dartdoc_test.dart | 36 +++++++++++++++--------------- test/src/utils.dart | 4 ++-- 10 files changed, 80 insertions(+), 79 deletions(-) diff --git a/bin/dartdoc.dart b/bin/dartdoc.dart index 8204b8b80d..b0a4632206 100644 --- a/bin/dartdoc.dart +++ b/bin/dartdoc.dart @@ -200,7 +200,7 @@ main(List arguments) async { ]); } - DartDocConfig config = new DartDocConfig.fromParameters( + DartdocConfig config = new DartdocConfig.fromParameters( addCrossdart: args['add-crossdart'], autoIncludeDependencies: args['auto-include-dependencies'], dropTextFrom: dropTextFrom, @@ -230,20 +230,20 @@ main(List arguments) async { verboseWarnings: args['verbose-warnings'], ); - DartDoc dartdoc = - await DartDoc.withDefaultGenerators(config, outputDir, packageMeta); + Dartdoc dartdoc = + await Dartdoc.withDefaultGenerators(config, outputDir, packageMeta); dartdoc.onCheckProgress.listen(logProgress); await Chain.capture(() async { await runZoned(() async { - DartDocResults results = await dartdoc.generateDocs(); + DartdocResults results = await dartdoc.generateDocs(); logInfo('Success! Docs generated into ${results.outDir.absolute.path}'); }, zoneSpecification: new ZoneSpecification( print: (Zone self, ZoneDelegate parent, Zone zone, String line) => logPrint(line))); }, onError: (e, Chain chain) { - if (e is DartDocFailure) { + if (e is DartdocFailure) { stderr.writeln('\nGeneration failed: ${e}.'); exit(1); } else { diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart index 13e5796d9e..37c6c78519 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -65,7 +65,7 @@ Future> _initGenerators(String url, String relCanonicalPrefix, /// Generates Dart documentation for all public Dart libraries in the given /// directory. -class DartDoc extends PackageBuilder { +class Dartdoc extends PackageBuilder { final List generators; final Directory outputDir; final Set writtenFiles = new Set(); @@ -74,13 +74,13 @@ class DartDoc extends PackageBuilder { final StreamController _onCheckProgress = new StreamController(sync: true); - DartDoc._(DartDocConfig config, this.generators, this.outputDir, + Dartdoc._(DartdocConfig config, this.generators, this.outputDir, PackageMeta packageMeta) : super(config, packageMeta); /// An asynchronous factory method that builds Dartdoc's file writers - /// and returns a DartDoc object with them. - static withDefaultGenerators(DartDocConfig config, Directory outputDir, + /// and returns a Dartdoc object with them. + static withDefaultGenerators(DartdocConfig config, Directory outputDir, PackageMeta packageMeta) async { var generators = await _initGenerators( config.hostedUrl, config.relCanonicalPrefix, @@ -92,12 +92,12 @@ class DartDoc extends PackageBuilder { for (var generator in generators) { generator.onFileCreated.listen(logProgress); } - return new DartDoc._(config, generators, outputDir, packageMeta); + return new Dartdoc._(config, generators, outputDir, packageMeta); } - factory DartDoc.withoutGenerators( - DartDocConfig config, Directory outputDir, PackageMeta packageMeta) { - return new DartDoc._(config, [], outputDir, packageMeta); + factory Dartdoc.withoutGenerators( + DartdocConfig config, Directory outputDir, PackageMeta packageMeta) { + return new Dartdoc._(config, [], outputDir, packageMeta); } Stream get onCheckProgress => _onCheckProgress.stream; @@ -145,19 +145,19 @@ class DartDoc extends PackageBuilder { if (errors.isNotEmpty) { int len = errors.length; - throw new DartDocFailure( + throw new DartdocFailure( "encountered ${len} analysis error${len == 1 ? '' : 's'}"); } } PackageGraph packageGraph; - /// Generate DartDoc documentation. + /// Generate Dartdoc documentation. /// - /// [DartDocResults] is returned if dartdoc succeeds. [DartDocFailure] is + /// [DartdocResults] is returned if dartdoc succeeds. [DartdocFailure] is /// thrown if dartdoc fails in an expected way, for example if there is an /// analysis error in the code. - Future generateDocs() async { + Future generateDocs() async { Stopwatch _stopwatch = new Stopwatch()..start(); double seconds; packageGraph = await buildPackageGraph(); @@ -193,15 +193,15 @@ class DartDoc extends PackageBuilder { "in ${seconds.toStringAsFixed(1)} seconds"); if (packageGraph.publicLibraries.isEmpty) { - throw new DartDocFailure( + throw new DartdocFailure( "dartdoc could not find any libraries to document. Run `pub get` and try again."); } if (packageGraph.packageWarningCounter.errorCount > 0) { - throw new DartDocFailure("dartdoc encountered errors while processing"); + throw new DartdocFailure("dartdoc encountered errors while processing"); } - return new DartDocResults(packageMeta, packageGraph, outputDir); + return new DartdocResults(packageMeta, packageGraph, outputDir); } /// Warn on file paths. @@ -421,22 +421,22 @@ class DartDoc extends PackageBuilder { /// This class is returned if dartdoc fails in an expected way (for instance, if /// there is an analysis error in the library). -class DartDocFailure { +class DartdocFailure { final String message; - DartDocFailure(this.message); + DartdocFailure(this.message); @override String toString() => message; } -/// The results of a [DartDoc.generateDocs] call. -class DartDocResults { +/// The results of a [Dartdoc.generateDocs] call. +class DartdocResults { final PackageMeta packageMeta; final PackageGraph packageGraph; final Directory outDir; - DartDocResults(this.packageMeta, this.packageGraph, this.outDir); + DartdocResults(this.packageMeta, this.packageGraph, this.outDir); } class _Error implements Comparable<_Error> { diff --git a/lib/src/config.dart b/lib/src/config.dart index 1af09e4a94..6fd1e1e769 100644 --- a/lib/src/config.dart +++ b/lib/src/config.dart @@ -26,7 +26,7 @@ String _resolveTildePath(String originalPath) { return pathLib.join(homeDir, originalPath.substring(2)); } -class DartDocConfig { +class DartdocConfig { final bool addCrossdart; final bool autoIncludeDependencies; final List dropTextFrom; @@ -51,7 +51,7 @@ class DartDocConfig { final bool showWarnings; final bool validateLinks; final bool verboseWarnings; - DartDocConfig._( + DartdocConfig._( this.addCrossdart, this.autoIncludeDependencies, this.dropTextFrom, @@ -78,13 +78,13 @@ class DartDocConfig { this.verboseWarnings, ) { if (sdkDir == null || !sdkDir.existsSync()) { - throw new DartDocFailure("Error: unable to locate the Dart SDK."); + throw new DartdocFailure("Error: unable to locate the Dart SDK."); } footerFilePaths = footerFilePaths.map((p) => _resolveTildePath(p)).toList(); for (String footerFilePath in footerFilePaths) { if (!new File(footerFilePath).existsSync()) { - throw new DartDocFailure( + throw new DartdocFailure( "fatal error: unable to locate footer file: ${footerFilePath}."); } } @@ -93,7 +93,7 @@ class DartDocConfig { footerTextFilePaths.map((p) => _resolveTildePath(p)).toList(); for (String footerTextFilePath in footerTextFilePaths) { if (!new File(footerTextFilePath).existsSync()) { - throw new DartDocFailure( + throw new DartdocFailure( "fatal error: unable to locate footer-text file: ${footerTextFilePath}."); } } @@ -101,13 +101,13 @@ class DartDocConfig { headerFilePaths = headerFilePaths.map((p) => _resolveTildePath(p)).toList(); for (String headerFilePath in headerFilePaths) { if (!new File(headerFilePath).existsSync()) { - throw new DartDocFailure( + throw new DartdocFailure( "fatal error: unable to locate header file: ${headerFilePath}."); } } } - factory DartDocConfig.fromParameters({ + factory DartdocConfig.fromParameters({ bool addCrossdart: false, bool autoIncludeDependencies: false, List dropTextFrom, @@ -133,7 +133,7 @@ class DartDocConfig { bool validateLinks: true, bool verboseWarnings: true, }) { - return new DartDocConfig._( + return new DartdocConfig._( addCrossdart, autoIncludeDependencies, dropTextFrom ?? const [], diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart index dd225be6c5..b039b0517d 100644 --- a/lib/src/dartdoc_options.dart +++ b/lib/src/dartdoc_options.dart @@ -32,7 +32,7 @@ const int _kIntVal = 0; const double _kDoubleVal = 0.0; const bool _kBoolVal = true; -class DartdocOptionError extends DartDocFailure { +class DartdocOptionError extends DartdocFailure { DartdocOptionError(String details) : super(details); } @@ -85,7 +85,7 @@ class _OptionValueWithContext { } else if (value is String) { return pathContext.canonicalize(value as String) as T; } else { - throw new UnsupportedError("Type $T is not supported for resolvedValue"); + throw new UnsupportedError('Type $T is not supported for resolvedValue'); } } } @@ -371,9 +371,9 @@ class DartdocOptionBoth extends DartdocOption dartdocYaml = pathLib.canonicalize(pathLib.join( valueWithContext.canonicalDirectoryPath, valueWithContext.definingFile)); - description = "Field ${fieldName} from ${dartdocYaml}"; + description = 'Field ${fieldName} from ${dartdocYaml}'; } else { - description = "Argument --${argName}"; + description = 'Argument --${argName}'; } throw new DartdocFileMissing( '$description, set to ${valueWithContext.value}, resolves to missing path: "${missingPath}"'); @@ -429,7 +429,7 @@ class DartdocOptionFileOnly extends DartdocOption bool get parentDirOverridesChild => _parentDirOverridesChild; } -/// Implements checking for options +/// Implements checking for options contained in dartdoc.yaml. abstract class _DartdocFileOption implements DartdocOption { /// If true, the parent directory's value overrides the child's. Otherwise, the child's /// value overrides values in parents. @@ -504,7 +504,7 @@ abstract class _DartdocFileOption implements DartdocOption { _OptionValueWithContext _valueAtFromFile(Directory dir) { _YamlFileData yamlFileData = _yamlAtDirectory(dir); String contextPath = yamlFileData.canonicalDirectoryPath; - var yamlData = yamlFileData.data; + dynamic yamlData = yamlFileData.data; for (String key in keys) { if (!yamlData.containsKey(key)) return null; yamlData = yamlData[key]; @@ -514,7 +514,7 @@ abstract class _DartdocFileOption implements DartdocOption { if (_isListString) { if (yamlData is YamlList) { returnData = []; - for (var item in yamlData as YamlList) { + for (var item in yamlData) { returnData.add(item.toString()); } } @@ -527,14 +527,14 @@ abstract class _DartdocFileOption implements DartdocOption { } } else if (_isDouble) { if (yamlData is num) { - returnData = (yamlData as num).toDouble(); + returnData = yamlData.toDouble(); } } else if (_isInt || _isString || _isBool) { if (yamlData is T) { returnData = yamlData; } } else { - throw new UnsupportedError("Type ${T} is not supported"); + throw new UnsupportedError('Type ${T} is not supported'); } return new _OptionValueWithContext(returnData as T, contextPath, definingFile: 'dartdoc_options.yaml'); @@ -591,11 +591,11 @@ abstract class _DartdocArgOption implements DartdocOption { _throwErrorForTypes(String value) { String example; if (defaultsTo is Map) { - example = "key::value"; + example = 'key::value'; } else if (_isInt) { - example = "32"; + example = '32'; } else if (_isDouble) { - example = "0.76"; + example = '0.76'; } throw new DartdocOptionError( 'Invalid argument value: --${argName}, set to "${value}", must be a ${T}. Example: --${argName} ${example}'); @@ -628,10 +628,11 @@ abstract class _DartdocArgOption implements DartdocOption { if (_isBool || _isListString || _isString) { retval = _argResults[argName]; } else if (_isInt) { - retval = - int.parse(_argResults[argName], onError: _throwErrorForTypes) as T; + retval = int.tryParse(_argResults[argName]) as T; + if (retval == null) _throwErrorForTypes(_argResults[argName]); } else if (_isDouble) { - retval = double.parse(_argResults[argName], _throwErrorForTypes) as T; + retval = double.tryParse(_argResults[argName]) as T; + if (retval == null) _throwErrorForTypes(_argResults[argName]); } else if (_isMapString) { retval = {} as T; for (String pair in _argResults[argName]) { @@ -663,7 +664,7 @@ abstract class _DartdocArgOption implements DartdocOption { argName = argName.replaceAllMapped(camelCaseRegexp, (Match m) { String before = m.group(1); String after = m.group(2).toLowerCase(); - return "${before}-${after}"; + return '${before}-${after}'; }); return argName; } @@ -693,7 +694,7 @@ abstract class _DartdocArgOption implements DartdocOption { } else { defaultsToList.addAll((defaultsTo as Map) .entries - .map((m) => "${m.key}::${m.value}")); + .map((m) => '${m.key}::${m.value}')); } argParser.addMultiOption(argName, abbr: abbr, @@ -789,7 +790,7 @@ class _FileDartdocOptions extends DartdocOptions { if (_dartdocOptions['categoryOrder'] is YamlList) { _categoryOrder.addAll(_dartdocOptions['categoryOrder']); } else { - logWarning("${_path}: categoryOrder must be a list (ignoring)"); + logWarning('${_path}: categoryOrder must be a list (ignoring)'); } } _categoryOrder = new List.unmodifiable(_categoryOrder); diff --git a/lib/src/model.dart b/lib/src/model.dart index f2ce7bc009..82211a556c 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -1155,7 +1155,7 @@ abstract class Documentable extends Nameable { String get oneLineDoc; PackageGraph get packageGraph; bool get isDocumented; - DartDocConfig get config; + DartdocConfig get config; } /// Mixin implementing dartdoc categorization for ModelElements. @@ -2759,7 +2759,7 @@ abstract class ModelElement extends Canonicalization } @override - DartDocConfig get config => packageGraph.config; + DartdocConfig get config => packageGraph.config; @override Set get locationPieces { @@ -3833,7 +3833,7 @@ class PackageGraph extends Canonicalization /// Dartdoc's configuration flags. @override - final DartDocConfig config; + final DartdocConfig config; Map>>> __crossdartJson; // TODO(jcollins-g): move to [Package] @@ -5224,7 +5224,7 @@ class TypeParameter extends ModelElement { /// Everything you need to instantiate a PackageGraph object for documenting. class PackageBuilder { final PackageMeta packageMeta; - final DartDocConfig config; + final DartdocConfig config; PackageBuilder(this.config, this.packageMeta); diff --git a/lib/src/package_meta.dart b/lib/src/package_meta.dart index e14411cb77..4e5ca8c251 100644 --- a/lib/src/package_meta.dart +++ b/lib/src/package_meta.dart @@ -21,7 +21,7 @@ Directory get defaultSdkDir { return sdkDir; } -class PackageMetaFailure extends DartDocFailure { +class PackageMetaFailure extends DartdocFailure { PackageMetaFailure(String message) : super(message); } @@ -81,7 +81,7 @@ abstract class PackageMeta { /// Use this instead of fromDir where possible. factory PackageMeta.fromElement( - LibraryElement libraryElement, DartDocConfig config) { + LibraryElement libraryElement, DartdocConfig config) { // Workaround for dart-lang/sdk#32707. Replace with isInSdk once that works. if (libraryElement.source.uri.scheme == 'dart') return new PackageMeta.fromDir(config.sdkDir); diff --git a/pubspec.lock b/pubspec.lock index c14f22efa6..4432703f56 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -408,4 +408,4 @@ packages: source: hosted version: "2.1.13" sdks: - dart: ">=2.0.0-dev.23.0 <=2.0.0-dev.47.0" + dart: ">=2.0.0-dev.23.0 <=2.0.0-dev.48.0" diff --git a/test/dartdoc_options_test.dart b/test/dartdoc_options_test.dart index 6c22d20f92..e9f4e0b696 100644 --- a/test/dartdoc_options_test.dart +++ b/test/dartdoc_options_test.dart @@ -243,7 +243,7 @@ dartdoc: expect( dartdocOptionSetArgs['warn']['unrecognizedVegetable'] .valueAt(tempDir), - equals(false)); + isFalse); expect(dartdocOptionSetArgs['aFancyMapVariable'].valueAt(tempDir), equals({'hello': 'map world'})); expect( diff --git a/test/dartdoc_test.dart b/test/dartdoc_test.dart index 2b207cf52f..a0f6237145 100644 --- a/test/dartdoc_test.dart +++ b/test/dartdoc_test.dart @@ -29,12 +29,12 @@ void main() { test('generate docs for ${pathLib.basename(testPackageDir.path)} works', () async { PackageMeta meta = new PackageMeta.fromDir(testPackageDir); - DartDoc dartdoc = new DartDoc.withoutGenerators( - new DartDocConfig.fromParameters(inputDir: testPackageDir), + Dartdoc dartdoc = new Dartdoc.withoutGenerators( + new DartdocConfig.fromParameters(inputDir: testPackageDir), tempDir, meta); - DartDocResults results = await dartdoc.generateDocs(); + DartdocResults results = await dartdoc.generateDocs(); expect(results.packageGraph, isNotNull); PackageGraph p = results.packageGraph; @@ -47,8 +47,8 @@ void main() { test('generate docs for ${pathLib.basename(testPackageBadDir.path)} fails', () async { PackageMeta meta = new PackageMeta.fromDir(testPackageBadDir); - DartDoc dartdoc = new DartDoc.withoutGenerators( - new DartDocConfig.fromParameters(inputDir: testPackageBadDir), + Dartdoc dartdoc = new Dartdoc.withoutGenerators( + new DartdocConfig.fromParameters(inputDir: testPackageBadDir), tempDir, meta); @@ -56,18 +56,18 @@ void main() { await dartdoc.generateDocs(); fail('dartdoc should fail on analysis errors'); } catch (e) { - expect(e is DartDocFailure, isTrue); + expect(e is DartdocFailure, isTrue); } }); test('generate docs for a package that does not have a readme', () async { PackageMeta meta = new PackageMeta.fromDir(testPackageWithNoReadme); - DartDoc dartdoc = new DartDoc.withoutGenerators( - new DartDocConfig.fromParameters(inputDir: testPackageWithNoReadme), + Dartdoc dartdoc = new Dartdoc.withoutGenerators( + new DartdocConfig.fromParameters(inputDir: testPackageWithNoReadme), tempDir, meta); - DartDocResults results = await dartdoc.generateDocs(); + DartdocResults results = await dartdoc.generateDocs(); expect(results.packageGraph, isNotNull); PackageGraph p = results.packageGraph; @@ -79,13 +79,13 @@ void main() { test('generate docs including a single library', () async { PackageMeta meta = new PackageMeta.fromDir(testPackageDir); - DartDoc dartdoc = new DartDoc.withoutGenerators( - new DartDocConfig.fromParameters( + Dartdoc dartdoc = new Dartdoc.withoutGenerators( + new DartdocConfig.fromParameters( inputDir: testPackageDir, includeLibraries: ['fake']), tempDir, meta); - DartDocResults results = await dartdoc.generateDocs(); + DartdocResults results = await dartdoc.generateDocs(); expect(results.packageGraph, isNotNull); PackageGraph p = results.packageGraph; @@ -97,13 +97,13 @@ void main() { test('generate docs excluding a single library', () async { PackageMeta meta = new PackageMeta.fromDir(testPackageDir); - DartDoc dartdoc = new DartDoc.withoutGenerators( - new DartDocConfig.fromParameters( + Dartdoc dartdoc = new Dartdoc.withoutGenerators( + new DartdocConfig.fromParameters( inputDir: testPackageDir, excludeLibraries: ['fake']), tempDir, meta); - DartDocResults results = await dartdoc.generateDocs(); + DartdocResults results = await dartdoc.generateDocs(); expect(results.packageGraph, isNotNull); PackageGraph p = results.packageGraph; @@ -117,13 +117,13 @@ void main() { test('generate docs for package with embedder yaml', () async { PackageMeta meta = new PackageMeta.fromDir(testPackageWithEmbedderYaml); if (meta.needsPubGet) meta.runPubGet(); - DartDoc dartdoc = new DartDoc.withoutGenerators( - new DartDocConfig.fromParameters( + Dartdoc dartdoc = new Dartdoc.withoutGenerators( + new DartdocConfig.fromParameters( inputDir: testPackageWithEmbedderYaml), tempDir, meta); - DartDocResults results = await dartdoc.generateDocs(); + DartdocResults results = await dartdoc.generateDocs(); expect(results.packageGraph, isNotNull); PackageGraph p = results.packageGraph; diff --git a/test/src/utils.dart b/test/src/utils.dart index 6d00c69ee2..34593fb3db 100644 --- a/test/src/utils.dart +++ b/test/src/utils.dart @@ -49,7 +49,7 @@ init() async { Future bootSdkPackage() { Directory dir = new Directory(pathLib.current); return new PackageBuilder( - new DartDocConfig.fromParameters( + new DartdocConfig.fromParameters( inputDir: dir, sdkDir: sdkDir, ), @@ -62,7 +62,7 @@ Future bootBasicPackage( {bool withAutoIncludedDependencies = false, bool withCrossdart = false}) { Directory dir = new Directory(dirPath); return new PackageBuilder( - new DartDocConfig.fromParameters( + new DartdocConfig.fromParameters( inputDir: dir, sdkDir: sdkDir, excludeLibraries: excludeLibraries,