Change to use Pathos

pkg/docgen/lib/docgen.dart

Index: pkg/docgen/lib/docgen.dart
diff --git a/pkg/docgen/lib/docgen.dart b/pkg/docgen/lib/docgen.dart
index 49d659bf06488434e3a0ea94ada16df71200d2c7..b5149bfbca9518905f554a98ee228ed4659a1bc5 100644
--- a/pkg/docgen/lib/docgen.dart
+++ b/pkg/docgen/lib/docgen.dart
@@ -13,6 +13,7 @@
  * This creates files called `docs/<library_name>.yaml` in your current 
  * working directory.
  */
+
 library docgen;
 
 import 'dart:io';
@@ -22,8 +23,10 @@ import 'dart:async';
 import 'package:args/args.dart';
 import 'package:logging/logging.dart';
 import 'package:markdown/markdown.dart' as markdown;
+import 'package:pathos/path.dart' as path;
 
 import 'dart2yaml.dart';
+import 'io.dart';
 import '../../../sdk/lib/_internal/compiler/compiler.dart' as api;
 import '../../../sdk/lib/_internal/compiler/implementation/filenames.dart';
 import '../../../sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart'
@@ -34,47 +37,8 @@ import '../../../sdk/lib/_internal/compiler/implementation/source_file_provider.
 
 var logger = new Logger("Docgen");
 
-/// Counter used to provide unique IDs for each distinct item.  
-int _nextID = 0;
-
-int get nextID => _nextID++;
-
 const String usage = "Usage: dart docgen.dart [OPTIONS] [fooDir/barFile]";
 
-List<Path> listLibraries(List<String> args) {
-  if (args.length != 1) {
-    throw new UnsupportedError(usage);
-  }
-  var libraries = new List<Path>();
-  var type = FileSystemEntity.typeSync(args[0]);
-  
-  if (type == FileSystemEntityType.NOT_FOUND) { 
-    throw new UnsupportedError("File does not exist. $usage");
-  } else if (type == FileSystemEntityType.LINK) {
-    libraries.addAll(listLibrariesFromDir(new Link(args[0]).targetSync()));
-  } else if (type == FileSystemEntityType.FILE) {
-    libraries.add(new Path(args[0]));
-    logger.info("Added to libraries: ${libraries.last.toString()}");
-  } else if (type == FileSystemEntityType.DIRECTORY) {
-    libraries.addAll(listLibrariesFromDir(args[0]));
-  }
-  return libraries;
-}
-
-List<Path> listLibrariesFromDir(String path) {
-  var libraries = new List<Path>();
-  new Directory(path).listSync(recursive: true, 
-      followLinks: true).forEach((file) {
-        if (new Path(file.path).extension == "dart") {
-          if (!file.path.contains("/packages/")) {
-            libraries.add(new Path(file.path));
-            logger.info("Added to libraries: ${libraries.last.toString()}");
-          }
-        }
-      });
-  return libraries;
-}
-
 /**
  * This class documents a list of libraries.
  */
