patch_sdk: fix part/import order

tool/patch_sdk.dart

 #!/usr/bin/env dart
 // Copyright (c) 2015, 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.
 
 /// Command line tool to merge the SDK libraries and our patch files.
 /// This is currently designed as an offline tool, but we could automate it.
 library dev_compiler.tool.patch_sdk;
 
 import 'dart:io';
@@ skipping 141 matching lines @@
 
   bool _isLibrary = true; // until proven otherwise.
 
   PatchApplier(this.edits, this.patch);
 
   @override visitCompilationUnit(CompilationUnit node) {
     super.visitCompilationUnit(node);
     if (_isLibrary) _mergeUnpatched(node);
   }
 
+  void _merge(AstNode node, int pos) {
+    var code = patch.contents.substring(node.offset, node.end);
+    edits.insert(pos, '\n' + code);
+  }
+
   /// Merges directives and declarations that are not `@patch` into the library.
   void _mergeUnpatched(CompilationUnit unit) {
-    // Merge directives from the patch
+
+    // Merge imports from the patch
     // TODO(jmesserly): remove duplicate imports
-    var directivePos = unit.directives.last.end;
-    for (var directive in patch.unit.directives) {
-      var code = patch.contents.substring(directive.offset, directive.end);
-      edits.insert(directivePos, '\n' + code);
+
+    int partPos = unit.directives.last.end;

[Leaf, 2015/04/23 21:32:55]

I think directives can be null if there are no directives?  Not sure if it can
be non-null and empty.

[Jennifer Messerly, 2015/04/23 21:44:28]

maybe, yeah. That can only happen for "scripts" though (the entry point).
Everyone else needs at least a library directive.

+    var lastImport = unit.directives.lastWhere((d) => d is ImportDirective,
+        orElse: () => null);
+    int importPos = lastImport != null ? lastImport.end : partPos;

[Leaf, 2015/04/23 21:32:55]

Doesn't this still put the new imports after the parts if there no imports in
the original file?

[Jennifer Messerly, 2015/04/23 21:44:28]

Good catch! I'll have it go after the library directive instead.

+
+    for (var d in patch.unit.directives.where((d) => d is ImportDirective)) {
+      _merge(d, importPos);
+    }
+    for (var d in patch.unit.directives.where((d) => d is PartDirective)) {
+      _merge(d, partPos);
     }
 
     // Merge declarations from the patch
-    var declarationPos = edits.original.length;
-    for (var declaration in patch.mergeDeclarations) {
-      var code = patch.contents.substring(declaration.offset, declaration.end);
-      edits.insert(declarationPos, '\n' + code);
+    int declPos = edits.original.length;
+    for (var d in patch.mergeDeclarations) {
+      _merge(d, declPos);
     }
   }
 
   @override visitPartOfDirective(PartOfDirective node) {
     _isLibrary = false;
   }
 
   @override visitFunctionDeclaration(FunctionDeclaration node) {
     _maybePatch(node);
   }
@@ skipping 190 matching lines @@
     if (diff != 0) return diff;
     return end - other.end;
   }
 }
 
 List<SdkLibrary> _getSdkLibraries(String contents) {
   var libraryBuilder = new SdkLibrariesReader_LibraryBuilder(true);
   parseCompilationUnit(contents).accept(libraryBuilder);
   return libraryBuilder.librariesMap.sdkLibraries;
 }