Reorder flow object props (#1451)

This is very sad, but we need to do this to get the correct result :(

Fixes #1448

src/printer.js

@@ -751,7 +751,7 @@ function genericPrintNoParens(path, options, print, args) {
       var rightBrace = n.exact ? "|}" : "}";
       var parent = path.getParentNode(0);
       var parentIsUnionTypeAnnotation = parent.type === "UnionTypeAnnotation";
-      var propertiesField = isTypeScriptType 
+      var propertiesField = isTypeScriptType
         ? "members"
         : "properties";
       var prefix = ""
@@ -759,7 +759,7 @@ function genericPrintNoParens(path, options, print, args) {
       if (isTypeAnnotation) {
         fields.push("indexers", "callProperties");
       }
-      
+
       if (isTypeScriptInterfaceDeclaration) {
         prefix = concat([
           "interface ",
@@ -770,22 +770,34 @@ function genericPrintNoParens(path, options, print, args) {
 
       fields.push(propertiesField);
 
-      var props = [];
-      let separatorParts = [];
-
+      // Unfortunately, things are grouped together in the ast can be
+      // interleaved in the source code. So we need to reorder them before
+      // printing them.
+      const propsAndLoc = [];
       fields.forEach(function(field) {
         path.each(function(childPath) {
-          props.push(concat(separatorParts));
-          props.push(group(print(childPath)));
+          const node = childPath.getValue();
+          propsAndLoc.push({
+            node: node,
+            printed: print(childPath),
+            loc: util.locStart(node)
+          });
+        }, field);
+      });
 
+      let separatorParts = [];
+      const props = propsAndLoc
+        .sort((a, b) => a.loc - b.loc)
+        .map(prop => {
+          const result = concat(separatorParts.concat(group(prop.printed)));
           separatorParts = [separator, line];
           if (
-            util.isNextLineEmpty(options.originalText, childPath.getValue())
+            util.isNextLineEmpty(options.originalText, prop.node)
           ) {
             separatorParts.push(hardline);
           }
-        }, field);
-      });
+          return result;
+        });
 
       const lastElem = util.getLast(n[propertiesField]);
 
@@ -1490,9 +1502,9 @@ function genericPrintNoParens(path, options, print, args) {
 
       var variance = getFlowVariance(n, options);
       if (variance) parts.push(variance);
-      
+
       if (n.accessibility) parts.push(n.accessibility + " ");
- 
+
       if (n.type === "TSAbstractClassProperty") parts.push("abstract ");
 
       if (n.computed) {
@@ -2139,7 +2151,7 @@ function genericPrintNoParens(path, options, print, args) {
         join(", ", path.map(print, "parameters")),
         ")"
       )
-      
+
       if (n.typeAnnotation) {
         parts.push(
           ": ",
@@ -2155,15 +2167,15 @@ function genericPrintNoParens(path, options, print, args) {
         )
       } else {
         parts.push(
-          "export as namespace ", 
+          "export as namespace ",
           path.call(print, "name")
         )
-        
+
         if (options.semi) {
           parts.push(";")
         }
       }
-      
+
       return concat(parts)
     case "TSEnumDeclaration":
       parts.push(
@@ -2171,7 +2183,7 @@ function genericPrintNoParens(path, options, print, args) {
         path.call(print, "name"),
         " "
       )
-      
+
       if (n.members.length === 0) {
         parts.push(
           group(
@@ -2218,11 +2230,11 @@ function genericPrintNoParens(path, options, print, args) {
         " = ",
         path.call(print, "moduleReference")
       )
-      
+
       if (options.semi) {
         parts.push(";")
       }
-      
+
       return concat(parts)
     case "TSExternalModuleReference":
       return concat([
@@ -2244,7 +2256,7 @@ function genericPrintNoParens(path, options, print, args) {
         path.call(print, "body"),
         "}"
       )
-      
+
       return concat(parts)
     case "TSDeclareKeyword":
       return "declare"

tests/flow/constructor_annots/__snapshots__/jsfmt.spec.js.snap

@@ -43,8 +43,8 @@ interface IFooPrototype {
   m: () => number
 }
 interface IFoo extends IFooPrototype {
-  static (): void,
   x: boolean, // error, should have declared x: number instead
+  static (): void,
   static y: boolean // error, should have declared static y: number instead
 }
 exports.Foo2 = (Foo: Class<IFoo>);

tests/flow/dictionary/__snapshots__/jsfmt.spec.js.snap

@@ -764,7 +764,7 @@ function foo6(x: Array<{ foo: number }>): Array<{ [key: string]: number }> {
   return x;
 }
 
-function foo7(x: { [key: string]: number, bar: string }) {
+function foo7(x: { bar: string, [key: string]: number }) {
   (x.bar: string);
 }
 
@@ -842,7 +842,7 @@ var o: { foo: QueryFunction } = {
 
 module.exports = o;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-type Params = { [name: string]: string, count: number };
+type Params = { count: number, [name: string]: string };
 type QueryFunction = (params: Params) => string;
 
 var o: { foo: QueryFunction } = {

tests/flow/new_spread/__snapshots__/jsfmt.spec.js.snap

@@ -305,37 +305,37 @@ declare class T {}
 declare class U {}
 
 declare var o1: { ...{ [string]: T }, ...{ p: U } };
-(o1: { [string]: T, p?: T | U }); // ok
+(o1: { p?: T | U, [string]: T }); // ok
 
 declare var o2: { ...{ p: T }, ...{ [string]: U } };
-(o2: { [string]: U, p?: T | U }); // ok
+(o2: { p?: T | U, [string]: U }); // ok
 
 declare var o3: { ...{ [string]: T }, ...{ [string]: U } };
 (o3: { [string]: T | U }); // ok
 
 declare var o4: { ...{| [string]: T |}, ...{ p: U } };
-(o4: { [string]: T, p?: T | U }); // ok
+(o4: { p?: T | U, [string]: T }); // ok
 
 declare var o5: { ...{| p: T |}, ...{ [string]: U } };
-(o5: { [string]: U, p: T | U }); // ok
+(o5: { p: T | U, [string]: U }); // ok
 
 declare var o6: { ...{| [string]: T |}, ...{ [string]: U } };
 (o6: { [string]: T | U }); // ok
 
 declare var o7: { ...{ [string]: T }, ...{| p: U |} };
-(o7: { [string]: T, p: U }); // ok
+(o7: { p: U, [string]: T }); // ok
 
 declare var o8: { ...{ p: T }, ...{| [string]: U |} };
-(o8: { [string]: U, p?: T | U }); // ok
+(o8: { p?: T | U, [string]: U }); // ok
 
 declare var o9: { ...{ [string]: T }, ...{| [string]: U |} };
 (o9: { [string]: T | U }); // ok
 
 declare var o10: { ...{| [string]: T |}, ...{| p: U |} };
-(o10: {| [string]: T, p: U |}); // ok
+(o10: {| p: U, [string]: T |}); // ok
 
 declare var o11: { ...{| p: T |}, ...{| [string]: U |} };
-(o11: {| [string]: U, p: T | U |}); // ok
+(o11: {| p: T | U, [string]: U |}); // ok
 
 declare var o12: { ...{| [string]: T |}, ...{| [string]: U |} };
 (o12: {| [string]: T | U |}); // ok
@@ -432,12 +432,12 @@ declare var o2: O2;
 
 type O3 = { ...{ p: A } & { [string]: B } };
 declare var o3: O3;
-(o3: { [string]: B, p: B }); // ok: A&B = B
+(o3: { p: B, [string]: B }); // ok: A&B = B
 (o3.q: B); // ok
 
 type O4 = { ...{ [string]: A } & { p: B } };
 declare var o4: O4;
-(o4: { [string]: A, p: B }); // ok: A&B = B
+(o4: { p: B, [string]: A }); // ok: A&B = B
 
 type O5 = { ...{ [string]: A } & { [string]: B } };
 declare var o5: O5;

tests/flow/sealed_objects/__snapshots__/jsfmt.spec.js.snap

@@ -30,18 +30,18 @@ var s2: string = o2.y || ""; // ok
 var o3: { x: number, y?: string } = ({ x: 0, y: 0 }: { x: number });
 var s3: string = o3.y || ""; // error
 
-var o4: { x: number, y?: string } = ({ x: 0 }: { [_: any]: any, x: number });
+var o4: { x: number, y?: string } = ({ x: 0 }: { x: number, [_: any]: any });
 var s4: string = o4.y || ""; // ok
 
 var o5 = { x: 0, ...{} };
 var s5: string = o5.y; // ok (spreads make object types extensible)
 
-var o6: { [_: any]: any, x: number } = { x: 0 };
+var o6: { x: number, [_: any]: any } = { x: 0 };
 var s6: string = o6.y; // ok  (indexers make object types extensible)
 
 var o7: { x: number, y?: string } = ({ x: 0, y: 0 }: {
-  [_: any]: number,
-  x: number
+  x: number,
+  [_: any]: number
 }); // error
 
 `;

tests/flow_object_order/__snapshots__/jsfmt.spec.js.snap

@@ -0,0 +1,59 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`order.js 1`] = `
+type Foo = {
+  // a
+  alpha: 'hello',
+
+  // b
+  [key: string]: void,
+  // c
+
+  beta: 10,
+
+  // d
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+type Foo = {
+  // a
+  alpha: "hello",
+
+  // b
+  [key: string]: void,
+  // c
+
+  beta: 10
+
+  // d
+};
+
+`;
+
+exports[`order.js 2`] = `
+type Foo = {
+  // a
+  alpha: 'hello',
+
+  // b
+  [key: string]: void,
+  // c
+
+  beta: 10,
+
+  // d
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+type Foo = {
+  // a
+  alpha: "hello",
+
+  // b
+  [key: string]: void,
+  // c
+
+  beta: 10,
+
+  // d
+};
+
+`;

tests/flow_object_order/jsfmt.spec.js

@@ -0,0 +1,2 @@
+run_spec(__dirname);
+run_spec(__dirname, {trailingComma: 'all'});

tests/flow_object_order/order.js

@@ -0,0 +1,12 @@
+type Foo = {
+  // a
+  alpha: 'hello',
+
+  // b
+  [key: string]: void,
+  // c
+
+  beta: 10,
+
+  // d
+}