Use PackageWarning system for multiple file overwrite errors (#2111)
* Do not assert on multiple file crashes, use PackageWarning error instead.
* Clean up error messages
diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart
index 0a8e252..2c10570 100644
--- a/lib/dartdoc.dart
+++ b/lib/dartdoc.dart
@@ -105,7 +105,7 @@
for (var generator in generators) {
await generator.generate(packageGraph, outputDir.path);
- writtenFiles.addAll(generator.writtenFiles.map(path.normalize));
+ writtenFiles.addAll(generator.writtenFiles.keys.map(path.normalize));
}
if (config.validateLinks && writtenFiles.isNotEmpty) {
validateLinks(packageGraph, outputDir.path);
@@ -140,7 +140,7 @@
dartdocResults.packageGraph.packageWarningCounter.errorCount;
if (errorCount > 0) {
throw DartdocFailure(
- "dartdoc encountered $errorCount} errors while processing.");
+ "dartdoc encountered $errorCount errors while processing.");
}
logInfo(
'Success! Docs generated into ${dartdocResults.outDir.absolute.path}');
diff --git a/lib/src/empty_generator.dart b/lib/src/empty_generator.dart
index 016f2d1..d879574 100644
--- a/lib/src/empty_generator.dart
+++ b/lib/src/empty_generator.dart
@@ -5,9 +5,10 @@
import 'package:dartdoc/src/generator.dart';
import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/model_utils.dart';
+import 'package:dartdoc/src/warnings.dart';
/// A generator that does not generate files, but does traverse the [PackageGraph]
-/// and access [ModelElement.documetationAsHtml] for every element as though
+/// and access [ModelElement.documentationAsHtml] for every element as though
/// it were.
class EmptyGenerator extends Generator {
@override
@@ -36,5 +37,5 @@
Stream<void> get onFileCreated => _onFileCreated.stream;
@override
- Set<String> get writtenFiles => Set();
+ final Map<String, Warnable> writtenFiles = {};
}
diff --git a/lib/src/generator.dart b/lib/src/generator.dart
index 758659a..d062ae9 100644
--- a/lib/src/generator.dart
+++ b/lib/src/generator.dart
@@ -8,6 +8,7 @@
import 'dart:async' show Stream, Future;
import 'package:dartdoc/src/model/model.dart' show PackageGraph;
+import 'package:dartdoc/src/warnings.dart';
/// An abstract class that defines a generator that generates documentation for
/// a given package.
@@ -22,5 +23,5 @@
Stream<void> get onFileCreated;
/// Fetches all filenames written by this generator.
- Set<String> get writtenFiles;
+ Map<String, Warnable> get writtenFiles;
}
diff --git a/lib/src/html/html_generator.dart b/lib/src/html/html_generator.dart
index f346499..ca9b0c6 100644
--- a/lib/src/html/html_generator.dart
+++ b/lib/src/html/html_generator.dart
@@ -15,6 +15,7 @@
import 'package:dartdoc/src/html/template_data.dart';
import 'package:dartdoc/src/html/templates.dart';
import 'package:dartdoc/src/model/model.dart';
+import 'package:dartdoc/src/warnings.dart';
import 'package:path/path.dart' as path;
typedef Renderer = String Function(String input);
@@ -48,7 +49,7 @@
Stream<void> get onFileCreated => _onFileCreated.stream;
@override
- final Set<String> writtenFiles = Set<String>();
+ final Map<String, Warnable> writtenFiles = {};
static Future<HtmlGenerator> create(
{HtmlGeneratorOptions options,
@@ -83,14 +84,23 @@
assert(_instance == null);
var enabled = true;
- void write(String filePath, Object content, {bool allowOverwrite}) {
+ void write(String filePath, Object content,
+ {bool allowOverwrite, Warnable element}) {
allowOverwrite ??= false;
if (!enabled) {
throw StateError('`write` was called after `generate` completed.');
}
- // If you see this assert, we're probably being called to build non-canonical
- // docs somehow. Check data.self.isCanonical and callers for bugs.
- assert(allowOverwrite || !writtenFiles.contains(filePath));
+ if (!allowOverwrite) {
+ if (writtenFiles.containsKey(filePath)) {
+ assert(element != null,
+ 'Attempted overwrite of ${filePath} without corresponding element');
+ Warnable originalElement = writtenFiles[filePath];
+ Iterable<Warnable> referredFrom =
+ originalElement != null ? [originalElement] : null;
+ element?.warn(PackageWarning.duplicateFile,
+ message: filePath, referredFrom: referredFrom);
+ }
+ }
var file = File(path.join(outputDirectoryPath, filePath));
var parent = file.parent;
@@ -107,7 +117,7 @@
content, 'content', '`content` must be `String` or `List<int>`.');
}
_onFileCreated.add(file);
- writtenFiles.add(filePath);
+ writtenFiles[filePath] = element;
}
try {
diff --git a/lib/src/html/html_generator_instance.dart b/lib/src/html/html_generator_instance.dart
index b3adb6c..0ac5d7f 100644
--- a/lib/src/html/html_generator_instance.dart
+++ b/lib/src/html/html_generator_instance.dart
@@ -21,7 +21,7 @@
import 'package:path/path.dart' as path;
typedef FileWriter = void Function(String path, Object content,
- {bool allowOverwrite});
+ {bool allowOverwrite, Warnable element});
class HtmlGeneratorInstance {
final HtmlGeneratorOptions _options;
@@ -412,8 +412,8 @@
// Replaces '/' separators with proper separators for the platform.
String outFile = path.joinAll(filename.split('/'));
String content = template.renderString(data);
-
- _writer(outFile, content);
+ _writer(outFile, content,
+ element: data.self is Warnable ? data.self : null);
if (data.self is Indexable) _indexedElements.add(data.self as Indexable);
}
}
diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart
index 1025e27..83043bb 100644
--- a/lib/src/model/package_graph.dart
+++ b/lib/src/model/package_graph.dart
@@ -405,6 +405,11 @@
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;
}
List<String> messageParts = [warningMessage];
diff --git a/lib/src/warnings.dart b/lib/src/warnings.dart
index 2d59d7b..e1941fb 100644
--- a/lib/src/warnings.dart
+++ b/lib/src/warnings.dart
@@ -193,6 +193,19 @@
"unresolved-export",
"An export refers to a URI that cannot be resolved.",
defaultWarningMode: PackageWarningMode.error),
+ PackageWarning.duplicateFile: PackageWarningDefinition(
+ PackageWarning.duplicateFile,
+ "duplicate-file",
+ "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."
+ ],
+ defaultWarningMode: PackageWarningMode.error),
};
/// Something that package warnings can be called on. Optionally associated
@@ -222,6 +235,7 @@
unknownMacro,
unknownHtmlFragment,
brokenLink,
+ duplicateFile,
orphanedFile,
unknownFile,
missingFromSearchIndex,
diff --git a/test/html_generator_test.dart b/test/html_generator_test.dart
index cda62d7..6cfdee0 100644
--- a/test/html_generator_test.dart
+++ b/test/html_generator_test.dart
@@ -9,9 +9,13 @@
import 'package:dartdoc/src/html/html_generator.dart';
import 'package:dartdoc/src/html/templates.dart';
import 'package:dartdoc/src/html/resources.g.dart';
+import 'package:dartdoc/src/model/package_graph.dart';
+import 'package:dartdoc/src/warnings.dart';
import 'package:path/path.dart' as path;
import 'package:test/test.dart';
+import 'src/utils.dart' as utils;
+
void main() {
group('Templates', () {
Templates templates;
@@ -90,6 +94,37 @@
}
});
});
+
+ group('for a package that causes duplicate files', () {
+ HtmlGenerator generator;
+ PackageGraph packageGraph;
+ Directory tempOutput;
+
+ setUp(() async {
+ generator = await HtmlGenerator.create();
+ packageGraph = await utils
+ .bootBasicPackage(utils.testPackageDuplicateDir.path, []);
+ tempOutput = await Directory.systemTemp.createTemp('doc_test_temp');
+ });
+
+ tearDown(() {
+ if (tempOutput != null) {
+ tempOutput.deleteSync(recursive: true);
+ }
+ });
+
+ test('run generator and verify duplicate file error', () async {
+ await generator.generate(packageGraph, tempOutput.path);
+ expect(generator, isNotNull);
+ expect(tempOutput, isNotNull);
+ String expectedPath =
+ path.join('aDuplicate', 'aDuplicate-library.html');
+ expect(
+ packageGraph.localPublicLibraries,
+ anyElement((l) => packageGraph.packageWarningCounter
+ .hasWarning(l, PackageWarning.duplicateFile, expectedPath)));
+ }, timeout: Timeout.factor(2));
+ });
});
}
diff --git a/test/src/utils.dart b/test/src/utils.dart
index f387dcc..b5d8a79 100644
--- a/test/src/utils.dart
+++ b/test/src/utils.dart
@@ -59,6 +59,8 @@
final Directory testPackageBadDir = Directory('testing/test_package_bad');
final Directory testPackageDir = Directory('testing/test_package');
+final Directory testPackageDuplicateDir =
+ Directory('testing/test_package_duplicate');
final Directory testPackageExperimentsDir =
Directory('testing/test_package_experiments');
final Directory testPackageMinimumDir =
diff --git a/testing/test_package_duplicate/lib/first.dart b/testing/test_package_duplicate/lib/first.dart
new file mode 100644
index 0000000..d369bda
--- /dev/null
+++ b/testing/test_package_duplicate/lib/first.dart
@@ -0,0 +1 @@
+library aDuplicate;
diff --git a/testing/test_package_duplicate/lib/second.dart b/testing/test_package_duplicate/lib/second.dart
new file mode 100644
index 0000000..d369bda
--- /dev/null
+++ b/testing/test_package_duplicate/lib/second.dart
@@ -0,0 +1 @@
+library aDuplicate;
diff --git a/testing/test_package_duplicate/pubspec.yaml b/testing/test_package_duplicate/pubspec.yaml
new file mode 100644
index 0000000..f0f2729
--- /dev/null
+++ b/testing/test_package_duplicate/pubspec.yaml
@@ -0,0 +1 @@
+name: test_package_duplicate
