Shorten targets from //path/to/foo:foo to //path/to/foo Improve formatting to remove redundancy that often results from developers copy and pasting pedantic output from "gn check". Bug: 40760278 Change-Id: I068e7121c67790e1347d498ef1aa487873e6ba73 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/19322 Commit-Queue: Takuto Ikuta <tikuta@google.com> Reviewed-by: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/command_format.cc b/src/gn/command_format.cc index 1a6f9ec..c9b9233 100644 --- a/src/gn/command_format.cc +++ b/src/gn/command_format.cc
@@ -163,6 +163,10 @@ // Whether there's a blank separator line at the current position. bool HaveBlankLine(); + // Shorten entries if possible. e.g. Shorten "//path/to/foo:foo" to + // "/path/to/foo". Applies to 'visibility', 'deps', or ends in 'deps'. + void ShortenIfApplicable(const BinaryOpNode* binop); + // Sort a list on the RHS if the LHS is one of the following: // 'sources': sorted alphabetically. // 'deps' or ends in 'deps': sorted such that relative targets are first, @@ -396,6 +400,23 @@ return n > 2 && output_[n - 1] == '\n' && output_[n - 2] == '\n'; } +void Printer::ShortenIfApplicable(const BinaryOpNode* binop) { + const IdentifierNode* ident = binop->left()->AsIdentifier(); + if ((binop->op().value() == "=" || binop->op().value() == "+=" || + binop->op().value() == "-=") && + ident) { + const std::string_view lhs = ident->value().value(); + if (lhs.ends_with("deps") || lhs == "visibility") { + TraverseBinaryOpNode(binop->right(), [](const ParseNode* node) { + const ListNode* list = node->AsList(); + if (list) { + const_cast<ListNode*>(list)->ShortenTargets(); + } + }); + } + } +} + void Printer::SortIfApplicable(const BinaryOpNode* binop) { if (const Comments* comments = binop->comments()) { const std::vector<Token>& before = comments->before(); @@ -757,6 +778,8 @@ } else if (const BinaryOpNode* binop = root->AsBinaryOp()) { CHECK(precedence_.find(binop->op().value()) != precedence_.end()); + // Shorten before sorting, since the shortening may affect the ordering. + ShortenIfApplicable(binop); SortIfApplicable(binop); Precedence prec = precedence_[binop->op().value()];
diff --git a/src/gn/format_test_data/063.golden b/src/gn/format_test_data/063.golden index 8972886..73cfc04 100644 --- a/src/gn/format_test_data/063.golden +++ b/src/gn/format_test_data/063.golden
@@ -5,13 +5,13 @@ ":a", ":b", "a", - "a:a", + "a", "a:b", "a/a", "a/b", "b", "//a", - "//a:a", + "//a", "//a:b", "//a/a", "//a/b", @@ -25,10 +25,10 @@ public_deps += [ ":a", "a", - "a:a", + "a", "a/a", "//a", - "//a:a", + "//a", "//a/a", a, ]
diff --git a/src/gn/parse_tree.cc b/src/gn/parse_tree.cc index feccdb2..b9f04f1 100644 --- a/src/gn/parse_tree.cc +++ b/src/gn/parse_tree.cc
@@ -996,6 +996,14 @@ } } +void ListNode::ShortenTargets() { + for (auto& cur : contents_) { + if (cur->AsLiteral()) { + const_cast<LiteralNode*>(cur->AsLiteral())->ShortenTarget(); + } + } +} + void ListNode::SortAsStringsList() { // Sorts alphabetically. SortList([](const ParseNode* a, const ParseNode* b) { @@ -1147,6 +1155,48 @@ value_.set_location(Location(old.file(), line_number, old.column_number())); } +void LiteralNode::ShortenTarget() { + std::string_view str = value_.value(); + DepsCategory category = GetDepsCategory(str); + if (category != DEPS_CATEGORY_RELATIVE && + category != DEPS_CATEGORY_ABSOLUTE) { + return; + } + + str = str.substr(1, str.length() - 2); // Remove quotes. + + // Slashes may not exist in relative paths. + size_t last_slash = str.rfind('/'); + if (last_slash != std::string::npos) { + str = str.substr(last_slash + 1); + } + + // Split on the last colon. + size_t last_separator = str.rfind(':'); + if (last_separator == std::string::npos) { + return; + } + + std::string_view lhs = str.substr(0, last_separator); + std::string_view rhs = str.substr(last_separator + 1); + if (lhs != rhs) { + return; + } + + // Even if the two sides match, sanity check there are no other colons. + last_separator = lhs.rfind(':'); + if (last_separator != std::string::npos) { + return; + } + + // Accounts for ':' and '"'. + size_t new_length = value_.value().length() - lhs.length() - 2; + shortened_value_ = value_.value().substr(0, new_length); + // Then put the '"' back. + shortened_value_.push_back('"'); + value_ = Token(value_.location(), value_.type(), shortened_value_); +} + // UnaryOpNode ---------------------------------------------------------------- UnaryOpNode::UnaryOpNode() = default;
diff --git a/src/gn/parse_tree.h b/src/gn/parse_tree.h index 2ed8c54..6ad2aec 100644 --- a/src/gn/parse_tree.h +++ b/src/gn/parse_tree.h
@@ -8,6 +8,7 @@ #include <stddef.h> #include <memory> +#include <string> #include <string_view> #include <utility> #include <vector> @@ -467,6 +468,7 @@ return contents_; } + void ShortenTargets(); void SortAsStringsList(); void SortAsTargetsList(); @@ -516,11 +518,13 @@ void set_value(const Token& t) { value_ = t; } void SetNewLocation(int line_number); + void ShortenTarget(); static constexpr const char* kDumpNodeName = "LITERAL"; private: Token value_; + std::string shortened_value_; LiteralNode(const LiteralNode&) = delete; LiteralNode& operator=(const LiteralNode&) = delete;
diff --git a/src/gn/parse_tree_unittest.cc b/src/gn/parse_tree_unittest.cc index 9b684f0..d72e579 100644 --- a/src/gn/parse_tree_unittest.cc +++ b/src/gn/parse_tree_unittest.cc
@@ -6,9 +6,11 @@ #include <stdint.h> +#include <algorithm> #include <memory> #include <utility> +#include "base/containers/span.h" #include "gn/input_file.h" #include "gn/scope.h" #include "gn/test_with_scope.h" @@ -172,6 +174,62 @@ EXPECT_EQ(20, err.location().column_number()); } +TEST(ParseTree, ShortenTargets) { + TestParseInput input( + "deps = [\n" + " \":abc\",\n" + " \"bcd/efg\",\n" + " \"cde/fgh:fgh\",\n" + " \"cde/fgh:fghz\",\n" + " \"//def/ghi\",\n" + " \"//efg/hij:hij\",\n" + " \"//efg/hij:hihihi\",\n" + "]\n"); + EXPECT_FALSE(input.has_error()); + ASSERT_TRUE(input.parsed()->AsBlock()); + ASSERT_TRUE(input.parsed()->AsBlock()->statements()[0]->AsBinaryOp()); + const BinaryOpNode* binop = + input.parsed()->AsBlock()->statements()[0]->AsBinaryOp(); + ASSERT_TRUE(binop->right()->AsList()); + auto* list = const_cast<ListNode*>(binop->right()->AsList()); + const auto& contents = list->contents(); + ASSERT_EQ(7u, contents.size()); + + auto all_elements_are_literal_nodes = + [](base::span<const std::unique_ptr<const ParseNode>> container) -> bool { + // TODO(thestig): Switch to std::ranges::all_of() when the CI/CQ bots all + // support it. + return std::all_of(container.begin(), container.end(), + [](const std::unique_ptr<const ParseNode>& element) { + return element->AsLiteral(); + }); + }; + + auto get_literal_value = [](const ParseNode& node) { + return node.AsLiteral()->value().value(); + }; + + ASSERT_TRUE(all_elements_are_literal_nodes(contents)); + EXPECT_EQ("\":abc\"", get_literal_value(*contents[0])); + EXPECT_EQ("\"bcd/efg\"", get_literal_value(*contents[1])); + EXPECT_EQ("\"cde/fgh:fgh\"", get_literal_value(*contents[2])); + EXPECT_EQ("\"cde/fgh:fghz\"", get_literal_value(*contents[3])); + EXPECT_EQ("\"//def/ghi\"", get_literal_value(*contents[4])); + EXPECT_EQ("\"//efg/hij:hij\"", get_literal_value(*contents[5])); + EXPECT_EQ("\"//efg/hij:hihihi\"", get_literal_value(*contents[6])); + + list->ShortenTargets(); + + ASSERT_TRUE(all_elements_are_literal_nodes(contents)); + EXPECT_EQ("\":abc\"", get_literal_value(*contents[0])); + EXPECT_EQ("\"bcd/efg\"", get_literal_value(*contents[1])); + EXPECT_EQ("\"cde/fgh\"", get_literal_value(*contents[2])); + EXPECT_EQ("\"cde/fgh:fghz\"", get_literal_value(*contents[3])); + EXPECT_EQ("\"//def/ghi\"", get_literal_value(*contents[4])); + EXPECT_EQ("\"//efg/hij\"", get_literal_value(*contents[5])); + EXPECT_EQ("\"//efg/hij:hihihi\"", get_literal_value(*contents[6])); +} + TEST(ParseTree, SortRangeExtraction) { TestWithScope setup;