diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart index 6d8143667b..f4e8da6326 100644 --- a/lib/src/dartdoc_options.dart +++ b/lib/src/dartdoc_options.dart @@ -450,7 +450,7 @@ abstract class DartdocOption { /// To avoid accessing early, call [add] on the option's parent before /// looking up unless this is a [DartdocRootOption]. - late final DartdocOption parent; + late final DartdocOption parent; /// The [DartdocOptionRoot] containing this object. DartdocOptionRoot get root { @@ -516,10 +516,13 @@ abstract class DartdocOption { void addAll(Iterable options) => options.forEach(add); /// Get the immediate child of this node named [name]. - DartdocOption operator [](String name) { + DartdocOption operator [](String name) { return _children[name]!; } + /// Get the immediate child of this node named [name] as a [DartdocOption]. + DartdocOption getAs(String name) => _children[name] as DartdocOption; + /// Apply the function [visit] to [this] and all children. void traverse(void Function(DartdocOption option) visit) { visit(this); diff --git a/lib/src/model/accessor.dart b/lib/src/model/accessor.dart index 0ac4bb091e..d37f70aa2c 100644 --- a/lib/src/model/accessor.dart +++ b/lib/src/model/accessor.dart @@ -100,10 +100,12 @@ class Accessor extends ModelElement implements EnclosedElement { } @override - void warn(PackageWarning kind, - {String message, - Iterable referredFrom, - Iterable extendedDebug}) { + void warn( + PackageWarning kind, { + String message, + Iterable referredFrom = const [], + Iterable extendedDebug = const [], + }) { enclosingCombo.warn(kind, message: message, referredFrom: referredFrom, diff --git a/lib/src/warnings.dart b/lib/src/warnings.dart index 46669104a7..ddf05aa2fd 100644 --- a/lib/src/warnings.dart +++ b/lib/src/warnings.dart @@ -2,8 +2,6 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -// @dart=2.9 - import 'dart:collection'; import 'package:analyzer/dart/element/element.dart'; @@ -14,19 +12,22 @@ import 'package:dartdoc/src/model/comment_referable.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:dartdoc/src/package_meta.dart'; +const _namePlaceholder = '@@name@@'; + 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. + // 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( +Future>> createPackageWarningOptions( PackageMetaProvider packageMetaProvider, ) async { var resourceProvider = packageMetaProvider.resourceProvider; @@ -39,21 +40,21 @@ Future>> createPackageWarningOptions( // 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. - DartdocOptionArgOnly>( + DartdocOptionArgOnly?>( 'allowWarningsInPackages', null, resourceProvider, splitCommas: true, help: - 'Package names to display warnings for (ignore all others if set).'), - DartdocOptionArgOnly>( + 'Package names to display warnings for (ignore all others if set)'), + DartdocOptionArgOnly?>( 'allowErrorsInPackages', null, resourceProvider, splitCommas: true, help: 'Package names to display errors for (ignore all others if set)'), - DartdocOptionArgOnly>( + DartdocOptionArgOnly?>( 'ignoreWarningsInPackages', null, resourceProvider, splitCommas: true, help: 'Package names to ignore warnings for. Takes priority over ' 'allow-warnings-in-packages'), - DartdocOptionArgOnly>( + DartdocOptionArgOnly?>( 'ignoreErrorsInPackages', null, resourceProvider, splitCommas: true, help: 'Package names to ignore errors for. Takes priority over ' @@ -61,40 +62,24 @@ Future>> createPackageWarningOptions( // Options for globally enabling/disabling warnings and errors across // packages. Loaded from dartdoc_options.yaml, but command line arguments // will override. - DartdocOptionArgFile>('errors', null, resourceProvider, + DartdocOptionArgFile?>('errors', null, resourceProvider, splitCommas: true, 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')), - DartdocOptionArgFile>('ignore', null, resourceProvider, + 'list to force defaults (overriding ' + 'dartdoc_options.yaml)\nDefaults:\n' + + _warningsListHelpText(PackageWarningMode.error)), + DartdocOptionArgFile?>('ignore', null, resourceProvider, splitCommas: true, 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')), - DartdocOptionArgFile>('warnings', null, resourceProvider, + 'force defaults (overriding ' + 'dartdoc_options.yaml).\nDefaults:\n' + + _warningsListHelpText(PackageWarningMode.ignore)), + DartdocOptionArgFile?>('warnings', null, resourceProvider, splitCommas: true, 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')), + _warningsListHelpText(PackageWarningMode.warn)), // Synthetic option uses a factory to build a PackageWarningOptions from all the above flags. DartdocOptionSyntheticOnly( 'packageWarningOptions', @@ -105,6 +90,15 @@ Future>> createPackageWarningOptions( ]; } +String _warningsListHelpText(PackageWarningMode mode) { + return (packageWarningDefinitions.values + .where((d) => d.defaultWarningMode == mode) + .toList() + ..sort()) + .map((d) => ' ${d.warningName}: ${d.shortHelp}') + .join('\n'); +} + class PackageWarningDefinition implements Comparable { final String warningName; final String shortHelp; @@ -112,10 +106,13 @@ class PackageWarningDefinition implements Comparable { final PackageWarning kind; final PackageWarningMode defaultWarningMode; - const PackageWarningDefinition(this.kind, this.warningName, this.shortHelp, - {List longHelp, PackageWarningMode defaultWarningMode}) - : longHelp = longHelp ?? const [], - defaultWarningMode = defaultWarningMode ?? PackageWarningMode.warn; + const PackageWarningDefinition( + this.kind, + this.warningName, + this.shortHelp, { + this.longHelp = const [], + this.defaultWarningMode = PackageWarningMode.warn, + }); @override int compareTo(PackageWarningDefinition other) { @@ -140,14 +137,16 @@ const Map packageWarningDefinitions = PackageWarning.ambiguousReexport: 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', + 'A symbol is exported from private to public in more than one library ' + 'and dartdoc can not determine which one is canonical', longHelp: [ - "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 ', - 'all candidates -- this only controls the location where it will be written ', - 'and which library will be displayed in navigation for the relevant pages.', - 'The flag --ambiguous-reexport-scorer-min-confidence allows you to set the', + "Use {@canonicalFor $_namePlaceholder} 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 all candidates --', + 'this only controls the location where it will be written and which', + 'library will be displayed in navigation for the relevant pages. The', + 'flag --ambiguous-reexport-scorer-min-confidence allows you to set the', 'threshold at which this warning will appear.' ]), PackageWarning.ignoredCanonicalFor: PackageWarningDefinition( @@ -176,10 +175,10 @@ const Map packageWarningDefinitions = 'no-documentable-libraries', 'The package is to be documented but has no Dart libraries to document', longHelp: [ - 'Dartdoc could not find any public libraries to document in @@name@@, ', - 'but documentation was requested. This might be expected for an ', - 'asset only package, in which case, disable this warning in your ', - 'dartdoc_options.yaml file.', + 'Dartdoc could not find any public libraries to document in', + '$_namePlaceholder, but documentation was requested. This might be', + 'expected for an asset only package, in which case, disable this', + 'warning in your dartdoc_options.yaml file.', ], ), PackageWarning.noLibraryLevelDocs: PackageWarningDefinition( @@ -243,14 +242,15 @@ const Map packageWarningDefinitions = PackageWarning.duplicateFile: PackageWarningDefinition( PackageWarning.duplicateFile, 'duplicate-file', - 'Dartdoc is trying to write to a duplicate filename based on the names of Dart symbols.', + 'Dartdoc is trying to write to a duplicate filename based on the names ' + 'of Dart symbols.', longHelp: [ 'Dartdoc generates a path and filename to write to for each symbol.', - '@@name@@ conflicts with another symbol in the generated path, and', - 'therefore can not be written out. Changing the name, library name, or', - 'class name (if appropriate) of one of the conflicting items can resolve', - "the conflict. Alternatively, use the @nodoc tag in one symbol's", - 'documentation comments to hide it.' + '$_namePlaceholder conflicts with another symbol in the generated', + 'path, and therefore can not be written out. Changing the name,', + 'library name, or class name (if appropriate) of one of the', + 'conflicting items can resolve the conflict. Alternatively, use the', + "@nodoc tag in one symbol's documentation comments to hide it." ], defaultWarningMode: PackageWarningMode.error), PackageWarning.missingConstantConstructor: PackageWarningDefinition( @@ -260,8 +260,8 @@ const Map packageWarningDefinitions = longHelp: [ 'To resolve a constant into its literal value, Dartdoc relies on the', "analyzer to resolve the constructor. The analyzer didn't provide", - 'the constructor for @@name@@, which is usually due to an error in the', - 'code. Use the analyzer to find missing imports.', + 'the constructor for $_namePlaceholder, which is usually due to an', + 'error in the code. Use the analyzer to find missing imports.', ], // Defaults to ignore as this doesn't impact the docs severely but is // useful for debugging package structure. @@ -289,10 +289,12 @@ mixin Warnable implements Canonicalization, CommentReferable { Package get package; - void warn(PackageWarning kind, - {String message, - Iterable referredFrom, - Iterable extendedDebug}) { + void warn( + PackageWarning kind, { + String? message, + Iterable referredFrom = const [], + Iterable extendedDebug = const [], + }) { packageGraph.warnOnElement(this, kind, message: message, referredFrom: referredFrom, @@ -354,19 +356,13 @@ class PackageWarningOptions { for (var definition in packageWarningDefinitions.values) { switch (definition.defaultWarningMode) { case PackageWarningMode.warn: - { - warn(definition.kind); - } + warn(definition.kind); break; case PackageWarningMode.error: - { - error(definition.kind); - } + error(definition.kind); break; case PackageWarningMode.ignore: - { - ignore(definition.kind); - } + ignore(definition.kind); break; } } @@ -379,42 +375,55 @@ class PackageWarningOptions { ) { // First, initialize defaults. var newOptions = PackageWarningOptions(); - var packageMeta = packageMetaProvider.fromDir(dir); - - // 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); + var packageMeta = packageMetaProvider.fromDir(dir)!; + + // Interpret errors/warnings/ignore options. In the event of conflict, + // warning overrides error and ignore overrides warning. + var errorsForDir = + option.parent.getAs?>('errors').valueAt(dir) ?? []; + for (var warningName in errorsForDir) { + var packageWarnings = packageWarningsByName[warningName]; + if (packageWarnings != null) { + newOptions.error(packageWarnings.kind); } } - for (String warningName in option.parent['warnings'].valueAt(dir) ?? []) { - if (packageWarningsByName[warningName] != null) { - newOptions.warn(packageWarningsByName[warningName].kind); + var warningsForDir = + option.parent.getAs?>('warnings').valueAt(dir) ?? []; + for (var warningName in warningsForDir) { + var packageWarnings = packageWarningsByName[warningName]; + if (packageWarnings != null) { + newOptions.warn(packageWarnings.kind); } } - for (String warningName in option.parent['ignore'].valueAt(dir) ?? []) { - if (packageWarningsByName[warningName] != null) { - newOptions.ignore(packageWarningsByName[warningName].kind); + var ignoredForDir = + option.parent.getAs?>('ignore').valueAt(dir) ?? []; + for (var warningName in ignoredForDir) { + var packageWarnings = packageWarningsByName[warningName]; + if (packageWarnings != null) { + newOptions.ignore(packageWarnings.kind); } } // 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); + var allowWarningsInPackages = option.parent + .getAs?>('allowWarningsInPackages') + .valueAt(dir); + var allowErrorsInPackages = option.parent + .getAs?>('allowErrorsInPackages') + .valueAt(dir); + var ignoreWarningsInPackages = option.parent + .getAs?>('ignoreWarningsInPackages') + .valueAt(dir); + var ignoreErrorsInPackages = option.parent + .getAs?>('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)) { + !allowErrorsInPackages.contains(packageMeta.name)) { PackageWarning.values .forEach((PackageWarning kind) => newOptions.ignore(kind)); } @@ -440,7 +449,7 @@ class PackageWarningOptions { void error(PackageWarning kind) => warningModes[kind] = PackageWarningMode.error; - PackageWarningMode getMode(PackageWarning kind) => warningModes[kind]; + PackageWarningMode? getMode(PackageWarning kind) => warningModes[kind]; } class PackageWarningCounter { @@ -466,13 +475,15 @@ class PackageWarningCounter { PackageWarningCounter(this.packageGraph); - /// Actually write out the warning. Assumes it is already counted with add. - void _writeWarning(PackageWarning kind, PackageWarningMode mode, + /// Actually write out the warning. + /// + /// Assumes it is already counted with [addWarning]. + void _writeWarning(PackageWarning kind, PackageWarningMode? mode, bool verboseWarnings, String name, String fullMessage) { if (mode == PackageWarningMode.ignore) { return; } - String type; + String? type; if (mode == PackageWarningMode.error) { type = 'error'; } else if (mode == PackageWarningMode.warn) { @@ -480,17 +491,16 @@ class PackageWarningCounter { } if (type != null) { var entry = ' $type: $fullMessage'; - _displayedWarningCounts.putIfAbsent(kind, () => 0); - _displayedWarningCounts[kind] += 1; - if (_displayedWarningCounts[kind] == 1 && + var displayedWarningCount = _displayedWarningCounts.increment(kind); + var packageWarningDefinition = packageWarningDefinitions[kind]!; + if (displayedWarningCount == 1 && verboseWarnings && - packageWarningDefinitions[kind].longHelp.isNotEmpty) { + packageWarningDefinition.longHelp.isNotEmpty) { // First time we've seen this warning. Give a little extra info. final separator = '\n '; - final nameSub = r'@@name@@'; var verboseOut = - '$separator${packageWarningDefinitions[kind].longHelp.join(separator)}' - .replaceAll(nameSub, name); + '$separator${packageWarningDefinition.longHelp.join(separator)}' + .replaceAll(_namePlaceholder, name); entry = '$entry$verboseOut'; } assert(entry == entry.trimRight()); @@ -507,8 +517,11 @@ class PackageWarningCounter { /// Returns `true` if we've already warned for this /// combination of [element], [kind], and [message]. - bool hasWarning(Warnable element, PackageWarning kind, String message) { - final warning = _countedWarnings[element?.element]; + bool hasWarning(Warnable? element, PackageWarning kind, String message) { + if (element == null) { + return false; + } + final warning = _countedWarnings[element.element]; if (warning != null) { final messages = warning[kind]; return messages != null && messages.contains(message); @@ -518,27 +531,32 @@ class PackageWarningCounter { /// Adds the warning to the counter, and writes out the fullMessage string /// if configured to do so. - void addWarning(Warnable element, PackageWarning kind, String message, + 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; - var warningMode = config.packageWarningOptions.getMode(kind); - if (!config.allowNonLocalWarnings && !(element?.package?.isLocal ?? true)) { + PackageWarningMode? warningMode; + var isLocal = element?.package.isLocal ?? true; + if (!config.allowNonLocalWarnings && !isLocal) { warningMode = PackageWarningMode.ignore; + } else { + warningMode = config.packageWarningOptions.getMode(kind); } if (warningMode == PackageWarningMode.warn) { _warningCount += 1; } else if (warningMode == PackageWarningMode.error) { _errorCount += 1; } - _countedWarnings - .putIfAbsent(element?.element, () => {}) - .putIfAbsent(kind, () => {}) - .add(message); - _writeWarning(kind, warningMode, config.verboseWarnings, - element?.fullyQualifiedName, fullMessage); + if (element != null) { + _countedWarnings + .putIfAbsent(element.element, () => {}) + .putIfAbsent(kind, () => {}) + .add(message); + _writeWarning(kind, warningMode, config.verboseWarnings, + element.fullyQualifiedName, fullMessage); + } } @override @@ -563,8 +581,20 @@ class _JsonWarning extends Jsonable { @override Map toJson() => { 'type': type, - 'kind': packageWarningDefinitions[kind].warningName, + 'kind': packageWarningDefinitions[kind]!.warningName, 'message': message, 'text': text }; } + +extension on Map { + int increment(PackageWarning kind) { + if (this[kind] == null) { + this[kind] = 1; + return 1; + } else { + this[kind] = this[kind]! + 1; + return this[kind]!; + } + } +}