@@ -82,7 +46,7 @@ class Docgen {
 
   /// Libraries to be documented.
   List<LibraryMirror> _libraries;
-  
+
   /// Current library being documented to be used for comment links.
   LibraryMirror _currentLibrary;
   
@@ -95,6 +59,9 @@ class Docgen {
   /// Resolves reference links
   markdown.Resolver linkResolver;
   
+  /// Package Directory of 

[Andrei Mouravski, 2013/06/25 18:37:41]

Of what? don't leave me in suspense!

Also, comments should start with a capital and end with a period. Fix the one
above?

[janicejl, 2013/06/25 19:47:50]

Done.

+  String packageDir;
+  
   bool outputToYaml;
   bool outputToJson;
   bool includePrivate;
@@ -125,30 +92,64 @@ class Docgen {
     
     this.linkResolver = (name) => 
         fixReference(name, _currentLibrary, _currentClass, _currentMember);
+    
+    analyze(argResults.rest);
+  }
+  
+  List<String> listLibraries(List<String> args) {
+    if (args.length != 1) {

[Andrei Mouravski, 2013/06/25 18:37:41]

Remove the curlies and put this on one line.

[janicejl, 2013/06/25 19:47:50]

Done.

+      throw new UnsupportedError(usage);
+    }
+    var libraries = new List<String>();
+    var type = FileSystemEntity.typeSync(args[0]);
+    
+    if (type == FileSystemEntityType.NOT_FOUND) { 

[Andrei Mouravski, 2013/06/25 18:37:41]

How about you create an "else" for this whole big if else if stuff and move the
exception into the else block.

That way, you don't need to special case NOT_FOUND.

Like, what if type is null?

[janicejl, 2013/06/25 19:47:50]

Done.

+      throw new UnsupportedError("File does not exist. $usage");
+    } else if (type == FileSystemEntityType.LINK) {
+      var files = listDir(resolveLink(args[0]), recursive: true);

[Andrei Mouravski, 2013/06/25 18:37:41]

Split the first else if block into a separate function.

[janicejl, 2013/06/25 19:47:50]

Done.

+      libraries.addAll(files.where((f) => 
+          f.endsWith(".dart") && !f.contains("/packages")));
+      packageDir = files.firstWhere((f) => 
+          f.endsWith("/pubspec.yaml"), orElse: () => "");
+      packageDir = packageDir == "" ? 

[Andrei Mouravski, 2013/06/25 18:37:41]

Rewrite this as:
if (packageDir != '') packageDir = path.dirname(packageDir) + '/packages';

[janicejl, 2013/06/25 19:47:50]

Done.

+          "" : path.dirname(packageDir) + "/packages"; 

[Andrei Mouravski, 2013/06/25 18:37:41]

Change all " to '

[janicejl, 2013/06/25 19:47:50]

Done.

+    } else if (type == FileSystemEntityType.FILE) {
+      libraries.add(path.absolute(args[0]));
+      packageDir = "";
+    } else if (type == FileSystemEntityType.DIRECTORY) {
+      var files = listDir(args[0], recursive: true);

[Andrei Mouravski, 2013/06/25 18:37:41]

This is copypasta from above. Another good reason to pull out the function (use
the editor for this!)

[janicejl, 2013/06/25 19:47:50]

Done.

+      libraries.addAll(files.where((f) => 
+          f.endsWith(".dart") && !f.contains("/packages")));
+      packageDir = files.firstWhere((f) => 
+          f.endsWith("/pubspec.yaml"), orElse: () => "");
+      packageDir = packageDir == "" ? 
+          "" : path.dirname(packageDir) + "/packages"; 
+    }
+    logger.info("Package Directory: $packageDir");
+    libraries.forEach((lib) => logger.info("Added to libraries: $lib"));

[Andrei Mouravski, 2013/06/25 18:37:41]

You should be logging this as you add each library to libraries in case one of
them breaks, you know exactly which one it was.

[janicejl, 2013/06/25 19:47:50]

Done.

+    return libraries;
   }
   
   /**
    * Analyzes set of libraries by getting a mirror system and triggers the 
    * documentation of the libraries. 
    */
-  void analyze(List<Path> libraries) {
+  void analyze(List<String> args) {
+    var libraries = listLibraries(args);
+    if (libraries.isEmpty) throw new UnsupportedError("No Libraries. ");

[Andrei Mouravski, 2013/06/25 18:37:41]

Why is there a space after the period?

Also, make sure all " are '

[janicejl, 2013/06/25 19:47:50]

Done.

     // DART_SDK should be set to the root of the SDK library. 
     var sdkRoot = Platform.environment["DART_SDK"];
     if (sdkRoot != null) {
       logger.info("Using DART_SDK to find SDK at $sdkRoot");
-      sdkRoot = new Path(sdkRoot);
     } else {
       // If DART_SDK is not defined in the environment, 
       // assuming the dart executable is from the Dart SDK folder inside bin. 
-      sdkRoot = new Path(new Options().executable).directoryPath
-          .directoryPath;
-      logger.info("SDK Root: ${sdkRoot.toString()}");
+      sdkRoot = path.dirname(path.dirname(new Options().executable));
+      logger.info("SDK Root: ${sdkRoot}");
     }
     
-    Path packageDir = libraries.last.directoryPath.append("packages");
-    logger.info("Package Root: ${packageDir.toString()}");
-    getMirrorSystem(libraries, sdkRoot, 
-        packageRoot: packageDir).then((MirrorSystem mirrorSystem) {
+    getMirrorSystem(libraries, new Path(sdkRoot), 
+        packageRoot: new Path(packageDir)).then((MirrorSystem mirrorSystem) {

[Andrei Mouravski, 2013/06/25 18:37:41]

Put the ".then" on the next line. Also, don't call new Path here. Do it inside
of getMirrorSystem.

[janicejl, 2013/06/25 19:47:50]

Done.

           if (mirrorSystem.libraries.values.isEmpty) {
             throw new UnsupportedError("No Library Mirrors.");
           } 
@@ -161,7 +162,7 @@ class Docgen {
    * Analyzes set of libraries and provides a mirror system which can be used 
    * for static inspection of the source code.
    */
-  Future<MirrorSystem> getMirrorSystem(List<Path> libraries,
+  Future<MirrorSystem> getMirrorSystem(List<String> libraries,
         Path libraryRoot, {Path packageRoot}) {

[Andrei Mouravski, 2013/06/25 18:37:41]

Take strings instead of Paths.

[janicejl, 2013/06/25 19:47:50]

Done.

     SourceFileProvider provider = new SourceFileProvider();
     api.DiagnosticHandler diagnosticHandler =
@@ -173,7 +174,7 @@ class Docgen {
     }
     List<Uri> librariesUri = <Uri>[];
     libraries.forEach((library) {
-      librariesUri.add(currentDirectory.resolve(library.toString()));
+      librariesUri.add(currentDirectory.resolve(library));
     });
     return dart2js.analyze(librariesUri, libraryUri, packageUri,
         provider.readStringFromUri, diagnosticHandler,
@@ -190,7 +191,7 @@ class Docgen {
         _currentLibrary = library;
         var result = new Library(library.qualifiedName, _getComment(library),
             _getVariables(library.variables), _getMethods(library.functions),
-            _getClasses(library.classes), nextID);
+            _getClasses(library.classes));
         if (outputToJson) {
           _writeToFile(stringify(result.toMap()), "${result.name}.json");
         } 
@@ -200,12 +201,12 @@ class Docgen {
      }
     });
    }
-  
+
   /// Saves list of libraries for Docgen object.
   void set libraries(value){
     _libraries = value;
   }
-   
+  
   /**
    * Returns any documentation comments associated with a mirror with
    * simple markdown converted to html.
@@ -248,9 +249,9 @@ class Docgen {
     mirrorMap.forEach((String mirrorName, VariableMirror mirror) {
       if (includePrivate || !mirror.isPrivate) {
         _currentMember = mirror;
-        data[mirrorName] = new Variable(mirrorName, mirror.isFinal,
-            mirror.isStatic, mirror.type.toString(), _getComment(mirror), 
-            nextID);
+        data[mirrorName] = new Variable(mirrorName, mirror.qualifiedName, 
+            mirror.isFinal, mirror.isStatic, mirror.type.qualifiedName, 
+            _getComment(mirror));
       }
     });
     return data;
@@ -264,10 +265,10 @@ class Docgen {
     mirrorMap.forEach((String mirrorName, MethodMirror mirror) {
       if (includePrivate || !mirror.isPrivate) {
         _currentMember = mirror;
-        data[mirrorName] = new Method(mirrorName, mirror.isSetter,
-            mirror.isGetter, mirror.isConstructor, mirror.isOperator, 
-            mirror.isStatic, mirror.returnType.toString(), _getComment(mirror),
-            _getParameters(mirror.parameters), nextID);
+        data[mirrorName] = new Method(mirrorName, mirror.qualifiedName, 
+            mirror.isSetter, mirror.isGetter, mirror.isConstructor, 
+            mirror.isOperator, mirror.isStatic, mirror.returnType.qualifiedName, 
+            _getComment(mirror), _getParameters(mirror.parameters));
       }
     });
     return data;
@@ -285,10 +286,10 @@ class Docgen {
             mirror.superclass.qualifiedName : "";
         var interfaces = 
             mirror.superinterfaces.map((interface) => interface.qualifiedName);
-        data[mirrorName] = new Class(mirrorName, superclass, mirror.isAbstract,
-            mirror.isTypedef, _getComment(mirror), interfaces.toList(),
-            _getVariables(mirror.variables), _getMethods(mirror.methods), 
-            nextID);
+        data[mirrorName] = new Class(mirrorName, mirror.qualifiedName, 
+            superclass, mirror.isAbstract, mirror.isTypedef, 
+            _getComment(mirror), interfaces.toList(),
+            _getVariables(mirror.variables), _getMethods(mirror.methods));
       }
     });
     return data;
@@ -302,8 +303,9 @@ class Docgen {
     mirrorList.forEach((ParameterMirror mirror) {
       _currentMember = mirror;
       data[mirror.simpleName] = new Parameter(mirror.simpleName, 
-          mirror.isOptional, mirror.isNamed, mirror.hasDefaultValue, 
-          mirror.type.toString(), mirror.defaultValue, nextID);
+          mirror.qualifiedName, mirror.isOptional, mirror.isNamed, 
+          mirror.hasDefaultValue, mirror.type.qualifiedName, 
+          mirror.defaultValue);
     });
     return data;
   }
@@ -325,9 +327,6 @@ Map recurseMap(Map inputMap) {
  */
 class Library {
   
-  /// Unique ID number for resolving links. 
-  int id;
-  
   /// Documentation comment with converted markdown.
   String comment;
   
@@ -343,12 +342,11 @@ class Library {
   String name;
   
   Library(this.name, this.comment, this.variables, 
-      this.functions, this.classes, this.id);
+      this.functions, this.classes);
   
   /// Generates a map describing the [Library] object.
   Map toMap() {
     var libraryMap = {};
-    libraryMap["id"] = id;
     libraryMap["name"] = name;
     libraryMap["comment"] = comment;
     libraryMap["variables"] = recurseMap(variables);
@@ -364,9 +362,6 @@ class Library {
 // TODO(tmandel): Figure out how to do typedefs (what is needed)
 class Class {
   
-  /// Unique ID number for resolving links. 
-  int id;
-  
   /// Documentation comment with converted markdown.
   String comment;
   
@@ -380,18 +375,19 @@ class Class {
   Map<String, Method> methods;
   
   String name;
+  String qualifiedName;
   String superclass;
   bool isAbstract;
   bool isTypedef;
  
-  Class(this.name, this.superclass, this.isAbstract, this.isTypedef,
-      this.comment, this.interfaces, this.variables, this.methods, this.id);
+  Class(this.name, this.qualifiedName, this.superclass, this.isAbstract, this.isTypedef,
+      this.comment, this.interfaces, this.variables, this.methods);
   
   /// Generates a map describing the [Class] object.
   Map toMap() {
     var classMap = {};
-    classMap["id"] = id;
     classMap["name"] = name;
+    classMap["qualifiedname"] = qualifiedName;
     classMap["comment"] = comment;
     classMap["superclass"] = superclass;
     classMap["abstract"] = isAbstract.toString();
@@ -408,25 +404,23 @@ class Class {
  */
 class Variable {
   
-  /// Unique ID number for resolving links. 
-  int id;
-  
   /// Documentation comment with converted markdown.
   String comment;
   
   String name;
+  String qualifiedName;
   bool isFinal;
   bool isStatic;
   String type;
   
-  Variable(this.name, this.isFinal, this.isStatic, this.type, 
-      this.comment, this.id);
+  Variable(this.name, this.qualifiedName, this.isFinal, this.isStatic, 
+      this.type, this.comment);
   
   /// Generates a map describing the [Variable] object.
   Map toMap() {
     var variableMap = {};
-    variableMap["id"] = id;
     variableMap["name"] = name;
+    variableMap["qualifiedname"] = qualifiedName;
     variableMap["comment"] = comment;
     variableMap["final"] = isFinal.toString();
     variableMap["static"] = isStatic.toString();
@@ -440,9 +434,6 @@ class Variable {
  */
 class Method {
   
-  /// Unique ID number for resolving links. 
-  int id;
-  
   /// Documentation comment with converted markdown.
   String comment;
   
@@ -450,6 +441,7 @@ class Method {
   Map<String, Parameter> parameters;
   
   String name;
+  String qualifiedName;
   bool isSetter;
   bool isGetter;
   bool isConstructor;
@@ -457,15 +449,15 @@ class Method {
   bool isStatic;
   String returnType;
   
-  Method(this.name, this.isSetter, this.isGetter, this.isConstructor,
-      this.isOperator, this.isStatic, this.returnType, this.comment,
-      this.parameters, this.id);
+  Method(this.name, this.qualifiedName, this.isSetter, this.isGetter, 
+      this.isConstructor, this.isOperator, this.isStatic, this.returnType, 
+      this.comment, this.parameters);
   
   /// Generates a map describing the [Method] object.
   Map toMap() {
     var methodMap = {};
-    methodMap["id"] = id;
     methodMap["name"] = name;
+    methodMap["qualifiedname"] = qualifiedName;
     methodMap["comment"] = comment;
     methodMap["type"] = isSetter ? "setter" : isGetter ? "getter" :
       isOperator ? "operator" : isConstructor ? "constructor" : "method";
@@ -481,24 +473,22 @@ class Method {
  */
 class Parameter {
   
-  /// Unique ID number for resolving links. 
-  int id;
-  
   String name;
+  String qualifiedName;
   bool isOptional;
   bool isNamed;
   bool hasDefaultValue;
   String type;
   String defaultValue;
   
-  Parameter(this.name, this.isOptional, this.isNamed, this.hasDefaultValue,
-      this.type, this.defaultValue, this.id);
+  Parameter(this.name, this.qualifiedName, this.isOptional, this.isNamed, this.hasDefaultValue,
+      this.type, this.defaultValue);
   
   /// Generates a map describing the [Parameter] object.
   Map toMap() {
     var parameterMap = {};
-    parameterMap["id"] = id;
     parameterMap["name"] = name;
+    parameterMap["qualifiedname"] = qualifiedName;
     parameterMap["optional"] = isOptional.toString();
     parameterMap["named"] = isNamed.toString();
     parameterMap["default"] = hasDefaultValue.toString();