Format lists in assignments with binary operations
The content of lists in assignments to the regular variables
sources or deps should be sorted even when another list or
variable is concatenated to it.
This traverses the content of a BinaryOpNode recursively
to repeat the formatting on both sides of the node when
formatting is applicable.
Bug: 298
Change-Id: Ia659f8024e05a546347a4c30f316048176c8067d
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/14180
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Brett Wilson <brettw@google.com>
Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/command_format.cc b/src/gn/command_format.cc
index 6118fa4..041a037 100644
--- a/src/gn/command_format.cc
+++ b/src/gn/command_format.cc
@@ -170,6 +170,10 @@
// 'visibility': same as 'deps'.
void SortIfApplicable(const BinaryOpNode* binop);
+ // Traverse a binary op node tree and apply a callback to each leaf node.
+ void TraverseBinaryOpNode(const ParseNode* node,
+ std::function<void(const ParseNode*)> callback);
+
// Sort contiguous import() function calls in the given ordered list of
// statements (the body of a block or scope).
template <class PARSENODE>
@@ -403,17 +407,37 @@
}
}
const IdentifierNode* ident = binop->left()->AsIdentifier();
- const ListNode* list = binop->right()->AsList();
if ((binop->op().value() == "=" || binop->op().value() == "+=" ||
binop->op().value() == "-=") &&
- ident && list) {
+ ident) {
const std::string_view lhs = ident->value().value();
if (base::EndsWith(lhs, "sources", base::CompareCase::SENSITIVE) ||
- lhs == "public")
- const_cast<ListNode*>(list)->SortAsStringsList();
- else if (base::EndsWith(lhs, "deps", base::CompareCase::SENSITIVE) ||
- lhs == "visibility")
- const_cast<ListNode*>(list)->SortAsTargetsList();
+ lhs == "public") {
+ TraverseBinaryOpNode(binop->right(), [](const ParseNode* node) {
+ const ListNode* list = node->AsList();
+ if (list)
+ const_cast<ListNode*>(list)->SortAsStringsList();
+ });
+ } else if (base::EndsWith(lhs, "deps", base::CompareCase::SENSITIVE) ||
+ lhs == "visibility") {
+ TraverseBinaryOpNode(binop->right(), [](const ParseNode* node) {
+ const ListNode* list = node->AsList();
+ if (list)
+ const_cast<ListNode*>(list)->SortAsTargetsList();
+ });
+ }
+ }
+}
+
+void Printer::TraverseBinaryOpNode(
+ const ParseNode* node,
+ std::function<void(const ParseNode*)> callback) {
+ const BinaryOpNode* binop = node->AsBinaryOp();
+ if (binop) {
+ TraverseBinaryOpNode(binop->left(), callback);
+ TraverseBinaryOpNode(binop->right(), callback);
+ } else {
+ callback(node);
}
}
diff --git a/src/gn/command_format_unittest.cc b/src/gn/command_format_unittest.cc
index 9112dd3..de7e004 100644
--- a/src/gn/command_format_unittest.cc
+++ b/src/gn/command_format_unittest.cc
@@ -136,3 +136,4 @@
FORMAT_TEST(081)
FORMAT_TEST(082)
FORMAT_TEST(083)
+FORMAT_TEST(084)
diff --git a/src/gn/format_test_data/084.gn b/src/gn/format_test_data/084.gn
new file mode 100644
index 0000000..b58b634
--- /dev/null
+++ b/src/gn/format_test_data/084.gn
@@ -0,0 +1,34 @@
+# Checks that lists are also sorted if concatenated during an assignment
+other_sources = [
+ "f.cc",
+ "e.cc"
+] + [
+ "h.cc",
+ "g.cc"
+]
+
+other_sources = [
+ ":f",
+ ":e"
+] + [
+ ":h",
+ "g"
+]
+
+executable("foo") {
+ sources = [
+ "b.cc",
+ "a.cc"
+ ] + [
+ "d.cc",
+ "c.cc"
+ ] + other_sources
+
+ deps = [
+ ":b",
+ ":a"
+ ] + [
+ ":d",
+ ":c"
+ ] + other_sources
+}
diff --git a/src/gn/format_test_data/084.golden b/src/gn/format_test_data/084.golden
new file mode 100644
index 0000000..fa18037
--- /dev/null
+++ b/src/gn/format_test_data/084.golden
@@ -0,0 +1,38 @@
+# Checks that lists are also sorted if concatenated during an assignment
+other_sources = [
+ "e.cc",
+ "f.cc",
+ ] +
+ [
+ "g.cc",
+ "h.cc",
+ ]
+
+other_sources = [
+ ":e",
+ ":f",
+ ] +
+ [
+ ":h",
+ "g",
+ ]
+
+executable("foo") {
+ sources = [
+ "a.cc",
+ "b.cc",
+ ] +
+ [
+ "c.cc",
+ "d.cc",
+ ] + other_sources
+
+ deps = [
+ ":a",
+ ":b",
+ ] +
+ [
+ ":c",
+ ":d",
+ ] + other_sources
+}
