Sort import() lines by group Test: Test cases 071.gn 072.gn 073.gn added Change-Id: I70417520e4534f65bd827e8cfe56d97b3d9f68ca Reviewed-on: https://gn-review.googlesource.com/c/2980 Reviewed-by: Brett Wilson <brettw@google.com> Commit-Queue: Scott Graham <scottmg@chromium.org>

diff --git a/tools/gn/command_format.cc b/tools/gn/command_format.cc
index 3b7a54e..d656dd8 100644
--- a/tools/gn/command_format.cc
+++ b/tools/gn/command_format.cc

@@ -111,7 +111,6 @@
   // Format a list of values using the given style.
   enum SequenceStyle {
     kSequenceStyleList,
-    kSequenceStyleBlock,
     kSequenceStyleBracedBlock,
   };
 
@@ -148,6 +147,11 @@
   // (both sorted alphabetically).
   void SortIfSourcesOrDeps(const BinaryOpNode* binop);
 
+  // Sort contiguous import() function calls in the given ordered list of
+  // statements (the body of a block or scope).
+  template <class PARSENODE>
+  void SortImports(std::vector<std::unique_ptr<PARSENODE>>& statements);
+
   // Heuristics to decide if there should be a blank line added between two
   // items. For various "small" items, it doesn't look nice if there's too much
   // vertical whitespace added.
@@ -343,6 +347,90 @@
   }
 }
 
+template <class PARSENODE>
+void Printer::SortImports(std::vector<std::unique_ptr<PARSENODE>>& statements) {
+  // Build a set of ranges by indices of FunctionCallNode's that are imports.
+
+  std::vector<std::vector<size_t>> import_statements;
+
+  auto is_import = [](const PARSENODE* p) {
+    const FunctionCallNode* func_call = p->AsFunctionCall();
+    return func_call && func_call->function().value() == "import";
+  };
+
+  std::vector<size_t> current_group;
+  for (size_t i = 0; i < statements.size(); ++i) {
+    if (is_import(statements[i].get())) {
+      if (i > 0 && (!is_import(statements[i - 1].get()) ||
+                    ShouldAddBlankLineInBetween(statements[i - 1].get(),
+                                                statements[i].get()))) {
+        if (!current_group.empty()) {
+          import_statements.push_back(current_group);
+          current_group.clear();
+        }
+      }
+      current_group.push_back(i);
+    }
+  }
+
+  if (!current_group.empty())
+    import_statements.push_back(current_group);
+
+  struct CompareByImportFile {
+    bool operator()(const std::unique_ptr<PARSENODE>& a,
+                    const std::unique_ptr<PARSENODE>& b) const {
+      const auto& a_args = a->AsFunctionCall()->args()->contents();
+      const auto& b_args = b->AsFunctionCall()->args()->contents();
+      base::StringPiece a_name;
+      base::StringPiece b_name;
+      if (!a_args.empty())
+        a_name = a_args[0]->AsLiteral()->value().value();
+      if (!b_args.empty())
+        b_name = b_args[0]->AsLiteral()->value().value();
+
+      auto is_absolute = [](base::StringPiece import) {
+        return import.size() >= 3 && import[0] == '"' && import[1] == '/' &&
+               import[2] == '/';
+      };
+      int a_is_rel = !is_absolute(a_name);
+      int b_is_rel = !is_absolute(b_name);
+
+      return std::tie(a_is_rel, a_name) < std::tie(b_is_rel, b_name);
+    }
+  };
+
+  int line_after_previous = -1;
+
+  for (const auto& group : import_statements) {
+    size_t begin = group[0];
+    size_t end = group.back() + 1;
+
+    // Save the original line number so that ranges can be re-assigned. They're
+    // contiguous because of the partitioning code above. Later formatting
+    // relies on correct line number to know whether to insert blank lines,
+    // which is why these need to be fixed up. Additionally, to handle multiple
+    // imports on one line, they're assigned sequential line numbers, and
+    // subsequent blocks will be gapped from them.
+    int start_line =
+        std::max(statements[begin]->GetRange().begin().line_number(),
+                 line_after_previous + 1);
+
+    std::sort(statements.begin() + begin, statements.begin() + end,
+              CompareByImportFile());
+
+    const PARSENODE* prev = nullptr;
+    for (size_t i = begin; i < end; ++i) {
+      const PARSENODE* node = statements[i].get();
+      int line_number =
+          prev ? prev->GetRange().end().line_number() + 1 : start_line;
+      const_cast<FunctionCallNode*>(node->AsFunctionCall())
+          ->SetNewLocation(line_number);
+      prev = node;
+      line_after_previous = line_number + 1;
+    }
+  }
+}
+
 bool Printer::ShouldAddBlankLineInBetween(const ParseNode* a,
                                           const ParseNode* b) {
   LocationRange a_range = a->GetRange();
@@ -382,6 +470,9 @@
     }
   }
 
