Skip to content

Commit fea0b0d

Browse files
committed
Merge branch 'master' into functiontypeimpl-crash
2 parents 365e034 + 285b4df commit fea0b0d

12 files changed

+85
-14
lines changed

lib/dartdoc.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ class Dartdoc extends PackageBuilder {
105105

106106
for (var generator in generators) {
107107
await generator.generate(packageGraph, outputDir.path);
108-
writtenFiles.addAll(generator.writtenFiles.map(path.normalize));
108+
writtenFiles.addAll(generator.writtenFiles.keys.map(path.normalize));
109109
}
110110
if (config.validateLinks && writtenFiles.isNotEmpty) {
111111
validateLinks(packageGraph, outputDir.path);
@@ -140,7 +140,7 @@ class Dartdoc extends PackageBuilder {
140140
dartdocResults.packageGraph.packageWarningCounter.errorCount;
141141
if (errorCount > 0) {
142142
throw DartdocFailure(
143-
"dartdoc encountered $errorCount} errors while processing.");
143+
"dartdoc encountered $errorCount errors while processing.");
144144
}
145145
logInfo(
146146
'Success! Docs generated into ${dartdocResults.outDir.absolute.path}');

lib/src/empty_generator.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@ import 'dart:async';
55
import 'package:dartdoc/src/generator.dart';
66
import 'package:dartdoc/src/model/model.dart';
77
import 'package:dartdoc/src/model_utils.dart';
8+
import 'package:dartdoc/src/warnings.dart';
89

910
/// A generator that does not generate files, but does traverse the [PackageGraph]
10-
/// and access [ModelElement.documetationAsHtml] for every element as though
11+
/// and access [ModelElement.documentationAsHtml] for every element as though
1112
/// it were.
1213
class EmptyGenerator extends Generator {
1314
@override
@@ -36,5 +37,5 @@ class EmptyGenerator extends Generator {
3637
Stream<void> get onFileCreated => _onFileCreated.stream;
3738

3839
@override
39-
Set<String> get writtenFiles => Set();
40+
final Map<String, Warnable> writtenFiles = {};
4041
}

lib/src/generator.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ library dartdoc.generator;
88
import 'dart:async' show Stream, Future;
99

1010
import 'package:dartdoc/src/model/model.dart' show PackageGraph;
11+
import 'package:dartdoc/src/warnings.dart';
1112

1213
/// An abstract class that defines a generator that generates documentation for
1314
/// a given package.
@@ -22,5 +23,5 @@ abstract class Generator {
2223
Stream<void> get onFileCreated;
2324

2425
/// Fetches all filenames written by this generator.
25-
Set<String> get writtenFiles;
26+
Map<String, Warnable> get writtenFiles;
2627
}

lib/src/html/html_generator.dart

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import 'package:dartdoc/src/html/html_generator_instance.dart';
1515
import 'package:dartdoc/src/html/template_data.dart';
1616
import 'package:dartdoc/src/html/templates.dart';
1717
import 'package:dartdoc/src/model/model.dart';
18+
import 'package:dartdoc/src/warnings.dart';
1819
import 'package:path/path.dart' as path;
1920

2021
typedef Renderer = String Function(String input);
@@ -48,7 +49,7 @@ class HtmlGenerator extends Generator {
4849
Stream<void> get onFileCreated => _onFileCreated.stream;
4950

5051
@override
51-
final Set<String> writtenFiles = Set<String>();
52+
final Map<String, Warnable> writtenFiles = {};
5253

5354
static Future<HtmlGenerator> create(
5455
{HtmlGeneratorOptions options,
@@ -83,14 +84,23 @@ class HtmlGenerator extends Generator {
8384
assert(_instance == null);
8485

8586
var enabled = true;
86-
void write(String filePath, Object content, {bool allowOverwrite}) {
87+
void write(String filePath, Object content,
88+
{bool allowOverwrite, Warnable element}) {
8789
allowOverwrite ??= false;
8890
if (!enabled) {
8991
throw StateError('`write` was called after `generate` completed.');
9092
}
91-
// If you see this assert, we're probably being called to build non-canonical
92-
// docs somehow. Check data.self.isCanonical and callers for bugs.
93-
assert(allowOverwrite || !writtenFiles.contains(filePath));
93+
if (!allowOverwrite) {
94+
if (writtenFiles.containsKey(filePath)) {
95+
assert(element != null,
96+
'Attempted overwrite of ${filePath} without corresponding element');
97+
Warnable originalElement = writtenFiles[filePath];
98+
Iterable<Warnable> referredFrom =
99+
originalElement != null ? [originalElement] : null;
100+
element?.warn(PackageWarning.duplicateFile,
101+
message: filePath, referredFrom: referredFrom);
102+
}
103+
}
94104

95105
var file = File(path.join(outputDirectoryPath, filePath));
96106
var parent = file.parent;
@@ -107,7 +117,7 @@ class HtmlGenerator extends Generator {
107117
content, 'content', '`content` must be `String` or `List<int>`.');
108118
}
109119
_onFileCreated.add(file);
110-
writtenFiles.add(filePath);
120+
writtenFiles[filePath] = element;
111121
}
112122

113123
try {

lib/src/html/html_generator_instance.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import 'package:mustache/mustache.dart';
2121
import 'package:path/path.dart' as path;
2222

2323
typedef FileWriter = void Function(String path, Object content,
24-
{bool allowOverwrite});
24+
{bool allowOverwrite, Warnable element});
2525

2626
class HtmlGeneratorInstance {
2727
final HtmlGeneratorOptions _options;
@@ -412,8 +412,8 @@ class HtmlGeneratorInstance {
412412
// Replaces '/' separators with proper separators for the platform.
413413
String outFile = path.joinAll(filename.split('/'));
414414
String content = template.renderString(data);
415-
416-
_writer(outFile, content);
415+
_writer(outFile, content,
416+
element: data.self is Warnable ? data.self : null);
417417
if (data.self is Indexable) _indexedElements.add(data.self as Indexable);
418418
}
419419
}

lib/src/model/package_graph.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,11 @@ class PackageGraph {
413413
case PackageWarning.unresolvedExport:
414414
warningMessage = 'unresolved export uri: ${message}';
415415
break;
416+
case PackageWarning.duplicateFile:
417+
warningMessage = 'failed to write file at: ${message}';
418+
warnablePrefix = 'for symbol';
419+
referredFromPrefix = 'conflicting with file already generated by';
420+
break;
416421
}
417422

418423
List<String> messageParts = [warningMessage];

lib/src/warnings.dart

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,19 @@ final Map<PackageWarning, PackageWarningDefinition> packageWarningDefinitions =
193193
"unresolved-export",
194194
"An export refers to a URI that cannot be resolved.",
195195
defaultWarningMode: PackageWarningMode.error),
196+
PackageWarning.duplicateFile: PackageWarningDefinition(
197+
PackageWarning.duplicateFile,
198+
"duplicate-file",
199+
"Dartdoc is trying to write to a duplicate filename based on the names of Dart symbols.",
200+
longHelp: [
201+
"Dartdoc generates a path and filename to write to for each symbol.",
202+
"@@name@@ conflicts with another symbol in the generated path, and",
203+
"therefore can not be written out. Changing the name, library name, or",
204+
"class name (if appropriate) of one of the conflicting items can resolve",
205+
"the conflict. Alternatively, use the @nodoc tag in one symbol's",
206+
"documentation comments to hide it."
207+
],
208+
defaultWarningMode: PackageWarningMode.error),
196209
};
197210

198211
/// Something that package warnings can be called on. Optionally associated
@@ -222,6 +235,7 @@ enum PackageWarning {
222235
unknownMacro,
223236
unknownHtmlFragment,
224237
brokenLink,
238+
duplicateFile,
225239
orphanedFile,
226240
unknownFile,
227241
missingFromSearchIndex,

test/html_generator_test.dart

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,13 @@ import 'dart:io' show File, Directory;
99
import 'package:dartdoc/src/html/html_generator.dart';
1010
import 'package:dartdoc/src/html/templates.dart';
1111
import 'package:dartdoc/src/html/resources.g.dart';
12+
import 'package:dartdoc/src/model/package_graph.dart';
13+
import 'package:dartdoc/src/warnings.dart';
1214
import 'package:path/path.dart' as path;
1315
import 'package:test/test.dart';
1416

17+
import 'src/utils.dart' as utils;
18+
1519
void main() {
1620
group('Templates', () {
1721
Templates templates;
@@ -90,6 +94,37 @@ void main() {
9094
}
9195
});
9296
});
97+
98+
group('for a package that causes duplicate files', () {
99+
HtmlGenerator generator;
100+
PackageGraph packageGraph;
101+
Directory tempOutput;
102+
103+
setUp(() async {
104+
generator = await HtmlGenerator.create();
105+
packageGraph = await utils
106+
.bootBasicPackage(utils.testPackageDuplicateDir.path, []);
107+
tempOutput = await Directory.systemTemp.createTemp('doc_test_temp');
108+
});
109+
110+
tearDown(() {
111+
if (tempOutput != null) {
112+
tempOutput.deleteSync(recursive: true);
113+
}
114+
});
115+
116+
test('run generator and verify duplicate file error', () async {
117+
await generator.generate(packageGraph, tempOutput.path);
118+
expect(generator, isNotNull);
119+
expect(tempOutput, isNotNull);
120+
String expectedPath =
121+
path.join('aDuplicate', 'aDuplicate-library.html');
122+
expect(
123+
packageGraph.localPublicLibraries,
124+
anyElement((l) => packageGraph.packageWarningCounter
125+
.hasWarning(l, PackageWarning.duplicateFile, expectedPath)));
126+
}, timeout: Timeout.factor(2));
127+
});
93128
});
94129
}
95130

test/src/utils.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ Future<PackageGraph> get testPackageGraphSdk =>
5959

6060
final Directory testPackageBadDir = Directory('testing/test_package_bad');
6161
final Directory testPackageDir = Directory('testing/test_package');
62+
final Directory testPackageDuplicateDir =
63+
Directory('testing/test_package_duplicate');
6264
final Directory testPackageExperimentsDir =
6365
Directory('testing/test_package_experiments');
6466
final Directory testPackageMinimumDir =
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
library aDuplicate;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
library aDuplicate;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
name: test_package_duplicate

0 commit comments

Comments
 (0)