Skip to content

Warnings enhancements from #1524 #1539

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ main(List<String> arguments) async {
assert(message.isNotEmpty);

if (record.level < logging.Level.WARNING) {
if (message.endsWith('...')) {
if (showProgress && message.endsWith('...')) {
// Assume there may be more progress to print, so omit the trailing
// newline
writingProgress = true;
Expand Down
21 changes: 16 additions & 5 deletions lib/src/markdown_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ final RegExp isConstructor = new RegExp(r'^new[\s]+', multiLine: true);
// Covers anything with leading digits/symbols, empty string, weird punctuation, spaces.
final RegExp notARealDocReference = new RegExp(r'''(^[^\w]|^[\d]|[,"'/]|^$)''');

final RegExp operatorPrefix = new RegExp(r'^operator[ ]*');

final HtmlEscape htmlEscape = const HtmlEscape(HtmlEscapeMode.ELEMENT);

final List<md.InlineSyntax> _markdown_syntaxes = [
Expand Down Expand Up @@ -397,8 +399,8 @@ Element _findRefElementInLibrary(String codeRef, ModelElement element,
final Set<ModelElement> results = new Set();

// This might be an operator. Strip the operator prefix and try again.
if (results.isEmpty && codeRef.startsWith('operator')) {
String newCodeRef = codeRef.replaceFirst('operator', '');
if (results.isEmpty && codeRef.startsWith(operatorPrefix)) {
String newCodeRef = codeRef.replaceFirst(operatorPrefix, '');
return _findRefElementInLibrary(
newCodeRef, element, commentRefs, preferredClass);
}
Expand Down Expand Up @@ -594,9 +596,13 @@ Element _findRefElementInLibrary(String codeRef, ModelElement element,
} else if (results.length == 1) {
result = results.first.element;
} else {
element.warn(PackageWarning.ambiguousDocReference,
message:
"[$codeRef] => ${results.map((r) => "'${r.fullyQualifiedName}'").join(", ")}");
// Squelch ambiguous doc reference warnings for parameters, because we
// don't link those anyway.
if (!results.every((r) => r is Parameter)) {
element.warn(PackageWarning.ambiguousDocReference,
message:
"[$codeRef] => ${results.map((r) => "'${r.fullyQualifiedName}'").join(", ")}");
}
result = results.first.element;
}
return result;
Expand Down Expand Up @@ -635,6 +641,11 @@ void _getResultsForClass(Class tryClass, String codeRefChomped,
for (final modelElement in c.allModelElements) {
if (!_ConsiderIfConstructor(codeRef, modelElement)) continue;
String namePart = modelElement.fullyQualifiedName.split('.').last;
if (modelElement is Accessor) {
// TODO(jcollins-g): Individual classes should be responsible for
// this name comparison munging.
namePart = namePart.split('=').first;
}
// TODO(jcollins-g): fix operators so we can use 'name' here or similar.
if (codeRefChomped == namePart) {
results.add(package.findCanonicalModelElementFor(
Expand Down
35 changes: 24 additions & 11 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ int byFeatureOrdering(String a, String b) {
return compareAsciiLowerCaseNatural(a, b);
}

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

/// Mixin for subclasses of ModelElement representing Elements that can be
/// inherited from one class to another.
Expand Down Expand Up @@ -2152,6 +2152,7 @@ ModelElement resolveMultiplyInheritedElement(
/// ModelElement will reference itself as part of the "wrong" [Library]
/// from the public interface perspective.
abstract class ModelElement extends Nameable
with Warnable
implements Comparable, Documentable {
final Element _element;
final Library _library;
Expand Down Expand Up @@ -2325,14 +2326,6 @@ abstract class ModelElement extends Nameable
return library.package.libraryElementReexportedBy[this.element.library];
}

Set<String> get locationPieces {
return new Set()
..addAll(element.location
.toString()
.split(_locationSplitter)
.where((s) => s.isNotEmpty));
}

// Use components of this element's location to return a score for library
// location.
ScoredCandidate scoreElementWithLibrary(Library lib) {
Expand Down Expand Up @@ -3318,7 +3311,7 @@ abstract class Nameable {
String get name;

Set<String> get namePieces => new Set()
..addAll(name.split(_locationSplitter).where((s) => s.isNotEmpty));
..addAll(name.split(locationSplitter).where((s) => s.isNotEmpty));
}

class Operator extends Method {
Expand Down Expand Up @@ -3379,7 +3372,7 @@ class Operator extends Method {
String get typeName => 'operator';
}

class Package extends Nameable implements Documentable {
class Package extends Nameable with Documentable, Warnable {
// Library objects serving as entry points for documentation.
final List<Library> _libraries = [];

Expand Down Expand Up @@ -3483,10 +3476,30 @@ class Package extends Nameable implements Documentable {
extendedDebug: extendedDebug);
}

final Set<Tuple3<Element, PackageWarning, String>> _warnAlreadySeen =
new Set();
void warnOnElement(Warnable warnable, PackageWarning kind,
{String message,
Iterable<Locatable> referredFrom,
Iterable<String> extendedDebug}) {
var newEntry = new Tuple3(warnable?.element, kind, message);
if (_warnAlreadySeen.contains(newEntry)) {
return;
}
// Warnings can cause other warnings. Queue them up via the stack but
// don't allow warnings we're already working on to get in there.
_warnAlreadySeen.add(newEntry);
_warnOnElement(warnable, kind,
message: message,
referredFrom: referredFrom,
extendedDebug: extendedDebug);
_warnAlreadySeen.remove(newEntry);
}

void _warnOnElement(Warnable warnable, PackageWarning kind,
{String message,
Iterable<Locatable> referredFrom,
Iterable<String> extendedDebug}) {
if (warnable != null) {
// This sort of warning is only applicable to top level elements.
if (kind == PackageWarning.ambiguousReexport) {
Expand Down
13 changes: 11 additions & 2 deletions lib/src/warnings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.

import 'package:analyzer/dart/element/element.dart';
import 'package:dartdoc/src/model.dart';
import 'package:tuple/tuple.dart';

import 'config.dart';
Expand Down Expand Up @@ -82,10 +83,18 @@ final Map<PackageWarning, PackageWarningHelpText> packageWarningText = const {
};

/// Something that package warnings can be called on.
/// TODO(jcollins-g): Complete object model refactoring for #1524.
abstract class Warnable implements Locatable {
void warn(PackageWarning warning,
{String message, Iterable<Locatable> referredFrom});
Warnable get enclosingElement;

Set<String> get locationPieces {
return new Set.from(element.location
.toString()
.split(locationSplitter)
.where((s) => s.isNotEmpty));
}
}

/// Something that can be located for warning purposes.
Expand Down Expand Up @@ -240,7 +249,7 @@ class PackageWarningCounter {
int get errorCount {
return _warningCounts.keys
.map((w) => options.asErrors.contains(w) ? _warningCounts[w] : 0)
.reduce((a, b) => a + b);
.fold(0, (a, b) => a + b);
}

int get warningCount {
Expand All @@ -249,7 +258,7 @@ class PackageWarningCounter {
options.asWarnings.contains(w) && !options.asErrors.contains(w)
? _warningCounts[w]
: 0)
.reduce((a, b) => a + b);
.fold(0, (a, b) => a + b);
}

@override
Expand Down
2 changes: 1 addition & 1 deletion pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -350,4 +350,4 @@ packages:
source: hosted
version: "2.1.13"
sdks:
dart: ">=1.23.0 <=2.0.0-dev.6.0"
dart: ">=1.23.0 <=2.0.0-dev.7.0"