Fixed TODOs/bugs in codegen_tools.dart for code generation of a directory.

pkg/analysis_server/tool/spec/codegen_tools.dart

 // Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * Tools for code generation.
  */
 library codegen.tools;
 
 import 'dart:io';
@@ skipping 65 matching lines @@
   }
 
   /**
    * Execute [callback], indenting any code it outputs by two spaces.
    */
   void indent(void callback()) => indentSpecial('  ', '  ', callback);
 
   /**
    * Execute [callback], using [additionalIndent] to indent any code it outputs.
    */
-  void indentBy(String additionalIndent, void callback()) => indentSpecial(additionalIndent, additionalIndent, callback);
+  void indentBy(String additionalIndent, void callback()) =>
+      indentSpecial(additionalIndent, additionalIndent, callback);
 
   /**
    * Execute [callback], using [additionalIndent] to indent any code it outputs.
    * The first line of output is indented by [firstAdditionalIndent] instead of
    * [additionalIndent].
    */
-  void indentSpecial(String firstAdditionalIndent, String additionalIndent, void callback()) {
+  void indentSpecial(String firstAdditionalIndent, String additionalIndent, void
+      callback()) {
     String oldNextIndent = _state.nextIndent;
     String oldIndent = _state.indent;
     try {
       _state.nextIndent += firstAdditionalIndent;
       _state.indent += additionalIndent;
       callback();
     } finally {
       _state.nextIndent = oldNextIndent;
       _state.indent = oldIndent;
     }
   }
 
   /**
    * Measure the width of the current indentation level.
    */
   int get indentWidth => _state.nextIndent.length;
 
   /**
    * Generate a doc comment based on the HTML in [docs].
    *
    * If [javadocStyle] is true, then the output is compatable with Javadoc,
    * which understands certain HTML constructs.
    */
-  void docComment(List<dom.Node> docs, {int width: 79, bool javadocStyle: false}) {
+  void docComment(List<dom.Node> docs, {int width: 79, bool javadocStyle:
+      false}) {
     writeln('/**');
     indentBy(' * ', () {
       write(nodesToText(docs, width - _state.indent.length, javadocStyle));
     });
     writeln(' */');
   }
 
   void outputHeader({bool javaStyle: false}) {
     String header;
     if (javaStyle) {
@@ skipping 164 matching lines @@
       buffer.add(new dom.Text(lines.join('\n$indent') + '\n'));
       indentNeeded = true;
     } else {
       buffer.add(new dom.Text(lines.join('\n$indent')));
       indentNeeded = false;
     }
   }
 }
 
 /**
- * Type of functions used to compute the contents of generated files.
+ * Type of functions used to compute the contents of a generated file.
  */
 typedef String FileContentsComputer();
 
+/**
+ * Type of functions used to compute the contents of a set of generated files.
+ */
 typedef Map<String, FileContentsComputer> DirectoryContentsComputer();
 
 abstract class GeneratedContent {
   FileSystemEntity get outputFile;
   bool check();
   void generate();
 }
 
 /**
  * Class representing a single output file (either generated code or generated
@@ skipping 39 matching lines @@
   /**
    * Replace the file with the correct contents.  [spec] is the "tool/spec"
    * directory.  If [spec] is unspecified, it is assumed to be the directory
    * containing Platform.executable.
    */
   void generate() {
     outputFile.writeAsStringSync(computeContents());
   }
 }
 
+/**
+ * Class representing a single output directory (either generated code or
+ * generated HTML). No other content should exisit in the directory.
+ */
 class GeneratedDirectory extends GeneratedContent {
 
+  /**
+   * The path to the directory that will have the generated content.
+   */
   final String outputDirPath;
+
+  /**
+   * Callback function which computes the directory contents.
+   */
   final DirectoryContentsComputer directoryContentsComputer;
+
   GeneratedDirectory(this.outputDirPath, this.directoryContentsComputer);
 
   /**
    * Get a Directory object representing the output directory.
    */
-  Directory get outputFile => new Directory(joinAll(posix.split(outputDirPath)));
+  Directory get outputFile =>
+      new Directory(joinAll(posix.split(outputDirPath)));
 
+  /**
+   * Check whether the directory has the correct contents, and return true if it
+   * does.
+   */
   @override
   bool check() {
-    // TODO (jwren) the lists of files in the directories need to be compared to
-    // ensure no unexpected files have been added
     Map<String, FileContentsComputer> map = directoryContentsComputer();
     map.forEach((String file, FileContentsComputer fileContentsComputer) {
       String expectedContents = fileContentsComputer();
       File outputFile = new File(joinAll(posix.split(outputDirPath + file)));

[Paul Berry, 2014/08/14 12:44:05]

Can we change this to:

  File outputFile = new File(joinAll(posix.split(posix.join(outputDirPath,
file))));

That way the correct thing will happen even if outputDirPath doesn't end in '/'.

[jwren, 2014/08/14 18:12:29]

Done.

       try {
         if (expectedContents != outputFile.readAsStringSync()) {
           return false;
         }
       } catch (e) {

[Paul Berry, 2014/08/14 12:44:05]

Let's move the try/catch out so that its scope includes the
outputFile.listSync() call.  That way if the directory doesn't exist the user
will get an error message telling them to run code generation, rather than a
stacktrace.

[jwren, 2014/08/14 18:12:29]

Done.

         // There was a problem reading the file (most likely because it didn't
         // exist).  Treat that the same as if the file doesn't have the expected
         // contents.
         return false;
       }
     });
-    return true;
+    return outputFile.listSync().length == map.length;
   }
 
+  /**
+   * Replace the directory with the correct contents.  [spec] is the "tool/spec"
+   * directory.  If [spec] is unspecified, it is assumed to be the directory
+   * containing Platform.executable.
+   */
   @override
   void generate() {
-    // TODO (jwren) Delete contents in the directory first.
+    // delete the contents of the directory (and the directory itself)
+    outputFile.deleteSync(recursive: true);

[Paul Berry, 2014/08/14 12:44:05]

Put a try/catch around this line as well, so that if the directory doesn't exist
at the time generate() is called, we just create it without complaining.

[jwren, 2014/08/14 18:12:29]

Done.

+    // re-create the empty directory
+    outputFile.createSync(recursive: true);
+
+    // generate all of the files in the directory
     Map<String, FileContentsComputer> map = directoryContentsComputer();
     map.forEach((String file, FileContentsComputer fileContentsComputer) {
       File outputFile = new File(joinAll(posix.split(outputDirPath + file)));
       outputFile.writeAsStringSync(fileContentsComputer());
     });
   }
 }