Skip to content

Commit 5974085

Browse files
authored
Add unresolved export warning to dartdoc (#1748)
* Add unresolved export warning * Simplification * Split test package docs into dev and stable versions * Make stable and dev trees separate for Dart compare_output_test
1 parent db16c7f commit 5974085

File tree

11 files changed

+97
-17
lines changed

11 files changed

+97
-17
lines changed

analysis_options.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ analyzer:
88
- 'pub.dartlang.org/**'
99
- 'testing/**'
1010
- 'testing/test_package_flutter_plugin/**'
11+
- 'testing/test_package_export_error/**'
1112
linter:
1213
rules:
1314
- annotate_overrides

lib/dartdoc.dart

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ class Dartdoc extends PackageBuilder {
133133
/// [DartdocResults] is returned if dartdoc succeeds. [DartdocFailure] is
134134
/// thrown if dartdoc fails in an expected way, for example if there is an
135135
/// analysis error in the code.
136-
Future<DartdocResults> generateDocs() async {
136+
Future<DartdocResults> generateDocsBase() async {
137137
Stopwatch _stopwatch = new Stopwatch()..start();
138138
double seconds;
139139
packageGraph = await buildPackageGraph();
@@ -167,18 +167,22 @@ class Dartdoc extends PackageBuilder {
167167
logInfo(
168168
"Documented ${packageGraph.publicLibraries.length} public librar${packageGraph.publicLibraries.length == 1 ? 'y' : 'ies'} "
169169
"in ${seconds.toStringAsFixed(1)} seconds");
170+
return new DartdocResults(
171+
config.topLevelPackageMeta, packageGraph, outputDir);
172+
}
170173

171-
if (packageGraph.publicLibraries.isEmpty) {
174+
Future<DartdocResults> generateDocs() async {
175+
DartdocResults dartdocResults = await generateDocsBase();
176+
if (dartdocResults.packageGraph.publicLibraries.isEmpty) {
172177
throw new DartdocFailure(
173178
"dartdoc could not find any libraries to document. Run `pub get` and try again.");
174179
}
175180

176-
if (packageGraph.packageWarningCounter.errorCount > 0) {
181+
if (dartdocResults.packageGraph.packageWarningCounter.errorCount > 0) {
177182
throw new DartdocFailure("dartdoc encountered errors while processing");
178183
}
179184

180-
return new DartdocResults(
181-
config.topLevelPackageMeta, packageGraph, outputDir);
185+
return dartdocResults;
182186
}
183187

184188
/// Warn on file paths.

lib/src/model.dart

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4357,6 +4357,9 @@ class PackageGraph {
43574357
case PackageWarning.deprecated:
43584358
warningMessage = 'deprecated dartdoc usage: ${message}';
43594359
break;
4360+
case PackageWarning.unresolvedExport:
4361+
warningMessage = 'unresolved export uri: ${message}';
4362+
break;
43604363
}
43614364

43624365
List<String> messageParts = [warningMessage];
@@ -4425,11 +4428,26 @@ class PackageGraph {
44254428

44264429
Map<LibraryElement, Set<Library>> _libraryElementReexportedBy = new Map();
44274430
void _tagReexportsFor(
4428-
final Library tll, final LibraryElement libraryElement) {
4431+
final Library topLevelLibrary,
4432+
final LibraryElement libraryElement,
4433+
[ExportElement lastExportedElement]) {
4434+
if (libraryElement == null) {
4435+
// The first call to _tagReexportFor should not have a null libraryElement.
4436+
assert(lastExportedElement != null);
4437+
warnOnElement(
4438+
findOrCreateLibraryFor(lastExportedElement.enclosingElement),
4439+
PackageWarning.unresolvedExport,
4440+
message: '"${lastExportedElement.uri}"',
4441+
referredFrom: <Locatable>[topLevelLibrary]);
4442+
return;
4443+
}
44294444
_libraryElementReexportedBy.putIfAbsent(libraryElement, () => new Set());
4430-
_libraryElementReexportedBy[libraryElement].add(tll);
4445+
_libraryElementReexportedBy[libraryElement].add(topLevelLibrary);
44314446
for (ExportElement exportedElement in libraryElement.exports) {
4432-
_tagReexportsFor(tll, exportedElement.exportedLibrary);
4447+
_tagReexportsFor(
4448+
topLevelLibrary,
4449+
exportedElement.exportedLibrary,
4450+
exportedElement);
44334451
}
44344452
}
44354453

@@ -5674,6 +5692,7 @@ class PackageBuilder {
56745692
for (PackageWarning kind in PackageWarning.values) {
56755693
switch (kind) {
56765694
case PackageWarning.invalidParameter:
5695+
case PackageWarning.unresolvedExport:
56775696
warningOptions.error(kind);
56785697
break;
56795698
default:

lib/src/warnings.dart

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ final Map<PackageWarning, PackageWarningHelpText> packageWarningText = const {
9494
PackageWarning.deprecated,
9595
"deprecated",
9696
"A dartdoc directive has a deprecated format."),
97+
PackageWarning.unresolvedExport: const PackageWarningHelpText(
98+
PackageWarning.unresolvedExport,
99+
"unresolvedExport",
100+
"An export refers to a URI that can not be resolved."),
97101
};
98102

99103
/// Something that package warnings can be called on. Optionally associated
@@ -135,6 +139,7 @@ enum PackageWarning {
135139
typeAsHtml,
136140
invalidParameter,
137141
deprecated,
142+
unresolvedExport,
138143
}
139144

140145
/// Warnings it is OK to skip if we can determine the warnable isn't documented.
@@ -158,6 +163,7 @@ class PackageWarningOptions {
158163
PackageWarningOptions(this.verboseWarnings) {
159164
asWarnings.addAll(PackageWarning.values);
160165
ignore(PackageWarning.typeAsHtml);
166+
error(PackageWarning.unresolvedExport);
161167
}
162168

163169
void _assertInvariantsOk() {
@@ -194,7 +200,7 @@ class PackageWarningOptions {
194200
}
195201

196202
class PackageWarningCounter {
197-
final _countedWarnings =
203+
final countedWarnings =
198204
new Map<Element, Set<Tuple2<PackageWarning, String>>>();
199205
final _warningCounts = new Map<PackageWarning, int>();
200206
final PackageWarningOptions options;
@@ -249,8 +255,8 @@ class PackageWarningCounter {
249255
/// Returns true if we've already warned for this.
250256
bool hasWarning(Warnable element, PackageWarning kind, String message) {
251257
Tuple2<PackageWarning, String> warningData = new Tuple2(kind, message);
252-
if (_countedWarnings.containsKey(element?.element)) {
253-
return _countedWarnings[element?.element].contains(warningData);
258+
if (countedWarnings.containsKey(element?.element)) {
259+
return countedWarnings[element?.element].contains(warningData);
254260
}
255261
return false;
256262
}
@@ -263,8 +269,8 @@ class PackageWarningCounter {
263269
Tuple2<PackageWarning, String> warningData = new Tuple2(kind, message);
264270
_warningCounts.putIfAbsent(kind, () => 0);
265271
_warningCounts[kind] += 1;
266-
_countedWarnings.putIfAbsent(element?.element, () => new Set());
267-
_countedWarnings[element?.element].add(warningData);
272+
countedWarnings.putIfAbsent(element?.element, () => new Set());
273+
countedWarnings[element?.element].add(warningData);
268274
_writeWarning(kind, element?.fullyQualifiedName, fullMessage);
269275
}
270276

test/dartdoc_test.dart

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import 'dart:io';
99

1010
import 'package:dartdoc/dartdoc.dart';
1111
import 'package:dartdoc/src/model.dart';
12+
import 'package:dartdoc/src/tuple.dart';
13+
import 'package:dartdoc/src/warnings.dart';
1214
import 'package:path/path.dart' as pathLib;
1315
import 'package:test/test.dart';
1416

@@ -33,9 +35,25 @@ void main() {
3335
argv..addAll(['--input', packageRoot.path])..addAll(outputParam)));
3436
}
3537

38+
test('with broken reexport chain', () async {
39+
Dartdoc dartdoc = await buildDartdoc([], testPackageImportExportError);
40+
DartdocResults results = await dartdoc.generateDocsBase();
41+
PackageGraph p = results.packageGraph;
42+
Iterable<String> unresolvedExportWarnings = p
43+
.packageWarningCounter.countedWarnings.values
44+
.expand<String>((Set<Tuple2<PackageWarning, String>> s) => s
45+
.where((Tuple2<PackageWarning, String> t) =>
46+
t.item1 == PackageWarning.unresolvedExport)
47+
.map<String>((Tuple2<PackageWarning, String> t) => t.item2));
48+
49+
expect(unresolvedExportWarnings.length, equals(1));
50+
expect(unresolvedExportWarnings.first,
51+
equals('"package:not_referenced_in_pubspec/library3.dart"'));
52+
});
53+
3654
group('include/exclude parameters', () {
3755
test('with config file', () async {
38-
Dartdoc dartdoc = await buildDartdoc([], testPackageImportExport);
56+
Dartdoc dartdoc = await buildDartdoc([], testPackageIncludeExclude);
3957
DartdocResults results = await dartdoc.generateDocs();
4058
PackageGraph p = results.packageGraph;
4159
expect(p.localPublicLibraries.map((l) => l.name),
@@ -44,7 +62,7 @@ void main() {
4462

4563
test('with include command line argument', () async {
4664
Dartdoc dartdoc = await buildDartdoc(
47-
['--include', 'another_included'], testPackageImportExport);
65+
['--include', 'another_included'], testPackageIncludeExclude);
4866
DartdocResults results = await dartdoc.generateDocs();
4967
PackageGraph p = results.packageGraph;
5068
expect(p.localPublicLibraries.length, equals(1));
@@ -53,7 +71,7 @@ void main() {
5371

5472
test('with exclude command line argument', () async {
5573
Dartdoc dartdoc = await buildDartdoc(
56-
['--exclude', 'more_included'], testPackageImportExport);
74+
['--exclude', 'more_included'], testPackageIncludeExclude);
5775
DartdocResults results = await dartdoc.generateDocs();
5876
PackageGraph p = results.packageGraph;
5977
expect(p.localPublicLibraries.length, equals(1));

test/src/utils.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@ final Directory testPackageWithEmbedderYaml =
2727
new Directory('testing/test_package_embedder_yaml');
2828
final Directory testPackageWithNoReadme =
2929
new Directory('testing/test_package_small');
30-
final Directory testPackageImportExport =
30+
final Directory testPackageIncludeExclude =
3131
new Directory('testing/test_package_include_exclude');
32+
final Directory testPackageImportExportError =
33+
new Directory('testing/test_package_import_export_error');
3234

3335
/// Convenience factory to build a [DartdocGeneratorOptionContext] and associate
3436
/// it with a [DartdocOptionSet] based on the current working directory and/or
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
library library1;
2+
3+
/// An invalid reexport - a non-existent library3.dart from 'not_referenced_in_pubspec'
4+
/// package.
5+
export 'package:not_referenced_in_pubspec/library3.dart' show Lib3Class;
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
library library2;
2+
3+
export 'package:test_package_export_error/library1.dart';
4+
5+
class Lib2Class {}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
name: test_package_export_error
2+
version: 0.0.1
3+
description: A simple console application.
4+
environment:
5+
sdk: <=3.0.0
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
library main;
2+
3+
export 'package:test_package_export_error/library2.dart';
4+
5+
/// This is an important class.
6+
class BugFreeClass {}

0 commit comments

Comments
 (0)