Skip to content

Move much PackageWarning logic _out_ of PackageGraph, into the enum. #3251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 15, 2022
Merged

Move much PackageWarning logic _out_ of PackageGraph, into the enum. #3251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/src/model/locatable.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,8 @@ abstract class Locatable {
}

final RegExp locationSplitter = RegExp(r'(package:|[\\/;.])');

extension NullableLocatable on Locatable? {
String get safeWarnableName =>
this?.fullyQualifiedName.replaceFirst(':', '-') ?? '<unknown>';
}
159 changes: 32 additions & 127 deletions lib/src/model/package_graph.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {

void warnOnElement(Warnable? warnable, PackageWarning kind,
{String? message,
Iterable<Locatable>? referredFrom,
Iterable<String>? extendedDebug}) {
Iterable<Locatable> referredFrom = const [],
Iterable<String> extendedDebug = const []}) {
var newEntry = Tuple3(warnable?.element, kind, message);
if (_warnAlreadySeen.contains(newEntry)) {
return;
Expand All @@ -310,16 +310,14 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {

void _warnOnElement(Warnable? warnable, PackageWarning kind,
{required String message,
Iterable<Locatable>? referredFrom,
Iterable<String>? extendedDebug}) {
if (warnable is ModelElement) {
Iterable<Locatable> referredFrom = const [],
Iterable<String> extendedDebug = const []}) {
if (warnable is ModelElement && kind == PackageWarning.ambiguousReexport) {
// This sort of warning is only applicable to top level elements.
if (kind == PackageWarning.ambiguousReexport) {
var enclosingElement = warnable.enclosingElement;
while (enclosingElement != null && enclosingElement is! Library) {
warnable = enclosingElement;
enclosingElement = warnable.enclosingElement;
}
var enclosingElement = warnable.enclosingElement;
while (enclosingElement != null && enclosingElement is! Library) {
warnable = enclosingElement;
enclosingElement = warnable.enclosingElement;
}
}

Expand All @@ -329,165 +327,72 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder {
if (packageWarningCounter.hasWarning(warnable, kind, message)) {
return;
}
// Some kinds of warnings it is OK to drop if we're not documenting them.
// Some kinds of warnings are 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) {
return;
}
// Elements that are part of the Dart SDK can have colons in their FQNs.
// This confuses IntelliJ and makes it so it can't link to the location
// of the error in the console window, so separate out the library from
// the path.
// TODO(jcollins-g): What about messages that may include colons? Substituting
// them out doesn't work as well there since it might confuse
// the user, yet we still want IntelliJ to link properly.
final warnableName = _safeWarnableName(warnable);

var warnablePrefix = 'from';
var referredFromPrefix = 'referred to by';
// This confuses IntelliJ and makes it so it can't link to the location of
// the error in the console window, so separate out the library from the
// path.
// TODO(jcollins-g): What about messages that may include colons?
// Substituting them out doesn't work as well there since it might confuse
// the user, yet we still want IntelliJ to link properly.
final warnableName = warnable.safeWarnableName;

String warningMessage;
switch (kind) {
case PackageWarning.noCanonicalFound:
// Fix these warnings by adding libraries with --include, or by using
// --auto-include-dependencies.
// TODO(jcollins-g): pipeline references through linkedName for error
// messages and warn for non-public canonicalization
// errors.
warningMessage =
'no canonical library found for $warnableName, not linking';
break;
case PackageWarning.ambiguousReexport:
// Fix these warnings by adding the original library exporting the
// symbol with --include, by using --auto-include-dependencies,
// or by using --exclude to hide one of the libraries involved
warningMessage =
'ambiguous reexport of $warnableName, canonicalization candidates: $message';
warningMessage = kind.messageFor([warnableName, message]);
break;
case PackageWarning.noCanonicalFound:
case PackageWarning.noDefiningLibraryFound:
warningMessage =
'could not find the defining library for $warnableName; the '
'library may be imported or exported with a non-standard URI';
warningMessage = kind.messageFor([warnableName]);
break;
case PackageWarning.noLibraryLevelDocs:
warningMessage =
'${warnable!.fullyQualifiedName} has no library level documentation comments';
break;
case PackageWarning.noDocumentableLibrariesInPackage:
warningMessage =
'${warnable!.fullyQualifiedName} has no documentable libraries';
warningMessage = kind.messageFor([warnable!.fullyQualifiedName]);
break;
case PackageWarning.ambiguousDocReference:
warningMessage = 'ambiguous doc reference $message';
break;
case PackageWarning.ignoredCanonicalFor:
warningMessage =
"library says it is {@canonicalFor $message} but $message can't be canonical there";
break;
case PackageWarning.packageOrderGivesMissingPackageName:
warningMessage =
"--package-order gives invalid package name: '$message'";
break;
case PackageWarning.reexportedPrivateApiAcrossPackages:
warningMessage =
'private API of $message is reexported by libraries in other packages: ';
break;
case PackageWarning.notImplemented:
warningMessage = message;
break;
case PackageWarning.unresolvedDocReference:
warningMessage = 'unresolved doc reference [$message]';
referredFromPrefix = 'in documentation inherited from';
break;
case PackageWarning.unknownDirective:
warningMessage = 'undefined directive: $message';
break;
case PackageWarning.unknownMacro:
warningMessage = 'undefined macro [$message]';
break;
case PackageWarning.unknownHtmlFragment:
warningMessage = 'undefined HTML fragment identifier [$message]';
break;
case PackageWarning.brokenLink:
warningMessage = 'dartdoc generated a broken link to: $message';
warnablePrefix = 'to element';
referredFromPrefix = 'linked to from';
break;
case PackageWarning.duplicateFile:
case PackageWarning.orphanedFile:
warningMessage = 'dartdoc generated a file orphan: $message';
break;
case PackageWarning.unknownFile:
warningMessage =
'dartdoc detected an unknown file in the doc tree: $message';
break;
case PackageWarning.missingFromSearchIndex:
warningMessage =
'dartdoc generated a file not in the search index: $message';
break;
case PackageWarning.typeAsHtml:
// The message for this warning can contain many punctuation and other symbols,
// so bracket with a triple quote for defense.
warningMessage = 'generic type handled as HTML: """$message"""';
break;
case PackageWarning.invalidParameter:
warningMessage = 'invalid parameter to dartdoc directive: $message';
break;
case PackageWarning.toolError:
warningMessage = 'tool execution failed: $message';
break;
case PackageWarning.deprecated:
warningMessage = 'deprecated dartdoc usage: $message';
break;
case PackageWarning.unresolvedExport:
warningMessage = 'unresolved export uri: $message';
break;
case PackageWarning.duplicateFile:
warningMessage = 'failed to write file at: $message';
warnablePrefix = 'for symbol';
referredFromPrefix = 'conflicting with file already generated by';
break;
case PackageWarning.missingConstantConstructor:
warningMessage = 'constant constructor missing: $message';
break;
case PackageWarning.missingExampleFile:
warningMessage = 'example file not found: $message';
break;
case PackageWarning.missingCodeBlockLanguage:
warningMessage = 'missing code block language: $message';
break;
warningMessage = kind.messageFor([message]);
}

var messageParts = <String>[warningMessage];
if (warnable != null) {
messageParts.add('$warnablePrefix $warnableName: ${warnable.location}');
}
if (referredFrom != null) {
for (var referral in referredFrom) {
if (referral != warnable) {
var referredFromStrings = _safeWarnableName(referral);
messageParts.add(
'$referredFromPrefix $referredFromStrings: ${referral.location}');
}
}
}
if (config.verboseWarnings && extendedDebug != null) {
messageParts.addAll(extendedDebug.map((s) => ' $s'));
}
var fullMessage = messageParts.join('\n ');
var fullMessage = [
warningMessage,
if (warnable != null) kind.messageForWarnable(warnable),
for (var referral in referredFrom)
if (referral != warnable) kind.messageForReferral(referral),
if (config.verboseWarnings) ...extendedDebug.map((s) => ' $s')
].join('\n ');

packageWarningCounter.addWarning(warnable, kind, message, fullMessage);
}

String _safeWarnableName(Locatable? locatable) {
if (locatable == null) {
return '<unknown>';
}

return locatable.fullyQualifiedName.replaceFirst(':', '-');
}

List<Package> get packages => packageMap.values.toList(growable: false);
Iterable<Package> get packages => packageMap.values;

late final List<Package> publicPackages = () {
assert(allLibrariesAdded);
Expand Down
103 changes: 76 additions & 27 deletions lib/src/warnings.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,33 +313,82 @@ mixin Warnable implements Canonicalization, CommentReferable {

// The kinds of warnings that can be displayed when documenting a package.
enum PackageWarning {
ambiguousDocReference,
ambiguousReexport,
ignoredCanonicalFor,
noCanonicalFound,
noDefiningLibraryFound,
notImplemented,
noDocumentableLibrariesInPackage,
noLibraryLevelDocs,
packageOrderGivesMissingPackageName,
reexportedPrivateApiAcrossPackages,
unresolvedDocReference,
unknownDirective,
unknownMacro,
unknownHtmlFragment,
brokenLink,
duplicateFile,
orphanedFile,
unknownFile,
missingFromSearchIndex,
typeAsHtml,
invalidParameter,
toolError,
deprecated,
unresolvedExport,
missingConstantConstructor,
missingExampleFile,
missingCodeBlockLanguage,
ambiguousDocReference('ambiguous doc reference {0}'),

// Fix these warnings by adding the original library exporting the symbol with
// `--include`, by using `--auto-include-dependencies`, or by using
// `--exclude` to hide one of the libraries involved.
ambiguousReexport(
'ambiguous reexport of {0}, canonicalization candidates: {1}'),
ignoredCanonicalFor(
"library says it is {@canonicalFor {0}} but {0} can't be canonical "
'there'),

// Fix these warnings by adding libraries with `--include`, or by using
// `--auto-include-dependencies`.
// TODO(jcollins-g): pipeline references through `linkedName` for error
// messages and warn for non-public canonicalization errors.
noCanonicalFound('no canonical library found for {0}, not linking'),
noDefiningLibraryFound('could not find the defining library for {0}; the '
'library may be imported or exported with a non-standard URI'),
notImplemented('{0}'),
noDocumentableLibrariesInPackage('{0} has no documentable libraries'),
noLibraryLevelDocs('{0} has no library level documentation comments'),
packageOrderGivesMissingPackageName(
"--package-order gives invalid package name: '{0}'"),
reexportedPrivateApiAcrossPackages(
'private API of {0} is reexported by libraries in other packages: '),
unresolvedDocReference('unresolved doc reference [{0}]',
referredFromPrefix: 'in documentation inherited from'),
unknownDirective('undefined directive: {0}'),
unknownMacro('undefined macro [{0}]'),
unknownHtmlFragment('undefined HTML fragment identifier [{0}]'),
brokenLink('dartdoc generated a broken link to: {0}',
warnablePrefix: 'to element', referredFromPrefix: 'linked to from'),
duplicateFile('failed to write file at: {0}',
warnablePrefix: 'for symbol',
referredFromPrefix: 'conflicting with file already generated by'),
orphanedFile('dartdoc generated a file orphan: {0}'),
unknownFile('dartdoc detected an unknown file in the doc tree: {0}'),
missingFromSearchIndex(
'dartdoc generated a file not in the search index: {0}'),

// The message for this warning can contain many punctuation and other
// symbols, so bracket with a triple quote for defense.
typeAsHtml('generic type handled as HTML: """{0}"""'),
invalidParameter('invalid parameter to dartdoc directive: {0}'),
toolError('tool execution failed: {0}'),
deprecated('deprecated dartdoc usage: {0}'),
unresolvedExport('unresolved export uri: {0}'),
missingConstantConstructor('constant constructor missing: {0}'),
missingExampleFile('example file not found: {0}'),
missingCodeBlockLanguage('missing code block language: {0}');

final String template;

final String warnablePrefix;

final String referredFromPrefix;

const PackageWarning(
this.template, {
this.warnablePrefix = 'from',
this.referredFromPrefix = 'referred to by',
});

String messageFor(List<String> messageParts) {
var message = template;
for (var i = 0; i < messageParts.length; i++) {
message = message.replaceAll('{$i}', messageParts[i]);
}
return message;
}

String messageForWarnable(Warnable warnable) =>
'$warnablePrefix ${warnable.safeWarnableName}: ${warnable.location}';

String messageForReferral(Locatable referral) =>
'$referredFromPrefix ${referral.safeWarnableName}: ${referral.location}';
}

/// Used to declare defaults for a particular package warning.
Expand Down