Add support for creating new targets in `gn edit` Change-Id: Ie07922d0822ce42b00dac7471c25cc386a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/25581 Commit-Queue: Matt Stark <msta@google.com> Reviewed-by: Takuto Ikuta <tikuta@google.com>
diff --git a/docs/reference.md b/docs/reference.md index c55cef6..994203e 100644 --- a/docs/reference.md +++ b/docs/reference.md
@@ -751,6 +751,14 @@ Example: gn edit "move deps public_deps //base" //src/tools:* + new <rule_kind> [(before|after) <relative_rule_name>] + Adds a new rule at the end of the BUILD file (or before/after + <relative_rule_name>). The rule name is determined by the target label. + + Examples: + gn edit "new source_set" //src/tools:my_target + gn edit "new static_library before old_target" //src/tools:helper + remove <attribute> Removes <attribute> entirely.
diff --git a/src/gn/build_file_editor.cc b/src/gn/build_file_editor.cc index 8e438e2..8fefdae 100644 --- a/src/gn/build_file_editor.cc +++ b/src/gn/build_file_editor.cc
@@ -264,28 +264,28 @@ } } -void TreeNode::RemoveSelfUnconditionally() const { +TreeNode::NodeList& TreeNode::container() const { DCHECK(parent()); if (auto* block = parent()->AsBlockMut()) { - auto& stmts = block->statements(); - for (auto it = stmts.begin(); it != stmts.end(); ++it) { - if (it->get() == node()) { - stmts.erase(it); - return; - } - } + return block->statements(); } else if (auto* list = parent()->AsListMut()) { - auto& items = list->contents(); - for (auto it = items.begin(); it != items.end(); ++it) { - if (it->get() == node()) { - items.erase(it); - return; - } - } + return list->contents(); } else { - CHECK(false) << "Unsupported type to remove from"; + NOTREACHED() << "Unsupported parent type in container"; } - CHECK(false) << "child node not found in parent's children"; +} + +TreeNode::NodeLocation TreeNode::node_location() const { + auto& c = container(); + auto it = std::find_if(c.begin(), c.end(), + [this](const auto& p) { return p.get() == node(); }); + CHECK(it != c.end()) << "child node not found in parent container"; + return {c, it}; +} + +void TreeNode::RemoveSelfUnconditionally() const { + auto [container, it] = node_location(); + container.erase(it); } LabelMatcher::LabelMatcher(SourceDir source_dir, @@ -300,7 +300,10 @@ globbed_ = true; } else if (pattern.type() == LabelPattern::MATCH && pattern.dir() == source_dir_) { - used_[pattern.name()] = false; + if (!used_.contains(pattern.name())) { + explicit_names_.push_back(pattern.name()); + used_[pattern.name()] = false; + } } } } @@ -313,6 +316,18 @@ return globbed_ ? GLOB : NONE; } +Result<std::vector<std::string>> LabelMatcher::explicit_target_names() { + if (globbed_) { + return Err(Location(), + "Explicit target label required (wildcard patterns are not " + "supported for this command)."); + } + for (const auto& name : explicit_names_) { + used_[name] = true; + } + return explicit_names_; +} + Err LabelMatcher::done() const { std::vector<std::string> unused; for (const auto& [name, used] : used_) { @@ -382,23 +397,38 @@ return Location(input_file_.get(), 1, 1); } -std::vector<EditTarget> BuildFile::targets() { +std::vector<EditTarget> BuildFile::targets( + std::function<bool(EditTarget&)> filter) { + if (!filter) { + filter = [this](EditTarget& t) { + switch (label_matcher_.matches(t.label.name())) { + case LabelMatcher::NONE: + return false; + case LabelMatcher::EXACT: + return true; + case LabelMatcher::GLOB: + t.is_explicit = false; + return true; + } + NOTREACHED(); + }; + } return FindStatement<EditTarget>( tree_root_.get(), - [this](TreeNode& node_ref) -> std::optional<EditTarget> { + [this, &filter](TreeNode& node_ref) -> std::optional<EditTarget> { if (auto* func = node_ref->AsFunctionCallMut()) { if (func->block() && func->args() && func->args()->contents().size() == 1) { if (auto name = AsStringLiteral(func->args()->contents()[0].get())) { - auto match_type = label_matcher_.matches(*name); - if (match_type != LabelMatcher::NONE) { - return EditTarget{ - .is_explicit = match_type == LabelMatcher::EXACT, - .label = Label(source_file_.GetDir(), *name), - .node = node_ref, - .block = func->block(), - }; + EditTarget target{ + .is_explicit = true, + .label = Label(source_file_.GetDir(), *name), + .node = node_ref, + .block = func->block(), + }; + if (filter(target)) { + return target; } } } @@ -407,6 +437,16 @@ }); } +std::optional<EditTarget> BuildFile::find_target(std::string_view target_name) { + auto found = targets([target_name](const EditTarget& t) { + return t.label.name() == target_name; + }); + if (!found.empty()) { + return std::move(found.front()); + } + return std::nullopt; +} + std::unique_ptr<ParseNode> BuildFile::to_node(const Value& value) { auto file = std::make_unique<InputFile>(SourceFile("//dummy")); file->SetContents(value.ToString(true)); @@ -443,6 +483,35 @@ return assign; } +std::unique_ptr<BlockNode> BuildFile::create_block( + std::vector<std::unique_ptr<ParseNode>> statements) { + auto block = std::make_unique<BlockNode>(BlockNode::DISCARDS_RESULT); + block->set_begin_token(Token(location(), Token::LEFT_BRACE, "{")); + block->set_end( + std::make_unique<EndNode>(Token(location(), Token::RIGHT_BRACE, "}"))); + block->statements() = std::move(statements); + return block; +} + +std::unique_ptr<FunctionCallNode> BuildFile::create_target( + std::string_view type, + std::string_view name, + std::unique_ptr<BlockNode> block) { + auto func = std::make_unique<FunctionCallNode>(); + func->set_function( + Token(location(), Token::IDENTIFIER, StringAtom(type).str())); + + auto args = std::make_unique<ListNode>(); + args->set_begin_token(Token(location(), Token::LEFT_PAREN, "(")); + args->set_end( + std::make_unique<EndNode>(Token(location(), Token::RIGHT_PAREN, ")"))); + args->append_item(to_node(Value(nullptr, std::string(name)))); + func->set_args(std::move(args)); + + func->set_block(std::move(block)); + return func; +} + Result<bool> BuildFile::Write() { ASSIGN_OR_RETURN(std::string formatted, commands::FormatNodeToString(root())); if (input_file_->contents() == formatted) {
diff --git a/src/gn/build_file_editor.h b/src/gn/build_file_editor.h index 8ef9037..ebc0bde 100644 --- a/src/gn/build_file_editor.h +++ b/src/gn/build_file_editor.h
@@ -56,6 +56,17 @@ // probably be removed. void RemoveSelf(EditState& state, const EditTarget& target) const; + using NodeList = std::vector<std::unique_ptr<ParseNode>>; + using NodeListIterator = NodeList::iterator; + using NodeLocation = std::pair<NodeList&, NodeListIterator>; + + // Returns the parent node's container (statements for BlockNode, contents for + // ListNode). + NodeList& container() const; + + // Returns the parent node's container and an iterator pointing to this node. + NodeLocation node_location() const; + // Removes self from the tree unconditionally without adding TODO comments. void RemoveSelfUnconditionally() const; @@ -140,6 +151,9 @@ // Checks whether a label was a match for a given pattern. MatchType matches(const std::string& name); + // Returns all explicitly named targets for this build file. + Result<std::vector<std::string>> explicit_target_names(); + // Call this when done editing a build file. // Any explicitly requested targets that were unused will trigger an error. Err done() const; @@ -147,6 +161,7 @@ private: SourceDir source_dir_; bool globbed_ = false; + std::vector<std::string> explicit_names_; std::unordered_map<std::string, bool> used_; }; @@ -185,8 +200,13 @@ // This is relevant because generated nodes won't have location information. Location location() const; - // Returns all targets matching the patterns. - std::vector<EditTarget> targets(); + // Returns all targets matching the patterns, or matching the given filter. + std::vector<EditTarget> targets( + std::function<bool(EditTarget&)> filter = nullptr); + + // Finds a target by name in the build file. + // Returns std::nullopt if not found. + std::optional<EditTarget> find_target(std::string_view target_name); // Creates a node to insert into the graph. std::unique_ptr<ParseNode> to_node(const Value& value); @@ -198,6 +218,16 @@ std::string_view name, std::unique_ptr<ParseNode> value); + // Creates a BlockNode `{ ... }` with the given statements. + std::unique_ptr<BlockNode> create_block( + std::vector<std::unique_ptr<ParseNode>> statements = {}); + + // Synthesizes a new target: `<type>("<name>") { ... }` + std::unique_ptr<FunctionCallNode> create_target( + std::string_view type, + std::string_view name, + std::unique_ptr<BlockNode> block); + // Serializes the AST to the build file if it has changed. // Returns Ok(true) if the file was written, Ok(false) if it was unchanged. Result<bool> Write();
diff --git a/src/gn/edit_command.cc b/src/gn/edit_command.cc index d83bdc7..fc0c20e 100644 --- a/src/gn/edit_command.cc +++ b/src/gn/edit_command.cc
@@ -55,6 +55,16 @@ " Example:\n" " gn edit \"move deps public_deps //base\" //src/tools:*\n" "\n" + " new <rule_kind> [(before|after) <relative_rule_name>]\n" + " Adds a new rule at the end of the BUILD file (or before/after\n" + " <relative_rule_name>). The rule name is determined by the target " + "label.\n" + "\n" + " Examples:\n" + " gn edit \"new source_set\" //src/tools:my_target\n" + " gn edit \"new static_library before old_target\" " + "//src/tools:helper\n" + "\n" " remove <attribute>\n" " Removes <attribute> entirely.\n" "\n"
diff --git a/src/gn/edit_command_unittest.cc b/src/gn/edit_command_unittest.cc index e856313..7b27d81 100644 --- a/src/gn/edit_command_unittest.cc +++ b/src/gn/edit_command_unittest.cc
@@ -290,6 +290,110 @@ "\"//nonexistent\" in attribute \"deps\".")}})); } +TEST_F(EditCommandTest, NewSubcommand) { + // Append new target to the end of the file. + EXPECT_SUCCESS(DoEdit("new source_set", {"//:bar"}, + R"( +executable("foo") { + sources = [ "foo.cc" ] +} +)"), + Edited(R"( +executable("foo") { + sources = [ "foo.cc" ] +} +source_set("bar") { +} +)")); + + // Insert before a relative target. + EXPECT_SUCCESS(DoEdit("new source_set before foo", {"//:bar"}, + R"( +executable("foo") { + sources = [ "foo.cc" ] +} +)"), + Edited(R"( +source_set("bar") { +} +executable("foo") { + sources = [ "foo.cc" ] +} +)")); + + // Insert after a relative target. + EXPECT_SUCCESS(DoEdit("new source_set after foo", {"//:bar"}, + R"( +executable("foo") { + sources = [ "foo.cc" ] +} + +executable("baz") { + sources = [ "baz.cc" ] +} +)"), + Edited(R"( +executable("foo") { + sources = [ "foo.cc" ] +} +source_set("bar") { +} + +executable("baz") { + sources = [ "baz.cc" ] +} +)")); + + // Insert multiple new targets after a relative target. + EXPECT_SUCCESS(DoEdit("new source_set after foo", {"//:bar", "//:qux"}, + R"( +executable("foo") { + sources = [ "foo.cc" ] +} + +executable("baz") { + sources = [ "baz.cc" ] +} +)"), + Edited(R"( +executable("foo") { + sources = [ "foo.cc" ] +} +source_set("bar") { +} +source_set("qux") { +} + +executable("baz") { + sources = [ "baz.cc" ] +} +)")); + + // Error when target already exists. + EXPECT_FAILURE(DoEdit("new source_set", {"//:foo"}, + R"( +executable("foo") { + sources = [ "foo.cc" ] +} +)")); + + // Error when relative target not found. + EXPECT_FAILURE(DoEdit("new source_set before nonexistent", {"//:bar"}, + R"( +executable("foo") { + sources = [ "foo.cc" ] +} +)")); + + // Error when no explicit target name specified (e.g. glob pattern). + EXPECT_FAILURE(DoEdit("new source_set", {"//*"}, + R"( +executable("foo") { + sources = [ "foo.cc" ] +} +)")); +} + TEST_F(EditCommandTest, RemoveAttributeSubcommand) { EXPECT_SUCCESS(DoEdit("remove testonly", R"(
diff --git a/src/gn/edit_subcommands.cc b/src/gn/edit_subcommands.cc index 841f9eb..ceddf05 100644 --- a/src/gn/edit_subcommands.cc +++ b/src/gn/edit_subcommands.cc
@@ -205,6 +205,41 @@ }); } +using LocationProvider = + std::function<Result<TreeNode::NodeLocation>(BuildFile&)>; + +EditCommand NewCommand(std::string rule_kind, + LocationProvider location_provider) { + return [rule_kind = std::move(rule_kind), + location_provider = std::move(location_provider)]( + BuildFile& build_file, EditState& state) -> Err { + ASSIGN_OR_RETURN(auto rule_names, + build_file.label_matcher().explicit_target_names()); + DCHECK(!rule_names.empty()); + + ASSIGN_OR_RETURN(auto loc, location_provider(build_file)); + auto& [container, it] = loc; + + for (const auto& rule_name : rule_names) { + if (auto target = build_file.find_target(rule_name); + target && !target->node.is_conditional()) { + return Err(Location(), "Target \"" + rule_name + + "\" already exists in " + + build_file.source_file().value() + "."); + } + + // Reassign and advance the iterator returned by insert() to ensure valid + // iterators across vector reallocations and preserve insertion order + // when adding multiple targets. + it = container.insert( + it, build_file.create_target(rule_kind, rule_name, + build_file.create_block())); + ++it; + } + return Ok(); + }; +} + EditCommand RemoveAttributeCommand(std::string attribute) { return EditTargetCommand([attribute = std::move(attribute)]( BuildFile& build_file, const EditTarget& target, @@ -304,6 +339,36 @@ ASSIGN_OR_RETURN(std::vector<Value> values, ParseValues(base::make_span(args).subspan(3))); return MoveCommand(args[1], args[2], std::move(values)); + } else if (args[0] == "new") { + if (args.size() == 2) { + return NewCommand( + args[1], [](BuildFile& build_file) -> Result<TreeNode::NodeLocation> { + auto* root = build_file.root()->AsBlockMut(); + return std::make_pair(std::ref(root->statements()), + root->statements().end()); + }); + } else if (args.size() == 4 && + (args[2] == "before" || args[2] == "after")) { + return NewCommand( + args[1], + [rel = args[3], after = args[2] == "after"]( + BuildFile& build_file) -> Result<TreeNode::NodeLocation> { + auto target = build_file.find_target(rel); + if (!target) { + return Err(Location(), "Target \"" + rel + "\" not found in " + + build_file.source_file().value() + + "."); + } + auto [container, it] = target->node.node_location(); + if (after) + it++; + return std::make_pair(std::ref(container), it); + }); + } else { + return Err(Location(), "Invalid new command.", + "Usage: new <rule_kind> [(before|after) " + "<relative_rule_name>]"); + } } else if (args[0] == "remove") { if (args.size() < 2) { return Err(Location(), "Invalid remove command.",