From cb3989a1d05c99ba0fdd1f07ec58cd7d9b695a2a Mon Sep 17 00:00:00 2001
From: Nick Glorioso <glorioso@google.com>
Date: Tue, 26 Nov 2024 12:18:10 -0800
Subject: [PATCH] avoid multiple charAt calls for injectedLogSite equals()

RELNOTES=n/a
PiperOrigin-RevId: 700422386
---
 api/BUILD                                     |  1 +
 .../com/google/common/flogger/LogSite.java    | 17 +++--
 .../google/common/flogger/LogSiteTest.java    | 64 +++++++++++++++++++
 3 files changed, 73 insertions(+), 9 deletions(-)
 create mode 100644 api/src/test/java/com/google/common/flogger/LogSiteTest.java

diff --git a/api/BUILD b/api/BUILD
index c19d3a5..8620084 100644
--- a/api/BUILD
+++ b/api/BUILD
@@ -262,6 +262,7 @@ gen_java_tests(
         ":testing",
         "@google_bazel_common//third_party/java/auto:service",
         "@google_bazel_common//third_party/java/guava",
+        "@google_bazel_common//third_party/java/guava:testlib",
         "@google_bazel_common//third_party/java/jspecify_annotations",
         "@google_bazel_common//third_party/java/junit",
         "@google_bazel_common//third_party/java/mockito",
diff --git a/api/src/main/java/com/google/common/flogger/LogSite.java b/api/src/main/java/com/google/common/flogger/LogSite.java
index e412858..ae29f2f 100644
--- a/api/src/main/java/com/google/common/flogger/LogSite.java
+++ b/api/src/main/java/com/google/common/flogger/LogSite.java
@@ -223,16 +223,15 @@ private static boolean internalNamesCompatible(String s1, String s2) {
       }
 
       for (int i = 0; i < s1.length(); i++) {
-        if (s1.charAt(i) == s2.charAt(i)) {
-          continue;
+        char c1 = s1.charAt(i);
+        char c2 = s2.charAt(i);
+        if (c1 != c2) {
+          // If the characters are not equal, but the slash/dot difference is accounted for, then
+          // consider them equivalent.
+          if (!(c1 == '/' && c2 == '.') && !(c1 == '.' && c2 == '/')) {
+            return false;
+          }
         }
-        if (s1.charAt(i) == '/' && s2.charAt(i) == '.') {
-          continue;
-        }
-        if (s1.charAt(i) == '.' && s2.charAt(i) == '/') {
-          continue;
-        }
-        return false;
       }
       return true;
     }
diff --git a/api/src/test/java/com/google/common/flogger/LogSiteTest.java b/api/src/test/java/com/google/common/flogger/LogSiteTest.java
new file mode 100644
index 0000000..142a6a8
--- /dev/null
+++ b/api/src/test/java/com/google/common/flogger/LogSiteTest.java
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2024 The Flogger Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.common.flogger;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.testing.EqualsTester;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public final class LogSiteTest {
+
+  @Test
+  public void injectedLogSite_internalVsExternalName() {
+    String className = "com.google.common.flogger.LogSiteTest";
+    String internalClassName = "com/google/common/flogger/LogSiteTest";
+
+    // Using the same class, but represented by the internal vs external name, should
+    // be considered equal.
+    new EqualsTester()
+        .addEqualityGroup(logSite(className), logSite(internalClassName))
+        .addEqualityGroup(logSite(className + "2"), logSite(internalClassName + "2"))
+        .addEqualityGroup(logSite(className + "3"), logSite(internalClassName + "3"))
+        .testEquals();
+
+    // The public "className()" method will be the same
+    assertThat(logSite(className).getClassName()).isEqualTo(className);
+    assertThat(logSite(internalClassName).getClassName()).isEqualTo(className);
+  }
+
+  // This technically passes, but is not what we want, since the botched name isn't a real class
+  // identifier. The complexity of tracking the separator is not worth it, however.
+  @Test
+  public void injectedLogSite_invalidClassName_stillValid() {
+    String className = "com.google.common.flogger.LogSiteTest";
+    String internalClassName = "com/google/common/flogger/LogSiteTest";
+    String botchedClassName = "com.google/common.flogger/LogSiteTest";
+
+    new EqualsTester()
+        .addEqualityGroup(logSite(className), logSite(internalClassName), logSite(botchedClassName))
+        .testEquals();
+  }
+
+  @SuppressWarnings("deprecation") // Intentionally calling injectedLogSite to test it.
+  private static LogSite logSite(String className) {
+    return LogSite.injectedLogSite(className, "someMethod", 42, "MyFile.java");
+  }
+}