diff --git a/README.md b/README.md index 7225a024aa..748852db09 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,7 @@ authoring doc comments for Dart with `dartdoc`. Creating a file named dartdoc_options.yaml at the top of your package can change how Dartdoc generates docs. -An example: +An example (not necessarily recommended settings): ```yaml dartdoc: @@ -119,6 +119,12 @@ dartdoc: linkTo: url: "https://my.dartdocumentationsite.org/dev/%v%" showUndocumentedCategories: true + ignore: + - ambiguous-doc-reference + errors: + - unresolved-doc-reference + warnings: + - tool-error ``` Unrecognized options will be ignored. Supported options: @@ -133,10 +139,14 @@ Unrecognized options will be ignored. Supported options: directives. * **exclude**: Specify a list of library names to avoid generating docs for, overriding any specified in include. + * **errors**: Specify warnings to be treated as errors. See the lists of valid warnings in the command + line help for `--errors`, `--warnings`, and `--ignore`. * **favicon**: A path to a favicon for the generated docs. * **footer**: A list of paths to footer files containing HTML text. * **footerText**: A list of paths to text files for optional text next to the package name and version * **header**: A list of paths to header files containing HTML text. + * **ignore**: Specify warnings to be completely ignored. See the lists of valid warnings in the command + line help for `--errors`, `--warnings`, and `--ignore`. * **include**: Specify a list of library names to generate docs for, ignoring all others. * **includeExternal**: Specify a list of library filenames to add to the list of documented libraries. * **linkTo**: For other packages depending on this one, if this map is defined those packages @@ -152,6 +162,8 @@ Unrecognized options will be ignored. Supported options: No branch is considered to be "stable". * `%n%`: The name of this package, as defined in pubspec.yaml. * `%v%`: The version of this package as defined in pubspec.yaml. + * **warnings**: Specify otherwise ignored or set-to-error warnings to simply warn. See the lists + of valid warnings in the command line help for `--errors`, `--warnings`, and `--ignore`. In general, paths are relative to the directory the dartdoc_options.yaml the option is defined in and should be specified as POSIX paths. Dartdoc will convert POSIX paths automatically on Windows. diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart index 8e6d186a89..f2abfdf8ea 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -131,7 +131,6 @@ class Dartdoc extends PackageBuilder { final int errorCount = dartdocResults.packageGraph.packageWarningCounter.errorCount; if (errorCount > 0) { - dartdocResults.packageGraph.flushWarnings(); throw new DartdocFailure( "dartdoc encountered $errorCount} errors while processing."); } diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart index bb95737670..e97fe4fdbe 100644 --- a/lib/src/dartdoc_options.dart +++ b/lib/src/dartdoc_options.dart @@ -24,6 +24,7 @@ import 'package:dartdoc/src/experiment_options.dart'; import 'package:dartdoc/src/io_utils.dart'; import 'package:dartdoc/src/tool_runner.dart'; import 'package:dartdoc/src/tuple.dart'; +import 'package:dartdoc/src/warnings.dart'; import 'package:path/path.dart' as pathLib; import 'package:yaml/yaml.dart'; @@ -1271,7 +1272,7 @@ abstract class DartdocOptionContextBase { /// a single [ModelElement], [Package], [Category] and so forth has a single context /// and so this can be made a member variable of those structures. class DartdocOptionContext extends DartdocOptionContextBase - with DartdocExperimentOptionContext { + with DartdocExperimentOptionContext, PackageWarningOptionContext { @override final DartdocOptionSet optionSet; @override @@ -1357,12 +1358,10 @@ class DartdocOptionContext extends DartdocOptionContextBase String get sdkDir => optionSet['sdkDir'].valueAt(context); bool get showUndocumentedCategories => optionSet['showUndocumentedCategories'].valueAt(context); - bool get showWarnings => optionSet['showWarnings'].valueAt(context); PackageMeta get topLevelPackageMeta => optionSet['topLevelPackageMeta'].valueAt(context); bool get useCategories => optionSet['useCategories'].valueAt(context); bool get validateLinks => optionSet['validateLinks'].valueAt(context); - bool get verboseWarnings => optionSet['verboseWarnings'].valueAt(context); bool isLibraryExcluded(String name) => exclude.any((pattern) => name == pattern); @@ -1540,8 +1539,6 @@ Future> createDartdocOptions() async { }, help: 'Path to the SDK directory.', isDir: true, mustExist: true), new DartdocOptionArgFile('showUndocumentedCategories', false, help: "Label categories that aren't documented", negatable: true), - new DartdocOptionArgOnly('showWarnings', false, - help: 'Display all warnings.'), new DartdocOptionSyntheticOnly('topLevelPackageMeta', (DartdocSyntheticOption option, Directory dir) { PackageMeta packageMeta = new PackageMeta.fromDir( @@ -1574,5 +1571,7 @@ Future> createDartdocOptions() async { 'command.'), // TODO(jcollins-g): refactor so there is a single static "create" for // each DartdocOptionContext that traverses the inheritance tree itself. - ]..addAll(await createExperimentOptions()); + ] + ..addAll(await createExperimentOptions()) + ..addAll(await createPackageWarningOptions()); } diff --git a/lib/src/model.dart b/lib/src/model.dart index b72ee5b525..783adb0759 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -3632,6 +3632,7 @@ abstract class ModelElement extends Canonicalization @override PackageGraph get packageGraph => _packageGraph; + @override Package get package => library.package; bool get isPublicAndPackageDocumented => @@ -4621,15 +4622,10 @@ class Operator extends Method { class PackageGraph { PackageGraph.UninitializedPackageGraph( - this.config, - PackageWarningOptions packageWarningOptions, - this.driver, - this.sdk, - this.hasEmbedderSdk) + this.config, this.driver, this.sdk, this.hasEmbedderSdk) : packageMeta = config.topLevelPackageMeta, - session = driver.currentSession, - _packageWarningCounter = - new PackageWarningCounter(packageWarningOptions) { + session = driver.currentSession { + _packageWarningCounter = new PackageWarningCounter(this); // Make sure the default package exists, even if it has no libraries. // This can happen for packages that only contain embedder SDKs. new Package.fromPackageMeta(packageMeta, this); @@ -4815,14 +4811,6 @@ class PackageGraph { return (_allRootDirs.contains(element.library.packageMeta?.resolvedDir)); } - /// Flush out any warnings we might have collected while - /// [PackageWarningOptions.autoFlush] was false. - void flushWarnings() { - _packageWarningCounter.maybeFlush(); - } - - Tuple2 get lineAndColumn => null; - PackageWarningCounter get packageWarningCounter => _packageWarningCounter; final Set> _warnAlreadySeen = @@ -4871,6 +4859,7 @@ class PackageGraph { return; } // Some kinds of warnings it is OK to drop if we're not documenting them. + // TODO(jcollins-g): drop this and use new flag system instead. if (warnable != null && skipWarningIfNotDocumentedFor.contains(kind) && !warnable.isDocumented) { @@ -5610,6 +5599,7 @@ class Category extends Nameable Indexable implements Documentable { /// All libraries in [libraries] must come from [package]. + @override Package package; String _name; @override @@ -5965,6 +5955,9 @@ class Package extends LibraryContainer @override String get name => _name; + @override + Package get package => this; + @override PackageGraph get packageGraph => _packageGraph; @@ -6360,7 +6353,7 @@ class PackageBuilder { } PackageGraph newGraph = new PackageGraph.UninitializedPackageGraph( - config, getWarningOptions(), driver, sdk, hasEmbedderSdkFiles); + config, driver, sdk, hasEmbedderSdkFiles); await getLibraries(newGraph); await newGraph.initializePackageGraph(); return newGraph; @@ -6480,25 +6473,6 @@ class PackageBuilder { return _driver; } - PackageWarningOptions getWarningOptions() { - PackageWarningOptions warningOptions = - new PackageWarningOptions(config.verboseWarnings); - // TODO(jcollins-g): explode this into detailed command line options. - for (PackageWarning kind in PackageWarning.values) { - switch (kind) { - case PackageWarning.toolError: - case PackageWarning.invalidParameter: - case PackageWarning.unresolvedExport: - warningOptions.error(kind); - break; - default: - if (config.showWarnings) warningOptions.warn(kind); - break; - } - } - return warningOptions; - } - /// Return an Iterable with the sdk files we should parse. /// Filter can be String or RegExp (technically, anything valid for /// [String.contains]) diff --git a/lib/src/warnings.dart b/lib/src/warnings.dart index 64aa4a17da..93c5e61790 100644 --- a/lib/src/warnings.dart +++ b/lib/src/warnings.dart @@ -2,34 +2,121 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:io'; + import 'package:analyzer/dart/element/element.dart'; +import 'package:dartdoc/src/dartdoc_options.dart'; import 'package:dartdoc/src/logging.dart'; import 'package:dartdoc/src/model.dart'; +import 'package:dartdoc/src/package_meta.dart'; import 'package:dartdoc/src/tuple.dart'; -class PackageWarningHelpText { +abstract class PackageWarningOptionContext implements DartdocOptionContextBase { + bool get allowNonLocalWarnings => + optionSet['allowNonLocalWarnings'].valueAt(context); + // allowWarningsInPackages, ignoreWarningsInPackages, errors, warnings, and ignore + // are only used indirectly via the synthetic packageWarningOptions option. + PackageWarningOptions get packageWarningOptions => + optionSet['packageWarningOptions'].valueAt(context); + bool get verboseWarnings => optionSet['verboseWarnings'].valueAt(context); +} + +Future> createPackageWarningOptions() async { + return [ + new DartdocOptionArgOnly('allowNonLocalWarnings', false, + negatable: true, + help: 'Show warnings from packages we are not documenting locally.'), + + // Options for globally enabling/disabling all warnings and errors + // for individual packages are command-line only. This will allow + // meta-packages like Flutter to control whether warnings are displayed for + // packages they don't control. + new DartdocOptionArgOnly>('allowWarningsInPackages', null, + help: + 'Package names to display warnings for (ignore all others if set).'), + new DartdocOptionArgOnly>('allowErrorsInPackages', null, + help: 'Package names to display errors for (ignore all others if set)'), + new DartdocOptionArgOnly>('ignoreWarningsInPackages', null, + help: + 'Package names to ignore warnings for. Takes priority over allow-warnings-in-packages'), + new DartdocOptionArgOnly>('ignoreErrorsInPackages', null, + help: + 'Package names to ignore errors for. Takes priority over allow-errors-in-packages'), + // Options for globally enabling/disabling warnings and errors across + // packages. Loaded from dartdoc_options.yaml, but command line arguments + // will override. + new DartdocOptionArgFile>('errors', null, + help: + 'Additional warning names to force as errors. Specify an empty list to force defaults (overriding dartdoc_options.yaml)\nDefaults:\n' + + (packageWarningDefinitions.values + .where((d) => + d.defaultWarningMode == PackageWarningMode.error) + .toList() + ..sort()) + .map((d) => ' ${d.warningName}: ${d.shortHelp}') + .join('\n')), + new DartdocOptionArgFile>('ignore', null, + help: + 'Additional warning names to ignore. Specify an empty list to force defaults (overriding dartdoc_options.yaml).\nDefaults:\n' + + (packageWarningDefinitions.values + .where((d) => + d.defaultWarningMode == PackageWarningMode.ignore) + .toList() + ..sort()) + .map((d) => ' ${d.warningName}: ${d.shortHelp}') + .join('\n')), + new DartdocOptionArgFile>('warnings', null, + help: + 'Additional warning names to show as warnings (instead of error or ignore, if not warning by default).\nDefaults:\n' + + (packageWarningDefinitions.values + .where((d) => + d.defaultWarningMode == PackageWarningMode.warn) + .toList() + ..sort()) + .map((d) => ' ${d.warningName}: ${d.shortHelp}') + .join('\n')), + // Synthetic option uses a factory to build a PackageWarningOptions from all the above flags. + new DartdocOptionSyntheticOnly( + 'packageWarningOptions', PackageWarningOptions.fromOptions), + ]; +} + +class PackageWarningDefinition implements Comparable { final String warningName; final String shortHelp; final List longHelp; - final PackageWarning warning; + final PackageWarning kind; + final PackageWarningMode defaultWarningMode; + + const PackageWarningDefinition(this.kind, this.warningName, this.shortHelp, + {List longHelp, PackageWarningMode defaultWarningMode}) + : this.longHelp = longHelp ?? const [], + this.defaultWarningMode = defaultWarningMode ?? PackageWarningMode.warn; - const PackageWarningHelpText(this.warning, this.warningName, this.shortHelp, - [List longHelp]) - : this.longHelp = longHelp ?? const []; + @override + int compareTo(PackageWarningDefinition other) { + return warningName.compareTo(other.warningName); + } } +/// Same as [packageWarningDefinitions], except keyed by the warning name. +final Map packageWarningsByName = + new Map.fromEntries(packageWarningDefinitions.values + .map((definition) => new MapEntry(definition.warningName, definition))); + /// Provides description text and command line flags for warnings. /// TODO(jcollins-g): Actually use this for command line flags. -final Map packageWarningText = const { - PackageWarning.ambiguousDocReference: const PackageWarningHelpText( +final Map packageWarningDefinitions = + const { + PackageWarning.ambiguousDocReference: const PackageWarningDefinition( PackageWarning.ambiguousDocReference, "ambiguous-doc-reference", "A comment reference could refer to two or more different objects"), - PackageWarning.ambiguousReexport: const PackageWarningHelpText( + PackageWarning.ambiguousReexport: const PackageWarningDefinition( PackageWarning.ambiguousReexport, "ambiguous-reexport", "A symbol is exported from private to public in more than one library and dartdoc can not determine which one is canonical", - const [ + longHelp: const [ "Use {@canonicalFor @@name@@} in the desired library's documentation to resolve", "the ambiguity and/or override dartdoc's decision, or structure your package ", "so the reexport is less ambiguous. The symbol will still be referenced in ", @@ -38,70 +125,75 @@ final Map packageWarningText = const { "The flag --ambiguous-reexport-scorer-min-confidence allows you to set the", "threshold at which this warning will appear." ]), - PackageWarning.ignoredCanonicalFor: const PackageWarningHelpText( + PackageWarning.ignoredCanonicalFor: const PackageWarningDefinition( PackageWarning.ignoredCanonicalFor, "ignored-canonical-for", "A @canonicalFor tag refers to a library which this symbol can not be canonical for"), - PackageWarning.noCanonicalFound: const PackageWarningHelpText( + PackageWarning.noCanonicalFound: const PackageWarningDefinition( PackageWarning.noCanonicalFound, "no-canonical-found", "A symbol is part of the public interface for this package, but no library documented with this package documents it so dartdoc can not link to it"), - PackageWarning.noLibraryLevelDocs: const PackageWarningHelpText( + PackageWarning.noLibraryLevelDocs: const PackageWarningDefinition( PackageWarning.noLibraryLevelDocs, "no-library-level-docs", "There are no library level docs for this library"), - PackageWarning.packageOrderGivesMissingPackageName: const PackageWarningHelpText( - PackageWarning.packageOrderGivesMissingPackageName, - "category-order-gives-missing-package-name", - "The category-order flag on the command line was given the name of a nonexistent package"), - PackageWarning.reexportedPrivateApiAcrossPackages: const PackageWarningHelpText( + PackageWarning.packageOrderGivesMissingPackageName: + const PackageWarningDefinition( + PackageWarning.packageOrderGivesMissingPackageName, + "category-order-gives-missing-package-name", + "The category-order flag on the command line was given the name of a nonexistent package"), + PackageWarning.reexportedPrivateApiAcrossPackages: const PackageWarningDefinition( PackageWarning.reexportedPrivateApiAcrossPackages, "reexported-private-api-across-packages", "One or more libraries reexports private API members from outside its own package"), - PackageWarning.unresolvedDocReference: const PackageWarningHelpText( + PackageWarning.unresolvedDocReference: const PackageWarningDefinition( PackageWarning.unresolvedDocReference, "unresolved-doc-reference", "A comment reference could not be found in parameters, enclosing class, enclosing library, or at the top level of any documented library with the package"), - PackageWarning.brokenLink: const PackageWarningHelpText( + PackageWarning.brokenLink: const PackageWarningDefinition( PackageWarning.brokenLink, - "brokenLink", + "broken-link", "Dartdoc generated a link to a non-existent file"), - PackageWarning.unknownMacro: const PackageWarningHelpText( + PackageWarning.unknownMacro: const PackageWarningDefinition( PackageWarning.unknownMacro, - "unknownMacro", + "unknown-macro", "A comment reference contains an unknown macro"), - PackageWarning.orphanedFile: const PackageWarningHelpText( + PackageWarning.orphanedFile: const PackageWarningDefinition( PackageWarning.orphanedFile, - "orphanedFile", + "orphaned-file", "Dartdoc generated files that are unreachable from the index"), - PackageWarning.unknownFile: const PackageWarningHelpText( + PackageWarning.unknownFile: const PackageWarningDefinition( PackageWarning.unknownFile, - "unknownFile", + "unknown-file", "A leftover file exists in the tree that dartdoc did not write in this pass"), - PackageWarning.missingFromSearchIndex: const PackageWarningHelpText( + PackageWarning.missingFromSearchIndex: const PackageWarningDefinition( PackageWarning.missingFromSearchIndex, - "missingFromSearchIndex", + "missing-from-search-index", "A file generated by dartdoc is not present in the generated index.json"), - PackageWarning.typeAsHtml: const PackageWarningHelpText( + PackageWarning.typeAsHtml: const PackageWarningDefinition( PackageWarning.typeAsHtml, - "typeAsHtml", - "Use of <> in a comment for type parameters is being treated as HTML by markdown"), - PackageWarning.invalidParameter: const PackageWarningHelpText( + "type-as-html", + "Use of <> in a comment for type parameters is being treated as HTML by markdown", + defaultWarningMode: PackageWarningMode.ignore), + PackageWarning.invalidParameter: const PackageWarningDefinition( PackageWarning.invalidParameter, - "invalidParameter", - "A parameter given to a dartdoc directive was invalid."), - PackageWarning.toolError: const PackageWarningHelpText( + "invalid-parameter", + "A parameter given to a dartdoc directive was invalid.", + defaultWarningMode: PackageWarningMode.error), + PackageWarning.toolError: const PackageWarningDefinition( PackageWarning.toolError, - "toolError", - "Unable to execute external tool."), - PackageWarning.deprecated: const PackageWarningHelpText( + "tool-error", + "Unable to execute external tool.", + defaultWarningMode: PackageWarningMode.error), + PackageWarning.deprecated: const PackageWarningDefinition( PackageWarning.deprecated, "deprecated", "A dartdoc directive has a deprecated format."), - PackageWarning.unresolvedExport: const PackageWarningHelpText( + PackageWarning.unresolvedExport: const PackageWarningDefinition( PackageWarning.unresolvedExport, - "unresolvedExport", - "An export refers to a URI that cannot be resolved."), + "unresolved-export", + "An export refers to a URI that cannot be resolved.", + defaultWarningMode: PackageWarningMode.error), }; /// Something that package warnings can be called on. Optionally associated @@ -111,6 +203,7 @@ abstract class Warnable implements Canonicalization { {String message, Iterable referredFrom}); Element get element; Warnable get enclosingElement; + Package get package; } /// Something that can be located for warning purposes. @@ -148,6 +241,13 @@ enum PackageWarning { unresolvedExport, } +/// Used to declare defaults for a particular package warning. +enum PackageWarningMode { + ignore, + warn, + error, +} + /// Warnings it is OK to skip if we can determine the warnable isn't documented. /// In particular, this set should not include warnings around public/private /// or canonicalization problems, because those can break the isDocumented() @@ -156,106 +256,141 @@ final Set skipWarningIfNotDocumentedFor = new Set() ..addAll([PackageWarning.unresolvedDocReference, PackageWarning.typeAsHtml]); class PackageWarningOptions { - // PackageWarnings must be in one of _ignoreWarnings or union(_asWarnings, _asErrors) - final Set ignoreWarnings = new Set(); - // PackageWarnings can be in both asWarnings and asErrors, latter takes precedence - final Set asWarnings = new Set(); - final Set asErrors = new Set(); - // Display verbose warnings. - final bool verboseWarnings; - - bool autoFlush = true; - - PackageWarningOptions(this.verboseWarnings) { - asWarnings.addAll(PackageWarning.values); - ignore(PackageWarning.typeAsHtml); - error(PackageWarning.unresolvedExport); - } + final Map warningModes = {}; - void _assertInvariantsOk() { - assert(asWarnings - .union(asErrors) - .union(ignoreWarnings) - .containsAll(PackageWarning.values.toSet())); - assert(asWarnings.union(asErrors).intersection(ignoreWarnings).isEmpty); + PackageWarningOptions() { + for (PackageWarningDefinition definition + in packageWarningDefinitions.values) { + switch (definition.defaultWarningMode) { + case PackageWarningMode.warn: + { + warn(definition.kind); + } + break; + case PackageWarningMode.error: + { + error(definition.kind); + } + break; + case PackageWarningMode.ignore: + { + ignore(definition.kind); + } + break; + } + } } - void ignore(PackageWarning kind) { - _assertInvariantsOk(); - asWarnings.remove(kind); - asErrors.remove(kind); - ignoreWarnings.add(kind); - _assertInvariantsOk(); - } + /// [packageMeta] parameter is for testing. + static PackageWarningOptions fromOptions( + DartdocSyntheticOption option, Directory dir) { + // First, initialize defaults. + PackageWarningOptions newOptions = PackageWarningOptions(); + PackageMeta packageMeta = new PackageMeta.fromDir(dir); - void warn(PackageWarning kind) { - _assertInvariantsOk(); - ignoreWarnings.remove(kind); - asWarnings.add(kind); - asErrors.remove(kind); - _assertInvariantsOk(); - } + // Interpret errors/warnings/ignore options. In the event of conflict, warning overrides error and + // ignore overrides warning. + for (String warningName in option.parent['errors'].valueAt(dir) ?? []) { + if (packageWarningsByName[warningName] != null) { + newOptions.error(packageWarningsByName[warningName].kind); + } + } + for (String warningName in option.parent['warnings'].valueAt(dir) ?? []) { + if (packageWarningsByName[warningName] != null) { + newOptions.warn(packageWarningsByName[warningName].kind); + } + } + for (String warningName in option.parent['ignore'].valueAt(dir) ?? []) { + if (packageWarningsByName[warningName] != null) { + newOptions.ignore(packageWarningsByName[warningName].kind); + } + } - void error(PackageWarning kind) { - _assertInvariantsOk(); - ignoreWarnings.remove(kind); - asWarnings.add(kind); - asErrors.add(kind); - _assertInvariantsOk(); + // Check whether warnings are allowed at all in this package. + List allowWarningsInPackages = + option.parent['allowWarningsInPackages'].valueAt(dir); + List allowErrorsInPackages = + option.parent['allowErrorsInPackages'].valueAt(dir); + List ignoreWarningsInPackages = + option.parent['ignoreWarningsInPackages'].valueAt(dir); + List ignoreErrorsInPackages = + option.parent['ignoreErrorsInPackages'].valueAt(dir); + if (allowWarningsInPackages != null && + !allowWarningsInPackages.contains(packageMeta.name)) { + PackageWarning.values + .forEach((PackageWarning kind) => newOptions.ignore(kind)); + } + if (allowErrorsInPackages != null && + !allowWarningsInPackages.contains(packageMeta.name)) { + PackageWarning.values + .forEach((PackageWarning kind) => newOptions.ignore(kind)); + } + if (ignoreWarningsInPackages != null && + ignoreWarningsInPackages.contains(packageMeta.name)) { + PackageWarning.values + .forEach((PackageWarning kind) => newOptions.ignore(kind)); + } + if (ignoreErrorsInPackages != null && + ignoreErrorsInPackages.contains(packageMeta.name)) { + PackageWarning.values + .forEach((PackageWarning kind) => newOptions.ignore(kind)); + } + return newOptions; } + + void ignore(PackageWarning kind) => + warningModes[kind] = PackageWarningMode.ignore; + void warn(PackageWarning kind) => + warningModes[kind] = PackageWarningMode.warn; + void error(PackageWarning kind) => + warningModes[kind] = PackageWarningMode.error; + + PackageWarningMode getMode(PackageWarning kind) => warningModes[kind]; } class PackageWarningCounter { final countedWarnings = new Map>>(); - final _warningCounts = new Map(); - final PackageWarningOptions options; - final _items = []; + final _displayedWarningCounts = {}; + final PackageGraph packageGraph; - PackageWarningCounter(this.options); - - /// Flush to stderr, but only if [options.autoFlush] is true. - /// - /// We keep a buffer because under certain conditions (--auto-include-dependencies) - /// warnings here might be duplicated across multiple Package constructions. - void maybeFlush() { - if (options.autoFlush) { - for (var item in _items) { - logWarning(item); - } - _items.clear(); - } - } + PackageWarningCounter(this.packageGraph); /// Actually write out the warning. Assumes it is already counted with add. - void _writeWarning(PackageWarning kind, String name, String fullMessage) { - if (options.ignoreWarnings.contains(kind)) { + void _writeWarning(PackageWarning kind, PackageWarningMode mode, + bool verboseWarnings, String name, String fullMessage) { + if (mode == PackageWarningMode.ignore) { return; } String type; - if (options.asErrors.contains(kind)) { + if (mode == PackageWarningMode.error) { type = "error"; - } else if (options.asWarnings.contains(kind)) { + } else if (mode == PackageWarningMode.warn) { type = "warning"; } if (type != null) { var entry = " $type: $fullMessage"; - if (_warningCounts[kind] == 1 && - options.verboseWarnings && - packageWarningText[kind].longHelp.isNotEmpty) { + _displayedWarningCounts.putIfAbsent(kind, () => 0); + _displayedWarningCounts[kind] += 1; + if (_displayedWarningCounts[kind] == 1 && + verboseWarnings && + packageWarningDefinitions[kind].longHelp.isNotEmpty) { // First time we've seen this warning. Give a little extra info. final String separator = '\n '; final String nameSub = r'@@name@@'; String verboseOut = - '$separator${packageWarningText[kind].longHelp.join(separator)}' + '$separator${packageWarningDefinitions[kind].longHelp.join(separator)}' .replaceAll(nameSub, name); entry = '$entry$verboseOut'; } assert(entry == entry.trimRight()); _items.add(new _JsonWarning(type, kind, fullMessage, entry)); } - maybeFlush(); + for (var item in _items) { + logWarning(item); + } + _items.clear(); } /// Returns true if we've already warned for this. @@ -272,28 +407,27 @@ class PackageWarningCounter { void addWarning(Warnable element, PackageWarning kind, String message, String fullMessage) { assert(!hasWarning(element, kind, message)); + // TODO(jcollins-g): Make addWarning not accept nulls for element. + PackageWarningOptionContext config = + element?.config ?? packageGraph.defaultPackage.config; + PackageWarningMode warningMode = config.packageWarningOptions.getMode(kind); + if (!config.allowNonLocalWarnings && + element != null && + !element.package.isLocal) { + warningMode = PackageWarningMode.ignore; + } + if (warningMode == PackageWarningMode.warn) + warningCount += 1; + else if (warningMode == PackageWarningMode.error) errorCount += 1; Tuple2 warningData = new Tuple2(kind, message); - _warningCounts.putIfAbsent(kind, () => 0); - _warningCounts[kind] += 1; countedWarnings.putIfAbsent(element?.element, () => new Set()); countedWarnings[element?.element].add(warningData); - _writeWarning(kind, element?.fullyQualifiedName, fullMessage); + _writeWarning(kind, warningMode, config.verboseWarnings, + element?.fullyQualifiedName, fullMessage); } - int get errorCount { - return _warningCounts.keys - .map((w) => options.asErrors.contains(w) ? _warningCounts[w] : 0) - .fold(0, (a, b) => a + b); - } - - int get warningCount { - return _warningCounts.keys - .map((w) => - options.asWarnings.contains(w) && !options.asErrors.contains(w) - ? _warningCounts[w] - : 0) - .fold(0, (a, b) => a + b); - } + int errorCount = 0; + int warningCount = 0; @override String toString() { @@ -317,7 +451,7 @@ class _JsonWarning extends Jsonable { @override Map toJson() => { 'type': type, - 'kind': packageWarningText[kind].warningName, + 'kind': packageWarningDefinitions[kind].warningName, 'message': message, 'text': text }; diff --git a/test/warnings_test.dart b/test/warnings_test.dart new file mode 100644 index 0000000000..4e12f234db --- /dev/null +++ b/test/warnings_test.dart @@ -0,0 +1,147 @@ +// 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. + +/// Unit tests for lib/src/warnings.dart. +library dartdoc.warnings_test; + +import 'dart:io'; + +import 'package:dartdoc/src/dartdoc_options.dart'; +import 'package:dartdoc/src/warnings.dart'; +import 'package:path/path.dart' as pathLib; +import 'package:test/test.dart'; + +void main() { + Directory tempDir, testPackageOne, testPackageTwo, testPackageThree; + File pubspecYamlOne, pubspecYamlTwo, pubspecYamlThree, dartdocYamlThree; + DartdocOptionSet optionSet; + + setUpAll(() { + tempDir = Directory.systemTemp.createTempSync('warnings_test'); + testPackageOne = Directory(pathLib.join(tempDir.path, 'test_package_one')) + ..createSync(); + testPackageTwo = Directory(pathLib.join(tempDir.path, 'test_package_two')) + ..createSync(); + testPackageThree = + Directory(pathLib.join(tempDir.path, 'test_package_three')) + ..createSync(); + pubspecYamlOne = + new File(pathLib.join(testPackageOne.path, 'pubspec.yaml')); + pubspecYamlOne.writeAsStringSync('name: test_package_one'); + pubspecYamlTwo = + new File(pathLib.join(testPackageTwo.path, 'pubspec.yaml')); + pubspecYamlTwo.writeAsStringSync('name: test_package_two'); + dartdocYamlThree = + new File(pathLib.join(testPackageThree.path, 'dartdoc_options.yaml')); + dartdocYamlThree.writeAsStringSync(''' +dartdoc: + warnings: + - type-as-html + - unresolved-export + errors: + - unresolved-doc-reference + ignore: + - ambiguous-reexport + '''); + pubspecYamlThree = + new File(pathLib.join(testPackageThree.path, 'pubspec.yaml')); + pubspecYamlThree.writeAsStringSync('name: test_package_three'); + }); + + setUp(() async { + optionSet = await DartdocOptionSet.fromOptionGenerators( + 'dartdoc', [createDartdocOptions]); + }); + + test('Verify that options for enabling/disabling packages work', () { + optionSet.parseArguments([ + '--allow-warnings-in-packages', + 'test_package_two,test_package_three', + '--allow-errors-in-packages', + 'test_package_two,test_package_three', + '--ignore-warnings-in-packages', + 'test_package_three', + '--ignore-errors-in-packages', + 'test_package_three', + ]); + PackageWarningOptions optionsOne = + optionSet['packageWarningOptions'].valueAt(testPackageOne); + PackageWarningOptions optionsTwo = + optionSet['packageWarningOptions'].valueAt(testPackageTwo); + PackageWarningOptions optionsThree = + optionSet['packageWarningOptions'].valueAt(testPackageThree); + + expect(optionsOne.warningModes.values, + everyElement(equals(PackageWarningMode.ignore))); + expect(optionsOne.warningModes.values, + everyElement(equals(PackageWarningMode.ignore))); + expect(optionsTwo.warningModes.values, contains(PackageWarningMode.warn)); + expect(optionsTwo.warningModes.values, contains(PackageWarningMode.error)); + expect(optionsThree.warningModes.values, + everyElement(equals(PackageWarningMode.ignore))); + expect(optionsThree.warningModes.values, + everyElement(equals(PackageWarningMode.ignore))); + }); + + test('Verify that loading warning options from files works', () { + optionSet.parseArguments([]); + PackageWarningOptions optionsThree = + optionSet['packageWarningOptions'].valueAt(testPackageThree); + + expect(optionsThree.warningModes[PackageWarning.typeAsHtml], + equals(PackageWarningMode.warn)); + expect(optionsThree.warningModes[PackageWarning.unresolvedExport], + equals(PackageWarningMode.warn)); + expect(optionsThree.warningModes[PackageWarning.unresolvedDocReference], + equals(PackageWarningMode.error)); + expect(optionsThree.warningModes[PackageWarning.ambiguousReexport], + equals(PackageWarningMode.ignore)); + }); + + test('Verify that args override warning options from files', () { + optionSet.parseArguments([ + '--warnings', + 'ambiguous-reexport', + '--errors', + 'type-as-html', + '--ignore', + 'unresolved-export', + ]); + PackageWarningOptions optionsThree = + optionSet['packageWarningOptions'].valueAt(testPackageThree); + expect(optionsThree.warningModes[PackageWarning.typeAsHtml], + equals(PackageWarningMode.error)); + expect(optionsThree.warningModes[PackageWarning.unresolvedExport], + equals(PackageWarningMode.ignore)); + // unresolved-doc-reference is not mentioned in command line, so it reverts to default + expect(optionsThree.warningModes[PackageWarning.unresolvedDocReference], + equals(PackageWarningMode.warn)); + expect(optionsThree.warningModes[PackageWarning.ambiguousReexport], + equals(PackageWarningMode.warn)); + }); + + test( + 'Verify that null values for warnings, ignore, and errors reset to defaults', + () { + optionSet.parseArguments([ + '--warnings', + '', + '--errors', + '', + '--ignore', + '', + ]); + PackageWarningOptions optionsThree = + optionSet['packageWarningOptions'].valueAt(testPackageThree); + + expect(optionsThree.warningModes[PackageWarning.typeAsHtml], + equals(PackageWarningMode.ignore)); + expect(optionsThree.warningModes[PackageWarning.unresolvedExport], + equals(PackageWarningMode.error)); + expect(optionsThree.warningModes[PackageWarning.unresolvedDocReference], + equals(PackageWarningMode.warn)); + expect(optionsThree.warningModes[PackageWarning.ambiguousReexport], + equals(PackageWarningMode.warn)); + }); +}