Refactor command_format.cc - Add IsAssignment() and IsTargetsList() helper functions to consolidate some repeated code. - Reorder conditionals to do the cheaper checks first. - Use early returns in some cases. Change-Id: I277665e3c013cd945c38ee82f23f51995ee1d8fb Reviewed-on: https://gn-review.googlesource.com/c/gn/+/19400 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/command_format.cc b/src/gn/command_format.cc index c9b9233..a509eed 100644 --- a/src/gn/command_format.cc +++ b/src/gn/command_format.cc
@@ -116,6 +116,14 @@ .size()); } +bool IsAssignment(std::string_view op) { + return op == "=" || op == "+=" || op == "-="; +} + +bool IsTargetsList(std::string_view ident) { + return ident.ends_with("deps") || ident == "visibility"; +} + class Printer { public: Printer(); @@ -402,19 +410,20 @@ 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(); - } - }); - } + if (!ident || !IsAssignment(binop->op().value())) { + return; } + + if (!IsTargetsList(ident->value().value())) { + return; + } + + TraverseBinaryOpNode(binop->right(), [](const ParseNode* node) { + const ListNode* list = node->AsList(); + if (list) { + const_cast<ListNode*>(list)->ShortenTargets(); + } + }); } void Printer::SortIfApplicable(const BinaryOpNode* binop) { @@ -428,23 +437,23 @@ } } 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("sources") || lhs == "public") { - TraverseBinaryOpNode(binop->right(), [](const ParseNode* node) { - const ListNode* list = node->AsList(); - if (list) - const_cast<ListNode*>(list)->SortAsStringsList(); - }); - } else if (lhs.ends_with("deps") || lhs == "visibility") { - TraverseBinaryOpNode(binop->right(), [](const ParseNode* node) { - const ListNode* list = node->AsList(); - if (list) - const_cast<ListNode*>(list)->SortAsTargetsList(); - }); - } + if (!ident || !IsAssignment(binop->op().value())) { + return; + } + + const std::string_view lhs = ident->value().value(); + if (lhs.ends_with("sources") || lhs == "public") { + TraverseBinaryOpNode(binop->right(), [](const ParseNode* node) { + const ListNode* list = node->AsList(); + if (list) + const_cast<ListNode*>(list)->SortAsStringsList(); + }); + } else if (IsTargetsList(lhs)) { + TraverseBinaryOpNode(binop->right(), [](const ParseNode* node) { + const ListNode* list = node->AsList(); + if (list) + const_cast<ListNode*>(list)->SortAsTargetsList(); + }); } } @@ -806,9 +815,7 @@ int start_line = CurrentLine(); int start_column = CurrentColumn(); - bool is_assignment = binop->op().value() == "=" || - binop->op().value() == "+=" || - binop->op().value() == "-="; + bool is_assignment = IsAssignment(binop->op().value()); int indent_column = start_column; if (is_assignment) {