Implement `gn edit "add attribute value(s)"` Change-Id: I9a327e9f6e71ec2e3587ed25219e145b6a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/25441 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Matt Stark <msta@google.com>
diff --git a/docs/reference.md b/docs/reference.md index 44a5842..33e1c6d 100644 --- a/docs/reference.md +++ b/docs/reference.md
@@ -728,6 +728,13 @@ #### **Commands**: ``` + add <attribute> <value(s)> + Adds <value(s)> to the list attribute <attribute>. + If the attribute does not exist, it is created. + + Example: + gn edit "add deps //base //src/tools:utils" //src/tools:* + delete Deletes the matched targets entirely.
diff --git a/src/gn/build_file_editor.cc b/src/gn/build_file_editor.cc index b8c5721..8e438e2 100644 --- a/src/gn/build_file_editor.cc +++ b/src/gn/build_file_editor.cc
@@ -185,6 +185,23 @@ }); } +std::optional<ListNode*> FindListInAssignment(const TreeNode& assignment) { + auto* op = assignment.AsAssignment(); + if (!op) + return std::nullopt; + auto results = FindExpression<ListNode*>( + assignment.Descend(op->right()), + [](TreeNode& node_ref) -> std::optional<ListNode*> { + if (auto* list = node_ref->AsListMut()) { + return list; + } + return std::nullopt; + }); + if (results.empty()) + return std::nullopt; + return results.front(); +} + TreeNode TreeNode::Descend(ParseNode* child) const { std::vector<ParseNode*> s = stack_; s.push_back(child); @@ -243,11 +260,11 @@ if (is_conditional()) { add_todo(state, target); } else { - RemoveSelf(); + RemoveSelfUnconditionally(); } } -void TreeNode::RemoveSelf() const { +void TreeNode::RemoveSelfUnconditionally() const { DCHECK(parent()); if (auto* block = parent()->AsBlockMut()) { auto& stmts = block->statements();
diff --git a/src/gn/build_file_editor.h b/src/gn/build_file_editor.h index dd39702..8ef9037 100644 --- a/src/gn/build_file_editor.h +++ b/src/gn/build_file_editor.h
@@ -56,6 +56,9 @@ // probably be removed. void RemoveSelf(EditState& state, const EditTarget& target) const; + // Removes self from the tree unconditionally without adding TODO comments. + void RemoveSelfUnconditionally() const; + ParseNode* operator->() const { return stack_.back(); } const std::vector<ParseNode*>& stack() const { return stack_; } @@ -63,9 +66,6 @@ TreeNode Descend(ParseNode* child) const; private: - // Low-level deletion from parent block or list. - void RemoveSelf() const; - std::vector<ParseNode*> stack_; }; @@ -119,6 +119,9 @@ const TreeNode& root, const Value& value); +// Finds the first list node within an assignment expression. +std::optional<ListNode*> FindListInAssignment(const TreeNode& assignment); + // Represents a set of patterns within a build file. class LabelMatcher { public:
diff --git a/src/gn/edit_command.cc b/src/gn/edit_command.cc index 7a464c3..ff4d127 100644 --- a/src/gn/edit_command.cc +++ b/src/gn/edit_command.cc
@@ -35,6 +35,13 @@ " in your build files instructing you what to do.\n" "\n" "Commands:\n" + " add <attribute> <value(s)>\n" + " Adds <value(s)> to the list attribute <attribute>.\n" + " If the attribute does not exist, it is created.\n" + "\n" + " Example:\n" + " gn edit \"add deps //base //src/tools:utils\" //src/tools:*\n" + "\n" " delete\n" " Deletes the matched targets entirely.\n" "\n"
diff --git a/src/gn/edit_command_unittest.cc b/src/gn/edit_command_unittest.cc index 25c0948..0ec3cfc 100644 --- a/src/gn/edit_command_unittest.cc +++ b/src/gn/edit_command_unittest.cc
@@ -125,6 +125,94 @@ "Target(s) not found: //:nonexistent"); } +TEST_F(EditCommandTest, AddSubcommand) { + EXPECT_SUCCESS(DoEdit("add deps //add1 //add2 //add3 :dep2", + R"( +executable("foo") { + deps = [ "//dep1" ] + deps += [ "//:dep2" ] + if (is_linux) { + deps += [ "//add3" ] + } +} +)"), + Edited(R"( +executable("foo") { + deps = [ + "//add1", + "//add2", + "//add3", + "//dep1", + ] + deps += [ "//:dep2" ] + if (is_linux) { + deps += [] + } +} +)")); + + // Adding to a target where attribute is not defined should it at the end + EXPECT_SUCCESS(DoEdit("add deps //base", + R"( +executable("foo") { + sources = [ "foo.cc" ] +} +)"), + Edited(R"( +executable("foo") { + sources = [ "foo.cc" ] + deps = [ "//base" ] +} +)")); + + // Attribute defined only conditionally should hoist to start of block and + // convert = to += + EXPECT_SUCCESS(DoEdit("add deps //base", + R"( +executable("foo") { + if (is_linux) { + deps = [ "//dep" ] + } +} +)"), + Edited(R"( +executable("foo") { + deps = [ "//base" ] + + if (is_linux) { + deps += [ "//dep" ] + } +} +)")); + + EXPECT_SUCCESS(DoEdit("add deps //base", + R"( +executable("foo") { + deps = foo + bar + [ "//baz" ] +} +)"), + Edited(R"( +executable("foo") { + deps = foo + bar + [ + "//base", + "//baz", + ] +} +)")); + + EXPECT_SUCCESS(DoEdit("add deps //base", + R"( +executable("foo") { + deps = other_deps +} +)"), + Edited(R"( +executable("foo") { + deps = [ "//base" ] + other_deps +} +)")); +} + TEST_F(EditCommandTest, DeleteSubcommand) { EXPECT_SUCCESS(DoEdit("delete", {"//:bar"}, R"(
diff --git a/src/gn/edit_subcommands.cc b/src/gn/edit_subcommands.cc index 26a7dbb..9be56c8 100644 --- a/src/gn/edit_subcommands.cc +++ b/src/gn/edit_subcommands.cc
@@ -43,9 +43,10 @@ return list_elements; } -const TreeNode* FirstAssignment(const std::vector<TreeNode>& assignments) { +const TreeNode* FirstUnconditionalAssignment( + const std::vector<TreeNode>& assignments) { for (const auto& assignment : assignments) { - if (!assignment.is_conditional() && !assignment.is_modification()) + if (!assignment.is_conditional() && assignment.AsAssignment()) return &assignment; } return nullptr; @@ -87,6 +88,94 @@ return done; } +void AddToTarget(BuildFile& build_file, + const EditTarget& target, + const std::string& attribute, + const std::vector<Value>& values) { + auto assignments = target.assignments(attribute); + std::vector<Value> to_add = values; + + // Iterate over a copy of values since we're mutating it. + for (const auto& value : values) { + for (auto& assignment : assignments) { + auto matches = FindListElementInAssignment(target, assignment, value); + for (const auto& match : matches) { + if (assignment.is_conditional()) { + // If it's assigned conditionally, remove it from the list first, + // since we're going to assign it unconditionally. + // Unlike usual we don't mark this with a comment, because this is + // safe. + match.RemoveSelfUnconditionally(); + } else { + // If it's added unconditionally, we don't need to worry about + // adding it anymore. + std::erase(to_add, value); + } + } + } + } + + if (const auto* first = FirstUnconditionalAssignment(assignments); first) { + // Case A: There exists an unconditional assignment -> add values to it. + ListNode* target_list = nullptr; + if (auto list = FindListInAssignment(*first)) { + // The expression is something like `[ "a" ]` or `foo + [ "a" ]` + // In this case we just add directly to the first list "literal" we + // find. + target_list = *list; + } else { + // The expression doesn't have a list literal (eg. `foo`) + // Rewrite it as `[] + foo` so we can add to the empty list. + auto* op = first->AsAssignment(); + auto empty_list_val = + build_file.to_node(Value(nullptr, std::vector<Value>{})); + target_list = empty_list_val->AsListMut(); + + auto plus_node = std::make_unique<BinaryOpNode>(); + plus_node->set_op(Token(build_file.location(), Token::PLUS, "+")); + plus_node->set_left(std::move(empty_list_val)); + plus_node->set_right(op->take_right()); + + op->set_right(std::move(plus_node)); + } + + for (const auto& value : to_add) { + target_list->append_item(build_file.to_node(value)); + } + } else if (!assignments.empty()) { + // Case B: attr is only defined conditionally -> add attr = [value] at the + // start of the block, change all other assignments to "+=". + for (auto& assignment : assignments) { + if (auto* op = assignment->AsBinaryOpMut()) { + if (op->op().type() == Token::EQUAL) { + op->set_op(Token(op->op().location(), Token::PLUS_EQUALS, "+=")); + } + } + } + target.block->statements().insert( + target.block->statements().begin(), + build_file.create_assignment( + attribute, + build_file.to_node(Value(nullptr, std::vector<Value>(to_add))))); + } else { + // Case C: attr is not defined -> add attr = [value] at the end of the + // block. + target.block->append_statement(build_file.create_assignment( + attribute, + build_file.to_node(Value(nullptr, std::vector<Value>(to_add))))); + } +} + +EditCommand AddToAttributeCommand(std::string attribute, + std::vector<Value> values) { + return EditTargetCommand( + [attribute = std::move(attribute), values = std::move(values)]( + BuildFile& build_file, const EditTarget& target, + EditState& state) -> Err { + AddToTarget(build_file, target, attribute, values); + return Ok(); + }); +} EditCommand DeleteCommand() { return EditTargetCommand([](BuildFile& build_file, const EditTarget& target, EditState& state) -> Err { @@ -129,7 +218,7 @@ return EditTargetCommand([=](BuildFile& build_file, const EditTarget& target, EditState& state) -> Err { auto assignments = target.assignments(attribute); - const auto* first = FirstAssignment(assignments); + const auto* first = FirstUnconditionalAssignment(assignments); for (const auto& assignment : assignments) { if (&assignment != first) { assignment.RemoveSelf(state, target); @@ -154,7 +243,15 @@ return Err(Location(), "Empty command."); } - if (args[0] == "delete") { + if (args[0] == "add") { + if (args.size() < 3) { + return Err(Location(), "Invalid add command.", + "Usage: add <attribute> <value(s)>"); + } + ASSIGN_OR_RETURN(std::vector<Value> values, + ParseValues(base::make_span(args).subspan(2))); + return AddToAttributeCommand(args[1], std::move(values)); + } else if (args[0] == "delete") { if (args.size() != 1) { return Err(Location(), "Invalid delete command.", "Usage: delete"); }
diff --git a/src/gn/parse_tree.h b/src/gn/parse_tree.h index f30c59e..47a7b43 100644 --- a/src/gn/parse_tree.h +++ b/src/gn/parse_tree.h
@@ -280,6 +280,8 @@ void set_right(std::unique_ptr<ParseNode> right) { right_ = std::move(right); } + std::unique_ptr<ParseNode> take_left() { return std::move(left_); } + std::unique_ptr<ParseNode> take_right() { return std::move(right_); } static constexpr const char* kDumpNodeName = "BINARY";