+  SortImports(const_cast<std::vector<std::unique_ptr<ParseNode>>&>(
+      block->statements()));
+
   size_t i = 0;
   for (const auto& stmt : block->statements()) {
     Expr(stmt.get(), kPrecedenceLowest, std::string());
@@ -692,8 +783,10 @@
   else if (style == kSequenceStyleBracedBlock)
     Print("{");
 
-  if (style == kSequenceStyleBlock || style == kSequenceStyleBracedBlock)
+  if (style == kSequenceStyleBracedBlock) {
     force_multiline = true;
+    SortImports(const_cast<std::vector<std::unique_ptr<PARSENODE>>&>(list));
+  }
 
   force_multiline |= ListWillBeMultiline(list, end);
 

diff --git a/tools/gn/command_format_unittest.cc b/tools/gn/command_format_unittest.cc
index cd69e5d..db63a8c 100644
--- a/tools/gn/command_format_unittest.cc
+++ b/tools/gn/command_format_unittest.cc

@@ -106,3 +106,6 @@
 FORMAT_TEST(068)
 FORMAT_TEST(069)
 FORMAT_TEST(070)
+FORMAT_TEST(071)
+FORMAT_TEST(072)
+FORMAT_TEST(073)

diff --git a/tools/gn/format_test_data/028.golden b/tools/gn/format_test_data/028.golden
index a1d54c5..9506896 100644
--- a/tools/gn/format_test_data/028.golden
+++ b/tools/gn/format_test_data/028.golden

@@ -1,6 +1,6 @@
 # Don't separate these.
-import("wee.gni")
 import("waa.gni")
+import("wee.gni")
 
 import("woo.gni")
 

diff --git a/tools/gn/format_test_data/071.gn b/tools/gn/format_test_data/071.gn
new file mode 100644
index 0000000..665d099
--- /dev/null
+++ b/tools/gn/format_test_data/071.gn

@@ -0,0 +1,26 @@
+# Copyright 2018 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import("z.gni")
+import("x.gni")
+import("y.gni")
+
+import("b.gni")
+import("a.gni")
+
+
+
+import("m.gni")
+import("d1.gni")
+# Comment here
+import("c1.gni")
+
+import("../something/relative.gni")
+import("//build/stuff.gni")
+import("nopath.gni")
+import("//abc/things.gni")
+
+import("")
+import()
+import("a")

diff --git a/tools/gn/format_test_data/071.golden b/tools/gn/format_test_data/071.golden
new file mode 100644
index 0000000..6d8f98f
--- /dev/null
+++ b/tools/gn/format_test_data/071.golden

@@ -0,0 +1,25 @@
+# Copyright 2018 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import("x.gni")
+import("y.gni")
+import("z.gni")
+
+import("a.gni")
+import("b.gni")
+
+import("d1.gni")
+import("m.gni")
+
+# Comment here
+import("c1.gni")
+
+import("//abc/things.gni")
+import("//build/stuff.gni")
+import("../something/relative.gni")
+import("nopath.gni")
+
+import()
+import("")
+import("a")

diff --git a/tools/gn/format_test_data/072.gn b/tools/gn/format_test_data/072.gn
new file mode 100644
index 0000000..10b5a81
--- /dev/null
+++ b/tools/gn/format_test_data/072.gn

@@ -0,0 +1,8 @@
+import("b") import("c") import("a") import("d")
+
+import("z") declare_args() {} import("y") import("x") import("w")
+
+import("3") import("2") import("1")
+
+import("x") import("y")
+import("z") import("w")

diff --git a/tools/gn/format_test_data/072.golden b/tools/gn/format_test_data/072.golden
new file mode 100644
index 0000000..b4e5a4b
--- /dev/null
+++ b/tools/gn/format_test_data/072.golden

@@ -0,0 +1,21 @@
+import("a")
+import("b")
+import("c")
+import("d")
+
+import("z")
+declare_args() {
+}
+
+import("w")
+import("x")
+import("y")
+
+import("1")
+import("2")
+import("3")
+
+import("w")
+import("x")
+import("y")
+import("z")

diff --git a/tools/gn/format_test_data/073.gn b/tools/gn/format_test_data/073.gn
new file mode 100644
index 0000000..e3a6561
--- /dev/null
+++ b/tools/gn/format_test_data/073.gn

@@ -0,0 +1,23 @@
+import("//root/a")
+import("//root/c")
+import("//root/b")
+
+if (stuff) {
+import("3") import("2") import("1")
+  if (things) {
+      import("x")
+      import("z") import("y") import("w") import("q")
+
+      import("f") import("e") if (other) {import("z") import("a")} import("d")
+
+      template("wee") {
+        import("6")
+        import("7")
+        import("5")
+      }
+  }
+} else {
+  import("i")
+  import("h")
+  import("g")
+}

diff --git a/tools/gn/format_test_data/073.golden b/tools/gn/format_test_data/073.golden
new file mode 100644
index 0000000..e32977b
--- /dev/null
+++ b/tools/gn/format_test_data/073.golden

@@ -0,0 +1,34 @@
+import("//root/a")
+import("//root/b")
+import("//root/c")
+
+if (stuff) {
+  import("1")
+  import("2")
+  import("3")
+  if (things) {
+    import("q")
+    import("w")
+    import("x")
+    import("y")
+    import("z")
+
+    import("e")
+    import("f")
+    if (other) {
+      import("a")
+      import("z")
+    }
+
+    import("d")
+    template("wee") {
+      import("5")
+      import("6")
+      import("7")
+    }
+  }
+} else {
+  import("g")
+  import("h")
+  import("i")
+}

diff --git a/tools/gn/parse_tree.cc b/tools/gn/parse_tree.cc
index e2201d4..6dde01d 100644
--- a/tools/gn/parse_tree.cc
+++ b/tools/gn/parse_tree.cc

@@ -485,6 +485,21 @@
     block_->Print(out, indent + 1);
 }
 
