Other cleanup (#1549)

* HtmlGeneratorInstance: cleanup fields and properties

Remove unused properties
Mark private anything that's not needed externally

* Pass around paths, not Directories

Existing codepaths ensure the directory is already created

Author: kevmoo

lib/dartdoc.dart

@@ -162,7 +162,7 @@ class DartDoc extends PackageBuilder {
     if (!outputDir.existsSync()) outputDir.createSync(recursive: true);
 
     for (var generator in generators) {
-      await generator.generate(package, outputDir);
+      await generator.generate(package, outputDir.path);
       writtenFiles.addAll(generator.writtenFiles.map(path.normalize));
     }
 

lib/src/generator.dart

@@ -6,7 +6,7 @@
 library dartdoc.generator;
 
 import 'dart:async' show Stream, Future;
-import 'dart:io' show Directory, File;
+import 'dart:io' show File;
 
 import 'model.dart' show Package;
 
@@ -17,7 +17,7 @@ import 'model.dart' show Package;
 abstract class Generator {
   /// Generate the documentation for the given package in the specified
   /// directory. Completes the returned future when done.
-  Future generate(Package package, Directory out);
+  Future generate(Package package, String outputDirectoryPath);
 
   /// Fires when a file is created.
   Stream<File> get onFileCreated;

lib/src/html/html_generator.dart

@@ -5,7 +5,7 @@
 library dartdoc.html_generator;
 
 import 'dart:async' show Future, StreamController, Stream;
-import 'dart:io' show Directory, File;
+import 'dart:io' show File;
 
 import '../generator.dart';
 import '../model.dart';
@@ -46,7 +46,6 @@ class HtmlGenerator extends Generator {
   @override
   Set<String> get writtenFiles => _instance.writtenFiles;
 
-  /// [url] - optional URL for where the docs will be hosted.
   static Future<HtmlGenerator> create(
       {HtmlGeneratorOptions options,
       List<String> headers,
@@ -67,10 +66,10 @@ class HtmlGenerator extends Generator {
 
   /// Actually write out the documentation for [package].
   /// Stores the HtmlGeneratorInstance so we can access it in [writtenFiles].
-  Future generate(Package package, Directory out) {
+  Future generate(Package package, String outputDirectoryPath) {
     assert(_instance == null);
     _instance = new HtmlGeneratorInstance(
-        _options, _templates, package, out, _onFileCreated);
+        _options, _templates, package, outputDirectoryPath, _onFileCreated);
     return _instance.generate();
   }
 }

lib/src/html/html_generator_instance.dart

@@ -21,52 +21,52 @@ import 'templates.dart';
 
 class HtmlGeneratorInstance implements HtmlOptions {
   final HtmlGeneratorOptions _options;
-
-  String get url => _options.url;
   final Templates _templates;
-  final Package package;
-  final Directory out;
-  final List<ModelElement> documentedElements = <ModelElement>[];
+  final Package _package;
+  final String _outputDirectoryPath;
+  final List<ModelElement> _documentedElements = <ModelElement>[];
   final StreamController<File> _onFileCreated;
+
   @override
   String get relCanonicalPrefix => _options.relCanonicalPrefix;
+
   @override
   String get toolVersion => _options.toolVersion;
-  String get faviconPath => _options.faviconPath;
-  bool get useCategories => _options.useCategories;
-  bool get prettyIndexJson => _options.prettyIndexJson;
+
+  String get _faviconPath => _options.faviconPath;
+  bool get _useCategories => _options.useCategories;
 
   // Protect against bugs in canonicalization by tracking what files we
   // write.
   final Set<String> writtenFiles = new Set<String>();
 
-  HtmlGeneratorInstance(this._options, this._templates, this.package, this.out,
-      this._onFileCreated);
+  HtmlGeneratorInstance(this._options, this._templates, this._package,
+      this._outputDirectoryPath, this._onFileCreated);
 
   Future generate() async {
-    if (!out.existsSync()) out.createSync();
-
-    if (package != null) {
+    if (_package != null) {
       _generateDocs();
       _generateSearchIndex();
     }
 
     await _copyResources();
-    if (faviconPath != null) {
-      var bytes = new File(faviconPath).readAsBytesSync();
+    if (_faviconPath != null) {
+      var bytes = new File(_faviconPath).readAsBytesSync();
       // Allow overwrite of favicon.
-      String filename = path.join(out.path, 'static-assets', 'favicon.png');
+      String filename =
+          path.join(_outputDirectoryPath, 'static-assets', 'favicon.png');
       writtenFiles.remove(filename);
-      _writeFile(path.join(out.path, 'static-assets', 'favicon.png'), bytes);
+      _writeFile(filename, bytes);
     }
   }
 
   void _generateSearchIndex() {
-    var encoder =
-        prettyIndexJson ? new JsonEncoder.withIndent(' ') : new JsonEncoder();
+    var encoder = _options.prettyIndexJson
+        ? new JsonEncoder.withIndent(' ')
+        : new JsonEncoder();
 
     final List<Map> indexItems =
-        documentedElements.where((e) => e.isCanonical).map((ModelElement e) {
+        _documentedElements.where((e) => e.isCanonical).map((ModelElement e) {
       Map data = {
         'name': e.name,
         'qualifiedName': e.name,
@@ -95,91 +95,91 @@ class HtmlGeneratorInstance implements HtmlOptions {
     });
 
     String json = encoder.convert(indexItems);
-    _writeFile(path.join(out.path, 'index.json'), '${json}\n');
+    _writeFile(path.join(_outputDirectoryPath, 'index.json'), '${json}\n');
   }
 
   void _generateDocs() {
-    if (package == null) return;
+    if (_package == null) return;
 
     generatePackage();
 
-    for (var lib in filterNonDocumented(package.libraries)) {
+    for (var lib in filterNonDocumented(_package.libraries)) {
       // if (lib.name != 'extract_messages') continue;
-      generateLibrary(package, lib);
+      generateLibrary(_package, lib);
 
       for (var clazz in filterNonDocumented(lib.allClasses)) {
-        generateClass(package, lib, clazz);
+        generateClass(_package, lib, clazz);
 
         for (var constructor in filterNonDocumented(clazz.constructors)) {
           if (!constructor.isCanonical) continue;
-          generateConstructor(package, lib, clazz, constructor);
+          generateConstructor(_package, lib, clazz, constructor);
         }
 
         for (var constant in filterNonDocumented(clazz.constants)) {
           if (!constant.isCanonical) continue;
-          generateConstant(package, lib, clazz, constant);
+          generateConstant(_package, lib, clazz, constant);
         }
 
         for (var property in filterNonDocumented(clazz.staticProperties)) {
           if (!property.isCanonical) continue;
-          generateProperty(package, lib, clazz, property);
+          generateProperty(_package, lib, clazz, property);
         }
 
         for (var property in filterNonDocumented(clazz.propertiesForPages)) {
           if (!property.isCanonical) continue;
-          generateProperty(package, lib, clazz, property);
+          generateProperty(_package, lib, clazz, property);
         }
 
         for (var method in filterNonDocumented(clazz.methodsForPages)) {
           if (!method.isCanonical) continue;
-          generateMethod(package, lib, clazz, method);
+          generateMethod(_package, lib, clazz, method);
         }
 
         for (var operator in filterNonDocumented(clazz.operatorsForPages)) {
           if (!operator.isCanonical) continue;
-          generateMethod(package, lib, clazz, operator);
+          generateMethod(_package, lib, clazz, operator);
         }
 
         for (var method in filterNonDocumented(clazz.staticMethods)) {
           if (!method.isCanonical) continue;
-          generateMethod(package, lib, clazz, method);
+          generateMethod(_package, lib, clazz, method);
         }
       }
 
       for (var eNum in filterNonDocumented(lib.enums)) {
-        generateEnum(package, lib, eNum);
+        generateEnum(_package, lib, eNum);
         for (var property in filterNonDocumented(eNum.propertiesForPages)) {
-          generateProperty(package, lib, eNum, property);
+          generateProperty(_package, lib, eNum, property);
         }
         for (var operator in filterNonDocumented(eNum.operatorsForPages)) {
-          generateMethod(package, lib, eNum, operator);
+          generateMethod(_package, lib, eNum, operator);
         }
         for (var method in filterNonDocumented(eNum.methodsForPages)) {
-          generateMethod(package, lib, eNum, method);
+          generateMethod(_package, lib, eNum, method);
         }
       }
 
       for (var constant in filterNonDocumented(lib.constants)) {
-        generateTopLevelConstant(package, lib, constant);
+        generateTopLevelConstant(_package, lib, constant);
       }
 
       for (var property in filterNonDocumented(lib.properties)) {
-        generateTopLevelProperty(package, lib, property);
+        generateTopLevelProperty(_package, lib, property);
       }
 
       for (var function in filterNonDocumented(lib.functions)) {
-        generateFunction(package, lib, function);
+        generateFunction(_package, lib, function);
       }
 
       for (var typeDef in filterNonDocumented(lib.typedefs)) {
-        generateTypeDef(package, lib, typeDef);
+        generateTypeDef(_package, lib, typeDef);
       }
     }
   }
 
   void generatePackage() {
-    TemplateData data = new PackageTemplateData(this, package, useCategories);
-    logInfo('documenting ${package.name}');
+    TemplateData data = new PackageTemplateData(this, _package, _useCategories);
+    logInfo('documenting ${_package.name}');
 
     _build('index.html', _templates.indexTemplate, data);
   }
@@ -191,7 +191,7 @@ class HtmlGeneratorInstance implements HtmlOptions {
       package.warnOnElement(lib, PackageWarning.noLibraryLevelDocs);
     }
     TemplateData data =
-        new LibraryTemplateData(this, package, lib, useCategories);
+        new LibraryTemplateData(this, package, lib, _useCategories);
 
     _build(path.join(lib.dirName, '${lib.fileName}'),
         _templates.libraryTemplate, data);
@@ -285,20 +285,20 @@ class HtmlGeneratorInstance implements HtmlOptions {
             'encountered $resourcePath');
       }
       String destFileName = resourcePath.substring(prefix.length);
-      _writeFile(path.join(out.path, 'static-assets', destFileName),
+      _writeFile(path.join(_outputDirectoryPath, 'static-assets', destFileName),
           await loader.loadAsBytes(resourcePath));
     }
   }
 
   void _build(String filename, TemplateRenderer template, TemplateData data) {
-    String fullName = path.join(out.path, filename);
+    String fullName = path.join(_outputDirectoryPath, filename);
 
     String content = template(data,
         assumeNullNonExistingProperty: false, errorOnMissingProperty: true);
 
     _writeFile(fullName, content);
     if (data.self is ModelElement) {
-      documentedElements.add(data.self);
+      _documentedElements.add(data.self);
     }
   }
 

test/html_generator_test.dart

@@ -76,7 +76,7 @@ void main() {
       setUp(() async {
         generator = await HtmlGenerator.create();
         tempOutput = Directory.systemTemp.createTempSync('doc_test_temp');
-        return generator.generate(null, tempOutput);
+        return generator.generate(null, tempOutput.path);
       });
 
       tearDown(() {