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(); }
