commit e9fc92ac17deefc369690f412dc32f1b23bf0ce3
author Lei Zhang <thestig@chromium.org> Thu Jul 24 11:34:35 2025 -0700
committer GN LUCI <gn-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jul 24 19:42:16 2025 -0700
tree 49e62608fa7c7ddace3b894045f18c17efc3aa9d
parent 05e031634869be95570643bfad014c6dcbfc1a5c

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>

src/gn/command_format.cc

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) {