+void FunctionCallNode::SetNewLocation(int line_number) {
+  Location func_old_loc = function_.location();
+  Location func_new_loc =
+      Location(func_old_loc.file(), line_number, func_old_loc.column_number(),
+               func_old_loc.byte());
+  function_.set_location(func_new_loc);
+
+  Location args_old_loc = args_->Begin().location();
+  Location args_new_loc =
+      Location(args_old_loc.file(), line_number, args_old_loc.column_number(),
+               args_old_loc.byte());
+  const_cast<Token&>(args_->Begin()).set_location(args_new_loc);
+  const_cast<Token&>(args_->End()->value()).set_location(args_new_loc);
+}
+
 // IdentifierNode --------------------------------------------------------------
 
 IdentifierNode::IdentifierNode() = default;

diff --git a/tools/gn/parse_tree.h b/tools/gn/parse_tree.h
index 415041e..9927307 100644
--- a/tools/gn/parse_tree.h
+++ b/tools/gn/parse_tree.h

@@ -334,6 +334,8 @@
   const BlockNode* block() const { return block_.get(); }
   void set_block(std::unique_ptr<BlockNode> b) { block_ = std::move(b); }
 
+  void SetNewLocation(int line_number);
+
  private:
   Token function_;
   std::unique_ptr<ListNode> args_;
@@ -385,6 +387,7 @@
   void Print(std::ostream& out, int indent) const override;
 
   void set_begin_token(const Token& t) { begin_token_ = t; }
+  const Token& Begin() const { return begin_token_; }
   void set_end(std::unique_ptr<EndNode> e) { end_ = std::move(e); }
   const EndNode* End() const { return end_.get(); }