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;
